diff --git a/CHANGELOG.md b/CHANGELOG.md index 30d40dea..9f66c346 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,15 @@ this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed +- **Report taxonomy no longer bills added-node style inventories as restyles.** A + brand-new element still emits its full resting/state inventory (raw findings and + exit codes unchanged), but presentation counts and copy reserve + "computed-style difference(s)" / "state-delta difference(s)" / "restyled" for + matched-path before→after changes. Added-node tables are labelled + **Style inventory (head-side — no baseline)** with value-only rows, so wrapper + / path-churn inserts read as DOM adds and removals — never as a pile of + restyles. `report.json` `counts` follows the presentation taxonomy; + `rawCounts` / `reviewableCounts` keep the full certification tallies. - **Dependency audit is green again.** Updated the transitive `fast-uri` development dependency from 3.1.4 to 3.1.5, closing GHSA-7p8r-x3mc-p8w7 without changing the shipped runtime package. diff --git a/README.md b/README.md index 6cb92dc7..91d36df5 100644 --- a/README.md +++ b/README.md @@ -284,6 +284,13 @@ New pages, states, and surfaces appear before element-level changes. Existing surfaces render one distinct change per section, with aligned crops, truthful annotations, a one-line summary, and exact properties under a toggle. +Headline counts distinguish **matched-path restyles** ("N computed-style +difference(s)") from **one-sided DOM adds/removes**. A brand-new node's full +resting and interaction-state inventory still appears under the element (value-only +tables labelled _Style inventory (head-side — no baseline)_), but those rows are +not billed as before→after restyles — so a wrapper insert or path churn reads as +DOM structure, not a cascade of restyles. + Tiny changes also receive a magnified crop. Structural matching avoids painting an unchanged shifted subtree as changed, while ambiguous duplicate elements stay explicit rather than receiving invented provenance. diff --git a/bin/styleproof-diff.mjs b/bin/styleproof-diff.mjs index 23b2b1b3..4a2812d1 100755 --- a/bin/styleproof-diff.mjs +++ b/bin/styleproof-diff.mjs @@ -431,8 +431,9 @@ printBaselineSurfaceFailureCallout(); // One finding's lines: a heading, then its summarised property deltas (the same // dedupe the report shows). Returns [] for a DOM finding (handled separately) or a -// finding whose props all summarised away. -function findingLines(f) { +// finding whose props all summarised away. `inventory` = one-sided added path — +// head-side values with no baseline, never printed as before → after restyles. +function findingLines(f, inventory = false) { if (f.kind === 'dom') return []; const rows = summarizeProps(f.props); if (!rows.length) return []; @@ -440,7 +441,11 @@ function findingLines(f) { f.kind === 'state' ? ` [:${f.state}] ${findingLabel(f.path, f.cls)}${f.sub !== f.path ? ` ⇒ ${f.sub}` : ''}` : ` ${findingLabel(f.path, f.cls)}${f.pseudo || ''}`; - return [head, ...rows.map((p) => ` ${p.prop}: ${p.before} → ${p.after}`)]; + const note = inventory ? ' (head-side inventory — no baseline)' : ''; + return [ + head + note, + ...rows.map((p) => (inventory ? ` ${p.prop}: ${p.after}` : ` ${p.prop}: ${p.before} → ${p.after}`)), + ]; } // A DOM finding's one-line heading (added/removed/retagged). @@ -457,7 +462,8 @@ function elementLines(findings) { for (const group of groupByPath(findings)) { const dom = group.find((f) => f.kind === 'dom'); if (dom) lines.push(domLine(dom)); - for (const f of group) lines.push(...findingLines(f)); + const inventory = dom?.change === 'added'; + for (const f of group) lines.push(...findingLines(f, inventory)); } return lines; } diff --git a/docs/demo/report.md b/docs/demo/report.md index 10daec44..088e3bea 100644 --- a/docs/demo/report.md +++ b/docs/demo/report.md @@ -2,7 +2,7 @@ 🆕 **1 new surface(s)** captured with no baseline to compare: `pricing @ 900`. Approve them before they become the baseline. -**5 DOM change(s) · 8 computed-style difference(s)** across 3 distinct change(s) in 3 changed surface bases with an existing baseline. +**5 DOM change(s) · 4 computed-style difference(s)** across 3 distinct change(s) in 3 changed surface bases with an existing baseline. _**Surface base** = one product UI state; capture keys with `@width` or live-state/popup variants are width or state captures of that base._ ## 🆕 New pages, states, or surfaces — review first @@ -34,11 +34,11 @@ _duplicate-insertion @ 900_ - **1** element added
-Show the property change +Show the head-side style inventory **Added** `button.duplicate-control` -Style: +Style inventory (head-side — no baseline): | Property | Value | | --- | --- | @@ -144,7 +144,7 @@ Style: **Added** `button.filter` -Style: +Style inventory (head-side — no baseline): | Property | Value | | --- | --- | @@ -152,7 +152,7 @@ Style: **Added** `div.grid` -Style: +Style inventory (head-side — no baseline): | Property | Value | | --- | --- | @@ -160,7 +160,7 @@ Style: **Added** `article.card` -Style: +Style inventory (head-side — no baseline): | Property | Value | | --- | --- | diff --git a/docs/what-it-catches.md b/docs/what-it-catches.md index 97425160..09f28cb6 100644 --- a/docs/what-it-catches.md +++ b/docs/what-it-catches.md @@ -14,10 +14,10 @@ On every **captured surface**, base vs head: | Change | Surfaced as | Pinned by | | ------------------------------------------------------------ | ----------------------------------- | -------------- | -| A computed style differs (resting) | `style` finding, property named | pr-surfacing ✓ | +| A computed style differs on a matched path (resting) | `style` finding; headline "computed-style difference(s)" — not used for added-node inventory | pr-surfacing ✓ | | A `:hover` / `:focus` / `:active` variant dropped or changed | `state` finding | pr-surfacing ✓ | | A `::before` / `::after` style differs | `style` finding, pseudo tagged | pr-surfacing ✓ | -| An element is added or removed | `dom` finding (added / removed) | pr-surfacing ✓ | +| An element is added or removed | `dom` finding (added / removed); report shows head-side style **inventory** (value-only, no baseline) — not a restyle | pr-surfacing ✓ | | An element is retagged (`button` → `a`) | removed + added at that position | pr-surfacing ✓ | | A nav item / route disappears | inventory guard, named, **gates** | pr-surfacing ✓ | | A surface exists on only one side | reported as a new / removed surface | pr-surfacing ✓ | diff --git a/src/describe.ts b/src/describe.ts index e57b9e88..05d1f660 100644 --- a/src/describe.ts +++ b/src/describe.ts @@ -393,7 +393,9 @@ function restyleLines(els: ElementChange[], ctx: DescribeCtx): string[] { */ export function describeChange(els: ElementChange[], ctx: DescribeCtx = {}, maxBullets = 6): string[] { const lines = [...domVerbLines(els), ...restyleLines(els, ctx)]; - const states = [...new Set(els.flatMap((e) => e.states ?? []))]; + // Interaction-state *changes* only on matched-path elements. Added/removed nodes + // carry head- or base-side state inventory (no baseline pair) — the DOM verb covers them. + const states = [...new Set(els.filter((e) => !e.added && !e.removed).flatMap((e) => e.states ?? []))]; if (states.length) lines.push(`interaction states changed: ${states.map((s) => `\`:${s}\``).join(', ')}`); if (lines.length <= maxBullets) return lines; return [...lines.slice(0, maxBullets - 1), `…and ${lines.length - (maxBullets - 1)} more change(s)`]; diff --git a/src/report.ts b/src/report.ts index 8d0436e3..c7cf23ad 100644 --- a/src/report.ts +++ b/src/report.ts @@ -435,7 +435,9 @@ function annotationPaths( return { before: innermost([...beforePaths]), after: innermost([...afterPaths]) }; } -/** Headline counts with the zeros dropped — `0 state-delta difference(s)` is noise. */ +/** Headline counts with the zeros dropped — `0 state-delta difference(s)` is noise. + * `style`/`state` here are matched-path restyles only (see {@link countShownChanges}); + * one-sided added-node inventories are billed under DOM, never as differences. */ function changeCountLabel(shown: DiffCounts): string { const parts: string[] = []; if (shown.dom) parts.push(`${shown.dom} DOM change(s)`); @@ -444,6 +446,16 @@ function changeCountLabel(shown: DiffCounts): string { return parts.join(' · '); } +/** Paths that are one-sided DOM adds/removes — their style/state rows are full + * head- or base-side inventories, not before→after restyles on a matched path. */ +function oneSidedDomPaths(findings: Finding[]): Set { + const paths = new Set(); + for (const f of findings) { + if (f.kind === 'dom' && (f.change === 'added' || f.change === 'removed')) paths.add(f.path); + } + return paths; +} + type Group = { paths: string[]; before: Box | null; after: Box | null }; function groupRegions(paths: string[], a: StyleMap, b: StyleMap, padBy: number): Group[] { @@ -740,6 +752,13 @@ function valueTable(rows: PropChange[]): string[] { return ['| Property | Value |', '| --- | --- |', ...rows.map((r) => `| ${codeValue(r.prop)} | ${cell(r.after)} |`)]; } +/** Heading for an added node's style block — inventory, never a before→after restyle. */ +function addedStyleHeading(pseudo: string | null): string { + return pseudo + ? `On \`${pseudo}\` (head-side inventory — no baseline):` + : 'Style inventory (head-side — no baseline):'; +} + /** `Button (variant=primary, size=sm)` — the React component + sanitized props * the element captured (advisory; present only with captureComponent). */ function renderComponent(c: { name: string; props?: Record }): string { @@ -749,7 +768,7 @@ function renderComponent(c: { name: string; props?: Record }): s } /** One element's heading + body lines (no leading blank, no ×N suffix). */ -// Base/pseudo style rows. Added elements render value-only (no meaningful before). +// Base/pseudo style rows. Added elements render value-only inventory (no baseline). function styleSection(styles: Extract[], added: boolean): string[] { const out: string[] = []; for (const s of styles) { @@ -757,7 +776,7 @@ function styleSection(styles: Extract[], added: bool if (rows.length) out.push( '', - s.pseudo ? `On \`${s.pseudo}\`:` : 'Style:', + added ? addedStyleHeading(s.pseudo) : s.pseudo ? `On \`${s.pseudo}\`:` : 'Style:', '', ...(added ? valueTable(rows) : beforeAfterTable(rows)), ); @@ -857,10 +876,17 @@ function renderElements(findings: Finding[], maxElements = 40): string[] { } /** Plain-text `` affordance — GitHub renders markdown inside `` - * literally, so no backticks or bold here. */ + * literally, so no backticks or bold here. Added-node-only inventories say + * "inventory", not "property change", so reviewers don't read them as restyles. */ function foldSummary(findings: Finding[]): string { - const n = findings.flatMap((f) => (f.kind === 'dom' ? [] : summarizeProps(f.props))).length; + const oneSided = oneSidedDomPaths(findings); + const propFindings = findings.filter((f) => f.kind !== 'dom'); + const n = propFindings.flatMap((f) => summarizeProps(f.props)).length; if (!n) return 'Show details'; + const allInventory = propFindings.length > 0 && propFindings.every((f) => oneSided.has(f.path)); + if (allInventory) { + return n === 1 ? 'Show the head-side style inventory' : `Show all ${n} head-side inventory properties`; + } return n === 1 ? 'Show the property change' : `Show all ${n} property changes`; } @@ -1258,15 +1284,21 @@ function groupBySignature(prepared: PreparedSurface[], beforeDir: string, afterD } // Counts reflect the GROUPED view: each distinct change counts once, not once per -// surface it appears on (after shorthand/dedupe collapsing). +// surface it appears on (after shorthand/dedupe collapsing). Style/state tallies +// are matched-path restyles only — props on a one-sided added/removed path are +// head- or base-side inventories and already covered by the DOM count; billing +// them as "computed-style difference(s)" mislabels path churn as restyles. function countShownChanges(changeGroups: ChangeGroup[]): DiffCounts { const shown: DiffCounts = { dom: 0, style: 0, state: 0 }; - for (const cg of changeGroups) + for (const cg of changeGroups) { + const oneSided = oneSidedDomPaths(cg.findings); for (const f of cg.findings) { if (f.kind === 'dom') shown.dom++; + else if (oneSided.has(f.path)) continue; else if (f.kind === 'style') shown.style += summarizeProps(f.props).length; else shown.state += summarizeProps(f.props).length; } + } return shown; } diff --git a/test/report.test.mjs b/test/report.test.mjs index 194a5c90..92ea4fec 100644 --- a/test/report.test.mjs +++ b/test/report.test.mjs @@ -715,14 +715,152 @@ test('an added element reports its full resting computed style, value-only', () }); const md = fs.readFileSync(generateStyleMapReport({ beforeDir, afterDir, outDir }).reportMdPath, 'utf8'); assert.match(md, /\*\*Added\*\* `button\.btn`/); - assert.match(md, /Style:/); + assert.match(md, /Style inventory \(head-side — no baseline\)/); assert.match(md, /\| Property \| Value \|/); // value-only, no bogus Before column + assert.doesNotMatch(md, /\| Property \| Before \| After \|/, 'added-node inventory is never a before→after table'); assert.match(md, /`background-color` \| `#005afc`/); assert.match(md, /`padding` \| `6px 12px`/); assert.match(md, /`border-radius` \| `4px`/); rmTmp(root); }); +// Taxonomy: a brand-new node's full resting/state inventory must not be billed as +// matched-path computed-style / state-delta *differences*. Headline counts and copy +// must reserve those words for paths present on both sides. +test('add-only: headline counts DOM only — head-side inventory is not a restyle difference', () => { + const before = makeMap({ elements: { body: { tag: 'body', rect: [0, 0, 1280, 800], style: {} } } }); + const after = makeMap({ + elements: { + body: { tag: 'body', rect: [0, 0, 1280, 800], style: {} }, + 'body > button:nth-child(1)': { + tag: 'button', + cls: 'btn', + rect: [0, 0, 90, 32], + style: { + 'background-color': 'rgb(0, 90, 252)', + color: 'rgb(255, 255, 255)', + padding: '6px 12px', + }, + }, + }, + states: { + 'body > button:nth-child(1)': { + hover: { 'body > button:nth-child(1)': { 'background-color': 'rgb(0, 60, 200)' } }, + }, + }, + }); + const { beforeDir, afterDir, outDir, root } = pairFixture({ + surface: 's@1280', + before, + after, + beforePng: solidPng(1280, 800), + afterPng: solidPng(1280, 800), + }); + const res = generateStyleMapReport({ beforeDir, afterDir, outDir }); + const md = fs.readFileSync(res.reportMdPath, 'utf8'); + const json = JSON.parse(fs.readFileSync(res.reportJsonPath, 'utf8')); + // Presentation counts: DOM add only. Style/state rows are inventory on the new node. + assert.deepEqual(json.counts, { dom: 1, style: 0, state: 0 }); + assert.match(md, /\*\*1 DOM change\(s\)\*\* across/); + assert.doesNotMatch(md, /computed-style difference/); + assert.doesNotMatch(md, /state-delta difference/); + assert.match(md, /Style inventory \(head-side — no baseline\)/); + assert.match(md, /Interactive states:/); + assert.doesNotMatch(md, /Interactive-state changes:/); + assert.doesNotMatch(md, /Before → After/); + // Raw certification findings still carry the full inventory (backward compatible). + assert.equal(json.rawCounts.dom, 1); + assert.ok(json.rawCounts.style > 0 || json.reviewableCounts.style > 0, 'raw findings still inventory the added node'); + rmTmp(root); +}); + +test('matched-path restyle still counts as a computed-style difference (not inventory)', () => { + const panel = (color) => + makeMap({ + elements: { + body: { tag: 'body', rect: [0, 0, 1280, 800], style: {} }, + 'body > div:nth-child(1)': { + tag: 'div', + cls: 'panel', + rect: [0, 0, 400, 300], + style: { 'background-color': color }, + }, + }, + }); + const { beforeDir, afterDir, outDir, root } = pairFixture({ + surface: 's@1280', + before: panel('rgb(0, 0, 0)'), + after: panel('rgb(255, 0, 0)'), + beforePng: solidPng(1280, 800), + afterPng: solidPng(1280, 800), + }); + const res = generateStyleMapReport({ beforeDir, afterDir, outDir }); + const md = fs.readFileSync(res.reportMdPath, 'utf8'); + const json = JSON.parse(fs.readFileSync(res.reportJsonPath, 'utf8')); + assert.deepEqual(json.counts, { dom: 0, style: 1, state: 0 }); + assert.match(md, /\*\*1 computed-style difference\(s\)\*\* across/); + assert.match(md, /1 element restyled/); + assert.match(md, /\| Property \| Before \| After \|/); + assert.doesNotMatch(md, /Style inventory \(head-side/); + assert.doesNotMatch(md, /no baseline/); + rmTmp(root); +}); + +// Wrapper / nth-child path churn: insert a parent around existing content. Paths +// become one-sided adds/removes with full inventories — never matched-path restyles. +test('wrapper path-churn: inventory copy, not restyle differences', () => { + const before = makeMap({ + elements: { + body: { tag: 'body', rect: [0, 0, 1280, 800], style: {} }, + 'body > button:nth-child(1)': { + tag: 'button', + cls: 'cta', + rect: [10, 10, 120, 40], + style: { 'background-color': 'rgb(0, 90, 252)', color: 'rgb(255, 255, 255)' }, + }, + }, + }); + // Wrap the button: old path removed, wrapper + button-at-new-path added. + const after = makeMap({ + elements: { + body: { tag: 'body', rect: [0, 0, 1280, 800], style: {} }, + 'body > div:nth-child(1)': { + tag: 'div', + cls: 'wrap', + rect: [0, 0, 200, 80], + style: { padding: '8px' }, + }, + 'body > div:nth-child(1) > button:nth-child(1)': { + tag: 'button', + cls: 'cta', + rect: [18, 18, 120, 40], + style: { 'background-color': 'rgb(0, 90, 252)', color: 'rgb(255, 255, 255)' }, + }, + }, + }); + const { beforeDir, afterDir, outDir, root } = pairFixture({ + surface: 's@1280', + before, + after, + beforePng: solidPng(1280, 800), + afterPng: solidPng(1280, 800), + }); + const res = generateStyleMapReport({ beforeDir, afterDir, outDir }); + const md = fs.readFileSync(res.reportMdPath, 'utf8'); + const json = JSON.parse(fs.readFileSync(res.reportJsonPath, 'utf8')); + assert.equal(json.counts.style, 0, 'wrapper path-churn inventories are not style differences'); + assert.equal(json.counts.state, 0); + assert.ok(json.counts.dom >= 2, 'adds + remove are DOM changes'); + assert.match(md, /DOM change/); + assert.doesNotMatch(md, /computed-style difference/); + assert.doesNotMatch(md, /1 element restyled|elements restyled/); + assert.match(md, /element(?:s)? added/); + assert.match(md, /element(?:s)? removed/); + assert.match(md, /Style inventory \(head-side — no baseline\)/); + assert.doesNotMatch(md, /\| Property \| Before \| After \|/); + rmTmp(root); +}); + // Regression, seen in a downstream report: a gradient diff rendered as the same // "representative" rgba in BOTH cells — the real change (a dropped `0px` stop) // was invisible. Long values must excerpt around the differing substring. @@ -1138,6 +1276,24 @@ test('describeChange reports added/removed counts', () => { assert.ok(out.some((l) => /\*\*1\*\* element removed/.test(l))); }); +test('describeChange never bills added-node inventory as a restyle or state change', () => { + const out = describeChange([ + { + label: 'button.btn', + added: true, + props: [ + { prop: 'background-color', before: '(unset)', after: 'rgb(0, 90, 252)' }, + { prop: 'color', before: '(unset)', after: 'rgb(255, 255, 255)' }, + ], + states: ['hover'], + }, + ]); + assert.ok(out.some((l) => /\*\*1\*\* element added/.test(l))); + assert.ok(!out.some((l) => /button\.btn/.test(l) && /—/.test(l)), 'no restyle phrase for an added node'); + assert.ok(!out.some((l) => /interaction states/.test(l)), 'added-node states are inventory, not changes'); + assert.ok(!out.some((l) => /restyled/.test(l))); +}); + // ------------------------------------------------ colour tokens / hex / folding test('toHex renders opaque colours as #hex and keeps alpha as rgba', () => { @@ -1642,7 +1798,13 @@ test('end-to-end: unchanged forced-state movement is suppressed as path churn', report.surfaces[0].regions.every((region) => !region.images.annotated), 'path churn has no annotation', ); - assert.equal(report.counts.state, 2, 'state findings remain in the audit data'); + // Presentation counts reserve style/state for matched-path restyles; one-sided + // inventories stay in raw/reviewable audit tallies. + assert.equal(report.counts.state, 0, 'path-churn state inventory is not a matched-path state-delta'); + assert.ok( + report.rawCounts.state >= 2 || report.reviewableCounts.state >= 2, + 'state findings remain in the audit data', + ); rmTmp(root); }); @@ -1679,7 +1841,11 @@ test('end-to-end: moved forced-state changes stay annotated', () => { }); assert.ok(highlights.before > 0, 'the changed before state remains visible in proof'); assert.ok(highlights.after > 0, 'the changed after state remains visible in proof'); - assert.equal(report.counts.state, 2, 'state findings remain in the audit data'); + assert.equal(report.counts.state, 0, 'one-sided path inventories are not matched-path state-deltas'); + assert.ok( + report.rawCounts.state >= 2 || report.reviewableCounts.state >= 2, + 'state findings remain in the audit data', + ); rmTmp(root); }); @@ -1725,7 +1891,11 @@ test('end-to-end: owner pseudo-element state movement is suppressed as path chur report.surfaces[0].regions.every((region) => !region.images.annotated), 'owner pseudo-element path churn has no annotation', ); - assert.equal(report.counts.state, 2, 'pseudo-element state findings remain in the audit data'); + assert.equal(report.counts.state, 0, 'path-churn pseudo state inventory is not a matched-path state-delta'); + assert.ok( + report.rawCounts.state >= 2 || report.reviewableCounts.state >= 2, + 'pseudo-element state findings remain in the audit data', + ); rmTmp(root); }); @@ -1776,7 +1946,11 @@ test('end-to-end: moved owner pseudo-element state changes stay annotated', () = }); assert.ok(highlights.before > 0, 'the changed before pseudo-state remains visible in proof'); assert.ok(highlights.after > 0, 'the changed after pseudo-state remains visible in proof'); - assert.equal(report.counts.state, 2, 'pseudo-element state findings remain in the audit data'); + assert.equal(report.counts.state, 0, 'one-sided path inventories are not matched-path state-deltas'); + assert.ok( + report.rawCounts.state >= 2 || report.reviewableCounts.state >= 2, + 'pseudo-element state findings remain in the audit data', + ); rmTmp(root); });