Fix Usage & Spend scroll lag on long ranges - #3998
Yuxin-Qiao wants to merge 2 commits into
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
clawsweeper
Bot
added
P2
|
Codex review: needs real behavior proof before merge. Reviewed September 25, 2026, 1:53 PM ET / 17:53 UTC (Revision 2). ClawSweeper reviewWhat this changesThe PR initially shows 30 recent daily spend rows with an option to expand them, combines provider tooltips per row, and avoids repeated Dock activation-policy queries during window updates. Merge readiness⛔ Blocked before merge - 2 items remain The long-range ledger still renders every day on current main, and this focused PR addresses that path. No definite introduced bug emerged, but the reported scrolling improvement has no after-change observation from a built macOS app, so merge readiness remains blocked on real behavior proof. Priority: P2 Review scores
Verification
How this fits togetherCodexBar turns provider usage into daily spend summaries in its Settings window. The ledger renders those summaries, while Settings window updates also drive Dock icon policy. flowchart LR
A[Provider usage] --> B[Daily spend summaries]
B --> C[Selected history range]
C --> D[Ledger row limit]
D --> E[Settings window]
E --> F[Window updates]
F --> G[Dock policy]
Before merge
Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Keep the short initial ledger and full-history control, then validate smoother All-range scrolling, expansion, and normal Dock behavior in a freshly built macOS app. Do we have a high-confidence way to reproduce the issue? No independent current-main runtime reproduction was run. Current source renders a row for every covered day, and the contributor supplies a specific v0.66.0 sampling scenario. Is this the best way to solve the issue? Yes, provisionally: the 30-row initial limit follows the existing Projects-list control and targets the reported rendering cost. A native after-change observation is needed to verify the improvement. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against d8c2af958176. LabelsLabel changes: No label changes. Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
Co-authored-by: Cursor <cursoragent@cursor.com>
Show the newest 30 daily rows initially and retain the complete ledger behind Show all. Consolidate daily/hourly chart styling and per-row provider help without changing source history, totals, chart scope, or exports. Adopt the bounded-ledger portion of #3998 with expanded fixture coverage. Retain the v0.65.0 default status-item width contract while investigating #3993. Co-authored-by: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com>
|
Thanks @Yuxin-Qiao. The ledger portion landed on main via #4001 with your co-author credit and changelog thanks, and ships in the next release. On a synthetic 365-day fixture the warm initial layout went from 248.7 ms to 41.1 ms with totals unchanged ($365, 365,000 tokens, 365 requests): the ledger now shows the newest 30 rows first and Show all restores every row, and the daily and hourly charts share their presentation types. Your three-day regression fixture was replaced with explicit long-range coverage. The Dock-policy change was left out because nothing here measures LaunchServices or live scrolling; if you have a reproducible measurement for it, a separate PR is welcome. |
Show the newest 30 daily ledger rows first with Show all/Show less, and let the daily and hourly charts share their presentation types, colors, legend, and currency axis, so a 365-day range lays out about six times faster with totals, models, export, and chart range unchanged. Adopts the ledger portion of steipete#3998. Closes steipete#3998 Co-authored-by: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com>
2 participants
Summary
Scrolling Settings → Usage & Spend with the All range stutters badly. Sampling the running app (0.66.0) while scrolling showed where the time goes:
NSWindow _resetDragMarginsIfNeededOn every scroll frame AppKit rebuilds the window's titlebar drag region by unioning the opaque content paths of the hosted SwiftUI views (
CGRegionCreateWithBoundsBlock→shape_union). The daily ledger renders one row per day, so All gives 365 rows. ItsLazyVStacksits inside a horizontalScrollViewnested in the pane's verticalScrollView, so it isn't lazy on the vertical axis and every row gets built. Each provider icon also had its own.help, which adds tracking areas (about 100 more samples went tocursorUpdate:forwarding).Changes:
Show all (N)/Show lesstoggle (same pattern as the Projects list). I replaced the non-functionalLazyVStackwith a plainVStack..helpper icon to one.helpper row, listing all active providers. Accessibility labels are unchanged.DockIconController.ensureRegularPolicyruns on everyNSWindow.didUpdateNotification, and it calledNSApp.activationPolicy()each time. That call is a synchronous LaunchServices XPC round trip, and it showed up at 1–7% of main-thread samples while Settings was open. It's now skipped when the controller has already promoted the app to.regular.Testing
long ledger ranges start collapsed to the newest daystoSpendDashboardDailyLedgerTests.swift test --filter SpendDashboardDailyLedgerTests: 17/17 passed.make check: 0 files need formatting, 0 SwiftLint violations.make test: all 128 groups (1407 selections) passed on the first run, with no retries.