test(#351): close the mark-attr coverage hole + reclassify table spans (review round 1)
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 <table> 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-<table> fallback drops them). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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-<table> 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-<table> 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<string, AttrPolicy> = {
|
||||
'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 <table>
|
||||
// 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-<table> 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:<name>.<attr>`, 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<string, unknown>;
|
||||
for (const attr of Object.keys(attrs)) keys.push(`mark:${name}.${attr}`);
|
||||
}
|
||||
return keys.sort();
|
||||
}
|
||||
|
||||
export type AttrMode = 'p1' | 'fuzz';
|
||||
|
||||
/**
|
||||
|
||||
@@ -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<string>([
|
||||
'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<string>([
|
||||
'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<string>([
|
||||
// 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', () => {
|
||||
|
||||
Reference in New Issue
Block a user