fix(cli): exclude clip-path-hidden text from inspect layout and contrast audits

[miguel-heygen]

Problem

The inspect layout audit and the validate contrast auditor false-positive on text that is visually hidden by a clip-path at the sampled timestamp. Authors hit this with typewriter / staged-reveal spans (clipped to nothing pre-reveal) and worked around it with data-layout-allow-overlap / data-layout-ignore. Two symptoms:

• a fully visible block flagged as content_overlap against a clip-path-hidden sibling that paints nothing;
• a clip-path-hidden span reported as a WCAG contrast failure (measured background-on-background, ~1:1 ratio).

Root cause

Both auditors already excluded opacity:0, visibility:hidden and display:none from collection, but neither accounted for clip-path. A clip-path (e.g. inset(0 100% 0 0) or circle(0px)) shrinks an element's painted region to nothing while its layout box, opacity, visibility and display all still read as present, so the element survived the visibility filter and was measured even though it paints zero pixels.

Repro

Composition with, at the sampled time: a visible block sharing a box with a clip-path: inset(0 100% 0 0) block, and white-on-white text inside a fully-clipped span. Before the fix, inspect emits content_overlap referencing the clipped sibling and the contrast pass reports a ~1:1 failure on the clipped text. The pre-existing opacity:0 / visibility:hidden / display:none cases were already filtered; only clip-path leaked through.

Fix

Add a shared check at the in-page collection chokepoint: when a non-none clip-path is in effect on the element or an ancestor, probe a grid of points across the element's box with elementFromPoint; if none resolve to the element or a descendant, it is clipped to nothing and is skipped. clip-path drives hit-testing, so a fully-clipped element is unreachable across its box, while a genuinely occluded (but unclipped) element still resolves and is still measured. The probe runs only when a clip-path is present, so occlusion detection for normal elements is untouched. Applied in both layout-audit.browser.js (covers content_overlap and text_occluded via isVisibleElement) and contrast-audit.browser.js, kept consistent.

The probe grid is intentionally finite; edge strips narrower than the nearest probe point are treated as clipped away to avoid noisy typewriter pre-reveal reports. If a real visible-strip bug appears, adding edge probes is the follow-up.

data-layout-allow-overlap / data-layout-ignore are honored exactly as before; genuine overlaps between visible elements are still flagged.

Verification

• New focused unit tests assert a clip-path-cleared block is excluded from content_overlap, a partially clipped but still-painted block still reports real overlap, and clip-path-hidden text is excluded from contrast reports.
• End-to-end against headless Chrome: clip-path / opacity:0 / visibility:hidden / display:none false positives are gone; the visible-vs-visible control overlap is still reported.
• bun run --filter @hyperframes/cli test -- src/commands/layout-audit.browser.test.ts green locally (16 tests).
• oxfmt / oxlint clean; pre-commit fallow / typecheck / commitlint gates pass.

The two audit scripts and the layout-audit test are added to the fallow ignore lists: their pre-existing IIFE-level complexity and per-rule test scaffold re-flag under the line-shift fingerprint when the small probe helpers are inserted (the same documented handling used elsewhere in the config). The new helpers themselves are all at the lowest complexity tier.

[james-russo-rames-d-jusso]

Reviewed at c0434875aa9497e0e2fa30f69a6fe4ae40c5b1a7. Diff is 124 additions / 3 deletions across .fallowrc.jsonc (fallow ignore entries), layout-audit.browser.js, contrast-audit.browser.js, and layout-audit.browser.test.ts. Symptom (typewriter/staged-reveal spans clipped to nothing triggering content_overlap + WCAG contrast failures) is real and well-diagnosed, and the shared-chokepoint approach (hit-test with elementFromPoint on a probe grid, only when a clip-path is in effect) is the right one — geometry-empirical over shape-parsing. Two concerns, one nit.

🟡 Test coverage covers the true-positive-suppression direction but not the false-positive-suppression direction

Per feedback_defensive_bug_pinning_tests_fp_suppression (my own operating memo, applied here): a false-positive-suppression PR needs tests in both directions:

1. Correctly excludes fully-clipped text from the audit — ✅ this PR adds it at layout-audit.browser.test.ts:172-183 (the new "excludes a block clipped to nothing by clip-path from overlap" case).
2. Still catches clip-path-present-but-not-fully-clipping text — ❌ no test.

Without direction (2), a future refactor of isClippedAway that accidentally over-suppresses (e.g. returns true whenever hasClipPath is true, skipping the probe entirely) has no test failure to catch it. Suggested addition: a scene where element A has clip-path: inset(0 25% 0 0) (75% visible), overlaps element B, and the probe grid hits at least one point inside the visible 75%. Assert content_overlap IS still reported. Same shape as the existing test, just with a partial clip.

Contrast-audit gets no new test at all in this PR — even the true-positive-suppression direction is absent for contrast-audit.browser.js. Given the same shared helper (isClippedAway) is applied to both audits at their collection chokepoints, a mirror test on the contrast side would cheaply cover the "clip-path-hidden text no longer reports ~1:1 contrast failure" case the PR body describes.

🟡 Probe grid is coarse for extreme clip shapes — a narrow visible strip can still be excluded

The probe grid is 5×3 at cols [0.05, 0.25, 0.5, 0.75, 0.95] × rows [0.25, 0.5, 0.75]. A clip-path: inset(0 99% 0 0) (1% visible strip on the left) has zero probe points inside the visible region — the closest column is 0.05, and the visible region is 0..0.01. That element would be classified as fully-clipped and skipped.

For contrast this is generally fine — 1% of a text element's width is well below WCAG minimum viable letter width; users won't read that. For layout overflow, though, a 1% visible strip that overflows its container is still a real overflow bug the audit should surface. The current grid can't see it.

Two options:

1. Accept the coarse grid: document it. The isClippedAway comment says "probe a grid of points across the element's box"; a follow-up sentence like "elements with less than ~5% visible width or height on any axis fall below the probe resolution and are treated as clipped-away" would flag the limitation for the next reader.
2. Sample denser near the box edges: the failure mode is edge-hugging strips (typewriter-style clips are ALL edge-hugging). Adding probes at 0.01 and 0.99 (or lowest-percent that satisfies pixel geometry given the element's box size) would catch the narrow strips. Not free — 5 more probe points per element per audit run. Might not be worth the cost.

I'd lean (1) — document the limitation. If a real user hits it, (2) is a cheap follow-up. But knowing about the failure mode is better than "we assert this handles clip-path" and then a specific shape leaks a false positive later.

🟢 Sibling audits are consistently extended

Both layout-audit.browser.js (all layout checks: overflow, overlap, occlusion) and contrast-audit.browser.js get the same helper trio (hasClipPath / paintsAnyProbePoint / isClippedAway). No asymmetry between visual audits. Per feedback_sibling_asymmetry_as_security_evidence, this is the correct shape: 2/2 visual audits, both applying the same exclusion.

Scoping to visual audits only is also correct — text clipped away by clip-path is still readable by screen readers unless aria-hidden is set. If future a11y/ARIA audits land in this codebase, they should NOT inherit this exclusion. Nothing in this PR risks that (it's confined to layout-audit.browser.js and contrast-audit.browser.js), but worth flagging so the next audit author reads this and gets the scope right.

🟢 Detection covers all clip-path shapes without shape-parsing

getComputedStyle(...).clipPath !== "none" catches every clip-path form: inset(), circle(), polygon(), path(), url(#svg-clip). Then the empirical elementFromPoint probe determines actual visibility — no need to parse the shape geometry. This is exactly the right level of abstraction.

The ancestor walk in hasClipPath (loop up parentElement) correctly catches inherited clip contexts. clip-path doesn't inherit through the CSS cascade in the usual sense, but a parent with clip-path DOES visually clip descendants (SVG-style), so walking up is correct.

🟢 data-layout-allow-overlap / data-layout-ignore opt-outs are preserved

The PR body claims these are honored exactly as before, and the code confirms it — hasIgnoreFlag(element) is checked at isVisibleElement's top before the clip-path check runs. The existing test respects the data-layout-allow-overlap opt-out still passes.

Authors who previously worked around this bug with data-layout-allow-overlap on their typewriter spans should be able to remove those workarounds after this lands. Worth calling out in the PR body / release notes ("if you added data-layout-allow-overlap to a clip-path-hidden element as a workaround, you can safely remove it after this ships") — otherwise those attrs accumulate as dead code and become confusing to the next author.

🟢 Fallow ignore additions are legitimate

The two .fallowrc.jsonc additions match a documented pattern used elsewhere in the config — the line-shift fingerprint re-flags pre-existing complexity when helpers are inserted. Both layout-audit.browser.js and contrast-audit.browser.js have pre-existing browser-side IIFE structure that this refactor doesn't rearchitect, and the new helpers (hasClipPath / paintsAnyProbePoint / isClippedAway) are all at the lowest cyclomatic tier. The layout-audit.browser.test.ts clone-group entry is similarly consistent with the existing per-audit-rule scaffold.

Nits

• hasClipPath at layout-audit.browser.js:106 and contrast-audit.browser.js:48 — the two implementations are byte-identical except for const/var and parentElement capitalization. Both audits load as raw strings and inject via page.addScriptTag, so cross-file sharing at runtime isn't easy, but future readers may wonder if these are supposed to stay in sync. A one-line comment at each site pointing at the sibling would help ("mirror of contrast-audit.browser.js's hasClipPath; keep in sync").
• CLIP_PROBE_COLS / CLIP_PROBE_ROWS — same values in both files. Same drift risk. Consider a comment or // keep-in-sync-with: contrast-audit.browser.js:CLIP_PROBE_COLS.
• The comment block in layout-audit.browser.js:100-107 is genuinely helpful for the next reader — thank you for writing it. Consider mirroring the same explanation verbatim in contrast-audit.browser.js (currently the shorter comment there references the pattern but lightly).

What I didn't verify

• End-to-end against headless Chrome — the PR body claims the E2E false positives are gone; I trust that.
• Whether the mocked elementFromPoint in auditOverlapScene accurately represents Chrome's behavior for a clip-path: inset(...) box. The mock returns block-b unconditionally when b has no clip-path — which is the "b covers a and is visible" case — but the code under test relies on elementFromPoint returning null or a NON-descendant when the probed point is on a clipped-away element. Verified by inspection of the mock (clipPaths.b === "none" fallback returns block b, else block a, else null) — that IS the right shape for the fully-clipped case. But I did not confirm what real Chrome returns for a probe point in the middle of a circle(0px)'d element (my expectation: the underlying element or null depending on stacking).
• Dynamic clip-path animation timing — the audit samples at a fixed time; a clip-path animated in/out across the composition timeline may be visible at some time in the range but invisible at inspect-time. Out of scope for this PR (audit already samples per-time), but a good follow-up filed as "animation-aware audit" if worth it.

Non-blocking overall — the approach is right, the sibling audits are consistently extended, and the FP suppression closes a documented author workaround. Add the false-positive-not-suppressed direction test and a note about the probe-grid resolution limit and this ships confidently.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

R2 @ 1e98c3ff3440ac1b26f375ce3cbbc85571d0b503 — verifying against my R1 at c0434875.

R1 findings

🟡 (a) FP-suppression negative direction not tested → ✅ RESOLVED

New test at layout-audit.browser.test.ts:190-199:

it("still flags overlap when clip-path leaves painted text visible", () => {
  const issues = auditOverlapScene({
    a: { textRect: rect({ left: 100, top: 100, width: 400, height: 100 }) },
    b: {
      textRect: rect({ left: 300, top: 120, width: 400, height: 100 }),
      clipPath: "inset(0px 25% 0px 0px)",  // 75% visible
    },
  });
  expect(issues.some((issue) => issue.code === "content_overlap")).toBe(true);
});

Exactly the shape I asked for — 75%-visible element still gets caught. Mock updated (isFullyClipped at layout-audit.browser.test.ts:315-317 matches only 100% inset / circle(0px), so partial clips fall through and elementFromPoint returns the visible block). Bug-pinning test is in place; a future refactor that over-suppresses gets a red on this.

🟡 (a) Contrast-audit gets no test at all → ✅ RESOLVED

New describe("contrast-audit.browser clip-path visibility") block at layout-audit.browser.test.ts:203-243. Test asserts await __contrastAudit(...) returns [] when the headline element has clip-path: inset(0px 100% 0px 0px), mocked elementFromPoint returns null, and a full-white canvas would otherwise produce a low-contrast report. Mirror shape to the layout test — same helper, same assertion pattern. Full test scaffold (installContrastScript, runContrastAudit) at test.ts:449-478. Solid.

🟡 (b) Probe grid coarse, narrow-strip failure mode not documented → ✅ RESOLVED (as documentation)

Both audit files now carry an explicit note about the resolution floor:

• contrast-audit.browser.js:57-59: "Edge strips narrower than the nearest probe point are treated as clipped away to avoid noisy typewriter pre-reveal contrast reports."
• layout-audit.browser.js:117-120: "Probe resolution intentionally treats edge strips narrower than the nearest probe point as clipped away. That avoids noisy reports for typewriter pre-reveal states; if a real visible-strip bug appears, add edge probes here before widening the audit surface."

The layout-audit comment is the sharper of the two — it names the trade-off (noise vs. narrow-strip coverage) AND the escape hatch (add edge probes) for the next reader who hits the narrow-strip case. That's the shape I was after; the specific 99% inset number isn't cited but the resolution-floor concept is captured plainly. Credit as resolved per feedback_open_item_alternative_resolution — operational goal (next author sees the limit + how to widen if needed) met.

Nits from R1

• hasClipPath cross-file byte-identical duplication — ⚠️ still not cross-linked with a "keep-in-sync" comment. Nit-level, non-blocking; the two files continue to load as raw strings via page.addScriptTag, and the layout-audit test now reads the contrast-audit script for mirror testing so drift risk is slightly reduced (a divergence would show up as one test file failing).

Adjacent-file check

The isFullyClipped helper in the test (layout-audit.browser.test.ts:315-317) uses /inset\([^)]*100%|circle\(0px/i.test(clipPath) to decide when the mocked elementFromPoint should return the block. That regex has a subtle asymmetry: inset(0 100%) matches (fully clipped horizontally) but inset(100% 0) doesn't (fully clipped vertically — 100% appears at a position the regex doesn't gate on). Not a bug for THIS PR's tests (only inset-100%-right is used), but a future test author extending with inset(100% 0 0 0) would silently get a "not clipped" mock. Non-blocking nit; consider tightening the regex or replacing with an explicit enum of clipped-shape strings if the test grows.

CI status

BLOCKED (needs 1 approval), MERGEABLE. All required checks green as of head SHA; CodeQL javascript-typescript still IN_PROGRESS, plus Windows render + a few CI jobs finishing on the R2 push.

Peer reviews since R1

None. Only my R1 review on the record.

R2 verdict

LGTM — all three 🟡 findings resolved with the correct shape (bug-pinning test in the neglected direction, mirror test on the sibling audit, resolution-floor documentation with an escape hatch for future authors). Non-blocking nits (byte-identical helpers cross-link, mock-helper regex asymmetry) can ride a follow-up.

— Review by Rames D Jusso