Skip to content

feat(cli): declarative motion verification in inspect (#1437)#1459

Merged
miguel-heygen merged 1 commit into
mainfrom
feat/motion-verification-inspect
Jun 15, 2026
Merged

feat(cli): declarative motion verification in inspect (#1437)#1459
miguel-heygen merged 1 commit into
mainfrom
feat/motion-verification-inspect

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

Closes #1437.

Why this matters

The hardest class of agent-authored bug is render ≠ preview: animation that looks right while scrubbing the studio but is wrong in the rendered MP4, because the renderer seeks a paused timeline instead of playing it forward. A forward-only entrance reveal the seek lands past, a broken stagger, an element drifting off-frame mid-tween, a shot that silently freezes — lint and inspect's layout sweep can't see any of it, and the only feedback today is "render the MP4 and watch it." That's exactly the step that doesn't scale for agents (cf. #911, #1174, #1387 — all preview-vs-render divergences).

This adds the missing lever: state the motion intent, and have it checked against the same seeked timeline the renderer uses — no extra Chrome launch, no render, no guesswork.

What it does

Drop a *.motion.json sidecar next to a composition. inspect discovers it automatically (no flag), seeks a dense time grid over the asserted selectors, and verifies four framework-agnostic assertions:

{
  "duration": 6,
  "assertions": [
    { "kind": "appearsBy", "selector": "#headline", "bySec": 0.5 },
    { "kind": "before", "a": "#headline", "b": "#cta" },
    { "kind": "staysInFrame", "selector": ".card" },
    { "kind": "keepsMoving", "withinSelector": ".scene" }
  ]
}
Assertion Fails (code) when
appearsBy(selector, bySec) not visible (opacity ≥ 0.5) by bySec — the seek skipped a reveal (motion_appears_late)
before(a, b) a doesn't first appear strictly before b — broken stagger order (motion_out_of_order)
staysInFrame(selector) once visible, its box leaves the canvas — off-frame drift (motion_off_frame)
keepsMoving(withinSelector?) a fully-static window exceeds maxStaticSec (default 2s) — frozen shot (motion_frozen)

A selector matching nothing is reported as motion_selector_missing rather than silently passing, so a typo'd selector fails loudly. duration, withinSelector, and maxStaticSec are optional.

This implements the maintainer's scoping decisions on the RFC: (1) extend inspect rather than add a verify verb, (2) sidecar JSON with no authoring-framework changes, (3) error severity by default.

How an agent/human uses it (the feedback loop)

lintvalidate → author a *.motion.json stating what the motion should do → inspect (auto-discovers it, errors on divergence, non-zero exit) → fix → repeat. Findings appear in both the human output and --json, in the same shape and envelope as layout findings.

◆  Inspecting layout for my-project (9 timeline samples + motion spec (2 assertion(s)))
  ✗ t=0.83s motion_appears_late #headline — appears at 0.83s but should be visible by 0.5s (check its entrance reveal fires under seek)

How it works

  • Browser samples; Node decides. A thin in-page sampler (motion-sample.browser.js) reads {rect, opacity, visible} per selector at the current seek, plus a per-scope liveness signature (visible elements' positions bucketed to 2px / opacity to 0.08) for keepsMoving. All assertion logic lives in evaluateMotion, a pure function over the collected element × time matrix — fully unit-testable without a browser.
  • Temporal, not per-frame. Unlike the layout audit (each frame checked independently), motion assertions are claims about a time series, so the pass collects the whole matrix on a dedicated dense grid (fps-based, capped at 300 samples) before evaluating — layout's 9 samples can't answer "visible by 0.5s."
  • Reuses the finding pipeline. Motion findings are LayoutIssues with four new motion_* codes; dedupe, static-collapse, limit, the JSON envelope, the human formatter, and exit-code logic all work unchanged.
  • Opt-in by presence. No sidecar → the dense pass never runs and inspect output is byte-identical to before. Sidecar present → the pass runs in the layout audit's existing Chrome session (no extra launch).

Builds directly on the layout-audit work in #1435 / #1436 — same seek path, same finding shape, same JSON envelope.

Scope

All changes are in packages/cli — no engine, runtime, or authoring-framework changes.

Deferred (one-liners for later, if wanted): --assert <path> for a non-sidecar location, --motion-fps / --motion-samples tuning, and assertion kinds beyond the four. Beat/music-relative timing (onBeat) is out of scope per the RFC.

Docs

  • docs/packages/cli.mdx — Motion verification section under inspect.
  • skills/hyperframes-cli/ — the dev-loop skill now documents the sidecar spec and the loop, so agents reach for it instead of eyeballing renders.

Testing

  • 29 new unit tests (spec parse/discovery, the four evaluators incl. edge/error paths, the in-page sampler via happy-dom); full CLI suite 835/835 green.
  • End-to-end verified against a real composition: no-sidecar output byte-identical to before; passing spec → exit 0; missing selector → motion_selector_missing + exit 1; invalid spec → clean error + exit 1.
  • Typecheck, oxlint, oxfmt, and the fallow complexity/dead-code/duplication gate all pass.

@mintlify

mintlify Bot commented Jun 15, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
hyperframes 🟢 Ready View Preview Jun 15, 2026, 4:58 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified at HEAD 6f0a607.

Reviewed against the canonical engineering rubric (CLI ergonomics, code health, test coverage, schema/forward-compat, security/output stability). HF runtime-interop semantics (__hf / __player / __timelines lookups, seek-pause contract, sampler's interaction with the rendered timeline) are explicitly deferred to Vai's parallel pass.

Net read: well-shaped feature, clean lane separation (browser samples / Node decides), good test coverage on the pure evaluator. Two real concerns worth addressing before merge, plus a handful of smaller items.

Concerns

1. Multi-match selectors silently sample only the first element. packages/cli/src/commands/motion-sample.browser.js:100 does document.querySelector(selector) and the inline comment ("first match only; assertions name single elements by design") asserts a single-element contract — but docs/packages/cli.mdx:594 advertises .card (class selector) as an example assertion target, and staysInFrame(".card") reads naturally as "no .card drifts off-frame." With the current impl, if there are 3 .cards and only the second drifts off-canvas, the run silently passes. Two reasonable fixes — pick one and document it: either (a) reject multi-match selectors at parse time / sample time with a clear motion_selector_ambiguous finding so the author has to disambiguate (.card:nth-of-type(2)), or (b) iterate all matches and fail if any match violates the assertion. Today's "first match wins" silently under-reports and is the trap class this PR is specifically trying to close.

2. No version field on the motion sidecar — strict unknown assertion kind rejection means a forward-evolving spec breaks old CLIs hard. packages/cli/src/utils/motionSpec.ts:70 rejects unknown kinds outright. That's the right default (typo'd appearBy shouldn't silently no-op), but with no version field on the sidecar and the PR body already listing future assertion kinds as deferred, a v2 spec that adds onBeat will hard-fail on every pre-rollout CLI. Suggest adding an optional version: 1 field now (validated when present, defaulted when absent) so future authors can opt their spec into "best-effort, warn on unknown kinds for kinds added after the CLI's version." Cheap to add now, painful to bolt on later. The JSON envelope's schemaVersion: 1 (packages/cli/src/commands/layout.ts:27) is a separate concept from the sidecar's authoring schema — both deserve to exist.

Nits

  • packages/cli/src/utils/motionAudit.ts:128staysInFrame considers a sample "visible" if sample.visible is true, but isVisibleElement in the browser sampler accepts opacity down to 0.2, while appearsBy uses the stricter APPEAR_OPACITY = 0.5 gate. So an element can appearBy at t=0.5s (opacity 0.6) then drop to opacity 0.3 at t=1.5s while drifting off-canvas, and staysInFrame will still fire. Defensible (the box really is off-screen and partially visible), but worth either gating staysInFrame on the same APPEAR_OPACITY threshold or documenting "once visible" as opacity ≥ 0.2 to match the implementation. (nit)
  • packages/cli/src/commands/motion-sample.browser.js:79root.querySelectorAll("*") per liveness frame is O(DOM size) × 300 samples in the worst case. Plausibly fine for typical compositions; flagging in case anyone wires this into a heavy-DOM stress test. Could cap with root.querySelectorAll("*[data-hf-track], *[id], *[class]") or similar if it ever bites. (nit)
  • packages/cli/src/utils/motionSpec.ts:78duration validation rejects <= 0 but parseMotionSpec accepts NaN if it slips past typeof === "number"... actually NaN <= 0 is false, so NaN would pass the duration check. Tiny edge but worth adding Number.isFinite like the isPositive helper already does for maxStaticSec. (nit)
  • packages/cli/src/utils/motionAudit.ts:181lastTime = frame.time reassigned every loop iteration; pulling it outside the loop tail (or just using frames[frames.length - 1].time) is microscopically clearer. (nit)
  • findMotionSpec ties tiebreak to "first matching html basename" then "first alphabetical" — solid for single-composition projects, but a project with two compositions (hero.html + landing.html) plus two sidecars (hero.motion.json + landing.motion.json) will pick whichever sorts first alphabetically. The deferred --assert <path> flag mentioned in the PR body handles this; just worth being explicit in the docs that "one composition per project for motion verification today." (nit)

Questions

  • The JSON envelope adds motionSpec (path) and motionSamples (count) fields conditionally without bumping schemaVersion. Additive is generally safe for consumers using property existence, but if anything downstream (CI parser, agent loop) does strict schema validation, this is a quiet break. Intentional to keep schemaVersion: 1? If so, worth a comment near the constant about "additive-only" policy.
  • Tests cover the four evaluator kinds and the in-page sampler via happy-dom, but I didn't spot a test for resolveMotionSpec's exit-1-with-clean-error path on an invalid sidecar — the PR body claims it's e2e-verified, just confirming this isn't unit-tested intentionally (since it process.exits).
  • keepsMoving uses "*" as the magic "whole canvas" scope key. If a user writes a literal CSS universal-selector assertion (unlikely, but keepsMoving({ withinSelector: "*" })), the parse accepts it and it'll collide with the default scope. Probably fine to leave but worth knowing.

What I didn't verify

  • HF runtime-interop semantics — whether the sampler's seekTo + paused-timeline contract correctly reflects what the renderer sees, whether the liveness signature matches the renderer's notion of "moving," and whether __timelines/__player/__hf fallback chain is the right one for current HF versions. Deferred to Vai's pass.
  • I did not run the full CLI test suite locally; PR body claims 835/835 green and the fallow gate passes.
  • Did not exercise against a real composition end-to-end; reviewed semantics only from the diff.

Review by Rames D Jusso

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified at HEAD 83fddbb.

R2 verification against R1 at 6f0a607. Rebased onto main (R1 commit replaced wholesale by a single squashed commit at the new tip), but the substantive code changes are clearly authored against the R1 feedback. Net: approve-with-concerns → approve. Both concerns resolved, three of four nits resolved, one question genuinely resolved by design choice. CLI-rubric lane only (Vai owns the HF runtime-interop pass).

R1 verification table

R1 item Status Evidence
Concern 1 — multi-match selector silently first-match-only ✅ resolved packages/cli/src/commands/layout.ts:358-376findAmbiguousSelectors runs before sampling, calls document.querySelectorAll(sel).length > 1, returns motion_selector_ambiguous findings via packages/cli/src/utils/motionAudit.ts:35-46 with an actionable fixHint ("use #id or :nth-child()"). Picked option (a) from the R1 suggestion (reject) rather than (b) (iterate); reasonable for a "claim names one element" contract. Pass aborts before frame collection when ambiguous selectors are present, so no wasted seek work.
Concern 2 — no version field, unknown-kind hard reject breaks old CLIs ✅ resolved (alternative) packages/cli/src/utils/motionSpec.ts:18 adds version?: number; packages/cli/src/utils/motionSpec.ts:85-89 rejects unsupported versions with a clear "spec version N is not supported — upgrade the hyperframes CLI" message. Resolution is "fail loudly with an actionable upgrade message" rather than the "warn-on-unknown-kinds tolerance" I suggested — but the operational goal (old CLI doesn't silently misread future specs, and the failure mode now has a clear remediation) is met. Per feedback_open_item_alternative_resolution, crediting as resolved-differently. Tested at packages/cli/src/utils/motionSpec.test.ts:79-86.
Nit 1appearsBy 0.5 vs staysInFrame 0.2 opacity threshold ⚠️ partial packages/cli/src/utils/motionAudit.ts:5 defines APPEAR_OPACITY = 0.5 and uses it in firstAppear. staysInFrame at packages/cli/src/utils/motionAudit.ts:145-148 still gates on sample.visible (the sampler's 0.2-threshold field), not the 0.5 appear threshold. Code is unchanged from R1; docs at docs/packages/cli.mdx:603 still describe staysInFrame as "once visible" without disambiguating the threshold. Defensible (the box really is partially visible) but the R1 ask was either align thresholds or document — neither was done. Low-stakes, drop if intentional.
Nit 2querySelectorAll("*") per-frame cost ❌ still-open packages/cli/src/commands/motion-sample.browser.js:78-86 still iterates root.querySelectorAll("*") per liveness frame. Acknowledged as low-priority in R1; safe to defer. Worth a comment near the loop ("O(DOM) × MOTION_MAX_SAMPLES; revisit if heavy-DOM compositions hit it") so future maintainers know the cost shape.
Nit 3NaN slipping past duration check ✅ resolved packages/cli/src/utils/motionSpec.ts:94-95 now guards with !Number.isFinite(raw.duration). Test at packages/cli/src/utils/motionSpec.test.ts:88-95 exercises duration: NaN.
Nit 4 — multi-composition alphabetical tiebreak ambiguity ⚠️ partial findMotionSpec logic at packages/cli/src/utils/motionSpec.ts:115-126 unchanged — still falls back to first-alphabetical when no html basename matches. Docs at docs/packages/cli.mdx:582-606 don't mention the "one composition per project" caveat. The new test at packages/cli/src/utils/motionSpec.test.ts:113-121 exercises the html-basename-match path but not the alphabetical-tiebreak path. Minor; the deferred --assert <path> will subsume this.
Question 1schemaVersion not bumped for additive fields ✅ resolved (by design) packages/cli/src/commands/layout.ts:31 keeps INSPECT_SCHEMA_VERSION = 1; new motionSpec/motionSamples fields added conditionally at packages/cli/src/commands/layout.ts:581-582 (only emitted when a spec is present). Treating as resolved-by-intent (additive-only is a defensible JSON-envelope policy), but no explanatory comment was added near INSPECT_SCHEMA_VERSION documenting the policy — a one-line "// additive-only; bump on breaking changes" would future-proof against the next contributor.
Question 2 — no test for resolveMotionSpec exit-1 path ❌ still-open resolveMotionSpec at packages/cli/src/commands/layout.ts:411-437 still process.exit(1)s on parse failure; not unit-tested. Defensible (process.exit is hostile to vitest) — the PR body claims end-to-end coverage. Worth refactoring to a "render error envelope" pure function + a thin process.exit wrapper, but a non-blocking R3 follow-up.
Question 3"*" magic scope collision with literal universal selector ✅ resolved packages/cli/src/utils/motionSpec.ts:65-66 now explicitly rejects withinSelector: "*" at parse time with a clear error pointing the user at "omit it for whole-composition liveness." Tested at packages/cli/src/utils/motionSpec.test.ts:97-103. Clean fix.

New since R1

  • findAmbiguousSelectors (packages/cli/src/commands/layout.ts:343-359) runs evaluate with a try/catch swallow for invalid selectors. Not new behavior — safeQuery in the browser sampler has the same try/catch — but means a malformed selector (e.g. :where(:not( mid-string) silently skips the ambiguity check rather than raising. Low-stakes: it'd flow through to safeQuery → null → motion_selector_missing, so the user still sees a finding. Not a blocker.
  • The runMotionPass short-circuits on ambiguous.length > 0 and returns sampleCount: 0 (packages/cli/src/commands/layout.ts:447). Means the JSON envelope reports motionSamples: 0 when any selector is ambiguous, which is honest — no samples were taken. Fine.
  • _examples.ts and the inspect/layout description copy mention "motion verification" — good discoverability.

Verdict

approve at HEAD 83fddbb. Both R1 concerns are resolved (concern 2 in spirit if not in shape, which is the right call). Remaining items are minor — nit 1 opacity-threshold consistency and the schemaVersion policy comment are the highest-value drop-ins if you want a tidy R3, but nothing here blocks merge.

Review by Rames D Jusso

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified delta at HEAD 83fddbb, R2 baseline was 83fddbb — no new commits pushed since R2. Re-verifying the four R2-open items at the same SHA per Miguel's "review again."

R2-item status

  1. Nit 1 — staysInFrame gates on 0.2-threshold sample.visible vs appearsBy's 0.5 — ❌ still-open

    • packages/cli/src/utils/motionAudit.ts:5 defines APPEAR_OPACITY = 0.5; appearsBy (L61) uses sample.visible && sample.opacity >= APPEAR_OPACITY.
    • staysInFrame (L142) still gates on !sample.visible alone — uses the 0.2 visibility threshold rather than APPEAR_OPACITY.
    • Mismatch unchanged. An element can be visible: true at opacity 0.3 (so staysInFrame evaluates it) while appearsBy considers it not-appeared. R3-tidy.
  2. Nit 4 — alphabetical tiebreak + missing "one composition" doc caveat — ❌ still-open

    • packages/cli/src/utils/motionSpec.ts:116 still entries.filter(... endsWith(".motion.json")).sort() — first alphabetically when no basename match.
    • No doc clarification added on multi-composition behavior. R3-tidy.
  3. Nit 2 — querySelectorAll("*") per liveness frame — ❌ still-open

    • packages/cli/src/commands/motion-sample.browser.js:79 still root.querySelectorAll("*"). R1-acknowledged, low-stakes, no change.
  4. Q2 — resolveMotionSpec exit-1 path untested — ❌ still-open

    • packages/cli/src/utils/motionSpec.test.ts has no process.exit coverage. Test gap unchanged.

Delta verdict

No code changes since R2 approval at 83fddbb. The four open items are all R3-tidy or low-stakes — none are merge blockers. R2 verdict (approve) stands; these are follow-up cleanups, not gates.

If "review again" was expecting a new HEAD, there isn't one — happy to re-verify on push.

Review by Rames D Jusso

Extend `inspect` to verify motion intent against the same seeked timeline
the renderer uses, catching render-≠-preview bugs that layout sampling can't:
entrance reveals the seek skips, broken stagger order, off-frame drift, and
frozen shots.

A `*.motion.json` sidecar next to the composition opts in (auto-discovered,
no flag, no authoring-framework changes); without one, inspect is unchanged.
inspect seeks a dense grid over the asserted selectors, builds an
element × time matrix of {rect, opacity, visible} plus per-scope liveness
signatures, and evaluates four assertions in Node:

  appearsBy    -> motion_appears_late
  before       -> motion_out_of_order
  staysInFrame -> motion_off_frame
  keepsMoving  -> motion_frozen

A selector matching nothing is reported as motion_selector_missing rather
than silently passing. Findings reuse the LayoutIssue shape and flow through
the existing dedupe/collapse/limit/format pipeline and JSON envelope; they
are errors by default, so a failed assertion fails the run.

The motion pass runs in the same Chrome session as the layout audit (no extra
launch) and only when a sidecar is present.
@miguel-heygen miguel-heygen force-pushed the feat/motion-verification-inspect branch from 0f834fa to 1d1d00d Compare June 15, 2026 20:14
@miguel-heygen miguel-heygen merged commit 9175ece into main Jun 15, 2026
36 checks passed
@miguel-heygen miguel-heygen deleted the feat/motion-verification-inspect branch June 15, 2026 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RFC: declarative motion verification — assert entrance/order/in-frame/liveness against the seeked timeline

2 participants