Add focused daily spend ledger - #2615
Conversation
clawsweeper
Bot
added
rating: 🦪 silver shellfish
![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: 96157c4fc5
ℹ️ 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".
| calendar: Calendar) -> [DailySummary] | ||
| { | ||
| guard !summaries.isEmpty, | ||
| summaries.allSatisfy({ !$0.hasInvalidCostHistory }), |
There was a problem hiding this comment.
When a snapshot establishes history coverage but has both an empty daily array and a nil last30DaysCostUSD—as an empty scanner result can—hasInvalidCostHistory remains false even though InputSummary.totalCost is nil. This guard therefore accepts the source, and dailyProviderRow fills every covered day with 0, causing the new ledger to report daily $0 values while the aggregate and chart correctly report spend as unavailable; require complete cost proof, such as a non-nil summary total, before constructing the ledger.
Useful? React with 👍 / 👎.
| guard let aggregate = input.snapshot.last30DaysRequests else { return true } | ||
| return Self.nonnegative(aggregate) == dailyTotal |
There was a problem hiding this comment.
The ledger's request completeness now depends on last30DaysRequests, but SpendDashboardSource.snapshotRevision fingerprints the token/cost aggregates and daily request counts without fingerprinting this aggregate. If a provider-derived or cached snapshot is installed without advancing its publication revision and only this aggregate changes, the pane's configuration remains equal and the controller does not rebuild, leaving request totals available when they should fail closed (or vice versa); add last30DaysRequests to the snapshot revision alongside the other aggregates.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
the
merge-risk: 🚨 other
|
Codex review: needs real behavior proof before merge. Reviewed August 3, 2026, 3:00 PM ET / 19:00 UTC. ClawSweeper reviewWhat this changesThis PR adds a read-only per-day ledger beneath the existing Usage & Spend chart, showing each day’s provider activity, tracked tokens, request totals, and estimated spend. Merge readiness⛔ Blocked until real behavior proof is added - 11 items remain Keep open: the same two P2 correctness defects remain at the reviewed head, and the claimed local runtime verification has no inspectable after-fix evidence. Priority: P2 Review scores
Verification
How this fits togetherCodexBar collects provider usage snapshots and builds a shared spend-dashboard model for the Settings UI. The proposed ledger consumes those snapshots alongside the existing chart, so its totals must preserve the dashboard’s established completeness and refresh semantics. flowchart LR
A[Provider usage snapshots] --> B[Coverage and completeness checks]
B --> C[Spend dashboard model]
C --> D[Existing spend chart]
C --> E[Daily ledger aggregation]
E --> F[Settings spend dashboard]
A --> G[Snapshot revision fingerprint]
G --> C
Decision needed
Why: The repository vision requires sign-off for new features, and this PR introduces a persistent new UI capability rather than a repair to an existing dashboard contract. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: If the feature is sponsored, preserve unavailable costs as unavailable, fingerprint every aggregate used by ledger completeness, add focused regressions, and attach a redacted or synthetic native-app capture that visibly demonstrates refreshed ledger rows. Do we have a high-confidence way to reproduce the issue? Yes, by source: create an established-coverage snapshot with empty daily data and no aggregate cost, then build the dashboard model; the proposed ledger accepts it and emits zero-cost rows while the aggregate stays unavailable. A second source path changes only Is this the best way to solve the issue? No. The scoped model/UI approach is plausible, but it is not ready until it preserves unavailable-cost semantics, invalidates on all request inputs it consumes, and has inspectable runtime proof; the new feature also needs the repository’s required sign-off. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against e12a824fdb49. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
clawsweeper
Bot
added
P2
96157c4 to
e897510
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
e897510 to
8403735
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: Add focused daily spend ledger This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
1 participant
Summary
This is the focused follow-up requested in #2485: the daily spend ledger only.
This PR does not add currency conversion, exchange-rate polling, preference changes, packaging changes, or fork customization. It uses the app-wide currency presentation already present upstream.
Aggregation semantics
Request totals are derived from complete daily rows when a snapshot does not provide
last30DaysRequests, which covers sources such as Codex and OpenCode Go. If a covered row omits its request count, arithmetic overflows, or a provided aggregate contradicts the daily rows, request totals fail closed to unavailable.The ledger uses the intersection of every provider's established coverage. It renders every day in that intersection, including zero-usage days, and renders no combined ledger when provider coverage is disjoint. Sources without a proven aggregate cost remain unavailable instead of becoming zero-spend rows. The existing chart remains unchanged and the ledger is read-only, avoiding selection snapping from chart-only days outside common ledger coverage.
Review fixes
last30DaysRequestsin the snapshot revision so cached/provider-derived aggregate changes rebuild the ledgerVerification
make checkSpendDashboardDailyLedgerTests: 7 testsSpendDashboardControllerTestsand revision coverage: 25 tests across 3 suitesmake test: 810/810 selections passed in 68 groupsupstream/mainbefore final verificationRuntime proof
The original implementation was verified in a signed local macOS build with 30 days of Codex history. After the review fixes, the production SwiftUI ledger component was rendered again with synthetic Claude and OpenAI data so no personal spend history is exposed.
The synthetic capture shows combined-provider totals, single-provider days, token and request columns, and the app's native dark Settings presentation.