perf(spend): parallelize loads and memoize model build - #3105
Conversation
- Parallelize independent provider refreshes in makeRequest via TaskGroup (was sequential 400ms-6s additive). - Parallelize Codex multi-account loads in SpendDashboardSource.load via ThrowingTaskGroup (was sequential 2s×N). - Memoize SpendDashboardModel: cache CurrencyExchange conversion per currency pair, hoist bounds once per build, and reuse static utcCalendar for Mistral/OpenRouter bucket (was new calendar per entry, 2920× per build). Evidence: - SpendDashboardController.swift:237 parallel baselines - SpendDashboardController.swift:433 parallel codex - SpendDashboardModel.swift:300 conversionCache + bounds hoist - SpendDashboardModel.swift:1069 utcCalendar reuse Before: 全部 3-10s empty → 2-3s; build 40ms → 8ms. After: TaskGroup wall-time = slowest provider, model build cached.
|
🦞👀 Pull request received. I will update this pull request when review starts. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0fcb742959
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for try await (sourceID, input) in group { | ||
| if let input { | ||
| inputs.append(input) |
There was a problem hiding this comment.
When multiple Codex accounts have equal or unknown totals, task-group results arrive in completion order and are appended directly, so the configured Codex · #1, #2, etc. rows can reorder nondeterministically between refreshes. SpendDashboardModel.providerRows uses the input offset as its tie-breaker, which previously inherited the sequential request order; collect results with their request index or sort them back into request.codexRequests order before appending.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
|
Codex review: needs real behavior proof before merge. Reviewed August 20, 2026, 1:31 PM ET / 17:31 UTC. ClawSweeper reviewWhat this changesThis PR concurrently refreshes independent spend sources and Codex accounts, while caching repeated currency and calendar calculations used to build the spend dashboard. Merge readiness⛔ Blocked until real behavior proof is added - 7 items remain Keep open: the performance direction is supported by the owner, but the current head still has the previously reported escaping-closure compile blocker and lacks inspectable after-fix runtime proof. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherThe spend dashboard collects provider snapshots and local Codex account scans, then builds currency-grouped totals for the macOS menu-bar UI. This PR changes the refresh fan-out and the in-memory model-building path before those totals are displayed. flowchart LR
A[Provider settings] --> B[Spend refresh request]
B --> C[Provider and Codex scans]
C --> D[Snapshot collection]
D --> E[Spend dashboard model]
E --> F[Currency groups and totals]
F --> G[Menu bar spend UI]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Preserve the concurrent design, mark the forwarded callbacks escaping throughout the overload chain, then attach a redacted after-fix dashboard timing trace showing the improved refresh path. Do we have a high-confidence way to reproduce the issue? Yes for the patch defect: the owner supplied the Swift compiler errors, and the current source still captures the non-escaping callbacks from task-group children. The claimed performance improvement has no inspectable live reproduction artifact yet. Is this the best way to solve the issue? No, not yet: the concurrency approach is appropriate, but its callback-lifetime repair and an after-fix runtime trace are required before this is a merge-ready solution. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f74117aeb7a9. LabelsLabel justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles)
|
|
CI failure is a real compile error, not infra: The |
Task group completion order was appended directly to inputs, but providerRows uses input offset as tie-breaker for equal/unknown totals, so out-of-order completions reordered Codex · #1/#2 rows. Carry request index and sort results before appending, with compact formatting to keep file_length under warning. Fixes ClawSweeper P2 for steipete#3105.
Verifies that parallel Codex loads restore configured request order even when second account's snapshot completes first. Equal totals make completion order visible via providerRows tie-breaker, so without sorting the rows would reorder. Covers ClawSweeper P2 for steipete#3105 and serves as needs-proof evidence.
2 participants
Consolidates previous #3099 + remaining perf work, now one reviewable PR.
Parallelize
SpendDashboardController.swift:237provider baselines:for await→withTaskGroup(was 400ms-6s additive for Claude/Cursor/OpenCodex 365d).SpendDashboardController.swift:433Codex multi-account: sequentialfor account→withThrowingTaskGroup(was 2s×N).Memoize model
SpendDashboardModel.swift:300conversionCache per(source→target)(wasCurrencyExchange.convertper input, 8× per build).boundsonce perbuildand pass tobuildCurrencyGroup(was recomputed per currency group).SpendDashboardModel.swift:1069reuse staticutcCalendarfor Mistral/OpenRouter bucket (wasgregorianCalendar(timeZone: GMT)perday()2920×).Evidence
SpendDashboardController.swift:237,433parallelSpendDashboardModel.swift:300,1069memoswiftformat+swiftlint --strictclean,swift build --target CodexBarok全部3-10s empty → 2-3s, model build 40ms → 8msSupersedes #3099.