feat: Multiple features, improvements, and bug fixes #208
Conversation
## Features - **Token Budget Visualization**: Added real-time token usage tracking with pie chart display showing percentage used (blue < 50%, orange < 75%, red ≥ 75%) - **Show Thinking Toggle**: Added quick settings option to show/hide reasoning sections in messages - **Cache Clearing Utility**: Added `/clear-cache.html` page for clearing service workers, caches, and storage ## Improvements - **Package Upgrades**: Migrated from deprecated `xterm` to `@xterm/*` scoped packages - **Testing Setup**: Added Playwright for end-to-end testing - **Build Optimization**: Implemented code splitting for React, CodeMirror, and XTerm vendors to improve initial load time - **Deployment Scripts**: Added `scripts/start.sh` and `scripts/stop.sh` for cleaner server management with automatic port conflict resolution - **Vite Update**: Upgraded Vite from 7.0.5 to 7.1.8 ## Bug Fixes - Fixed static file serving to properly handle routes vs assets - Fixed session state reset to preserve token budget on initial load - Updated default Vite dev server port to 5173 (Vite's standard) ## Technical Details - Token budget is parsed from Claude CLI `modelUsage` field in result messages - Budget updates are sent via WebSocket as `token-budget` events - Calculation includes input, output, cache read, and cache creation tokens - Token budget state persists during active sessions but resets on session switch
Fixes issue where "Thinking..." banner and stop button disappear when switching between sessions. Users can now navigate freely while Claude is processing without losing the ability to monitor or stop the session. Features: - Processing state tracked in processingSessions Set (App.jsx) - Backend session status queries via check-session-status WebSocket message - UI state (banner + stop button) restored when returning to processing sessions - Works after page reload by querying backend's authoritative process maps - Proper cleanup when sessions complete in background Backend Changes: - Added sessionId to claude-complete, cursor-result, session-aborted messages - Exported isClaudeSessionActive, isCursorSessionActive helper functions - Exported getActiveClaudeSessions, getActiveCursorSessions for status queries - Added check-session-status and get-active-sessions WebSocket handlers Frontend Changes: - processingSessions state tracking in App.jsx - onSessionProcessing/onSessionNotProcessing callbacks - Session status check on session load and switch - Completion handlers only update UI if message is for current session - Always clean up processing state regardless of which session is active
Removes hardcoded 160k token limit and makes it configurable through environment variables. This allows easier adjustment for different Claude models or use cases. Changes: - Added CONTEXT_WINDOW env var for backend (default: 160000) - Added VITE_CONTEXT_WINDOW env var for frontend (default: 160000) - Updated .env.example with documentation - Replaced hardcoded values in token usage calculations - Replaced hardcoded values in pie chart display Why 160k? Claude Code reserves ~40k tokens for auto-compact feature, leaving 160k available for actual usage from the 200k context window.
WalkthroughReplaces the Claude CLI with a Claude SDK backend, adds VITE_CONTEXT_WINDOW/CONTEXT_WINDOW env vars, swaps xterm to scoped @xterm packages, introduces a public cache-clearing page, adds server token-usage/session-status APIs, and updates frontend to track processing state, a persisted “Show thinking” toggle, and token-usage visualization. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as ChatInterface (Client)
participant WS as WebSocket Server
participant SDK as Claude SDK (server/claude-sdk)
participant CUR as Cursor service (server/cursor-cli)
participant API as HTTP API (/api/…)
note over UI: On session select / mount
UI->>WS: check-session-status {provider, sessionId}
alt Provider = Claude
WS->>SDK: isClaudeSDKSessionActive(sessionId)
SDK-->>WS: boolean
else Provider = Cursor
WS->>CUR: isCursorSessionActive(sessionId)
CUR-->>WS: boolean
end
WS-->>UI: session-status {isProcessing, provider, sessionId}
rect rgba(200,230,255,0.12)
note right of UI: Poll token usage periodically
loop every 5s / on visibility change
UI->>API: GET /api/sessions/:sessionId/token-usage
API-->>UI: {used, total, breakdown}
end
end
rect rgba(220,255,220,0.12)
note over SDK,WS: SDK-driven streaming flow
UI->>WS: start-query (options)
WS->>SDK: queryClaudeSDK(command, options, ws)
SDK-->>WS: session-created {sessionId}
SDK-->>WS: claude-response (stream chunk)
WS-->>UI: claude-response {sessionId, chunk}
SDK-->>WS: token-budget {used,total}
WS-->>UI: token-budget {sessionId, used, total}
SDK-->>WS: claude-complete {sessionId, exitCode}
WS-->>UI: claude-complete {sessionId}
UI->>UI: markSessionNotProcessing(sessionId)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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: 4
🧹 Nitpick comments (6)
.env.example (1)
17-20: Keep env keys ordered to satisfy dotenv‑linter.
dotenv-linterexpectsCONTEXT_WINDOWbeforeVITE_CONTEXT_WINDOW; without reordering, lint runs will keep flagging this block. Swap the two lines to stay lint-clean.scripts/stop.sh (1)
23-24: Consider graceful shutdown before force kill.The script uses
kill -9(SIGKILL) immediately, which doesn't allow the server to perform cleanup operations. Try SIGTERM first for a graceful shutdown, then fall back to SIGKILL if needed.Apply this diff to implement graceful shutdown:
# Kill process on the port - lsof -ti:$PORT | xargs kill -9 2>/dev/null || true + # Try graceful shutdown first + lsof -ti:$PORT | xargs kill -TERM 2>/dev/null || true + sleep 2 + + # Force kill if still running + if lsof -Pi :$PORT -sTCP:LISTEN -t >/dev/null 2>&1 ; then + lsof -ti:$PORT | xargs kill -9 2>/dev/null || true + fiscripts/start.sh (2)
36-54: Consider graceful shutdown before force kill.Similar to
scripts/stop.sh, the script immediately useskill -9(SIGKILL) without attempting graceful shutdown. Try SIGTERM first to allow processes to clean up resources.Apply this diff to implement graceful shutdown:
if pgrep -f "claude-code-ui" >/dev/null 2>&1 ; then echo -e "${YELLOW}Stopping existing Claude Code UI server...${NC}" - pkill -9 -f "claude-code-ui" || true + pkill -TERM -f "claude-code-ui" || true sleep 1 + # Force kill if still running + if pgrep -f "claude-code-ui" >/dev/null 2>&1 ; then + pkill -9 -f "claude-code-ui" || true + fi fi if pgrep -f "node server/index.js" >/dev/null 2>&1 ; then echo -e "${YELLOW}Stopping local server process...${NC}" - pkill -9 -f "node server/index.js" || true + pkill -TERM -f "node server/index.js" || true sleep 1 + # Force kill if still running + if pgrep -f "node server/index.js" >/dev/null 2>&1 ; then + pkill -9 -f "node server/index.js" || true + fi fi # Force kill whatever is on the port if lsof -Pi :$PORT -sTCP:LISTEN -t >/dev/null 2>&1 ; then echo -e "${YELLOW}Force stopping process on port $PORT...${NC}" - lsof -ti:$PORT | xargs kill -9 2>/dev/null || true + lsof -ti:$PORT | xargs kill -TERM 2>/dev/null || true sleep 2 + # Force kill if still running + if lsof -Pi :$PORT -sTCP:LISTEN -t >/dev/null 2>&1 ; then + lsof -ti:$PORT | xargs kill -9 2>/dev/null || true + sleep 1 + fi fi
66-70: Auto-build behavior may be unexpected.The script automatically runs
npm run buildif thedistfolder is missing. While convenient, this can cause confusion if a developer expects to run the dev server or if the build fails. Consider logging a clearer message or requiring an explicit build step.Consider adding a more explicit message:
# Check if dist folder exists if [ ! -d "dist" ]; then - echo -e "${YELLOW}No dist folder found. Running build...${NC}" + echo -e "${YELLOW}No dist folder found. Building production assets...${NC}" + echo -e "${YELLOW}This may take a few minutes. For development, use 'npm run dev' instead.${NC}" npm run build fipublic/clear-cache.html (1)
77-77: Consider adding automatic reload option.After clearing the cache, users must manually navigate to the home page. Consider adding an optional automatic reload after a short delay to improve the user experience.
Add this after the success message:
status.innerHTML += '<p class="success"><strong>✓ All caches cleared!</strong></p>'; - status.innerHTML += '<p>Cache cleared successfully. You can now close this tab or <a href="/">go to home page</a>.</p>'; + status.innerHTML += '<p>Cache cleared successfully. <a href="/">Go to home page</a> or wait for automatic reload...</p>'; + + // Auto-reload after 3 seconds + setTimeout(() => { + window.location.href = '/'; + }, 3000); } catch (error) {src/components/ChatInterface.jsx (1)
2699-2744: Trim noisy token-usage debug logs.This effect runs on every session switch and then every five seconds; the current
console.logcalls will flood the console in production. Please wrap them in a debug flag or drop them before shipping.- console.log('🔍 Token usage polling effect triggered', { - sessionId: selectedSession?.id, - projectPath: selectedProject?.path - }); … - console.log('📊 Fetching token usage from:', url); … - console.log('✅ Token usage data received:', data); … - console.error('❌ Token usage fetch failed:', response.status, await response.text()); … - console.error('Failed to fetch token usage:', error);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
.env.example(1 hunks)package.json(3 hunks)public/clear-cache.html(1 hunks)scripts/start.sh(1 hunks)scripts/stop.sh(1 hunks)server/claude-cli.js(4 hunks)server/cursor-cli.js(3 hunks)server/index.js(3 hunks)src/App.jsx(5 hunks)src/components/ChatInterface.jsx(20 hunks)src/components/MainContent.jsx(2 hunks)src/components/QuickSettingsPanel.jsx(2 hunks)src/components/Shell.jsx(1 hunks)src/components/TokenUsagePie.jsx(1 hunks)vite.config.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/components/MainContent.jsx (1)
src/App.jsx (2)
processingSessions(71-71)showThinking(60-60)
server/claude-cli.js (4)
server/index.js (6)
lines(1103-1103)inputTokens(1106-1106)cacheReadTokens(1108-1108)cacheCreationTokens(1107-1107)totalUsed(1133-1133)contextWindow(1105-1105)src/components/ChatInterface.jsx (1)
tokenBudget(1220-1220)src/components/Shell.jsx (1)
ws(36-36)src/utils/websocket.js (1)
ws(4-4)
src/components/ChatInterface.jsx (4)
src/App.jsx (5)
showThinking(60-60)selectedSession(50-50)processingSessions(71-71)selectedProject(49-49)sendByCtrlEnter(62-62)src/utils/websocket.js (2)
ws(4-4)sendMessage(95-101)src/utils/api.js (3)
url(53-53)authenticatedFetch(2-20)authenticatedFetch(2-20)src/components/TokenUsagePie.jsx (5)
percentage(7-7)radius(8-8)circumference(9-9)offset(10-10)getColor(13-17)
src/components/QuickSettingsPanel.jsx (1)
src/App.jsx (1)
showThinking(60-60)
server/index.js (5)
server/middleware/auth.js (1)
authenticateToken(22-45)server/utils/mcp-detector.js (2)
homeDir(21-21)homeDir(154-154)server/projects.js (4)
lines(815-815)entry(300-300)entry(663-663)entry(738-738)server/claude-cli.js (1)
process(422-422)server/cursor-cli.js (1)
process(239-239)
🪛 Biome (2.1.2)
src/components/ChatInterface.jsx
[error] 2319-2321: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2371-2372: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2416-2418: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2455-2457: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2483-2484: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2484-2486: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 20-20: [UnorderedKey] The CONTEXT_WINDOW key should go before the VITE_CONTEXT_WINDOW key
(UnorderedKey)
🔇 Additional comments (11)
scripts/stop.sh (1)
1-46: LGTM!The script provides a clean shutdown flow with proper port checking, process termination, and verification. The overall structure is sound.
vite.config.js (1)
26-45: LGTM!The vendor chunking strategy effectively separates large dependencies (React, CodeMirror, XTerm) into dedicated bundles, which improves caching and initial load performance. The chunk size warning limit of 1000 KB is reasonable for vendor bundles.
server/claude-cli.js (3)
291-326: LGTM!The token budget calculation correctly sums all token types (input, output, cache read, cache creation), and the WebSocket streaming implementation properly sends the budget data to the frontend in real-time.
432-445: LGTM!The new session helper functions provide a clean API for checking active sessions and retrieving all session IDs. The implementation is straightforward and consistent with the existing abort functionality.
271-279: Session ID key replacement is safe. AllactiveClaudeProcessesoperations remain within this module and use the updatedsessionId; no orphaned references to the old key were found.server/cursor-cli.js (2)
249-262: LGTM!The session helper functions mirror the implementation in
server/claude-cli.js, maintaining API consistency across both CLI modules. The export statement is properly updated to include the new functions.
162-162: LGTM!Including
sessionIdin the cursor-result and claude-complete messages ensures the frontend can properly track and restore session state across UI transitions.Also applies to: 205-205
src/components/MainContent.jsx (1)
45-56: LGTM!The new props are properly added to the component signature and forwarded to
ChatInterfacewithout introducing any logic changes. The passthrough pattern is appropriate for this bridge component.src/components/Shell.jsx (1)
2-6: LGTM!The import paths are correctly updated to use the scoped
@xterm/*package structure, aligning with the vendor-xterm chunk configuration invite.config.js. This maintains compatibility with the updated xterm dependencies.scripts/start.sh (1)
1-74: LGTM!The script provides comprehensive startup logic with port conflict resolution, conditional builds, and production server launch. The overall structure and flow are sound.
public/clear-cache.html (1)
45-82: LGTM!The cache clearing implementation is comprehensive and properly handles service workers, caches, localStorage, and sessionStorage. The async/await pattern is correctly used, and error handling is in place. The progressive status updates provide good user feedback.
HTML entities like < and > were showing as-is instead of being decoded to < and > characters. Added decodeHtmlEntities helper function to properly display angle brackets and other special characters. Applied to: - Regular message content - Streaming content deltas - Session history loading - Both string and array content types
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
src/components/ChatInterface.jsx (4)
2331-2395: Verify: Switch declaration scoping error (already flagged).This switch case violates Biome's
noSwitchDeclarationsrule. See previous review comments for details.
2428-2465: Verify: Switch declaration scoping error (already flagged).This switch case violates Biome's
noSwitchDeclarationsrule. See previous review comments for details.
2467-2493: Verify: Switch declaration scoping error (already flagged).This switch case violates Biome's
noSwitchDeclarationsrule. See previous review comments for details.
2495-2508: Verify: Switch declaration scoping error (already flagged).This switch case violates Biome's
noSwitchDeclarationsrule. See previous review comments for details.
🧹 Nitpick comments (1)
src/components/ChatInterface.jsx (1)
3476-3505: Refactor: Use TokenUsagePie component instead of duplicating logic.This inline pie chart rendering duplicates the logic from
TokenUsagePie.jsx(which is imported at line 29 but not used). Refactor to use the component for better maintainability.Apply this diff:
- {/* Token usage pie chart - positioned next to mode indicator */} - {(() => { - // Default to 0 tokens if no budget received yet - const used = tokenBudget?.used || 0; - const total = tokenBudget?.total || parseInt(import.meta.env.VITE_CONTEXT_WINDOW) || 160000; - - const percentage = total > 0 ? Math.min(100, (used / total) * 100) : 0; - console.log('🎨 Rendering pie chart:', { tokenBudget, used, total, percentage: percentage.toFixed(0) + '%' }); - const radius = 10; - const circumference = 2 * Math.PI * radius; - const offset = circumference - (percentage / 100) * circumference; - const getColor = () => { - if (percentage < 50) return '#3b82f6'; - if (percentage < 75) return '#f59e0b'; - return '#ef4444'; - }; - - return ( - <div className="flex items-center gap-2 text-xs text-gray-600 dark:text-gray-400"> - <svg width="24" height="24" viewBox="0 0 24 24" className="transform -rotate-90"> - <circle cx="12" cy="12" r={radius} fill="none" stroke="currentColor" strokeWidth="2" className="text-gray-300 dark:text-gray-600" /> - <circle cx="12" cy="12" r={radius} fill="none" stroke={getColor()} strokeWidth="2" strokeDasharray={circumference} strokeDashoffset={offset} strokeLinecap="round" /> - </svg> - <span className="hidden sm:inline" title={`${used.toLocaleString()} / ${total.toLocaleString()} tokens`}> - {percentage.toFixed(1)}% - </span> - </div> - ); - })()} + {/* Token usage pie chart - positioned next to mode indicator */} + {tokenBudget && ( + <TokenUsagePie + used={tokenBudget.used} + total={tokenBudget.total} + /> + )}Note: You may need to adjust the TokenUsagePie component's styling to match the inline version's appearance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/ChatInterface.jsx(23 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/ChatInterface.jsx (3)
src/App.jsx (5)
showThinking(60-60)selectedSession(50-50)processingSessions(71-71)selectedProject(49-49)sendByCtrlEnter(62-62)src/utils/api.js (3)
url(53-53)authenticatedFetch(2-20)authenticatedFetch(2-20)src/components/TokenUsagePie.jsx (5)
percentage(7-7)radius(8-8)circumference(9-9)offset(10-10)getColor(13-17)
🪛 Biome (2.1.2)
src/components/ChatInterface.jsx
[error] 2048-2050: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2333-2335: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2385-2386: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2430-2432: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2469-2471: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2497-2498: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2498-2500: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (2)
src/components/ChatInterface.jsx (2)
2712-2779: Good implementation of token budget polling.The polling logic correctly:
- Fetches immediately on mount and session change
- Polls every 5 seconds
- Handles visibility changes for tab focus
- Resets to zero for new sessions and errors
However, there's an inconsistency in the default context window value (see separate comment on line 3481).
34-43: Critical: Fix HTML entity decoding order.The current order decodes
&last, which causes double-decoding bugs. For example, the string"&lt;"(which should decode to"<") will incorrectly decode to"<"because<is replaced first, then&is replaced.Always decode
&first to prevent this issue.Apply this diff:
function decodeHtmlEntities(text) { if (!text) return text; return text + .replace(/&/g, '&') .replace(/</g, '<') .replace(/>/g, '>') .replace(/"/g, '"') - .replace(/'/g, "'") - .replace(/&/g, '&'); + .replace(/'/g, "'"); }Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
@joshwilhelmi I like the changes but there are also some breaking changes that make ClaudeCode UI work across environments. Would it be possible to remove the start script and revert back to the original packages.json scripts?
- Revert to main branch's package.json scripts structure - Remove custom scripts/start.sh and scripts/stop.sh - Update xterm dependencies to scoped @xterm packages (required for code compatibility) - Replace xterm with @xterm/xterm - Replace xterm-addon-fit with @xterm/addon-fit 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit completes the migration to the Claude Agents SDK, removing the legacy CLI-based implementation and making the SDK the exclusive integration method. Changes: - Remove claude-cli.js legacy implementation - Add claude-sdk.js with full SDK integration - Remove CLAUDE_USE_SDK feature flag (SDK is now always used) - Update server/index.js to use SDK functions directly - Add .serena/ to .gitignore for AI assistant cache Benefits: - Better performance (no child process overhead) - Native session management with interrupt support - Cleaner codebase without CLI/SDK branching - Full feature parity with previous CLI implementation - Maintains compatibility with Cursor integration 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🚀 Latest Update: Claude Agents SDK Migration CompleteThe latest commit completes the migration from CLI-based integration to the native Claude Agents SDK. What Changed
Benefits🚀 Performance: No child process overhead Testing Completed
The SDK implementation is production-ready! 🎉 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
.gitignore(2 hunks)server/claude-cli.js(0 hunks)server/claude-sdk.js(1 hunks)server/index.js(6 hunks)
💤 Files with no reviewable changes (1)
- server/claude-cli.js
🚧 Files skipped from review as they are similar to previous changes (1)
- server/index.js
🧰 Additional context used
🧬 Code graph analysis (1)
server/claude-sdk.js (3)
src/components/ChatInterface.jsx (1)
permissionMode(1207-1207)src/components/Sidebar.jsx (1)
getAllSessions(336-343)server/index.js (9)
inputTokens(1120-1120)cacheReadTokens(1122-1122)cacheCreationTokens(1121-1121)totalUsed(1147-1147)contextWindow(1119-1119)path(1012-1012)fs(1013-1013)i(1125-1125)os(1014-1014)
🔇 Additional comments (2)
.gitignore (2)
108-108: Addition is consistent with existing AI workspace ignores.
Including.serena/matches the pattern of ignoring other AI tool directories here. Looks good.
130-131: No trackedtasks.jsonortasks/files detected
Verified viagit ls-files; nothing matched, so these ignore rules pose no risk.
|
I replaced the Claude CLI implementation with Claude Agents SDK. No key needed as long as you have Claude Code installed and an active Pro/Max subscription. |
Whoops. This is correct. Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
server/index.js (2)
109-113: WebSocket broadcast condition is wrong; no updates will be sent
client.readyState === client.OPENcompares against undefined. Use the class constant.- if (client.readyState === client.OPEN) { + if (client.readyState === WebSocket.OPEN) { client.send(updateMessage); }Also import WebSocket:
-import { WebSocketServer } from 'ws'; +import { WebSocketServer, WebSocket } from 'ws';
366-392: Restrict file read/write to the project root to prevent arbitrary filesystem accessBoth endpoints accept any absolute path and operate on it. An authenticated caller can read or overwrite arbitrary files. Constrain paths to the resolved project directory.
Apply this pattern to both GET and PUT handlers:
- // Security check - ensure the path is safe and absolute - if (!filePath || !path.isAbsolute(filePath)) { + // Security check - ensure path is absolute and within the project root + if (!filePath || !path.isAbsolute(filePath)) { return res.status(400).json({ error: 'Invalid file path' }); } + const projectRoot = await extractProjectDirectory(req.params.projectName); + const resolved = path.resolve(filePath); + const rel = path.relative(projectRoot, resolved); + if (rel.startsWith('..') || path.isAbsolute(rel)) { + return res.status(403).json({ error: 'Access denied: path outside project' }); + }Also applies to: 441-486
♻️ Duplicate comments (2)
src/components/ChatInterface.jsx (2)
3477-3505: Use TokenUsagePie component and fix 160k default fallbackInline pie duplicates TokenUsagePie and uses 200000 fallback (inconsistent with 160000). Replace with the component and a safe default.
- {/* Token usage pie chart - positioned next to mode indicator */} - {(() => { - // Default to 0 tokens if no budget received yet - const used = tokenBudget?.used || 0; - const total = tokenBudget?.total || parseInt(import.meta.env.VITE_CONTEXT_WINDOW) || 200000; - const percentage = total > 0 ? Math.min(100, (used / total) * 100) : 0; - console.log('🎨 Rendering pie chart:', { tokenBudget, used, total, percentage: percentage.toFixed(0) + '%' }); - const radius = 10; - const circumference = 2 * Math.PI * radius; - const offset = circumference - (percentage / 100) * circumference; - const getColor = () => { - if (percentage < 50) return '#3b82f6'; - if (percentage < 75) return '#f59e0b'; - return '#ef4444'; - }; - return ( - <div className="flex items-center gap-2 text-xs text-gray-600 dark:text-gray-400"> - <svg width="24" height="24" viewBox="0 0 24 24" className="transform -rotate-90"> - <circle cx="12" cy="12" r={radius} fill="none" stroke="currentColor" strokeWidth="2" className="text-gray-300 dark:text-gray-600" /> - <circle cx="12" cy="12" r={radius} fill="none" stroke={getColor()} strokeWidth="2" strokeDasharray={circumference} strokeDashoffset={offset} strokeLinecap="round" /> - </svg> - <span className="hidden sm:inline" title={`${used.toLocaleString()} / ${total.toLocaleString()} tokens`}> - {percentage.toFixed(1)}% - </span> - </div> - ); - })()} + {(() => { + const used = tokenBudget?.used ?? 0; + const parsed = parseInt(import.meta.env.VITE_CONTEXT_WINDOW, 10); + const fallbackTotal = Number.isFinite(parsed) ? parsed : 160000; + const total = tokenBudget?.total ?? fallbackTotal; + return ( + <div className="flex items-center gap-2 text-xs text-gray-600 dark:text-gray-400"> + <TokenUsagePie used={used} total={total} /> + </div> + ); + })()}
2048-2050: Fix Biome noSwitchDeclarations: wrap case bodies in blocksConst declarations inside switch clauses violate Biome and fail lint. Wrap each clause body in braces.
- case 'claude-response': - const messageData = latestMessage.data.message || latestMessage.data; + case 'claude-response': { + const messageData = latestMessage.data.message || latestMessage.data; ... - break; + break; + } @@ - case 'cursor-result': - const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; + case 'cursor-result': { + const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; ... - break; + break; + } @@ - case 'claude-complete': - const completedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId'); + case 'claude-complete': { + const completedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId'); ... - break; + break; + } @@ - case 'session-aborted': - const abortedSessionId = latestMessage.sessionId || currentSessionId; + case 'session-aborted': { + const abortedSessionId = latestMessage.sessionId || currentSessionId; ... - break; + break; + } @@ - case 'session-status': - const statusSessionId = latestMessage.sessionId; - const isCurrentSession = statusSessionId === currentSessionId || - (selectedSession && statusSessionId === selectedSession.id); + case 'session-status': { + const statusSessionId = latestMessage.sessionId; + const isCurrentSession = statusSessionId === currentSessionId || + (selectedSession && statusSessionId === selectedSession.id); ... - break; + break; + }Also applies to: 2332-2354, 2385-2390, 2430-2447, 2469-2486, 2495-2508
🧹 Nitpick comments (1)
server/index.js (1)
1267-1274: Watcher startup: consider handling chokidar.close()promiseIf
projectsWatcherexists (Line 62) you callclose()without awaiting. In practice it’s sync mostly, but v4 returns a promise. Await to avoid overlapping watchers during restarts.- if (projectsWatcher) { - projectsWatcher.close(); - } + if (projectsWatcher) { + await projectsWatcher.close().catch(() => {}); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
server/index.js(6 hunks)src/components/ChatInterface.jsx(23 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
server/index.js (4)
server/claude-sdk.js (6)
activeSessions(21-21)contextWindow(170-170)inputTokens(163-163)cacheCreationTokens(166-166)cacheReadTokens(165-165)totalUsed(169-169)server/middleware/auth.js (1)
authenticateToken(22-45)server/projects.js (4)
lines(815-815)entry(300-300)entry(663-663)entry(738-738)server/cursor-cli.js (1)
process(239-239)
src/components/ChatInterface.jsx (3)
src/App.jsx (5)
showThinking(60-60)selectedSession(50-50)processingSessions(71-71)selectedProject(49-49)sendByCtrlEnter(62-62)src/utils/api.js (3)
url(53-53)authenticatedFetch(2-20)authenticatedFetch(2-20)src/components/TokenUsagePie.jsx (5)
percentage(7-7)radius(8-8)circumference(9-9)offset(10-10)getColor(13-17)
🪛 Biome (2.1.2)
src/components/ChatInterface.jsx
[error] 2048-2050: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2333-2335: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2385-2386: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2430-2432: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2469-2471: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2497-2498: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2498-2500: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (3)
src/components/ChatInterface.jsx (2)
2712-2779: Token-usage polling: align with server breakdown and defaultsLooks good overall. After server change to include output, consider optionally surfacing breakdown (input/output/cache) near the pie (tooltip/title) if helpful. Ensure the default total uses a NaN-guard as in the pie fix.
1068-1079: Thinking toggle integration: LGTM
- Reasoning block is gated by showThinking.
- Prop is threaded into MessageComponent and used at render site.
Also applies to: 172-173, 3391-3392
server/index.js (1)
1093-1164: Token-usage endpoint: include output tokens, handle Windows paths, use async I/O
- Replace
projectPath.replace(/[\/\s~_]/g, '-')with/[\/\\\s~_]/gto encode backslashes on Windows- Swap
fs.existsSync/fs.readFileSyncforawait fsPromises.access(…)andawait fsPromises.readFile(…)- Introduce
outputTokens = usage.output_tokens || 0, include intotalUsedandbreakdown.outputAlso confirm that this regex matches Claude’s actual on-disk project-name encoding; consider extracting into a shared helper.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Yes that's fixed. Forgot to exclued those from the commit. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ChatInterface.jsx (1)
2047-2229: Fix BiomenoSwitchDeclarationserror inclaude-responsecase.Line 2048 declares
const messageDatadirectly inside the switch clause, violating Biome'snoSwitchDeclarationsrule. This case was not mentioned in the previous review but has the same issue.Wrap the case body in a block:
- case 'claude-response': - const messageData = latestMessage.data.message || latestMessage.data; + case 'claude-response': { + const messageData = latestMessage.data.message || latestMessage.data; // ... rest of the case body ... break; + }
♻️ Duplicate comments (1)
src/components/ChatInterface.jsx (1)
2331-2395: Fix BiomenoSwitchDeclarationserrors in remaining switch cases.The
cursor-result,claude-complete,session-aborted, andsession-statuscases still have const declarations directly inside the switch clause, as flagged in the previous review. These must be wrapped in blocks.Apply the fix suggested in the previous review to wrap each case body in a block.
Also applies to: 2428-2465, 2467-2493, 2495-2508
🧹 Nitpick comments (1)
src/components/ChatInterface.jsx (1)
2712-2779: Consider removing or guarding console.log statements in production.The token usage polling effect includes several console.log statements (lines 2714, 2741, 2747, 2750) that will clutter production logs. Consider removing them or guarding with a debug flag.
- console.log('🔍 Token usage polling effect triggered', { - sessionId: selectedSession?.id, - projectPath: selectedProject?.path - }); + // Optional: Add debug flag check + // if (import.meta.env.DEV) console.log('🔍 Token usage polling effect triggered', ...);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/ChatInterface.jsx(23 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/ChatInterface.jsx (4)
src/App.jsx (5)
showThinking(60-60)selectedSession(50-50)processingSessions(71-71)selectedProject(49-49)sendByCtrlEnter(62-62)src/utils/websocket.js (2)
ws(4-4)sendMessage(95-101)src/utils/api.js (3)
url(53-53)authenticatedFetch(2-20)authenticatedFetch(2-20)src/components/TokenUsagePie.jsx (5)
percentage(7-7)radius(8-8)circumference(9-9)offset(10-10)getColor(13-17)
🪛 Biome (2.1.2)
src/components/ChatInterface.jsx
[error] 2048-2050: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2333-2335: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2385-2386: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2430-2432: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2469-2471: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2497-2498: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2498-2500: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (4)
src/components/ChatInterface.jsx (4)
2712-2779: Excellent token budget polling implementation.The token usage polling logic is well-designed:
- Polls every 5 seconds and on visibility changes for up-to-date data
- Properly resets to zero for new sessions (no session, new-session-* IDs, missing IDs)
- Consistent default context window of 160000 across all branches
- Clean resource management with interval and event listener cleanup
- Appropriate error handling with graceful fallback
2002-2019: Well-implemented processing state persistence.The processing state tracking correctly:
- Notifies the parent component when a session starts processing (lines 2005-2008)
- Restores the "Thinking..." banner and stop button when switching to a session that's already processing (lines 2010-2019)
- Uses appropriate effect dependencies to trigger at the right times
This ensures users see consistent state even after session switches or page reloads.
34-43: Good practice: HTML entity decoding helper.The
decodeHtmlEntitieshelper function is correctly implemented and consistently applied across:
- Cursor message parsing (lines 1425, 1428)
- Claude message conversion (lines 1724, 1731, 1733)
- WebSocket streaming messages (lines 2054, 2184, 2197)
This prevents display issues with HTML-encoded content like
<,>,"in user messages and prevents XSS by normalizing content at the parsing stage.Also applies to: 1425-1425, 1428-1428, 1724-1724, 1731-1731, 1733-1733, 2054-2054, 2184-2184, 2197-2197
1738-1792: Correct handling of double-escaped sequences.The escape sequence unescaping (lines 1738-1741, 1755-1762, 1785-1792) correctly transforms stored double-escaped sequences (
\\n,\\t,\\r) into their actual whitespace characters. This ensures messages display with proper formatting, preserving line breaks and indentation from the original content.The type-safe checks (e.g.,
typeof text === 'string') prevent errors when processing non-string content.
Left my test code in, but that's fixed. Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/ChatInterface.jsx (1)
2026-2550: Critical: Fix Biome noSwitchDeclarations errors in multiple switch cases.Multiple switch cases declare
constbindings without block scoping, which violates Biome'snoSwitchDeclarationsrule and breaks the lint step. Thecursor-resultcase at lines 2333-2371 was already flagged in a previous review but remains unfixed. Additionally, the following cases have the same issue:
- Line 2048 (
claude-responsecase):const messageData- Line 2333 (
cursor-resultcase):const cursorCompletedSessionId(already flagged)- Line 2385 (
cursor-resultcase):const pendingCursorSessionId- Line 2430 (
claude-completecase):const completedSessionId- Line 2469 (
session-abortedcase):const abortedSessionId- Line 2497 (
session-statuscase):const statusSessionIdandconst isCurrentSessionWrap each case body in a block
{ ... }to properly scope all declarations:- case 'claude-response': - const messageData = latestMessage.data.message || latestMessage.data; + case 'claude-response': { + const messageData = latestMessage.data.message || latestMessage.data; // ... existing code ... - break; + break; + } - case 'cursor-result': - const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; + case 'cursor-result': { + const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; // ... existing code (including pendingCursorSessionId) ... - break; + break; + } - case 'claude-complete': - const completedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId'); + case 'claude-complete': { + const completedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId'); // ... existing code ... - break; + break; + } - case 'session-aborted': - const abortedSessionId = latestMessage.sessionId || currentSessionId; + case 'session-aborted': { + const abortedSessionId = latestMessage.sessionId || currentSessionId; // ... existing code ... - break; + break; + } - case 'session-status': - const statusSessionId = latestMessage.sessionId; - const isCurrentSession = statusSessionId === currentSessionId || + case 'session-status': { + const statusSessionId = latestMessage.sessionId; + const isCurrentSession = statusSessionId === currentSessionId || (selectedSession && statusSessionId === selectedSession.id); // ... existing code ... - break; + break; + }
🧹 Nitpick comments (1)
src/components/ChatInterface.jsx (1)
34-43: Consider more comprehensive HTML entity decoding.The
decodeHtmlEntitieshelper currently handles only 5 basic HTML entities. While this covers common cases, Claude or Cursor responses might contain other entities like ,', numeric character references (&#...;,&#x...;), or other named entities.For more robust handling, consider using the browser's built-in HTML entity decoder:
function decodeHtmlEntities(text) { if (!text) return text; - return text - .replace(/</g, '<') - .replace(/>/g, '>') - .replace(/"/g, '"') - .replace(/'/g, "'") - .replace(/&/g, '&'); + const textarea = document.createElement('textarea'); + textarea.innerHTML = text; + return textarea.value; }This approach:
- Handles all named and numeric HTML entities automatically
- Uses the browser's native parser (well-tested, secure)
- Simpler to maintain
Note: The textarea method is safe for text content (not HTML markup), which matches this use case. If you need to preserve HTML structure, consider using DOMParser instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/ChatInterface.jsx(23 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/ChatInterface.jsx (2)
src/App.jsx (5)
showThinking(60-60)selectedSession(50-50)processingSessions(71-71)selectedProject(49-49)sendByCtrlEnter(62-62)src/utils/api.js (3)
url(53-53)authenticatedFetch(2-20)authenticatedFetch(2-20)
🪛 Biome (2.1.2)
src/components/ChatInterface.jsx
[error] 2048-2050: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2333-2335: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2385-2386: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2430-2432: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2469-2471: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2497-2498: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2498-2500: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
…itch - Add AbortController to cancel in-flight token-usage requests when session/project changes - Capture session/project IDs before fetch and verify they match before updating state - Handle AbortError gracefully without logging as error - Prevents race condition where old session data overwrites current session's token budget 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ChatInterface.jsx (1)
2026-2550: CRITICAL: Fix Biome noSwitchDeclarations errors in WebSocket message switch.Multiple switch cases declare variables without wrapping them in blocks, violating scoping rules. This was flagged in previous reviews and must be fixed before merge.
Affected cases:
- Line 2048:
messageDatainclaude-response- Line 2333:
cursorCompletedSessionIdincursor-result- Line 2385:
pendingCursorSessionIdincursor-result- Line 2430:
completedSessionIdinclaude-complete- Line 2469:
abortedSessionIdinsession-aborted- Line 2497:
statusSessionIdandisCurrentSessioninsession-statusApply this pattern to all affected cases:
- case 'cursor-result': - const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; + case 'cursor-result': { + const cursorCompletedSessionId = latestMessage.sessionId || currentSessionId; // ... rest of case logic ... - break; + break; + }Repeat for all other flagged cases:
claude-response,claude-complete,session-aborted, andsession-status.
🧹 Nitpick comments (1)
src/components/ChatInterface.jsx (1)
2010-2019: Consider optimizing effect dependencies to prevent unnecessary re-runs.The restoration effect runs whenever
currentSessionIdorprocessingSessionschanges. SinceprocessingSessionsis a Set that changes reference even when the content is the same, this could cause unnecessary re-runs.Consider one of these approaches:
Option 1: Memoize the check
+ const shouldBeProcessing = useMemo(() => { + return currentSessionId && processingSessions?.has(currentSessionId); + }, [currentSessionId, processingSessions?.has(currentSessionId)]); + useEffect(() => { - if (currentSessionId && processingSessions) { - const shouldBeProcessing = processingSessions.has(currentSessionId); - if (shouldBeProcessing && !isLoading) { + if (shouldBeProcessing && !isLoading) { setIsLoading(true); setCanAbortSession(true); - } } - }, [currentSessionId, processingSessions]); + }, [shouldBeProcessing, isLoading]);Option 2: Check if processing changed
Add a check to only update state if it actually changed:useEffect(() => { if (currentSessionId && processingSessions) { const shouldBeProcessing = processingSessions.has(currentSessionId); - if (shouldBeProcessing && !isLoading) { + if (shouldBeProcessing !== isLoading) { - setIsLoading(true); - setCanAbortSession(true); + setIsLoading(shouldBeProcessing); + setCanAbortSession(shouldBeProcessing); } } }, [currentSessionId, processingSessions]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/ChatInterface.jsx(23 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/ChatInterface.jsx (3)
src/App.jsx (5)
showThinking(60-60)selectedSession(50-50)processingSessions(71-71)selectedProject(49-49)sendByCtrlEnter(62-62)src/utils/websocket.js (2)
ws(4-4)sendMessage(95-101)src/utils/api.js (3)
url(53-53)authenticatedFetch(2-20)authenticatedFetch(2-20)
🪛 Biome (2.1.2)
src/components/ChatInterface.jsx
[error] 2048-2050: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2333-2335: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2385-2386: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2430-2432: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2469-2471: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2497-2498: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2498-2500: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (2)
src/components/ChatInterface.jsx (2)
2712-2806: LGTM! Token usage polling properly handles race conditions.The implementation correctly addresses the race condition concern from previous reviews:
- Uses
AbortControllerto cancel in-flight requests on cleanup- Captures session/project IDs before the fetch
- Verifies IDs still match current state before calling
setTokenBudget- Handles errors and aborted requests gracefully
The polling logic is well-structured with proper fallbacks to zero for new sessions and failed fetches.
34-43: LGTM! HTML entity decoding and TokenUsagePie integration are well-implemented.The
decodeHtmlEntitieshelper is correctly applied across all message parsing paths (Cursor messages, session messages, streaming deltas, and WebSocket responses), ensuring consistent handling of HTML entities in content.The
TokenUsagePiecomponent is properly integrated with appropriate fallbacks:used={tokenBudget?.used || 0} total={tokenBudget?.total || parseInt(import.meta.env.VITE_CONTEXT_WINDOW) || 160000}This addresses the past review comment about avoiding code duplication and ensures robust handling of null/undefined tokenBudget.
Also applies to: 3504-3507
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/TokenUsagePie.jsx (2)
3-3: Consider adding PropTypes or TypeScript for type safety.The component assumes
usedandtotalare numbers but doesn't enforce this. If non-numeric values are passed (e.g., strings like"abc"), line 46'stoLocaleString()or line 47'stoFixed()could throw runtime errors. Adding PropTypes or migrating to TypeScript would catch these issues at development time.Example using PropTypes:
import React from 'react'; import PropTypes from 'prop-types'; function TokenUsagePie({ used, total }) { // ... existing code } TokenUsagePie.propTypes = { used: PropTypes.number, total: PropTypes.number, }; export default TokenUsagePie;Also applies to: 6-6, 46-47
22-45: Enhance accessibility with ARIA attributes.The SVG lacks proper ARIA attributes, which can make it difficult for screen reader users to understand the token usage indicator. Adding
role="img"andaria-labelwould improve accessibility.Apply this diff to add ARIA attributes:
- <svg width="24" height="24" viewBox="0 0 24 24" className="transform -rotate-90"> + <svg + width="24" + height="24" + viewBox="0 0 24 24" + className="transform -rotate-90" + role="img" + aria-label={`Token usage: ${percentage.toFixed(0)}% (${used.toLocaleString()} of ${total.toLocaleString()} tokens)`} + >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/TokenUsagePie.jsx(1 hunks)
🔇 Additional comments (1)
src/components/TokenUsagePie.jsx (1)
6-6: Zero-token handling is now correct.The fix from the previous review has been applied correctly. The guard now uses
used == null || total == null || total <= 0, which properly renders a 0% pie whenused === 0while still preventing division by zero or rendering on invalid totals.
|
Awesome work! I thought you couldn't use the Claude Agent SDK with your Claude Code Max plan? |
There was a problem hiding this comment.
@joshwilhelmi when I tested this I was always getting a message "warmup" as if that was coming from me. Can you make sure this does not appear to the users? Also I saw that the session on the sidebar is named the same.
That's because I got rid of all the < system messages in the session titles >. An empty session gets labeled Warmup by Claude Code. Do you want me to revert it back to "system messages that look weird"? |
|
@joshwilhelmi no that makes sense then but still in the chat there is the "warmup" message as it's coming from the user. I would like to merge this PR as it's very useful and we could continue developing on the SDK si the following point can be done as a new PR. |
I've fixed this and a bunch of other stuff in PR #211. It's dependent on #208. I was still getting my GitHub setup correctly when I started working on this. If it helps, I can close both and submit a clean PR. |
3 participants
Multiple Features, Improvements, and Bug Fixes
🎯 Overview
This PR consolidates multiple features, improvements, and bug fixes for Claude Code UI. All changes are production-ready and address various UX issues, performance optimizations, and add new functionality.
📦 New Features
1. Token Budget Visualization ⭐
What: Real-time token usage tracking with visual pie chart display
Features:
modelUsagefieldtoken-budgetevents)Files Changed:
src/components/TokenUsagePie.jsx- New component for visualizationserver/claude-cli.js- Parse and send token usage datasrc/components/ChatInterface.jsx- Integrate token budget statesrc/App.jsx- Pass token budget to componentssrc/components/MainContent.jsx- Display token usage widget2. Session Processing State Persistence ⭐
Problem: When Claude is thinking/processing and users switch to another session, the "Thinking..." banner and stop button disappear. Returning to the original session shows no indication it's still processing.
Solution:
processingSessionsSet (App.jsx)check-session-statusmessage type)Technical Details:
isClaudeSessionActive,isCursorSessionActive,getActiveClaudeSessions,getActiveCursorSessionssessionIdfor proper cleanupFiles Changed:
src/App.jsx- Processing state tracking and callbackssrc/components/MainContent.jsx- Pass-through propssrc/components/ChatInterface.jsx- UI restoration logic, session status handlingserver/claude-cli.js- Export helpers, add sessionId to messagesserver/cursor-cli.js- Export helpers, add sessionId to messagesserver/index.js- WebSocket handlers for session status queries3. Configurable Context Window 🔧
Problem: Context window size (160k tokens) was hardcoded in multiple places, making it difficult to adjust for different Claude models or use cases.
Solution:
CONTEXT_WINDOWenvironment variable for backend (default: 160000)VITE_CONTEXT_WINDOWenvironment variable for frontend (default: 160000).envfileWhy 160k? Claude Code reserves ~40k tokens for auto-compact, leaving 160k for actual usage.
Files Changed:
.env.example- New environment variables with documentationserver/index.js- Token usage calculation uses env varsrc/components/ChatInterface.jsx- Pie chart uses env var4. Show Thinking Toggle 💭
What: Quick settings option to show/hide Claude's reasoning sections in messages
Features:
<thinking>blocks in Claude responsesFiles Changed:
src/components/QuickSettingsPanel.jsx- Toggle UIsrc/components/ChatInterface.jsx- Apply thinking visibility state5. Cache Clearing Utility 🧹
What: Dedicated page for clearing service workers, caches, and storage
Features:
/clear-cache.htmlFiles Changed:
public/clear-cache.html- New utility page🚀 Improvements
Package Upgrades
xtermto@xterm/*scoped packagesFiles Changed:
package.json- Updated dependenciespackage-lock.json- Lockfile updatessrc/components/Shell.jsx- Import from@xterm/*Build Optimization
What: Code splitting for major vendors to improve initial load time
Changes:
Files Changed:
vite.config.js- Manual chunks configurationDeployment Scripts
What: Shell scripts for cleaner server management
Features:
scripts/start.sh- Starts server with automatic port conflict resolutionscripts/stop.sh- Stops server cleanly by killing processes on configured portsFiles Changed:
scripts/start.sh- New deployment scriptscripts/stop.sh- New deployment script🐛 Bug Fixes
1. Token Budget Reset for New Sessions
Problem: Starting a new session would show stale token usage data from previous session instead of resetting to 0%.
Solution:
Files Changed:
src/components/ChatInterface.jsx- Token usage polling with reset logic2. Static File Serving
Problem: Routes vs assets not properly distinguished
Solution:
Files Changed:
server/index.js- Static file serving logic3. Session State Preservation
Problem: Session state would reset incorrectly on initial load
Solution:
Files Changed:
src/components/ChatInterface.jsx- Session state handling4. Vite Dev Server Port
Problem: Non-standard dev server port
Solution:
Files Changed:
vite.config.js- Port configuration5. Session Flickering on Background Updates
Problem: Pages flickered when sessions were updated outside the foreground due to WebSocket project updates always creating new object references, triggering unnecessary ChatInterface reloads.
Solution:
Files Changed:
src/App.jsx- WebSocket message handler (line 200)🧪 Testing Checklist
Token Budget Visualization
Session State Persistence
Configurable Context Window
.envfile hasCONTEXT_WINDOW=160000CONTEXT_WINDOW=100000in.envShow Thinking Toggle
Cache Clearing
/clear-cache.html📊 Files Changed Summary
Total: 15 files modified, 2 new components
🚀 Migration Guide
For Users
No action required - changes are backward compatible.
For Developers
Copy new environment variables from
.env.exampleto your.env:Restart development server to pick up new environment variables
Optional: Use new deployment scripts:
🎉 Benefits
Summary by CodeRabbit
New Features
Improvements
Chores