fix(check): audit the composition when the motion sidecar will not parse

[xuanruli]

Problem

runCheckPipeline returned early on motion.kind === "invalid", handing buildReport an emptyBrowserResult(). The browser never opened, so the report affirmed sections nothing had examined:

Runtime
  ◇ 0 errors, 0 warnings
Layout
  ◇ 0 issues across 0 sample(s)

One composition, three sidecar states:

sidecar	layout findings	layout.duration
valid	8	6
none	8	6
one missing field	0	0

So the duration was fabricated too, not just the findings. A JSON typo in an optional add-on suppressed the mandatory core of the command — a project that gains a typo'd sidecar became strictly worse off than the same project with no sidecar at all.

Change

The parse error becomes a specFindings entry passed to buildReport as extraMotionFindings; the audit runs as usual. A sidecar says nothing about the composition — the HTML is fine, a JSON file beside it is not.

Nothing downstream reads the unparsed spec. Both motion.spec accesses (planMotionSampling, and the runAuditGrid evaluate branch) sit behind a kind === "valid" guard, and planMotionSampling returns an empty plan for anything else. Verified independently by a reviewer: layout output for an invalid sidecar is now byte-identical to the no-sidecar path (same 757-byte serialisation, IDENTICAL), motion.samples stays 0, and no motion frame is collected.

enabled / specPath are unchanged from main — the old early return already passed the invalid resolution to buildReport, so {enabled: true, specPath: "…", samples: 0, findings: [motion_spec_invalid]} is the same shape as before and now has a test pinning it.

Cannot flip a passing run

motion_spec_invalid is error-severity, so ok was already false and checkExitCode already returned 1. This only adds findings to an already-red report. Confirmed both ways with and without --strict.

Cost

One ordinary check. The expensive part — the up-to-300-seek motion loop (MOTION_FPS=20, MOTION_MAX_SAMPLES=300) — is produced solely by buildMotionSampleTimes, which planMotionSampling declines to call for a non-valid spec. What runs is the default 9-sample layout grid plus ≤5 contrast measurements: 4.66s measured, the same cost every no-sidecar project already pays. A reviewer specifically checked whether a "skip contrast when the spec is invalid" guard was warranted and argued against it — it would re-create this exact bug one section over.

Verification

• m-badspec-mask: 0 → 8 layout findings, alongside the motion_spec_invalid error; duration 0 → 6. Deterministic over 3 sequential runs.
• The other 11 motion fixtures and all 13 registry examples are byte-identical.
• A runtime page error and the parse error coexist — neither masks the other (on main the page error was not reported at all).
• The catch around runBrowserCheck still works: specFindings is computed before the try, so a mid-audit browser failure yields both check_runtime_failure and motion_spec_invalid.
• --snapshots with contrast enabled produces all 10 correctly-named PNGs and no crop for the spec error (findingAtRoot uses ZERO_BBOX, and selectFindingCropRequests requires a real bbox).
• No double-reporting: for an invalid spec browser.motionIssues is [], so motionFindings is exactly the one parse error.
• 2149 tests pass. The single failure (src/telemetry/agent_runtime.test.ts, agent-env detection) fails identically on main — this sandbox injects ~16 CLAUDE_CODE_* vars that drown its assertion. oxlint / oxfmt / tsc --noEmit clean.

Two tests added. The first pins the fix (it fails on main: expected "spy" to be called 1 times, but got 0 times). The second — does not sample motion for an invalid sidecar — passes on main too, so it is not a regression pin for this bug; it guards the new path, where deleting planMotionSampling's kind !== "valid" guard would now crash on motion.spec. Its comment says so.

Known follow-ups, deliberately not in this PR

The reporting layer has no "not run" state. printSection prints ◇ 0 errors, 0 warnings with a success glyph whenever findings is empty, regardless of whether the section ran; printContrastSection branches only on options.contrast, so an un-run pass prints ◇ 0/0 text checks pass WCAG AA — an affirmative WCAG claim from zero measurements. A per-section ran/skipped state would subsume this bug's path, the lint gate below, the browser-failure path, and --no-contrast's existing ◇ skipped. That is a report-schema change touching the JSON envelope and every printer — its own PR, and it would not have fixed this defect (8 real findings beat a correctly-labelled empty section).

The lint gate has the same symptom for a different reason. shouldBlockRender short-circuits on any lint error with {kind: "none"} hard-coded, so a project shipping a sidecar reports enabled: false, specPath: undefined, and the invalid sidecar is never mentioned. Left untouched on purpose: a lint error means the composition is structurally invalid, so browser results measured against it are noise rather than untrusted-but-useful input. That gate is deliberate, named, and shares the render gate — flipping it would smuggle a behaviour change with render blast radius into a reporting fix. Its residual dishonesty is the not-run problem above, not an early-return problem.

Sequencing

Composes with #2805 (ambiguous selector no longer zeroes the motion audit): that PR works inside planMotionSampling and the evaluate branch and preserves the kind !== "valid" early return this fix relies on. Only check.test.ts conflicts textually (both insert after the same line). This one is smaller — land it first and let #2805 rebase.

The highest-value remaining fix in this family is one wrong predicate: an opaque element parked off-canvas is treated as having "appeared" at t=0, because the visibility test has no viewport intersection. It poisons appearsBy, before and staysInFrame at once, so any slide-in entrance trips it, and it shares a root cause with motion_off_frame flagging full-bleed backgrounds — one predicate retires two false-positive classes.

🤖 Generated with Claude Code