fix(vscode): correct accent tokens and share one token-speed measurement - #21
Merged
Merged
Conversation
--primary was oklch(0.21) in both themes: a near-black that sits DARKER than the unchecked --input (oklch 0.274) and barely above the dark background. Every accent surface built on it read as an unstyled dark chip, and a Switch turning on visibly got darker instead of lighting up. Point --primary at the CLI periwinkle per theme, and give toggles their own --success token so the on state is unambiguous. Switch geometry: p-[2px] gives the thumb an even inset. The old 18.4px track with a 16px thumb left ~0.6px above and below but 2px at the travel end, which read as a blob rather than a track. Token speed had two defects. useTokenSpeed kept per-hook state while both TokenInfo and ChatStatus call it, so each measured from its own mount time and the header and expanded panel disagreed about the same stream; the measurement now lives in one module-level sampler that every consumer subscribes to. The sampler also depended on the token count, so it was torn down and recreated on every streamed chunk and never survived its own 250ms interval on fast streams. The readout no longer wraps '74.7' and 't/s' onto separate lines. Secondary text moves from a near-neutral grey to the CLI periwinkle, which was too close to the background to read at 11px.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe webview updates theme colors, switch geometry, button styling, and generation-speed display formatting. ChangesWebview theme and status updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ChatStatus
participant useTokenSpeed
participant SharedSpeedStore
participant Sampler250ms
ChatStatus->>useTokenSpeed: provide token count and streaming state
useTokenSpeed->>SharedSpeedStore: update latest token count
useTokenSpeed->>Sampler250ms: start one measurement window
Sampler250ms->>SharedSpeedStore: publish token speed
SharedSpeedStore-->>ChatStatus: notify subscribed speed value
ChatStatus->>useTokenSpeed: end streaming
useTokenSpeed->>Sampler250ms: clear timer and reset speed
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/vscode/webview-ui/src/components/ChatStatus.tsx (1)
2-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the
#/import alias.Replace the
@/storesand@/lib/utilsimports with their#/equivalents.As per coding guidelines, prefer imports using
import ... from '#/...'rather than the equivalent@/...alias.🤖 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 `@apps/vscode/webview-ui/src/components/ChatStatus.tsx` around lines 2 - 3, Update the import statements in ChatStatus to use the `#/` alias instead of `@/` by changing the existing `useChatStore`, `useSettingsStore`, and `cn` imports to their `#/stores` and `#/lib/utils` equivalents. Keep the rest of the component unchanged and only adjust the import paths in this file.Source: Coding guidelines
🤖 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 `@apps/vscode/webview-ui/src/components/ui/switch.tsx`:
- Line 38: Update the switch thumb classes in the visible className to use
data-checked:bg-success-foreground instead of data-checked:bg-white, and change
the checked translation offsets to 12px for the default size and 8px for the
small size. Preserve the unchecked offsets and all other styling.
---
Nitpick comments:
In `@apps/vscode/webview-ui/src/components/ChatStatus.tsx`:
- Around line 2-3: Update the import statements in ChatStatus to use the `#/`
alias instead of `@/` by changing the existing `useChatStore`,
`useSettingsStore`, and `cn` imports to their `#/stores` and `#/lib/utils`
equivalents. Keep the rest of the component unchanged and only adjust the import
paths in this file.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8039ea98-f087-44f6-8db8-e32cda966963
📒 Files selected for processing (5)
apps/vscode/webview-ui/src/components/ChatArea.tsxapps/vscode/webview-ui/src/components/ChatStatus.tsxapps/vscode/webview-ui/src/components/ThinkingButton.tsxapps/vscode/webview-ui/src/components/ui/switch.tsxapps/vscode/webview-ui/src/styles/index.css
elkaix
added a commit
that referenced
this pull request
Aug 5, 2026
The periwinkle that #21 wrote into --primary and --muted-foreground tinted every neutral surface, so both tokens go back to their original oklch values. The accent it was compensating for now lives in its own --brand token, applied only where an accent is actually wanted: inline code in Markdown, and the DynamicWorkflow lane bars and running status dot, which were invisible against --muted once --primary went back to a near-black neutral. The scroll-to-bottom button and the effort toggle return to their blue accents for the same reason. Also: - a finished workflow lane renders a full bar. The bar is scaled to the busiest agent, so a lane that completed in fewer steps kept a permanent gap. - the generation-speed pill in the thinking row no longer wraps "46.0" and "t/s" onto two lines, and uses tabular-nums so it stops resizing.
5 tasks
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.
Related Issue
No issue — reported directly with screenshots of the extension sidebar.
Problem
Four defects, three of them rooted in one bad token.
--primarywasoklch(0.21 0.006 285.885)in both the light and dark blocks. In dark mode that is darker than the unchecked switch track (--input,oklch 0.274) and barely above the background, so:Switchturning on got visually darker than when it was offbg-primaryread as an unstyled dark chip, which is why accents had drifted to hardcodedblue-400/blue-500in individual componentsSeparately, generation speed was measured per component.
TokenInfoandChatStatusare mounted together and both calluseTokenSpeed, so each kept its own start timestamp and token baseline and averaged over a different window — the header and the expanded panel showed different rates for the same response. The sampler also listed the token count as an effect dependency, so it was torn down and recreated on every streamed chunk and never survived its own 250ms interval while tokens arrived faster than that.What changed
--primarypoints at the CLI periwinkle per theme (#4a5bc4light,#bbc6ffdark), matchingapps/pythinker-code/src/tui/theme/colors.ts.--successtoken (CLIsuccess,#0e7a38/#4ec87e) drives the switch "on" state, so on/off is unambiguous rather than two near-identical darks.p-[2px]for an even thumb inset. The old 18.4px track with a 16px thumb left ~0.6px above and below but 2px at the travel end.useTokenSpeednow measures once in a module-level sampler that consumers subscribe to viauseSyncExternalStore, and the sampler no longer depends on the token count. The readout no longer wraps74.7andt/sonto separate lines, and usestabular-numsso the pill does not resize as the rate changes.bg-primary.--muted-foregroundmoves from a near-neutral grey to the CLI periwinkle; the previous value sat too close to the background to read at 11px.Not included
Hardcoded
blue-*classes remain in 11 other components (QuestionDialog,ApprovalDialog,MCPServersModal,ToolRenderers,Markdown,SessionList,CompactionCard,ChatMessage,LoginScreen,WorkDirModal,ThinkingButtonhover states). Now that--primaryis correct they can be migrated to the token, but that is a wider sweep than this fix and is left for a follow-up.Checklist
build:webview, and asserting the emitted CSS custom properties and thedata-checked:bg-successrule in the bundle.gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.[skip changeset] — every changed file is under
apps/vscode, whose package (pythinker-code) isprivate: trueand is not published by changesets. Nothing here enters the@pythoughts/pythinker-codeCLI bundle.Summary by CodeRabbit
Enhancements
Style