From e2a3b5fc4dc5b159842c22e6a2a9248d25b43521 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Sat, 4 Jul 2026 08:52:02 +0300 Subject: [PATCH] feat(prosemirror-markdown): media family as md-form + discriminator comment (#293 canon #8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ten media/embed node types move their TOP-LEVEL serialization off raw schema HTML onto a readable markdown target plus an always-emitted discriminator comment whose NAME selects the node type. The schema-HTML form is retained on the raw-HTML/columns path (comments are dropped by the DOM parse stage there). image-form ![](src) youtube, video, audio, drawio, excalidraw link-form [text](src) pdf, attachment, embed (text=filename/provider) standalone / pageEmbed, transclusionReference The comment NAME is the node-type discriminator and is ALWAYS emitted, even when the attr JSON is empty (`![](u)`), so a bare `![](u)` is never mistaken for an `image` and a bare `[t](u)` stays a plain link — no URL-sniffing. src rides in the markdown target; every other non-default attr (incl. the id links attachmentId/sourcePageId/transclusionId) rides in the comment JSON (stable key order, numerics stringified, align="center" omitted). New src/lib/media-html.ts: byte-exact builders reproducing the schema HTML each old processNode case returned. Both the serializer's raw-HTML path (blockToHtml, now de-delegated from `return processNode(block)` to explicit per-type cases) and the importer call these, so serialize and parse cannot drift. Import (applyCommentDirectives): image-form binds the preceding (src from it), link-form the preceding (src=href, text=filename/provider), standalone replaces the comment (same leading-doc-level handling as #5). Each rebuilds the schema element via the media-html builder, then swaps it in; the empty-

hoist is absorbed by stripEmptyParagraphs. Fail-open: wrong element/position/name or malformed JSON -> inert, no throw. Link-form visible text is escaped (escapeLinkText) for the FULL set of CommonMark inline-active punctuation (\ ` * _ ~ [ ] < & ! ( )), not just [ ] \: the label is parsed as inline content, so a filename/provider like `report *v2*.pdf` or `![shot](x).pdf` would otherwise lose the markup (or fragment the parse) when the importer reads a.textContent back — a data-loss regression vs the old data-attachment-name form. Adversarial round-trip fixtures lock byte- and value-stability for emphasis/code/strike/autolink/entity/image markers and nested-link names. Tests: new media-comments.test.ts (40 cases: per-type exact md + lossless byte-stable round-trip incl. id links, minimal-node discriminator-still-emitted, in-column schema-HTML form, discriminator integrity, fail-open, active-punct filenames). Goldens in media-roundtrip / markdown-converter-golden / markdown-converter / diagram-roundtrip updated to the md+comment form (columns stay schema-HTML). The former known-limitation image-diagrams fixture is now byte- AND canonically-stable (canon #8 omits the diagram align="center" default) and was promoted from an it.fails into the green corpus (11-image-diagrams.json). git-sync stabilize.test.ts: the "diagram materializes data-align=center" fixpoint moved into a column (where the raw-HTML asymmetry still holds), since top level is now byte-stable. package vitest: 540 passed; tsc clean. git-sync: 268 passed. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/git-sync/test/stabilize.test.ts | 21 +- .../src/lib/markdown-converter.ts | 324 ++++++------ .../src/lib/markdown-to-prosemirror.ts | 150 +++++- .../src/lib/media-html.ts | 172 +++++++ .../test/diagram-roundtrip.test.ts | 40 +- .../11-image-diagrams.json} | 0 .../test/markdown-converter-golden.test.ts | 87 ++-- .../test/markdown-converter.test.ts | 19 +- .../test/media-comments.test.ts | 474 ++++++++++++++++++ .../test/media-roundtrip.test.ts | 79 +-- .../test/roundtrip-corpus.test.ts | 64 +-- 11 files changed, 1113 insertions(+), 317 deletions(-) create mode 100644 packages/prosemirror-markdown/src/lib/media-html.ts rename packages/prosemirror-markdown/test/fixtures/{known-limitations/image-diagrams.json => corpus/11-image-diagrams.json} (100%) create mode 100644 packages/prosemirror-markdown/test/media-comments.test.ts diff --git a/packages/git-sync/test/stabilize.test.ts b/packages/git-sync/test/stabilize.test.ts index 8a70e1b9..d5303e9f 100644 --- a/packages/git-sync/test/stabilize.test.ts +++ b/packages/git-sync/test/stabilize.test.ts @@ -22,16 +22,27 @@ const meta: PageMeta = { describe('stabilizePageFile — normalize-on-write fixpoint (SPEC §11)', () => { it('reaches a byte-identical fixpoint after one extra export/import/export pass', async () => { - // A diagram is the canonical one-pass asymmetry: drawio's `align` default of - // "center" materializes on import, so a NAIVE export differs on the second - // export. stabilizePageFile runs the convergence pass at write time, so the - // written body must already be at the fixpoint: re-importing its body and + // A diagram inside a column is the canonical one-pass asymmetry: on the + // raw-HTML/columns path a diagram's `align` default of "center" materializes + // on import, so a NAIVE export differs on the second export. (#293 canon #8 + // made the TOP-LEVEL diagram form — `![](src)` — byte-stable by + // omitting the default, so the asymmetry now lives only on the columns path + // where the schema `

` form is retained.) + // stabilizePageFile runs the convergence pass at write time, so the written + // body must already be at the fixpoint: re-importing its body and // re-stabilizing yields the exact same bytes. const content = { type: 'doc', content: [ { type: 'paragraph', content: [{ type: 'text', text: 'intro' }] }, - { type: 'drawio', attrs: { src: '/d.drawio' } }, + { + type: 'columns', + attrs: { layout: 'two_equal' }, + content: [ + { type: 'column', content: [{ type: 'drawio', attrs: { src: '/d.drawio' } }] }, + { type: 'column', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'side' }] }] }, + ], + }, { type: 'paragraph', content: [{ type: 'text', text: 'outro' }] }, ], }; diff --git a/packages/prosemirror-markdown/src/lib/markdown-converter.ts b/packages/prosemirror-markdown/src/lib/markdown-converter.ts index 60260bfa..17ab21db 100644 --- a/packages/prosemirror-markdown/src/lib/markdown-converter.ts +++ b/packages/prosemirror-markdown/src/lib/markdown-converter.ts @@ -3,6 +3,17 @@ import { attachedCommentFor, standaloneCommentFor, } from "./attached-comment.js"; +import { + attachmentToHtml, + audioToHtml, + diagramToHtml, + embedToHtml, + pageEmbedToHtml, + pdfToHtml, + transclusionReferenceToHtml, + videoToHtml, + youtubeToHtml, +} from "./media-html.js"; /** * Hard cap on processNode recursion depth (see the depth guard below). @@ -62,6 +73,24 @@ export function convertProseMirrorToMarkdown(content: any): string { .replace(/\(/g, "%28") .replace(/\)/g, "%29"); + // Backslash-escape every character a markdown link's `[text]` label would + // otherwise INTERPRET, so a link-form media node's visible text (a filename or + // provider carried in `attrs.name`/`attrs.provider`) round-trips byte-exact + // (#293 canon #8 link-form). Escaping only `[ ] \` is NOT enough: the label is + // parsed as inline content, so emphasis (`* _`), code (`` ` ``), strikethrough + // (`~`), autolinks/raw-HTML (`<`), HTML entities (`&`), and image markers (`!`) + // would all be consumed and lost when the importer reads `a.textContent` back + // (e.g. `report *v2*.pdf` -> `report v2.pdf`). CommonMark treats a backslash + // before ANY ASCII punctuation as that literal char, so escaping this active + // set is always lossless on re-parse; the old `
` + // form carried arbitrary strings via escapeAttr, so anything less is a + // data-loss regression on the git-sync data path. `( )` are escaped too: even + // with `[ ]` escaped, an unescaped `](x)` sequence inside the label (e.g. a + // name like `![shot](x).pdf`) forms a false nested-link destination and + // fragments the parse — escaping the parens removes that ambiguity entirely. + const escapeLinkText = (value: unknown): string => + String(value ?? "").replace(/[\\`*_~[\]<&!()]/g, (c: string) => `\\${c}`); + // Recursion depth guard. processNode is mutually recursive (directly and via // processListItem/processTaskItem/blockToHtml), and a pathologically nested // document (e.g. tens of thousands of nested blockquotes) would otherwise @@ -410,48 +439,44 @@ export function convertProseMirrorToMarkdown(content: any): string { } case "video": { - // Emit the schema-matching
so marked treats it as a block (a bare
for the same reason as video: a bare
` / `
`) * that the schema's parseHTML rebuilds into the atom. + * - MEDIA DISCRIMINATOR comments (#8): the comment NAME selects the node type. + * IMAGE-FORM (`youtube`/`video`/`audio`/`drawio`/`excalidraw`) binds to the + * preceding `` (`![](src)`); LINK-FORM (`pdf`/`attachment`/ + * `embed`) binds to the preceding `` (`[text](src)`); + * STANDALONE (`pageembed`/`transclusion`) is a lone comment line. Each is + * re-expressed as the SAME schema HTML the serializer's raw-HTML path emits + * (media-html.ts) — the img's `src`/the anchor's `href`+text plus the decoded + * comment attrs — then swapped in for the ``/``/comment. A bare + * `![](url)`/`[text](src)` with NO following discriminator stays an `image`/ + * plain link (never sniffed by URL). * * Position determines legality: an `attrs` comment is honored only in attached - * position, a `subpages`/`pagebreak` comment only in standalone position; a - * comment in the wrong position is left INERT (generateJSON drops it). Fail-open - * everywhere: a malformed comment (null from parseAttachedComment), an unknown - * name, a wrong-position comment, or an unknown/empty attr value is ignored. - * Future decisions (#4/#8) extend the per-name handling here without changing - * the parse primitive. + * position, `subpages`/`pagebreak`/`pageembed`/`transclusion` only in standalone + * position, an image-form comment only next to an `` and a link-form comment + * only next to an ``; a comment in the wrong position/next to the wrong element + * is left INERT (generateJSON drops it). Fail-open everywhere: a malformed comment + * (null from parseAttachedComment), an unknown name, a wrong-position comment, or + * an unknown/empty attr value is ignored. */ function applyCommentDirectives(html: string): string { // Cheap early-out: no comments at all -> nothing to intercept. @@ -377,6 +398,43 @@ function applyCommentDirectives(html: string): string { // comments at document level, prepending them to body preserves global order. const leadingDivs: any[] = []; + // #293 canon #8 discriminator NAME -> node form. The comment NAME alone selects + // the node type; a bare `![](url)`/`[text](src)` with NO following comment is an + // `image`/plain link (never sniffed). These are materialized below by rebuilding + // the SAME schema HTML the serializer's raw-HTML path emits (media-html.ts), so + // serialize and parse cannot drift. + const IMAGE_FORM_NAMES = new Set([ + "youtube", + "video", + "audio", + "drawio", + "excalidraw", + ]); + const LINK_FORM_NAMES = new Set(["pdf", "attachment", "embed"]); + + // Parse a schema-HTML string (from a media-html builder) into its top-level + // element so it can be swapped in for the //comment it replaces. + const buildElement = (htmlStr: string): any => { + const tmp = document.createElement("div"); + tmp.innerHTML = htmlStr; + return tmp.firstElementChild; + }; + + // Build the image-form schema HTML for a given discriminator name, using the + // 's src as the node src plus the decoded comment attrs. + const imageFormHtml = (name: string, attrs: Record): string => { + switch (name) { + case "video": + return videoToHtml(attrs); + case "audio": + return audioToHtml(attrs); + case "youtube": + return youtubeToHtml(attrs); + default: // drawio | excalidraw + return diagramToHtml(name as "drawio" | "excalidraw", attrs); + } + }; + for (const comment of comments) { const parsed = parseAttachedComment(comment.data); if (!parsed) continue; // malformed -> inert (dropped by generateJSON) @@ -413,6 +471,86 @@ function applyCommentDirectives(html: string): string { continue; } + if (parsed.name === "pageembed" || parsed.name === "transclusion") { + // #293 canon #8 STANDALONE media. Like subpages/pagebreak: a lone comment + // line placed under or at document level (leading). An attached- + // position comment (inside a

/ with a sibling) is INERT. We rebuild + // the schema div the raw-HTML path emits (media-html.ts) from the decoded + // attrs so serialize/parse stay in sync. + const standalone = tag === "" || tag === "body" || tag === "html"; + if (!standalone) continue; // wrong position -> inert + const el = buildElement( + parsed.name === "pageembed" + ? pageEmbedToHtml({ sourcePageId: parsed.attrs.sourcePageId }) + : transclusionReferenceToHtml({ + sourcePageId: parsed.attrs.sourcePageId, + transclusionId: parsed.attrs.transclusionId, + }), + ); + if (!el) continue; // defensive: builder always yields an element + if (tag === "body") { + comment.replaceWith(el); + } else { + comment.remove(); + leadingDivs.push(el); + } + continue; + } + + if (IMAGE_FORM_NAMES.has(parsed.name)) { + // #293 canon #8 IMAGE-FORM media (youtube/video/audio/drawio/excalidraw). + // `![](src)` renders as `

`, so + // the target is the comment's previous element sibling and it MUST be an + // . We rebuild the schema element from the img's src + decoded attrs + // and swap it in for the . No adjacent -> INERT. + const prev = comment.previousElementSibling as any; + const target = + prev && String(prev.tagName || "").toLowerCase() === "img" + ? prev + : null; + if (!target) continue; // no adjacent -> inert + const attrs = { ...parsed.attrs, src: target.getAttribute("src") || "" }; + const el = buildElement(imageFormHtml(parsed.name, attrs)); + if (!el) continue; + target.replaceWith(el); + comment.remove(); + continue; + } + + if (LINK_FORM_NAMES.has(parsed.name)) { + // #293 canon #8 LINK-FORM media (pdf/attachment/embed). + // `[text](src)` renders as `

text + //

`, so the target is the previous element sibling and it + // MUST be an . src = a.href; the visible text is the filename/provider. + // Not an -> INERT. + const prev = comment.previousElementSibling as any; + const target = + prev && String(prev.tagName || "").toLowerCase() === "a" ? prev : null; + if (!target) continue; // no adjacent -> inert + const src = target.getAttribute("href") || ""; + const text = target.textContent || ""; + let htmlStr: string; + if (parsed.name === "pdf") { + // pdf: src standard attr, filename in data-name (null when empty). + htmlStr = pdfToHtml({ ...parsed.attrs, src, name: text || null }); + } else if (parsed.name === "attachment") { + // attachment: the schema field is `url`, filename in data-attachment-name. + htmlStr = attachmentToHtml({ + ...parsed.attrs, + url: src, + name: text || null, + }); + } else { + // embed: the visible text is the provider (schema default ""). + htmlStr = embedToHtml({ ...parsed.attrs, src, provider: text }); + } + const el = buildElement(htmlStr); + if (!el) continue; + target.replaceWith(el); + comment.remove(); + continue; + } + if (parsed.name === "img") { // #293 canon #4 ATTACHED image attrs. `![](src) ` renders // as `

`, so the comment's target is the nearest diff --git a/packages/prosemirror-markdown/src/lib/media-html.ts b/packages/prosemirror-markdown/src/lib/media-html.ts new file mode 100644 index 00000000..733f5aed --- /dev/null +++ b/packages/prosemirror-markdown/src/lib/media-html.ts @@ -0,0 +1,172 @@ +/** + * Shared schema-HTML builders for the media/discriminator family (#293 canon + * #8). + * + * Canon #8 gives ten node types (youtube/video/audio/drawio/excalidraw — + * image-form; pdf/attachment/embed — link-form; pageEmbed/transclusionReference + * — standalone) a readable markdown TOP-LEVEL form (`![](src)`/`[text](src)`/a + * bare comment) plus a discriminator `` comment. But TWO other + * paths still need the RAW SCHEMA-HTML form of each node: + * + * 1. The serializer's raw-HTML/columns path (`blockToHtml`): a comment node is + * dropped by the DOM parse stage that reads a raw-HTML block back, so inside + * a column/cell these nodes MUST stay schema HTML or they vanish (data loss). + * 2. The importer's `applyCommentDirectives`: to materialize the discriminator + * comment it rebuilds the SAME schema element the raw-HTML path emits, then + * swaps it in for the ``/``/comment. + * + * Keeping these builders in ONE module means the serializer's raw-HTML path and + * the importer's materialization can never drift: both call the same function. + * Each builder reproduces BYTE-FOR-BYTE the schema HTML the top-level + * `processNode` cases previously returned (so existing columns/raw-HTML goldens + * stay green), and each output round-trips through the matching schema parseHTML + * in docmost-schema.ts. + */ + +/** + * Escape a value interpolated into an HTML double-quoted attribute value. + * Identical semantics to markdown-converter's `escapeAttr`: escape ONLY `&` and + * `"` (idempotent; parse5 decodes both back). `<`/`>`/`'` are deliberately left + * alone so values never accumulate escapes across round-trips. + */ +const escapeAttr = (value: unknown): string => + String(value).replace(/&/g, "&").replace(/"/g, """); + +/** + * Uploaded `