benstull/vscode-cowriting-plugin
1
0
Fork 0
Code Issues 9 Pull Requests Actions Packages Projects Releases Wiki Activity

fix: mermaid fence queue matches by body — misalignment degrades to verbatim (INV-29 hardening)

Browse Source 
Code review of 705de31's mermaid-diff wiring: buildMermaidQueue's overrides
were consumed positionally per rendered mermaid fence, so a fence the queue
didn't know about (a pending proposal's optimistic apply inserting a whole
new mermaid block, or annotateSource's committed-deletion reinsert) shifted
every later override by one and substituted the wrong diagram's augmented
source instead of the documented verbatim fallback. Queue entries now carry
{curBody, source} and are matched to fences by body (first unconsumed match,
scanning forward) so a non-matching fence renders verbatim without consuming
anything and a later matching fence still gets its augmentation. Also resets
the per-render consumed state in the renderer.render wrapper, since VS Code
can call md.parse()/renderer.render() separately and replay cached tokens
without a fresh parse.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
BenStullsBets
2026-07-02 15:21:00 -07:00
parent 705de3169b
commit ff42b5a539
2 changed files with 114 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/previewAnnotations.ts
+69 -43
View File
@@ -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 `<pre class="mermaid" style="all: unset;">${escapeMermaidSource(override ?? code)}</pre>${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 `<pre class="mermaid" style="all: unset;">${escapeMermaidSource(entry?.source ?? code)}</pre>${legend}`;
}
return baseHighlight?.(code, lang, attrs) ?? md.utils.escapeHtml(code);
};
test/previewAnnotations.test.ts
+45
View File
@@ -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('<pre class="mermaid"').slice(1); // one chunk per fence, document order
expect(fences).toHaveLength(2);
// the extra fence has no matching queue entry: verbatim, no wrong-diagram substitution
expect(fences[0]).toContain("sequenceDiagram");
expect(fences[0]).not.toContain("cwAdded");
// the changed fence still finds its own augmentation despite the extra fence ahead of it in document order
expect(fences[1]).toContain("classDef cwAdded");
expect(fences[1]).toMatch(/class\s+c\s+cwAdded/);
expect((html.match(/cw-mermaid-legend/g) ?? []).length).toBe(1); // legend only on the augmented fence
});
// Body-match must not collapse two changed fences that happen to render the
// SAME current diagram into a single queue entry — each occurrence gets its
// own independent augmentation (consume-first-match, not a body->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('<pre class="mermaid"').slice(1);
expect(fences).toHaveLength(2);
for (const f of fences) {
expect(f).toContain("classDef cwAdded");
expect(f).toMatch(/class\s+c\s+cwAdded/);
}
expect((html.match(/cw-mermaid-legend/g) ?? []).length).toBe(2);
});
});