Ade Usage Bar#327
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
@copilot review but do not make fixes |
📝 WalkthroughWalkthroughRefactored ChangesHeader Usage Control Refactoring and Test Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
PR Review
Scope: 4 file(s), +507 / −323
Verdict: Minor issues
This PR surfaces 5-hour and weekly/monthly usage in the header, starts polling as soon as the app loads (cached snapshot + forced refresh), and keeps the header in sync with the drawer via applySnapshot. The main concern is extra main-process usage work on a timer that largely duplicates the existing background poller.
⚡ Performance
[Medium] Header timer duplicates background usage polling
File: apps/desktop/src/renderer/components/usage/HeaderUsageControl.tsx:L143-L174
Issue: The header now calls usage.refresh() on mount, every 120s, and on window focus when data is older than 60s. The main process already runs createUsageTrackingService with pollIntervalMs: 120_000 and pushes updates through onUpdate (see apps/desktop/src/main/main.ts:L3875-L3887). Each renderer refresh() maps to forceRefresh(), which invalidates the 60s cost-log cache and re-runs Claude/Codex polls.
Impact: Up to ~2× poll cadence when the header interval and the service timer align; focus churn can add more. inFlightPoll dedupes concurrent polls, but forceRefresh() still forces extra cost scans versus a cached getSnapshot()/onUpdate-only path.
Fix: Rely on onUpdate for steady-state header updates; call refresh() only for the initial cold-start path and user-driven stale cases (e.g. focus) with a longer stale threshold, or expose a cheap snapshot read that does not invalidate the cost cache.
🎨 UI/UX
[Low] Plan window label shows wk before weekly/monthly data exists
File: apps/desktop/src/renderer/components/usage/HeaderUsageControl.tsx:L65-L72
Issue: planLabel defaults to "wk" whenever a weekly window is absent, even if only the 5-hour window is present (both planPercent values null). The chip then reads wk …, which implies a weekly quota is loading rather than “no plan window for this provider.”
Fix: Use a neutral label (e.g. omit the plan segment or show plan …) when weeklyPercent and monthlyPercent are both null.
[Low] Header usage chips are denser and no longer collapse to the gauge while loading
File: apps/desktop/src/renderer/components/usage/HeaderUsageControl.tsx:L299-L303
Issue: Removing hasAnyChip means detected providers always render logo + 5h … + wk … chips (9px mono text) instead of the single gauge icon during the first poll. With two tracked providers this widens the top-bar control versus the prior one-percent chip.
Fix: Restore a gauge fallback until at least one of fiveHourPercent or planPercent is non-null, or truncate/hide the plan segment until data arrives.
Notes
- Good fix:
UsageQuotaPanelnow usesonSnapshotChange={applySnapshot}so drawer updates keepsnapshotRefaligned for focus-stale checks and race guards. - Tests cover the cached-vs-forced refresh race and the 120s interval; consolidated
usage.test.tsxmatches the new behavior. - Security: no new trust boundaries; data stays on local IPC from the usage service.
Sent by Cursor Automation: BUGBOT in Versic
3be022c to
898e8f8
Compare
|
@copilot review but do not make fixes |


Summary
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Summary by CodeRabbit
Release Notes
Refactor
Tests
Greptile Summary
This PR upgrades the header usage bar from a single weekly-percent chip to a dual-window chip (5-hour + plan window), and replaces the startup-delay polling model with continuous 120s interval refreshes, focus-triggered refresh (when data is older than 60s), and serial-request deduplication to prevent stale cached reads from overwriting a fresher forced refresh.
HeaderUsageControl.tsx: NewwindowPercentFor/headerUsageForhelpers driveHeaderProviderUsageChip, which renders the 5h and plan (wk/mo) percentages with threshold colouring. TheuseEffectrefresh logic gainssnapshotRef-backed serial ordering, awindowfocus listener, and asetInterval— all cleaned up on unmount.usage.test.tsx(replacesUsageQuotaPanel.test.tsx): Migrates existingUsageQuotaPaneltests and adds newHeaderUsageControltests covering cold-start rendering, request-serial race protection, and interval polling with fake timers.Confidence Score: 5/5
Safe to merge — all changed paths are UI display and polling logic with no data-mutation side-effects, and the new tests cover the trickiest race scenario.
The serial-request deduplication logic is well-reasoned and directly tested. Cleanup is complete (interval, focus listener, onUpdate subscription all removed on unmount). The only notable issue is a local variable named
windowthat shadows the browser global — a lint-level naming concern with no runtime consequence.No files require special attention.
Important Files Changed
windowvariable inwindowPercentFormasks the globalwindow.Sequence Diagram
sequenceDiagram participant C as Component (mount) participant B as usageBridge participant W as window events C->>B: onUpdate?.subscribe C->>B: getSnapshot() [serial 1, cached] C->>B: refresh() [serial 2, forced] Note over C: whichever resolves first wins<br/>(serial deduplication) B-->>C: snapshot (serial 2 latest → apply) B-->>C: snapshot (serial 1 stale → discard if ref≠null) W->>C: focus event C->>C: lastPolledAt ≥ 60s stale? C->>B: refresh() [force] loop every 120s C->>B: refresh() [force] B-->>C: snapshot (apply if latest serial) end C->>C: unmount / cleanup C->>W: removeEventListener focus C->>C: "clearInterval, cancelled=true" C->>B: unsubscribe()Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "ship: prepare lane for review" | Re-trigger Greptile