Skip to content

perf: cache Context Breakdown parses per file instead of per scan#69

Merged
pitimon merged 4 commits into
mainfrom
perf/per-file-context-cache
Jul 20, 2026
Merged

perf: cache Context Breakdown parses per file instead of per scan#69
pitimon merged 4 commits into
mainfrom
perf/per-file-context-cache

Conversation

@pitimon

@pitimon pitimon commented Jul 19, 2026

Copy link
Copy Markdown
Owner

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:647
  • src/lib/codex-context-breakdown.js:175

Measured effect

On a copy of 800 real session files (108 MB) — copied so the benchmark never touches ~/.claude:

before after
cold scan 292 ms 347 ms
no change (aggregate-cache hit) 2 ms 2 ms
after one append 274 ms 16 ms

Cold is ~19% slower and that is disclosed deliberately: one extra stat plus 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: seenHashes

The 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.

categorizeSessionFile now parses into its own accumulators and returns them, together with the 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 — 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 files order rather than worker-completion order also makes the result deterministic, which the old shared-accumulator scan was not.

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.

top is deliberately absent from the Codex per-file key — truncation happens at merge time, so one cached parse serves every top. A test pins this so it does not get "fixed" into the key later.

Test plan

  • claude-category-per-file-cache 5/5, codex-context-per-file-cache 5/5
  • Negative-controlled: with the cache lookup disabled, the behavioral tests fail and the correctness tests still pass — the correct split
  • Real-corpus equivalence: full breakdown over ~6,500 local session files is byte-identical to main across 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 changing
  • Existing claude-categorizer 10/10 and codex-context-breakdown 6/6 pass unmodified
  • Full npm test 791/795; failing names diffed against main and byte-identical

Deliberately out of scope

  • No disk layer for the per-file cache. The existing aggregate disk cache still serves restarts; adding a second on-disk format is scope creep for a fix that already lands the win.
  • No byte-offset resume within a changed file. Re-parsing one changed file is a few ms; resuming mid-file would have to reconstruct systemPrefixSeen and the intra-file dedup state for no measurable gain.

Pre-existing CI failure

ci:local will be red for the same 4 parseKiroCliIncremental failures that are red on main (see #65). This PR does not touch that path.

itarun.p and others added 4 commits July 20, 2026 06:37
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.
@pitimon
pitimon merged commit 133a09e into main Jul 20, 2026
1 check passed
@pitimon
pitimon deleted the perf/per-file-context-cache branch July 20, 2026 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Context Breakdown cache invalidates on every active session write, causing multi-second dashboard stalls

1 participant