perf(coding-agent): stream session HTML export — 1.9x lower peak memory, byte-identical output - #898
Merged
Merged
Conversation
… in memory Exporting held the snapshot graph plus its full JSON string, a Buffer copy, the base64 string and the assembled HTML document all at once: an 80MiB transcript peaked at 1007MiB resident (12.6x its size). The template is now split at the session-data marker and the snapshot streams through an incremental serializer into a byte-aligned base64 sink written straight to the output file via temp file and rename. Same session now peaks at 532MiB with byte-identical output, verified against JSON.stringify across surrogate pairs, non-finite numbers, array holes, dropped undefined/function/symbol members and key order, and by the existing redaction round-trip test.
|
Warning Review limit reachedNext included review available in 14 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…wner The atomic-write source lock caught the streamed exporter hand-rolling temp-file + rename; atomicWriteFileWith owns that pattern.
3 tasks
`jsonPieces` matched `JSON.stringify` on every value shape the fixtures
carried, and diverged on two it did not. A boxed primitive fell through to the
own-property walk, so a `String` box serialized as its index map and a `Number`
box as `{}`. A reference cycle recursed forever; because the caller streams
each piece to the output file as it arrives, that is an unbounded write rather
than the `TypeError` the builtin raises, and no assertion on a value can see
it.
Both are closed at the serializer: a box unwraps to the primitive it holds,
and an ancestor set raises before the piece that would recur. The set is the
path to the current value, not everything seen, so one object shared by two
properties is still emitted twice the way the builtin emits it.
Refs #898
…in does
`jsonPieces` claims byte identity with `JSON.stringify`, and four inputs
broke it. A value carrying `toJSON` was transformed on the way down as well
as at its holder, so a transform returning another holder collapsed to the
inner result where the builtin emits `{}`. An array element's `toJSON` was
called with no key instead of its index. A `toJSON` returning `undefined`
threw a TypeError and emitted invalid JSON. A root that serializes to
nothing yielded a literal `null` document.
`toJSON` is now applied once, at the holder that owns the value, with the
property name or the index as its key; an empty root raises rather than
shipping a page whose payload silently became null.
Refs #898
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
1 participant
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.
What
veyyon --export(and/export) no longer assembles the whole HTML document in memory. Previously the pipeline held five representations of the session at once:SessionData),JSON.stringifystring,Buffer.from(...)copy of that string,Measured on an 80 MiB synthetic transcript: peak RSS 1007 MiB (12.6× the session size).
Now the template is split once at the
{{SESSION_DATA}}marker; the head is written to disk, then an incremental serializer (jsonPieces) feeds byte-aligned base64 chunks straight into the output file through a batched, promise-chained writer; the tail closes it. The file is written to a temp path and renamed on success, so a mid-stream failure leaves no half-written export. Peak for the same session: 532 MiB, with export time unchanged (798 ms vs 825 ms baseline).Byte-exactness is by construction and pinned by tests:
JSON.stringifyitself; keys in insertion order;NaN/Infinity→null, array holes and undefined/function/symbol elements →null, undefined/function/symbol properties dropped — all matching the builtin;Buffer.from(all).toString("base64")across every 3-byte alignment;Why
Exporting a long session should not cost an order of magnitude more memory than the session itself. This also removes the multi-second GC stalls those five whole-payload copies caused on large transcripts.
Testing
packages/coding-agent/test/html-export-streams-byte-identical.test.ts— torture fixtures vsJSON.stringify(surrogate pairs/astral text, escapes,-0/1e21, non-finite numbers, holes, member-dropping rules), base64 carry across alignments, many-small-pieces encode, end-to-end decode.export-html-redacts-secrets,export-subsessions, keyboard-shortcuts and thetest/export/suite all green: 44 pass / 0 fail in the sandbox./proc/<pid>/statusVmHWM high-water mark, 80 MiB / ~21k-entry session):bun run check:ts,bun run check:toolsclean.bun checkpasses