diff --git a/src/previewAnnotations.ts b/src/previewAnnotations.ts index 675dd4d..413c0d4 100644 --- a/src/previewAnnotations.ts +++ b/src/previewAnnotations.ts @@ -155,50 +155,64 @@ export function annotateSource(src: string, inputs: AnnotationInputs): string { return injectAllSentinels(text, marks); } +/** One `buildMermaidQueue` entry: a changed mermaid fence's own body (for + * matching, `mermaidFenceBody` normalization — no trailing newline) paired + * with its `diffMermaid`-augmented replacement source. `consumed` is reset by + * the `render` wrapper (see `cowritingMarkdownItPlugin`) so the SAME entries + * can back a re-render without a fresh parse. */ +interface MermaidQueueEntry { + readonly curBody: string; + readonly source: string; + consumed: boolean; +} + /** - * Per-render queue of mermaid-fence overrides, one entry per ```mermaid fence - * the CURRENT (landed) text contains, in document order — the same order - * `options.highlight` encounters them while markdown-it renders fence tokens - * (Task 7 §2.6 parity: reuses `diffBlocks`' baseline/current block pairing, - * the SAME pairing `renderReview`/`renderOp` used in the sunset webview, so a - * changed mermaid fence is re-emitted through `diffMermaid` exactly as it was - * there — INV-23 fences stay atomic, INV-29..31 styling directives). An entry - * is the augmented source string when the fence is a `changed` atomic mermaid - * op with a diffable before/current pair, else `null` (unchanged/added fence, - * or a fallback pair `diffMermaid` couldn't diff — render the fence verbatim). - * `[]` when there is no baseline to diff against, annotations are disabled, or - * the landed text equals the baseline (covers the #48 pinned-zero-diff case - * too, since `diffBlocks` on identical text yields no `changed` ops). + * Per-render queue of mermaid-fence overrides: one entry per `changed` atomic + * mermaid op (Task 7 §2.6 parity: reuses `diffBlocks`' baseline/current block + * pairing, the SAME pairing `renderReview`/`renderOp` used in the sunset + * webview, so a changed mermaid fence is re-emitted through `diffMermaid` + * exactly as it was there — INV-23 fences stay atomic, INV-29..31 styling + * directives) that has a diffable before/current pair AND a successful + * `diffMermaid` augmentation. `[]` when there is no baseline to diff against, + * annotations are disabled, or the landed text equals the baseline (covers + * the #48 pinned-zero-diff case too, since `diffBlocks` on identical text + * yields no `changed` ops). * * Diffed against `landedText` (proposals reverted), matching the "committed * change since baseline" boundary `annotateSource`'s own `wordEditHunks(src, * baselineText)` gate already draws (a pending, not-yet-accepted proposal * isn't a landed change) — a fence a pending proposal's anchor overlaps is - * left unaugmented, same as the rest of that gate. This assumes fence COUNT - * and ORDER match between `src` (what markdown-it actually parses fences - * from) and `landedText` (what this function diffs) — true whenever no - * pending proposal adds/removes a whole mermaid block; a rare edge case - * accepted as a known scope boundary (mirrors the built-in preview's other - * documented Q4/scope notes), not something the built-in preview's - * proposal-block accounting handles elsewhere either. + * left unaugmented, same as the rest of that gate. + * + * NOT positional: `src` (what markdown-it actually parses fences from) and + * `landedText` (what this function diffs) can disagree on mermaid-fence COUNT + * or ORDER — e.g. a pending proposal optimistically applied in `src` (F12, + * INV-48) inserts a whole new mermaid fence that `landedText` (proposals + * reverted) doesn't have, or `annotateSource`'s committed-deletion reinsert + * splices an extra fence into the rendered source. `options.highlight` + * (`cowritingMarkdownItPlugin`) matches each queue entry to a fence by BODY, + * not by position — a fence with no matching entry renders verbatim (the + * queue is a bag of `changed` fences to find, not a strict per-fence + * override list), so misalignment degrades to verbatim exactly as intended, + * never substitutes the wrong diagram's augmentation into the wrong fence. * * Pure, vscode-free (INV-6). */ -function buildMermaidQueue(src: string, inputs: AnnotationInputs): (string | null)[] { +function buildMermaidQueue(src: string, inputs: AnnotationInputs): MermaidQueueEntry[] { if (!inputs.enabled || inputs.baselineText === undefined) return []; const baselineText = inputs.baselineText; const landedText = landedTextOf(src, inputs.proposals ?? []); if (landedText === baselineText) return []; - return diffBlocks(baselineText, landedText) - .filter((op) => op.kind !== "removed" && op.block.type === "mermaid") - .map((op) => { - if (op.kind !== "changed") return null; // unchanged/added: render verbatim - const curBody = mermaidFenceBody(op.block.raw); - const beforeBody = op.before.type === "mermaid" ? mermaidFenceBody(op.before.raw) : null; - if (curBody === null || beforeBody === null) return null; - const result = diffMermaid(beforeBody, curBody); - return result.kind === "augmented" ? result.source : null; // INV-30 fallback: verbatim - }); + const entries: MermaidQueueEntry[] = []; + for (const op of diffBlocks(baselineText, landedText)) { + if (op.kind !== "changed" || op.block.type !== "mermaid") continue; // unchanged/added/removed: no override needed + const curBody = mermaidFenceBody(op.block.raw); + const beforeBody = op.before.type === "mermaid" ? mermaidFenceBody(op.before.raw) : null; + if (curBody === null || beforeBody === null) continue; + const result = diffMermaid(beforeBody, curBody); + if (result.kind === "augmented") entries.push({ curBody, source: result.source, consumed: false }); // INV-30 fallback: verbatim + } + return entries; } /** @@ -217,36 +231,48 @@ function buildMermaidQueue(src: string, inputs: AnnotationInputs): (string | nul * diagram diff (INV-29..31), not just the current diagram. `options.highlight` * has no `env`/`state` parameter of its own, so the per-render queue is * threaded through a closure variable set by the core rule (which DOES see - * `state.env`) and consumed in fence-encounter order by `highlight` — - * markdown-it's `parse` (core rules) always completes before `renderer.render` - * invokes `highlight`, and a single md instance never renders concurrently, so - * this is safe. + * `state.env`) and consumed BY BODY MATCH, not fence position (see + * `buildMermaidQueue`), by `highlight`: each fence's `code` is matched against + * unconsumed queue entries in order, the first match is consumed (so a later + * fence with the same body still finds its own entry), and a fence with no + * match renders verbatim WITHOUT consuming anything (so a later fence can + * still match). markdown-it's `parse` (core rules) always completes before + * `renderer.render` invokes `highlight`, and a single md instance never + * renders concurrently, so the closure handoff itself is safe — but VS Code + * may call `md.parse()` and `md.renderer.render()` separately and re-render + * cached tokens without a fresh parse (the same caller pattern the module + * docstring notes for scroll-sync), which would otherwise replay a queue whose + * entries were already marked consumed by the prior render; the `render` + * wrapper resets every entry's `consumed` flag so each render pass starts with + * the full queue available again. */ export function cowritingMarkdownItPlugin( md: MarkdownIt, host: { inputsFor(env: unknown): AnnotationInputs | undefined }, ): MarkdownIt { - let mermaidQueue: (string | null)[] = []; - let mermaidIdx = 0; + let mermaidQueue: MermaidQueueEntry[] = []; md.core.ruler.before("normalize", "cowriting-annotate", (state) => { const inputs = host.inputsFor(state.env); mermaidQueue = inputs ? buildMermaidQueue(state.src, inputs) : []; - mermaidIdx = 0; if (!inputs) return; state.src = annotateSource(state.src, inputs); }); const baseRender = md.renderer.render.bind(md.renderer); - md.renderer.render = (tokens, options, env) => sentinelsToSpans(baseRender(tokens, options, env), "ins"); + md.renderer.render = (tokens, options, env) => { + for (const entry of mermaidQueue) entry.consumed = false; // re-render-without-reparse safety net + return sentinelsToSpans(baseRender(tokens, options, env), "ins"); + }; const baseHighlight = md.options.highlight; md.options.highlight = (code, lang, attrs) => { if (lang?.trim().toLowerCase() === "mermaid") { - const override = mermaidIdx < mermaidQueue.length ? mermaidQueue[mermaidIdx] : null; - mermaidIdx++; - const legend = override !== null ? MERMAID_LEGEND : ""; - return `
${escapeMermaidSource(override ?? code)}
${legend}`; + const normalized = code.replace(/\n+$/, ""); // token.content keeps a trailing LF; mermaidFenceBody doesn't + const entry = mermaidQueue.find((e) => !e.consumed && e.curBody === normalized); + if (entry) entry.consumed = true; + const legend = entry ? MERMAID_LEGEND : ""; + return `
${escapeMermaidSource(entry?.source ?? code)}
${legend}`; } return baseHighlight?.(code, lang, attrs) ?? md.utils.escapeHtml(code); }; diff --git a/test/previewAnnotations.test.ts b/test/previewAnnotations.test.ts index ee71f95..7863dd1 100644 --- a/test/previewAnnotations.test.ts +++ b/test/previewAnnotations.test.ts @@ -131,4 +131,49 @@ describe("intra-diagram mermaid diff in the built-in preview (plan T7 §2.6, INV expect(html).not.toContain("cw-mermaid-legend"); expect(html).toContain('class="language-js"'); }); + + // Code-review hardening (705de31 follow-up): the queue used to be consumed + // POSITIONALLY per mermaid fence encountered, so a fence the queue doesn't + // know about (e.g. a pending proposal's optimistic apply, F12/INV-48, + // inserting a whole new mermaid fence ABOVE a changed one) shifted every + // later fence's override by one, substituting the wrong diagram's augmented + // source into the wrong fence instead of the documented verbatim fallback. + // The fix matches queue entries to fences by BODY: a fence with no matching + // entry renders verbatim without consuming anything, so a later matching + // fence still finds its own entry. + it("misalignment: an extra proposal-inserted mermaid fence above a changed one renders verbatim, and the changed fence still gets its own augmentation", () => { + const extraFence = "```mermaid\nsequenceDiagram\n A->>B: hi\n```\n"; + const src = extraFence + "\n" + flowchartAfter; + const html = render(src, { + ...base, + baselineText: flowchartBefore, + spans: [], + proposals: [{ id: "p1", anchorStart: 0, anchorEnd: extraFence.length, replaced: "", replacement: extraFence }], + }); + const fences = html.split('
entry map).
+  it("two identical-body changed mermaid fences each get their own augmentation", () => {
+    const baselineText = flowchartBefore + "\n" + flowchartBefore;
+    const src = flowchartAfter + "\n" + flowchartAfter;
+    const html = render(src, { ...base, baselineText, spans: [] });
+    const fences = html.split('