Reduce CPU and memory use when reading large spool envelopes

[philcunliffe]

Large spool envelopes were repeatedly flattened and searched as each 64 KiB chunk arrived. Both the flush reader and provisional reader now retain fragments and join them once per completed line, keeping scan work linear while preserving hashes, UTF-8 replay offsets, and trailing-envelope behavior.

On a synthetic 16 MiB envelope (1,024 rows), medians of three fresh Node v24.2.0 processes on macOS arm64 showed:

Path	CPU before / after	Peak RSS before / after
Flush	251 / 65 ms (-74%)	302 / 172 MiB (-43%)
Provisional read	217 / 34 ms (-84%)	290 / 132 MiB (-54%)

These numbers, their conditions, and the commands that reproduce them live in the header of benchmarks/spool-performance.mjs, which takes an optional baseline-checkout argument so a candidate and its baseline are measured on one fixture in one Node version. They are path-specific synthetic measurements, not whole-daemon savings. Open PR file lists were checked; this does not duplicate #1075's query-execution work.

Validation: npm test (6,065 pass, 1 skipped, 0 fail), npm run typecheck, the declaration build, and the query_grep_roundtrip smoke pass. The new scan-work regression test fails before the fix, measuring 18,352,789 characters searched for 1,048,884 input characters. Chunk-boundary behavior is pinned separately: a newline landing on the 64 KiB read boundary, and a 4-byte character split across it at every byte phase.

CPU/memory review: no new unbounded cache or busy loop. Each chunk is scanned once; memory remains proportional to the largest individual envelope, whose JSON must still be parsed intact. No dependency, schema, or spool-format changes: the diff touches the two readers only, leaving the envelope shape, the spool file layout, and the persisted progress/cursor format untouched, so the durable_cache_upgrade acceptance procedure is not triggered.

Companion server optimization: https://github.com/hyparam/hypaware-server/pull/438

[philcunliffe]

Verdict: approve with fixes applied. The refactor is correct. I could not find a behavioural difference between the old and new readers on any input I could construct, and I looked hard, because this is a performance rewrite of a data-integrity path. Findings are all low and all in the test/benchmark surface, not in src/. I fixed them on the branch; src/core/cache/ is byte-identical to what you pushed.

Behaviour preservation: verified, not assumed

Differential harness running the pre-PR streaming-reader.js/spool.js beside the new ones over the same fixtures, comparing decorated rows (including _hyp_cache_row_id), columns, resumeOffset, and malformedCount exactly:

• 4,113 randomised comparisons over generated spool files mixing empty lines, unparseable lines, version !== 1 and columns-less envelopes, column-signature switches, ASCII/Latin-1/emoji/ZWJ payloads, no-final-newline and mid-JSON-truncated tails, across four batchRowLimit/batchByteLimit settings, plus a resume-replay loop that re-reads from every emitted resumeOffset. Zero mismatches.
• 15 targeted boundary cases: terminating newline at exactly byte 65536 and at ±1, ±2; a 4-byte character starting at each of the four byte phases around 65536 so one is split mid-sequence; many complete lines inside one chunk; a 300k-character line spanning ~20 chunks with no newline at all; truncated tail; empty file; newlines only. Zero mismatches.

On each of the four points the PR claims:

• Chunk boundaries (src/core/cache/streaming-reader.js:191): correct. if (start < text.length) retains nothing when a line ends exactly on the boundary, and the while loop drains several lines from one chunk with a pending fragment only on the first. Behaviour matched the old reader in every boundary case above.
• Split UTF-8: correct, and structurally so. createReadStream(..., { encoding: 'utf8' }) (streaming-reader.js:78, spool.js:394) means Node's StringDecoder withholds an incomplete multi-byte sequence, so streamSpoolLines never sees a partial code point and never emits a lone surrogate. fragments.join('') reconstructs the exact concatenation of the slices, so Buffer.byteLength(line, 'utf8') + 1 at streaming-reader.js:107 is computed on the same string the old code computed it on. The JSDoc at streaming-reader.js:185 ("The caller's UTF-8 stream decoder owns split multibyte characters") is accurate.
• Hash equivalence: structural. decorateRow hashes the parsed row, and the line handed to JSON.parse is byte-identical, so the digests cannot differ. Confirmed empirically in the differential runs.
• Trailing / truncated envelope: correct. Flush discards the unterminated tail (includeTrailing defaults false, so fragments is dropped at end of stream); provisional read passes true and still parses it best-effort, matching the old yield* rowsFromSpoolLine(tail). The old code called rowsFromSpoolLine('') on an empty tail where the new code yields nothing; that is a no-op, since rowsFromSpoolLine returns immediately on an empty line (spool.js:427).

"No spool-format change" holds. The diff touches only reader loops. The envelope shape, SPOOL_DIR, the progress-file format, cache schema, partition declaration, and compaction output are untouched, and resumeOffset values are identical to the old reader on every fixture. durable_cache_upgrade is not triggered by this release.

The regression test is real. Reverting src/core/cache/{streaming-reader,spool}.js to origin/master in my worktree and re-running the new tests: not ok 12 - large spool envelopes scan each chunk once with newline search revisited 18352789 characters for 1048884 input characters, matching the figure in your report exactly. Restored, it passes. The other new test (trailing envelope) passes on both, which is right: it pins preserved behaviour rather than the fix.

Perf reproduced and exceeded on Linux / Node v22.23.1, old vs new on the same fixture: flush 1366 → 165 ms CPU and 247 → 138 MiB peak RSS; provisional 1299 → 76 ms CPU and 253 → 129 MiB.

The reported pre-existing failure did not reproduce. test/plugins/ai-gateway-absolute-form.test.js passes all 15 here, and the full suite on your head was 6064 pass / 0 fail / 1 skip. The HTTP 421 failure looks environmental, not a property of the tree. Nothing to do, but the PR body's claim is worth softening.

Findings

1. Low - benchmarks/performance-2026-09-04.md cannot be kept true from this repo. Lines 48-51 document ingest/spool.js, ingest/mover.js, and ingest/backpressure.js, which live in hyparam/hypaware-server; line 71 onward is a one-time validation transcript ("The initial sandboxed suite attempts could not bind localhost"), and lines 96-103 snapshot the open-PR list. benchmarks/ on master holds runnable .mjs scripts and no reports, and the repo has no dated document anywhere. CLAUDE.md is explicit that unasked-for docs belong in the PR description. Fixed: removed the file and moved the durable part (the two client measurements and the conditions they were taken under) into the header of benchmarks/spool-performance.mjs, beside the command that reproduces them. Nothing measured was lost; the server half belongs in hypaware-server#438.

2. Low - benchmarks/spool-performance.mjs:34 reports an incomparable peak RSS in silence. global.gc?.() no-ops without --expose-gc, so a plain node benchmarks/spool-performance.mjs flush leaves the fixture-build garbage resident and prints a peak_rss_mib that is not comparable to the recorded figures, in a valid-looking JSON line. Fixed: the record now carries gc: true|false.

3. Low - test/core/streaming-reader.test.js:357 can go inert without a signal. The guard counts scan work by intercepting String.prototype.indexOf, so it measures the splitter only while the splitter uses String#indexOf. A later reader built on Buffer#indexOf, readline, or split would score zero and pass the upper bound vacuously, retiring the O(n²) guard with nothing to say so. Fixed: added a lower bound (every input character is examined for a newline at least once), so a zero score now fails as "no longer measuring the splitter". Verified it fails when the splitter is moved off String#indexOf.

4. Low - the two chunk-boundary cases were not pinned. The differential proved them, but nothing in the committed tests did: the added envelope is padded with a 4-byte character whose run happens to align to the 65536-byte read, and no test places a newline on the boundary. Those are exactly the two ways a fragment-retaining reader corrupts a row. Fixed: added lines that end on, and characters that straddle, the 64 KiB read boundary survive intact, covering the newline at 65536 ± 1 and a 4-byte character at each of the four byte phases, asserting payload round-trip and resume offsets. It passes on the pre-refactor reader too, which is the point.

Style and repo rules

No new runtime dependencies, no new columns/config keys/schema fields, no em dashes, no stray semicolons, no NUL bytes. streamSpoolLines is one shared helper replacing two duplicated loops, which is the right side of "reuse before you add"; the other line-splitters in the tree (sse.js, session_file.js, rollout_session_meta.js) have different semantics and were correctly left alone. No LLP is needed: LLP 0013 holds the spool's on-disk format to be an implementation detail, this changes no behaviour, and CLAUDE.md exempts behaviour-preserving refactors.

Verification of my own changes

npm test 6065 pass / 0 fail / 1 skip, npm run typecheck clean, benchmark runs in both gc modes. git diff 4bd5f100 HEAD -- src/ is empty.

New head: 9bef7e99e3212655c0a27db3ad8f3645ce5bff58.

Reviewed at 4bd5f100 by neutral, with a second independent pass from /code-review, which reached the same conclusion (no correctness bugs; byte-identical differential over its own 12 randomised files and a 20k-case fuzz of streamSpoolLines, plus an fd-leak check on early break out of streamFlushFile: 18 fds before and after on both base and head). Findings 1-3 above are its three; finding 4 is mine.

[philcunliffe]

Verdict: findings (round 2 of 2). 2 low, both fixed and pushed. Nothing in src/. New head after fixes: ebb48cb5.

Findings

1. Low. benchmarks/spool-performance.mjs:28-29: a // @ts-check pragma that nothing checks, on a file that does not check.
Round 1 (a5d52c70) added // @ts-check, matching icebird-real-data.mjs and normal-usage-cpu.mjs. But benchmarks/ is not in tsconfig.json's include (bin, hypaware-plugin-kernel-types.d.ts, hypaware-core, scripts, src, test), so npm run typecheck never compiles it, and under the repo's own compilerOptions the file is red:

benchmarks/spool-performance.mjs(28,42): error TS7036: Dynamic import's specifier must be of type 'string', but here has type 'URL'.
benchmarks/spool-performance.mjs(29,54): error TS7036: (same)

Runtime is unaffected (dynamic import stringifies a URL), but the pragma reads as a clean signal it has not earned. Fixed in ebb48cb5 by passing pathToFileURL(...).href. Verified: both TS7036s gone under the repo's exact compilerOptions, and the benchmark still runs in both modes with gc:true under --expose-gc and gc:false without. Credit to /code-review, which raised this; confirmed independently before adopting. (normal-usage-cpu.mjs carries 6 similar errors, pre-existing on master and out of scope here.)

2. Low. PR description pointed at a file round 1 deleted.
The body said the measurement report lived in benchmarks/performance-2026-09-04.md, removed in a5d52c70, and quoted a stale suite result ("6,060 passed... one existing HTTP 421 test failure"; that test passes at this head). Fixed by editing the body to point at the benchmark script's header and to state the observed run.

Independent re-verification of round 1's correctness claims

Round 1's fixes touched no src/ file, so its correctness claims were re-established from scratch here, not carried over. I built a differential harness running base d42c7946 and this head side by side over both read paths (streamFlushFile and readSpooledRows), comparing every batch's rows, _hyp_cache_row_id hashes, columns, resumeOffset, and malformedCount, plus the full provisional row stream.

241 cases, 0 divergences. Coverage, case by case, against the specific risks:

Case	Result
Newline landing at bytes HWM-2 .. HWM+2, and the same around 2x and 3x HWM	identical to base; resumeOffset exactly the line-end byte
One chunk holding many complete lines (500 envelopes)	identical, at default and batchRowLimit: 7
Chunk with no newline at all (envelope spanning many chunks)	identical
Final line with no trailing newline: truncated mid-JSON, truncated after valid JSON, truncated at 1 byte	identical; flush drops it, provisional read keeps it best-effort
Truncated final envelope torn mid-4-byte-character (cut 1, 2, 3 bytes)	identical
4-byte character split across the chunk boundary at byte phases 0-3, plus 2-byte and 3-byte characters at every phase	identical, payload round-trips exactly
Empty file, only newlines, leading newline, interleaved blank lines	identical
Malformed JSON, version: 2, envelope with no columns	identical, same malformedCount
Column-signature change mid-file (seals a batch)	identical
startOffset resume from every one of 40 line boundaries in a multi-chunk astral-UTF-8 file	identical; each resumeOffset equals the exact cumulative byte length
120 randomized fuzz files (quotes, backslashes, tabs, embedded newlines, astral characters, garbage lines, ~35% with an unterminated tail)	identical

So yes: chunk-boundary, split-UTF-8, no-newline-chunk, trailing-no-newline, hash equivalence, and the truncated final envelope are all independently confirmed byte-identical to base. The durability semantics are preserved in the direction that matters: flush refuses the unterminated tail (it may still be being written) while the provisional reader parses it best-effort, exactly as before.

Also checked directly: no fd leak. 200 early-break iterations across both paths, 0 fd delta. The generator return chain (streamFlushFile -> streamSpoolLines -> for await over the stream) still destroys the read stream.

The durable_cache_upgrade question

The "no spool-format change" claim holds. The diff touches src/core/cache/streaming-reader.js and src/core/cache/spool.js and, within them, only the readers. No envelope writer, no version bump (the two version lines in the diff are relocated, not changed), no label, no cache schema or partition declaration, no generation or cursor format (writeProgress/readProgress are untouched), no maintenance or compaction output. The persisted cursor values are provably unchanged: the resume differential above compares base and head resumeOffset at every line boundary and they agree byte for byte. durable_cache_upgrade is not triggered by this release.

Round 1's deletion of benchmarks/performance-2026-09-04.md

Right call. Roughly half the 103 lines described changes in a different repository (hypaware-server ingest/spool.js, ingest/mover.js, ingest/backpressure.js) and quoted server benchmark numbers this tree can never keep true, plus a one-time open-PR survey. benchmarks/ holds runnable .mjs and no reports, and the repo carries no other dated documents.

The remaining benchmark is self-describing and the numbers are not orphaned. benchmarks/spool-performance.mjs's header carries the fixture (one 16 MiB envelope, 1,024 rows), the baseline SHA (d42c7946), the Node version and platform, the median-of-three method, the --expose-gc caveat with the emitted gc field that records which was used, both reproduction commands, and both before/after numbers, sitting next to the code that produces them. The one gap was the PR body still pointing at the deleted file, fixed above. I reproduced the direction and magnitude on this host (Linux, Node v22.23.1, median of 3, so not comparable to the header's macOS/Node 24 figures): flush 1367 -> 162 ms CPU and 246 -> 137 MiB peak RSS; provisional read 1247 -> 78 ms and 245 -> 128 MiB.

Is the O(n^2) guard still meaningful?

Yes, and I proved it can still fail in both directions.

• Upper bound. Copying the head's test file onto base d42c7946 and running it: not ok 12 ... newline search revisited 18352789 characters for 1048884 input characters. The regression genuinely fails without the fix, exactly the figure round 1 reported.
• Lower bound (the vacuity fix). I patched the head's splitter to find newlines with a charCodeAt loop instead of String#indexOf, leaving behavior identical, and re-ran: not ok 12 ... newline search only saw 0 characters, so it is no longer measuring the splitter. The guard detects a splitter that stops using the intercepted method rather than silently scoring zero and passing.
• The bound has structural margin, so it is not brittle: every chunk contributes at least its own length via the start = 0 call, putting the true floor above first.length and well inside the [1x, 3x] window. The other 13 tests in the file pass on base, which is the point: they pin preserved behavior rather than the new implementation.

Base drift

master moved d42c7946 -> 8e0dfadd. The only file changed is llp/0373-boundary-identity-is-the-id-not-the-snapshot.rfc.md. Nothing under src/core/cache/ or src/core/sinks/ moved, git merge-tree reports no conflict, and GitHub reports MERGEABLE / CLEAN. No rebase needed.

Repo rules

No new runtime dependencies (package.json and package-lock.json untouched). No trailing semicolons, no em dashes, no raw NUL bytes in the changed files. Types stay in JSDoc. streamSpoolLines is a genuine extraction shared by the two readers rather than a new abstraction beside them, so "reuse before you add" is satisfied. No LLP is required: this is a behavior-preserving performance refactor, not a design decision.

Gates at the pushed head

npm test: 6,066 tests, 6,065 pass, 0 fail, 1 skipped. npm run typecheck: clean. Benchmark runs in both modes.

/code-review (high) returned within the window and reported the same picture: src/ clean, one new low finding, the one adopted above. Its finding was reproduced against the code before adoption.

[philcunliffe]

Ship risk: low

Who could be affected: Anyone whose AI session history is being recorded on their own machine, particularly when a single recorded turn is very large.

What could happen: No user-visible change is expected. This only makes the step that files recorded sessions into local history faster and lighter on memory when a very large item comes through; what gets saved, and what searches return, stay the same.

The one thing worth guarding against was silent history damage: if the reader lost its place, some recorded turns could quietly go missing or appear twice, and long messages could come back garbled. That was checked directly.

Why this level: Nothing about how recordings are stored, read back, or searched changes, and nothing leaves the machine. Everything stays local, and there is no change to privacy, access, or anything that cannot be undone.

What was checked: The updated reader was run side by side with the previous one over hundreds of generated recordings, including very large ones, emoji and accented text, damaged entries, and half-written final entries. Both produced exactly the same results every time. Deliberately broken versions of the new reader were also run, and the check caught each one. The full test suite (6,065 tests), the type check, and a search-and-read end-to-end run all passed.