fix(string): re-pair split surrogates on += append (node parity, #6741)

[proggeramlug]

Fixes #6741.

The bug

js_string_append (the += / string-building runtime path) copied operand bytes verbatim and never called canonicalize_surrogate_pairs, unlike every js_string_concat* path. So a lone high surrogate at the end of dest plus a lone low surrogate at the start of src were stored as two separate 3-byte WTF-8 sequences instead of the astral code point's 4-byte UTF-8:

const e = "a👋b"; const hi = e[1], lo = e[2];
hi + lo;                      // 👋            (correct — concat canonicalizes)
let Y = ""; Y += hi; Y += lo; // was: lone high surrogate (cp 55357, len 2)

Node re-pairs adjacent surrogates in both cases; perry only did on concat. Any string rebuilt one UTF-16 code unit at a time over emoji/astral text (ANSI stripping, manual parsers, width calc) was silently corrupted — codePointAt / [...s].length / Intl.Segmenter all disagreed with Node.

How it surfaced

Running the pi coding-agent TUI under perry: pi's visibleWidth strips ANSI via stripped += clean[i], which splits an emoji's surrogate pair across two += appends → over-counted line width → pi's Rendered line exceeds terminal width invariant aborted → uncaughtException, crashing the UI on any emoji in a colored line (a normal streamed assistant reply). Found while validating #6728.

The fix

Propagate the lone-surrogate flag through append and, when a high→low pair straddles the dest|src join, canonicalize the result — mirroring js_string_concat. The join is detected in O(1) from the boundary bytes (both operands are already canonical, so a pair can only newly form there), so ordinary ASCII appends stay allocation-free and never pay for a whole-string scan. Both the in-place (refcount==1) and fresh-alloc append paths are covered.

Validation

• New differential regression test tests/test_string_append_surrogate_repair.sh — perry output must byte-match Node's (covers the += repro, char-by-char rebuild, multiple emoji, genuinely-lone surrogates that must stay lone, the ASCII fast path, and split-by-unrelated-append).
• perry-runtime string/append/concat/surrogate suites: 172 passed, 0 failed.
• End-to-end: pi's own visibleWidth on the exact crashing line now returns 114 (== Node), down from 124 — the render invariant no longer trips.

Independent of #6740 (that PR fixes the async/codegen blockers for #6728; this fixes a string-subsystem parity bug found while testing the result). Built on current main.

Summary by CodeRabbit

• Bug Fixes

  • Fixed string appends involving UTF-16 surrogate pairs to ensure characters are correctly reconstructed across incremental appends.
  • Preserved lone surrogate characters when concatenating strings.
  • Maintained correct behavior for emoji, ASCII-heavy appends, and string rebuild scenarios.

• Tests

  • Added coverage comparing string append output with Node.js across surrogate and emoji cases.

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

Surrogate-aware string append

Layer / File(s)	Summary
Boundary-aware append canonicalization crates/perry-runtime/src/string/append.rs	js_string_append preserves lone-surrogate flags and canonicalizes high-to-low surrogate pairs formed across append boundaries for in-place and newly allocated strings.
Differential surrogate regression coverage tests/test_string_append_surrogate_repair.sh	A regression script compares Perry and Node.js output for split surrogate, lone surrogate, emoji, and ASCII append scenarios.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TypeScriptProgram
  participant js_string_append
  participant canonicalize_surrogate_pairs
  participant OutputComparison
  TypeScriptProgram->>js_string_append: append split surrogate strings
  js_string_append->>js_string_append: detect boundary pair and propagate flags
  js_string_append->>canonicalize_surrogate_pairs: repair formed surrogate pair
  canonicalize_surrogate_pairs-->>TypeScriptProgram: return canonicalized string
  TypeScriptProgram->>OutputComparison: emit metrics and equality results
  OutputComparison->>OutputComparison: compare Perry output with Node.js

Loading

Possibly related issues

• Issue 6741: The changes implement split-surrogate repair in js_string_append and add its regression coverage.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is detailed, but it does not follow the required template and omits explicit Summary/Changes/Test plan/Checklist sections.	Reformat the PR body to match the template, add Summary/Changes/Test plan/Checklist headings, and include concrete verification commands.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: surrogate re-pairing during += string appends for Node parity.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_string_append_surrogate_repair.sh`:
- Around line 52-56: Extend the surrogate-repair test around the multi-character
append case to include a surrogate pair whose completion forces fresh
allocation, exercising the allocation path in the append implementation near the
referenced boundary handling. Assert that the resulting string preserves the
pair and expected code-point count, while retaining the existing checks for the
capacity-available path.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 96321c78-1391-4832-87d4-715cfb87fa83

📥 Commits

Reviewing files that changed from the base of the PR and between 574612d and 73c628a.

📒 Files selected for processing (2)

• crates/perry-runtime/src/string/append.rs
• tests/test_string_append_surrogate_repair.sh

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Cover surrogate repair in the fresh-allocation path.

multi repairs pairs while capacity remains available; bulk reallocates but has no surrogate boundary. Add a boundary pair that forces allocation, or regressions in lines 156-159 will pass unnoticed.

Proposed regression case

 // 3. Multiple emoji, some with a leading ASCII run (exercises both branches).
 const t = "x😀y👋z🎉w";
 let r = "";
 for (let i = 0; i < t.length; i++) r += t[i];
 console.log("multi", r === t, [...r].length, r.codePointAt(1));
 
+// 3b. Fill the 32-byte append buffer, then form a pair across a fresh allocation.
+let fresh = "";
+for (let i = 0; i < 29; i++) fresh += "a";
+fresh += hi; // 29 ASCII bytes + 3-byte WTF-8 high surrogate.
+fresh += lo; // Forces allocation and must re-pair at the boundary.
+console.log("fresh", fresh.length, [...fresh].length, fresh.codePointAt(29));
+
 // 4. A genuinely lone surrogate must STAY lone (no false merge).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// 3. Multiple emoji, some with a leading ASCII run (exercises both branches).
	const t = "x😀y👋z🎉w";
	let r = "";
	for (let i = 0; i < t.length; i++) r += t[i];
	console.log("multi", r === t, [...r].length, r.codePointAt(1));
	// 3. Multiple emoji, some with a leading ASCII run (exercises both branches).
	const t = "x😀y👋z🎉w";
	let r = "";
	for (let i = 0; i < t.length; i++) r += t[i];
	console.log("multi", r === t, [...r].length, r.codePointAt(1));
	// 3b. Fill the 32-byte append buffer, then form a pair across a fresh allocation.
	let fresh = "";
	for (let i = 0; i < 29; i++) fresh += "a";
	fresh += hi; // 29 ASCII bytes + 3-byte WTF-8 high surrogate.
	fresh += lo; // Forces allocation and must re-pair at the boundary.
	console.log("fresh", fresh.length, [...fresh].length, fresh.codePointAt(29));

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_string_append_surrogate_repair.sh` around lines 52 - 56, Extend
the surrogate-repair test around the multi-character append case to include a
surrogate pair whose completion forces fresh allocation, exercising the
allocation path in the append implementation near the referenced boundary
handling. Assert that the resulting string preserves the pair and expected
code-point count, while retaining the existing checks for the capacity-available
path.