feat(cli): coordinate-frame layout findings in check (#2354)

* feat(cli): coordinate-frame layout findings in check

Four production compositions shipped with 100-600px layout drift, each a
different coordinate-frame confusion the check graded info or missed
entirely: viewport pixels written as container left/top, gsap x/y
treated as absolute position, a -350px margin fighting flex centering,
and stage-relative path coords drawn into a nested SVG.

Three new layout findings close the class:
- positioned_out_of_parent: an absolute/fixed element rendering mostly
  outside its positioning ancestor (warning) — the parent needs no
  overflow clipping, which is what let container_overflow miss it.
- box_out_of_canvas: a painted panel breaching the canvas (warning) —
  text is canvas_overflow's, media is frame_out_of_frame's, painted
  boxes were nobody's.
- connector_detached: a connector path whose endpoints land far from
  every anchorable element (warning) — measured coordinates drawn into
  an SVG with a different origin.

canvas_overflow additionally promotes from info to warning when held
across samples AND the breach exceeds 5% of the canvas.

All three are persistence-tiered and respect data-layout-allow-overflow.
Verified against the four incident compositions: every one now surfaces
its drift as held warnings (previously: info or silence).

* fix(cli): harden coordinate-frame findings against review false positives

Reworks all three findings after two-lens review (adversarial FP hunt in
real Chrome + maintainer pass):

- escaped_container (was positioned_out_of_parent): uses offsetParent
  (transform-aware, skips fixed-as-canvas), exempts fully-detached
  callouts within an attachment allowance while still flagging
  touching-but-mostly-outside drift.
- panel_out_of_canvas (was box_out_of_canvas): paint alone qualifies
  (flat solid panels were a false negative), fully off-canvas rects are
  parked entrances and stay silent, pointer-events:none marks decorative
  layers, hero-sized breaches warn while small bleeds stay info.
- connector_detached: endpoints via getPointAtLength + getScreenCTM
  (viewBox, preserveAspectRatio, group transforms, every command type),
  defs/marker/clipPath subtrees skipped, word-boundary connector naming,
  containment tier limited to opaque non-ancestor targets (a text-bearing
  wrapper contains its own diagram's endpoints).
- canvas_overflow promotion requires partial visibility — a fully
  off-canvas rect is a parked entrance, not drift.

Verified: the four incident compositions still surface their drift as
held warnings; the review's false-positive repros (fixed HUD, callout,
parked entrance, corner bleed, marker arrowheads, g-transform and
viewBox-scaled connectors) are clean at warning level. Docs and the CLI
skill reference now describe the coordinate-frame findings.

* fix(cli): panel ownership is geometric — direct-text panels were a silent false negative

A painted panel whose direct text stays in-bounds while its box breaches
the canvas produced neither finding: canvas_overflow measures the text
range and panel_out_of_canvas skipped every own-text element. Skip the
panel finding only when the element's own text ALSO breaches (that
geometry belongs to canvas_overflow); pin the message/fixHint wording of
all three findings with positive assertions; document the SVG-internal
anchor blind spot.

* fix(cli): classify panel decoration by paint kind, not pointer-events

pointer-events:none exempted the framed-painting incident's gold frame
layers — hero content that happens to disable hit-testing. Decoration is
now gradient-only paint (spotlights, textures, vignettes); url() images,
solid fills and borders are content regardless of pointer-events.

* fix(cli): add fixHint to the test-local AuditIssue shape

* fix(cli): gradient stops decide content vs decoration; ownership matches canvas_overflow's tolerance

A gradient with any solid stop (alpha >= 0.6) is content — heroes and
cards painted with linear-gradient were invisible under the blanket
gradient exemption; all-translucent stops (spotlights, vignettes) stay
decoration. The text-ownership check now uses the audit tolerance that
canvas_overflow itself fires at, making the contract strict-mutex: any
text breach past that tolerance cedes the element, so a shallow 20px
text breach no longer double-reports.
This commit is contained in:
Xuanru Li
2026-07-13 15:35:03 -07:00
committed by GitHub
parent 8e50e8477f
commit 7f4eaeb568
8 changed files with 686 additions and 11 deletions
+3
View File
@@ -955,6 +955,9 @@ const LAYOUT_ISSUE_CODES: readonly LayoutIssueCode[] = [
"text_not_painted",
"caption_zone_collision",
"frame_out_of_frame",
"escaped_container",
"panel_out_of_canvas",
"connector_detached",
"motion_appears_late",
"motion_out_of_order",
"motion_off_frame",
+65 -1
View File
@@ -227,6 +227,70 @@ describe("persistence-tiered severity (#U10)", () => {
expect(collapsed[0]).toMatchObject({ severity: "error", occurrences: 2 });
});
it("promotes a held, canvas-scale canvas_overflow breach from info to warning", () => {
const breach = {
...issue("canvas_overflow", "info"),
overflow: { top: 140 },
containerRect: { left: 0, top: 0, right: 1920, bottom: 1080, width: 1920, height: 1080 },
};
const collapsed = collapseStaticLayoutIssues(
[
{ ...breach, time: 1 },
{ ...breach, time: 3 },
],
9,
);
expect(collapsed).toHaveLength(1);
expect(collapsed[0]).toMatchObject({ severity: "warning", occurrences: 2 });
});
it("keeps a held, large but fully off-canvas canvas_overflow at info — a parked entrance, not drift", () => {
const breach = {
...issue("canvas_overflow", "info"),
rect: { left: 2200, top: 300, right: 2800, bottom: 700, width: 600, height: 400 },
overflow: { right: 880 },
containerRect: { left: 0, top: 0, right: 1920, bottom: 1080, width: 1920, height: 1080 },
};
const collapsed = collapseStaticLayoutIssues(
[
{ ...breach, time: 1 },
{ ...breach, time: 3 },
],
9,
);
expect(collapsed[0]).toMatchObject({ severity: "info", occurrences: 2 });
});
it("demotes single-sample coordinate-frame findings to info", () => {
for (const code of [
"escaped_container",
"panel_out_of_canvas",
"connector_detached",
] as const) {
const collapsed = collapseStaticLayoutIssues([{ ...issue(code, "warning"), time: 3 }], 9);
expect(collapsed[0]).toMatchObject({ severity: "info", occurrences: 1 });
}
});
it("keeps a held but small canvas_overflow at info", () => {
const breach = {
...issue("canvas_overflow", "info"),
overflow: { top: 30 },
containerRect: { left: 0, top: 0, right: 1920, bottom: 1080, width: 1920, height: 1080 },
};
const collapsed = collapseStaticLayoutIssues(
[
{ ...breach, time: 1 },
{ ...breach, time: 3 },
],
9,
);
expect(collapsed[0]).toMatchObject({ severity: "info", occurrences: 2 });
});
it("does not demote a finding held at every sample — persistence, not a single hit", () => {
const collapsed = collapseStaticLayoutIssues(
[
@@ -241,7 +305,7 @@ describe("persistence-tiered severity (#U10)", () => {
expect(collapsed[0]).toMatchObject({ severity: "error", occurrences: 3 });
});
it("only re-promotes content_overlap — other held codes keep their original severity", () => {
it("does not promote held codes without a promotion rule — container_overflow keeps its severity", () => {
const collapsed = collapseStaticLayoutIssues(
[
{ ...issue("container_overflow", "warning"), time: 3 },
+40 -7
View File
@@ -19,6 +19,10 @@ export type LayoutIssueCode =
| "text_not_painted"
| "caption_zone_collision"
| "frame_out_of_frame"
// Coordinate-frame findings — geometry computed in one frame, rendered in another.
| "escaped_container"
| "panel_out_of_canvas"
| "connector_detached"
// Frozen-sweep guard (#U10) — a whole-run meta-finding, not a per-sample
// geometry observation; never persistence-tiered (see `applyPersistenceTier`).
| "sweep_static"
@@ -203,6 +207,9 @@ const PERSISTENCE_TIERED_CODES: ReadonlySet<LayoutIssueCode> = new Set([
"container_overflow",
"content_overlap",
"text_occluded",
"escaped_container",
"panel_out_of_canvas",
"connector_detached",
]);
export function collapseStaticLayoutIssues(
@@ -255,12 +262,13 @@ export function collapseStaticLayoutIssues(
* Held-duration severity tiering (#U10). A finding observed at only one
* sample among several (held 0ms) is an entrance/exit transient, not a held
* defect demote to info so it stays in the data (verbose/--json output)
* without gating the run. `content_overlap` specifically re-promotes from
* warning to error once it's held long enough to be a real, sustained
* collision rather than a crossfade/transition blip (resolves the TODO in
* layout-audit.browser.js's `overlapIssue`). A finding held at every sample
* (a genuinely static defect) is well past both thresholds and is left
* untouched either way persistence, not the code, decides the tier.
* without gating the run. Two codes re-promote once held: `content_overlap`
* warning->error when the collision is sustained rather than a crossfade blip
* (resolves the TODO in layout-audit.browser.js's `overlapIssue`), and
* `canvas_overflow` info->warning when the breach is held, canvas-scale
* (>= 5% of the short edge) AND partially visible a fully off-canvas rect
* is a parked entrance, not drift. Codes without a promotion rule are left
* untouched when held persistence, not the code, decides their tier.
*/
function applyPersistenceTier(issue: LayoutIssue, multiSampleRun: boolean): LayoutIssue {
if (!multiSampleRun) return issue;
@@ -276,9 +284,33 @@ function applyPersistenceTier(issue: LayoutIssue, multiSampleRun: boolean): Layo
if (issue.code === "content_overlap" && isContentOverlapHeldLongEnough(issue, occurrences)) {
return { ...issue, severity: "error" };
}
if (issue.code === "canvas_overflow" && isCanvasBreachHeldLarge(issue, occurrences)) {
return { ...issue, severity: "warning" };
}
return issue;
}
// A held, canvas-scale, PARTIALLY visible breach is drift; a fully off-canvas rect is a parked entrance.
function isCanvasBreachHeldLarge(issue: LayoutIssue, occurrences: number): boolean {
if (
occurrences < HELD_ACROSS_SAMPLES_MIN_OCCURRENCES ||
!issue.overflow ||
!issue.containerRect
) {
return false;
}
const breach = Math.max(
...Object.values(issue.overflow).filter((value) => typeof value === "number"),
);
if (breach < Math.min(issue.containerRect.width, issue.containerRect.height) * 0.05) return false;
const container = issue.containerRect;
const overlapX =
Math.min(issue.rect.right, container.right) - Math.max(issue.rect.left, container.left);
const overlapY =
Math.min(issue.rect.bottom, container.bottom) - Math.max(issue.rect.top, container.top);
return overlapX > 0 && overlapY > 0;
}
// Split out of applyPersistenceTier so the two independent "held long enough"
// signals (sample count vs. wall-clock span) read as one boolean question
// instead of adding a third compound branch to the tiering ladder above.
@@ -326,7 +358,8 @@ function staticIssueKey(issue: LayoutIssue): string {
}
function framePositionKey(issue: LayoutIssue): string {
return issue.code === "frame_out_of_frame"
// 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"
? `${Math.round(issue.rect.left)},${Math.round(issue.rect.top)}`
: "";
}