-
Notifications
You must be signed in to change notification settings - Fork 489
feat: add Claude usage tracking via CLI #203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add Claude usage tracking via CLI #203
Conversation
Adds a Claude usage tracking feature that displays session, weekly, and Sonnet usage stats. Uses the Claude CLI's /usage command to fetch data (no API key required). Features: - Usage popover in board header showing session, weekly, and Sonnet limits - Progress bars with color-coded status (green/orange/red) - Auto-refresh with configurable interval - Caching of usage data with stale indicator - Settings section for refresh interval configuration Server: - ClaudeUsageService: Executes Claude CLI via PTY (expect) to fetch usage - New /api/claude/usage endpoint UI: - ClaudeUsagePopover component with usage cards - ClaudeUsageSection in settings for configuration - Integration with app store for persistence
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds end-to-end Claude CLI usage tracking: server route mounted at Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI (Popover / Settings)
participant Store as App Store
participant Bridge as Electron / Http Client
participant Server as Express /api/claude
participant Service as ClaudeUsageService
participant CLI as Claude CLI
UI->>Store: read claudeRefreshInterval / lastUpdated
UI->>Bridge: request claude.getUsage() (on open / refresh)
Bridge->>Server: GET /api/claude/usage
Server->>Service: isAvailable()
Service->>CLI: probe `claude` binary
alt CLI unavailable
Service-->>Server: unavailable error
Server-->>Bridge: 503
Bridge-->>UI: show unavailable
else CLI available
Server->>Service: fetchUsageData()
Service->>CLI: spawn PTY/run `claude /usage` (30s)
CLI-->>Service: stdout (usage text) or auth/timeout
Service->>Service: strip ANSI, parse sections & resets
alt auth error
Service-->>Server: auth error
Server-->>Bridge: 401
Bridge-->>UI: show auth-required
else timeout/error
Service-->>Server: timeout/error
Server-->>Bridge: 504/500
Bridge-->>UI: show error
else success
Service-->>Server: ClaudeUsage JSON
Server-->>Bridge: 200 OK with usage
Bridge-->>UI: usage payload
UI->>Store: setClaudeUsage(...), setClaudeUsageLastUpdated(...)
UI->>UI: render metrics / progress
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @maddada, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a significant new feature: integrated Claude AI usage tracking. It empowers users to conveniently monitor their Claude session, weekly, and Sonnet model usage directly from the application's board header through an interactive popover. The implementation is designed for ease of use, leveraging the existing Claude CLI for data retrieval, which means no additional API key configuration is required. The feature also includes a dedicated settings section for customizing the data refresh interval, ensuring users have up-to-date information on their AI consumption. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a valuable feature for tracking Claude usage via the CLI. The server-side implementation is clever but introduces a critical platform dependency on the expect command, which will cause issues on systems where it's not installed, like Windows. The UI components are well-structured, though there are some improvements to be made regarding React hook usage and code duplication. I've also noted some type-safety and naming consistency issues that should be addressed to improve maintainability. Overall, a great addition with a few key areas for refinement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (7)
apps/ui/src/lib/http-api-client.ts (1)
1020-1023: Consider adding proper return type instead ofany.The
getUsagemethod returnsPromise<any>, which loses type safety. The server returns a structuredClaudeUsageobject (or error responses). Consider importing/defining a proper response type to improve type safety and IDE support.🔎 Suggested improvement
+import type { ClaudeUsage } from "@/store/app-store"; + +interface ClaudeUsageResponse extends ClaudeUsage { + error?: string; + message?: string; +} + // Claude API claude = { - getUsage: (): Promise<any> => this.get("/api/claude/usage"), + getUsage: (): Promise<ClaudeUsageResponse> => this.get("/api/claude/usage"), };apps/ui/src/lib/electron.ts (1)
485-487: Consider using a proper return type instead ofany.Same as in
http-api-client.ts, thegetUsagemethod usesPromise<any>. Using a proper type would improve type safety across the codebase.apps/ui/src/components/views/settings-view/api-keys/claude-usage-section.tsx (2)
59-70: Consider adding accessibility attributes to the slider.The slider works correctly, but could benefit from an accessible label for screen readers.
🔎 Suggested improvement
<Slider + aria-label="Refresh interval in seconds" value={[Math.max(30, Math.min(120, localInterval || 30))]} onValueChange={(vals) => setLocalInterval(vals[0])} onValueCommit={(vals) => setClaudeRefreshInterval(vals[0])} min={30} max={120} step={5} className="flex-1" />
61-61: Redundant clamping - slider already enforces min/max.The
Math.max(30, Math.min(120, ...))clamping is defensive but unnecessary since the Slider component already enforcesmin={30}andmax={120}. The fallback|| 30for undefined/null is sufficient.🔎 Simplified approach
- value={[Math.max(30, Math.min(120, localInterval || 30))]} + value={[localInterval || 30]}- <span className="w-12 text-sm font-mono text-right">{Math.max(30, Math.min(120, localInterval || 30))}s</span> + <span className="w-12 text-sm font-mono text-right">{localInterval || 30}s</span>apps/ui/src/components/claude-usage-popover.tsx (1)
218-225: Refresh button doesn't animate when loading.The button has
loading && "opacity-80"but theRefreshCwicon doesn't haveanimate-spinapplied during loading, unlike the loading state spinner on line 243.🔎 Proposed fix
<Button variant="ghost" size="icon" className={cn("h-6 w-6", loading && "opacity-80")} onClick={() => !loading && fetchUsage(false)} > - <RefreshCw className="w-3.5 h-3.5" /> + <RefreshCw className={cn("w-3.5 h-3.5", loading && "animate-spin")} /> </Button>apps/server/src/services/claude-usage-service.ts (2)
81-87: Consider using SIGKILL for more reliable process termination.
proc.kill()sends SIGTERM by default, which theexpectprocess might not handle immediately. For a timeout scenario, SIGKILL ensures immediate termination.🔎 Proposed fix
const timeoutId = setTimeout(() => { if (!settled) { settled = true; - proc.kill(); + proc.kill("SIGKILL"); reject(new Error("Command timed out")); } }, this.timeout);
167-174: Good resilience in parsing multiple label variations.The fallback approach for parsing Sonnet/Opus sections handles CLI output variations well. However, the field naming (
opusWeeklyPercentage) doesn't match the primary label being searched ("Sonnet only"). Consider renaming for consistency with the UI which displays "Sonnet".
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/server/src/index.ts(2 hunks)apps/server/src/routes/claude/index.ts(1 hunks)apps/server/src/routes/claude/types.ts(1 hunks)apps/server/src/services/claude-usage-service.ts(1 hunks)apps/ui/src/components/claude-usage-popover.tsx(1 hunks)apps/ui/src/components/views/board-view/board-header.tsx(2 hunks)apps/ui/src/components/views/settings-view.tsx(2 hunks)apps/ui/src/components/views/settings-view/api-keys/claude-usage-section.tsx(1 hunks)apps/ui/src/lib/electron.ts(2 hunks)apps/ui/src/lib/http-api-client.ts(1 hunks)apps/ui/src/store/app-store.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
apps/server/src/index.ts (1)
apps/server/src/routes/claude/index.ts (1)
createClaudeRoutes(4-44)
apps/ui/src/components/views/board-view/board-header.tsx (1)
apps/ui/src/components/claude-usage-popover.tsx (1)
ClaudeUsagePopover(20-309)
apps/ui/src/components/views/settings-view.tsx (1)
apps/ui/src/components/views/settings-view/api-keys/claude-usage-section.tsx (1)
ClaudeUsageSection(7-75)
apps/ui/src/components/claude-usage-popover.tsx (2)
apps/ui/src/store/app-store.ts (1)
useAppStore(922-2439)apps/ui/src/lib/electron.ts (1)
getElectronAPI(567-576)
apps/ui/src/store/app-store.ts (1)
apps/server/src/routes/claude/types.ts (1)
ClaudeUsage(5-28)
apps/server/src/services/claude-usage-service.ts (2)
apps/server/src/routes/claude/types.ts (1)
ClaudeUsage(5-28)apps/ui/src/store/app-store.ts (1)
ClaudeUsage(518-538)
apps/server/src/routes/claude/types.ts (1)
apps/ui/src/store/app-store.ts (1)
ClaudeUsage(518-538)
🪛 Biome (2.1.2)
apps/server/src/services/claude-usage-service.ts
[error] 134-134: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
🔇 Additional comments (11)
apps/ui/src/components/views/board-view/board-header.tsx (1)
41-43: LGTM!The
ClaudeUsagePopoverintegration follows the existing pattern of usingisMountedto prevent hydration issues, consistent with other dynamic UI elements in the header.apps/ui/src/lib/electron.ts (1)
885-911: Mock implementation looks comprehensive.The mock provides realistic test data including all usage metrics, reset times, and metadata. This will be useful for development and testing.
apps/server/src/index.ts (2)
47-47: LGTM!The import follows the existing pattern for route modules.
145-145: LGTM!The Claude routes are correctly mounted under
/api/claudeand protected by the authentication middleware (applied at line 126), consistent with other authenticated endpoints.apps/ui/src/components/views/settings-view.tsx (1)
96-103: LGTM!Clean integration of the
ClaudeUsageSectionalongsideClaudeCliStatus. Thespace-y-6class provides appropriate vertical spacing between the two related sections.apps/server/src/routes/claude/index.ts (1)
4-43: LGTM - Clean route implementation with proper error handling.The route correctly checks CLI availability before attempting to fetch, and maps errors to appropriate HTTP status codes. The error message matching approach (lines 26, 31) is pragmatic for CLI-based errors.
One minor note: the endpoint has no rate limiting, which could be relevant if the CLI calls are expensive. However, given this is a local tool and the UI already implements client-side refresh intervals, this is acceptable.
apps/ui/src/store/app-store.ts (2)
540-571: Well-implemented limit checking utility.The
isClaudeUsageAtLimitfunction correctly handles all three limit types (session, weekly, cost) and safely returnsfalsewhen no usage data is available, avoiding blocking auto-mode unnecessarily.
2352-2358: LGTM - Clean action implementations.The Claude usage actions are correctly implemented.
setClaudeUsageatomically updates both the usage data and timestamp, which prevents inconsistent state.apps/server/src/services/claude-usage-service.ts (3)
132-135: Control character in regex is intentional and correct.The Biome static analysis warning about control characters is a false positive here. The
\x1B(ESC) character is intentionally matched to strip ANSI escape sequences from terminal output. This pattern is standard for ANSI code removal.
255-336: Comprehensive reset time parsing with good fallback behavior.The parser handles multiple time formats (duration, simple time, date+time) robustly. The 12-hour to 24-hour conversion correctly handles edge cases (12am→0, 12pm→12). The year rollover logic (line 327-329) properly handles dates that appear to be in the past.
20-30: Add Windows support for Claude CLI detection.The
whichcommand is Unix-only and will fail on Windows. Since the codebase already handles platform differences elsewhere (seecommon.ts,dev-server-service.ts), this service should detect the platform and usewhereon Windows or adopt a cross-platform approach like the npmwhichpackage. Note that the entire service currently relies on Unix-only tools (expect,which,process.env.HOME) and needs platform support.
Usage tracking via CLI only works for Claude Code subscription users. Hide the Usage button and settings section when an Anthropic API key is set.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Replace Unix-only 'expect' command with node-pty library which works on Windows, macOS, and Linux. Also fixes 'which' command to use 'where' on Windows for checking if Claude CLI is available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/server/src/routes/claude/types.ts(1 hunks)apps/server/src/services/claude-usage-service.ts(1 hunks)apps/ui/src/lib/electron.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/ui/src/lib/electron.ts (2)
apps/server/src/routes/claude/types.ts (1)
ClaudeUsage(5-28)apps/ui/src/store/app-store.ts (1)
ClaudeUsage(518-538)
apps/server/src/routes/claude/types.ts (1)
apps/ui/src/store/app-store.ts (1)
ClaudeUsage(518-538)
🪛 Biome (2.1.2)
apps/server/src/services/claude-usage-service.ts
[error] 136-136: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
🔇 Additional comments (6)
apps/server/src/services/claude-usage-service.ts (5)
22-35: Cross-platform CLI detection implemented correctly.The method appropriately uses
whereon Windows andwhichon Unix-like systems to check for Claude CLI availability, addressing cross-platform compatibility.
49-129: Well-implemented PTY-based CLI execution.The method uses
node-ptyfor cross-platform pseudo-terminal support, includes proper timeout handling (30s), detects authentication errors, and gracefully handles the interactive Claude CLI by sending escape key after detecting usage data output.
207-252: Robust parsing logic for CLI output sections.The method correctly handles terminal refresh artifacts by finding the last occurrence of each section, parses percentages in multiple formats ("left", "used", "remaining"), and converts to a consistent "used" percentage. The 5-line search window is appropriate for the CLI output format.
257-338: Comprehensive reset time parsing with multiple format support.The method handles three different CLI output formats (duration, time-only, calendar date) with proper AM/PM conversion and smart date handling (assumes next year for past dates). The fallback to
getDefaultResetTimeensures the method always returns a valid timestamp.
343-359: Reasonable default reset times as fallback.The defaults (5 hours for session, next Monday noon for weekly) align with Claude's typical reset schedule and provide sensible fallbacks when parsing fails. The Monday edge case is properly handled.
apps/ui/src/lib/electron.ts (1)
886-911: Mock implementation provides appropriate test data.The mock Claude usage implementation returns a well-structured
ClaudeUsageobject with realistic values for development testing, including session/weekly percentages, reset times, and metadata fields.
…upport" This reverts commit 5e789c2.
Platform-specific implementations: - macOS: Uses 'expect' command (unchanged, working) - Windows: Uses node-pty for PTY support Also fixes 'which' vs 'where' for checking Claude CLI availability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (4)
apps/ui/src/components/claude-usage-popover.tsx (4)
52-57: Stale closure:useEffectmissing dependencies.This effect references
isStaleand callsfetchUsagebut has an empty dependency array, causing stale closures.Past reviews already flagged this issue. Once
fetchUsageis wrapped inuseCallback, add bothisStaleandfetchUsageto the dependency array, or use a ref to capture the initial stale state if mount-only behavior is intended.
59-84: Missing dependencies and duplicated stale check logic.This effect is missing several dependencies and duplicates the stale check calculation:
- Dependencies: The effect reads
claudeUsage,claudeUsageLastUpdated,claudeRefreshInterval, and callsfetchUsage, but only includesopenin the dependency array.- Duplication: Lines 66 recalculate staleness instead of reusing the
isStalememoized value from line 28.Both issues were already flagged in past reviews. After wrapping
fetchUsageinuseCallback, update the dependency array and replace the duplicated stale check with theisStalevariable.
267-273: UI label and data field name mismatch.The card displays "Sonnet" but uses
opusWeeklyPercentageandopusResetTextfrom the data model.This naming inconsistency was already flagged in past reviews. Either rename the data fields to match "Sonnet" or add a comment explaining that
opus*fields represent Sonnet data from CLI parsing.
32-50: WrapfetchUsageinuseCallbackto stabilize effect dependencies.The
fetchUsagefunction is used in multipleuseEffecthooks but isn't memoized, causing it to be recreated on every render. This makes it unsuitable as an effect dependency and can lead to infinite re-render loops when the dependency arrays are corrected (as flagged in past reviews).🔎 Proposed fix
+ const fetchUsage = useCallback(async (isAutoRefresh = false) => { - const fetchUsage = async (isAutoRefresh = false) => { if (!isAutoRefresh) setLoading(true); setError(null); try { const api = getElectronAPI(); if (!api.claude) { throw new Error("Claude API not available"); } const data = await api.claude.getUsage(); if (data.error) { throw new Error(data.message || data.error); } setClaudeUsage(data); } catch (err) { setError(err instanceof Error ? err.message : "Failed to fetch usage"); } finally { if (!isAutoRefresh) setLoading(false); } - }; + }, [setClaudeUsage]);Note: Import
useCallbackfrom React at line 1.
🧹 Nitpick comments (2)
apps/ui/src/components/claude-usage-popover.tsx (2)
161-161: Consider using a prop instead of string comparison.The condition
title === "Session Usage"is fragile and couples the component logic to specific title text. If the title changes (e.g., for i18n), the icon disappears.🔎 Proposed refactor
Add a
showClockIconprop toUsageCard:const UsageCard = ({ title, subtitle, percentage, resetText, isPrimary = false, stale = false, + showClockIcon = false, }: { title: string; subtitle: string; percentage: number; resetText?: string; isPrimary?: boolean; stale?: boolean; + showClockIcon?: boolean; }) => { // ... {resetText && ( <div className="mt-2 flex justify-end"> <p className="text-xs text-muted-foreground flex items-center gap-1"> - {title === "Session Usage" && <Clock className="w-3 h-3" />} + {showClockIcon && <Clock className="w-3 h-3" />} {resetText} </p> </div> )}Then pass
showClockIcon={true}at line 249.
218-226: Addaria-labelfor accessibility.The refresh button uses only an icon without visible text, but lacks an
aria-labelfor screen readers.🔎 Proposed fix
<Button variant="ghost" size="icon" className={cn("h-6 w-6", loading && "opacity-80")} onClick={() => !loading && fetchUsage(false)} + aria-label="Refresh usage data" > <RefreshCw className="w-3.5 h-3.5" /> </Button>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/server/src/services/claude-usage-service.ts(1 hunks)apps/ui/src/components/claude-usage-popover.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/ui/src/components/claude-usage-popover.tsx (1)
apps/ui/src/lib/electron.ts (1)
getElectronAPI(567-576)
🪛 Biome (2.1.2)
apps/server/src/services/claude-usage-service.ts
[error] 226-226: 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)
apps/server/src/services/claude-usage-service.ts (7)
1-20: LGTM! Clean class setup with appropriate platform detection.The imports and class initialization are well-structured. Using
os.platform()for platform detection and configuring a reasonable 30-second timeout are good practices.
25-36: LGTM! Platform compatibility issue resolved.The method now correctly uses
whereon Windows andwhichon Unix-like systems (line 27), addressing the cross-platform compatibility concern from previous reviews. Error handling is appropriate.
60-143: Robust implementation with good error handling.The macOS implementation using
expectis well-structured with:
- Timeout handling (lines 97-103)
- Authentication error detection (lines 119-123)
- Pragmatic approach accepting useful output even with non-zero exit codes (lines 126-132)
The hardcoded sleep delays (lines 76, 80) in the expect script are necessary for CLI timing but could be fragile if the CLI behavior changes.
Note: The previous review comment about documenting the
expectdependency in the README Prerequisites section remains valid, as this implementation still requires theexpectcommand on macOS.
148-219: LGTM! Solid Windows implementation using node-pty.The Windows implementation properly uses
node-ptyfor PTY interaction with appropriate:
- Working directory fallback chain (line 154)
- Type assertion for environment variables (line 164) to satisfy node-pty typing
- Sequential detection logic with
hasSeenUsageDataflag to prevent race conditions- Authentication error handling matching the macOS implementation
The
settledflag prevents any race conditions between timeouts and exit handlers.
224-227: LGTM! Standard ANSI escape code pattern.The regex pattern using
\x1B(ESC character) is the standard way to strip ANSI escape codes. The static analysis warning is a false positive—the control character is intentional and necessary. The existing eslint-disable comment on line 225 is appropriate.
249-292: Previous critical issue resolved - field names now match type definition.The critical issue from the previous review has been fixed. The code now correctly:
- Uses
sonnetDatavariable (line 260)- Returns
sonnetWeeklyTokensUsed,sonnetWeeklyPercentage, andsonnetResetText(lines 281-283)This matches the
ClaudeUsagetype definition.
297-449: LGTM! Comprehensive parsing logic with good fallback handling.The parsing implementation is well-designed:
parseSection (lines 297-342): Finds the last occurrence to handle terminal refreshes (lines 303-309), correctly converts "left" to "used" percentages (line 325), and strips timezone info from display text (line 338).
parseResetTime (lines 347-428): Handles multiple formats comprehensively:
- Duration format: "Resets in 2h 15m" (lines 351-365)
- Simple time: "Resets 11am" (lines 368-390)
- Date format: "Resets Dec 22 at 8pm" (lines 393-424)
getDefaultResetTime (lines 433-449): Provides sensible defaults (session: 5 hours, weekly: next Monday at 12:59) with correct day-of-week calculation (lines 441-446).
The edge cases (time already passed, date in past year) are handled pragmatically with reasonable assumptions.
- Show 'N/A' and dim card when percentage is NaN/invalid - Use gray progress bar for invalid values - Rename opusWeekly* properties to sonnetWeekly* to match server types
…handling in Claude components
…ngsView components
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/ui/src/components/views/board-view/board-header.tsx(2 hunks)apps/ui/src/components/views/settings-view.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/ui/src/components/views/board-view/board-header.tsx
🔇 Additional comments (2)
apps/ui/src/components/views/settings-view.tsx (2)
12-12: LGTM! Import and store integration look good.The import of
ClaudeUsageSectionand extraction ofapiKeysfrom the store follow the existing patterns in the file and are necessary for the new usage tracking feature.Also applies to: 50-50
102-109: LGTM! Conditional rendering logic is correct.The wrapping div with
space-y-6provides appropriate vertical spacing, and the conditional rendering ofClaudeUsageSectionbased on theshowUsageTrackingflag is implemented correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/ui/src/store/app-store.ts (1)
2179-2179: Consider adding bounds validation for refresh interval.The action accepts any number, but PR objectives specify a 30–120 second range. While UI-level validation may exist, adding bounds checking here would be more defensive.
🔎 Proposed enhancement
-setClaudeRefreshInterval: (interval: number) => set({ claudeRefreshInterval: interval }), +setClaudeRefreshInterval: (interval: number) => { + const clampedInterval = Math.max(30, Math.min(120, interval)); + set({ claudeRefreshInterval: clampedInterval }); +},
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/ui/src/store/app-store.ts(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/ui/src/store/app-store.ts (1)
apps/server/src/routes/claude/types.ts (1)
ClaudeUsage(5-28)
🔇 Additional comments (4)
apps/ui/src/store/app-store.ts (4)
441-465: LGTM! ClaudeUsage type is complete and matches server response.The type definition correctly includes all fields from the server-side type, including
lastUpdatedanduserTimezonethat were previously missing (per past review comments). The type consistency between client and server prevents potential runtime issues.
2178-2184: LGTM! Claude usage actions are well-designed.The implementation correctly:
- Couples usage data with timestamps in
setClaudeUsageto maintain consistency- Provides
setClaudeUsageLastUpdatedfor manual timestamp management- Handles null usage by clearing both data and timestamp
2258-2261: LGTM! Persistence correctly configured for Claude usage caching.All three fields are persisted as required by PR objectives for cross-restart data caching and user preference (refresh interval) persistence.
472-503: Consider whethersonnetWeeklyPercentageshould gate auto-mode.The function checks
sessionPercentageandweeklyPercentagebut notsonnetWeeklyPercentage. Since theClaudeUsagetype includessonnetWeeklyPercentage(line 456), clarify whether auto-mode should also pause when Sonnet's weekly limit is reached, separate from the general weekly limit.If
weeklyPercentagealready represents Sonnet-specific usage or if auto-mode doesn't exclusively use Sonnet, this is intentional. Otherwise, adding a check forclaudeUsage.sonnetWeeklyPercentage >= 100may be necessary to prevent auto-mode from attempting operations when Sonnet specifically is exhausted.
Summary
/usagecommand to fetch data (no API key required - relies onclaude login)Features
Implementation Details
Server
ClaudeUsageService: Executes Claude CLI via PTY (expectcommand) to fetch usage/api/claude/usageendpointUI
ClaudeUsagePopovercomponent with usage cardsClaudeUsageSectionin settings for configurationTest plan
claude loginSummary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.