From 44ef0a21db85034cbc6a40d13912704670c2c1ae Mon Sep 17 00:00:00 2001 From: BenStullsBets Date: Fri, 26 Jun 2026 14:09:19 -0700 Subject: [PATCH] fix: render added blocks via markdown-safe sentinel path emitting cw-ins-* (heading/list structure preserved) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Defect: an ADDED block was colored via `render(wordDiffByAuthor("", raw, ...))`, which wrapped the whole raw markdown in an inline `` BEFORE markdown-it parsed it — so `## X` rendered as literal text, not `

`. Same structural bug for list, blockquote, and thematic-break blocks. Fix (three-part, pure / vscode-free): 1. `blockContentStart(raw)`: new helper returning the offset past block-level markers (ATX headings `## `, unordered/ordered list markers, blockquotes). Open sentinels must not precede these markers or markdown-it cannot recognise the block construct. 2. `injectSentinels`: clamp the open-sentinel lower-bound to `blockContentStart(raw)`, so for `## New Section` the sentinel lands AFTER `## ` (at position 3) instead of at position 0, letting the heading parse correctly. 3. `sentinelsToSpans` / `colorByAuthor`: add optional `kind: "by" | "ins" = "by"` parameter so the sentinel path can emit `cw-ins-{author}` for added blocks while all existing `colorByAuthor`/`cw-by-*` call-sites are unchanged. 4. `renderReview` ADDED block path: `render(wordDiffByAuthor("", raw, ...))` → `colorByAuthor(raw, blk.start, landedSpans, render, "ins")`. The CHANGED non-atomic prose path (`wordDiffByAuthor(before, after, ...)`) is untouched — it correctly keeps the block markers at column 0 of the unchanged prefix. 5. `renderReviewOp`: removed the now-vestigial `colored` parameter (was always `render` after the added-block path moved to `changedHtml`); unchanged blocks now call `render(op.block.raw)` directly. Regression test added: "renderReview: an added heading block renders as a heading AND is author-colored as an insertion" — RED on old code, GREEN after fix. All 259 tests pass; typecheck clean. Co-Authored-By: Claude Sonnet 4.6 --- src/trackChangesModel.ts | 52 ++++++++++++++++++++++++++-------- test/trackChangesModel.test.ts | 12 ++++++++ 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/trackChangesModel.ts b/src/trackChangesModel.ts index 104a533..4d10fd9 100644 --- a/src/trackChangesModel.ts +++ b/src/trackChangesModel.ts @@ -671,11 +671,38 @@ function clampOffDelimiterRun(raw: string, at: number): number { return p; } +/** + * Returns the offset within `raw` after block-level markdown markers (ATX + * headings, list markers, blockquote markers). Open sentinels must not be + * injected before these markers — a PUA char at position 0 would prevent + * markdown-it from recognising the block construct (e.g. `##` is only a + * heading marker when it is the very first char on the line). + */ +function blockContentStart(raw: string): number { + // ATX headings: "#{1,6} " at the start of the line. + const heading = raw.match(/^#{1,6} /); + if (heading) return heading[0].length; + // Unordered list markers: "- ", "* ", "+ ". + const ul = raw.match(/^[-*+] /); + if (ul) return ul[0].length; + // Ordered list markers: "N. " or "N) ". + const ol = raw.match(/^\d+[.)]\s+/); + if (ol) return ol[0].length; + // Blockquotes: "> " or ">". + const bq = raw.match(/^> ?/); + if (bq) return bq[0].length; + return 0; +} + /** Inject paired sentinels into a prose block's raw text for the spans clipped to it. */ function injectSentinels(raw: string, blockStart: number, spans: AuthorSpan[]): string { const inserts: { at: number; marker: string }[] = []; + // Open sentinels must not precede block-level markdown markers (heading/list/ + // blockquote syntax at position 0) — a PUA char before "## " prevents markdown-it + // from recognising the heading. Clamp to the content-start offset. + const minOpen = blockContentStart(raw); for (const s of spans) { - const lo = clampOffDelimiterRun(raw, Math.max(0, s.start - blockStart)); + const lo = Math.max(minOpen, clampOffDelimiterRun(raw, Math.max(0, s.start - blockStart))); const hi = clampOffDelimiterRun(raw, Math.min(raw.length, s.end - blockStart)); if (hi <= lo) continue; // empty (or clamped to empty) span contributes nothing inserts.push({ at: lo, marker: SENT[s.author].open }); @@ -711,13 +738,13 @@ const ALL_SENTINELS = new RegExp( * (one `` segment per text run). Tags are copied verbatim (with any stray * sentinel stripped, so no Private-Use-Area char ever leaks). Pure, deterministic. */ -function sentinelsToSpans(html: string): string { +function sentinelsToSpans(html: string, kind: "by" | "ins" = "by"): string { const out: string[] = []; let current: AuthorKind | null = null; // which author region we're inside let spanOpen = false; // whether a is currently open in `out` const openSpan = () => { if (current && !spanOpen) { - out.push(``); + out.push(``); spanOpen = true; } }; @@ -765,10 +792,11 @@ export function colorByAuthor( blockStart: number, spans: AuthorSpan[], render: (src: string) => string, + kind: "by" | "ins" = "by", ): string { const overlapping = spans.filter((s) => s.end > blockStart && s.start < blockStart + raw.length); const injected = injectSentinels(raw, blockStart, overlapping); - return sentinelsToSpans(render(injected)); + return sentinelsToSpans(render(injected), kind); } /** @@ -839,7 +867,6 @@ function proposalBlockHtml(p: ProposalView, render: (src: string) => string): st function renderReviewOp( op: BlockOp, render: (src: string) => string, - colored: (raw: string) => string, src: string, changedHtml?: string, ): string { @@ -851,9 +878,9 @@ function renderReviewOp( return `
${changedHtml}
`; } if (op.kind === "removed" || op.kind === "changed") return renderOp(op, render, src); - // "added": use changedHtml (author-colored word-diff from empty before, all insertions). - // "unchanged": colored() returns plain render(raw) — color means "changed since baseline". - return `
${changedHtml ?? colored(op.block.raw)}
`; + // "added": changedHtml is colorByAuthor sentinel output (markdown-safe, cw-ins-*). + // "unchanged": render(raw) — plain, no author coloring (color = "changed since baseline"). + return `
${changedHtml ?? render(op.block.raw)}
`; } /** @@ -967,19 +994,20 @@ export function renderReview( const blk = op.kind === "removed" ? undefined : ranges[ci++]; // Only ADDED and CHANGED prose blocks are author-colored (they differ since baseline); // UNCHANGED blocks render plain (color means "changed since baseline"). - const colored = (raw: string): string => render(raw); // A non-atomic changed prose block: author-colored word diff (ins by author, - // del by adjacency). An added block: word diff from empty before — all insertions. + // del by adjacency). An added block: sentinel-safe colorByAuthor with kind="ins" + // (markdown-it parses the block first, so ## →

before sentinels become spans; + // the old wordDiffByAuthor path wrapped raw in BEFORE parsing — structural bug). // `blk.start` is the landed offset of the after-side text. const changedHtml = blk && !clean ? op.kind === "changed" && !op.atomic ? render(wordDiffByAuthor(op.before.raw, op.block.raw, blk.start, landedSpans)) : op.kind === "added" - ? render(wordDiffByAuthor("", op.block.raw, blk.start, landedSpans)) + ? colorByAuthor(op.block.raw, blk.start, landedSpans, render, "ins") : undefined : undefined; - bodyParts.push(renderReviewOp(op, render, colored, srcAttr(blk), changedHtml)); + bodyParts.push(renderReviewOp(op, render, srcAttr(blk), changedHtml)); const here = blockIndex >= 0 ? byBlock.get(blockIndex) : undefined; if (here) for (const p of here) bodyParts.push(proposalBlockHtml(p, render)); } diff --git a/test/trackChangesModel.test.ts b/test/trackChangesModel.test.ts index 2af831e..dfabb87 100644 --- a/test/trackChangesModel.test.ts +++ b/test/trackChangesModel.test.ts @@ -491,6 +491,18 @@ describe("renderReview", () => { const html = renderReview("Keep.", "Keep.\n\nFresh line.", [{ start: 6, end: 17, author: "human" }], []); expect(html).toContain("cw-ins-human"); }); + + test("renderReview: an added heading block renders as a heading AND is author-colored as an insertion", () => { + // "Intro.\n\n" = 8 chars; "## New Section" = 14 chars → offset 8..22 in currentText. + // Before fix: wordDiffByAuthor wrapped raw in BEFORE markdown-it parsed it, + // so "## New Section" rendered as literal text (structural bug). After fix: + // colorByAuthor uses the sentinel technique — markdown-it parses the block first, + // so ## becomes a real

, THEN sentinels are replaced with cw-ins-* spans. + const html = renderReview("Intro.", "Intro.\n\n## New Section", [{ start: 8, end: 22, author: "human" }], []); + expect(html).toMatch(/]*>/); // real heading element, not literal "## " + expect(html).toContain("cw-ins-human"); // author-colored insertion + expect(html).not.toContain(">>"); // sanity: markers not escaped as text + }); }); import { renderTrackChanges as rtc2 } from "../src/trackChangesModel";