Split the codex parser hash into session and priority-turns domains#1422
Split the codex parser hash into session and priority-turns domains#1422ProspectOre wants to merge 8 commits into
Conversation
makeCodexRefreshPlan re-ran a full-table double-LIKE query over the codex CLI trace database on every refresh past the scan interval. The logs table only appends (INTEGER PRIMARY KEY AUTOINCREMENT), so the query result is now accumulated per database in process memory and only rows appended since the last call are examined. The database shrinking or being replaced (file identity change) and window expansion trigger a full rescan; windows ending before today keep the original bounded one-shot query so historical lookups never pay an open-ended scan.
…efreshes Two overlapping refreshes could both read the same memo, scan independently, and write back out of order, letting an older cursor replace newer accumulated state. Stored state now only advances unless coverage expands or the file is replaced. Also drop the release-owned CHANGELOG edit per review.
Whether a turn is priority is unknowable when its completion row is parsed, so non-priority completions accumulate in completedModelsByTurnID for the process lifetime. Evict the oldest completions beyond a fixed retention limit, keeping memory constant while preserving completion-before-request ordering and late model upgrades within the retention window.
The rowid-cursor memo keeps live refreshes incremental, but it is process-local: every app launch and every CLI invocation pays one full trace-database scan before incremental refreshes resume. Persist the memo to a versioned cost-usage artifact (parser-hash producer key, atomic replace-on-write) and seed it once per process through the monotonic store, so the first refresh after launch resumes from the persisted cursor.
…ains One hash over every non-Claude CostUsage source producer-keys the session cost cache, so a priority-turns-only parser change forces every user to re-parse their full sessions corpus on update. Hash the two domains separately: session parsing keeps invalidating the session cache, while priority-turns changes invalidate only the priority memo artifact and flow through the existing changedPriorityTurnIDs targeted re-derivation.
|
Codex review: needs maintainer review before merge. Reviewed June 11, 2026, 3:24 AM ET / 07:24 UTC. Summary Reproducibility: not applicable. as a conventional failing bug; the invalidation behavior is reproducible through controlled priority-only, session, and shared-source edits followed by hash regeneration. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land or finalize the prerequisite memo changes, then rebase and merge the isolated dependency-complete hash split with its producer-key assertions and controlled source-edit proof intact. Do we have a high-confidence way to reproduce the issue? Not applicable as a conventional failing bug; the invalidation behavior is reproducible through controlled priority-only, session, and shared-source edits followed by hash regeneration. Is this the best way to solve the issue? Yes in design: the asymmetric priority superset is dependency-complete without a fragile hand-maintained list and preserves the expensive session cache for priority-only edits; final approval should follow an isolated rebase. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 9015e94901c4. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
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. How this review workflow works
|
clawsweeper
Bot
added
proof: sufficient
Priority parsing depends on shared scanner helpers (timestamps, day ranges, the scanner core), so a filename-only partition could leave the persisted memo's producer key unchanged across a shared-dependency change. Hash all non-Claude sources into the priority domain: shared edits invalidate both keys by construction, while priority-only edits still leave the expensive session cache untouched.
|
Addressed the [P2] dependency-completeness finding in Fix: the priority hash now covers all non-Claude sources (superset), while the sessions hash stays the non-priority subset. This is dependency-complete by construction — no hand-maintained dependency list to drift: any shared-helper change (timestamps, day ranges, scanner core) necessarily moves the priority hash. The asymmetry is deliberate: the session cache is the expensive artifact to rebuild, while a priority memo rebuild is one bounded background scan, so the memo conservatively invalidates on any scanner change. The original win is intact — priority-only edits leave the session cache untouched. Coverage for the three edit classes (live script runs, restored to baseline after each): On the rebase note: agreed — I'll rebase to the final small diff as soon as the prerequisite stack (#1404, #1421) lands. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
rating: 🐚 platinum hermit
|
Superseded by the smaller measured fix in #1430, landed as dd8cf8b. Exact-head profiling on the same 60,607-file real archive showed the priority SQLite scan was about 4% of the expired-refresh cost; repeated Foundation metadata/resource-identifier reads and cached-path validation were dominant. The landed replacement removes that cost with +124/-24 lines, no persisted cursor/memo state, and measured a 6.74s warm expired refresh versus 20.22-37.43s across the stacked candidates. Thanks @ProspectOre for the investigation, implementation work, and benchmark evidence. Contributor credit is retained in the landed commit and changelog. |
2 participants
Problem
CodexParserHashis one hash over every non-Claude source inVendored/CostUsage/, and it producer-keys the codex session cost cache. Any change to codex priority-turns parsing therefore invalidates the entire session cache and forces every user to re-parse their full~/.codex/sessionscorpus on update — even though session JSONL parsing didn't change a byte.This isn't hypothetical: #1404 and #1421 each touch only
*CodexPriority*sources, and each would trigger a full session re-parse for every user on release day. The 0.32.x "release-day lag wave" pattern (#1387) was exactly this class of invalidation.Change
Split the generated hash into two domains:
CodexParserHash.sessions— non-Claude, non-priority-turns sources → producer-keys the codex session cost cache (codex:cu:p…)CodexParserHash.priorityTurns—*CodexPriority*sources → producer-keys the priority-turns memo artifact from Persist the codex priority-turns memo across launches #1421 (codex:pt:p…)Correctness is preserved by machinery that already exists: when priority parsing changes, the priority artifact invalidates and re-derives, and the changed turn set flows through
codexPriorityTurnKeys/changedPriorityTurnIDs, which already re-derives only the session files whose cachedcodexTurnIDsintersect the changed turns (cachedCodexFileNeedsPriorityRescan). Files untouched by priority changes keep their cache.A session-parsing change still invalidates the session cache exactly as before.
Runtime Proof (mechanism, live script run)
Editing a priority-turns source and regenerating: the sessions hash is byte-identical, only the priority domain moves — and reverting restores it:
Under the previous single-domain hash, that same edit changed the global
valueand would have producer-key-invalidated the session cost cache (observable in this repo's history: everyCodexParserHashbump in #1404's commits was a forced full re-parse for all users).Migration cost
One-time: this PR changes the session producer key format itself, so the release that ships it triggers one final full re-parse — after which priority-only parser changes stop causing them.
Validation
swift test --filter 'CostUsageCacheTests|CostUsageScannerCodexPriority'— all pass, including the updated domain-key test assertingcodex:cu:p<sessions>/codex:pt:p<priorityTurns>.Scripts/regenerate-codex-parser-hash.sh check— passes; check mode validates both domains.make check— clean.No CHANGELOG edit per current review guidance (release-owned).