fix(check): keep evaluating a motion spec past an ambiguous selector

[xuanruli]

Problem

One selector matching more than one element disabled the whole motion audit. planMotionSampling set times = [], and the evaluate step was skipped a second time by a motionIssues.length === 0 guard — so every other assertion in the sidecar went unevaluated while report.motion.enabled still read true.

Two fixtures with identical HTML:

fixture	spec	result on main
m-selector	3 assertions, none ambiguous	samples=121, reports motion_selector_missing #typo-does-not-exist
m-ambig	adds one ambiguous .item	samples=0, reports only motion_selector_ambiguous — the missing-selector finding disappears

A single typo silently switched off the rest of the checks.

Change

Only the assertions naming an ambiguous selector are dropped. The rest are sampled and evaluated, and their findings are appended to the preflight ones instead of replacing them.

Filtering must not be silent either. samples === 0 was the only machine-readable signal that part of a spec went unevaluated, and sampling again removes it — that would have traded one silent skip for another, one level down. The ambiguity finding now names what it cost:

error motion_selector_ambiguous | .item matches multiple elements — use a more specific
  selector so the assertion targets exactly one 1 assertion(s) naming it were not evaluated.
error motion_selector_missing  | #typo-does-not-exist matched no element in any sampled frame

The selector-to-assertion mapping is single-sourced. assertionTargets lives in motionAudit.ts next to collectSamplingTargets, which now folds over it, behind an exhaustive switch. Verified: adding a fifth MotionAssertion kind produces a compile error rather than a silent gap — without that, a new kind would have its ambiguous selector reported as ambiguous and still evaluated against an arbitrary first match, resurrecting exactly this bug for the new kind.

Verification

m-ambig now reports samples=121 with both findings. The other 11 motion fixtures keep their verdicts byte-for-byte; 13 registry examples are identical (they carry no sidecar, so the motion section is disabled there). 2150 tests pass — the one failure (src/telemetry/agent_runtime.test.ts, agent-env detection) reproduces on clean HEAD and is environment-dependent. oxlint / oxfmt / tsc --noEmit clean.

Reviewers independently confirmed the parts I was least sure of:

• preflightIssues[].selector is the verbatim spec string (a deliberately double-spaced #list .item round-trips unchanged), so the filter keys correctly; and a dropped assertion is genuinely not evaluated — a spec with an ambiguous selector plus a bySec that would fail produced no finding for it.
• Sampling-target shrinkage cannot strand a surviving assertion: targets are derived from the surviving list, so plan.selectors is always a superset of what they need.
• No duplicate findings: ambiguous selectors are no longer in plan.selectors, so evaluateMotion never sees them.

Strictly louder, never newly-breaking

Reaching this path means a hardcoded-error motion_selector_ambiguous was already present, so the run had already failed — the change cannot flip a passing check to failing. What it does is front-load errors the author would have hit on the next run anyway.

Worth knowing: users with an ambiguous selector will now see additional real errors they had never been shown, and the motion grid is sampled again in that case (up to MOTION_MAX_SAMPLES seeks; --caption-zone / --frame-check findings can also increase, since those run at motion sample times). Layout findings cannot change — collectLayout is gated on the layout sample set.

Related, not fixed here

Eight other defects in this family are confirmed and left for follow-ups. Two are worth naming because they interact with this one:

• An invalid sidecar returns before the browser opens, so a JSON typo prints 0 errors, 0 warnings for Layout, Runtime and Contrast — an affirmatively false clean bill of health for four sections (measured: 8 layout findings on the same composition become 0). Same principle as this PR, bigger blast radius; a reviewer argued it is the more urgent next PR and I agree.
• An opaque element parked off-canvas is treated as having "appeared" at 0s, because isVisibleElement has no viewport test — so any slide-in entrance breaks both before and staysInFrame. This PR increases how often that is seen (it was accidentally masked whenever the grid was zeroed), which raises its priority; it shares a root cause with motion_off_frame flagging full-bleed backgrounds and the two should land together.

Also pre-existing and untouched: an ambiguous keepsMoving scope is never preflighted (scopes do not go through findAmbiguousSelectors) and reports a factually false motion_selector_missing. Confirmed identical on main when a scope is the only ambiguity, so this change does not widen it — but it becomes reachable in one more configuration.

🤖 Generated with Claude Code