fix(cli): render SVG selector proof strips

[miguel-heygen]

Summary

Focused keyframes --layout strip proofs now show the complete painted SVG target instead of collapsed dots or transparent bounding boxes. SVG groups use native bbox/screen-transform geometry for marker sampling, while strip mode rasterizes and fits the actual selector pixels into each tile; an exclusive-end sample retries one nominal frame inside the clip.

Fixes reported:1787912373.237609. Feedback: https://slack.com/archives/C0BGC335AQY/p1787912373237609

Test plan

• packages/cli/src/commands/keyframes.test.ts, motionShot.test.ts, and motionShotLayout.test.ts (47 passed)
• CLI typecheck
• oxlint and oxfmt --check on changed files
• CLI bundle build via tsup
• Exact Terra bike fixture: all seven #bike strip tiles show the full bicycle, including the authored 4s endpoint; the 2s full-frame snapshot remains the control

---

[jrusso1020]

Reviewed at head beb963914aef5d6b4e4532895e4b87221df4d18a.

Strengths

• The diagnosis is exactly right, and the mechanism is worth stating because it explains both reported symptoms at once. The marker rig appends HTML <div>s into the target (motionShot.ts:123-125), and an HTML div inside an SVG <g> is foreign content without a <foreignObject> - never laid out. So all five getBoundingClientRect() reads come back zeroed, the five points collapse to one, and you get "collapsed dots" for a group and a transparent box for the fitted quad. getBBox() + getScreenCTM() is the correct SVG-native substitute, and the SVG branch also stops appending five never-removed divs per element into the user's DOM.
• The new unit test does real projection math rather than asserting a shape. I checked all five points by hand against the fixture - bbox {-44, -120, 238, 164} through a=1, d=1, e=60, f=200 gives corners {16,80} {254,80} {254,244} {16,244} and centre {135,162} - and they are right. A transposed matrix term would fail it, which is the failure this code is actually exposed to.
• stripCaptureTimeCandidates is a two-line pure function with its own test covering both branches, rather than a bare t - 1/30 at the call site. The half-open-clip-end reason is in the comment at motionShotLayout.ts:107-109.

important - the extracted browser function loses the constraint its siblings carry, and its new test runs it where that constraint does not apply

Pulling the sampler out to sampleMarkerOnionElements (:90) is good for testability, but it is still handed to page.evaluate at :712, so it is serialized with Function.prototype.toString() and must reference nothing outside browser globals. Today it satisfies that - I checked every free identifier and it is only window, document, getComputedStyle, Math and parseFloat, with the types erased.

The problem is the guard rail. This file already states the rule three times, most explicitly at :250: "Self-contained (only tt + window/document - never a module-scope reference)." The new function's comment at :88-89 describes what it samples and drops that constraint. And the new test calls it directly in jsdom, where module scope is available - so the one edit this extraction makes easy (reach for a helper at the top of the file) passes the unit test and then fails in the browser as a ReferenceError inside page.evaluate, which is about the least legible error surface in the CLI.

Restating the :250 constraint on :88 is most of the fix. A one-line assertion on sampleMarkerOnionElements.toString() in the test would make it enforceable rather than advisory, which matters more now that the function is exported and looks ordinary.

important - one invisible frame aborts the whole strip

:671 throws when neither candidate time yields bounds. So a seven-tile filmstrip of an element that fades in after t=0 produces no output at all, where the marker path produced a degenerate but rendered tile. Failing loudly on a proof tool is defensible, but the failure is total rather than per-tile, and six good tiles plus one placeholder is strictly more useful than nothing - especially since the user's next move is to guess which sample time was the problem. The message names the selector and t but not that a pre-end retry was already tried, so "no visible SVG bounds at 4s" understates what was attempted.

nit - a tile's label can be off by one frame

When the retry wins, the capture happens at t - 1/30 while frames.push records t (:674-679), so a tile labelled 4s can be showing 3.967s. That is the intended behaviour for a half-open clip end, but nothing in the rendered strip says a substitution occurred.

nit - 1/30 hardcodes the frame rate

The comment honestly says "one nominal 30fps frame", but the composition's real duration and rate are already resolved upstream. On a 60fps composition the retry steps back two frames rather than one.

nit - the 60ms settle at :688

A bare setTimeout sleep after injecting the markup, with no comment, and it is the only such sleep in this file. The <image> hrefs are data URLs so decode should be quick, but a timer is the wrong instrument for "the images have painted" and this is the shape that becomes an unexplainable flake later. Polling the injected <image> elements, or at minimum a comment naming what the delay waits for, keeps the next person from deleting or doubling it at random.

Checked, not a finding

• requests[0] at :785 is not a narrowing. buildOnionSvg already renders only the first element in strip mode (stripBody(elements[0]?.samples ?? [], W, H)), so the new path matches the existing behaviour exactly. Neither path warns when --layout strip is handed several selectors and ignores the rest - pre-existing, just now in two places, and resolveShotSelectors does commonly return several.
• No injection surface in the new markup. attrs (motionShotLayout.ts:156-159) does not escape, but every value on the new <image> is a number or a base64 data URL, and the label goes through text(), which runs escapeXml. So insertAdjacentHTML at :687 is safe here.
• The SVG detection is consistent across its two sites (:110 in the sampler, :787-795 for the strip decision) - both test typeof getBBox === "function" && typeof getScreenCTM === "function", so a target cannot take the SVG branch in one and the HTML branch in the other.
• rig.el.style.visibility = "hidden" still works on the SVG branch, since SVGElement carries style.

Verdict: APPROVE
Reasoning: Correct root cause, correct SVG-native fix, and the strongest single test in this batch - the projection math is verified rather than asserted. The two important findings are both about the blast radius of the extraction and the hard abort, not about the geometry, and the geometry is what this PR is for.

— Rames Jusso

[miga-heygen]

Reviewed at exact head. Extracts sampleMarkerOnionElements for shared use and adds SVG-native proof strips via captureRenderedSvgStrip — uses getBBox + getScreenCTM to project SVG group bounds instead of relying on marker divs that don't work inside <svg>. Fallback to one-frame-early capture for half-open clip ends is a nice touch. Tests verify the SVG path, the filmstrip layout, and the capture-time candidates. LGTM.

— Miga