feat: Auto-Compact Token Monitoring - Prevent Context Loss in Long Sessions #212
Conversation
- Add token budget parsing and monitoring in claude-cli.js - Create TokenBudgetIndicator component with color-coded progress bar - Create AutoCompactNotification component for toast notifications - Integrate WebSocket handlers in ChatInterface for token updates - Add auto-compact settings UI in Settings.jsx - Auto-trigger /context save when tokens < 30,000 remaining - 5-minute cooldown to prevent auto-compact loops - User-configurable threshold and notification preferences Implements auto-compact feature from AUTO_COMPACT_IMPLEMENTATION_SUMMARY.md
- Add AUTO_COMPACT_FEATURE_PROPOSAL.md with complete technical design - Add AUTO_COMPACT_IMPLEMENTATION_SUMMARY.md for quick reference - Documents architecture, implementation phases, and integration points - Includes testing checklist and success criteria
WalkthroughIntroduces an Auto‑Compact feature: backend token‑budget parsing, per‑session tracking and auto‑compact trigger/workflow with WebSocket events; new frontend TokenBudgetIndicator and AutoCompactNotification components; ChatInterface and Settings UI integration; and documentation and implementation notes. Changes
Sequence Diagram(s)sequenceDiagram
participant Claude as Claude Process
participant Backend as server/claude-cli.js
participant WS as WebSocket Server
participant Frontend as ChatInterface
participant User as User
Claude->>Backend: stream stdout (system warnings / token info)
activate Backend
Backend->>Backend: parseSystemWarnings() -> tokenData
Backend->>WS: emit token-budget-update(sessionId, tokenData)
deactivate Backend
WS->>Frontend: token-budget-update
activate Frontend
Frontend->>Frontend: set tokenBudget(tokenData)
Frontend->>User: render TokenBudgetIndicator
deactivate Frontend
Note over Backend,Frontend: if remaining < threshold and not cooling down
Backend->>Backend: shouldTriggerAutoCompact() == true
Backend->>WS: emit auto-compact-triggered
WS->>Frontend: auto-compact-triggered
activate Frontend
Frontend->>Frontend: show AutoCompactNotification(triggered)
Frontend->>User: show triggered toast
deactivate Frontend
Backend->>Backend: executeContextSave() (/context save)
Backend->>Claude: run /context save
Claude->>Backend: output (tokensSaved or error)
Backend->>Backend: parseTokensSavedFromOutput()
Backend->>WS: emit auto-compact-complete(tokensSaved) or auto-compact-error
WS->>Frontend: auto-compact-complete / auto-compact-error
activate Frontend
Frontend->>Frontend: update notification (complete -> auto-dismiss / show error)
Frontend->>User: show completion or error toast
deactivate Frontend
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (1)server/claude-cli.js (3)
🪛 Biome (2.1.2)server/claude-cli.js[error] 33-33: Unexpected control character in a regular expression. Control characters are unusual and potentially incorrect inputs, so they are disallowed. (lint/suspicious/noControlCharactersInRegex) 🔇 Additional comments (7)
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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
server/claude-cli.js (2)
199-244: Image note never reaches the CLI command.You build modifiedCommand before --print is added, then never update command. Result: images are saved but paths aren’t referenced in the prompt.
Apply:
- // Build Claude CLI command - start with print/resume flags first + // Build Claude CLI command - start with print/resume flags first const args = []; + // Command that will actually be sent (may be augmented with image note) + let commandToSend = command; ... - if (tempImagePaths.length > 0 && command && command.trim()) { + if (tempImagePaths.length > 0 && command && command.trim()) { const imageNote = `\n\n[Images provided at the following paths:]\n${tempImagePaths.map((p, i) => `${i + 1}. ${p}`).join('\n')}`; - const modifiedCommand = command + imageNote; - - // Update the command in args - now that --print and command are separate - const printIndex = args.indexOf('--print'); - if (printIndex !== -1 && printIndex + 1 < args.length && args[printIndex + 1] === command) { - args[printIndex + 1] = modifiedCommand; - } + commandToSend = command + imageNote; } ... - if (command && command.trim()) { + if (commandToSend && commandToSend.trim()) { // Separate arguments for better cross-platform compatibility ... - args.push(command); + args.push(commandToSend); }Also applies to: 388-397
282-288: Project‑specific MCP check uses process.cwd(); should use workingDir.process.cwd() is the server’s cwd, not the project’s. This mis-detects project MCP servers.
Apply:
- const currentProjectPath = process.cwd(); + const currentProjectPath = workingDir; ... - const currentProjectPath = process.cwd(); + const currentProjectPath = workingDir;Also applies to: 306-309
src/components/Settings.jsx (1)
389-398: process.cwd() in browser can throw; guard or remove.process is undefined in browsers without a polyfill.
Apply:
- setSelectedProject(projects?.[0] || { name: 'default', fullPath: process.cwd() }); + const fallbackPath = (typeof process !== 'undefined' && typeof process.cwd === 'function') ? process.cwd() : ''; + setSelectedProject(projects?.[0] || { name: 'default', fullPath: fallbackPath });Apply similarly in handleCursorLogin.
🧹 Nitpick comments (13)
server/claude-cli.js (1)
427-499: Safer WebSocket sends (prevent throw on closed sockets).Wrap ws.send with a readiness check.
Apply once and reuse:
+function safeSend(ws, payload) { + try { + if (ws && ws.readyState === 1 /* OPEN */) { + ws.send(JSON.stringify(payload)); + } + } catch (e) { + console.warn('WS send failed:', e.message); + } +}Then replace occurrences like:
-ws.send(JSON.stringify({ type: 'token-budget-update', data: { ...tokenData, sessionId: capturedSessionId } })); +safeSend(ws, { type: 'token-budget-update', data: { ...tokenData, sessionId: capturedSessionId } });Would you like me to submit a follow-up PR with these replacements?
src/components/Settings.jsx (1)
691-699: Redundant nested appearance check.{activeTab === 'appearance'} is checked twice; remove inner duplicate wrapper.
src/components/ChatInterface.jsx (2)
2187-2191: Only update tokenBudget for the current session when server includes sessionId.Prevents cross-session bleed once backend adds it.
Apply:
- case 'token-budget-update': - // Update token budget display - setTokenBudget(latestMessage.data); + case 'token-budget-update': { + const sid = latestMessage.data?.sessionId; + if (!sid || sid === currentSessionId) { + setTokenBudget(latestMessage.data); + } break; + }
3002-3007: Optional: Hide toast in render if setting is off (defense in depth).If you prefer a render-time guard instead of message gating:
- <AutoCompactNotification - notification={autoCompactNotification} - onDismiss={() => setAutoCompactNotification(null)} - /> + {(() => { + try { + const s = JSON.parse(localStorage.getItem('claude-settings') || '{}'); + if (s.showAutoCompactNotifications === false) return null; + } catch {} + return ( + <AutoCompactNotification + notification={autoCompactNotification} + onDismiss={() => setAutoCompactNotification(null)} + /> + ); + })()}AUTO_COMPACT_IMPLEMENTATION_SUMMARY.md (4)
42-52: Token parsing regex needs robustness validation.The regex pattern assumes a specific format for system warnings, but Claude CLI output may vary across versions or contexts. Consider:
- What if the format includes commas in numbers (e.g.,
Token usage: 95,000/200,000)?- What if the message is prefixed/suffixed with extra text?
- How will parsing degrade gracefully if the pattern doesn't match?
The current implementation returns
nullon mismatch, which is reasonable, but the proposal doesn't specify fallback behavior or logging for debugging failed parses.Consider adding:
function parseSystemWarnings(output) { // Handle both formats: with and without commas const match = output.match(/Token usage:\s*(\d+,?\d*)\s*\/\s*(\d+,?\d*);\s*(\d+,?\d*)\s*remaining/i); if (match) { return { used: parseInt(match[1].replace(/,/g, '')), total: parseInt(match[2].replace(/,/g, '')), remaining: parseInt(match[3].replace(/,/g, '')) }; } console.warn('Failed to parse token budget from output:', output); return null; }
57-77: Backend logic snippets in SUMMARY are incomplete.Lines 57–59 show
shouldTriggerAutoCompact(tokenData)taking onlytokenData, but the full implementation (per PROPOSAL.md lines 81–92) requiressessionIdand cooldown tracking. This discrepancy could confuse developers reading the SUMMARY as a standalone guide.Recommend updating the SUMMARY version to match the full signature or adding a note referring to PROPOSAL.md for complete details.
Update line 57 to match the full implementation:
function shouldTriggerAutoCompact(sessionId, tokenData) { if (tokenData.remaining < 30000) { const lastCompact = sessionTokenUsage.get(sessionId)?.lastCompactTime; const now = Date.now(); if (!lastCompact || (now - lastCompact) > 300000) { // 5 min cooldown return true; } } return false; }(Note: This is already detailed in PROPOSAL.md lines 81–92.)
119-139: Settings UI example lacks critical details for backend integration.The Settings JSX example (lines 123–138) shows the UI controls but omits:
- How settings are persisted to backend
- Default values in the input components
- API endpoint or message type for transmitting threshold changes
- Whether threshold changes apply to current or future sessions
These details are essential for end-to-end functionality.
Clarify the example with:
- Default value in the input:
value={threshold || 30000}- Handler to persist changes:
onSettingsChange(autoCompactSettings)- Note indicating backend integration is required
Or reference PROPOSAL.md for more complete details.
186-197: Testing checklist covers happy path but omits critical edge cases.Missing scenarios:
- Concurrent auto-compact requests from the same session
- Network failures or timeouts during context save
- Malformed or unexpected system warning formats
- Session cleanup and token tracking memory leaks
- Auto-compact triggering while a previous one is in progress
- Settings changes while auto-compact is active
These edge cases could surface bugs in production.
Expand the testing checklist to include:
- [ ] Auto-compact handles concurrent requests gracefully - [ ] Network timeouts during context save are handled with retry/fallback - [ ] Malformed system warnings don't crash token parsing - [ ] Session cleanup properly removes token tracking data - [ ] Settings changes propagate to backend correctly - [ ] Auto-compact skips if previous compact is in progressAUTO_COMPACT_FEATURE_PROPOSAL.md (5)
61-79: Hardcoded token budget needs to be documented as configurable assumption.Lines 61–63 hardcode
TOKEN_BUDGET_TOTAL = 200000based on current Claude model limits, but this may vary:
- Different Claude models (claude-3-opus vs claude-3-sonnet) have different budgets
- Future Claude versions may increase or decrease budget
- Users may run locally with custom budgets
The implementation should at least document this assumption and consider making it configurable or fetched from Claude's system info.
Add a comment explaining the assumption:
// TODO: Make configurable or fetch from Claude system info // Currently assumes 200k budget for Claude 3 Opus // May vary for other models or custom deployments const TOKEN_BUDGET_TOTAL = 200000;Or better: fetch from Claude's initial system message if available.
81-92: Cooldown period (5 minutes) lacks justification.Line 87 hardcodes a 5-minute cooldown (
(now - lastCompact) > 300000) to prevent auto-compact loops, but no rationale is provided for why 5 minutes vs 2 minutes, 10 minutes, etc.Consider: If a user's context barely recovers after compression, they might hit the threshold again in < 5 minutes, leading to frustration. Conversely, 5 minutes might be too short if context grows quickly.
Either:
- Document why 5 minutes is chosen (e.g., "empirically observed recovery time")
- Make cooldown configurable in settings (e.g., 1–10 minutes)
- Use adaptive cooldown based on tokens saved (e.g., longer cooldown for smaller savings)
For now, add a comment explaining the choice:
// 5-minute cooldown: empirically chosen to prevent loops while allowing // users to work on new problems that trigger context growth const COOLDOWN_MS = 300000;
350-355: Settings persistence architecture not specified.The Settings component state (lines 350–355) defines
autoCompactSettings, but the proposal doesn't detail:
- How settings persist across page reloads (localStorage vs backend database?)
- How frontend settings sync with backend (API endpoint? WebSocket message?)
- How backend applies threshold changes to active sessions (immediately? after next compact?)
- Whether settings are per-user or per-session
Without this clarity, settings integration could be incomplete or inconsistent.
Clarify the settings architecture:
**Settings Persistence**: - Frontend: Store in localStorage under `claudecodeui:autoCompactSettings` - Backend: Read from localStorage on page load via API call - Updates: Send to backend via POST `/api/settings` to persist to user database - Active sessions: Apply threshold changes immediately via WebSocket `settings-update` messageAdd this to Phase 4 section (lines 477–483).
541-546: Risk table assessment of "Token parsing accuracy" may be overly optimistic.Line 546 rates token parsing accuracy risk as "Low" likelihood, but the regex-based approach (lines 68–79) is fragile:
- Minor Claude CLI output format changes break parsing
- Localization (non-English system messages) could cause failures
- Edge cases (missing tokens in some outputs, malformed messages) are likely
"Low" likelihood seems optimistic; "Medium" would be more realistic.
Revise the risk table:
| Token parsing accuracy | Medium | Medium | Comprehensive testing with multiple Claude models and versions; regex validation and fallback logic; detailed logging for failed parses |Also add to mitigation: "Version detection to adapt regex patterns for Claude CLI version changes."
514-523: Success criteria lack verification methods.Success criteria (lines 516–523) are reasonable but vague:
- "Displays correctly" — what constitutes correct display?
- "Seamlessly continues" — how is seamlessness measured?
- "Cooldown prevents loops" — what's the test for this?
Without clear verification methods, success criteria can't be validated during testing.
Make success criteria testable:
1. ✅ Token budget displays in UI within 1 second of stream-json event 2. ✅ Auto-compact triggers when remaining ≤ 30,000 (configurable) and ≥ 5 min since last compact 3. ✅ Notification appears within 2 seconds and auto-dismisses after 5 seconds (success) or requires manual dismiss (error) 4. ✅ Context save completes within 30 seconds with tokens saved ≥ 50,000 5. ✅ Threshold configurable via Settings UI and persists across reloads 6. ✅ No auto-compact occurs within 5 minutes of previous compact (test by artificially triggering threshold twice) 7. ✅ Token budget display functional on mobile (320px width) and desktop (1920px width) 8. ✅ Session continues without errors for ≥ 10 minutes post-compact
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
AUTO_COMPACT_FEATURE_PROPOSAL.md(1 hunks)AUTO_COMPACT_IMPLEMENTATION_SUMMARY.md(1 hunks)server/claude-cli.js(3 hunks)src/components/AutoCompactNotification.jsx(1 hunks)src/components/ChatInterface.jsx(4 hunks)src/components/Settings.jsx(2 hunks)src/components/TokenBudgetIndicator.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/TokenBudgetIndicator.jsx (1)
src/lib/utils.js (1)
cn(4-6)
server/claude-cli.js (3)
src/components/TokenBudgetIndicator.jsx (1)
tokenData(25-25)src/components/Shell.jsx (1)
ws(36-36)src/utils/websocket.js (1)
ws(4-4)
🔇 Additional comments (4)
src/components/AutoCompactNotification.jsx (1)
21-113: LGTM — notification lifecycle and UX match spec.Auto-dismiss only on success, explicit dismiss for errors. Clean and focused.
server/claude-cli.js (1)
456-474: Verify frontend integration before applying backend changes.The review comment correctly identifies where to initialize
sessionTokenUsage(lines 246-249 and 456-474), but verification reveals an incomplete feature integration. While the backend code changes are technically sound, the frontend does not currently sendautoCompactEnabledandautoCompactThresholdsettings to the backend. The Settings component defines these values (src/components/Settings.jsx lines 32-33), but they are not included in thetoolsSettingspayload sent to the backend (src/components/ChatInterface.jsx line 2811).Apply these backend changes only after confirming that:
- Frontend's
getToolsSettings()includesautoCompactEnabledandautoCompactThreshold- These values are passed through
toolsSettingsin the claude-command message- Backend receives and correctly accesses them via the
settingsvariable (renamed fromtoolsSettingson line 187 of server/claude-cli.js)AUTO_COMPACT_FEATURE_PROPOSAL.md (2)
405-452: Decision between context save options (A vs B) not finalized—risk of incomplete implementation.Lines 405–452 present two approaches for executing
/context save:
- Option A (lines 410–440): Spawn separate Claude process with
--resume- Option B (lines 445–451): Send command to stdin of current process
The proposal doesn't decide which to use. This ambiguity could lead to:
- Implementation choosing the wrong approach and needing rework
- Incomplete handling of edge cases for the unchosen path
- Confusion during code review on which path is production-ready
Critical decision needed: Which approach is preferred? Option A is safer (isolated process, clear completion) but may create a new session ID that needs tracking. Option B is simpler but requires tracking completion in stream parsing, which is complex and error-prone.
Please clarify:
- Which context save approach will be used?
- If Option A: How will session ID changes be tracked and communicated to frontend?
- If Option B: How will auto-compact completion be detected in the stream? (This seems underspecified.)
Recommend Option A for robustness, with explicit session tracking.
499-504: Challenge 3 (session continuity) solution is underspecified.The proposal acknowledges that
/context savecreates a new session ID (Challenge 3, lines 502–503), but the mitigation ("Track session ID changes and update frontend session reference") lacks critical implementation details:
- Detection: How does the backend detect the new session ID?
- Communication: How does the backend notify the frontend of the ID change?
- State consistency: How are pending frontend requests handled during the transition?
- Persistence: If the page reloads after context save, how does the frontend know to use the new session ID?
Without these details, auto-compact could leave the UI in an inconsistent state.
Please clarify:
- Does
claude --resume <sessionId> --print "/context save"return the new session ID in output?- Should the new session ID be communicated via a WebSocket message (e.g.,
{ type: 'session-changed', data: { oldId, newId } })?- Should chat history be cleared after context save, or should the UI transition seamlessly to the new session?
This should be added to Phase 3 implementation details (lines 470–475).
Fixes all 10 CodeRabbit review issues on PR siteboon#212: Backend (server/claude-cli.js): - Add in-progress guard (activeAutoCompacts Set) to prevent double-trigger - Enhance regex robustness: handle commas, case-insensitive matching - Honor CLAUDE_CLI_PATH environment variable for context save - Return null for unknown tokensSaved instead of inflated estimate - Include sessionId in token-budget-update WebSocket events - Wire user settings (autoCompactThreshold, autoCompactEnabled) to backend Frontend (Settings.jsx): - Persist auto-compact settings in localStorage (load/save) - Add input validation: clamp threshold to 10000-100000, handle NaN Frontend (TokenBudgetIndicator.jsx): - Guard against division by zero when calculating percentage Frontend (ChatInterface.jsx): - Respect showAutoCompactNotifications user setting Technical details: - tokenCriticalThreshold now configurable via toolsSettings - Settings stored as 'claude-settings' in localStorage - Validation ensures threshold range 10000-100000 - activeAutoCompacts prevents concurrent /context save executions Co-authored-by: CodeRabbit <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/components/TokenBudgetIndicator.jsx (1)
25-64: Handle undefined token metrics to avoid crashes
toLocaleStringon an undefinedremaining(orused/total) blows up as soon as the backend omits a field, and the progress bar can still emit NaN/overflow because we never clamp. Please normalise the payload first (fallback to 0, coerce to numbers, clamp the percentage) before rendering. This was called out in the earlier review and still needs fixing. Apply something like:- const { used, total, remaining } = tokenData; - // Guard against division by zero - const percentage = total > 0 ? (used / total) * 100 : 0; + const { + used: rawUsed = 0, + total: rawTotal = 0, + remaining: rawRemaining = 0, + } = tokenData ?? {}; + const used = Number.isFinite(rawUsed) ? rawUsed : 0; + const total = Number.isFinite(rawTotal) ? rawTotal : 0; + const remaining = Number.isFinite(rawRemaining) ? rawRemaining : 0; + const rawPercentage = total > 0 ? (used / total) * 100 : 0; + const percentage = Math.max(0, Math.min(100, rawPercentage));…and keep the rest of the component referencing these sanitised values.
server/claude-cli.js (1)
13-71: Store auto-compact thresholds per session
tokenCriticalThresholdis still a process-wide mutable singleton. When two sessions run with different thresholds (or one disables auto-compact), the most recent call tospawnClaudewins and every other session inherits that setting, so the trigger either fires too early or never fires at all. Please persist theautoCompactEnabled/autoCompactThresholdvalues alongside each session’s entry insessionTokenUsage(initialise when the session is created/captured, fall back to defaults per session) and haveshouldTriggerAutoCompactread from that per-session state instead of a shared global. This was flagged previously and remains unresolved.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
server/claude-cli.js(3 hunks)src/components/ChatInterface.jsx(4 hunks)src/components/Settings.jsx(4 hunks)src/components/TokenBudgetIndicator.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/TokenBudgetIndicator.jsx (1)
src/lib/utils.js (1)
cn(4-6)
server/claude-cli.js (3)
src/components/TokenBudgetIndicator.jsx (1)
tokenData(25-25)server/routes/mcp.js (7)
process(24-26)process(109-109)process(202-202)process(270-272)process(313-315)lines(470-470)lines(532-532)src/components/Settings.jsx (1)
autoCompactEnabled(32-32)
🪛 Biome (2.1.2)
src/components/ChatInterface.jsx
[error] 2196-2197: 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] 2198-2201: 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)
src/components/ChatInterface.jsx
Outdated
| case 'token-budget-update': | ||
| // Update token budget display | ||
| setTokenBudget(latestMessage.data); | ||
| break; | ||
|
|
||
| case 'auto-compact-triggered': | ||
| case 'auto-compact-complete': | ||
| case 'auto-compact-error': |
There was a problem hiding this comment.
Ignore token updates for other sessions
The new handler applies every token-budget-update to the current view, even when the payload belongs to a different Claude session. That cross-talk will surface as the wrong budget bar when multiple sessions are active. Please key off latestMessage.data.sessionId (covering currentSessionId, selectedSession?.id, or the pending session id used during creation) and only call setTokenBudget when it matches; otherwise ignore the update.
🤖 Prompt for AI Agents
In src/components/ChatInterface.jsx around lines 2187 to 2194, the token budget
update handler currently applies every 'token-budget-update' to the current view
regardless of which Claude session it belongs to; change it to read
latestMessage.data.sessionId and only call setTokenBudget when that sessionId
matches the active session context (compare against currentSessionId,
selectedSession?.id, or the pending session id used during creation), otherwise
ignore the update; make the comparison null-safe (handle undefined session ids)
and short-circuit early to avoid cross-talk between sessions.
| // Display auto-compact notification only if user has enabled notifications | ||
| const savedSettings = localStorage.getItem('claude-settings'); | ||
| const showNotifications = savedSettings | ||
| ? JSON.parse(savedSettings).showAutoCompactNotifications !== false | ||
| : true; // default true | ||
|
|
||
| if (showNotifications) { | ||
| setAutoCompactNotification(latestMessage); | ||
| } | ||
| break; | ||
|
|
There was a problem hiding this comment.
Scope auto-compact notifications and wrap the switch clause
Like the budget handler, auto-compact events need to be filtered to the active session; otherwise a background session will spook the foreground chat. Also, declaring const savedSettings directly in the case body violates Biome’s noSwitchDeclarations rule and breaks the lint step. Wrap the case in braces, guard the JSON.parse, and only call setAutoCompactNotification when both (a) notifications are enabled and (b) the payload’s session id matches the active session. Example:
- case 'auto-compact-triggered':
- case 'auto-compact-complete':
- case 'auto-compact-error':
- // Display auto-compact notification only if user has enabled notifications
- const savedSettings = localStorage.getItem('claude-settings');
- const showNotifications = savedSettings
- ? JSON.parse(savedSettings).showAutoCompactNotifications !== false
- : true; // default true
-
- if (showNotifications) {
- setAutoCompactNotification(latestMessage);
- }
- break;
+ case 'auto-compact-triggered':
+ case 'auto-compact-complete':
+ case 'auto-compact-error': {
+ const { sessionId: notifSessionId } = latestMessage.data ?? {};
+ if (!isRelevantSession(notifSessionId)) break;
+
+ let showNotifications = true;
+ const stored = localStorage.getItem('claude-settings');
+ if (stored) {
+ try {
+ const parsed = JSON.parse(stored);
+ showNotifications = parsed.showAutoCompactNotifications !== false;
+ } catch (err) {
+ console.warn('Failed to read claude-settings', err);
+ }
+ }
+ if (showNotifications) {
+ setAutoCompactNotification(latestMessage);
+ }
+ break;
+ }(where isRelevantSession is a small helper defined before the switch).
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (2.1.2)
[error] 2196-2197: 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] 2198-2201: 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)
🤖 Prompt for AI Agents
In src/components/ChatInterface.jsx around lines 2195 to 2205, the auto-compact
notification case both declares a const inside the switch (violating
noSwitchDeclarations) and unconditionally triggers notifications for background
sessions; wrap the switch case body in braces, move/limit variable declarations
to the block scope, guard the JSON.parse call by checking savedSettings exists
before parsing, and only call setAutoCompactNotification(latestMessage) when
parsed settings indicate notifications are enabled AND the payload’s sessionId
matches the current active session (use the existing isRelevantSession helper or
compare to activeSessionId).
…fensive code
Fixes 2 critical CodeRabbit issues from latest review:
Backend (server/claude-cli.js):
- Change tokenCriticalThreshold from global to per-session storage
- Store autoCompactEnabled and autoCompactThreshold in sessionTokenUsage Map
- Initialize settings when session ID is captured or provided
- Read per-session settings in shouldTriggerAutoCompact()
- Prevents cross-session setting pollution in multi-session scenarios
Frontend (TokenBudgetIndicator.jsx):
- Add proper undefined/NaN handling with fallbacks to 0
- Sanitize rawUsed, rawTotal, rawRemaining with Number.isFinite check
- Clamp percentage to 0-100 range to prevent overflow
- Prevents crashes from malformed token data
Frontend (ChatInterface.jsx):
- Wrap switch case variables in block scope to fix Biome lint error
- Prevents variable leakage across switch cases
Technical details:
- sessionTokenUsage structure: { lastCompactTime, autoCompactEnabled, autoCompactThreshold }
- Settings initialized via initializeSessionSettings() helper
- Called when sessionId provided or captured from stream
- Default: autoCompactEnabled=true, threshold=30000
Addresses CodeRabbit feedback on commit d39d433
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 (3)
server/claude-cli.js (3)
258-303: Image note never applied (args mutated before --print is added).You try to replace the
commandToSendfirst, then push it when addingApply:
@@ - const tempImagePaths = []; + const tempImagePaths = []; + // Build command payload lazily so we can append image notes before pushing args + let commandToSend = command; @@ - // Include the full image paths in the prompt for Claude to reference - // Only modify the command if we actually have images and a command - if (tempImagePaths.length > 0 && command && command.trim()) { - const imageNote = `\n\n[Images provided at the following paths:]\n${tempImagePaths.map((p, i) => `${i + 1}. ${p}`).join('\n')}`; - const modifiedCommand = command + imageNote; - - // Update the command in args - now that --print and command are separate - const printIndex = args.indexOf('--print'); - if (printIndex !== -1 && printIndex + 1 < args.length && args[printIndex + 1] === command) { - args[printIndex + 1] = modifiedCommand; - } - } + // Include the full image paths in the prompt for Claude to reference + if (tempImagePaths.length > 0 && command && command.trim()) { + const imageNote = `\n\n[Images provided at the following paths:]\n${tempImagePaths.map((p, i) => `${i + 1}. ${p}`).join('\n')}`; + commandToSend = command + imageNote; + } @@ - if (command && command.trim()) { + if (command && command.trim()) { @@ - args.push(command); + args.push(commandToSend);Also applies to: 447-456
485-509: NDJSON parsing can break on chunk boundaries; buffer across chunks.Splitting each data chunk by newline risks parsing partial JSON or misclassifying it as text. Buffer and only parse complete lines.
Apply:
@@ - // Handle stdout (streaming JSON responses) - claudeProcess.stdout.on('data', (data) => { - const rawOutput = data.toString(); - console.log('📤 Claude CLI stdout:', rawOutput); + // Handle stdout (streaming JSON responses) with line buffering + let stdoutBuffer = ''; + claudeProcess.stdout.on('data', (data) => { + const rawOutput = data.toString(); + console.log('📤 Claude CLI stdout chunk:', rawOutput); + stdoutBuffer += rawOutput; @@ - // Parse system warnings for token budget BEFORE JSON parsing - const tokenData = parseSystemWarnings(rawOutput); - if (tokenData && capturedSessionId) { - console.log(`💰 Token budget update: ${tokenData.used}/${tokenData.total} (${tokenData.remaining} remaining)`); - - // Send token budget update to frontend with sessionId - ws.send(JSON.stringify({ - type: 'token-budget-update', - data: { - ...tokenData, - sessionId: capturedSessionId - } - })); - - // Check if auto-compact should trigger (uses per-session settings) - if (shouldTriggerAutoCompact(capturedSessionId, tokenData)) { - triggerAutoCompact(capturedSessionId, tokenData, ws); - } - } - - const lines = rawOutput.split('\n').filter(line => line.trim()); + const lines = stdoutBuffer.split('\n'); + stdoutBuffer = lines.pop() ?? ''; + // Process each complete line + for (const lineRaw of lines) { + const line = lineRaw.trim(); + if (!line) continue; + // Parse token warnings line-by-line + const tokenData = parseSystemWarnings(line); + if (tokenData && capturedSessionId) { + console.log(`💰 Token budget update: ${tokenData.used}/${tokenData.total} (${tokenData.remaining} remaining)`); + ws.send(JSON.stringify({ + type: 'token-budget-update', + data: { ...tokenData, sessionId: capturedSessionId } + })); + if (shouldTriggerAutoCompact(capturedSessionId, tokenData)) { + triggerAutoCompact(capturedSessionId, tokenData, ws); + } + } + // Then try JSON + try { + const response = JSON.parse(line); + console.log('📄 Parsed JSON response:', response); + // Capture session ID if it's in the response + if (response.session_id && !capturedSessionId) { + capturedSessionId = response.session_id; + console.log('📝 Captured session ID:', capturedSessionId); + // Initialize auto-compact settings for newly captured session + initializeSessionSettings(capturedSessionId); + // Update process key with captured session ID + if (processKey !== capturedSessionId) { + activeClaudeProcesses.delete(processKey); + activeClaudeProcesses.set(capturedSessionId, claudeProcess); + } + // Send session-created event only once for new sessions + if (!sessionId && !sessionCreatedSent) { + sessionCreatedSent = true; + ws.send(JSON.stringify({ type: 'session-created', sessionId: capturedSessionId })); + } + } + // Send parsed response to WebSocket + ws.send(JSON.stringify({ type: 'claude-response', data: response })); + } catch { + // Not JSON; forward as raw text + ws.send(JSON.stringify({ type: 'claude-output', data: line })); + } + } - for (const line of lines) { - try { - const response = JSON.parse(line); - console.log('📄 Parsed JSON response:', response); - // Capture session ID if it's in the response - if (response.session_id && !capturedSessionId) { - capturedSessionId = response.session_id; - console.log('📝 Captured session ID:', capturedSessionId); - // Initialize auto-compact settings for newly captured session - initializeSessionSettings(capturedSessionId); - // Update process key with captured session ID - if (processKey !== capturedSessionId) { - activeClaudeProcesses.delete(processKey); - activeClaudeProcesses.set(capturedSessionId, claudeProcess); - } - // Send session-created event only once for new sessions - if (!sessionId && !sessionCreatedSent) { - sessionCreatedSent = true; - ws.send(JSON.stringify({ - type: 'session-created', - sessionId: capturedSessionId - })); - } - } - // Send parsed response to WebSocket - ws.send(JSON.stringify({ - type: 'claude-response', - data: response - })); - } catch (parseError) { - console.log('📄 Non-JSON response:', line); - // If not JSON, send as raw text - ws.send(JSON.stringify({ - type: 'claude-output', - data: line - })); - } - }Also applies to: 510-554
617-630: Avoid writing a blank line to stdin when no command is provided.This sends an unintended newline in interactive sessions.
Apply:
-// Handle stdin for interactive mode -if (command) { - // For --print mode with arguments, we don't need to write to stdin - claudeProcess.stdin.end(); -} else { - // For interactive mode, we need to write the command to stdin if provided later - // Keep stdin open for interactive session - if (command !== undefined) { - claudeProcess.stdin.write(command + '\n'); - claudeProcess.stdin.end(); - } - // If no command provided, stdin stays open for interactive use -} +// Handle stdin for interactive mode +if (typeof command === 'string' && command.trim().length > 0) { + // --print path; no stdin needed + claudeProcess.stdin.end(); +} else { + // Keep stdin open for interactive use; do not write anything yet +}
♻️ Duplicate comments (1)
server/claude-cli.js (1)
31-42: Strip tags/ANSI before parsing token warnings.Stream may contain <system_warning> tags or ANSI codes. Sanitize first, then match.
Apply:
function parseSystemWarnings(output) { - // Make regex more robust: handle commas in numbers, case-insensitive matching - const warningMatch = output.match(/Token usage:\s*([\d,]+)\s*\/\s*([\d,]+);\s*([\d,]+)\s+remaining/i); + // Strip tags and ANSI, then match commas and variable whitespace + const clean = output.replace(/<[^>]+>/g, '').replace(/\x1B\[[0-9;]*m/g, ''); + const warningMatch = clean.match(/Token usage:\s*([\d,]+)\s*\/\s*([\d,]+);\s*([\d,]+)\s*remaining/i);
🧹 Nitpick comments (6)
server/claude-cli.js (6)
557-565: Also parse token warnings from stderr.Some CLIs emit warnings to stderr. Mirror the stdout parsing to avoid missing updates.
Apply:
claudeProcess.stderr.on('data', (data) => { - console.error('Claude CLI stderr:', data.toString()); - ws.send(JSON.stringify({ + const text = data.toString(); + console.error('Claude CLI stderr:', text); + // Try token budget parsing from stderr too + const tokenData = parseSystemWarnings(text); + if (tokenData && capturedSessionId) { + ws.send(JSON.stringify({ type: 'token-budget-update', data: { ...tokenData, sessionId: capturedSessionId } })); + if (shouldTriggerAutoCompact(capturedSessionId, tokenData)) { + triggerAutoCompact(capturedSessionId, tokenData, ws); + } + } + ws.send(JSON.stringify({ type: 'claude-error', - error: data.toString() + error: text })); });
458-466: Log the actual CLI path (avoid hardcoded 'claude' in logs).Current logs can mislead when CLAUDE_CLI_PATH is set.
Apply:
-console.log('Spawning Claude CLI:', 'claude', args.map(arg => { +console.log('Spawning Claude CLI args:', args.map(arg => { const cleanArg = arg.replace(/\n/g, '\\n').replace(/\r/g, '\\r'); return cleanArg.includes(' ') ? `"${cleanArg}"` : cleanArg; }).join(' ')); console.log('Working directory:', workingDir); @@ -console.log('🔍 Final Claude command will be: claude ' + args.join(' ')); +// After resolving cli path: @@ const claudePath = process.env.CLAUDE_CLI_PATH || 'claude'; console.log('🔍 Using Claude CLI path:', claudePath); +console.log('🔍 Final Claude command will be: ' + claudePath + ' ' + args.join(' '));Also applies to: 467-470
566-599: Cleanup per‑session state on process exit to avoid leaks.Remove sessionTokenUsage and any lingering active flags.
Apply:
// Clean up process reference const finalSessionId = capturedSessionId || sessionId || processKey; activeClaudeProcesses.delete(finalSessionId); + sessionTokenUsage.delete(finalSessionId); + activeAutoCompacts.delete(finalSessionId);
50-82: Trigger logic looks good; consider tiny polish.Use >= for cooldown check and allow optional per‑session cooldown later if needed.
Apply:
- if (!lastCompactTime || (now - lastCompactTime) > AUTO_COMPACT_COOLDOWN) { + if (!lastCompactTime || (now - lastCompactTime) >= AUTO_COMPACT_COOLDOWN) { console.log(`⚡ Auto-compact trigger conditions met: ${tokenData.remaining} tokens remaining (threshold: ${threshold}), cooldown satisfied`); return true; }
163-215: WS sends: add a small guard to avoid throws on closed sockets.Non-blocking, but prevents crashes if the client disconnects mid‑run.
You can wrap sends with a tiny helper:
const safeSend = (ws, msg) => { try { ws?.readyState === 1 && ws.send(JSON.stringify(msg)); } catch {} };Then replace ws.send({...}) calls here with safeSend(ws, {...}).
12-23: Minor: remove unused TOKEN_BUDGET_TOTAL or use it.It’s defined but not referenced.
Apply:
-const TOKEN_BUDGET_TOTAL = 200000;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
server/claude-cli.js(3 hunks)src/components/ChatInterface.jsx(4 hunks)src/components/TokenBudgetIndicator.jsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/ChatInterface.jsx
- src/components/TokenBudgetIndicator.jsx
🧰 Additional context used
🧬 Code graph analysis (1)
server/claude-cli.js (3)
src/components/Settings.jsx (1)
autoCompactEnabled(32-32)src/components/Shell.jsx (1)
ws(36-36)src/utils/websocket.js (1)
ws(4-4)
🔇 Additional comments (2)
server/claude-cli.js (2)
57-66: Per‑session settings respected. LGTM.Threshold and enablement are read from session data with sane defaults.
557-565: Include sessionId on 'claude-error' and 'claude-output'/'claude-response' for multi‑session WS.Prevents cross‑pollution when a single socket handles multiple sessions.
If frontend expects it, append
sessionId: capturedSessionIdto these payloads. Do you want me to generate a targeted patch?
| async function executeContextSave(sessionId) { | ||
| return new Promise((resolve, reject) => { | ||
| console.log(`📦 Executing /context save for session ${sessionId}`); | ||
|
|
||
| const args = [ | ||
| '--resume', sessionId, | ||
| '--output-format', 'stream-json', | ||
| '--print', | ||
| '--', | ||
| '/context save' | ||
| ]; | ||
|
|
||
| // Honor CLAUDE_CLI_PATH environment variable | ||
| const claudePath = process.env.CLAUDE_CLI_PATH || 'claude'; | ||
| console.log('🔍 Using Claude CLI path for context save:', claudePath); | ||
|
|
||
| const compactProcess = spawnFunction(claudePath, args, { | ||
| cwd: process.cwd(), | ||
| env: process.env | ||
| }); | ||
|
|
||
| let compactOutput = ''; | ||
|
|
||
| compactProcess.stdout.on('data', (data) => { | ||
| compactOutput += data.toString(); | ||
| console.log('📤 Context save output:', data.toString()); | ||
| }); | ||
|
|
||
| compactProcess.stderr.on('data', (data) => { | ||
| console.error('❌ Context save error:', data.toString()); | ||
| }); | ||
|
|
||
| compactProcess.on('close', (code) => { | ||
| if (code === 0) { | ||
| // Parse output to extract tokens saved | ||
| const tokensSaved = parseTokensSavedFromOutput(compactOutput); | ||
| console.log(`✅ Context save completed: ${tokensSaved !== null ? tokensSaved + ' tokens saved' : 'tokens saved unknown'}`); | ||
| resolve({ tokensSaved }); | ||
| } else { | ||
| reject(new Error(`Context save failed with exit code ${code}`)); | ||
| } | ||
| }); | ||
|
|
||
| compactProcess.on('error', (error) => { | ||
| console.error('❌ Context save process error:', error); | ||
| reject(error); | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
No timeout for context save; hung subprocess blocks auto-compact.
Add a kill timeout (e.g., 60s) to prevent indefinite waits and surface a clear error.
Apply:
@@
async function executeContextSave(sessionId) {
return new Promise((resolve, reject) => {
@@
- const compactProcess = spawnFunction(claudePath, args, {
+ const compactProcess = spawnFunction(claudePath, args, {
cwd: process.cwd(),
env: process.env
});
let compactOutput = '';
+ const CONTEXT_SAVE_TIMEOUT_MS = 60_000;
+ const timeoutId = setTimeout(() => {
+ try { compactProcess.kill('SIGTERM'); } catch (_) {}
+ reject(new Error(`Context save timed out after ${CONTEXT_SAVE_TIMEOUT_MS / 1000}s`));
+ }, CONTEXT_SAVE_TIMEOUT_MS);
@@
- compactProcess.on('close', (code) => {
+ compactProcess.on('close', (code) => {
+ clearTimeout(timeoutId);
if (code === 0) {
@@
- compactProcess.on('error', (error) => {
+ compactProcess.on('error', (error) => {
+ clearTimeout(timeoutId);
console.error('❌ Context save process error:', error);
reject(error);
});🤖 Prompt for AI Agents
In server/claude-cli.js around lines 89 to 137, the executeContextSave function
spawns a subprocess but has no timeout, which can hang the process; add a kill
timeout (e.g., 60_000 ms) that calls compactProcess.kill('SIGTERM') (or SIGKILL
if necessary) after the timeout, reject the Promise with a clear timeout error,
and ensure you clear the timeout and remove/guard event listeners on
'close'/'error' so you don't double-resolve or leak timers; also include the
timeout case in logs and the returned/rejected error message.
…st parsing Fixes critical issues from CodeRabbit review on commit dde40ac: Frontend (ChatInterface.jsx): - Wire auto-compact settings to backend via getToolsSettings() - Return autoCompactEnabled, autoCompactThreshold, showAutoCompactNotifications - Settings now flow from localStorage → toolsSettings → backend initialization - Fixes "frontend does not currently send settings to backend" issue Backend (server/claude-cli.js): - Strip HTML tags and ANSI codes before parsing token warnings - Regex now handles <system_warning> tags and color codes - Add line buffering for NDJSON parsing to handle chunk boundaries - Buffer incomplete lines across data chunks, parse only complete lines - Prevents partial JSON parsing errors and misclassification - Cleanup per-session state on process exit (prevent memory leaks) - Delete sessionTokenUsage and activeAutoCompacts entries on close Technical details: - HTML tag stripping: /<[^>]+>/g - ANSI code stripping: /\x1B\[[0-9;]*m/g - Line buffer pattern: append chunks, split on \n, keep incomplete last line - Session cleanup: delete from all Maps/Sets on process exit - Settings defaults: autoCompactEnabled=true, threshold=30000 Addresses CodeRabbit actionable comments and prevents: - Frontend-backend settings disconnect - Token parsing failures from formatted output - Partial JSON parsing across chunk boundaries - Memory leaks from abandoned sessions
|
@shockstricken the latest version migrated to claude-agent-sdk instead of the clil. Do we still need this? |
2 participants
Summary
Implements automatic token budget monitoring and context compression to prevent context loss during long-running Claude sessions.
Problem: Users lose conversation context when token budget reaches 200.000 limit
Solution: Automatically compress context when < 30.000 tokens remaining
Key Features
✅ Token Budget Monitoring
✅ Automatic Context Compression
/context saveat 30.000 token threshold✅ User Notifications
✅ Configurable Settings
Implementation Details
Backend (
server/claude-cli.js)parseSystemWarnings()- Extracts token budget from system warningsshouldTriggerAutoCompact()- Threshold and cooldown validationexecuteContextSave()- Spawns Claude CLI with/context savetriggerAutoCompact()- Orchestrates compression workflowtoken-budget-update,auto-compact-triggered/complete/errorFrontend Components
TokenBudgetIndicator.jsx- Visual token budget display with progress barAutoCompactNotification.jsx- Toast notifications for compression eventsChatInterface.jsx- WebSocket handlers and UI integrationSettings.jsx- Configuration UI in appearance tabFiles Changed
Total: 1.347 lines added across 7 files
Testing Checklist
User Experience
Before Auto-Compact:
After Auto-Compact:
Documentation
AUTO_COMPACT_FEATURE_PROPOSAL.md- Complete technical design (582 lines)AUTO_COMPACT_IMPLEMENTATION_SUMMARY.md- Quick reference guide (249 lines)Benefits
Breaking Changes
None - feature is additive and opt-out via settings
Related Issues
Addresses user pain point: context loss during long development sessions with Claude CLI
🤖 Generated with Claude Code
Implementation based on infra-assist auto-compact patterns
Summary by CodeRabbit