From 4c1ee50dc9a01e1641686f6aadd788dff71b68ad Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sun, 5 Jul 2026 06:40:13 +0300 Subject: [PATCH] test(#351): close the mark-attr coverage hole + reclassify table spans (review round 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1 [WARNING] The 'no invisible coverage hole' guard enumerated only schema.nodes, so MARK attributes silently escaped the value-fuzz completeness check — link.internal/target/rel/class are never fuzzed and nothing flagged it, and a new attributed mark would slip through. Added allSchemaMarkAttrKeys() plus a MARK_ATTR_FUZZED / MARK_ATTR_ALLOWLIST registry and two tests: every schema mark attr must be in exactly one set (a new one turns it red), and neither set may hold a stale row. F2 [WARNING] The ACCEPTED annotation misclassified table colspan/rowspan as having 'no md representation'. They DO round-trip — a spanned cell makes the converter emit the whole table as a raw with colspan/rowspan, which the tiptap parser reads back. They are frozen only because generating a geometrically-valid spanned table is deferred PR-2 structural work (the flat generator hardcodes span = 1), not a markdown limit. Reclassified them as DEFERRED-BUG (distinct from ACCEPTED) so a maintainer does not read them as an inherent limitation; colwidth / backgroundColor(Name) stay ACCEPTED (the raw-
fallback drops them). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../test/generative/attr-arbitraries.ts | 57 ++++++++++++++----- .../flat-roundtrip.property.test.ts | 53 +++++++++++++++++ 2 files changed, 97 insertions(+), 13 deletions(-) diff --git a/packages/prosemirror-markdown/test/generative/attr-arbitraries.ts b/packages/prosemirror-markdown/test/generative/attr-arbitraries.ts index 50e66a5b..9fa7ccf1 100644 --- a/packages/prosemirror-markdown/test/generative/attr-arbitraries.ts +++ b/packages/prosemirror-markdown/test/generative/attr-arbitraries.ts @@ -31,12 +31,11 @@ * * (a) ACCEPTED LIMITATION — the attribute has NO markdown representation, * so the loss is inherent to targeting markdown, not a converter - * defect. GFM/CommonMark simply cannot encode it. These: `paragraph`/ - * `heading` `indent`, `callout.icon`, `orderedList.type` (a/A/i - * markers), table `colspan`/`rowspan`/`colwidth`/`backgroundColor(Name)` - * (GFM tables are span-less/style-less). Each is tagged - * `// ACCEPTED:` inline. Freezing them is correct — there is nothing to - * preserve in the target format. + * defect. These: `paragraph`/`heading` `indent`, `callout.icon`, + * `orderedList.type` (a/A/i markers), table `colwidth` / + * `backgroundColor(Name)` (dropped by the raw-
fallback). Each is + * tagged `// ACCEPTED:` inline. Freezing them is correct — there is + * nothing to preserve in the target format. * * (b) PINNED BUG — the attribute IS representable in markdown but the * converter drops it anyway (a real defect). These are NOT silently @@ -46,6 +45,13 @@ * on accept-vs-fix (the epic guardrail reserves that call). These: * `column.width` (parseFloat drops `%`), `orderedList.start` (non-1 * start renders as `1.`). Tagged `// PINNED-BUG:` inline. + * + * (c) DEFERRED-BUG — representable AND round-trips, frozen only because the + * flat generator can't yet build a valid instance. Table + * `colspan`/`rowspan` round-trip via the raw-
fallback, but a + * geometrically-valid spanned table is PR-2 structural work; the flat + * generator hardcodes span = 1. Tagged `// DEFERRED-BUG:` inline so a + * maintainer does not read them as an inherent limitation. * - Several non-null-default attrs are MATERIALIZED on import but are not * in canonicalize's KNOWN_DEFAULTS (`callout.type`, `status.color`, * table `colspan`/`rowspan`, `columns.layout`/`widthMode`, @@ -160,18 +166,26 @@ const OVERRIDES: Record = { 'callout.icon': { frozen: true }, // ACCEPTED: no md representation (dropped on export) 'status.text': { noDefault: true, arb: phraseArb, degen: '' }, 'status.color': { always: true, arb: str('green', 'orange', 'red', 'blue', 'yellow', 'purple') }, - // ── table cells — ACCEPTED: GFM tables cannot express spans / bg / colwidth ─ + // ── table cells ──────────────────────────────────────────────────────────── + // DEFERRED-BUG (not ACCEPTED): colspan/rowspan ARE representable and round-trip + // — a spanned cell makes the converter emit the whole table as a raw
+ // with colspan/rowspan attrs (markdown-converter.ts tableToHtml), which the + // tiptap parser reads back. They are frozen only because generating a + // geometrically-valid spanned table is deferred STRUCTURAL work (the flat + // generator hardcodes colspan/rowspan = 1), NOT a markdown limitation. 'tableCell.colspan': { always: true, frozen: true }, 'tableCell.rowspan': { always: true, frozen: true }, + // ACCEPTED: colwidth / backgroundColor(Name) have no representation — the + // raw-
fallback (tableToHtml) drops them, so there is nothing to preserve. 'tableCell.colwidth': { frozen: true }, 'tableCell.backgroundColor': { frozen: true }, 'tableCell.backgroundColorName': { frozen: true }, 'tableCell.align': { arb: str('left', 'center', 'right') }, - 'tableHeader.colspan': { always: true, frozen: true }, - 'tableHeader.rowspan': { always: true, frozen: true }, - 'tableHeader.colwidth': { frozen: true }, - 'tableHeader.backgroundColor': { frozen: true }, - 'tableHeader.backgroundColorName': { frozen: true }, + 'tableHeader.colspan': { always: true, frozen: true }, // DEFERRED-BUG (see tableCell.colspan) + 'tableHeader.rowspan': { always: true, frozen: true }, // DEFERRED-BUG (see tableCell.rowspan) + 'tableHeader.colwidth': { frozen: true }, // ACCEPTED: no representation + 'tableHeader.backgroundColor': { frozen: true }, // ACCEPTED: no representation + 'tableHeader.backgroundColorName': { frozen: true }, // ACCEPTED: no representation 'tableHeader.align': { arb: str('left', 'center', 'right') }, // ── details ────────────────────────────────────────────────────────────── 'details.open': { always: true, arb: fc.constant(true) }, // boolean, default false @@ -230,7 +244,7 @@ export function attrIsValueFuzzed(type: string, attr: string): boolean { return !!policyFor(type, attr, def).arb; } -/** Every `type.attr` in the schema (excluding the auto `id`), sorted. */ +/** Every node `type.attr` in the schema (excluding the auto `id`), sorted. */ export function allSchemaAttrKeys(): string[] { const keys: string[] = []; for (const type of Object.keys(schema.nodes)) { @@ -242,6 +256,23 @@ export function allSchemaAttrKeys(): string[] { return keys.sort(); } +/** + * Every MARK attribute in the schema, keyed `mark:.`, sorted. Marks + * are not driven by the node OVERRIDES table (they are fuzzed by the text + * generator, text-arbitraries.ts), so their value-fuzz coverage is tracked with a + * separate snapshot (see flat-roundtrip.property.test.ts) — without this the + * "no invisible coverage hole" guarantee would hold for node attrs only, letting a + * new mark attr slip through unfuzzed and unallowlisted. + */ +export function allSchemaMarkAttrKeys(): string[] { + const keys: string[] = []; + for (const [name, mark] of Object.entries(schema.marks)) { + const attrs = (mark.spec?.attrs ?? {}) as Record; + for (const attr of Object.keys(attrs)) keys.push(`mark:${name}.${attr}`); + } + return keys.sort(); +} + export type AttrMode = 'p1' | 'fuzz'; /** diff --git a/packages/prosemirror-markdown/test/generative/flat-roundtrip.property.test.ts b/packages/prosemirror-markdown/test/generative/flat-roundtrip.property.test.ts index 8543bc4a..46621537 100644 --- a/packages/prosemirror-markdown/test/generative/flat-roundtrip.property.test.ts +++ b/packages/prosemirror-markdown/test/generative/flat-roundtrip.property.test.ts @@ -10,6 +10,7 @@ import { firstDivergence } from '../roundtrip-helpers.js'; import { schema, allSchemaAttrKeys, + allSchemaMarkAttrKeys, attrIsValueFuzzed, } from './attr-arbitraries.js'; import { @@ -57,6 +58,33 @@ const ATTR_VALUE_FUZZ_ALLOWLIST = new Set([ 'youtube.align', 'youtube.height', 'youtube.width', ]); +// ── MARK attribute-value coverage ─────────────────────────────────────────── +// Marks are fuzzed by the text generator (text-arbitraries.ts markedTextRunArb), +// not the node OVERRIDES table, so their value-fuzz coverage is tracked with this +// separate registry — otherwise the "no invisible coverage hole" guarantee would +// hold for node attrs only, and a new mark attr (or a new attributed mark) would +// silently escape the fuzz set. Every schema mark attr must be in exactly one of: +// MARK_ATTR_FUZZED — actually driven at a non-default value by the generator; +// MARK_ATTR_ALLOWLIST — deliberately not value-fuzzed, with a reason. +const MARK_ATTR_FUZZED = new Set([ + 'mark:link.href', // markedTextRunArb sets a random webUrl href + 'mark:link.title', // ...and an optional letter-bearing title + 'mark:highlight.color', // highlight mark carries a generated color + 'mark:textStyle.color', // textStyle mark carries a generated color + 'mark:comment.commentId', // comment anchor id (alphanumeric token) + 'mark:comment.resolved', // comment resolved flag (rides only when true) +]); +const MARK_ATTR_ALLOWLIST = new Set([ + // link presentational/routing attrs: not part of the markdown link surface the + // converter emits (it round-trips href + title only), so there is no + // non-default value to assert here — a deferred concern for a link-specific + // fixture, not the flat generative pass. + 'mark:link.internal', + 'mark:link.target', + 'mark:link.rel', + 'mark:link.class', +]); + // Each run does a real convert + marked + jsdom parse (~ms). Give ample headroom // so the suite is deterministic regardless of parallel worker load (like the // sibling property file). @@ -176,6 +204,31 @@ describe('#351 flat generative round-trip — completeness contract', () => { ).toBe(true); } }); + + it('every MARK attribute is value-fuzzed OR allowlisted (no invisible hole)', () => { + // The node guard above covers node attrs; marks are fuzzed by the text + // generator, so their coverage is tracked separately. A new mark attr (or a + // newly-attributed mark) that lands in neither set turns this red. + const unaccounted: string[] = []; + for (const key of allSchemaMarkAttrKeys()) { + if (!MARK_ATTR_FUZZED.has(key) && !MARK_ATTR_ALLOWLIST.has(key)) { + unaccounted.push(key); + } + } + expect( + unaccounted, + `these mark attrs are neither in MARK_ATTR_FUZZED nor MARK_ATTR_ALLOWLIST:\n ${unaccounted.join( + '\n ', + )}`, + ).toEqual([]); + }); + + it('the MARK fuzz/allowlist sets have no stale rows (every entry is a real schema mark attr)', () => { + const all = new Set(allSchemaMarkAttrKeys()); + for (const key of [...MARK_ATTR_FUZZED, ...MARK_ATTR_ALLOWLIST]) { + expect(all.has(key), `stale mark-attr registry row: ${key}`).toBe(true); + } + }); }); describe('#351 flat generative round-trip — properties', () => {