perf: rewrite WritableTrackingBuffer as a chunked buffer list

[arthurschreiber]

Problem

WritableTrackingBuffer grows by concatenating everything written so far into a composite buffer on every growth step, and data returns that composite. Every growth is a full copy of the data written so far, Buffer.alloc zero-fills each new chunk, and callers work around it by computing exact sizes up front (e.g. the RPC payload computes the byte length of each parameter name purely to size a buffer for it). writeBuffer always copies, so a large value written through it costs its full size again.

This is the first of a series of changes to the parameter serialization path (a resolve/write type contract and streaming table-valued parameters follow), all of which write into a shared buffer and depend on it not copying on growth.

Change

WritableTrackingBuffer is now a write-side buffer list:

• Bytes are written into an open chunk of at most 8 KB (WritableTrackingBuffer.CHUNK_SIZE). The first chunk is 64 bytes and each sealed chunk is followed by one twice its size, up to the cap; a single write larger than the open chunk gets a chunk of exactly its size. When a write does not fit, the open chunk is sealed into a list and a new one started. There is no concatenation on growth.
• Buffers of 8 KB or more passed to writeBuffer are referenced rather than copied, so large values cost no extra memory. (Such buffers must not be modified until consumed.)
• Strings are encoded in place into the open chunk with buffer.write, as before; there is no intermediate Buffer.from.
• The consumer side mirrors bl's BufferList: length, getBuffers, consume, slice. data stays for the existing callers and is a view rather than a copy when everything fits into a single chunk. The returned buffer must not be modified; the one caller that did (see ALL_HEADERS below) is rewritten.

Cleanups that go with the new storage model:

• No constructor arguments. The initial size was a hint that cannot be wrong under the chunk model and measurably does not matter (256 B to 8 KB starting chunks are within run-to-run noise); the growth flag is meaningless without growth-by-concatenation. The size computations at the call sites are removed.
• No instance-level default encoding. writeString, writeBVarchar and writeUsVarchar take the encoding explicitly. Three call sites in src relied on the default, all ucs2: the RPC procedure name, the RPC parameter names, and the TVP TYPE_INFO. The constructor encodings in the transaction payloads (three ascii, one ucs2) were dead, since every string written there passes ucs2 explicitly. The test call sites now pass what their constructor used to.
• Unused methods removed. writePLPBody and writeMoney had no callers (PLP framing lives in the types, money.ts has its own serializer). writeUsVarbyte has no callers in src and its only test callers pass a Buffer, so its string branch and encoding parameter are gone. copyFrom is replaced by writeBuffer.
• The buffer, position, compositeBuffer, makeRoomFor and newBuffer internals are gone. Nothing outside the class used them (the NTLM payload reset position to zero immediately after construction, a no-op).
• ALL_HEADERS length. writeToTrackingBuffer patched the TotalLength in place through data, relying on it being a live view of the composite. It also wrote the length of everything in the buffer, which only equalled the headers' length because the buffer happened to be fresh. It now writes the constant length up front: 4 (TotalLength) + 18 (the transaction descriptor header), per [MS-TDS] s2.2.5.3.

The class is not exported from src/tedious.ts. It is reachable by deep-importing tedious/lib/tracking-buffer/writable-tracking-buffer, since package.json has no exports map; anyone doing that sees the constructor and method signature changes.

Measurements

Against tedious 20.0.0 on the same machine. New benchmark benchmarks/tracking-buffer/writable-tracking-buffer.js (a WritableTrackingBuffer built from pieces groups of small fixed-width writes, a short UCS-2 string and a 16 byte buffer, then read through data; it exercises the coalescing path, not pass-through):

pieces	20.0.0 (ops/s)	this PR (ops/s)
10	~85-93k	~260-290k
100	~23k	~42-45k
1000	~3.5k	~4.6-4.8k

RPC request serialization (consuming RpcRequestPayload for one request; the payload builds a small tracking buffer per parameter and, for several types, per value):

request	20.0.0 (req/s)	this PR (req/s)
20 params: Int, NVarChar, VarBinary, DateTime, Decimal	~11.4-12.7k	~16.6-17.8k
20 params: BigInt, DateTime2, Time, DateTimeOffset, UniqueIdentifier	~9.5-10.1k	~14.5k
1 param: 10 MB varbinary(max) (benchmarks/request/rpcrequest-payload-varbinary.js n=200)	~7.4-9.6k	~9.8-10.1k

The second table is why the first chunk starts at 64 bytes and data returns a view for single-chunk contents: with a 1 KB first chunk and a copying data, the many tiny per-value buffers made request serialization ~15% slower than 20.0.0 despite the class being 2-3x faster in isolation.

End to end (bulk load, TVP call, varbinary insert against SQL Server 2022) is unchanged within noise; the server round-trip dominates there.

Validation

• Every payload builder that uses the class (RPC requests across 29 parameter types, SQL batch, the four transaction payloads, PRELOGIN, NTLM, bulk load COLMETADATA and DONE, TVP TYPE_INFO) was built on the base and on this branch with pinned randomness and timestamps and compared byte for byte: identical.
• The class was differentially fuzzed against the previous implementation (3,000 random write sequences across every method).
• Unit suite (457 tests, including tests for coalescing, the chunk size bound, pass-through by identity at the 8 KB threshold and copying below it, string encoding against Buffer.from including surrogates and strings larger than the open chunk, data being a view of a single chunk and a copy of several, fixed-width writes across chunk boundaries, consume and slice).
• Full integration suite against SQL Server 2022, all passing except a pre-existing environment-only failure (should not leave any dangling sockets after connection timeout).
• Lint and typecheck clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Fxv5h4UMCGJEpjgCKcAxug

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-02T14:10:13.388839Z	621f106	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9cdd6e3b81

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[arthurschreiber]

CI status on 0979d93 (the rebase onto current master; the branch had been created from a stale local master, which is why no pull_request workflow ran on the earlier pushes):

• SQL Server Linux / Node.js 22.x, 24.x, 26.x failed within ~70 seconds with every integration test reporting Failed to connect to localhost:1433 - Could not connect. The workflow starts SQL Server with docker compose up --detach and runs the integration tests without waiting for the container to accept connections, so a fast runner races it. Nothing in this diff touches connection setup, and the same jobs needed repeated attempts on feat: add support for TABNAME and COLINFO tokens #1770 and fix: wrap all parameter serialization errors in InputError #1772 this week. The proper fix is a readiness wait in nodejs.yml after the compose step; that is a CI change outside this PR's scope, so I'm not adding it here.
• Azure SQL Server / Node.js 22.x (one of the three Azure configurations) failed with 7 ETIMEOUT errors in connection-test.ts, Failed to connect to ***:1433 in 15000ms; the other two Azure configurations passed on the same commit. Same category: the service was unreachable, not a test assertion.

I'll re-run the failed jobs once when the run completes. Locally on this commit the full integration suite passes against SQL Server 2022 (except the environment-only dangling-sockets timeout test), and lint, CodeQL and the passing Azure jobs are green.

---

Generated by Claude Code

[claude]

Reviewed the diff (WritableTrackingBuffer rewrite + call-site cleanup). This is a well-scoped, well-tested change. Notes below are mostly confirmations/nits rather than blockers.

Correctness

• ALL_HEADERS_LEN hard-coding (src/all-headers.ts): replacing the "write 0, patch in place via data" trick with a constant 4 + TXNDESCRIPTOR_HEADER_LEN is safe because txnDescriptor is always the fixed 8-byte buffer from connection.ts (this.transactionDescriptors = [Buffer.from([0,0,0,0,0,0,0,0])]), matching [MS-TDS] §2.2.5.3. This is also a genuine bug fix vs. the old code, which measured data.length of the whole buffer rather than just the headers — it happened to be correct only because callers always wrote headers into a fresh buffer first.
• UCS-2 length math (writeString: value.length * 2 instead of Buffer.byteLength(value, 'ucs2')): these are equivalent for Node's ucs2/utf16le encoding (always 2 bytes per UTF-16 code unit, surrogate-pair-agnostic), and the new test case explicitly covers surrogate pairs and lone surrogates. Looks correct.
• No uninitialized-memory leaks despite Buffer.allocUnsafe: traced through _ensure/_seal/writeBuffer/data/slice — every exposed chunk is sliced to exactly [0, _pos) or is a full write-length copy/reference, so the unwritten tail of an allocUnsafe chunk is never exposed. This is the risky part of switching from Buffer.alloc to Buffer.allocUnsafe, and it's handled correctly.
• Verified all internal src call sites that dropped the default encoding (writeString/writeBVarchar/writeUsVarchar) now pass an explicit encoding, and writeUsVarbyte's narrowed signature (Buffer only) matches its only remaining callers (test-only, per the PR description).

Minor observation (not a bug)

The data getter's no-copy fast path only applies when everything still fits in the single open chunk (_bufs.length === 0). Once any large buffer (>= CHUNK_SIZE) is written via writeBuffer, it moves straight into _bufs, and .data (unlike getBuffers()/consume()) will then always go through slice() and copy — including the referenced-not-copied large buffer. Right now no src caller both writes a >= CHUNK_SIZE buffer into a WritableTrackingBuffer and reads it back via .data (the large-value paths, e.g. varbinary.ts's PLP generator, bypass the tracking buffer entirely and yield raw buffers), so this isn't a live issue today — but worth keeping in mind for the follow-up streaming-TVP work mentioned in the PR description: consumers of that work will need getBuffers()/consume() rather than .data to actually realize the zero-copy benefit for large values.

Test coverage

Very thorough for the core class: chunk coalescing, chunk-boundary writes, large-buffer referencing vs. copying, consume (including cross-chunk consumption), stability of a previously-returned data/getBuffers() snapshot across later writes, and string encoding edge cases (surrogate pairs/lone surrogates, exact chunk-size boundaries). The existing call-site tests were mechanically updated to pass explicit encodings with behavior preserved. Nice benchmark addition that isolates the coalescing path specifically (vs. the pass-through path exercised by the RPC payload benchmark).

Performance

Numbers in the PR description are consistent with the design (removing per-growth Buffer.concat, avoiding Buffer.alloc zero-fill, referencing rather than copying large writeBuffer values). No concerns.

Security

No concerns — this is pure binary wire-protocol serialization with no untrusted-input parsing changes, and the allocUnsafe usage was verified as sound above.

Code quality

The rewrite is a nice simplification: dropping constructor-time size/encoding hints (which the growth model makes unnecessary) and dead code (writePLPBody, writeMoney, copyFrom, the string branch of writeUsVarbyte) removes real complexity. The bl-style consumer API (length/getBuffers/consume/slice) is a sensible, minimal surface for the described follow-up work. Docstrings are appropriately focused on non-obvious invariants (view-vs-copy semantics, must-not-mutate contract for referenced buffers) rather than restating what the code does.

Nice work overall — didn't find anything blocking.

[claude]

Review: WritableTrackingBuffer chunked rewrite

I read through the full diff and traced the buffer-list logic (_ensure/_seal/writeBuffer/data/slice/consume) by hand, plus had a subagent verify every src/ call site that reads .data off a WritableTrackingBuffer instance. Overall this is a careful, well-scoped perf rewrite — the correctness reasoning below didn't turn up any real bugs.

Correctness

• Aliasing/view safety: data returns a live view (_open.subarray(0, this._pos)) only when there's a single chunk. Since this._pos is captured by value at call time, later writes into the same open chunk land after that fixed end offset and can't corrupt an already-returned view. I checked every src/ caller that does buffer.data — in every case it's the terminal read on that instance (return buffer.data, a final yield, or an immediate copy elsewhere), so there's no live case of "read .data, then keep writing the same instance" today.
• ALL_HEADERS fix (src/all-headers.ts) is correct and actually fixes a latent bug in the old code: patching TotalLength by writing into data.writeUInt32LE(data.length, 0) only worked because the buffer happened to contain nothing but the headers at that point. Writing the precomputed constant (4 + TXNDESCRIPTOR_HEADER_LEN) up front is strictly better.
• Encoding-default removal: I checked all three call sites the description says relied on the implicit ucs2 default (RPC procedure name, RPC parameter name, TVP TYPE_INFO) plus the other WritableTrackingBuffer construction sites (bulk-load.ts, transaction.ts, ntlm-payload.ts, prelogin-payload.ts, sqlbatch-payload.ts) — every one now passes the same encoding it used to get implicitly or already used explicitly. No behavior change there.
• Chunk growth/seal logic: traced _ensure/_seal through the boundary cases (write exactly fills the open chunk, write larger than the doubled chunk, write when _pos === 0 so no empty chunk gets sealed in). All consistent with the doubling-up-to-CHUNK_SIZE + exact-size-chunk-for-oversized-writes design, and the new tests exercise exactly these cases well (chunk-boundary test, "never produces coalesced chunks larger than the chunk size", pass-through vs. copy at the CHUNK_SIZE threshold).
• writeString's ucs2 fast path (value.length * 2 instead of Buffer.byteLength) is correct — UCS-2/UTF-16LE always encodes each UTF-16 code unit as 2 bytes regardless of (lone) surrogates, and the new test explicitly covers a lone surrogate plus surrogate pairs against Buffer.from as the oracle.
• Removal of writePLPBody, writeMoney, copyFrom, and the string branch of writeUsVarbyte all check out as genuinely dead — no remaining callers in src/ or test/.

Minor observations (non-blocking)

• New dual-export pattern: writable-tracking-buffer.ts is the only file in src/ that combines export const CHUNK_SIZE with the module.exports = Class override (grep'd for the combination). It works — the manual module.exports.CHUNK_SIZE = CHUNK_SIZE line re-adds the named export after the override — but it's a one-off pattern; a future maintainer copy-pasting the more common module.exports = ClassName idiom elsewhere in the codebase without the extra line would silently drop a named export. Might be worth a one-line comment explaining why the extra line is there.
• getBuffers()/consume()/slice() are unused in src/ today (only exercised by tests) — expected per the PR description since this is prep for the streaming TVP follow-up, but worth confirming reviewers are fine merging consumer-side API that has no current production caller.
• Reference-not-copy contract for writeBuffer on buffers ≥ CHUNK_SIZE is a real semantic change (previously everything was defensively copied). Today no call site hands it a large, externally-mutable buffer — the only writeBuffer callers pass internally-computed, ephemeral buffers (NTLM sub-hashes, randomBytes, txnDescriptor, generateTypeInfo() output), and user-supplied large parameter values bypass this class entirely (they're yielded straight from the data-type generators). So there's no live bug, but this is precisely the contract the follow-up streaming TVP work will need to be careful about if it starts feeding user-owned buffers through here.
• writeString has no special-casing for a single string longer than CHUNK_SIZE (unlike writeBuffer) — it'll just allocate one big exact-size chunk via _ensure, which is correct but means the "chunks are at most CHUNK_SIZE" framing in the class doc comment isn't quite true for that path. Not a bug given current usage (large string/varchar(max) values go through PLP in the data-type serializers, not writeString), just a doc nit.

Test coverage

Strong. The rewritten test file explicitly covers: view-vs-copy semantics of data, stability of a previously-read data view across later writes/chunk seals, coalescing of small buffers into one chunk with a bound on chunk count, copy-vs-reference at the CHUNK_SIZE threshold, chunk-boundary write splitting, consume (including across a referenced large chunk, and confirming a previously-returned getBuffers() array is unaffected), and string encoding (surrogates, encoding-length threshold crossing the open chunk) verified against Buffer.from as an oracle. The commit history shows the author caught and fixed a flaky assertion (ArrayBuffer identity depending on Node's allocation pool) rather than leaving it — good sign.

Performance

The benchmark methodology (isolating the class itself vs. end-to-end RPC serialization) is sound, and the writeup is honest about a real pitfall it found (a 1 KB first-chunk + copying data regressed RPC serialization ~15% vs. baseline despite the class being faster in isolation) rather than just reporting the best number. allocUnsafe instead of alloc is safe here since every reader (data, slice, getBuffers) bounds its output to _pos/sealed chunk lengths, so uninitialized memory never leaks out.

Security

No concerns. No user-controlled data reaches unbounded allocation sizes here (the largest single-write allocation is caller-computed, and all current large-buffer callers are internal/ephemeral), and the allocUnsafe usage doesn't leak uninitialized memory for the reason above.

Nice work — this is a large, mechanical-looking diff that's actually backed by careful reasoning (differential fuzzing, byte-for-byte comparison against the base for every payload builder) and it shows in how few loose ends there are.

[claude]

Review

Reviewed by walking the full diff and the class implementation by hand (_ensure/_seal/writeBuffer/writeString/consume/slice), plus grepping every call site of the removed/changed APIs (copyFrom, writePLPBody, writeMoney, writeUsVarbyte, the default-encoding string writers, and the constructor). Couldn't run the test suite locally (no network access for npm ci in this sandbox), so this is a static review rather than an execution-verified one — the PR description's own validation (byte-for-byte comparison, fuzzing, 457 unit tests, full integration suite) covers that gap well.

Overall: this is a careful, well-scoped rewrite. The chunk/seal/ensure state machine is correct as far as I can trace it, the migration of call sites is complete (no leftover references to doubleSizeGrowth, compositeBuffer, makeRoomFor, newBuffer, or the implicit default encoding), and the ALL_HEADERS fix is a genuine correctness improvement, not just a refactor — the old code's data.writeUInt32LE(data.length, 0) only produced the right TotalLength because the buffer happened to be freshly allocated for that one call; writing the constant 4 + TXNDESCRIPTOR_HEADER_LEN up front removes that implicit assumption entirely.

Things I specifically checked and found sound

• No uninitialized-memory leak from Buffer.allocUnsafe. Every place that exposes a chunk (data, slice, getBuffers) only ever exposes subarray(0, pos) or buffers whose length exactly matches what was written (this.length is incremented by precisely the same amount in every write path, including the ucs2 fast path in writeString where value.length * 2 is used instead of Buffer.byteLength — those two are equivalent for ucs2, so no truncation/overrun risk there). This matters here because a mismatch would leak adjacent heap bytes onto the wire, which the old zero-filled Buffer.alloc couldn't do.
• Aliasing threshold (writeBuffer pass-through at ≥8 KB) is inert today. I grepped for every writeBuffer call in src/ — none of them currently pass a large, potentially-mutable, user-supplied buffer through WritableTrackingBuffer; the one place large parameter values exist (VarBinary's PLP path) yields buffers directly from the generator, bypassing this class entirely. So the "must not be modified until consumed" contract isn't exercised by any real code path yet. Worth flagging for whoever picks up the follow-up streaming-TVP work mentioned in the description: once a real large-buffer path routes through writeBuffer, it's worth double-checking that the source buffer genuinely isn't reused/mutated by application code between the call and the eventual socket write.
• Growth doubling logic (MIN_CHUNK_SIZE 64 → doubles via _seal → capped at CHUNK_SIZE, with an exact-size chunk for single writes larger than the current open chunk) matches the description and the tests exercise the boundary cases well (62/64-byte straddle, 8 KB-minus-1 vs exactly 8 KB, coalescing count bounds).
• Removed methods (copyFrom, writePLPBody, writeMoney, the string branch of writeUsVarbyte) are confirmed to have zero remaining callers in src/.

Minor nits (non-blocking)

• test/unit/tracking-buffer/writable-tracking-buffer-test.ts: const { CHUNK_SIZE } = WritableTrackingBuffer; is inserted between the two import statements. Not a lint violation in this repo's config (no import/first-style rule configured), just a small readability nit — would read more conventionally after all imports.
• In _ensure, when the open chunk is non-empty and the incoming write is larger than the just-doubled chunk, _seal() allocates a doubled buffer that's immediately discarded and replaced by an exact-size one a few lines later. Harmless (a single small extra allocation on an already-cold path), not worth restructuring.

Test coverage

Strong — the new tests cover view-vs-copy semantics of data, the copy/reference boundary at exactly CHUNK_SIZE, chunk-count bounds under coalescing, string encoding against Buffer.from including surrogate pairs and lone surrogates, fixed-width writes, chunk-boundary straddling, and consume/slice including cross-chunk consumption. The description's mention of byte-for-byte comparison against the previous implementation across all 29 parameter types plus differential fuzzing is exactly the right validation strategy for a rewrite like this where subtle off-by-ones would be easy to miss in unit tests alone but catastrophic on the wire.

No security or performance concerns beyond what's already covered above — the benchmark numbers in the description are consistent with what I'd expect from eliminating concatenation-on-growth.

[claude]

Review

Reviewed the WritableTrackingBuffer rewrite (src/tracking-buffer/writable-tracking-buffer.ts) and its call sites. This is a careful, well-executed piece of engineering — walking through the implementation line-by-line, I didn't find any correctness bugs.

What I checked specifically

• Chunking/growth logic (_ensure, _seal, doubling from MIN_CHUNK_SIZE to CHUNK_SIZE): traced through boundary conditions (writes that exactly fill a chunk, writes larger than the open chunk, writes larger than CHUNK_SIZE) — all sound.
• Buffer.allocUnsafe usage: this replaces the old Buffer.alloc (zero-filled) allocation, which is a reasonable place to double-check for uninitialized-memory exposure. Every path that returns data (data, slice, the sealed _bufs entries) is bounded by the tracked write position/length, so no unwritten/garbage bytes can ever leak out. Good.
• writeBuffer's pass-through-vs-copy threshold (>= CHUNK_SIZE): consistent with the doc comment, and I confirmed no current src call site actually passes a buffer at/above that threshold (RPC parameter values, e.g. varbinary(max), are yielded directly as raw buffers, never routed through WritableTrackingBuffer.writeBuffer) — so the "must not be modified until consumed" aliasing contract isn't actually exercised yet in production code. That matches the PR description (this is prep for the streaming-TVP follow-up).
• consume: worked through the multi-chunk partial-consume case by hand (bufs.slice(i) + subarray on the boundary chunk) against the "consumes across chunks" test — matches.
• ALL_HEADERS constant (src/all-headers.ts): verified ALL_HEADERS_LEN = 4 + TXNDESCRIPTOR_HEADER_LEN = 22 against the actual bytes written (TotalLength 4 + HeaderLength 4 + Type 2 + TransactionDescriptor 8 + OutstandingRequestCount 4), and confirmed txnDescriptor is always the fixed 8-byte buffer from connection.ts (this.transactionDescriptors = [Buffer.from([0,0,0,0,0,0,0,0])]). The old in-place patch-through-data trick this replaces was indeed relying on the buffer being "fresh," as the PR description says — this fix is correct and more robust.
• Encoding cleanup: grepped every remaining writeString/writeBVarchar/writeUsVarchar/writeUsVarbyte call site in src and test — all now pass an explicit encoding, none left relying on a removed default.
• No stale references: confirmed no leftover usages of copyFrom, writePLPBody, writeMoney, doubleSizeGrowth, compositeBuffer, makeRoomFor, or the old constructor signature anywhere in src/test, and no hand-written .d.ts in types/ references this class.

Minor, non-blocking observations

1. getBuffers()/consume() have no production callers today — only the test suite exercises them. The PR description explains this is intentional groundwork for a follow-up (streaming TVP) PR, which is a reasonable exception to "don't add speculative API surface," just flagging it so it doesn't get lost as unexplained dead code if this lands well before the follow-up.
2. The class doc says the buffer returned by data "must not be modified," but test/unit/token/token-stream-parser-test.ts's createDbChangeBuffer() helper does mutate it in place (data.writeUInt16LE(...)) to backpatch a length field, relying on the single-chunk view behavior. It works correctly today and the PR commit history shows this was deliberately tightened (reading data once instead of twice), but it's worth a short comment noting the reliance on view-vs-copy semantics, since it'd silently produce wrong output (patching a copy instead of the real buffer) if that helper's payload ever grew past one chunk.
3. For the future streaming work: since writeBuffer now references rather than copies large buffers, that "must not modify until consumed" contract becomes load-bearing for the first time once a caller actually hits the >= CHUNK_SIZE path. Worth keeping in mind for review of the follow-up PR, since a violation would be a silent data-corruption bug rather than a crash.

Test coverage

The added/rewritten unit tests in test/unit/tracking-buffer/writable-tracking-buffer-test.ts are thorough: view-vs-copy semantics for data, chunk-size boundary behavior, coalescing bounds, string encoding parity against Buffer.from (including surrogate pairs and lone surrogates), fixed-width writes, and consume/slice across chunk boundaries. Combined with the described byte-for-byte comparison against the old implementation and differential fuzzing mentioned in the PR description, this gives good confidence in behavioral equivalence.

Performance / security

No concerns. The chunk-doubling growth strategy is the standard amortized-O(1) approach and matches the benchmark claims structurally. No user-controlled input influences allocation sizes in a way that differs from before, and as noted above, no uninitialized-memory exposure is possible given how allocUnsafe results are bounded before being exposed.

Nice work — this is a meaningful perf win with a notably careful validation story for a low-level buffer rewrite.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.64%. Comparing base (170fabc) to head (6306338).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1773      +/-   ##
==========================================
+ Coverage   81.02%   81.64%   +0.62%     
==========================================
  Files          92       92              
  Lines        4948     4931      -17     
  Branches      938      932       -6     
==========================================
+ Hits         4009     4026      +17     
+ Misses        640      609      -31     
+ Partials      299      296       -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[arthurschreiber]

On 6306338, SQL Server 2016 Windows / Node.js 26.x failed in the "install SQL Server" step: the SQL Server 2016 setup.exe on the Windows runner exited with code 2233073689 (Process returned -2061893607 in the setup log) after the Service Pack 3 slipstream, before npm ci or any test ran. The same job passed on the two previous heads of this PR (95d1217, 621f106) and the 2016 / Node 22 and 24 jobs on this head are running normally, so this is the installer on that runner, not the diff. I'll re-run the failed job once the run completes.

---

Generated by Claude Code

[arthurschreiber]

Run 33640561947 on 6306338 finished: 23 of 24 jobs passed; the only failure is the SQL Server 2016 / Node.js 26.x installer step described above. I tried to re-run the failed job and the API refused (403 Resource not accessible by integration), so it needs a "Re-run failed jobs" from the Actions UI by someone with write access. Nothing in the diff is involved: the job never reached npm ci.

---

Generated by Claude Code

[github-actions]

🎉 This PR is included in version 20.2.1 🎉

The release is available on:

• npm package (@next dist-tag)
• GitHub release

Your semantic-release bot 📦🚀