fix: make Codex cost catch-up append-linear - #2726
Conversation
clawsweeper
Bot
added
proof: sufficient
![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: 6dce98a9bb
ℹ️ 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".
| _ = try usageRowStore.garbageCollect( | ||
| publishedGenerationIDs: publishedGenerationIDs, | ||
| gracePeriod: 24 * 60 * 60, | ||
| now: maintenanceNow) |
There was a problem hiding this comment.
When a Workspace refresh loads the published JSON cache and another app/CLI process subsequently replaces a generation, this sweep can immediately delete that old generation if its created_at_ms is over 24 hours old. CodexLocalProjectUsageIndexer releases the refresh lease before loading the cache and later opens the row store during synchronizeSources, so it can still hold the old JSON reference when this deletion occurs; the ensuing load/loadSuffix reports needsRebuild and aborts an otherwise valid Workspace refresh. Base the grace period on when the generation became unreferenced, or keep readers under the same lease through sidecar import.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
status: ⏳ waiting on author
|
Codex review: needs maintainer review before merge. Reviewed August 7, 2026, 7:00 AM ET / 11:00 UTC. ClawSweeper reviewWhat this changesThis PR makes bounded Codex cost catch-up append-linear by storing token and usage-row history in SQLite sidecars while retaining JSON as the published cache authority. Merge readinessKeep open for maintainer review: current main does not contain this sidecar migration, and no line-level correctness blocker was found in the final head; its persistent-cache and cross-process-lock scope warrants an explicit merge-risk decision. Priority: P2 Review scores
Verification
How this fits togetherCodexBar scans local Codex JSONL sessions to derive local cost history for Usage & Spend and the CLI. This change sits between bounded session parsing and those consumers, replacing repeated in-memory reconstruction with persisted, validated cache state. flowchart LR
A[Codex JSONL sessions] --> B[Bounded cost scanner]
B --> C[Append and source validation]
C --> D[SQLite token index]
C --> E[SQLite usage-row store]
D --> F[Published JSON cache]
E --> F
F --> G[Usage & Spend]
F --> H[CLI cache commands]
Decision needed
Why: Tests and real-corpus evidence support the implementation, but accepting a new on-disk storage protocol and its release timing is a product-risk decision rather than a mechanical code repair. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land only if an owner accepts the SQLite-sidecar upgrade boundary with JSON publication remaining authoritative and the tested migration, contention, and cache-clear recovery behavior preserved. Do we have a high-confidence way to reproduce the issue? Yes. A growing Codex JSONL corpus exercised through repeated bounded catch-up passes exposes the historical rebuild path; the PR also supplies a privacy-safe real-corpus run and deterministic regression coverage for the final head. Is this the best way to solve the issue? Yes, conditionally. Persisting validated append state while retaining JSON as the atomic publication authority is the narrow maintainable way to remove repeated historical reconstruction, subject to maintainer acceptance of the migration risk. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 22b24b885693. 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 (5 earlier review cycles)
|
6dce98a to
2556dff
Compare
clawsweeper
Bot
added
status: 👀 ready for maintainer look
|
Decision on this PR — and it's the good kind of decline. Your analysis convinced the owner to go further than the sidecar: the verified review of this PR (quadratic catch-up confirmed at the old head, your linear-work regression proof, invariants preserved) triggered the call to migrate Codex cost storage to a single SQLite database entirely, deleting the JSON artifact and its whole bounding/pruning/producer-key machinery rather than maintaining a dual store. The monolithic decode/encode is the root of every incident this subsystem produced this month (#2637, #2646, #2703), and your append-state design showed the way out. So this PR won't merge as-is — not because anything in it is wrong, but because "JSON stays publication authority" is the part we're removing. The migration series is starting immediately (tracking issue follows shortly, linking here); your accumulator schema and linearity test design are being carried into it with credit. You're the person who understands this subsystem best from the outside — review on the series would be genuinely valued, and if you'd rather build any phase of it yourself, say so on the tracking issue and it's yours. Thanks for a rigorous PR — fourth one in this area, and each one has moved the architecture forward. |
2 participants
Summary
Make bounded Codex usage catch-up append-linear while preserving the accounting and fork-safety guarantees added by #2452, #2525, and #2648.
This change:
The existing automatic limits remain unchanged: at most 256 MiB per file and 512 MiB per pass, followed by the existing background duty cycle.
Problem
#2452 bounded scans of very large Codex session corpora. #2525 restored accurate fork accounting during bounded catch-up, and #2648 preserved ordinary-fork progress when a session file was appended.
The remaining slowdown came from older cache architecture shared by those paths: after reading each new bounded JSONL suffix, the scanner rebuilt increasingly large in-memory token snapshots, checkpoints, fingerprints, and workspace usage rows from all accumulated history. Work therefore grew with every pass and approached quadratic behavior. Finish Now exposed that behavior by running passes back-to-back; it did not create it.
There were also narrow resume gaps around metadata-free files, predecessor producer generations, and bounded metadata parsing. Those could force redundant work or make an otherwise reusable cache generation unavailable.
Implementation
Append-linear token index
The token sidecar stores the accumulator state needed at the next byte offset plus append-only usage events. A pass reads and parses only the new JSONL suffix when the stored prefix is still valid.
Append reuse is accepted only after validating the physical file identity, ctime/size relationship, and sampled prefix anchors. Ambiguous ownership or content changes still use the conservative byte-zero reconciliation path.
Transactional usage-row publication
The workspace sidecar stores immutable usage-row generations. Compact JSON cache state remains the publication authority, so a crash may leave SQLite ahead of JSON but cannot expose an unpublished partial generation.
Per-entry producer keys allow compatible cached rows to survive a global producer migration. The first append after migration imports the predecessor generation and then processes only the JSONL suffix.
Reader-safe generation retirement
Garbage collection now records when a generation first becomes absent from the complete published JSON reference set. It deletes that generation only after it has remained unreferenced for the full 24-hour grace period, rather than measuring age from generation creation time.
The SQLite schema migrates from v1 to v2 in place while the refresh lease is held. A generation selected by a concurrent reader therefore remains loadable across a replacement publication, and republishing a retired generation clears its prior retirement deadline before the next sweep.
Fork and subagent safety
Resolved forks reuse validated inherited baselines. Missing-parent forks and subagent buffers remain deferred and fail closed: the scanner does not publish incomplete suffix accounting and does not silently fall back to counting inherited cumulative totals as new usage.
Finish Now coordination
Finish Now bypasses the normal scanner debounce, retries transient lock contention and no-progress passes, and identifies metadata-only progress without resetting the visible byte count. Cross-process locking also provides a stable cache-clear barrier.
Privacy-safe representative validation
I ran Finish Now-style bounded catch-up against an isolated projection of real local Codex JSONL data. The projection was copied before the run, and the original session files were not modified.
No paths, usernames, session IDs, dates, model names, token totals, prompts, responses, or JSONL content are included in this proof.
The representative run validates the core indexing and publication design. After the final narrow edge-case fixes, the exact submitted tree was also checked with deterministic regressions for:
The reviewer-reported reader/GC interleaving also has a deterministic, privacy-safe before/after proof using two row-store instances sharing a synthetic database:
deletedGenerationCount = 1)deletedGenerationCount = 0)needsRebuilddeletedGenerationCount = 1)Additional regressions verify that v1-to-v2 migration preserves published rows and that republishing a retired generation restarts its grace window. All identifiers and token values in these tests are synthetic.
Verification
swift test --jobs 2 listprebuild, followed by the repository's isolated sharded test runner: 834 test selections in 70 groups; 70/70 passed on the first attempt; 0 failures, retries, or timeouts; 806.8 s total.ProviderArchitectureGatekeeperTests: 38/38 passed, including the exact-anchor and provider-reference fingerprint scan added on currentmain.make check: portable repository checks, parser hash, SwiftFormat, and strict SwiftLint all passed; 0 SwiftLint violations across 1,821 files.git diff --checkpassed.Scope and limitations
No UI strings or layouts changed, so screenshots are not applicable.