#33: harden author-coloring PUA sentinels against intra-emphasis markdown
The F9/F10 author-coloring technique injects paired PUA sentinels into prose source at span offsets, renders, then maps sentinels → cw-by-* spans. Two failure modes when a span boundary met emphasis markup (characterized in session 0032): - CASE1: a boundary strictly inside a delimiter run (a**b**c, between the two *) split `**`, breaking markdown-it's delimiter pairing (stray <em></em>, raw **). - CASE3: a boundary inside an emphasis run (**bold** span covering **bo) rendered the emphasis but MISNESTED span/element (<strong>bo</span>ld</strong>). Token-aware fix (both, per the issue #33 comment): - injectSentinels: clamp any sentinel offset that lands strictly inside a delimiter run (* _ ~ `) to the run's start, so a sentinel never splits a run (CASE1). Delimiters are invisible once rendered, so this only shifts the colored boundary across markup. Drop spans that clamp to empty. - sentinelsToSpans: replace the naive global split/join with a walker over the rendered HTML that emits the cw-by-* span only around TEXT runs — closing it before any <tag> and reopening after — so a span is always well-nested within inline elements (one span segment per text run, CASE3). Tags are copied verbatim with any stray sentinel stripped (no Private-Use-Area char leaks). Pure, vscode-free, deterministic (INV-33). No regression: existing colorByAuthor / renderReview cases stay green; the common no-emphasis case is byte-identical. 222 unit + 74/5 host E2E green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -545,13 +545,33 @@ function authorBadge(authors: Set<AuthorKind>): { cls: string; label: string } |
|
|||||||
: { cls: "cw-by-human", label: "You" };
|
: { cls: "cw-by-human", label: "You" };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Markdown emphasis / code delimiters whose RUNS must never be split by an
|
||||||
|
// injected sentinel — a sentinel between two run chars (e.g. `*|*`) breaks
|
||||||
|
// markdown-it's delimiter pairing (#33 CASE1).
|
||||||
|
const isDelimChar = (c: string): boolean => c === "*" || c === "_" || c === "~" || c === "`";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* #33 (CASE1): a sentinel must not land STRICTLY INSIDE a delimiter run. If `at`
|
||||||
|
* sits between two identical delimiter chars, snap it to the run's start (a
|
||||||
|
* position outside the run) so the run stays intact. Delimiters are invisible once
|
||||||
|
* rendered, so snapping only shifts the colored boundary across markup, never over
|
||||||
|
* visible text.
|
||||||
|
*/
|
||||||
|
function clampOffDelimiterRun(raw: string, at: number): number {
|
||||||
|
if (at <= 0 || at >= raw.length) return at;
|
||||||
|
if (!(isDelimChar(raw[at]) && raw[at - 1] === raw[at])) return at;
|
||||||
|
let p = at;
|
||||||
|
while (p > 0 && raw[p - 1] === raw[at]) p--;
|
||||||
|
return p;
|
||||||
|
}
|
||||||
|
|
||||||
/** Inject paired sentinels into a prose block's raw text for the spans clipped to it. */
|
/** 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 {
|
function injectSentinels(raw: string, blockStart: number, spans: AuthorSpan[]): string {
|
||||||
const inserts: { at: number; marker: string }[] = [];
|
const inserts: { at: number; marker: string }[] = [];
|
||||||
for (const s of spans) {
|
for (const s of spans) {
|
||||||
const lo = Math.max(0, s.start - blockStart);
|
const lo = clampOffDelimiterRun(raw, Math.max(0, s.start - blockStart));
|
||||||
const hi = Math.min(raw.length, s.end - blockStart);
|
const hi = clampOffDelimiterRun(raw, Math.min(raw.length, s.end - blockStart));
|
||||||
if (hi <= lo) continue;
|
if (hi <= lo) continue; // empty (or clamped to empty) span contributes nothing
|
||||||
inserts.push({ at: lo, marker: SENT[s.author].open });
|
inserts.push({ at: lo, marker: SENT[s.author].open });
|
||||||
inserts.push({ at: hi, marker: SENT[s.author].close });
|
inserts.push({ at: hi, marker: SENT[s.author].close });
|
||||||
}
|
}
|
||||||
@@ -564,13 +584,69 @@ function injectSentinels(raw: string, blockStart: number, spans: AuthorSpan[]):
|
|||||||
return out;
|
return out;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Replace the rendered sentinels with author <span> tags. */
|
const SENTINEL_OF: Record<string, { author: AuthorKind; open: boolean } | undefined> = {
|
||||||
|
[SENT.claude.open]: { author: "claude", open: true },
|
||||||
|
[SENT.claude.close]: { author: "claude", open: false },
|
||||||
|
[SENT.human.open]: { author: "human", open: true },
|
||||||
|
[SENT.human.close]: { author: "human", open: false },
|
||||||
|
};
|
||||||
|
const ALL_SENTINELS = new RegExp(
|
||||||
|
`[${SENT.claude.open}${SENT.claude.close}${SENT.human.open}${SENT.human.close}]`,
|
||||||
|
"g",
|
||||||
|
);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* #33: token-aware replacement of the rendered author sentinels with `cw-by-*`
|
||||||
|
* spans. A naive global string-replace (the old approach) could emit a span that
|
||||||
|
* CROSSES an element boundary — `<span><strong>bo</span>ld</strong>` — when a
|
||||||
|
* boundary fell inside an emphasis run (CASE3). This walks the rendered HTML and
|
||||||
|
* emits the author span only around TEXT runs, CLOSING it before any `<tag>` and
|
||||||
|
* REOPENING it after, so a span is always well-nested within the inline elements
|
||||||
|
* (one `<span>` 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): string {
|
||||||
return html
|
const out: string[] = [];
|
||||||
.split(SENT.claude.open).join('<span class="cw-by-claude">')
|
let current: AuthorKind | null = null; // which author region we're inside
|
||||||
.split(SENT.claude.close).join("</span>")
|
let spanOpen = false; // whether a <span> is currently open in `out`
|
||||||
.split(SENT.human.open).join('<span class="cw-by-human">')
|
const openSpan = () => {
|
||||||
.split(SENT.human.close).join("</span>");
|
if (current && !spanOpen) {
|
||||||
|
out.push(`<span class="cw-by-${current}">`);
|
||||||
|
spanOpen = true;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
const closeSpan = () => {
|
||||||
|
if (spanOpen) {
|
||||||
|
out.push("</span>");
|
||||||
|
spanOpen = false;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
for (let i = 0; i < html.length; i++) {
|
||||||
|
const ch = html[i];
|
||||||
|
const sentinel = SENTINEL_OF[ch];
|
||||||
|
if (sentinel) {
|
||||||
|
if (sentinel.open) current = sentinel.author; // span opens lazily before the next text char
|
||||||
|
else {
|
||||||
|
closeSpan();
|
||||||
|
current = null;
|
||||||
|
}
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (ch === "<") {
|
||||||
|
// An HTML tag: never let an author span straddle it (text `<` is escaped to
|
||||||
|
// <, so a raw `<` is always a real tag). Copy the tag verbatim, sentinel-free.
|
||||||
|
closeSpan();
|
||||||
|
const gt = html.indexOf(">", i);
|
||||||
|
const end = gt === -1 ? html.length - 1 : gt;
|
||||||
|
out.push(html.slice(i, end + 1).replace(ALL_SENTINELS, ""));
|
||||||
|
i = end;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
openSpan();
|
||||||
|
out.push(ch);
|
||||||
|
}
|
||||||
|
closeSpan();
|
||||||
|
return out.join("");
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -168,6 +168,45 @@ describe("colorByAuthor", () => {
|
|||||||
expect(html).toContain('<span class="cw-by-human">hello</span>');
|
expect(html).toContain('<span class="cw-by-human">hello</span>');
|
||||||
expect(html).toContain("world");
|
expect(html).toContain("world");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// #33: author-coloring must be sentinel-safe around markdown emphasis. These
|
||||||
|
// exercise the REAL markdown-it renderer (via renderReview on an unchanged doc),
|
||||||
|
// since the failure is in markdown-it's inline parsing / element nesting.
|
||||||
|
// CASE1 — a span boundary strictly inside a delimiter run must not split it.
|
||||||
|
test("#33 CASE1: a span boundary inside ** does not break emphasis parsing", () => {
|
||||||
|
const doc = "a**b**c";
|
||||||
|
const html = renderReview(doc, doc, [{ start: 0, end: 2, author: "human" }], []);
|
||||||
|
expect(html).toContain("<strong>b</strong>"); // emphasis still renders
|
||||||
|
expect(html).not.toContain("**"); // no raw delimiters left
|
||||||
|
expect(html).not.toContain("<em></em>"); // no stray empty emphasis (the parse-break symptom)
|
||||||
|
expect(html).toContain('class="cw-by-human"'); // coloring present
|
||||||
|
});
|
||||||
|
// CASE3 — a span boundary inside an emphasis run must color the text without
|
||||||
|
// misnesting span/element (the span is split at the element boundary).
|
||||||
|
test("#33 CASE3: a span boundary inside **bold** colors the text without misnesting", () => {
|
||||||
|
const doc = "**bold**";
|
||||||
|
const html = renderReview(doc, doc, [{ start: 0, end: 4, author: "human" }], []); // covers "**bo"
|
||||||
|
expect(html).toContain("<strong>");
|
||||||
|
expect(html).toContain('<span class="cw-by-human">bo</span>ld</strong>'); // span INSIDE strong, closed before "ld"
|
||||||
|
expect(html).not.toContain('cw-by-human"><strong>'); // NOT the old misnest (span wrapping the <strong> open)
|
||||||
|
});
|
||||||
|
// CASE2 — a span covering a whole emphasis run stays correct (regression).
|
||||||
|
test("#33 CASE2: a span over the whole **bold** colors it correctly", () => {
|
||||||
|
const doc = "**bold**";
|
||||||
|
const html = renderReview(doc, doc, [{ start: 0, end: 8, author: "human" }], []);
|
||||||
|
expect(html).toContain("<strong>");
|
||||||
|
expect((html.match(/cw-by-human/g) ?? []).length).toBe(1);
|
||||||
|
expect(html).toContain("bold");
|
||||||
|
});
|
||||||
|
test("#33: no Private-Use-Area sentinel chars leak into the rendered output", () => {
|
||||||
|
const doc = "a**b**c and `co de` and _x_";
|
||||||
|
const spans: AuthorSpan[] = [
|
||||||
|
{ start: 0, end: 2, author: "human" },
|
||||||
|
{ start: 12, end: 16, author: "claude" },
|
||||||
|
];
|
||||||
|
const html = renderReview(doc, doc, spans, []);
|
||||||
|
expect(html).not.toMatch(/[\uE000-\uF8FF]/); // no leftover BMP Private-Use-Area sentinels
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
import { renderPlain } from "../src/trackChangesModel";
|
import { renderPlain } from "../src/trackChangesModel";
|
||||||
|
|||||||
Reference in New Issue
Block a user