refactor(web): migrate to heroicons, tokenize colors, polish UI (P0+P1)#90
Merged
Conversation
P0 — design tokens: - Add code-block (--code-bg/--code-border) and on-primary/on-destructive foreground tokens, plus a full ANSI terminal palette (--term-bg/fg/cursor/selection + 16 colors) in tokens.css - TerminalInstance.vue resolves terminal colors from tokens at runtime via getComputedStyle (xterm needs color strings, not var()) and guards the ws handlers against a torn-down term - Replace hardcoded colors in style.css (.prose-chat pre, .xterm, .diff-bar-empty) with the new tokens P1 — icon system: - Migrate the entire web/ frontend from lucide-vue-next to @heroicons/vue/24/outline (drop lucide-vue-next, add @heroicons/vue ^2.2.0) - Replace inline <svg> blocks across all components with heroicons; non-1:1 icons use the semantically-closest outline icon - Document the convention (heroicons-only, w-N h-N sizing, color tokens, terminal token resolution) under Frontend in AGENTS.md P1 — UX polish (no layout changes): - Add hover tooltips (with shortcuts where relevant) - Simplify the welcome-screen copy (drop the "Pick a workspace" prompt) - ToolCallCard.truncate() is now code-point aware ([...text]) - ApprovalBanner: two-step confirm on "Allow all", unified icons - SettingsDialog: fix translate-x-4.5/translate-x-0.76 toggle values (-> translate-x-[20px]/translate-x-[2px]); iconFor returns components - App.vue: flex-anchor the scroll-to-bottom button (remove bottom-40) - GoalBanner: Clear button hover; ChatInput: send-btn disabled opacity 0.3 -> 0.45, composer shadow uses --shadow-xl Alongside (pre-existing in-progress work, bundled here): - BLE status channel: GET/POST /api/channel/ble backend (channel.go, server.go) + api.ts client - Persist approval mode as the startup default (server.go) - Stamp turn durationMs on the final assistant message; expose setAutoApprove; drop retryFromMessage (chat.ts, types/api.ts) - Favicon swap icon.svg -> icon.png (index.html, notifications.ts, web/public/) and regenerated desktop Tauri icons Verification: pnpm lint OK, pnpm build OK (vue-tsc + vite), make build OK.
|
Warning Review limit reached
More reviews will be available in 50 minutes and 5 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughThis PR migrates all Vue icons from ChangesIcon Migration, Design Tokens, BLE Channel, and Chat Improvements
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/web/channel.go`:
- Around line 141-144: The error handling in the SaveConfig call is exposing the
raw error to the client via writeJSON, which can leak sensitive file path
information. Instead, log the actual error using config.Logger() for diagnostic
purposes and return a generic, stable error message to the client. Modify the
error handling block after the SaveConfig call to capture the error, log it with
config.Logger(), and then call writeJSON with a user-safe error message that
does not expose internal path details.
- Around line 112-117: The handleChannelBLEStatus function reads from
s.cfg.Channel without synchronization while handleSetChannelBLE writes to it
under the s.mu mutex, creating a race condition. Wrap the read access to
s.cfg.Channel inside a lock acquired from s.mu (similar to how
handleSetChannelBLE protects its write), ensuring the entire block that accesses
s.cfg.Channel and s.cfg is protected by the same mutex to prevent concurrent
read-write races.
In `@internal/web/server.go`:
- Line 2241: The assignment to the deprecated field s.cfg.AutoApprove on line
2241 triggers the SA1019 staticcheck warning. Add a nolint directive comment
(//nolint:sa1019) at the end of the line with the s.cfg.AutoApprove assignment
to suppress this intentional use of the deprecated field while maintaining
compliance with the staticcheck linter configured in .golangci.yml.
In `@web/src/components/SettingsDialog.vue`:
- Line 931: The back buttons in the SettingsDialog component are using
ChevronRightIcon which points in the wrong direction for a back navigation
control. Replace the two occurrences of ChevronRightIcon (at lines 931 and 952)
with ChevronLeftIcon to properly indicate backward navigation. Maintain the
existing class styling (w-3.5 h-3.5) when making this change.
- Around line 765-766: Replace the hardcoded `bg-white` class in the toggle
thumb element (the inline-block element with transition-transform styling that
uses the translate-x binding for store.autoApprove) with a CSS custom property
token from src/styles/tokens.css. The element currently uses `bg-white` for the
background color, which must be replaced with an appropriate token variable to
follow the color-only-from-tokens rule. Also apply the same fix to the similar
toggle element at lines 788-789 that has the same hardcoded white background
color.
- Around line 452-453: The hardcoded hex fallback values in the getComputedStyle
calls for both the fg and bg color variables violate the token-only color
contract. Replace the hardcoded hex fallbacks (the '||' operators with '`#18181b`'
and '`#ffffff`') with references to appropriate CSS custom properties defined in
src/styles/tokens.css. Instead of using hardcoded fallback colors, reference the
actual token CSS variables as fallbacks so all colors come from the centralized
token system.
In `@web/src/components/SetupView.vue`:
- Line 273: The chevron icons used for navigation controls in the SetupView
component are reversed. Forward navigation buttons are currently using
ChevronLeftIcon when they should use ChevronRightIcon, and back buttons are
using ChevronRightIcon when they should use ChevronLeftIcon. Swap the icon
references throughout the component so that forward affordances display
ChevronRightIcon and back affordances display ChevronLeftIcon to provide correct
visual navigation cues. This affects all forward and back button implementations
in the file.
In `@web/src/stores/chat.ts`:
- Around line 268-274: The loop that searches for the assistant message to stamp
durationMs at line 269 iterates through the entire timeline backward, which can
incorrectly apply the duration to an assistant message from a previous turn if
the current turn hasn't produced one yet. Modify the search loop to only search
through messages that belong to the current turn by tracking the starting index
of the current turn in the timeline when turnStartedAt is set, and then bound
the loop to only search from that index forward to the end of the timeline. This
ensures durationMs is only applied to assistant messages that are actually part
of the current turn.
In `@web/src/styles/tokens.css`:
- Around line 118-122: The --term-bright-white terminal color variable has the
same value (`#fafafa`) as the --term-bg background variable, making bright-white
ANSI text invisible in light mode. Change the --term-bright-white value to a
contrasting color that is visually distinct from the `#fafafa` background. Ensure
the new value provides adequate contrast for text readability against the light
terminal background.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 862f160c-6800-4984-8234-a4868b2e8230
⛔ Files ignored due to path filters (52)
desktop/src-tauri/icons/128x128.pngis excluded by!**/*.pngdesktop/src-tauri/icons/128x128@2x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/32x32.pngis excluded by!**/*.pngdesktop/src-tauri/icons/64x64.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square107x107Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square142x142Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square150x150Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square284x284Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square30x30Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square310x310Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square44x44Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square71x71Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square89x89Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/StoreLogo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-hdpi/ic_launcher.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-hdpi/ic_launcher_foreground.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-hdpi/ic_launcher_round.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-mdpi/ic_launcher.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-mdpi/ic_launcher_foreground.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-mdpi/ic_launcher_round.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xhdpi/ic_launcher.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xhdpi/ic_launcher_round.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xxhdpi/ic_launcher.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xxhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xxhdpi/ic_launcher_round.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xxxhdpi/ic_launcher.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xxxhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xxxhdpi/ic_launcher_round.pngis excluded by!**/*.pngdesktop/src-tauri/icons/icon.icois excluded by!**/*.icodesktop/src-tauri/icons/icon.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-20x20@1x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-20x20@2x-1.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-20x20@2x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-20x20@3x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-29x29@1x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-29x29@2x-1.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-29x29@2x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-29x29@3x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-40x40@1x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-40x40@2x-1.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-40x40@2x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-40x40@3x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-512@2x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-60x60@2x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-60x60@3x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-76x76@1x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-76x76@2x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-83.5x83.5@2x.pngis excluded by!**/*.pngweb/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlweb/public/icon.pngis excluded by!**/*.pngweb/public/icon.svgis excluded by!**/*.svg
📒 Files selected for processing (33)
AGENTS.mddesktop/src-tauri/icons/icon.icnsinternal/web/channel.gointernal/web/server.goweb/index.htmlweb/package.jsonweb/src/App.vueweb/src/components/ApprovalBanner.vueweb/src/components/AskUserCard.vueweb/src/components/BranchPicker.vueweb/src/components/ChatInput.vueweb/src/components/ChatMessage.vueweb/src/components/CommandPalette.vueweb/src/components/FileTreePanel.vueweb/src/components/GoalBanner.vueweb/src/components/ProjectSwitcher.vueweb/src/components/RemoteConnectWizard.vueweb/src/components/RightPanel.vueweb/src/components/SettingsDialog.vueweb/src/components/SetupView.vueweb/src/components/Sidebar.vueweb/src/components/TaskList.vueweb/src/components/TerminalInstance.vueweb/src/components/TerminalPanel.vueweb/src/components/ToolCallCard.vueweb/src/components/TopBar.vueweb/src/components/WorkspacePicker.vueweb/src/composables/api.tsweb/src/composables/notifications.tsweb/src/stores/chat.tsweb/src/style.cssweb/src/styles/tokens.cssweb/src/types/api.ts
Comment on lines
+112
to
+117
| func (s *Server) handleChannelBLEStatus(w http.ResponseWriter, r *http.Request) { | ||
| enabled := false | ||
| if s.cfg != nil && s.cfg.Channel != nil { | ||
| enabled = s.cfg.Channel.BLEEnabled | ||
| } | ||
| writeJSON(w, http.StatusOK, map[string]any{"enabled": enabled}) |
There was a problem hiding this comment.
Use the same mutex for BLE status reads.
handleSetChannelBLE writes s.cfg.Channel under s.mu, but this GET path reads the same pointer/field without synchronization, so concurrent requests can race.
Proposed fix
func (s *Server) handleChannelBLEStatus(w http.ResponseWriter, r *http.Request) {
+ s.mu.Lock()
enabled := false
if s.cfg != nil && s.cfg.Channel != nil {
enabled = s.cfg.Channel.BLEEnabled
}
+ s.mu.Unlock()
writeJSON(w, http.StatusOK, map[string]any{"enabled": enabled})
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func (s *Server) handleChannelBLEStatus(w http.ResponseWriter, r *http.Request) { | |
| enabled := false | |
| if s.cfg != nil && s.cfg.Channel != nil { | |
| enabled = s.cfg.Channel.BLEEnabled | |
| } | |
| writeJSON(w, http.StatusOK, map[string]any{"enabled": enabled}) | |
| func (s *Server) handleChannelBLEStatus(w http.ResponseWriter, r *http.Request) { | |
| s.mu.Lock() | |
| enabled := false | |
| if s.cfg != nil && s.cfg.Channel != nil { | |
| enabled = s.cfg.Channel.BLEEnabled | |
| } | |
| s.mu.Unlock() | |
| writeJSON(w, http.StatusOK, map[string]any{"enabled": enabled}) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/web/channel.go` around lines 112 - 117, The handleChannelBLEStatus
function reads from s.cfg.Channel without synchronization while
handleSetChannelBLE writes to it under the s.mu mutex, creating a race
condition. Wrap the read access to s.cfg.Channel inside a lock acquired from
s.mu (similar to how handleSetChannelBLE protects its write), ensuring the
entire block that accesses s.cfg.Channel and s.cfg is protected by the same
mutex to prevent concurrent read-write races.
Comment on lines
+141
to
+144
| if err := config.SaveConfig(s.cfg); err != nil { | ||
| s.mu.Unlock() | ||
| writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) | ||
| return |
There was a problem hiding this comment.
Do not return raw config save errors to the client.
SaveConfig wraps errors with the config file path, so err.Error() can disclose local path details. Log the diagnostic and return a stable API error.
Proposed fix
if err := config.SaveConfig(s.cfg); err != nil {
s.mu.Unlock()
- writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
+ config.Logger().Printf("[web] BLE channel save config failed: %v", err)
+ writeJSON(w, http.StatusInternalServerError, map[string]string{"error": "failed to save BLE setting"})
return
}As per coding guidelines, “All diagnostics must go to config.Logger() which writes to ~/.jcode/debug.log.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if err := config.SaveConfig(s.cfg); err != nil { | |
| s.mu.Unlock() | |
| writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) | |
| return | |
| if err := config.SaveConfig(s.cfg); err != nil { | |
| s.mu.Unlock() | |
| config.Logger().Printf("[web] BLE channel save config failed: %v", err) | |
| writeJSON(w, http.StatusInternalServerError, map[string]string{"error": "failed to save BLE setting"}) | |
| return |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/web/channel.go` around lines 141 - 144, The error handling in the
SaveConfig call is exposing the raw error to the client via writeJSON, which can
leak sensitive file path information. Instead, log the actual error using
config.Logger() for diagnostic purposes and return a generic, stable error
message to the client. Modify the error handling block after the SaveConfig call
to capture the error, log it with config.Logger(), and then call writeJSON with
a user-safe error message that does not expose internal path details.
Source: Coding guidelines
Comment on lines
+452
to
+453
| const fg = getComputedStyle(root).getPropertyValue('--term-fg').trim() || '#18181b' | ||
| const bg = getComputedStyle(root).getPropertyValue('--color-surface').trim() || '#ffffff' |
There was a problem hiding this comment.
Remove hardcoded QR fallback colors.
Line 452 and Line 453 use hardcoded hex fallback values, which breaks the token-only color contract in Vue code.
As per coding guidelines, "Every color in Vue components and stylesheets must come from a CSS custom property defined in src/styles/tokens.css. Never hardcode hex/rgb/#fff/white."
Suggested fix
- const fg = getComputedStyle(root).getPropertyValue('--term-fg').trim() || '`#18181b`'
- const bg = getComputedStyle(root).getPropertyValue('--color-surface').trim() || '`#ffffff`'
+ const styles = getComputedStyle(root)
+ const fg = styles.getPropertyValue('--term-fg').trim() || styles.getPropertyValue('--color-foreground').trim()
+ const bg = styles.getPropertyValue('--color-surface').trim() || styles.getPropertyValue('--color-background').trim()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/SettingsDialog.vue` around lines 452 - 453, The hardcoded
hex fallback values in the getComputedStyle calls for both the fg and bg color
variables violate the token-only color contract. Replace the hardcoded hex
fallbacks (the '||' operators with '`#18181b`' and '`#ffffff`') with references to
appropriate CSS custom properties defined in src/styles/tokens.css. Instead of
using hardcoded fallback colors, reference the actual token CSS variables as
fallbacks so all colors come from the centralized token system.
Source: Coding guidelines
Comment on lines
+765
to
+766
| class="inline-block h-3.5 w-3.5 rounded-full bg-white shadow-sm transition-transform" | ||
| :class="store.autoApprove ? 'translate-x-[20px]' : 'translate-x-[2px]'" |
There was a problem hiding this comment.
Replace hardcoded white toggle thumb color with a token.
The new toggle thumb uses bg-white, which violates the token-only color rule and may drift across themes.
As per coding guidelines, "Every color in Vue components and stylesheets must come from a CSS custom property defined in src/styles/tokens.css. Never hardcode hex/rgb/#fff/white."
Suggested fix
- class="inline-block h-3.5 w-3.5 rounded-full bg-white shadow-sm transition-transform"
+ class="inline-block h-3.5 w-3.5 rounded-full shadow-sm transition-transform"
+ style="background-color: var(--color-background)"Also applies to: 788-789
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/SettingsDialog.vue` around lines 765 - 766, Replace the
hardcoded `bg-white` class in the toggle thumb element (the inline-block element
with transition-transform styling that uses the translate-x binding for
store.autoApprove) with a CSS custom property token from src/styles/tokens.css.
The element currently uses `bg-white` for the background color, which must be
replaced with an appropriate token variable to follow the color-only-from-tokens
rule. Also apply the same fix to the similar toggle element at lines 788-789
that has the same hardcoded white background color.
Source: Coding guidelines
| <div class="flex items-center gap-1 mb-2"> | ||
| <button class="cursor-pointer" style="color: var(--color-muted-foreground)" @click="addProviderStep = 'select'"> | ||
| <svg class="w-3.5 h-3.5" viewBox="0 0 20 20" fill="currentColor"><path fill-rule="evenodd" d="M12.79 5.23a.75.75 0 01-.02 1.06L8.832 10l3.938 3.71a.75.75 0 11-1.04 1.08l-4.5-4.25a.75.75 0 010-1.08l4.5-4.25a.75.75 0 011.06.02z" clip-rule="evenodd" /></svg> | ||
| <ChevronRightIcon class="w-3.5 h-3.5" /> |
There was a problem hiding this comment.
Back controls use forward chevrons.
These two back buttons currently render ChevronRightIcon, which inverts the navigation cue.
Suggested fix
+ ChevronLeftIcon,
ChevronRightIcon,
@@
- <ChevronRightIcon class="w-3.5 h-3.5" />
+ <ChevronLeftIcon class="w-3.5 h-3.5" />
@@
- <ChevronRightIcon class="w-3.5 h-3.5" />
+ <ChevronLeftIcon class="w-3.5 h-3.5" />Also applies to: 952-952
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/SettingsDialog.vue` at line 931, The back buttons in the
SettingsDialog component are using ChevronRightIcon which points in the wrong
direction for a back navigation control. Replace the two occurrences of
ChevronRightIcon (at lines 931 and 952) with ChevronLeftIcon to properly
indicate backward navigation. Maintain the existing class styling (w-3.5 h-3.5)
when making this change.
| <svg class="setup-chevron w-4 h-4 transition-colors" viewBox="0 0 20 20" fill="currentColor"> | ||
| <path fill-rule="evenodd" d="M7.21 14.77a.75.75 0 01.02-1.06L11.168 10 7.23 6.29a.75.75 0 111.04-1.08l4.5 4.25a.75.75 0 010 1.08l-4.5 4.25a.75.75 0 01-1.06-.02z" clip-rule="evenodd" /> | ||
| </svg> | ||
| <ChevronLeftIcon class="setup-chevron w-4 h-4 transition-colors" /> |
There was a problem hiding this comment.
Chevron directions are reversed for forward/back controls.
Forward affordances use ChevronLeftIcon while back buttons use ChevronRightIcon, so navigation cues appear inverted.
Suggested fix
- <ChevronLeftIcon class="setup-chevron w-4 h-4 transition-colors" />
+ <ChevronRightIcon class="setup-chevron w-4 h-4 transition-colors" />
- <ChevronRightIcon class="w-4 h-4" />
+ <ChevronLeftIcon class="w-4 h-4" />
- <ChevronLeftIcon class="setup-chevron w-4 h-4 transition-colors" />
+ <ChevronRightIcon class="setup-chevron w-4 h-4 transition-colors" />
- <ChevronRightIcon class="w-4 h-4" />
+ <ChevronLeftIcon class="w-4 h-4" />Also applies to: 284-284, 319-319, 330-330
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/SetupView.vue` at line 273, The chevron icons used for
navigation controls in the SetupView component are reversed. Forward navigation
buttons are currently using ChevronLeftIcon when they should use
ChevronRightIcon, and back buttons are using ChevronRightIcon when they should
use ChevronLeftIcon. Swap the icon references throughout the component so that
forward affordances display ChevronRightIcon and back affordances display
ChevronLeftIcon to provide correct visual navigation cues. This affects all
forward and back button implementations in the file.
Comment on lines
+268
to
+274
| if (turnStartedAt > 0) { | ||
| for (let i = timeline.value.length - 1; i >= 0; i--) { | ||
| const item = timeline.value[i] | ||
| if (item && item.kind === 'message' && item.data.role === 'assistant') { | ||
| item.data.durationMs = Date.now() - turnStartedAt | ||
| break | ||
| } |
There was a problem hiding this comment.
Bound duration stamping to the current turn only.
Line 269 currently searches the entire timeline backward. If a turn ends without emitting an assistant message, Line 272 can stamp durationMs onto a previous turn’s assistant message, so the elapsed time shown in web/src/components/ChatMessage.vue becomes incorrect.
Suggested fix
- if (turnStartedAt > 0) {
- for (let i = timeline.value.length - 1; i >= 0; i--) {
- const item = timeline.value[i]
- if (item && item.kind === 'message' && item.data.role === 'assistant') {
- item.data.durationMs = Date.now() - turnStartedAt
- break
- }
- }
- turnStartedAt = 0
- }
+ if (turnStartedAt > 0) {
+ for (let i = timeline.value.length - 1; i >= 0; i--) {
+ const item = timeline.value[i]
+ if (!item || item.kind !== 'message') continue
+ if (item.data.role === 'assistant') {
+ item.data.durationMs = Date.now() - turnStartedAt
+ break
+ }
+ // Stop at the current turn's user prompt; don't cross into prior turns.
+ if (item.data.role === 'user') break
+ }
+ turnStartedAt = 0
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (turnStartedAt > 0) { | |
| for (let i = timeline.value.length - 1; i >= 0; i--) { | |
| const item = timeline.value[i] | |
| if (item && item.kind === 'message' && item.data.role === 'assistant') { | |
| item.data.durationMs = Date.now() - turnStartedAt | |
| break | |
| } | |
| if (turnStartedAt > 0) { | |
| for (let i = timeline.value.length - 1; i >= 0; i--) { | |
| const item = timeline.value[i] | |
| if (!item || item.kind !== 'message') continue | |
| if (item.data.role === 'assistant') { | |
| item.data.durationMs = Date.now() - turnStartedAt | |
| break | |
| } | |
| // Stop at the current turn's user prompt; don't cross into prior turns. | |
| if (item.data.role === 'user') break | |
| } | |
| turnStartedAt = 0 | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/stores/chat.ts` around lines 268 - 274, The loop that searches for
the assistant message to stamp durationMs at line 269 iterates through the
entire timeline backward, which can incorrectly apply the duration to an
assistant message from a previous turn if the current turn hasn't produced one
yet. Modify the search loop to only search through messages that belong to the
current turn by tracking the starting index of the current turn in the timeline
when turnStartedAt is set, and then bound the loop to only search from that
index forward to the end of the timeline. This ensures durationMs is only
applied to assistant messages that are actually part of the current turn.
Comment on lines
+118
to
+122
| --term-bg: #fafafa; | ||
| --term-fg: #27272a; | ||
| --term-cursor: var(--color-primary); | ||
| --term-selection: #d4d4d880; | ||
| --term-black: #18181b; |
There was a problem hiding this comment.
--term-bright-white matches the light terminal background, making ANSI bright-white text invisible.
At Line 118 and Line 137 both values resolve to #fafafa, so bright-white output disappears in light mode. Use a contrasting token/value for --term-bright-white.
💡 Proposed fix
- --term-bright-white: `#fafafa`;
+ --term-bright-white: `#3f3f46`;Also applies to: 137-137
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/styles/tokens.css` around lines 118 - 122, The --term-bright-white
terminal color variable has the same value (`#fafafa`) as the --term-bg background
variable, making bright-white ANSI text invisible in light mode. Change the
--term-bright-white value to a contrasting color that is visually distinct from
the `#fafafa` background. Ensure the new value provides adequate contrast for text
readability against the light terminal background.
handleSetApprovalMode was writing both cfg.DefaultMode (the authoritative unified field) and cfg.AutoApprove (its deprecated fallback). resolveStartupMode only reads AutoApprove when DefaultMode is empty, so the AutoApprove write was dead code — and staticcheck SA1019 flags it. DefaultMode alone covers the persistence; the fallback is read-only for old configs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UI modernization pass across the web frontend — design-token extraction, full icon-system migration, and a batch of UX polish. No layout changes.
--code-bg/--code-border,--color-on-primary/-on-destructive, and a full ANSI terminal palette intokens.css.TerminalInstance.vueresolves terminal colors at runtime viagetComputedStyle(xterm needs color strings, notvar()). Hardcoded colors removed fromstyle.css.lucide-vue-next→@heroicons/vue/24/outlinemigration across all 20 components (non-1:1 matches use the closest outline icon); inline<svg>blocks removed; convention documented under Frontend inAGENTS.md.truncate, two-step "Allow all" confirm, fixed thetranslate-x-4.5/0.76toggle values, flex-anchored scroll-to-bottom, GoalBanner hover, send-btn opacity 0.3→0.45.Also bundles pre-existing in-progress work that was in the tree:
GET/POST /api/channel/blebackend + api.ts clientdurationMson the final assistant message; exposesetAutoApprove; dropretryFromMessageicon.svg→icon.png; regenerated desktop Tauri iconsVerification
Summary by CodeRabbit
New Features
Bug Fixes
UI Improvements
Documentation