Show selected-provider spend in Overview - #3023
Conversation
|
🦞👀 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: 8e7340b2b1
ℹ️ 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".
| let spendModel = self.overviewSpendDashboardModel(providers: overviewProviders) | ||
| if !spendModel.groups.isEmpty { |
There was a problem hiding this comment.
When cost tracking is enabled but the user selects Cost summaries → Submenu only, this path still unconditionally inserts OverviewSpendSummaryCardView directly into the main Overview menu. The existing preference explicitly suppresses main-menu summaries through costSummaryShowsInline(for:), so the new overview summary should also be gated by the inline display preference.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
|
Codex review: needs real behavior proof before merge. Reviewed August 17, 2026, 7:16 PM ET / 23:16 UTC. ClawSweeper reviewWhat this changesThe PR adds a compact Usage & Spend card above Overview provider rows, scoped to selected providers and showing partial coverage and separate currencies. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 7 items remain Keep open: the new card bypasses the existing Cost summaries display preference, so users who chose submenu-only or off will still receive an inline Overview summary. The attached image is a component-render test rather than a real Overview-menu run. Priority: P2 Review scores
Verification
How this fits togetherCodexBar builds the macOS menu-bar Overview from configured usage providers and their stored token snapshots. This change aggregates those snapshots into a spend and token card before the provider rows. flowchart LR
A[Configured providers] --> B[Overview selection]
B --> C[Stored usage snapshots]
C --> D[Spend summary calculation]
D --> E[Overview menu card]
B --> F[Provider rows]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Gate the Overview card through the established inline-summary policy, add focused coverage for inline versus submenu-only/off, and provide a redacted live Overview-menu capture after the fix. Do we have a high-confidence way to reproduce the issue? Yes: with cost tracking enabled and Cost summaries set to submenu-only or off, the patched insertion at line 581 still adds the inline card because it never checks the display-style policy. Is this the best way to solve the issue? No: the card should reuse the established inline-summary gate rather than introducing a second path that overrides the saved preference. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f7723d33a169. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
|
Thanks @Chipagosfinest! This landed via #3027, which carried your commits onto current main (after the #3015–#3019 spend stack reshaped the same surfaces), reconciled the semantics — partial totals render with ~ and unpriced spend stays unavailable, matching the new coverage model — and refreshed the stale gatekeeper anchors that were failing CI. The changelog credits you. |
2 participants
Summary
Scope
This is the focused Overview slice extracted from #2759 and rebuilt on current
main. It preserves the existing six-provider Overview fallback and does not change provider refresh, Settings defaults, share/export, or token allocation.Current
mainalready contains the newer All Time, partial-total, project-model, and project-panel foundations, so none of those are duplicated here.Verification
swift test --filter OverviewSpendSummaryTestsCODEXBAR_SPEND_PROOF_DIR=.github/pr-proof swift test --filter SpendDashboardScreenshotRenderTestsmake check