perf: persist Claude cost report memo across launches - #3284
perf: persist Claude cost report memo across launches#3284eggyrooch-blip wants to merge 1 commit into
Conversation
Keep the local-cost daily report and source inventory next to claude-v6.json so an unchanged transcript set skips the full JSON cache decode after process restart. Pricing and cache-artifact stamp changes still miss the memo and reprice from the persisted rows. Investigated alongside steipete#3247.
|
🦞👀 Pull request received. I will update this pull request when review starts. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 778b3793c2
ℹ️ 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".
| } | ||
|
|
||
| static let shared = CostUsageClaudeReportMemo() | ||
| static let persistedVersion = 1 |
There was a problem hiding this comment.
When a user upgrades to a build that changes bundled Claude pricing, model aliases, or report aggregation while the transcripts, cache artifact, pricing artifact, and requested date range remain unchanged, this constant still accepts the report produced by the previous binary. loadClaudeDaily then returns that report before running the current CostUsagePricing/aggregation code, so costs can remain calculated with old application logic until another key input changes; include an application/report-logic version in the persisted key or invalidate this sidecar across relevant upgrades.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
|
Codex review: needs real behavior proof before merge. Reviewed August 30, 2026, 6:00 AM ET / 10:00 UTC. ClawSweeper reviewWhat this changesThe PR saves Claude’s computed local cost report beside its JSON cache so an unchanged transcript inventory can be reused after app relaunch. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 4 items remain Keep open: the persistent memo can return a report produced by an older app build after an upgrade, so cost totals or aggregation remain stale until an unrelated source or artifact changes. The PR also needs real after-fix behavior proof beyond its synthetic tests. Priority: P2 Review scores
Verification
How this fits togetherCodexBar scans local Claude transcripts into a JSON cache and derives daily cost reports for usage and spend views. This PR adds a sidecar report memo between that cache/transcript inventory and the report consumers. flowchart LR
A[Claude transcript files] --> B[Cost scan and JSON cache]
B --> C[Report memo validation]
C -->|valid| D[Saved daily report]
C -->|invalid| E[Decode cache and reprice]
D --> F[Usage and spend views]
E --> F
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Version the sidecar against report semantics, invalidate it when that revision changes, and demonstrate an upgraded cache being rebuilt before merge. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: save a sidecar, change report semantics between builds while leaving transcript, cache, and pricing stamps unchanged, then reload. The early return skips the current report builder; this review did not execute the scenario. Is this the best way to solve the issue? No. Persisting the memo is a reasonable performance approach, but its acceptance key must include an explicit report-semantics revision and upgrade regression coverage. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 39c15c6ad7f6. LabelsLabel changes:
Label 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
|
2 participants
Summary
Claude local-cost still uses the monolithic
claude-v6.jsonartifact. The in-memory report memo already skips that decode on a warm refresh, but it dies with the process. On a machine with ~7.7k Claude jsonl files the cache is 44 MB / 116kclaudeRows; a 0.56.0 cold start spent ~90s at ~90–115% CPU in_NSJSONReader parseDataoncom.steipete.codexbar.cost-usage-scanbefore going idle.This persists the existing memo (source inventory + report key including cache/pricing stamps + daily report) next to the cache as
claude-v6.report-memo.json. After launch, an unchanged transcript inventory returns the saved report and does not decode the JSON cache. Pricing-catalog and cache-artifact stamp changes still miss the memo and reprice from persisted rows, without rewriting transcripts.Does not migrate Claude onto SQLite (#2760 was Codex-only). Append/delete of a jsonl still invalidates the memo and takes the existing decode + delta-parse path.
Investigated alongside #3247.
Test plan
cold process reuses the persisted report memo without decoding the cache— evict in-memory memo, 0 cache decodes / 0 transcript parses, report identicalmissing persisted memo still decodes the cache without parsing transcripts— delete sidecar, still 1 cache decode / 0 parsesswift test --filter CostUsageScannerClaudeMemoTests/CostUsageClaudeKimiAliasTests/SpendDashboardFreshnessScannerTests(local toolchain is Swift 6.0.3; package requires 6.2 — relying on CI)