From 7b28267a2cc23eeae50aee85b0535ed92de2eb6e Mon Sep 17 00:00:00 2001 From: Xuanru Li <157947275+xuanruli@users.noreply.github.com> Date: Sun, 26 Jul 2026 00:52:33 -0700 Subject: [PATCH 1/2] fix(check): key content_overlap collapse by pair, not text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `staticIssueKey` put `issue.text` in the collapse key, but `overlapIssue` already names a collision by both of its selectors. When the leading element's text changes per sample — a count-up, typewriter, or rotating word — every dense-pass sample formed its own group with `occurrences: 1`, so `applyPersistenceTier` demoted all of them to `info` and a genuinely held collision never reached the `error` tier that gates a run. Empirically: two projects with identical geometry and an identical 5.9s overlap, differing only in DOM order, verdicted differently — label-first gave `error`/`ok=false`, count-up-first gave 48x `info`/`ok=true`. After this change both give `error`/`ok=false` with `occurrences: 48`. Seven other overlap fixtures (crossfades, static, masked reel, 600ms transient) are unchanged. Codes that identify themselves by text keep it; only `content_overlap`, which carries both selectors, drops it. Co-Authored-By: Claude Opus 5 --- packages/cli/src/utils/layoutAudit.test.ts | 36 ++++++++++++++++++++++ packages/cli/src/utils/layoutAudit.ts | 8 ++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/utils/layoutAudit.test.ts b/packages/cli/src/utils/layoutAudit.test.ts index 13ea482e32..94109aa21e 100644 --- a/packages/cli/src/utils/layoutAudit.test.ts +++ b/packages/cli/src/utils/layoutAudit.test.ts @@ -255,6 +255,42 @@ describe("persistence-tiered severity (#U10)", () => { expect(collapsed[0]).toMatchObject({ severity: "warning", occurrences: 2 }); }); + it("promotes a content_overlap whose text changes every sample (count-up over a label)", () => { + // The colliding pair is named by both selectors, so per-sample text must not split one held collision into transient groups. + const collapsed = collapseStaticLayoutIssues( + [ + { + ...issue("content_overlap", "warning"), + time: 4.0, + containerSelector: ".num", + text: "$1,204", + }, + { + ...issue("content_overlap", "warning"), + time: 4.5, + containerSelector: ".num", + text: "$8,930", + }, + ], + 73, + ); + + expect(collapsed).toHaveLength(1); + expect(collapsed[0]).toMatchObject({ severity: "error", occurrences: 2 }); + }); + + it("still separates two distinct text_box_overflow findings that differ only by text", () => { + const collapsed = collapseStaticLayoutIssues( + [ + { ...issue("text_box_overflow", "warning"), time: 4.0, text: "first" }, + { ...issue("text_box_overflow", "warning"), time: 4.5, text: "second" }, + ], + 73, + ); + + expect(collapsed).toHaveLength(2); + }); + it("promotes content_overlap whose two occurrences span exactly 500ms (at the floor)", () => { const collapsed = collapseStaticLayoutIssues( [ diff --git a/packages/cli/src/utils/layoutAudit.ts b/packages/cli/src/utils/layoutAudit.ts index 5f68cb7007..1dc4ee308c 100644 --- a/packages/cli/src/utils/layoutAudit.ts +++ b/packages/cli/src/utils/layoutAudit.ts @@ -342,12 +342,18 @@ function staticIssueKey(issue: LayoutIssue): string { issue.severity, issue.selector, issue.containerSelector ?? "", - issue.text ?? "", + collapseTextKey(issue), issue.overflow ? formatOverflow(issue.overflow) : "", framePositionKey(issue), ].join("|"); } +/** Text identity for the collapse key, omitted where the code identifies itself by geometry instead. */ +function collapseTextKey(issue: LayoutIssue): string { + // content_overlap names its pair by both selectors, so animated text would split one held collision into per-sample groups. + return issue.code === "content_overlap" ? "" : (issue.text ?? ""); +} + function framePositionKey(issue: LayoutIssue): string { // connector_detached shares it: id-less paths collapse to one selector, so distinct lines need geometry in the key. return issue.code === "frame_out_of_frame" || issue.code === "connector_detached" From ca4ee36e77277b03adda562607d6c3306c3ef65e Mon Sep 17 00:00:00 2001 From: Xuanru Li <157947275+xuanruli@users.noreply.github.com> Date: Sun, 26 Jul 2026 01:04:48 -0700 Subject: [PATCH 2/2] fix(check): uniquely select both ends before keying by pair MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the previous commit's premise false: `overlapIssue` used `selectorFor`, which returns `tag.class1.class2` with no uniqueness check, so dropping `text` left nothing to separate two aliased siblings. Two unrelated 125ms transients on sibling stat cards merged into one group whose span bridged the gap between them, promoting past the 500ms floor to `error` — a false failure worse than the false pass being fixed. Both builders now use the file's own `uniqueSelectorFor`, so the pair really is the identity the collapse key assumes. Measured on an aliased-sibling fixture: `error occ=2 div.num` / `ok=false` before, two separate warnings / `ok=true` after. `text_occluded` had the identical defect and is included: it carries both selectors plus the animating element's text, and is persistence-tiered, so a count-up held under an opaque scrim for 6s — a hard `error` at every sample — collapsed into 12 single-occurrence `info` rows and the run passed. Now `error occ=12` / `ok=false`. Per-code policy moved next to `PERSISTENCE_TIERED_CODES` as a named set, and `dedupeLayoutIssues` keeps raw text deliberately (it pins an exact time, where text still separates concurrent findings) with a note so the divergence is not "fixed" back. Co-Authored-By: Claude Opus 5 --- .../cli/src/commands/layout-audit.browser.js | 10 ++++--- packages/cli/src/utils/layoutAudit.test.ts | 27 +++++++++++++++++++ packages/cli/src/utils/layoutAudit.ts | 18 ++++++++----- 3 files changed, 44 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/commands/layout-audit.browser.js b/packages/cli/src/commands/layout-audit.browser.js index 382184309a..2713d22133 100644 --- a/packages/cli/src/commands/layout-audit.browser.js +++ b/packages/cli/src/commands/layout-audit.browser.js @@ -656,8 +656,9 @@ code: "content_overlap", severity: "warning", time, - selector: selectorFor(a.element), - containerSelector: selectorFor(b.element), + // Unique both ends: the collapse key names this pair by its selectors alone, so aliased siblings must not share one. + selector: uniqueSelectorFor(a.element), + containerSelector: uniqueSelectorFor(b.element), text: textContentFor(a.element), message: "Two text blocks overlap and may render unreadable.", rect: a.rect, @@ -1017,8 +1018,9 @@ code: "text_occluded", severity: "error", time, - selector: selectorFor(element), - containerSelector: selectorFor(occluder), + // Unique both ends: the collapse key names this pair by its selectors alone, so aliased siblings must not share one. + selector: uniqueSelectorFor(element), + containerSelector: uniqueSelectorFor(occluder), text, message: "Text is hidden beneath an opaque element.", rect: textRect, diff --git a/packages/cli/src/utils/layoutAudit.test.ts b/packages/cli/src/utils/layoutAudit.test.ts index 94109aa21e..5abb8185ad 100644 --- a/packages/cli/src/utils/layoutAudit.test.ts +++ b/packages/cli/src/utils/layoutAudit.test.ts @@ -279,6 +279,33 @@ describe("persistence-tiered severity (#U10)", () => { expect(collapsed[0]).toMatchObject({ severity: "error", occurrences: 2 }); }); + it("keeps two content_overlap pairs on different containers in separate groups", () => { + const collapsed = collapseStaticLayoutIssues( + [ + { ...issue("content_overlap", "warning"), time: 4.0, containerSelector: ".num" }, + { ...issue("content_overlap", "warning"), time: 4.5, containerSelector: ".pct" }, + ], + 73, + ); + + expect(collapsed).toHaveLength(2); + }); + + it("does not bridge two separate transients on one pair into a held collision", () => { + // Both blips sit under the 500ms floor; spanning them would fabricate a 4.1s collision that never happened. + const blip = { ...issue("content_overlap", "warning"), containerSelector: ".label" }; + const collapsed = collapseStaticLayoutIssues( + [ + { ...blip, time: 1.0 }, + { ...blip, time: 1.125 }, + ], + 73, + ); + + expect(collapsed).toHaveLength(1); + expect(collapsed[0]).toMatchObject({ severity: "warning", occurrences: 2 }); + }); + it("still separates two distinct text_box_overflow findings that differ only by text", () => { const collapsed = collapseStaticLayoutIssues( [ diff --git a/packages/cli/src/utils/layoutAudit.ts b/packages/cli/src/utils/layoutAudit.ts index 1dc4ee308c..d535df09a0 100644 --- a/packages/cli/src/utils/layoutAudit.ts +++ b/packages/cli/src/utils/layoutAudit.ts @@ -158,6 +158,7 @@ export function formatLayoutIssue(issue: LayoutIssue): string { return issue.fixHint ? `${line}\n Fix: ${issue.fixHint}` : line; } +// Keeps raw `text` where `staticIssueKey` drops it: this key pins an exact time, so text still separates concurrent findings rather than spanning samples. export function dedupeLayoutIssues(issues: LayoutIssue[]): LayoutIssue[] { const seen = new Set(); const result: LayoutIssue[] = []; @@ -204,6 +205,15 @@ const PERSISTENCE_TIERED_CODES: ReadonlySet = new Set([ "connector_detached", ]); +// Codes whose collapse key is the selector pair alone. Both builders uniquely +// select their two ends, so the pair IS the identity — and keeping per-sample +// text would split one held finding into transient groups whenever the subject's +// text animates (count-up, typewriter, rotating word). +const TEXT_AGNOSTIC_KEY_CODES: ReadonlySet = new Set([ + "content_overlap", + "text_occluded", +]); + export function collapseStaticLayoutIssues( issues: LayoutIssue[], totalSampleCount?: number, @@ -342,18 +352,12 @@ function staticIssueKey(issue: LayoutIssue): string { issue.severity, issue.selector, issue.containerSelector ?? "", - collapseTextKey(issue), + TEXT_AGNOSTIC_KEY_CODES.has(issue.code) ? "" : (issue.text ?? ""), issue.overflow ? formatOverflow(issue.overflow) : "", framePositionKey(issue), ].join("|"); } -/** Text identity for the collapse key, omitted where the code identifies itself by geometry instead. */ -function collapseTextKey(issue: LayoutIssue): string { - // content_overlap names its pair by both selectors, so animated text would split one held collision into per-sample groups. - return issue.code === "content_overlap" ? "" : (issue.text ?? ""); -} - function framePositionKey(issue: LayoutIssue): string { // connector_detached shares it: id-less paths collapse to one selector, so distinct lines need geometry in the key. return issue.code === "frame_out_of_frame" || issue.code === "connector_detached"