feat(cli): add Codex session cost grouping - #2854
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
bcbcdff to
7a851ad
Compare
clawsweeper
Bot
added
P3
|
Codex review: found issues before merge. Reviewed August 15, 2026, 3:01 PM ET / 19:01 UTC. ClawSweeper reviewWhat this changesAdds Merge readinessKeep this PR open: the prior P2 JSON-warning defect remains on the current head, while the proposed CLI capability is absent from current main and v0.50.0. Priority: P3 Review scores
Verification
How this fits togetherThe cross-platform flowchart LR
A[Local Codex and mirror session caches] --> B[Cost usage fetcher]
C[Provider and grouping flags] --> D[Cost command]
D --> B
B --> E[Grouping and warning decision]
E --> F[Text cost report]
E --> G[JSON cost payload]
Decision needed
Why: VISION.md requires sign-off for new features, and the source cannot determine whether this additional reporting mode is desired product surface. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: After product sign-off, gate skipped-grouping warnings to text output and add a focused JSON regression test so the new Codex-only text mode leaves JSON semantics untouched. Do we have a high-confidence way to reproduce the issue? Yes for the review finding: Is this the best way to solve the issue? No: the intended text-only boundary is maintainable, but the warning helper must also receive and respect the output format before the mode is safe to merge. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f15f142a7787. 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 (9 earlier review cycles; latest 8 shown)
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
![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: 7a851ad7f8
ℹ️ 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 historyLabel = snapshot.historyLabel | ||
| ?? (snapshot.historyDays == 1 ? "Today" : "Last \(snapshot.historyDays) days") | ||
| var lines = [header, "Conversations (\(historyLabel)):"] | ||
| guard !snapshot.sessions.isEmpty else { |
There was a problem hiding this comment.
When the default Pi/OMP inclusion finds any Codex activity in the selected window, CostUsageFetcher.loadLocalTokenScanResult deliberately clears snapshot.sessions after merging that activity (CostUsageFetcher.swift:585-587). This new guard therefore renders — even when native Codex conversations and nonzero aggregate totals exist, so codexbar cost --provider codex --group-by session is empty for affected users unless they discover --provider-native-only. The session-grouped path should either request native-only data or retain/build breakdowns for every merged session source.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
merge-risk: 🚨 other
|
@clawsweeper re-review |
|
🦞🧹 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: ea1e4ecf2d
ℹ️ 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".
| guard provider == .codex, groupBy == .session, format == .text else { return includePiSessions } | ||
| return false |
There was a problem hiding this comment.
When Codex has Pi/OMP session mirrors and the user runs text --group-by session without --provider-native-only, this unconditionally changes the requested default from merged usage to native-only, so those costs disappear without any notice. This conflicts with CLIHelp.swift:122-124, which says Pi/OMP sessions are included unless the explicit native-only flag is used, and can make the conversation report look complete while underreporting usage. The fresh helper avoids the previously reported empty native-session output, but it should also warn that Pi/OMP usage was omitted (or retain a grouped fallback for it).
Useful? React with 👍 / 👎.
|
@clawsweeper re-review |
|
🦞🧹 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: 839464de74
ℹ️ 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 historyLabel = snapshot.historyLabel | ||
| ?? (snapshot.historyDays == 1 ? "Today" : "Last \(snapshot.historyDays) days") | ||
| var lines = [header, "Conversations (\(historyLabel)):"] | ||
| guard !snapshot.sessions.isEmpty else { |
There was a problem hiding this comment.
When --refresh starts a bounded rescan of a Codex archive larger than the 512 MB per-refresh budget, CostUsageFetcher.loadLocalTokenScanResult returns the previous aggregate report while leaving sessions empty (CostUsageFetcher.swift:548-560) and marks historyCoverageIsEstablished false. This guard consequently prints — and exits successfully even though the snapshot can contain substantial nonzero usage and conversations will appear after catch-up. Fresh evidence distinct from the earlier Pi-merging case is the fetcher's previous-report branch during bounded scan catch-up; render that state as incomplete/scanning rather than as no conversations.
Useful? React with 👍 / 👎.
|
@clawsweeper re-review |
|
🦞🧹 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: ea4cf5a14e
ℹ️ 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".
| guard !snapshot.sessions.isEmpty else { | ||
| if snapshot.historyCoverageIsEstablished == false { |
There was a problem hiding this comment.
On an initial Codex scan whose archive exceeds the 512 MB refresh budget, the fetcher can build a nonempty session list from the partially populated cache while setting historyCoverageIsEstablished to false (CostUsageFetcher.swift:555-563, 589-595). Because coverage is checked only inside this empty-list guard, the CLI prints those partial conversations as a complete report with no catch-up notice. Fresh evidence beyond the already-handled empty-history case is this first-scan path, where partial session rows exist; show the incomplete-history message whenever coverage is false, including before nonempty rows.
Useful? React with 👍 / 👎.
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
P3
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
rating: 🦐 gold shrimp
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
2 participants
Summary
Add
codexbar cost --group-by sessionfor human-readable Codex session cost output.Why
Expose the existing local session breakdowns without changing scanning, pricing, persistence, JSON output, or macOS UI behavior. Session-grouped Codex text requests keep native session rows by excluding Pi/OMP aggregate merging only for that path.
User impact
Users can inspect recent Codex cost estimates per conversation while preserving the existing default and project-grouped output.
Validation
swift test --filter CLICostTests(26/26 passed, including partial and incomplete catch-up rendering)swift test --filter CostUsageFetcherTests(16/16 passed, including mixed native Codex + Pi coverage)swift test --filter ProviderArchitectureGatekeeperTests(38/38 passed)make check(passed; SwiftLint 0 violations across 1842 files)git diff --check(passed)CodexBarCLI cost --help(passed on the current head)Real behavior proof
Captured from an existing local Codex session cache using current head
24d9458bc. The cost cache and model-pricing cache were copied into a temporary home;CODEX_HOMEpointed at the existing session root, and the original cache files were not used for writes.Command:
Result: exit 0, 441 session rows, 886 output lines, and 1 warning line on stderr. The original
cost-usage.sqlite,-shm, and-walhashes, sizes, and mtimes were unchanged during the run. The current head also passes mixed native Codex + Pi coverage and the bounded catch-up renderer regressions for empty and partial session lists.Scope
CLI presentation, help text, focused tests, and CLI documentation only. No scanner, pricing, auth, persistence, JSON schema, or macOS UI changes.
Review follow-up (2026-08-16)
24d9458bc38388ac25ef82b4854eec07d0d0481f.swift test --filter CLICostTests(26/26),swift test --filter CostUsageFetcherTests(16/16), andmake check(SwiftFormat clean; SwiftLint 0 violations across 1842 files).