Show selected-provider spend in Overview - #3027
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0828161bd1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return SpendDashboardModel.build( | ||
| inputs: inputs, | ||
| requestedDays: self.settings.costUsageHistoryDays, | ||
| now: now, | ||
| preferredCurrencyCode: self.settings.preferredCurrencyCode) |
There was a problem hiding this comment.
Use the pinned calendar for Overview spend
When the system time zone has changed since cost tracking pinned costUsageBucketTimeZoneIdentifier, this call falls back to Calendar.current, unlike SpendDashboardController.rebuildModel, which passes the configured bucket calendar. Around either zone's midnight, the Overview can therefore interpret stored YYYY-MM-DD buckets in a different zone from the scanner and include or exclude a boundary day, making its spend and coverage disagree with Usage & Spend; pass settings.costUsageBucketCalendar into this build.
Useful? React with 👍 / 👎.
|
Codex review: needs changes before merge. Reviewed August 17, 2026, 10:12 PM ET / August 18, 2026, 02:12 UTC. ClawSweeper reviewWhat this changesThe branch adds a selected-provider Usage & Spend card above Overview provider rows, using current provider snapshots and existing inline-cost display preferences. Merge readinessKeep open. Current main has the spend dashboard but no Overview spend card; this PR supplies that missing integration, but it can disagree with the dashboard when the pinned cost-bucket time zone differs from the system time zone. Priority: P2 Review scores
Verification
How this fits togetherCodexBar converts provider usage snapshots into a shared spend-dashboard model and then renders menu cards. This change sends Overview’s selected providers through that model and presents the resulting spend, coverage, and provenance summary in the Overview menu. flowchart LR
A[Provider snapshots] --> B[Overview selected providers]
B --> C[Inline cost preference]
C --> D[Spend dashboard model]
D --> E[Coverage and provenance summary]
E --> F[Overview menu card]
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:
Copy recommended automerge instructionTechnical reviewBest possible solution: Pass SettingsStore.costUsageBucketCalendar to the Overview model build and add a boundary-day regression test with a different system and bucket time zone. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: configure a persisted bucket zone different from the system zone and open Overview near either midnight; it uses Calendar.current while Usage & Spend uses the pinned bucket calendar. Is this the best way to solve the issue? No. The feature integration is appropriate, but it must use the same configured calendar as the existing dashboard rather than the model’s system-calendar default. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4dd5e4258f5c. LabelsLabel changes:
Label justifications:
EvidenceAcceptance criteria:
What 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
|
Supersedes #3023 without closing it. This preserves @Chipagosfinest's original commit and credits the contribution in the changelog.
Summary
Review verdict
The design fits the new stack cleanly. A priced subset renders with a leading approximation marker and explicit subscription coverage; wholly unpriced data renders as unavailable. Distinct currencies stay separate, and the row does not borrow identity, plan, or spend data across providers.
UI proof
Before: Overview had no selected-provider aggregate spend row.
After:
Verification
swiftformat Sources Testsusing the repository-pinned SwiftFormat 0.61.1swiftlint --strictusing the repository-pinned SwiftLint 0.65.0make checkswift test --filter 'OverviewSpend|SpendDashboard|ProviderArchitectureGatekeeper'(203 tests passed; one opt-in screenshot test skipped in this invocation)CODEXBAR_SPEND_PROOF_DIR=.github/pr-proof swift test --filter SpendDashboardScreenshotRenderTestsmake test(892 selections in 75 groups; zero failures or retries)