Track multi-provider spend and model usage - #2759
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed August 9, 2026, 5:12 PM ET / 21:12 UTC. ClawSweeper reviewWhat this changesExpands CodexBar Usage & Spend into a multi-provider dashboard, menu overview, and local share card with 7-, 30-, and 365-day ranges, explicit partial coverage, and every tracked provider visible. Merge readiness⛔ Blocked until stronger real behavior proof is added - 5 items remain Keep open for maintainer product sign-off and stronger current-head proof. Current main still caps an unset merged Overview at six providers and documents only 7/30-day spend history, so this PR’s central capability is not already implemented; however, its all-provider default changes existing unset setups and its late Codex RPC timeout-race change lacks direct runtime evidence. Priority: P2 Review scores
Verification
How this fits togetherCodexBar collects provider status and optional local cost-history snapshots, aggregates them by provider and currency, then presents the result in Settings, the merged menu-bar Overview, and a local share card. This PR changes the roster and coverage rules that feed all three user-facing summaries. flowchart LR
A[Provider settings and accounts] --> B[Usage and cost snapshots]
B --> C[Spend aggregation]
C --> D[Coverage and partial-total rules]
D --> E[Settings dashboard]
D --> F[Merged menu overview]
D --> G[Local share card]
Decision needed
Why: This changes the default information density and menu interaction for existing users without a stored selection; source and tests can verify mechanics but cannot choose the intended product default. Before merge
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: Approve or narrow the all-provider default explicitly, then add a redacted production-boundary timeout trace showing that the Codex RPC EOF race now reports a timeout while retaining the existing visual proof. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction: this is a new dashboard and menu capability. The supplied captures provide high-confidence visual evidence for the UI portion, but not for the separate timeout-race behavior. Is this the best way to solve the issue? Unclear until product intent is confirmed. The aggregation and disclosure model is coherent, but changing the unset menu default needs explicit acceptance and the late RPC change needs direct runtime proof. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4a83b87aff22. LabelsLabel 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 (53 earlier review cycles; latest 8 shown)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b03a03bd1a
ℹ️ 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".
|
Thanks for the sharp review. Two clarifications on the product and architecture decisions:
I also fixed the two concrete hosted-Xcode regressions exposed by the previous run at |
|
@steipete exact-head CI is green and every inline review thread is resolved. ClawSweeper's remaining concrete concern is valid on QuickJS, but its suggested JavaScript repair is not cross-engine:
The honest maintainer choices are:
I recommend 1 if the bounded provider-specific tail is acceptable; otherwise 2. I do not recommend hiding the engine limitation behind Current head: |
|
Scoped this down in response to the compatibility P2: OpenRouter now keeps its existing credits and key-quota behavior, while Validation before push:
@codex review @steipete the remaining owner decision is now only whether an unset Overview should show every connected provider instead of the legacy first six. Exact head: |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2194fc72c4
ℹ️ 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".
What this changes
CodexBar Usage & Spend now works as a true multi-provider overview instead of stopping at Claude and Codex.
Unknown spend is never turned into
$0. Estimates use~, currencies remain separate, and incomplete coverage stays explicit.Screenshots
All values shown here are synthetic.
Combined Overview
Share card
Wide settings · Narrow settings · Proof notes
OpenRouter scope
OpenRouter stays inside the existing plugin boundary on JavaScriptCore and QuickJS. An ordinary API key continues to report credits and key quota.
Management
/activityenrichment is deliberately deferred to a separate follow-up. Removing it restores the existing single optional-request timeout budget and avoids adding a provider-specific history payload or a broader async QuickJS host contract to this PR.Verification
Exact head:
2194fc72cmake check: localization, manifests, docs, SwiftFormat, and SwiftLint passed.This is the conflict-free successor to #2469 and includes the provider-runtime convergence from #2758. OpenRouter
/activityis preserved separately for a clean follow-up after this lands.