perf: cache Context Breakdown parses per file instead of per scan#69
Merged
Conversation
The Codex Context Breakdown cache keyed on the GLOBAL maximum session-file mtime, so appending to whichever session is currently active invalidated the aggregate for every unchanged historical file and forced a full rescan. The reporter measured 2.19s cold against 2,066 files, and 9.1s under concurrent dashboard load, for what was a single-line append. Adds a per-file parse cache consulted when the aggregate cache misses, so that miss now costs one file rather than all of them. The aggregate cache is deliberately kept as the fast path: replacing it outright would turn every no-change request into "stat every file and recombine", regressing the case that matters most under dashboard load. `top` is deliberately absent from the per-file key. Truncation to the top N happens at merge time, so one cached parse serves every `top` value; a test pins this so it does not get "fixed" into the key later. File identity reuses the predicate rollout.js already relies on in 19 places — inode, size, mtimeMs, plus a sha1 of the first 256 bytes to catch a rotator that reuses the inode. Moved to src/lib/file-identity.js and imported back into rollout.js under the same name, so those 19 call sites are untouched. The comparison is equality, never ordering: a clock moving backwards or a restored mtime must read as changed. Verified the merge helpers build fresh targets and only read from their source rows, so a cached parse cannot be mutated by a later merge — the aliasing hazard that per-file caching would otherwise introduce. Tests were negative-controlled: with the cache lookup disabled, the two behavioral tests fail and the three correctness tests still pass, which is the correct split. Refs #62
Same fix as the Codex side, but this one had a real obstacle: the scan kept a single `seenHashes` dedup Set shared across every file, so whether a message in one file counted depended on what another file had already contributed. Summing independent per-file results would double count every cross-file duplicate — and #62's own acceptance criterion is that the aggregate stays identical to a clean full scan. categorizeSessionFile now parses into its own accumulators and returns them, with a file-local dedup set plus the list of hashes it claimed. The caller merges in `files` order and inserts every hash into one global Set; on any collision it discards the merge and re-runs a full sequential scan with a shared dedup set, which is exactly the old behavior. A scan of 6,529 real session files found zero cross-file duplicates, so the guard is expected to be dead code — it exists so correctness does not depend on that measurement being true of everyone's corpus. A test forces the collision and asserts the duplicate is counted once. Merging in `files` order rather than worker-completion order also makes the result deterministic, which the old shared-accumulator scan was not. Measured on a copy of 800 real session files (108 MB): cold scan 292 ms -> 347 ms no change (L1) 2 ms -> 2 ms after one append 274 ms -> 16 ms Cold is ~19% slower — one extra stat and 256-byte head read per file, plus building per-file structures. That is the trade: the append path, which is what #62 reports and what the dashboard actually hits while a session is live, gets 17x faster. Verified the refactor changes no real-world number: the full breakdown over ~6,500 local session files is byte-identical to main across a fixed date range. An earlier comparison appeared to differ, but that was the live corpus being appended to between the two runs, not the code. Parse instrumentation is opt-in — left always-on it would grow one Map entry per session file forever on a long-running server, purely to serve tests. Fixes #62
The fact file records evidence as file:line, so merging #70 shifted the line numbers this branch's snapshot was pinned to and docs:openwiki:check failed on a gate unrelated to this branch's change. Regenerated; no source change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #62
The bug
Both Context Breakdown implementations put the global maximum session-file mtime in their cache key, so appending to whichever session is currently active invalidated the cached aggregate for every unchanged historical file and forced a full rescan. The reporter measured 1.66 s / 2.19 s cold against 2,066 files, and 7.5 s / 9.1 s under concurrent dashboard load — for a single-line append.
src/lib/claude-categorizer.js:647src/lib/codex-context-breakdown.js:175Measured effect
On a copy of 800 real session files (108 MB) — copied so the benchmark never touches
~/.claude:Cold is ~19% slower and that is disclosed deliberately: one extra
statplus a 256-byte head read per file, plus building per-file structures. The trade is that the append path — what #62 reports, and what the dashboard hits whenever a session is live — gets 17× faster.Design
Two layers. The aggregate cache is kept as the fast path; replacing it outright would turn every no-change request into "stat every file and recombine", regressing the case that matters most under load. The new per-file cache only decides how expensive a miss is.
The hard part:
seenHashesThe Claude scan kept one dedup Set shared across every file, so whether a message counted depended on what another file had already contributed. Summing independent per-file results would double count every cross-file duplicate — and #62's own acceptance criterion is that the aggregate stays identical to a clean full scan.
categorizeSessionFilenow parses into its own accumulators and returns them, together with the hashes it claimed. The caller merges infilesorder and inserts every hash into one global Set; on any collision it discards the merge and re-runs a full sequential scan with a shared dedup set — exactly the old behavior.A scan of 6,529 real session files found zero cross-file duplicates, so the guard is expected to be dead code. It exists so correctness does not rest on that measurement holding for everyone's corpus. A test forces a collision and asserts the duplicate is counted once.
Merging in
filesorder rather than worker-completion order also makes the result deterministic, which the old shared-accumulator scan was not.File identity
Reuses the predicate
rollout.jsalready relies on in 19 places — inode, size, mtimeMs, plus a sha1 of the first 256 bytes to catch a rotator that reuses the inode. Moved tosrc/lib/file-identity.jsand imported back intorollout.jsunder the same name, so those 19 call sites are untouched. The comparison is equality, never ordering: a clock moving backwards or a restored mtime must read as changed.topis deliberately absent from the Codex per-file key — truncation happens at merge time, so one cached parse serves everytop. A test pins this so it does not get "fixed" into the key later.Test plan
claude-category-per-file-cache5/5,codex-context-per-file-cache5/5mainacross a fixed date range. An earlier comparison appeared to differ; that was the live corpus being appended to between runs, not the code — confirmed by re-running both revisions against a range that had stopped changingclaude-categorizer10/10 andcodex-context-breakdown6/6 pass unmodifiednpm test791/795; failing names diffed againstmainand byte-identicalDeliberately out of scope
systemPrefixSeenand the intra-file dedup state for no measurable gain.Pre-existing CI failure
ci:localwill be red for the same 4parseKiroCliIncrementalfailures that are red onmain(see #65). This PR does not touch that path.