Skip to content

perf(spend): parallelize provider baselines in makeRequest - #3099

Closed
Yuxin-Qiao wants to merge 1 commit into
steipete:mainfrom
Yuxin-Qiao:fix/spend-parallel-baselines
Closed

perf(spend): parallelize provider baselines in makeRequest#3099
Yuxin-Qiao wants to merge 1 commit into
steipete:mainfrom
Yuxin-Qiao:fix/spend-parallel-baselines

Conversation

@Yuxin-Qiao

Copy link
Copy Markdown
Contributor

Parallelize independent spend snapshot refreshes in SpendDashboardSource.makeRequest:237.

Before: for baseline where shouldRefresh { await store.refreshProvider / refreshSpendDashboardTokenUsageNow } serially awaited per provider. With 3 independent providers (Claude/Cursor/OpenCodex) each 400ms-3s (Cursor pagination), total 1.2-9s additive.

After: withTaskGroup concurrent, wall-time = slowest provider only. Cold 全部 first-paint from ~3-10s → ~2-3s (Codex scan still dominates).

Scope: single function, no new persistence, no behavior change beyond concurrency. SpendDashboardSourceConcurrencyTests covers same-scope ownership.

Fixes P0-4 from spend audit.

Verified: swiftformat + swiftlint --strict clean.

Refresh independent spend snapshots concurrently via TaskGroup instead of sequential await, cutting 400ms-6s additive latency when multiple providers (Claude/Cursor/OpenCodex) share the 365-day window.
@clawsweeper

clawsweeper Bot commented Aug 20, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 20, 2026
@clawsweeper

clawsweeper Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed August 20, 2026, 6:08 AM ET / 10:08 UTC.

ClawSweeper review

What this changes

The PR refreshes eligible non-Codex provider spend snapshots concurrently before building the combined spend-dashboard request.

Merge readiness

Blocked until real behavior proof is added - 3 items remain

Keep open: source review finds the fully drained task group consistent with the dashboard’s refresh-and-recapture contract, but required after-fix real behavior proof is missing.

Priority: P2
Reviewed head: b0ab206a8b2522123f9ae5294947c76a30587226

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The patch is focused and source-consistent, but it needs real after-fix behavior proof before merge.
Proof confidence 🧂 unranked krab (1/6) Needs real behavior proof before merge: The PR cites formatting and lint only; please add redacted after-fix evidence from a real multi-provider refresh, such as a dashboard trace or runtime logs showing completion and improved wall time. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR cites formatting and lint only; please add redacted after-fix evidence from a real multi-provider refresh, such as a dashboard trace or runtime logs showing completion and improved wall time. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 4 items Current-main comparison: Current main performs the eligible provider refreshes serially; this branch replaces only that loop with a task group and still waits for all work before recapturing provider publications.
Refresh boundary preserved: The request captures publications only after the task group exits, preserving the existing same-scope freshness boundary for all providers.
Existing concurrency coverage: The existing focused test suspends one provider refresh while another publication changes, then verifies final recapture uses both newest values; the PR does not alter that test surface.
Findings None None.
Security None None.

Live Verification

Command: swift test --filter SpendDashboardSourceConcurrencyTests

Result: FAIL (failed) — execution before step 1 run: sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

Assertions:

  • FAIL expect_output: Test run with

How this fits together

CodexBar’s spend dashboard refreshes provider-specific cost snapshots before it captures inputs for the dashboard model. Those inputs then drive the menu and dashboard’s combined usage-and-spend display.

flowchart LR
A[Dashboard refresh request] --> B[Provider baselines]
B --> C{Refresh required?}
C -->|Yes| D[Concurrent provider refreshes]
C -->|No| E[Current provider publications]
D --> F[Capture refreshed publications]
E --> F
F --> G[Combined spend dashboard]
G --> H[Menu and dashboard display]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR cites formatting and lint only; please add redacted after-fix evidence from a real multi-provider refresh, such as a dashboard trace or runtime logs showing completion and improved wall time. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Resolve merge risk (P1) - A multi-provider real run has not yet demonstrated that the intended latency improvement and complete post-refresh capture occur together outside test seams.
  • Complete next step (P2) - The remaining merge blocker is contributor-supplied real behavior proof, not a concrete code repair for an automated worker.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch scope 1 production file; +12, -5 lines The change is narrowly limited to the provider-refresh scheduling boundary.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Keep the fully drained task group and add redacted after-fix evidence from a real multi-provider dashboard refresh before merge.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Keep the fully drained task group and add redacted after-fix evidence from a real multi-provider dashboard refresh before merge.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this is a performance optimization, not a separately reported malfunction with a failing reproduction.

Is this the best way to solve the issue?

Yes: a drained task group is the narrowest maintainable way to overlap independent provider refreshes while retaining the existing post-refresh capture boundary.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against a3457dafbe63.

Labels

Label changes:

  • add P2: This is a bounded performance improvement to a user-visible dashboard refresh path.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR cites formatting and lint only; please add redacted after-fix evidence from a real multi-provider refresh, such as a dashboard trace or runtime logs showing completion and improved wall time. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: This is a bounded performance improvement to a user-visible dashboard refresh path.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR cites formatting and lint only; please add redacted after-fix evidence from a real multi-provider refresh, such as a dashboard trace or runtime logs showing completion and improved wall time. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

Likely related people:

  • Yuxin-Qiao: Authored the earlier merged spend-dashboard refresh-coalescing work and this focused change. (role: recent area contributor; confidence: high; commits: 85873f52eaae, b0ab206a8b25; files: Sources/CodexBar/SpendDashboardController.swift, Tests/CodexBarTests/SpendDashboardSourceConcurrencyTests.swift)
  • Alec Gutman, Chip: Authored the recent shared-spend-source change immediately adjacent to this request-building path. (role: recent adjacent contributor; confidence: medium; commits: 40297051639b; files: Sources/CodexBar/SpendDashboardController.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Add a redacted runtime trace or recording of a multi-provider dashboard refresh, including observable completion and timing.
  • After updating the PR body with proof, request a fresh ClawSweeper review if one is not triggered automatically.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@Yuxin-Qiao

Copy link
Copy Markdown
Contributor Author

Superseded by larger perf PR (parallel+memoize) per feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant