feat(studio): html-backed motion panel

[miguel-heygen]

Summary

Re-architects the studio motion panel to persist GSAP motion data directly in HTML element attributes instead of a .hyperframes/studio-motion.json JSON sidecar file. Same pattern as position/resize/rotation edits.

Before

MotionPanel → commitStudioMotionManifestOptimistically()
  → writes .hyperframes/studio-motion.json
  → applyStudioMotionManifest(doc, manifest)

After

<div id="hero" data-hf-studio-motion='{"start":0.5,"duration":1,"ease":"power3.out","from":{"opacity":0,"y":40},"to":{"opacity":1,"y":0}}'>

MotionPanel → writeStudioMotionToElement(element, motion)
  → buildMotionPatches(element)
  → commitPositionPatchToHtml(selection, patches)

What changed

• studioMotionOps.ts — Added readStudioMotionFromElement(), writeStudioMotionToElement(), clearStudioMotionFromElement() for attribute-based CRUD
• studioMotion.ts — Added applyStudioMotionFromDom() that reads motion from DOM attributes and builds GSAP timeline (kept applyStudioMotionManifest for render script compat)
• manualEditsDom.ts — Added buildMotionPatches() / buildClearMotionPatches(), integrated motion into reapplyPositionEditsAfterSeek()
• useDomEditCommits.ts — Rewrote handleDomMotionCommit / handleDomMotionClear to use HTML patching instead of manifest persistence
• useManifestPersistence.ts — Removed all motion manifest state (~200 lines): studioMotionManifestRef, commitStudioMotionManifestOptimistically, applyStudioMotionToPreview, motion SSE handler
• App.tsx — Reads motion from element attribute (readStudioMotionFromElement) instead of manifest ref
• manualEditsRenderScript.ts — Extended studioPositionSeekReapplyRuntime to rebuild GSAP motion timeline from data-hf-studio-motion attributes after each seek, including CustomEase support
• htmlCompiler.ts — Trigger seek-reapply script injection on data-hf-studio-motion= attributes

Benefits

• No sidecar file — motion survives git, copy-paste, and manual HTML editing
• Undo/redo works via HTML source history (same as position edits)
• Renders correctly via CLI — seek-reapply script handles motion timeline rebuild
• Simpler architecture — one persistence path for all studio edits

Test plan

• bun run build passes
• Pre-commit hooks pass (lint, format, typecheck)
• Set motion on element in Studio → data-hf-studio-motion attribute appears in HTML source
• Reload page → motion persists and plays correctly
• Clear motion → attribute removed, element returns to original state
• Undo/redo motion changes
• Render via CLI → motion visible in rendered video
• Seek animation → motion timeline re-syncs correctly

[vanceingalls]

PR re-architects the studio motion panel to bake GSAP motion as a JSON data-hf-studio-motion attribute on each element instead of a .hyperframes/studio-motion.json sidecar. Same architectural direction as #829 (HTML-baked positions/rotation) — good direction, removes a persistence axis. Required CI is green at the time of review (regression-shards pending, optional Windows + WIP shards skip). Pulled the rubric (heygen-skills/skills/pr-review/SKILL.md). No prior reviews to be additive to.

Calibrated strengths

• The new render-time runtime in manualEditsRenderScript.ts (lines 80-167) is correctly defensive: gates on [data-hf-studio-motion] presence, JSON.parse wrapped in try/catch, finite-number/object-shape validation, swallows malformed entries silently. Good shape for a script that runs at render with no error reporting.
• Capturing lastSeekTime inside the seek wrap (manualEditsRenderScript.ts:227) before delegating to GSAP — correct ordering; the rebuilt timeline picks up the new target time deterministically.
• clearStudioMotionFromElement in studioMotionOps.ts cleanly restores transform/opacity/visibility from the three data-hf-studio-motion-original-* attributes before removing them — symmetric to the write path. The render-script's restore loop (legacy studioMotionRenderScript.ts:156-165) uses the same convention, so attribute names stay shared.

Findings

blocker — JSON attribute values are not HTML-escaped before being written into source HTML

Where: manualEditsDom.ts buildMotionPatches (constructs { type: "attribute", property: STUDIO_MOTION_ATTR, value: motionJson }) → useDomEditCommits.ts:150 applyPatchByTarget → sourcePatcher.ts patchAttributeByTarget / patchAttribute (lines ~292-365 on PR head).

Failure mode: JSON.stringify(payload) produces strings delimited by ". The patcher writes the value verbatim with ${fullAttr}="${value}". For a concrete payload {"start":0,"duration":1,"ease":"power3.out","from":{"x":40},"to":{"x":0}}, the patcher produces:

data-hf-studio-motion="{"start":0,"duration":1,"ease":"power3.out","from":{"x":40},"to":{"x":0}}"

The HTML parser closes the attribute at the first inner " — the persisted element ends up with data-hf-studio-motion="{" plus a series of bareword attributes (start, duration, ease, …) and a stray }". On reload, element.getAttribute(STUDIO_MOTION_ATTR) returns "{", JSON.parse fails, the new HTML-baked motion silently no-ops at render. The element is also corrupted by spurious attributes that could collide with user attribute names.

The same pattern affects STUDIO_MOTION_ORIGINAL_TRANSFORM_ATTR (any user transform containing " — rare but possible with url(...) references — would corrupt the same way).

Why no CI catch: the build/lint/typecheck checks don't exercise the persisted file; the CLI smoke test renders from a fresh in-memory bundle, not a post-commit reload. PR description's unchecked test items 2 and 3 ("Reload page → motion persists and plays correctly" and "Clear motion → attribute removed") are exactly the cases that would surface this.

Fix shape: HTML-entity-escape the value before passing to the patcher. Either add a htmlAttrEscape(value) step in patchAttribute/patchAttributeByTarget (escape &, ", <, > — & first), or have buildMotionPatches pass an already-escaped value. The matching read regex (["'])([^"']*)\1 then needs to be tolerant of " content (or matched against the escaped form). Pair the fix with a sourcePatcher.test.ts case using a JSON payload containing a literal ", and a round-trip test for buildMotionPatches → applyPatchByTarget → re-read.

important — Migration hazard for projects that already have .hyperframes/studio-motion.json

Where: the legacy render-time path is still wired up. packages/core/src/studio-api/routes/preview.ts:96-112 injects createStudioMotionRenderBodyScript from studioMotionRenderScript.ts when the sidecar is non-empty, and studio/vite.studioMotion.ts:40 includes the same script in the body-scripts list used by vite.config.ts:283 for renders.

Failure mode: a project upgraded to this PR with an existing .hyperframes/studio-motion.json will execute BOTH render-time motion runtimes:

1. Legacy script reads the manifest, resolves targets, calls setAttribute(STUDIO_MOTION_ATTR, "true") (studioMotionRenderScript.ts:236), builds a timeline under __timelines["studio-motion"], pauses, seeks.
2. The new seek-reapply runtime (PR's manualEditsRenderScript.ts) sees [data-hf-studio-motion] matches (including the ="true" markers the legacy script just wrote), JSON.parse fails on "true" (silently skipped — good), JSON-baked elements still parse. Both runtimes will rebuild __timelines["studio-motion"] and overwrite each other.

The new runtime fires on every seek; the legacy runtime fires once at body load. Net effect: first frame can show the legacy manifest's motion until the first seek; after that, the new HTML-baked motion takes over but the legacy manifest's motions are dropped — even though the user never asked to remove them. And when the studio panel rewrites motion for an element that the legacy manifest also targets, the studio writes to the HTML attribute but doesn't remove the matching entry from the sidecar manifest — those entries linger and ship in the project.

Fix shape: either (a) explicitly delete .hyperframes/studio-motion.json (or rewrite to an empty { "motions": [] }) on the first motion edit after upgrade — same coalesceKey/save path as the HTML edit — and document the migration; or (b) on PR merge, run a one-shot migration that converts each manifest entry into a data-hf-studio-motion attribute on the targeted element, then deletes the sidecar; or (c) keep the legacy path running but have the new code skip elements that already have a legacy ="true" marker so the two paths are mutually exclusive. Option (b) is the most invariant-preserving for users with motion in production.

important — Zero unit test coverage for the new HTML-baked motion primitives

Where: studioMotion.test.ts and sourcePatcher.test.ts (PR head) contain no tests for readStudioMotionFromElement, writeStudioMotionToElement, clearStudioMotionFromElement, buildMotionPatches, buildClearMotionPatches, or applyStudioMotionFromDom. Same gap as #829 flagged earlier today, now reused in a JSON-payload variant.

Why this is more load-bearing now than for #829: the previous PR's patches carried numeric/CSS values, which trip the patcher far less than JSON. The blocker above is the direct consequence of skipping the round-trip test on buildMotionPatches → applyPatchByTarget → readStudioMotionFromElement. Even one round-trip test on a payload with a literal " would have caught it.

Concrete tests to add at minimum:

1. buildMotionPatches → applyPatchByTarget against a fixture HTML string → re-parse the HTML → assert the round-tripped attribute deserializes via readStudioMotionFromElement and equals the input motion.
2. buildClearMotionPatches round-trip — assert all four attributes are gone from the patched HTML.
3. readStudioMotionFromElement semantics: legacy ="true" returns null, malformed JSON returns null, start<0 / duration<=0 returns null, finite-number normalization works for from/to.
4. applyStudioMotionFromDom with a no-op DOM (no elements) — should not crash, should not call gsap.timeline().

important — Seek-time perf: full motion-timeline rebuild on every seek frame at render

Where: manualEditsRenderScript.ts:227-232 — reapplyAll() runs reapplyMotionTimeline(), which kills and recreates the GSAP timeline on every seek. At render, the producer seeks once per frame (e.g. 1800 seeks for 30fps × 60s), so this cost is per-frame and scales with motion-element count.

Risk: rebuilding a GSAP timeline + calling fromTo per element on every frame likely shows up as a perf regression in renders with many motion-tagged elements. The Perf: parity / Perf: scrub checks passed on this PR, but their fixtures probably don't exercise multi-element motion at scale.

Fix shape: cache the built timeline keyed by the concatenated motion JSON across all [data-hf-studio-motion] elements; rebuild only when the keyed hash changes. Or build once per render boundary (first seek) and only totalTime()-scrub afterward — the timeline doesn't change between seeks at render. The PR's lastSeekTime already supports the cheaper path.

nit — applyStudioMotionFromDom ignores its second argument

Where: studioMotion.ts new function signature applyStudioMotionFromDom(document, activeCompositionPath?, currentTime?). The body uses currentTime (via readCurrentTime(win, currentTime)) but activeCompositionPath is unused. Callers in useDomEditCommits.ts pass activeCompPath — pointless, will eventually drift. Drop the param or actually scope the per-comp filter (the legacy path scoped motion to a composition; the DOM-attribute path effectively goes whole-document, which is probably correct for studio but worth a comment).

nit — htmlCompiler.ts:1004 substring check on "data-hf-studio-motion=" is loose

Where: the HF_POSITION_ATTRS substring list is used by compileForRender to decide whether to inject the seek-reapply runtime. The new entry "data-hf-studio-motion=" matches any attribute name that starts with that prefix in any context (e.g. a user could legitimately have data-hf-studio-motion-something on an element and trigger spurious script injection). Low-impact (the script is idempotent and gated). Tightening to 'data-hf-studio-motion="' (include the opening quote) would match only valid attribute starts.

Required closing verdict line

Verdict: REQUEST CHANGES
Reasoning: Save→reload round-trips for JSON attribute values currently corrupt the HTML source (the blocker above) — unchecked test items 2 and 3 in the PR description would surface it. Plus a real migration story for existing .hyperframes/studio-motion.json users is missing. The architectural direction is right; the gaps are all addressable.

— Vai

[vanceingalls]

Re-review of f6e187ce against my prior REQUEST CHANGES (793c25b9). Single commit on top: fix(studio): address PR review — html-escape attrs, cache timeline, migrate sidecar, add tests. Verified the fixes against the actual code; required CI is green (regression-shards in progress — non-blocking optional).

Status of prior findings

• Blocker — JSON attr values not HTML-escaped: ADDRESSED. sourcePatcher.ts:14-30 adds a symmetric escapeHtmlAttribute / unescapeHtmlAttribute pair. Escape order is correct (& first); unescape order is correct (& last) — round-tripping " stays as " then ". Both write sites apply it (patchAttribute:365, patchAttributeByTarget:331) and readAttributeByTarget:302 unescapes on read. The matching regex (["'])([^"']*)\1 still works because " contains no literal ". The DOM path (writeStudioMotionToElement → getAttribute) goes via the browser which auto-handles entities. Covered by sourcePatcher.test.ts:212-291 (literal-quote round-trip, &/</> escape, re-edit of already-escaped value, full motion JSON via patcher in the new motion attribute round-trip via sourcePatcher describe block).

• Important #1 — legacy studio-motion.json migration: ADDRESSED (with caveat). useManifestPersistence.ts:143-158 runs a one-shot mount-time migration: reads the legacy file, and if motions.length > 0 writes back {"version":1,"motions":[]}. The legacy injection at preview.ts / vite.studioMotion.ts short-circuits via hasStudioMotionEntries returning false for empty motions, so the legacy render-runtime no longer fires once a studio session has touched the project. The chosen migration is neutralize not convert — a user who never opens studio between this PR landing and a render will keep the legacy path; if they then open studio, prior motions are silently dropped (no auto-convert to HTML attributes). Acceptable for a feature that landed days ago and is still iterating; not blocking.

• Important #2 — zero unit coverage: ADDRESSED. studioMotionOps.test.ts (new, 445 lines, ~25 cases) covers readStudioMotionFromElement semantics (legacy "true" marker, malformed JSON, start<0, duration<=0, missing from/to, customEase), write→read DOM round-trip, original-style capture-and-not-overwrite, clearStudioMotionFromElement symmetry, buildMotionPatches shape, buildClearMotionPatches round-trip via applyPatchByTarget. sourcePatcher.test.ts:212-491 adds 9 cases for escape/round-trip including the exact failure mode the blocker called out.

• Important #3 — per-frame timeline rebuild: ADDRESSED. manualEditsRenderScript.ts:84-191 caches the GSAP timeline keyed by the concatenated motion JSON across all [data-hf-studio-motion] elements (computeMotionKey). On each seek with an unchanged key + existing live timeline, the runtime just totalTime(lastSeekTime, false) on the cached timeline instead of kill+rebuild. Cache invalidates when an element appears/disappears or its JSON changes — correct. Edge case: same JSON content on different element references survives the cache, so a DOM-rebuild that doesn't go through a full script reload would animate stale references; in practice the studio reloads on HTML save and the render path's DOM is stable per frame, so this is fine.

• Nit — unused activeCompositionPath: ADDRESSED. studioMotion.ts:233 now applyStudioMotionFromDom(document, currentTime?); callers in useDomEditCommits.ts:320, 349 updated.

• Nit — loose substring check in htmlCompiler.ts: ADDRESSED. Now 'data-hf-studio-motion="' (with opening quote), preventing spurious match on data-hf-studio-motion-anything prefixes.

Fresh look — new code

• Escape/unescape ordering inside unescapeHtmlAttribute (sourcePatcher.ts:24-30) is the place this kind of fix typically goes wrong. Verified: & is the last replacement on unescape, so " decodes to " (literal) rather than ". Correct.
• The migration write uses JSON.stringify(...) without a trailing newline; the legacy producer wrote with JSON.stringify(..., null, 2) + "\n". Minor style mismatch but no functional consequence — file ends up empty-equivalent. Nit, optional.
• computeMotionKey concatenates with "\n" as a separator — fine for the cache hash since JSON values themselves never contain raw newlines (JSON.stringify escapes them).

Verdict

Verdict: APPROVE
Reasoning: Blocker resolved with a clean symmetric escape/unescape pair backed by direct round-trip tests; perf cache shipped with a sound invalidation key; migration neutralizes the legacy path with an idempotent one-shot. The architectural direction was already right; the gaps that mattered are closed.

Review by Vai (re-review)

[vanceingalls]

Verdict note: Intended APPROVE; stamp-harness gated the --approve write.

Re-review of f6e187ce against my prior REQUEST CHANGES (793c25b9). Single commit on top: fix(studio): address PR review — html-escape attrs, cache timeline, migrate sidecar, add tests. Verified the fixes against the actual code; required CI is green (regression-shards in progress — non-blocking optional).

Status of prior findings

• Blocker — JSON attr values not HTML-escaped: ADDRESSED. sourcePatcher.ts:14-30 adds a symmetric escapeHtmlAttribute / unescapeHtmlAttribute pair. Escape order is correct (& first); unescape order is correct (& last) — round-tripping " stays as " then ". Both write sites apply it (patchAttribute:365, patchAttributeByTarget:331) and readAttributeByTarget:302 unescapes on read. The matching regex (["'])([^"']*)\1 still works because " contains no literal ". The DOM path (writeStudioMotionToElement → getAttribute) goes via the browser which auto-handles entities. Covered by sourcePatcher.test.ts:212-291 (literal-quote round-trip, &/</> escape, re-edit of already-escaped value, full motion JSON via patcher in the new motion attribute round-trip via sourcePatcher describe block).

• Important #1 — legacy studio-motion.json migration: ADDRESSED (with caveat). useManifestPersistence.ts:143-158 runs a one-shot mount-time migration: reads the legacy file, and if motions.length > 0 writes back {"version":1,"motions":[]}. The legacy injection at preview.ts / vite.studioMotion.ts short-circuits via hasStudioMotionEntries returning false for empty motions, so the legacy render-runtime no longer fires once a studio session has touched the project. The chosen migration is neutralize not convert — a user who never opens studio between this PR landing and a render will keep the legacy path; if they then open studio, prior motions are silently dropped (no auto-convert to HTML attributes). Acceptable for a feature that landed days ago and is still iterating; not blocking.

• Important #2 — zero unit coverage: ADDRESSED. studioMotionOps.test.ts (new, 445 lines, ~25 cases) covers readStudioMotionFromElement semantics (legacy "true" marker, malformed JSON, start<0, duration<=0, missing from/to, customEase), write→read DOM round-trip, original-style capture-and-not-overwrite, clearStudioMotionFromElement symmetry, buildMotionPatches shape, buildClearMotionPatches round-trip via applyPatchByTarget. sourcePatcher.test.ts:212-491 adds 9 cases for escape/round-trip including the exact failure mode the blocker called out.

• Important #3 — per-frame timeline rebuild: ADDRESSED. manualEditsRenderScript.ts:84-191 caches the GSAP timeline keyed by the concatenated motion JSON across all [data-hf-studio-motion] elements (computeMotionKey). On each seek with an unchanged key + existing live timeline, the runtime just totalTime(lastSeekTime, false) on the cached timeline instead of kill+rebuild. Cache invalidates when an element appears/disappears or its JSON changes — correct. Edge case: same JSON content on different element references survives the cache, so a DOM-rebuild that doesn't go through a full script reload would animate stale references; in practice the studio reloads on HTML save and the render path's DOM is stable per frame, so this is fine.

• Nit — unused activeCompositionPath: ADDRESSED. studioMotion.ts:233 now applyStudioMotionFromDom(document, currentTime?); callers in useDomEditCommits.ts:320, 349 updated.

• Nit — loose substring check in htmlCompiler.ts: ADDRESSED. Now 'data-hf-studio-motion="' (with opening quote), preventing spurious match on data-hf-studio-motion-anything prefixes.

Fresh look — new code

• Escape/unescape ordering inside unescapeHtmlAttribute (sourcePatcher.ts:24-30) is the place this kind of fix typically goes wrong. Verified: & is the last replacement on unescape, so " decodes to " (literal) rather than ". Correct.
• The migration write uses JSON.stringify(...) without a trailing newline; the legacy producer wrote with JSON.stringify(..., null, 2) + "\n". Minor style mismatch but no functional consequence — file ends up empty-equivalent. Nit, optional.
• computeMotionKey concatenates with "\n" as a separator — fine for the cache hash since JSON values themselves never contain raw newlines (JSON.stringify escapes them).

Verdict

Verdict: APPROVE
Reasoning: Blocker resolved with a clean symmetric escape/unescape pair backed by direct round-trip tests; perf cache shipped with a sound invalidation key; migration neutralizes the legacy path with an idempotent one-shot. The architectural direction was already right; the gaps that mattered are closed.

Review by Vai (re-review)