Skip to content

fix(usage): stream JSONL instead of readFileSync+split in aggregateUsage - #83

Open
pitimon wants to merge 1 commit into
mainfrom
fix/usage-parser-streaming
Open

fix(usage): stream JSONL instead of readFileSync+split in aggregateUsage#83
pitimon wants to merge 1 commit into
mainfrom
fix/usage-parser-streaming

Conversation

@pitimon

@pitimon pitimon commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Outcome

aggregateUsage() read the entire local transcript corpus into memory every
605 s. On this machine that is 4717 files / 1.51 GB, read with
readFileSync(f,"utf-8") and then content.split("\n") -- the whole-file
string and the array of lines both live at once -- fully synchronously
(aggregateUsage contains zero await, so the event loop is blocked 2-4 s per
scan with no yield point for GC).

This is the confirmed mechanism behind two macOS kernel panics on a 16 GB M4
Mac mini (2026-07-14 and 2026-08-02), attributed by poll-cadence fingerprinting
in the unified log and by argv[0] invocation form in 20 days of RSS history.

Chain: sync/sync-poller.ts:418 -> usage/usage-sync.ts:49 ->
usage/jsonl-parser.ts:176. Present since f756eb0, in every version
2.10.2 -> 2.14.1.

Change

readLines() -- a generator over openSync/readSync/closeSync with
TextDecoder({stream:true, ignoreBOM:true}). Live bytes are bounded by the
chunk buffer (64 KiB default) plus the longest single line, rather than by file
size.

Where the memory went -- the ceiling MOVED, it did not vanish

Peak RSS, same corpus, fresh single-scan process: ~795 MB -> ~276 MB.

The remaining top term is the seen and agg Maps, which scale with the scan
window. MEMFORGE_USAGE_SCAN_DAYS is validated at sync-poller.ts:90-93 by
Number.isFinite(n) && n > 0 -- no upper bound. A large enough window will
walk this back. Worth a follow-up; not changed here.

Also note the 1.20-1.43 GB observed during the collapse is this peak plus the
long-lived MCP server's own heap, so the fix removes roughly 520 MB of the peak,
not all of it.

Correctness

  • 201 tests pass, 0 fail (34 in jsonl-parser.test.ts).
  • Frozen-corpus A/B: on a pinned 401-file corpus, old and new produce an
    identical result hash (6febca14c5da...) across alternating runs.
  • Two divergences from readFileSync().split() remain, both in the error
    direction, both documented at the readLines docblock: a mid-read failure
    yields a PREFIX where readFileSync would have thrown and yielded nothing;
    and a file above readFileSync's string cap is readable here and was not
    before.

A claim I had to retract during review

An earlier draft of the comment and test docblock asserted that per-chunk UTF-8
decoding would make JSON.parse throw and token totals go silently wrong. That
was wrong and is now corrected in the source.
Sweeping chunk sizes 1..90 on a
Thai-plus-emoji line: 61 sizes produce U+FFFD, 0 parse failures, 0 wrong
totals
-- because bytes >= 0x80 occur only inside JSON string values and 0x0A
is never a UTF-8 continuation byte, so line splitting is untouched. The real
justification for {stream:true} is contract equality with readFileSync. The
only data defect is dedup-key corruption (16 of 90 sizes), and only if
message.id/requestId were non-ASCII -- both are ASCII today, so that is a
guard against a format change, not a bug being fixed.

Hostile-input guard

chunkBytes=0 made readSync return 0 into a zero-length buffer, which
readLines cannot distinguish from EOF -- aggregateUsage returned no rows and
reported no error. assertChunkBytes now rejects it. Discrimination measured on
the test file: with the guard replaced by a bare Math.floor, 28 pass /
6 fail
; with it, 34 / 0.

The NUL bytes (source-encoding only, no runtime change)

jsonl-parser.ts held 4 raw NUL bytes: 2 real Map-key separators (offsets
7260, 7365) and 2 inside the comment describing them (5708, 5745). Git sniffs
the first 8000 bytes, so it was the comment's NUL that classified the file
binary and suppressed its diff entirely -- including from the repo's own audit
greps, which returned 0 matches as a false negative.

All four are now \0 escapes. The apparent "space -> NUL" in the diff is a
rendering artifact of the previous binary classification, not a content change.

.gitattributes sets diff only; text was dropped after confirming in a
scratch repo that it yields a byte-identical diff here, since EOL normalisation
is a standing policy rather than part of this fix.

Note on the NUL bytes (resolved via #84)

jsonl-parser.ts used to hold 4 raw NUL bytes -- 2 real Map-key separators
(offsets 7260, 7365) and 2 inside the comment describing them (5708, 5745). Git
sniffs the first 8000 bytes, so it was the comment's NUL that classified the
file binary and suppressed its diff entirely, including from the repo's own
audit greps, which returned 0 matches as a false negative.

.gitattributes does not fix that on GitHub: it sniffs the diff's
pre-image and ignores the head tree's attributes (measured -- the commits
API reported no patch even with .gitattributes in the same tree). The escape
therefore had to land on main first, which is #84. This branch is rebased on
top of it, so the diff below renders normally.

.gitattributes is still added here, as a guard against the file being
content-sniffed into binary again. It sets diff only; text was dropped after
checking in a scratch repo that it yields a byte-identical diff here, since EOL
normalisation is a standing policy and not part of this fix.

Deliberately out of scope

  • Cold-start scan. lastUsagePush = 0 (sync-poller.ts:170) means the
    10-minute throttle never short-circuits the first poll, so every new session
    triggers a full corpus scan ~2 s after start, with no cross-process
    coordination.
  • mtime prefilter. The runtime window is 30 days, not 7, so only ~24 % of
    the corpus is outside it. Streaming, not mtime, was the real bound.
  • Partial-scan undercount. Pre-existing: a file that fails mid-read
    contributes a prefix, and the resulting undercount is POSTed to an endpoint
    with replace/UPSERT semantics by (date, model), overwriting correct server
    data rather than merging. filesScanned reports enumeration count and
    linesSkipped is unconsumed end-to-end. Marked NOT TRACKED YET in the
    source -- no issue is filed as of this commit.

Test plan

  • bun test -- 201 pass / 0 fail
  • tsc --noEmit -- 1 pre-existing error in src/mcp/mcp-server.ts:84
    (unrelated file, unchanged here)
  • Frozen-corpus byte-identity, alternating A/B, 4 runs
  • Both coding-style.md audit greps on the diff -- 0 matches, run locally
    where the diff renders as text (they return 0 as a false negative on a
    binary-suppressed diff -- see the section above)
  • Rebased onto chore(usage): escape the raw NUL bytes in jsonl-parser.ts as \0 #84; tree byte-identical to the pre-rebase revision
    (git diff 914372a HEAD empty), 201 pass / 0 fail after the rebase
  • All three files render as TEXT in the Files-changed view (verified via the
    PR files API and the .diff endpoint)
  • Soak: confirm peak RSS in the live MCP server over a full day

aggregateUsage() read every transcript with readFileSync(f,"utf-8") then
content.split("\n") -- the whole-file string and the array of lines both live
at once -- over 4717 files / 1.51 GB on this machine, every 605 s
(sync-poller.ts:418), fully synchronously. The date filter sits at :213, i.e.
AFTER the read, so narrowing MEMFORGE_USAGE_SCAN_DAYS bounded the POST payload
but never the I/O. Present since f756eb0 (2.10.2 -> 2.14.1).

Replaced with a readLines() generator over openSync/readSync/closeSync plus
TextDecoder({stream:true, ignoreBOM:true}). Live bytes are bounded by the chunk
buffer (64 KiB default) plus the longest single line, rather than by file size.

Measured, same corpus, fresh single-scan process: peak RSS ~795 MB -> ~276 MB.
The ceiling MOVED rather than vanished -- the top term is now the `seen` and
`agg` Maps, which scale with the scan window (MEMFORGE_USAGE_SCAN_DAYS,
validated at sync-poller.ts:90-93 by `Number.isFinite(n) && n > 0`, with no
upper bound).

Output equality: on a frozen 401-file corpus, old and new produced an identical
result hash (6febca14c5da...) across alternating A/B runs. Two divergences
remain, both in the error direction and both written up at the readLines
docblock: a mid-read failure yields a PREFIX where readFileSync would have
thrown and yielded nothing, and a file above readFileSync's string cap is
readable here and was not before.

Added assertChunkBytes. chunkBytes=0 made readSync return 0 into a zero-length
buffer, which readLines cannot distinguish from EOF, so aggregateUsage returned
no rows and reported no error. Discrimination measured on the test file: with
the guard replaced by a bare Math.floor, 28 pass / 6 fail; with it, 34 / 0.

.gitattributes sets `diff` for source files so this file cannot be
content-sniffed into binary again. `text` was dropped after checking in a
scratch repo that it yields a byte-identical diff here, since EOL normalisation
is a standing policy and not part of this fix. The raw NUL bytes that caused the
original misclassification were escaped in the preceding commit, separately and
on purpose -- see it for why.

Not addressed here: the cold-start scan (lastUsagePush = 0 at sync-poller.ts:170
means the 10-minute throttle never short-circuits the first poll, so every new
session triggers a full scan ~2 s after start), and the pre-existing gap where a
partial scan pushes an undercount to a replace-semantics endpoint -- see the
NOT TRACKED YET note at the aggregateUsage caller comment.

201 tests pass, 0 fail.
@pitimon
pitimon force-pushed the fix/usage-parser-streaming branch from aaef2e2 to 6162150 Compare August 3, 2026 08:28
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.

1 participant