Repository navigation
Reduce token activity heatmap update costs - #4328
Yuxin-Qiao wants to merge 5 commits into
Conversation
|
🦞👀 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. |
|
Codex review: needs maintainer review before merge. Reviewed October 8, 2026, 5:22 AM ET / 09:22 UTC (Revision 8). ClawSweeper reviewWhat this changesCache annual token-activity dates, coverage counts, and localized date formatters, and calculate weekly totals only when needed. Review scores
ProductKind: Performance · Worth it: Yes Merge readiness✅ Ready for maintainer review This PR is useful, sufficiently proven, and has no actionable review findings or additional contributor blockers. Current main still performs the repeated work it addresses. Likely related people: steipete and Yuxin-Qiao are high-confidence routing candidates from merged heatmap history. Priority: P3 Before mergeNone. FindingsNone. Agent review detailsHow this fits togetherCodexBar’s Usage & Spend settings page turns retained token history into daily, weekly, and cumulative activity charts. This change reduces repeated chart calculations while preserving dates, totals, coverage, and selection. flowchart LR
A[Retained token history] --> B[Annual activity snapshot]
C[Reporting calendar] --> B
B --> D{Selected chart mode}
D --> E[Daily chart]
D --> F[Weekly or cumulative totals]
G[Cached localized date labels] --> E
G --> F
Technical reviewBest possible solution: Retain the bounded immutable caches and lazy aggregation while preserving existing chart output and reporting-calendar semantics. Do we have a high-confidence way to reproduce the issue? Not applicable as a functional bug reproduction: source confirms repeated work on main, and comparative measurements plus runtime counters demonstrate the optimization. Is this the best way to solve the issue? Yes: immutable snapshot data, bounded formatter reuse, and mode-specific aggregation address the measured costs without adding settings or another data source. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 08eb56931c8e. Provenance checked
TestingProof path: shipped entry point. Added test files: 3. SecurityNone. EvidenceWhat I checked:
Likely related people:
Review metrics
LabelsLabel changes: No label changes. Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (7 earlier review cycles)
|
Keep the contributor date and formatter caches, reuse saturating totals and navigation data, and move fixture-only construction into tests. Add reproducible helper measurements and document pixel-identical native render coverage. Sync the reviewed main baseline without rewriting contributor history. Co-authored-by: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com>
Annual token-activity updates repeatedly created medium-date formatters and recalculated the same calendar dates and coverage. Cache normalized dates, visible indices, and coverage in each immutable series; reuse at most 16 immutable formatter contexts keyed by calendar, locale, and time zone; build weekly aggregates only in weekly/cumulative modes.
Maintainer cleanup shares the existing saturating-add operation, removes redundant navigation wrappers, and keeps direct fixture construction in the tests. Production diff versus reviewed main
c3c6ce06120: +63/−70 lines. Contributor history is preserved. Thanks @Yuxin-Qiao!Measured helper elapsed time
Apple M3 Ultra, Swift 6.4 debug/native backend, 365 synthetic dates, Gregorian Asia/Shanghai, zh_Hans_CN; median of 21 samples after three warmups. These measurements describe helpers, not whole-app frame rates.
Verification
make check: passed with zero violations on retry. The first attempt hit an unchanged subprocess-cleanup fixture's exit-observation race; no check was weakened.swift test --build-system native --jobs 4 -Xswiftc -gnone --filter 'SpendActivityBenchmarkTests|SpendActivityPerformanceTests|SpendActivityHeatmapTests|SpendActivityAppearanceTests|SpendActivityReadabilityRenderTests|SpendDashboardLongRangeRenderTests': 41 Swift Testing tests passed. The optional long-range screenshot test was skipped without its separate proof flag.SpendActivityReadabilityRenderTestsretry passed all four tests. All 120 before/after captures have identical dimensions and decoded RGBA bytes across language, theme, width, mode, partial data, and recorded zero.CODEXBAR_ACTIVITY_BENCHMARK=1 swift test --skip-build --build-system native --jobs 4 -Xswiftc -gnone --filter SpendActivityBenchmarkTests: passed and produced the table above. No wall-clock assertions were added.Final combined source proof at
8f564053f16d0b1a027e35828dca44c49df34d2c:The wrapper forwards
test/buildtoswiftwith--build-system native --jobs 4 -Xswiftc -gnone. 1,592 selections, 144/144 groups successful on the first attempt, zero retries and zero timeouts. The earlier 180-second invocation stopped on an unrelated publication-suite timeout; source and assertions were unchanged for the successful rerun. The pushed integration branch istriage/20260921-spend-prs-23; its later commit adds only the contributor's runtime-proof documents.Synthetic before/after
Before:
After, pixel-identical:
These are production-component renders with synthetic fixtures. Reproduction and scope:
docs/proof/spend-activity-performance.md.Refs #4297.
The contributor subsequently added proof-only commit
1e2b5b25492c535612795e580bb05089f95257a9;Sources,Tests, andWidgetExtensionare unchanged from tested maintainer headf7a9968e3c4ebf8e958ebb86627f4b2ff10287fe. Its additional full-settings runtime capture is indocs/proof/spend-activity-performance/. The read-only verifier passed: 11 checkpoints, 731 date/context checks, zero mismatches, 9,375 cache hits and two formatter creations. The diagnostic app was not independently launched here.CI for proof-only head
1e2b5b254was queued at that status check. The previous head's run was cancelled when this proof commit arrived.Main synchronization
Commit
197422ae094239be93242dd7dda28364071ab878merges main atf8b75cf2aand resolves the soleCHANGELOG.mdconflict by retaining both sets of release notes. The activity heatmap and app entry source remain byte-identical to the pinned diagnostic build, and the committed runtime verifier still passes. New upstream provider and spend-trend changes are outside that recorded runtime capture.After synchronization,
make checkpassed (2,853 Swift files, zero lint violations);git diff --checkpassed. The full repository runner./Scripts/test.sh --direct-workers 4, using the default 180-second group timeout and scrubbed test environment, exited successfully: 1,600 selections, 145/145 groups successful on the first attempt, zero failures, retries, or timeouts. Total elapsed time was 313.5 seconds. The preceding serialmake testand one-worker direct invocations were deliberately interrupted to switch execution modes; those interrupted invocations are not counted as passed.The automated review of synchronized head
197422ae0accepts the logs and reports no actionable findings. GitHub lint, Linux x64/ARM64/musl, security checks, and the macOS compatibility build passed on that head; both macOS test shards remained queued before the next synchronization.Subsequent main synchronization
Commit
a13ab86eab173b22fdc0b1b374651475af7db55bmerges main at08eb56931after further upstream changes again overlapped inCHANGELOG.md. Both release-note sets are retained. The activity heatmap and app entry source remain byte-identical to the pinned diagnostic build, and the runtime verifier passes. The original capture does not claim runtime validation of newly imported provider, menu, or account changes.make checkpassed after this synchronization (2,870 Swift files, zero violations), andgit diff --checkpassed. The complete repository runner./Scripts/test.sh --direct-workers 4also exited successfully on this head with the unchanged default 180-second group timeout: 1,607 selections, 145/145 groups successful on the first attempt, zero failures, retries, or timeouts. Total elapsed time was 945.6 seconds, including 513.7 seconds of build/discovery and 425.0 seconds of execution. The test environment was scrubbed and Keychain access suppressed.Automatic review revision 7 covers
a13ab86ea, accepts the runtime logs, and reports no actionable findings or contributor blockers. GitHub lint, Linux x64/ARM64/musl, and security checks have passed; both macOS test shards plus the macOS compatibility build are queued. The PR is mergeable and awaiting maintainer review, and has not merged.