fix(cli): detect opacity-swapped raster slices

[miguel-heygen]

Summary

• include stable element identity in the layout sweep fingerprint
• detect opacity-only swaps between same-sized raster slices
• add a regression test for the reporter-shaped false positive

Reproduction

A 6s composition dynamically created two same-box <img> slices and swapped their opacity at 3s. hyperframes@0.7.58 check --json sampled changing content but emitted sweep_static because both visible states fingerprinted only as 80,60,480,240,1.

Verification

• red-first regression: new test failed with identical before/after fingerprints
• layout-audit.browser.test.ts: 72/72 passing
• oxfmt --check on changed files
• node --check packages/cli/src/commands/layout-audit.browser.js
• git diff --check

Notes

The fix does not raster-read every image. It extends the existing geometry/opacity fingerprint with the already-canonical selectorFor(element), so opacity swaps between distinct DOM slices are detected without adding per-sample image decode cost.

[vanceingalls]

RIGHT direction — the geometry+opacity fingerprint had a real coverage gap on opacity-swapped raster slices, adding stable element identity to the fingerprint is the right shape, the red-first regression test is on the same shape as the reporter. Should merge after addressing one extrapolation gap below.

First substantive review on this PR.

Strengths

• layout-audit.browser.js:1476 — extending the existing fingerprint with selectorFor(element) is cheaper and more targeted than the alternative (per-sample <img> decode). The added field is monotonic w.r.t. the original fingerprint (prepended field), so the original static-detect case is preserved: static compositions still hash identically → sweep_static still fires.
• Red-first regression test at layout-audit.browser.test.ts:66-99 reproduces the exact fingerprint collision (80,60,480,240,1 on both sides of the swap) with the getComputedStyle proxy plumbing inline opacity through to opacityChain. The test would have failed pre-fix.
• mediaPixelHash remains untouched — pixel-only media motion detection isn't destabilized by the identity addition.

Findings

🟠 important — class-only sibling shape still false-positives (extrapolation-blocker)

layout-audit.browser.js:1476 uses selectorFor(element), which returns \${tagname}.${classes}`(line 71-74) as its fallback whenever the element lacksidand lacksdata-layout-name/data-composition-id/data-start`. Two id-less, data-attr-less, same-tag, same-classed siblings sharing the same box therefore still produce identical fingerprints across an opacity swap — the exact bug shape the PR is fixing, just one variant over.

Concrete failing shape (siblings of the reporter's shape):

<img class="raster-slice" style="opacity: 1" />  <!-- selectorFor → "img.raster-slice" -->
<img class="raster-slice" style="opacity: 0" />  <!-- selectorFor → "img.raster-slice" -->

Both fingerprint to img.raster-slice,80,60,480,240,1. isVisibleElement drops the opacity-0 sibling in both samples, so the visible-element list has one entry either side of the swap → identical fingerprint → sweep_static false-positive persists. Dynamically-created raster slices (the failure mode named in the PR) are the shape most likely to lack ids in the wild.

The file already has the primitive for this. uniqueSelectorFor at :84 explicitly falls through to structural nth-of-type disambiguation when document.querySelectorAll(preferred).length !== 1 — precisely the identity contract the fingerprint needs. It's already used for the identity-context site at :1334 (occlusion reporting). Swapping selectorFor → uniqueSelectorFor at :1476 is a one-token change that closes the class-collision variant without expanding scope. Perf overhead: one querySelectorAll(preferred) per element per sample, matching what selectorFor itself already does for the data-* attribute path at :68, so no new order-of-magnitude cost.

Under the "if the fix extrapolates to another site in the codebase, close the class not the case" discipline, this is REQUEST_CHANGES-tier rather than follow-up — the extrapolation is one line away.

🟠 important — test only parameterizes the id shape

layout-audit.browser.test.ts:66-99 covers the #slice-a / #slice-b case. Any of the following would also merit a case (and the class-only one currently fails at head, which is what surfaces finding 1):

• id-only (present)
• data-layout-name-only
• class-only (fails at head → confirms the extrapolation)
• id-less, dataless, classless siblings (structural fallback — passes)

Recommend it.each over shape descriptor, share the getComputedStyle proxy in a helper. Under the same extrapolation-blocker rule as (1) — a regression test on a branch-shaped fix must exercise each branch.

Adversarial

• ✅ Static-detect preservation. Prepending selectorFor(...) is monotonic; a static composition still hashes identically across samples. sweep_static still fires on the original target.
• ✅ Mid-swap frame states. round(opacityChain) at 0.01 precision + opacityChain < 0.2 visibility floor together mean two mid-transition frames only collide if both slices happen to hold identical opacities across two consecutive samples, which requires a paused animation and thus is definitionally "static-ish."
• ⚠️ Class-collision variant — covered in finding 1.
• ✅ Same-box canvas / video swap. Out of scope — covered by mediaPixelHash (:1478-1481), which sees pixel content differences even when selector collides.

Verdict: REQUEST CHANGES
Reasoning: RIGHT-direction fix, but the same reported failure class (opacity-swapped raster slices) survives one variant over (id-less same-classed siblings) because the fingerprint uses selectorFor rather than the identity-safe uniqueSelectorFor already in-file. One-line change + test parameterization closes the class.

— Via

[james-russo-rames-d-jusso]

Reviewed at 82809a3.

Extending the sweep fingerprint with selectorFor(element) (id-first, then data-attrs, then tag.class, then structural fallback) as the first field of each per-element part string. Two same-box <img> slices swapping opacity now produce distinguishable fingerprints (#slice-a,80,60,480,240,1 → #slice-b,80,60,480,240,1) rather than collapsing to the false-positive 80,60,480,240,1 on both sides.

Blockers

(none)

Concerns

(none)

Nits

🟡 Merge order vs. #2418. #2418 (also touching the fingerprint block at layout-audit.browser.js:1470-1479) updated the sweep-static docstring and added an opacity-reveal regression test. This PR touches the same lines. Whichever lands second will need a trivial rebase to keep both the docstring update ("identity + box + opacity, in DOM order") and both regression tests. Zero-conflict merge if #2418 lands first (the new test is additive); trivial if this lands first.

Green notes

🟢 Fingerprint change is minimal and correct. selectorFor is deterministic per-element (id → data-attrs → class-list → structural nth-of-type) and stable across seek samples for the same DOM node. Two visually-swapped raster slices with distinct ids/data-attrs/classes now hash differently. ✓

🟢 Test uses a Proxy over getComputedStyle to bridge happy-dom's inline-style handling. inlineOpacity = (element as HTMLElement).style.opacity → Proxy returns that for the opacity property, everything else falls through to the native computed style. Correct workaround for happy-dom's opacity behavior; the same pattern would work for #2418's opacity-reveal fixture (using a getter-based mock is equivalent).

🟢 Red-first shape verified. Pre-PR, both slice fingerprints collapse to 80,60,480,240,1 (single opaque field), so after != before fails. Post-PR, identity distinguishes them → test passes. Real regression pin, not a re-assertion.

🟢 No new false negatives. selectorFor returns something semi-unique per element; two elements at identical geometry+opacity WITHOUT distinguishing identity would need to be genuinely indistinguishable siblings — the structural fallback (> tag:nth-of-type(N)) still separates them by position. Compositions that legitimately have "one element static across the whole seek" still hash identically as before.

🟢 Perf note stands. The PR body's note that this doesn't add per-sample image decode (uses the already-canonical selectorFor) is correct — selectorFor is a synchronous DOM walk, no raster access.

🟢 CI green at 82809a3.

Verdict framing

Small, exact fix for the reporter's shape. The identity extension is the right primitive — cheaper than any per-sample pixel probe, and it composes naturally with the existing box+opacity fields. LGTM from my side; the #2418 merge-order note is the only follow-up.

— Review by Rames D Jusso

[miguel-heygen]

Class-only sibling shape still false-positives; use the existing structural selector fallback and parameterize identity branches.

Addressed at 1a0222d4c: the sweep fingerprint now uses uniqueSelectorFor(element), and the regression matrix covers id, data-layout-name, shared-class, and structural-only siblings. Red-first proof: the shared-class case failed at the previous head with identical img.raster-slice,80,60,480,240,1 fingerprints. Verification: focused matrix 4/4, full layout-audit browser suite 75/75, CLI typecheck, lint, format, fallow, and tracked-artifact hooks pass.