Skip to content

feat: cut Codex cost persistence over to SQLite, delete the JSON artifact (refs #2760) - #2762

Merged
steipete merged 4 commits into
mainfrom
feat/cost-store-sqlite-cutover
Aug 8, 2026
Merged

feat: cut Codex cost persistence over to SQLite, delete the JSON artifact (refs #2760)#2762
steipete merged 4 commits into
mainfrom
feat/cost-store-sqlite-cutover

Conversation

@steipete

@steipete steipete commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Summary

Cut the Codex cost pipeline over to the Phase 1 CostUsageStore and delete the Codex JSON artifact authority entirely.

  • Scanner state, source-validation anchors, token snapshots, usage rows, fork lineage, retry buffers, day/model aggregates, discovery/lookback state, previous reports, and terminal token accumulators now persist in cost-usage.sqlite.
  • App, CLI, serve, cached token activity, and local project/session consumers now read Codex history through the store. Public CostUsageDailyReport and snapshot models are unchanged.
  • The former 1,159-line JSON cache implementation and its 1,677-line JSON-enforcement test suite are deleted. Claude and Vertex retain a separate, provider-specific compact cache implementation.
  • The legacy codex-v11.json artifact and atomic-write sidecars are deleted at the single startup cleanup boundary; the SQLite cache is rebuilt from bounded source scans instead of migrated.

Schema and behavior

  • Bumps the store base schema to v2; PRAGMA user_version continues to combine the base version with the generated parser hash, so incompatible/corrupt stores rebuild cleanly.
  • Adds append-addressable usage rows and preserves terminal accumulator state so one appended token row is parsed, repriced, fingerprinted, and stored once; stable EOF performs zero row work.
  • Keeps the fix: keep Codex fork catch-up progress across appends #2648 inode/size/SHA-256 source validation ladder intact and restores append anchors from SQLite.
  • Re-expresses Bound Codex cost cache persistence size (#2637) #2646 retention as store-native requested-window pruning. Incomplete files, buffered retries, referenced fork parents, and recently active zero-day sessions remain protected.
  • Enforces a 25,000 retained-session row-equivalent cap and 256 MiB database cap after every scan, using incremental vacuum. In-window trimming marks catch-up and preserves the prior complete report.
  • CostUsageStore uses a portable dedicated serial executor as the only in-process writer. WAL plus busy_timeout keeps independent app/CLI read-only snapshots safe during writes.
  • Workspace-sidecar event publication is idempotent by (rollout_path, event_index), including time-zone re-bucketing.

Thanks @xx205 for the terminal-accumulator and append-linearity design carried forward from #2726.

Proof

  • swift test --filter CostUsage: 389 tests in 25 suites passed.
  • swift test --filter CostUsageStoreCutoverTests: cleanup, fixture parity, and append-linearity tests passed.
  • make check: parser hash current; SwiftFormat clean; strict SwiftLint clean with 0 violations.
  • Docker Linux build, Swift 6.3.3 with libsqlite3-dev: build completed successfully.
  • Codex autoreview: clean, no accepted/actionable findings (patch is correct, confidence 0.98); documentation follow-up review also clean (0.99).
  • Broader ./Scripts/test.sh: stopped at group 10/69 after the unrelated BoundedChildProcessProofTests PTY cleanup timing assertion exceeded its 10-second budget twice (14.0 seconds); no cutover test failed.
  • git diff --check: clean.

No UI strings or layouts changed, so screenshots are not applicable.

Refs #2760.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Aug 8, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3167cd9d59

ℹ️ 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".

@@ -1,1159 +0,0 @@
import Foundation

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove stale provider-gatekeeper allowlist entries

Deleting this file leaves ProviderArchitectureGatekeeperTests with several SuppressedProviderReference/AllowedProviderConstruct entries that still point at Sources/CodexBarCore/Vendored/CostUsage/CostUsageCache.swift (for example around lines 1523 and 3609 in that test). The cross provider case clusters are derived or specifically justified test removes entries only when their file is scanned, then fails any leftover paths as “file does not exist in a shipped Swift target”, so make test will now fail until those stale allowlist rows are removed or retargeted.

Useful? React with 👍 / 👎.

@clawsweeper

clawsweeper Bot commented Aug 8, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge. Reviewed August 8, 2026, 5:07 AM ET / 09:07 UTC.

ClawSweeper review

What this changes

This PR replaces Codex cost-history JSON persistence with a SQLite store and redirects scanning, reports, CLI, and local-project consumers to that store.

Merge readiness

Blocked by patch quality or review findings - 9 items remain

Keep open: the SQLite cutover still allows a partially published scan generation and treats ordinary SQLite contention as destructive corruption recovery. Two user-facing docs also retain the deleted JSON-cache path.

Priority: P1
Reviewed head: e48215beba0e8c1eb7848c45597e1e6065cc0ce8

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The migration is substantial and directionally coherent, but two source-proven persistence blockers prevent a merge-ready rating.
Proof confidence 🌊 off-meta tidepool Not applicable: This owner-authored PR is not subject to the external-contributor proof gate; its reported tests remain supplemental validation.
Patch quality 🦪 silver shellfish (2/6) 3 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This owner-authored PR is not subject to the external-contributor proof gate; its reported tests remain supplemental validation.
Evidence reviewed 5 items Current main does not contain the cutover: Current main still loads Codex history through the JSON cache helper, so this PR remains necessary.
Save publishes multiple independent transactions: The save path persists files, rows, aggregates, metadata, discovery, lookback state, and retention through separate calls; a reader or interruption can observe a mixed generation.
Busy errors trigger database rebuild: Any database error falls into rebuildDatabase; SQLITE_BUSY or SQLITE_LOCKED after the configured timeout is not distinguished from corruption.
Findings 3 actionable findings [P1] Publish one complete cache generation atomically
[P1] Do not rebuild on ordinary SQLite contention
[P2] Update the remaining deleted-cache documentation
Security None None.

How this fits together

CodexBar scans local Codex session JSONL files into bounded cost history consumed by reports, the menu app, the CLI, and workspace summaries. This PR makes SQLite the persistence boundary for that history while retaining the existing in-memory report models.

flowchart LR
A[Codex session JSONL] --> B[Bounded cost scanner]
B --> C[SQLite cost history store]
C --> D[Cost reports and snapshots]
D --> E[Menu app and CLI]
C --> F[Workspace usage sidecar]
Loading

Before merge

  • Publish one complete cache generation atomically (P1) - saveCodexCache commits file state before it replaces global aggregates and metadata. A concurrent readSnapshot can therefore reconstruct a cache with new per-file rows but old totals, and termination between calls leaves that mismatch on disk. Wrap the whole publication in one transaction or gate readers on a completed generation.
  • Do not rebuild on ordinary SQLite contention (P1) - After the 5-second busy timeout, SQLITE_BUSY/SQLITE_LOCKED reaches this catch block and rebuildDatabase deletes the store rather than retrying or preserving the previous snapshot. The shared executor only serializes this process; another process can still hold the write lock.
  • Update the remaining deleted-cache documentation (P2) - The cutover removes codex-v11.json, but this page and docs/codex.md still tell users that it is the Codex cache. Update both paths to cost-usage.sqlite and explain that the legacy JSON artifact is removed during startup cleanup.
  • Resolve merge risk (P1) - A reader can observe newly written file rows alongside old global aggregates or metadata, and an interruption can leave that mixed cache state persisted.
  • Resolve merge risk (P1) - A transient SQLite busy/locked result can delete the cache and force an expensive rescan rather than preserving the prior report.
  • Resolve merge risk (P1) - Documentation still tells users to inspect or remove a JSON artifact this branch deletes.
  • Improve patch quality - Publish a whole scan generation atomically and add a reader/interruption regression.
  • Improve patch quality - Keep SQLITE_BUSY and SQLITE_LOCKED retryable without deleting the cache, with a separate-process contention regression.
  • Improve patch quality - Correct both remaining Codex cache-path documents.

Findings

  • [P1] Publish one complete cache generation atomically — Sources/CodexBarCore/Vendored/CostUsage/CostUsageStore+CodexCache.swift:23-43
  • [P1] Do not rebuild on ordinary SQLite contention — Sources/CodexBarCore/Vendored/CostUsage/CostUsageStore.swift:139-150
  • [P2] Update the remaining deleted-cache documentation — docs/codex-workspaces.md:36
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Migration scope 36 files affected; +2,147 / -3,271 lines The branch replaces the persisted Codex cache authority across scanner, reporting, and consumer paths.
Production versus tests production +1,716/-1,326; tests +429/-1,944 A large JSON implementation and test suite are removed, so SQLite-specific equivalence and failure-path coverage are merge-critical.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #2760
Summary: This PR is the implementation candidate for the owner-approved single-SQLite migration.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Repair publication and contention boundaries (recommended)
    Make a complete scan atomically visible, preserve the current store on SQLITE_BUSY or SQLITE_LOCKED, and cover both paths with focused regressions.
  2. Pause the cutover
    Do not merge the persistence migration until its atomicity and lock-contention behavior are demonstrated on the current head.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Make saveCodexCache publish file rows, aggregates, metadata, discovery/lookback state, retention, and previous-report state as one atomic generation; do not route SQLITE_BUSY or SQLITE_LOCKED through rebuildDatabase; add interruption and separate-process contention regressions.

Technical review

Best possible solution:

Publish every scan result as one SQLite transaction or reader-visible generation, preserve the prior cache on retryable lock contention, and update both Codex cache-path documents to SQLite.

Do we have a high-confidence way to reproduce the issue?

Yes — source establishes a deterministic reader-between-transactions path and a busy/locked error path that invokes rebuildDatabase; focused store regressions can exercise both.

Is this the best way to solve the issue?

No — a single SQLite authority matches the approved direction, but separate commits inside one save and destructive busy handling are not a safe publication design.

Full review comments:

  • [P1] Publish one complete cache generation atomically — Sources/CodexBarCore/Vendored/CostUsage/CostUsageStore+CodexCache.swift:23-43
    saveCodexCache commits file state before it replaces global aggregates and metadata. A concurrent readSnapshot can therefore reconstruct a cache with new per-file rows but old totals, and termination between calls leaves that mismatch on disk. Wrap the whole publication in one transaction or gate readers on a completed generation.
    Confidence: 0.99
  • [P1] Do not rebuild on ordinary SQLite contention — Sources/CodexBarCore/Vendored/CostUsage/CostUsageStore.swift:139-150
    After the 5-second busy timeout, SQLITE_BUSY/SQLITE_LOCKED reaches this catch block and rebuildDatabase deletes the store rather than retrying or preserving the previous snapshot. The shared executor only serializes this process; another process can still hold the write lock.
    Confidence: 0.98
  • [P2] Update the remaining deleted-cache documentation — docs/codex-workspaces.md:36
    The cutover removes codex-v11.json, but this page and docs/codex.md still tell users that it is the Codex cache. Update both paths to cost-usage.sqlite and explain that the legacy JSON artifact is removed during startup cleanup.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 56a763eaa34c.

Labels

Label justifications:

  • P1: Incorrect publication or lock handling can make active Codex cost history unavailable or inconsistent.
  • merge-risk: 🚨 compatibility: Existing persisted JSON cache behavior is replaced by rebuild-first SQLite storage.
  • merge-risk: 🚨 availability: Transient SQLite lock contention currently destroys the cache instead of retaining a usable snapshot.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🌊 off-meta tidepool and patch quality is 🦪 silver shellfish.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This owner-authored PR is not subject to the external-contributor proof gate; its reported tests remain supplemental validation.

Evidence

Acceptance criteria:

  • [P1] swift test --filter CostUsageStoreCutoverTests.
  • [P1] swift test --filter CostUsageStoreTests.
  • [P1] make check.
  • [P1] make test.

What I checked:

Likely related people:

  • steipete: Authored the merged SQLite store foundation, the cutover branch, and the follow-up shared-executor change. (role: current feature owner; confidence: high; commits: 56a763eaa34c, 631b4d6267ba, e48215beba0e; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageStore.swift, Sources/CodexBarCore/Vendored/CostUsage/CostUsageStore+CodexCache.swift)
  • xx205: Authored the merged append-resume work whose validation and accumulator design this migration retains. (role: adjacent feature contributor; confidence: high; commits: 4f99e6aba8c3; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner+CacheHelpers.swift)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (2 earlier review cycles)
  • reviewed 2026-08-08T08:14:27.677Z sha 3167cd9 :: needs changes before merge. :: [P2] Remove every allowlist entry for the deleted cache file
  • reviewed 2026-08-08T08:29:52.862Z sha 502c04a :: found issues before merge. :: [P1] Publish one complete cache generation atomically | [P1] Treat lock contention as retryable, not corruption | [P2] Update the remaining cache-path documentation

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. P1 Urgent regression or broken agent/channel workflow affecting real users now. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. P2 Normal priority bug or improvement with limited blast radius. labels Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant