Add SQLite CostUsageStore foundation - #2761
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b435e986f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| private static func deleteOldestRetainedRow(_ database: OpaquePointer) throws -> Bool { | ||
| let statements = [ | ||
| "DELETE FROM files WHERE id = (SELECT id FROM files ORDER BY updated_at_ms, id LIMIT 1)", |
There was a problem hiding this comment.
Reconcile day aggregates before deleting file rows
When row or byte budget enforcement trims a file that has already contributed to day_aggregates, this delete cascades away the file_day_aggregates row but leaves the global aggregate intact. readReport reads from day_aggregates, so after a budget trim the report can still include usage for a file that no longer exists and whose per-file contribution can no longer be subtracted; subtract the file contribution or mark/rebuild the report state before deleting the file row.
Useful? React with 👍 / 👎.
| state.filePathBySessionID = state.filePathBySessionID.filter { | ||
| !sessionIDs.contains($0.key) && !paths.contains($0.value) | ||
| } | ||
| state.nextFileIndex = min(state.nextFileIndex, state.filePaths.count) |
There was a problem hiding this comment.
Reset discovery cursors after pruning file paths
If retention removes a path before the current discovery cursor, this min can leave nextFileIndex pointing past an unprocessed survivor; for example [old, remaining] with nextFileIndex == 1 becomes [remaining] and nextFileIndex == 1, so the remaining file is treated as already scanned. Reset or recompute the discovery cursors after filtering paths so the next discovery pass re-enqueues the shortened list correctly.
Useful? React with 👍 / 👎.
|
Codex review: needs changes before merge. Reviewed August 8, 2026, 2:39 AM ET / 06:39 UTC. ClawSweeper reviewWhat this changesAdds a zero-call-site SQLite CostUsageStore with schema, read/write, retention, and test foundations for a later Codex cost-usage migration. Merge readinessThis owner-authored Phase 1 foundation is still required by the open SQLite migration plan, but three retention defects make the patch incorrect before its later scanner/report cutover. Priority: P2 Review scores
Verification
How this fits togetherCodex cost scanning derives usage from local session files and cache state. This store is intended to persist file scan state and aggregates so a later phase can serve app and CLI reports from SQLite. flowchart LR
A[Codex session files] --> B[Future cost scanner]
B --> C[SQLite usage store]
C --> D[File scan state]
C --> E[Usage aggregates]
C --> F[Retention and budgets]
E --> G[Future app and CLI reports]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Keep the Phase 1 no-call-site boundary, but make every budget eviction honor retention protections, reconcile global aggregates, and recompute discovery progress before later phases consume the store. Do we have a high-confidence way to reproduce the issue? Yes. Focused store tests can deterministically create protected files, per-file aggregates, and a shifted discovery list before invoking budget enforcement or retention. Is this the best way to solve the issue? No. The Phase 1 shape is owner-approved, but direct eviction must use the same protection and aggregate-reconciliation rules as normal retention before this is a safe foundation. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8a06c21c0ddf. LabelsLabel changes:
Label 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
|
Summary
Adds the Phase 1 SQLite persistence foundation from #2760 with zero production call sites.
The new actor-owned
CostUsageStorecreatescost-usage/cost-usage.sqlitewith WAL, a 5-second busy timeout, foreign keys, and incremental auto-vacuum. Schema compatibility combines an explicit base schema version with the generated Codex parser hash inPRAGMA user_version, with the exact parser hash also retained in metadata. Version, open, integrity, and runtime SQLite failures drop the database plus WAL/SHM artifacts and recreate it because this is derived data.Schema decisions
fileskeeps source identity, inode/size/mtime, parsed bytes, validation anchors, typed scan-state payload, coverage, and update ordering.token_snapshotsis append-addressed by file/event index.file_day_aggregatesretains each file's replaceable contribution so Phase 2 can subtract or replace changed sources safely.day_aggregatesstores report-ready day/model totals with cost, token, request, and priority breakdowns.fork_lineage,buffered_lines,discovery_state, andlookback_statepreserve fork/subagent/retry/discovery state.accumulatorspersists terminal cumulative-token state following the linear append concept from fix: make Codex cost catch-up append-linear #2726, with credit to @xx205.The actor makes single-writer ownership explicit while WAL permits independent app/CLI read-only connections. Window retention is inclusive, protects incomplete/buffered/parent-dependent files, prunes stale discovery references, and trims both per-file and global aggregates. Budget enforcement applies remembered-window retention, row caps, byte caps, WAL checkpoints, and incremental vacuum.
Storage-only
CostUsageStore*files are excluded from the parser-source hash generator. Store layout changes are instead required to bump the explicit store schema version; adding this zero-call-site persistence code therefore does not rotate current parser/cache behavior.No scanner/cache call sites, JSON cutover, changelog, or behavior changes are included.
Proof
Refs #2760