Add focused daily spend ledger - #2635
Open
sahilaidev wants to merge 4 commits into
Open
Conversation
clawsweeper
Bot
added
proof: sufficient
|
Codex review: needs changes before merge. Reviewed August 12, 2026, 8:11 AM ET / 12:11 UTC. ClawSweeper reviewWhat this changesAdds a read-only daily Usage & Spend ledger that shows per-day provider activity, tracked tokens, requests, and estimated spend. Merge readinessKeep open: the focused ledger remains useful and has direct UI proof, but the branch is merge-conflicting and still predates current main’s released 365-day token-activity heatmap. Priority: P2 Review scores
Verification
How this fits togetherThe Usage & Spend dashboard converts provider snapshots and cached Codex history into currency-group summaries, charts, and activity views in Settings. The proposed ledger would consume the same grouped daily data alongside the existing 365-day token-activity heatmap. flowchart LR
A[Provider snapshots] --> B[Cached Codex activity]
A --> C[Spend dashboard loader]
B --> C
C --> D[Dashboard model]
D --> E[Spend charts]
D --> F[Token activity heatmap]
D --> G[Daily spend ledger]
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: Rebase the ledger onto current main, retaining cached token activity and the heatmap, then validate both views from a freshly built Settings bundle. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR proposes a feature; the merge conflict and missing current-main activity path are directly established by the branch/current-main comparison. Is this the best way to solve the issue? No—the ledger approach is focused, but this branch must first be rebased onto the current activity model so it does not regress shipped dashboard behavior. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a90dfed5c264. LabelsLabel 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
HistoryReview history (26 earlier review cycles; latest 8 shown)
|
sahilaidev
marked this pull request as ready for review
August 4, 2026 04:02
![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: 8403735fef
ℹ️ 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".
Comment on lines
+691
to
+695
| Text(L("Day")).frame(width: 112, alignment: .leading) | ||
| Text(L("Providers")).frame(minWidth: 96, maxWidth: .infinity, alignment: .leading) | ||
| Text(L("Tracked tokens")).frame(width: 90, alignment: .trailing) | ||
| Text(L("Requests")).frame(width: 72, alignment: .trailing) | ||
| Text(L("Estimated spend")).frame(width: 116, alignment: .trailing) |
There was a problem hiding this comment.
At the default 880-point window—and more noticeably at the supported 800-point minimum—the fixed column widths, minimum provider width, spacing, and horizontal padding require more width than the dashboard viewport provides. Because the enclosing ScrollView is vertical-only, the ledger extends beyond or clips against the right edge instead of adapting or allowing horizontal scrolling; use flexible columns, reduce fixed widths, or add a horizontal scrolling layout.
Useful? React with 👍 / 👎.
|
|
||
| var body: some View { | ||
| HStack(spacing: 12) { | ||
| Text(self.summary.day, format: .dateTime.weekday(.abbreviated).day().month(.abbreviated)) |
There was a problem hiding this comment.
When CodexBar's selected language differs from the macOS system language, this Text date format uses the SwiftUI environment/system locale, while the surrounding labels use L(...); the preferences hierarchy only rebuilds on appLanguage changes and does not override EnvironmentValues.locale. The chart and this row's accessibility label already explicitly use codexBarLocalizedLocale(), so apply that locale to the visible date format as well to avoid mixed-language rows.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦐 gold shrimp
clawsweeper
Bot
added
the
merge-risk: 🚨 compatibility
clawsweeper
Bot
added
rating: 🐚 platinum hermit
clawsweeper
Bot
added
status: 👀 ready for maintainer look
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This is the focused follow-up requested by repository owner @steipete in #2485: daily spend ledger only, without currency conversion or fork-packaging changes. It supersedes the closed #2615 after moving the branch to a new fork.
last30DaysRequestsin the dashboard snapshot revision so request-only refreshes invalidate the rendered ledger.Maintainer direction
In #2485, @steipete identified the daily spend ledger as the interesting part of the original mixed-scope contribution and invited a focused follow-up containing only the ledger, with tests for request aggregation and coverage semantics. This PR implements that requested scope and leaves the final product-acceptance decision to normal maintainer review.
User impact
Users can inspect the daily activity behind the existing aggregate spend dashboard without changing provider settings or cost-calculation behavior.
Verification
swift test --filter SpendDashboardDailyLedgerTests— 7 passedswift test --filter SpendDashboardControllerTests— 25 passedmake check— passed with zero format or lint violationsCODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 make test— 810 selections passed across 68 groups; zero failures, retries, or timeoutsSynthetic UI evidence
All values below are synthetic and contain no account identity or personal spend data.