Skip to content

[87][88] Decode UTF-8 across Buffer chunks in the Node adapters, and document a faster encoder for the web pipeline - #93

Merged
TomStrepsil merged 6 commits into
mainfrom
87-88_utf8-decoding-and-encoder-pipeline
Oct 15, 2026
Merged

TomStrepsil merged 6 commits into
mainfrom
87-88_utf8-decoding-and-encoder-pipeline

Conversation

@TomStrepsil

@TomStrepsil TomStrepsil commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Issue

resolves #87
resolves #88

Details

#87: the Node adapters split multi-byte characters. ReplaceContentTransform and AsyncReplaceContentTransform called chunk.toString() on each Buffer, so a character split across two chunks became two U+FFFD. Each transform now has its own StringDecoder:

 TransformBase
+  #decoder = new StringDecoder("utf8")
+  _decode(chunk)
+    string → decoder.end() + chunk     # pending bytes first, so order is kept
+    Buffer → decoder.write(chunk)      # holds an incomplete sequence back
+  _decodeRemainder() → decoder.end()

 _transform(chunk)
-  engine.write(chunk.toString())
+  engine.write(_decode(chunk))

 _flush()
+  remainder = _decodeRemainder()
+  if remainder: await engine.write(remainder)   # trailing partial sequence → U+FFFD
   await engine.end()

"héllo {{x}} wörld" as two Buffers split inside é, through ReplaceContentTransform with StringAnchorSearchStrategy(["{{", "}}"]) → "X" (Node 24.20.0):

main:   h��llo X wörld
branch: héllo X wörld

The output side had the same flaw. Each pushed string was encoded to UTF-8 on its own, so an output chunk ending between the two halves of a surrogate pair (e.g. a match for /a./, no u flag, in "a😀") became two U+FFFD. The sink now holds a trailing high surrogate back until the next chunk, as TextEncoderStream does:

 TransformBase
   engine.start({
-    enqueue: chunk → push(chunk)
+    enqueue: chunk → text = held + chunk
+                     held = text ends in a high surrogate ? its last code unit : ""
+                     push(text without held)
   })

 _flush()
   ...
   await engine.end()
+  push(held)                                     # a lone high surrogate → U+FFFD

"a😀b" through ReplaceContentTransform with RegexSearchStrategy(/a./), echoing the match:

main:   a��b
branch: a😀b
  • String chunks. With decodeStrings: false, string chunks reach the engine unchanged. The default decodeStrings: true is still in place.
  • Docs. The README's Node Transform caution and the TransformBase JSDoc now describe how decoding works. For other charsets they say to decode upstream with a streaming TextDecoder and write strings. iconv-lite is mentioned only for re-encoding the output to a legacy charset.

#88: TextEncoderStream is slow on older Node. On Node 22, Node 24 before 24.21.0 and Node 26 before 26.8.0, TextEncoderStream costs about 5–10 ms per MB (nodejs/node#65414). The README's WHATWG pipeline now has a tip with a utf8EncoderStream() alternative: a TextEncoder in a TransformStream that holds a trailing high surrogate back, like TextEncoderStream, so it is safe whatever the strategy or replacement emits. On ~2 MB in 16 KiB chunks (Node 24.20.0) it took 4.34 ms, against 4.50 ms for a plain per-chunk encode and 35.91 ms for TextEncoderStream. The Compatibility section points to the tip.

Scout rule

Regex zero-length skip splits surrogate pairs. Not part of either issue; found while checking #88's suggestion that no search strategy ends an output chunk part-way through a surrogate pair. When RegexSearchStrategy skipped a zero-length match (e.g. from /a*/), it advanced by one code unit, so an emoji was split across two non-match chunks, which a per-chunk encoder turned into two U+FFFD. Both encoders in this PR now cope with a split pair, but the skip now advances by one code point, so the emoji stays in one chunk:

 src/search-strategies/regex/
-├── skip-one-code-unit-past-zero-length-match.ts
+├── skip-one-code-point-past-zero-length-match.ts   # +2 for an astral code point, else +1
 └── lean-search-strategy.ts                         # uses the new helper at both call sites

"x😀y" through ReplaceContentTransform with RegexSearchStrategy(/a*/) → "-":

main:   x��y
branch: x😀y

Footnote 10 in the regex README now describes the new behaviour and how it differs from matchAll without the u/v flag.

Tests:

RegexSearchStrategy
  for pattern in [/a?/, /a*/, /a*/u, /(?:)/, /x|/v], fed whole and one code point at a time
    no segment ends between the two halves of a surrogate pair, and output is lossless
  flush that settles held-back content skips a whole surrogate pair

ReplaceContentTransform + RegexSearchStrategy(/a*/) (integration)
  an emoji survives a skipped zero-length match

This replaces the old test "advances by one code unit rather than one code point, splitting a surrogate pair but remaining lossless".

textWrittenTo(engine) moves to test/utilities.ts. The sync and async Node adapter tests now share it; before, each file had its own copy.

Semantic Version Impact

  • PATCH - Bug fixes and minor changes (backwards compatible)
  • MINOR - New features (backwards compatible)
  • MAJOR - Breaking changes (not backwards compatible)

No public API is added or removed, and reverting restores the old decoding and the old skip. Output text only changes where it was corrupted before. Behaviour that does change:

  • Node adapters, input. A Buffer that ends part-way through a character now has those bytes held back until the next chunk, so engine.write gets slightly less text and the next write gets the rest.
  • Node adapters, output. An output chunk ending in a high surrogate now has it held back until the next chunk, or until after engine.end(), so readers see that character one chunk later.
  • Regex strategy. Where a nullable pattern skips a zero-length match in front of an astral character, that character now arrives in one chunk instead of two. A pattern that can match a lone low surrogate (e.g. /a*|\uDE00/) no longer matches the second half of a pair, which it did before, as matchAll without u/v does.

Checklist

  • PR starts with [87][88]
  • I have added an entry to the [Unreleased] section in CHANGELOG.md, if the published package will change
  • I have performed a self-review of my own code
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Copilot AI lite review requested due to automatic review settings October 15, 2026 17:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The README’s encoder safety guidance needs qualification for strategies that can target surrogate halves.

1 open finding
What changed in this PR

Fixes UTF-8 corruption across Node Buffer chunks, preserves surrogate pairs during regex processing, and documents faster web encoding.

Changes:

  • Added streaming UTF-8 decoding to Node adapters.
  • Updated regex zero-length handling to advance by code point.
  • Added tests, shared utilities, documentation, and changelog entries.
File Summary
test/​utilities.ts Shares engine-output test helpers.
src/​search-strategies/​regex/​skip-one-code-unit-past-zero-length-match.ts Removes obsolete helper.
src/​search-strategies/​regex/​skip-one-code-point-past-zero-length-match.ts Adds code-point advancement.
src/​search-strategies/​regex/​search-strategy.test.ts Tests surrogate-pair preservation.
src/​search-strategies/​regex/​README.md Documents regex behavior.
src/​search-strategies/​regex/​lean-search-strategy.ts Uses code-point advancement.
src/​adapters/​node/​transform-base.ts Adds stateful UTF-8 decoding.
src/​adapters/​node/​sync-transform.ts Decodes chunks and flushes remainders.
src/​adapters/​node/​sync-transform.test.ts Tests synchronous decoding.
src/​adapters/​node/​sync-transform.integration.test.ts Tests synchronous integration behavior.
src/​adapters/​node/​async-transform.ts Handles asynchronous decoding and remainders.
src/​adapters/​node/​async-transform.test.ts Tests asynchronous decoding.
src/​adapters/​node/​async-transform.integration.test.ts Tests asynchronous integration behavior.
README.md Documents encoding and performance guidance.
docs/​CHANGELOG.md Records the fixes and documentation updates.

🧠 Review effort: Lite


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread README.md Outdated
Copilot AI lite review requested due to automatic review settings October 15, 2026 18:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The encoder guidance is unsafe for chunked surrogate pairs, and one regression test does not exercise the claimed code-unit split.

1 open finding

🧠 Review effort: Lite

Copilot AI lite review requested due to automatic review settings October 15, 2026 18:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Retain code-unit advancement to preserve valid no-u regex matches and correct the README example.

0 open findings

1 resolved since last review

🧠 Review effort: Lite

Copilot AI lite review requested due to automatic review settings October 15, 2026 18:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved blocking issues were identified, and regression coverage is included.

0 open findings

🧠 Review effort: Lite

@TomStrepsil
TomStrepsil merged commit 43c5596 into main Oct 15, 2026
10 checks passed
@TomStrepsil
TomStrepsil deleted the 87-88_utf8-decoding-and-encoder-pipeline branch October 15, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants