perf(core): cut per-tick and per-record transient allocation in shell streaming and recording (Fixes #3432)

[acoliver]

TLDR

Cuts constant transient allocation in the shell streaming render path and halves per-record retention in the session recorder, with byte-identical observable behavior:

• maybeEmitRenderedOutput no longer runs two full JSON.stringify passes per 16 ms tick. It now uses outputsEqual, a staged allocation-free structural comparison that emits exactly when the old serialization comparison would have emitted.
• serializeTerminalToObject reuses two scratch Cell instances per call instead of allocating one Cell per terminal cell (rows × cols per tick), and can serialize colorless output directly via the new optional SerializeTerminalOptions.colorless flag, eliminating the per-token {...token, fg: '', bg: ''} strip copy in serializeTerminalForRender.
• SessionRecordingService pending records retain only {json, bytes}; the pre-content enqueue path returns the SessionRecordLine it built, so payload object graphs are no longer pinned until the async drain (~2x → 1x payload retention under a slow disk).

Fixes #3432

Dive Deeper

Why the emit check is exact (criterion 1). The old behavior emitted iff JSON.stringify(finalOutput) !== JSON.stringify(outputRef.current). Every AnsiToken is built by buildTokenFromCell with a fixed key order and primitive-only fields, and state.output has a single writer (the emit callback itself), so structural per-field equality is equivalent to JSON.stringify equality for every reachable state. outputsEqual stages the check cheaply: reference identity → null/string → line count → per-line token count → per-line text lengths → per-token fields. Notably, raw cursor position is deliberately not an emit trigger: the cursor affects serialization only through the token inverse flag (and run splitting) at the cursor cell, so consulting the raw cursor would over-emit relative to the old behavior. This was verified by a 400-round randomized fuzz comparing the new decision against the JSON.stringify-inequality spec on 1,200 transitions (0 mismatches; cursor-only transitions 113/113), plus the pinned 15×15 exactness matrix in shellPtyHelpers.bun.test.ts.

Serializer equivalence (criterion 1). The scratch-Cell refactor preserves the original algorithm quirks exactly: the x === 0 unconditional run start, the per-line null-cell seed (fg = 0, not -1), equals() semantics (attributes + colors + cursor flag), getChars() empty-string→space normalization, wide/wrapped-character handling, and blank-line → [] replacement. Fuzz-verified against a faithful reproduction of the HEAD algorithm and against the legacy two-step colorless strip (800 checks, 0 mismatches). The colorless path also drops the structurally-dead Array.isArray filter — serializer output is AnsiLine[] by construction.

Recorder (criterion 3). PendingRecord drops the line field; drain, byte accounting, queue byte-limit enforcement, high-water reporting, prepareContentBatch publish/rollback/finalize, and failure paths are untouched. On-disk JSONL bytes and ordering are provably unchanged (pinned by tests).

Allocation evidence (criterion 2). Per no-change render tick, before → after: 1,920 Cell objects → 2; one strip-copy object per emitted token → 0; two full JSON.stringify strings → 0. Measured with a fresh-process harness (3 processes per side, 7 rounds each, fixed 80×24 terminal with mixed plain/palette/RGB/256-color/bold/dim content): median wall time per tick dropped ~15% (0.0981/0.0954/0.0982 → 0.0833/0.0799/0.0822 ms). Direct allocated-bytes measurement is not obtainable on Bun 1.3.14 (heapStats().objectCount cannot see eden allocations — control-verified; the sampling profiler returns empty traces for sub-second synchronous workloads), documented in project-plans/issue3432/PLAN.md.

Scope. No buffer bounds, queue watermarks, scrollback, retention, or render-budget changes (#3428/#854 remain open follow-ups). The only API addition is the optional serializer options parameter.

Reviewer Test Plan

# focused suites (84 tests)
bun test packages/core/src/utils/terminalSerializer.test.ts \
  packages/core/src/services/shellPtyHelpers.bun.test.ts \
  packages/core/src/recording/SessionRecordingService.test.ts \
  packages/core/src/recording/SessionRecordingService.payloads.test.ts \
  packages/core/src/recording/SessionRecordingService.bounds.test.ts

# full verification cycle
npm run test && npm run lint && npm run typecheck && npm run format && npm run build

• The exactness matrix in shellPtyHelpers.bun.test.ts (15 mutated states × emit/no-emit vs the JSON.stringify spec) is the fastest way to check criterion 1 by hand.
• The WeakRef retention test in SessionRecordingService.test.ts shows payload objects becoming collectable while their record is still pending, then appearing on disk after dispose.
• Running any streaming shell command (make, npm install, a progress-bar script) with debug logging off exercises the render loop end to end.

Testing Matrix

	🍏	🪟	🐧
npm run	✅	❓	❓
npx	❓	❓	❓
Docker	❓	❓	❓
Podman	❓	-	-
Seatbelt	❓	-	-

macOS (arm64): full npm run test suite green (one zed-acp file initially failed with a transient ENOENT from a dist rebuild racing the suite; re-run clean 33/33), npm run lint green on all touched packages (one pre-existing, unrelated max-lines failure in packages/providers/src/openai/OpenAIStreamProcessor.ts exists on main), npm run typecheck/format/build green, smoke profile zai-glm-flash green.

Linked issues / bugs

Fixes #3432
Related: #3426 (profiling baseline), #3428 and #854 (out-of-scope UI-memory follow-ups)

Summary by CodeRabbit

• Bug Fixes
  • Improved terminal rendering updates so changes in text, styling, colors, and layout are reflected accurately without unnecessary redraws.
  • Preserved terminal output formatting when colors are omitted, including text styles, cursor state, and wide characters.
  • Improved session recording reliability by maintaining event ordering and sequential metadata when records are flushed.
  • Prevented pending session data from unnecessarily retaining large payloads before content is recorded.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 7726608d-c783-4c0f-b582-7a03529837c2

📥 Commits

Reviewing files that changed from the base of the PR and between 6dedf12 and 1a3599e.

📒 Files selected for processing (1)

• packages/providers/src/openai/OpenAIStreamProcessor.ts

Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

The change reduces transient allocations in terminal serialization and session recording. It adds structural rendered-output comparison, reusable scratch cells, colorless serialization support, pending-record retention changes, and tests for rendering, serialization, recording, and parser behavior.

Changes

Allocation reduction

Layer / File(s)	Summary
Reusable terminal serialization packages/core/src/utils/terminalSerializer.ts, packages/core/src/utils/terminalSerializer.test.ts	Cell instances are reused during serialization. The serializer supports colorless output. Tests cover color handling, cursor state, styles, wide characters, and repeated serialization.
Rendered output comparison packages/core/src/services/shellPtyHelpers.ts, packages/core/src/services/shellPtyHelpers.bun.test.ts	Rendered outputs are compared structurally instead of through two JSON.stringify calls. Tests cover changed fields, unchanged outputs, legacy values, and emission decisions.
Session recording retention packages/core/src/recording/SessionRecordingService.ts, packages/core/src/recording/SessionRecordingService.test.ts	Pending records retain serialized JSON and byte counts instead of the original event line. Tests verify payload release and unchanged JSONL ordering and sequencing.
Parser cleanup packages/providers/src/openai/OpenAIStreamProcessor.ts	parseBufferText passes sanitized text directly to the parser and removes a redundant intermediate variable.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 1a359

The changed paths preserve rendered output, recording behavior, and parser input, so the PR is ready to merge.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The shell, terminal serializer, recording, and related tests support #3432. The change to packages/providers/src/openai/OpenAIStreamProcessor.ts repairs an unrelated max-lines lint regression trac…	Remove the unrelated OpenAIStreamProcessor.ts lint repair from this pull request, or move it to a separate pull request for #3699.
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 7 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description is complete and directly matches the template. It includes the TLDR, technical details, reviewer test plan, testing matrix, linked issues, measured results, scope boundaries, and valid…
Title check	✅ Passed	The title clearly identifies the primary change: reducing per-tick and per-record transient allocations in shell streaming and session recording. It also includes the related issue reference.
Linked Issues check	✅ Passed	The changes satisfy the coding requirements in #3432. outputsEqual replaces the double JSON.stringify comparison, and tests compare its emit decisions with serialization-based decisions. Reusable …

Full details: Out of Scope Changes check

Explanation

The shell, terminal serializer, recording, and related tests support #3432. The change to packages/providers/src/openai/OpenAIStreamProcessor.ts repairs an unrelated max-lines lint regression tracked by #3699 and does not support shell streaming or recording allocation work. This is a demonstrated out-of-scope change.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue3432

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

OpenCodeReview — PR #3697

• Reviewed head SHA: 1a3599ef6dd6b55a3d930422a583373870d8d4d5
• Merge base: 5bedbd2385b8c4767e97c43d52b563d11e3b03db
• Range: full from 5bedbd2385b8c4767e97c43d52b563d11e3b03db
• Range fallback: checkpoint-missing
• Scope: selected 8 file(s), +930/-34; cumulative 8 file(s), +930/-34
• Tokens: 0 total (0 input, 0 output, 0 cache)
• OCR version: open-code-review v1.8.4 (e78474478) linux/amd64 built at: 2026-08-01T03:27:37Z https://github.com/alibaba/open-code-review
• Phase: llm-preflight
• Exit code: 1
• Run: https://github.com/vybestack/llxprt-code/actions/runs/35135149391
• OCR failed to run or parse output.
• Artifacts: ocr-review-output contains raw JSON, stdout, stderr, preview, phase, and exit-code diagnostics.
• Infrastructure diagnostic: phase=llm-preflight; reason=OCR LLM connectivity check failed (model=zai-5.3-flash)

OCR stderr excerpt

Set llm.extra_body = {"thinking": {"type": "disabled"}}
Set language = English
model=zai-5.3-flash
provider-url=configured
Error: llm request failed: POST "[REDACTED]/v1/messages": 401 Unauthorized {"error":{"message":"token expired or incorrect","type":"401"}}

OCR preflight excerpt

model=zai-5.3-flash
provider-url=configured
Error: llm request failed: POST "[REDACTED]/v1/messages": 401 Unauthorized {"error":{"message":"token expired or incorrect","type":"401"}}

OCR preview stderr excerpt

(empty)

[github-actions]

Walkthrough

This PR changes 8 file(s).

• packages/providers/src/openai/OpenAIStreamProcessor.ts: In parseBufferText, removes the intermediate parsingText binding: sanitizeProviderText(workingText) is now assigned directly to let cleanedText, and that same variable is passed to deps.textToolParser.parse(). This drops one redundant string variable per buffered-text parse, reducing transient allocations in the streaming hot path. No parsing behavior, control flow, or outputs change—parsed tool calls and cleaned text are returned exactly as before.
• packages/core/src/recording/SessionRecordingService.ts: PendingRecord no longer retains the live SessionRecordLine; only the serialized json string and byte count are kept, so the payload object graph is not pinned until drain (issue Cut per-tick and per-record transient allocation in the shell streaming and recording path #3432). toPendingRecord drops the line field, and bufferPreContent now builds and returns the SessionRecordLine so enqueue can return it directly instead of reading .line back from the pre-content buffer. Byte accounting, high-water reporting, and enqueue's SessionRecordLine | null contract are unchanged; the change cuts per-record transient allocation in pre-content buffering.
• project-plans/issue3432/PLAN.md: Adds a new project plan document (PLAN-20260916-PTYRENDERALLOC) for issue Cut per-tick and per-record transient allocation in the shell streaming and recording path #3432. It defines scope for three performance changes—allocation-free render change check in shellPtyHelpers, colorless serializer mode with scratch Cell reuse in terminalSerializer, and single-representation pending records in SessionRecordingService—plus shaped acceptance criteria, test-first bun:test cases, and an allocation-evidence harness protocol. A Progress section records TDD RED→GREEN results (84/84 focused tests), ~15% median per-tick wall-time improvement, full verification (lint/typecheck/format/build/smoke), an abandoned direct-allocation measurement, and an independent reviewer APPROVE with a 400-round randomized equivalence fuzz.
• packages/core/src/utils/terminalSerializer.ts: Cuts transient allocations in terminal serialization (issue Cut per-tick and per-record transient allocation in the shell streaming and recording path #3432). Cell is reworked from an immutable per-cell object into a reusable scratch cell: readonly constructor fields become mutable defaults, and a new load() method resets attributes/colors and re-extracts state from an IBufferCell. serializeTerminalToObject now allocates two scratch cells per call and ping-pongs between them while scanning columns, keeping lastCell valid. Also adds a SerializeTerminalOptions interface with a colorless flag; buildTokenFromCell takes colorless and emits empty fg/bg tokens without color conversion, replacing per-token stripping copies with direct colorless output.
• packages/core/src/services/shellPtyHelpers.ts: Cuts per-tick transient allocation in PTY output emission. maybeEmitRenderedOutput no longer JSON.stringify's both the previous and next outputs; it calls a new private outputsEqual() that does staged structural comparison: reference identity, previous-shape guards, line count, per-line token counts, per-token text length, then text/flags/colors. Documented as JSON-equality-equivalent given fixed token key order; cursor position is intentionally not compared directly (it surfaces only via the token inverse flag), preserving prior string-compare semantics. serializeTerminalForRender drops the clone-and-blank-fg/bg token mapping, instead calling serializeTerminalToObject(terminal, { colorless: true }).
• packages/core/src/utils/terminalSerializer.test.ts: Adds a test-only suite for serializeTerminalToObject's new colorless option (issue Cut per-tick and per-record transient allocation in the shell streaming and recording path #3432). Imports type AnsiOutput, defines legacyColorlessStrip as the two-step specification, and builds a rich terminal exercising palette/RGB/256-color, flags, blanks, cursor positioning, and wide chars. Asserts colorless output deep-equals the legacy strip, preserves attributes with empty fg/bg, is stable across interleaved serialization, doesn't mutate colored mode, keeps inverse on first cells, and retains wide-character padding.
• packages/core/src/services/shellPtyHelpers.bun.test.ts: Adds a new Bun test suite for maybeEmitRenderedOutput in shellPtyHelpers, tied to issue Cut per-tick and per-record transient allocation in the shell streaming and recording path #3432. Tests verify emission on null/legacy-string previous outputs, suppression for identical references and equal-value copies, and emission on text, foreground/background color, inverse-flag, line-count, and token-count changes. Confirms cursor movement alone never triggers an emit and that emit decisions exactly match JSON.stringify inequality across an exhaustive pairwise matrix of output variants (text, styles, colors, splits, added/changed lines). Uses a token factory preserving production key order and a runCheck helper capturing emit calls, updated previous-output ref, and emitted chunk.
• packages/core/src/recording/SessionRecordingService.test.ts: Adds a 'Pending-record retention' describe block (issue Cut per-tick and per-record transient allocation in the shell streaming and recording path #3432) with two tests. The first enqueues a session_event payload held only via WeakRef, asserts pending record/byte counts grow, runs Bun.gc(true), and verifies the payload is garbage-collected while the record is still pending, then flushes and confirms the serialized record drains intact to JSONL. The second asserts the on-disk byte format and ordering: lines start with {"v":1,"seq":N,"ts":" with expected type fields for session_start, session_event, and content, and seq values 1-4 in order.

Changes

Layer	File(s)	Summary
providers	packages/providers/src/openai/OpenAIStreamProcessor.ts	Removes a redundant intermediate string in parseBufferText to cut transient allocations in the streaming hot path with no behavior change.
core	packages/core/src/services/shellPtyHelpers.ts, packages/core/src/utils/terminalSerializer.ts, packages/core/src/recording/SessionRecordingService.ts	Cuts per-tick and per-record transient allocations via a structural outputsEqual comparison, a reusable scratch Cell with a colorless serializer mode, and pending records that retain only serialized JSON instead of the live line.
tests	packages/core/src/services/shellPtyHelpers.bun.test.ts, packages/core/src/utils/terminalSerializer.test.ts, packages/core/src/recording/SessionRecordingService.test.ts	Adds bun:test coverage proving the emit-decision equivalence, colorless-serializer equivalence to the legacy strip, and payload GC-ability plus JSONL byte format/ordering.
docs	project-plans/issue3432/PLAN.md	Adds the PLAN-20260916-PTYRENDERALLOC plan for issue #3432 with scope, acceptance criteria, TDD results, allocation evidence, and reviewer approval.

Magnitude

🎯 2 (M)
930 additions, 34 deletions, 8 changed files across 2 packages, 2 acceptance criteria

Related

No related items found.

---

Walkthrough generated by LLxprt PR Review. Planner issue: #2256

[acoliver]

Included repair for the main-level lint regression (#3699)

Commit 1a3599ef6 adds an unrelated-by-necessity one-file fix so this PR (and every other open PR) can go green: #3689's merge landed packages/providers/src/openai/OpenAIStreamProcessor.ts at 801 eslint-counted lines after its own green lint run, breaking the required Lint check repo-wide (also seen on #3698). Because max-lines skips blank and comment lines, the repair removes one counted code line: the single-use parsingText intermediate in parseBufferText is inlined into cleanedText's initializer — the value flows identically into the parser call (2 insertions, 3 deletions, zero behavior change).

Verification for that commit: npx eslint on the file clean (800/800), prettier stable, the 6 covering test files pass (56/56), npm run typecheck pass, and the driver re-ran the full cycle — npm run lint (all 19 groups), format, build, and the full npm run test suite, all exit 0.