fix(content): stop content truncation from splitting a surrogate pair - #391
Conversation
The clip cut on a UTF-16 code unit. A cut inside a surrogate pair left a lone high surrogate at the end of the text. The lone surrogate became U+FFFD after a UTF-8 round trip, so the output showed a replacement character. The clip now drops the unpaired code unit. A budget that fits both code units still keeps the character.
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. Reviewed August 30, 2026, 3:42 PM ET / 19:42 UTC. ClawSweeper reviewWhat this changesThe PR prevents content clipping from returning a string that ends with an unpaired UTF-16 high surrogate, with regression tests for emoji-boundary and BMP clipping. Merge readinessKeep open for normal maintainer review: current main still has the UTF-16 clipping defect, and this focused PR repairs it with direct before/after proof and regression coverage. Priority: P2 Review scores
Verification
How this fits togetherThe core content cleaner clips extracted webpage and transcript text to a character budget before downstream formatting and model-facing processing. Browser extraction, link-preview processing, asset extraction, and speaker identification consume its output. flowchart LR
A[Extracted page or transcript text] --> B[Content budget]
B --> C[Sentence-aware clipping]
C --> D[Surrogate-boundary check]
D --> E[Well-formed clipped text]
E --> F[Output and model input]
Before merge
Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Land the narrow boundary guard and regression coverage so every content-budget caller receives well-formed text without changing valid clipping behavior. Do we have a high-confidence way to reproduce the issue? Yes. Current main still slices by UTF-16 code-unit count without a trailing-high-surrogate check, and the supplied direct-helper trace shows the split-emoji failure and fixed result on this PR head. Is this the best way to solve the issue? Yes. Removing only a trailing high surrogate preserves the existing code-unit budget while avoiding replacement-character corruption and retaining complete pairs. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 66202d92f055. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (14 earlier review cycles; latest 8 shown)
|
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
@clawsweeper re-review I added a runtime terminal transcript to the PR body. It calls the exported helper directly, on this branch and on
|
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
proof: sufficient
|
Triage recommendation: LAND. I reproduced this through the built core package's exported Input: 80 ASCII characters, one emoji, and trailing text. The defect is the UTF-16 slice ending between the emoji's two code units. The narrow trailing-high-surrogate guard fixes that boundary without changing the existing code-unit budget or valid sentence-boundary behavior. Reviewed commit: Both content PRs also apply together cleanly on current main and pass all 12 cleaner regression tests. The baseline package build passed on Node 24.20.0, and each changed cleaner was compiled into the built core package for the integration checks. Local tests reused the installed dependency tree (Vitest 4.1.10); each original PR's exact-head CI is green. Codex autoreview was scoped-clean at the default P0 threshold. No source repair or branch rewrite was needed. No merge performed. Preserve Suggested landing changelog: “Content extraction: avoid splitting UTF-16 surrogate pairs when clipping to a character budget (#391, thanks @devYRPauli).” |
2 participants
Problem
clipAtSentenceBoundaryinpackages/core/src/content/link-preview/content/cleaner.tscuts withinput.slice(0, maxLength). That is a UTF-16 code unit index.Every non-BMP character is a surrogate pair of two code units. Emoji are the common case. When the cut lands between the two units, and no sentence break sits past
maxLength * 0.5, the rescue branch does not run and the function returns a string that ends in a lone high surrogate.applyContentBudgetthen calls.trim(), which does not remove a lone surrogate. Nothing later repairs it.Measured with the function as it is on
main, using"x".repeat(80) + "\u{1F600}" + " and more trailing text here"andmaxLength81:The text is written to stdout, to
--jsonoutput, and into the model request body. Each of those is a UTF-8 encode, so the reader sees a replacement character where the text should end cleanly.applyContentBudgetis exported frompackages/core/src/content/index.tsand is called from:packages/core/src/content/browser-html.ts:33packages/core/src/content/link-preview/content/utils.ts:249,335,358,362src/run/flows/asset/extract.ts:55,117src/speaker-identification/identify.ts:194So any page or transcript that carries an emoji near the budget boundary can hit this.
Change
Four lines. When the clipped text ends in an unpaired high surrogate, drop that one code unit.
The guard sits after the sentence-break return, so it only affects the plain truncation path. A cut at
.,!,?or a blank line is already safe, because those characters are all BMP.A trailing high surrogate is unpaired by definition, since its low half would follow it. A trailing low surrogate means the pair is complete, so it is left alone.
Scope
This fixes lone surrogates only. It does not add grapheme cluster handling.
A cut inside a ZWJ emoji sequence or before a combining mark still produces well-formed text, so it is a different question. A lone surrogate is invalid text. I kept the two separate to keep this change small. Say the word if you want the grapheme case handled too.
I did not use
String.prototype.toWellFormed(). It replaces the lone surrogate with U+FFFD, which keeps the visible corruption instead of removing it.Proof
cleaner.tsreverted, tests kept:With the change:
Full suite after the change: 3018 passed, 43 skipped, 0 failed, across 585 test files.
oxfmt --checkreports the correct format.oxlintexits 0.tsc -p packages/core/tsconfig.build.json --noEmitexits 0.The new tests also assert the opposite case. When the budget is 82 and both code units fit, the emoji is kept. The existing expectations for
clipAtSentenceBoundaryare unchanged, and a plain BMP truncation still returns the same result as before.Runtime check on the real helper
The results above come from Vitest. This is a direct terminal run of the exported helper, on this branch and on
main.Command:
mainat861fa4a9:This branch at
d88c576:The input is 80
xcharacters plus one emoji. The budget is 81 UTF-16 code units.mainreturns the raw slice, which ends in a lone high surrogate. This branch drops that one code unit and returns 80 well-formed units.