feat(studio): GSAP drag/commit/bridge editing infra

[miguel-heygen]

Summary

This PR lands the GSAP-aware drag/commit/bridge editing infrastructure for Studio: dragging a GSAP-animated element on the canvas now routes through GSAP's own x/y rather than colliding with a CSS translate, the keyframe-commit logic correctly extends/modifies tweens instead of spawning parallel ones, and the runtime bridge refuses to resurrect just-deleted tweens from a stale server parse. It also makes the drag commit path go through the canonical safe-commit wrapper so save failures surface a toast instead of silently reverting. Debug console.log scaffolding from the bring-up phase is removed and the behavior is locked down with three new test suites.

What's changed

packages/studio/src/components/editor/manualEditsDom.ts

• Extracted the GSAP-offset handling that previously lived inline in applyStudioPathOffsetDraft into a shared applyStudioPathOffsetViaGsap(element, offset) helper. It detects a GSAP-animated element (gsapAnimatesProperty(element, "x", "y")), forces translate: none, and pushes the drag delta into GSAP's x/y via gsap.set (seeding/reading data-hf-drag-gsap-base-x/y and subtracting the recorded initial offset). Returns true when it handled the element so the caller skips the CSS path.
• applyStudioPathOffset (the commit path) now calls applyStudioPathOffsetViaGsap and early-returns for GSAP elements. Previously only the draft path had this branch; the commit silently folded the offset into a CSS translate that composed on top of GSAP's matrix and flung the element off-canvas. Draft and commit are now symmetrical.

packages/studio/src/hooks/gsapDragCommit.ts

• New exported parkPlayheadOnKeyframe(anim, pct): after editing a selected keyframe, seeks the playhead to that keyframe's absolute time (tweenStart + pct/100 * tweenDuration, rounded) so the edit previews at the keyframe instead of the element's base pose.
• New backfillDefaults parameter threaded through extendTweenAndAddKeyframe, commitKeyframedPosition, commitFlatViaKeyframes, and the from/fromTo branches. When a drag introduces a property the tween didn't animate (e.g. y on an x-only tween), the other keyframes are backfilled at the element's base value so GSAP doesn't hold the new prop's value across keyframes that omit it. commitGsapPositionFromDrag derives backfillDefaults = { x: baseGsapX, y: baseGsapY }.
• Selected-keyframe handling across all branches: when activeKeyframePct != null (a clicked diamond), the commit modifies that keyframe and never extends/splits, even if the playhead has drifted a frame past the tween. After committing it clears activeKeyframePct and parks the playhead.
• commitFlatViaKeyframes outside-range behavior changed from spawning a parallel add-with-keyframes tween to a convert-to-keyframes + extendTweenAndAddKeyframe flow, so the element ends up with one tween instead of two competing ones. It also clears the live drag's GSAP overrides before seeking to read clean interpolated values for the 0% keyframe.
• The from/fromTo extend branch now emits replace-with-keyframes (reusing the existing position tween's id) instead of add-with-keyframes when a position tween already exists, eliminating the post-drop "jump" caused by two position tweens fighting on the shared axis.

packages/studio/src/hooks/gsapRuntimeBridge.ts

• Added a stale-parse guard in tryGsapDragIntercept: after resolving a candidate position tween from the cache/fetch fallback (an async server parse that lags a delete-all), it consults the live runtime via readRuntimeKeyframes(iframe, selector). If the live timeline has no non-hold tween for the element, the parse is stale and it bails (return false → CSS fallback) rather than re-committing a phantom tween and resurrecting a just-deleted animation.
• Removed the [drag:4] debug console.log calls.

packages/studio/src/hooks/useGsapAwareEditing.ts

• The thin commitMutation facade now routes through useSafeGsapCommitMutation (with useGsapSaveFailureTelemetry and showToast). This gives the drag path parity with the arc/keyframe/animation ops: a server-save failure surfaces a toast + save telemetry instead of silently reverting. Removed the [drag:3] debug log.

packages/studio/src/hooks/useGsapScriptCommits.ts

• Tracks whether a soft reload actually applied (applied: "soft" | "full") and falls back to a full reloadPreview() when applySoftReload returns false, making the soft-vs-full-reload decision explicit.

Why

Dragging GSAP-animated elements was broken in several distinct ways: an off-canvas fling (the commit folded the offset into a CSS translate while GSAP owns style.transform); parallel/competing tweens that snapped on the soft-reload reseek (a visible jump after dropping); cross-keyframe drift when introducing a new property; a snap-away preview when a selected keyframe edit left the playhead just outside the tween; and resurrection of a deleted tween because the server parse lags a delete-all. This PR fixes all of these at the root, removes the debug logging used to diagnose them, and makes save failures observable.

Testing

bun run test (studio). Three new suites:

• manualEditsDomGsap.test.ts (jsdom) — non-GSAP element folds the offset into a CSS translate var() + writes --hf-studio-offset-x/y; GSAP element keeps translate: none and routes via gsap.set; draft and commit treat a GSAP element identically (the regression guard for the old asymmetry).
• gsapDragCommit.test.ts — flat tween dragged outside range emits convert-to-keyframes + replace-with-keyframes and never add-with-keyframes; inside range emits add-keyframe; a selected keyframe past the end is modified (not split), activeKeyframePct cleared, playhead parked; keyframed-tween backfill passes backfillDefaults; from() outside range emits split-into-property-groups + replace-with-keyframes; parkPlayheadOnKeyframe seeks to absolute time for 0%/100%/50%.
• gsapRuntimeBridge.test.ts — empty runtime + stale cached anim → intercept returns false, commitMutation never called (no resurrection); tween still live → guard doesn't trip.

Stack

Part of the GSAP keyframe/motion-path stack, split into reviewable PRs: #1553 → #1554 → #1555 → #1607 → #1608 → #1609 → #1610 → #1611 → #1567 → #1612 → #1613 → #1605. This PR (#1608) sits on #1607 (read layer). Each PR builds independently; the combined diff is byte-identical to the originally-reviewed work.

[miguel-heygen]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• feat(studio): instant, flicker-free manual editing #1605
• feat(studio): instantPatch fast path in runCommit #1613
• feat(studio): patchRuntimeTweenInPlace — update a tween's values in place #1612
• feat(studio): GSAP keyframe + motion-path editing #1567
• feat(studio): keyframes flag, gesture recording + timeline/selection refinements #1611
• feat(studio): on-canvas motion-path overlay #1610
• feat(studio): motion-path geometry + commit helpers #1609
• feat(studio): GSAP drag/commit/bridge editing infra #1608 👈 (View in Graphite)
• feat(studio): GSAP runtime read layer + shared helpers #1607
• feat(core): route motion-path mutations through studio-api + fix clip stamping #1555 : 1 other dependent PR (#1556 )
• feat(core): add GSAP keyframe + motion-path source mutations #1554
• chore(producer): shim __filename/__dirname in the CJS banner #1553
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[james-russo-rames-d-jusso]

Reviewed at 57f4cd85078f248c420e9aced63c764ae7894a7d (Group 2, sibling to #1607). Cross-read with #1607 read-layer and traced the drag → commit → mutation chain end-to-end. Found one blocker.

Blockers

• useGsapAwareEditing.ts:240-246 — commitMutation facade now wraps via useSafeGsapCommitMutation, which is fire-and-forget (see useSafeGsapCommitMutation.ts:46 — void commitMutation(...).catch(...), returns void). The new facade calls safeGsapCommit(...) synchronously without await and the inner async function returns undefined immediately, so the outer commitMutation resolves before the server save lands. Pre-PR was await gsapCommitMutation?.(...) — the await is gone. This breaks at least one external caller: useEnableKeyframes.ts:153 does await session.commitMutation(...) (passed through DomEditContext.tsx:240 as commitMutation: stableCommitMutation). The await now resolves immediately; if the save errors the toast still fires async, but any caller expecting "after this resolves, the save is durable" is wrong. Suggested fix: either (a) make useSafeGsapCommitMutation return the inner Promise — return commitMutation(selection, mutation, options).catch(...) — and await safeGsapCommit(...) in the facade, OR (b) restore the explicit await gsapCommitMutation?.(...) in the facade and let useSafeGsapCommitMutation only apply to the new internal handleUnroll-style fire-and-forget paths. Worth flagging: handleGsapAwarePathOffsetCommit at line 112 passes gsapCommitMutation (the raw fn) directly to tryGsapDragIntercept — so the drag path itself is still awaitable. The regression is on the facade commitMutation consumed by useEnableKeyframes + DomEditContext.

Concerns

• gsapRuntimeBridge.ts:230 (stale-parse guard) — uses #1607's readRuntimeKeyframes(iframe, selector). The guard is the right shape (live runtime authoritative over async parse) and matches race-fix preempt-vs-narrow PREEMPT: phantom-resurrection is structurally impossible once the runtime check is in place. But the guard accepts ANY non-null read — including a flat 2-point to({x}) tween, an arcPath, a motion-path tween. Verify the read-layer's readTween returns non-null for every tween shape findGsapPositionAnimation would resolve (including from(), fromTo()). I spot-checked flatTweenKeyframes covers vars with numeric props — fine. But if the read layer ever returns null for a real-live tween that findGsapPositionAnimation matches (e.g. to({motionPath, ease, duration}) where vars has no flat numeric prop AND motionPath shape fails coords filter), the guard misfires and the drag falls to CSS even though the tween is genuine. Edge case but worth a unit test.

• gsapDragCommit.ts:218 (gsapLib.set(el, { clearProps }) before seek) — this clears props the tween doesn't animate to capture clean interpolated values. Good. But what if the user is mid-drag of a SECOND axis (props they DID intend to set)? The clear is keyed on Object.keys(properties) — the drag's props. After clear → seek → re-read, the dragged-but-unsaved values are now gone from the element. Saved by the commit immediately after, but if the commit fails (toast path now via safe wrapper) the element is left in a different state than before the drag. Worth a fail-path test: commitMutation rejects → does the element end up at the dragged position, the cleared position, or the pre-drag position? Failure-path observability rubric.

• gsapDragCommit.ts:155 (parkPlayheadOnKeyframe) — roundTo3(ts + (pct/100) * td). Good idempotent rounding. But: what if the parked seek time falls inside ANOTHER tween's range on a different element / same element via a second tween (per #1607's multi-tween model)? The new read-layer is playhead-aware — parking on one tween's keyframe might surface a different tween in the overlay for the same element. Not a bug per se, just user-confusing if it happens. Probably benign because the tween being edited IS the one whose keyframe was selected, so the playhead lands inside its range. Worth a glance.

• manualEditsDom.ts:253-291 — silently-removed check merge-base rubric: the -39 deletion in applyStudioPathOffsetDraft is the GSAP branch being extracted (not deleted) to applyStudioPathOffsetViaGsap + reused in both applyStudioPathOffset and applyStudioPathOffsetDraft. Verified merge-base; the asymmetry-fix is the real change. Good. The data-hf-drag-gsap-base-x/y + data-hf-drag-initial-offset-x/y attribute reads are now in the shared helper — both callers benefit. Nice cleanup.

• useGsapScriptCommits.ts:85-89 (applied: "soft" | "full") — the variable is assigned but never read. The fallback if (applied === "full") reloadPreview() is reachable via the prior applied = ... ? "soft" : "full" line. Currently dead — applied is just a local toggle. Either remove or wire to telemetry (trackStudioReloadFallback("soft-failed→full") would be the natural observability hook for "how often does soft fall to full"). Nit.

• useGsapAwareEditing.ts:231 — noopCommit = useCallback(async () => {}, []) + gsapCommitMutation ?? noopCommit — if gsapCommitMutation toggles between defined/undefined, safeGsapCommit ref churns and downstream useCallback deps invalidate. Minor.

Nits

• gsapDragCommit.ts:225 — gsapLib.set(el, { clearProps: Object.keys(properties).join(",") }). GSAP accepts arrays too; join(",") is fine but the explicit string conversion is one detail layer deep. Either works.

• gsapDragCommit.test.ts:48 — selection element is a partial mock with getPropertyValue: () => "". Reads cleanly but getAttribute: () => null paired with removeAttribute: () => {} reads as "we know exactly what's read" — that's brittle if restoreOffset is extended. Not a real problem at this size.

• Debug-console.log removal in gsapRuntimeBridge.ts + useGsapAwareEditing.ts is clean — verified no logger replacement intended; this is pure cleanup.

Questions

• The coalesceKey = gsap:convert-drag:${anim.id} in commitFlatViaKeyframes chains the convert + extend on the same key. Does coalescing depend on the two commitMutation calls reaching the server in order? With the safe-wrapper fix above, the facade is no longer in this path (drag uses raw gsapCommitMutation), so order is preserved. But worth confirming with the source-mutation API in #1554/#1555 that coalesceKey is reorder-safe vs ordered.

• Why does tryGsapDragIntercept guard with readRuntimeKeyframes(iframe, selector) but not also re-check readGsapPositionFromIframe for staleness? If the gsap-property read succeeds for a deleted element (e.g. x cached on the DOM element from the dead tween's last tick), the guard could pass but commit a phantom. Probably impossible — element-level x is on the GSAP timeline, deleted with the tween — but worth a one-line comment confirming.

What I didn't verify

• The source-mutation API end of replace-with-keyframes vs add-with-keyframes semantics — covered by #1554 review.
• Live drag UX under failure (commit rejects mid-chain) — manual QA needed.
• Cross-stack: does parkPlayheadOnKeyframe's requestSeek interact with #1605's instant-flicker-free editing on the same flow? Both touch playhead.

Blocker on the missing await regression. Once that's resolved (option a — return the Promise from useSafeGsapCommitMutation and await it — is the cleanest), this PR is in good shape. Not stamp-routable until blocker is addressed.

— Rames D Jusso

[vanceingalls]

Reviewed at 57f4cd85. Concur with @james-russo-rames-d-jusso's BLOCKER on the commitMutation facade regression. Independently verified at HEAD and the diagnosis is correct. Adding one critical scope expansion: Rames cites useEnableKeyframes.ts:153 as the awaiting consumer — there's a second consumer the diagnosis missed, and it's the canonical HF #1558 failure shape we collectively fixed two days ago.

Verified the regression (concur with Rames's diagnosis):

• packages/studio/src/hooks/useGsapAwareEditing.ts:240-246 at 57f4cd85: facade now calls safeGsapCommit(domEditSelection, mutation, options) WITHOUT await. Pre-PR was await gsapCommitMutation?.(...).
• useSafeGsapCommitMutation.ts:46: returns a callback whose body is void commitMutation(...).catch(...) — the void operator discards the promise, so safeGsapCommit(...) returns undefined.
• Net: the outer async commitMutation resolves on the next microtask BEFORE any save completes. The async type signature is now a lie.
• The bridge layer (tryGsapDragIntercept / tryGsapResizeIntercept / tryGsapRotationIntercept) uses the raw gsapCommitMutation parameter, NOT the facade — so drag/resize/rotation are still correctly awaitable.

Scope expansion (net-new):

Rames cites useEnableKeyframes.ts:153 as the affected awaiter via session.commitMutation (through DomEditContext.tsx). Verified at c3ec03a3 there's a second, more user-visible consumer:

packages/studio/src/hooks/useGestureCommit.ts:127, 172, 187, 200 (at c3ec03a3) — all four await liveSession.commitMutation(...) callsites, where liveSession = domEditSessionRef.current. After the await resolves:

• Line 93 → showToast(\Recorded ${sortedPcts.length} keyframes`, "info")`
• Line 98 → store.requestSeek(recordingStartTimeRef.current)
• Line 100 → setGestureState("idle")

This is the canonical HF #1558 shape — the exact bug Rames + @terencecho converged on two days ago: success toast fires before save lands; on save failure, an error toast appears at a timestamp uncorrelated with the user's gesture; gesture-state flips to idle while the disk write may still be in flight. The fix Rames suggests (option (a) — make useSafeGsapCommitMutation return the inner Promise and await it in the facade) cleanly closes both consumer paths. Strong concur.

Concur with @james-russo-rames-d-jusso on the remaining concerns:

• gsapRuntimeBridge.ts:230 stale-parse guard accepts ANY non-null readRuntimeKeyframes read — verify the read layer doesn't return null for from() / fromTo() / to({motionPath, ease}) shapes where vars carries no flat numeric prop. Edge-case unit test would pin it.
• gsapDragCommit.ts:218 clearProps before seek — concern about mid-drag-of-second-axis losing dragged values if the commit fails. Failure-path observability rubric applies; worth a fail-path test.
• useGsapScriptCommits.ts:85-89 applied: "soft" | "full" dead local — either remove or wire to telemetry. The latter is a natural observability hook ("how often does soft fall to full") that I'd actively want.
• Coalesce-key collision check (separate from Rames): verified disjoint at HEAD — gsap:convert-drag:${anim.id} (3 sites in gsapDragCommit.ts), gsap:resize:${anim.id} (gsapRuntimeBridge.ts:324), rotation has no key. Prefixes prevent cross-path collision. Clean.
• manualEditsDom.ts:253-291 applyStudioPathOffsetViaGsap extraction: this is anti-band-aid (clean dedup + the commit path now mirrors the draft path, fixing the GSAP-element off-canvas fling). The comment explaining symmetry is exactly the kind of in-code rationale future readers need. Good.

Other independently verified at HEAD:

• Backfill-defaults dispatch chain across extendTweenAndAddKeyframe / inside-range drag / outside-range drag / rotation / resize bridge — every keyframed branch backfills consistently. Dispatch-chain audit clean.
• Console.log removal: zero [drag:-prefixed log lines remain. Six removals, no replacements. Clean.

Verdict: BLOCK. Facade regression is the hard request-changes (Rames's fix option (a) is the cleanest); rest is comment-level. Once the facade returns the awaited promise, gesture recording's HF #1558 shape closes structurally and useEnableKeyframes.ts:153's consumer becomes truthful again.

Review by Via

[james-russo-rames-d-jusso]

Layering on @via's review for the facade-await blocker — second consumer worth flagging together:

useGestureCommit.ts:127/172/187/200 does await liveSession.commitMutation(...) then showToast("Recorded N keyframes") + requestSeek + setGestureState("idle") — canonical HF#1558 shape. Same root cause as the useEnableKeyframes.ts:153 consumer I cited at R1: the facade's fire-and-forget void useSafeGsapCommitMutation(...).catch() resolves the await before server save lands. This is the path that makes gesture-recording user-visibly broken (toast fires before save, seek lands on stale runtime, state machine returns to idle pre-persist).

Recommended fix (same one mechanism, two consumers): useSafeGsapCommitMutation returns the Promise (still .catch for toast/telemetry); facade awaits it. Closes both useEnableKeyframes and useGestureCommit consumers in one change.

Upgrades my R1 framing: this is a blocker for #1567 specifically (since #1567 lights up gesture recording), not just an abstract stack concern.

— Rames D Jusso