normalizeHtmlEntities deletes every printable non-ASCII entity, not just non-printable ones (32..126 bound)

Type: bug (content fidelity) — a separate root cause from #1763 Severity: medium Area: src/github/utils/sanitizer.ts Effort: small, but it interacts with the ordering fix — see "Sequencing"

Summary

normalizeHtmlEntities keeps a decoded numeric entity only when num >= 32 && num <= 126 (src/github/utils/sanitizer.ts:53 decimal, :60 hex) and returns "" for everything else.

The test that documents the intent is named "should remove non-printable entities" (test/sanitizer.test.ts:183-186) and asserts only � and �. But 126 is the top of ASCII, not the top of printable, so the implementation also deletes every printable non-ASCII codepoint: Latin-1 accented letters, curly quotes, em/en dashes, CJK, emoji.

sanitizeContent runs on untrusted issue/PR/comment text before it reaches the model (src/github/data/formatter.ts — formatContext:16, formatBody:42, formatComments:57, formatReviewComments:79 including comment.diffHunk:126; also src/create-prompt/index.ts and both MCP servers), so the model is handed text with those characters silently removed rather than decoded.

Reproduction

main @ a4f54ef2c58884867281bd8e2f8d63352ad019a9, bun 1.3.14, darwin arm64.

normalizeHtmlEntities("é")           // é   -> ""
normalizeHtmlEntities("’")          // ’   -> ""
normalizeHtmlEntities("—")          // —   -> ""
normalizeHtmlEntities("你")         // 你  -> ""
normalizeHtmlEntities("😀")        //   -> ""
normalizeHtmlEntities("A’")   // "A’" -> "A"

Same text raw vs entity-encoded, through the whole pipeline:

in : "Thanks — café naïve résumé, it’s fine"
out: "Thanks — café naïve résumé, it’s fine"        <- raw survives byte-for-byte

in : "Thanks &#8212; caf&#233; na&#239;ve r&#233;sum&#233;, it&#8217;s fine"
out: "Thanks  caf nave rsum, its fine"              <- entity-encoded is mangled

The second case is the defect: identical characters, different fate, decided only by whether the author's editor happened to emit entities. What the model sees is doubled spaces and glued words (nave rsum). A review comment whose diffHunk contains entity-encoded non-ASCII is mangled the same way.

Expected

é decodes to é, matching the raw character and matching the test's stated intent — remove non-printable, not non-ASCII.

Two constraints on any fix

1. String.fromCharCode cannot represent astral codepoints, so raising the bound alone silently corrupts them instead of fixing them:

String.fromCharCode(128512)   -> U+F600   (Private Use Area, garbage)
String.fromCodePoint(128512)  -> U+1F600  

Both replacements (:53 and :60) need fromCodePoint.

2. The over-deletion is currently load-bearing, so a widened decoder must keep rejecting invisible codepoints explicitly. Today ​ (U+200B zero width space), ­ (U+00AD soft hyphen) and ‮ (U+202E right-to-left override) are removed only because they fall outside 32..126 — nothing classifies them as invisible. They are Unicode Cf, not Cc, so a natural "drop control characters, keep the rest" predicate re-admits all three:

/\p{Cc}/u.test("\u200B") -> false
/\p{Cc}/u.test("\u00AD") -> false
/\p{Cc}/u.test("\u202E") -> false

stripInvisibleCharacters (sanitizer.ts:1-10) does remove all three when they are literal, but on main it runs at :70, before the decoder at :74, so it never sees a decoded one. A widened decoder therefore needs its own rejection of Cc/Cf/zero-width/bidi codepoints, or the invisible strip must run again after decoding.

This is where #1763 and this issue meet: #1771's reorder puts the decode before stripInvisibleCharacters, which makes the pipeline safe against entity-encoded invisibles. That order should be kept if the bound is ever widened — under #1771's order a widened decoder's output is re-checked by stripInvisibleCharacters; under main's current order it is not.

Sequencing — deliberately not a sixth PR

This is a distinct root cause from #1763, which is about pipeline order. The bound is wrong even with the order fixed, and #1771's reorder does not change it.

There are five open PRs in this function (#1035, #1105, #1504, #1739, #1771). I pulled each diff and checked: none of them changes the 32..126 predicate — zero occurrences of it in any added or removed line across all five. #1105 adds named-entity decoding but leaves both numeric blocks as context only. So this defect survives whichever of them lands.

I'm filing it as an issue rather than opening a PR that would conflict with all five at once. Happy to send the change on top of whichever ordering wins, since the fix belongs there.

AI-assisted investigation. Every output above was produced by running the code at the stated commit.