feat(studio): keyframes flag, gesture recording + timeline/selection refinements

[miguel-heygen]

Summary

This is the top studio-foundation PR in the GSAP keyframe / motion-path stack. It hardens the keyframe-editing surface and fixes a cluster of bugs across keyframe add/remove, gesture recording, deep-link URL state, and canvas selection. Headline fixes: keyframe mutations now key on the correct (tween-relative) percentage so they land on disk instead of silently no-op'ing; gesture recording connects to the real timeline and records the full on-screen position so committed gestures play back where they were drawn; deep links (?t=, element, panel tab) hydrate and respond to external navigation; and full-bleed scene wrappers no longer hijack canvas click-picking.

No new user-facing flag is introduced — "keyframes flag" refers to the keyframe affordances gated behind the existing inspector surface.

What's changed

Keyframe add / remove correctness (useEnableKeyframes.ts, KeyframeNavigation.tsx, TimelineToolbar.tsx)

• Tween-relative percentages everywhere. Add/remove now key on the tween-relative percentage the writer + runtime use, not the clip-relative playhead. Previously the Layout diamond emitted clip-relative % and missed every keyframe (a silent no-op the optimistic cache hid). New clipToTweenPercentage() recovers the clip→tween map from the keyframes' own (percentage, tweenPercentage) pairs.
• useEnableKeyframes rewritten into typed branches: applyArcWaypointAtPlayhead (arc/motionPath edited as waypoints via nearestPointOnPath, range-extend via update-meta, WAYPOINT_MERGE_PX guard); applyKeyframeAtPlayhead (native keyframes — toggle/add/extend); promoteSetToKeyframes (a set() → two-stop tween); flat to/from/fromTo converted to natural keyframes then routed uniformly.
• New exported, tested helpers: animatedProps(), isPlayheadWithinTween(), buildExtendedKeyframes() (grows range while rescaling stops to preserve absolute timing), resolveNewTweenRange() (clamps the playhead into range so a new keyframe lands at the playhead despite the runtime auto-stamping start=0/duration=root).
• Single-diamond enable — "Enable keyframes" creates one keyframe at the playhead (not 0/100%+mid) and drops bundled opacity so the new tween is position-only.
• Toolbar affordance — outside the tween the keyframe button stays an "add" affordance ("Add keyframe at playhead (extends animation)").

Gesture recording (useGestureRecording.ts)

• Connects to the real timeline — connectGsapRuntime picks the first __timelines entry that is an actual timeline (typeof value.seek === "function"), skipping the __proxied marker (the no-live-preview bug).
• Records the full on-screen position — recordSample no longer subtracts the CSS-var/pointer-snap offset (the server strips --hf-studio-offset on commit; subtracting shoved gestures off by the offset twice).
• Phase reorder — scale + element-center measured before clearing the optimistic offset; clean clearProps teardown before re-applying the offset.

URL / deep-link state (useStudioUrlState.ts, useTimelineSyncCallbacks.ts, playerStore.ts)

• External navigation handled — new hashchange listener re-applies URL state on external navigations (pasted link, back/forward); extracted shared applyUrlSelection().
• Pre-mount seek reconciliation — useTimelineSyncCallbacks reconciles pendingSeekRef ?? store.requestedSeekTime when the adapter becomes ready so a deep-linked ?t= lands instead of starting at 0.
• setSelectedElementId drops any active keyframe selection on element change; added a window.__playerStore read handle for bug-bash inspection.

Selection refinements (studioPreviewHelpers.ts, studioHelpers.ts, useDomSelection.ts, useExpandedTimelineElements.ts, useStudioContextValue.ts, StudioHeader.tsx)

• Full-bleed exclusion — coversComposition() (95% both axes, degenerate-viewport guard) excludes scene wrappers from canvas click-picking (Layers panel still selects them).
• Ancestor clip resolution — findTimelineIdByAncestor() walks up from a static descendant to its nearest clip ancestor.
• Expanded-child identity fix — expanded children keyed in hash form (<sourceFile>#<domId>) via buildTimelineElementKey so isSelected matches.
• Inspector collapse keeps selection — collapsing no longer clears the DOM selection.

Other

• NLELayout preview gets overflow-hidden; useRazorSplit forwards elementStart/elementDuration; RenderQueue copy trimmed; index.html root gains a data-hf-id.

Why

The keyframe editor was silently failing in the common case (clip-relative % never matched the on-disk tween-relative keyframes, masked by the optimistic cache). Gesture recording had no live preview (grabbed the __proxied marker) and committed gestures shifted by the path offset. Deep links didn't seek or restore selection. Full-bleed wrappers made the canvas hard to use. This PR makes each path correct with regression coverage.

Testing

bun run build + bun run test (studio). New/extended Vitest suites:

• useEnableKeyframes.test.ts (new) — resolveNewTweenRange, animatedProps, isPlayheadWithinTween, buildExtendedKeyframes (extend + rescale to preserve absolute timing).
• KeyframeNavigation.test.ts (new) — clipToTweenPercentage maps anchors, interpolates an interior playhead, falls back with <2 anchors.
• useExpandedTimelineElements.test.ts — expanded children keyed in hash form (index.html#eyebrow).
• studioHelpers.test.ts — findTimelineIdByAncestor (resolves .num → #stat1; null when no clip ancestor); switched to happy-dom.
• studioPreviewHelpers.test.ts — coversComposition (full-bleed covers; inner/wide-short/tall-narrow do not; needs both axes ≥95%; degenerate guard).

Stack

Part of the GSAP keyframe/motion-path stack: #1553 → #1554 → #1555 → #1607 → #1608 → #1609 → #1610 → #1611 → #1567 → #1612 → #1613 → #1605. Top studio-foundation PR, on #1610. 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 👈 (View in Graphite)
• 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
• 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 7268f634. Big PR by line count but it decomposes into orthogonal fixes (keyframe %-math, gesture-record runtime connect, URL state, selection refinements) — each lands cleanly with focused tests. Title still says "keyframes flag" but per the PR body and grep, the STUDIO_KEYFRAMES_ENABLED flag was introduced upstack in #1610 and remains default-false; this PR doesn't flip it. No soak-time concern — that's a non-issue here. Flag is consistently gated at every entry point I checked (TimelineToolbar L131, App.tsx L263/541/568, StudioPreviewArea L344, PropertyPanel 5 sites, propertyPanel3dTransform 2 sites, TimelineCanvas L432).

Looks good to me on the substantive fixes.

Concerns
• useEnableKeyframes.ts:339-341 — when selectedGsapAnimations is non-empty, the fetch-fallback is skipped. If the session cache is stale (e.g. just after a delete-all that lagged the parse), this proceeds with phantom anims and the resulting branch picks the wrong shape (the very class of bug the rest of this PR is fixing). The arcAnim/kfAnim/setAnim/flatAnim split papers over most stale shapes, but it doesn't help if the cache says "has keyframes" and disk says "no animations." Suggest: keep the cached-anims fast path, but in the no-match branches (line 365 else) re-fetch once before falling through to the create path, to avoid a doubled-create on stale data.
• useStudioUrlState.ts:152-174 — selection-hydration effect depends on currentTime, so it re-runs on every playhead tick until hydratedSelectionRef.current flips. The hydratedSelectionRef.current early-return is cheap but it's running through the whole effect graph on each render; consider gating with a top-level if (hydratedSelectionRef.current) return; AND dropping currentTime from deps in favor of a one-shot useEffect + a snapshot read. Cosmetic, but tightens the hot path.
• useGestureRecording.ts:399-401 — applyRuntimePreview failure → r.runtime = null silently. Recording continues without live preview, and the user gets no signal until commit (samples were still captured, but the gestural feedback they're chasing is gone). Suggest a one-shot console.warn so a dev/QA sees the runtime-disconnect cause in the console instead of silent dead-pixel preview.
• playerStore.ts:374-379 — window.__playerStore exposed unconditionally as a "bug-bash aid." Comparable to the iframe's __timelines/__player, so not a security concern (it's a Zustand handle, not state). Two thoughts: (1) gate behind import.meta.env.DEV so it's stripped from prod bundles, or (2) namespace under window.__hf.studio to match the other studio globals. Keep it if shipping intentionally; flag it in a ponytail comment with an expiry.

Nits
• useEnableKeyframes.ts:51-62 animatedProps() — falls back to ["x","y"] for unanimated elements. Sensible default, but promoteSetToKeyframes at L242-243 reads end position via this, so a set({rotation: 90}) followed by "add keyframe at playhead" would emit {x, y, rotation} keyframes instead of just {rotation, …}. Probably benign because gsap.getProperty returns the runtime value, but the resulting two-stop tween will carry position keyframes for a rotation-only set. Worth a sibling test or a tighter prop-source. (nit)
• KeyframeNavigation.tsx:38-43 — mapped[0]! / mapped[mapped.length-1]! non-null asserts. The length < 2 guard already proves these are present; consider a const [first, last] destructure to remove the !s. (nit)
• RenderQueue.tsx:122 — copy now reads "Final Cut Pro, DaVinci Resolve, and most video editors" — fine, but the CapCut and Premiere removals lose some discoverability for users explicitly searching for those tools. Was that a deliberate signal or just trimming? (nit/question)
• index.html root gets data-hf-id="hf-aph5" — unclear why the studio shell root needs an hf-id; the comment in NLELayout suggests it's for composition-id wiring, but the studio root isn't a composition. (question)

Questions
• useGestureCommit.ts:200-210 — add-with-keyframes without propertyGroup when there's no existing position tween but the gesture's hasPositionProps is true. If the gesture also recorded rotation/scale (modifier mid-gesture), all those properties land in a single new tween without a propertyGroup. Does the upstream parser later split this, or does it become a legacy mixed tween that the bridge then splits at the next drag? Either is fine, but worth tracing.
• gsapSoftReload.ts:166-168 — pluginScript.onerror = () => executeScript() runs the script even when the MotionPathPlugin CDN fails to load. The inner script throws once it hits MotionPathPlugin, but deferredToAsync=true already short-circuited verifyTimelinesPopulated — so caller thinks the soft-reload succeeded while the timeline is gone. Is this intentional (full reload on next edit would recover) or should we fall back to a full reload on plugin error?

What I didn't verify
• Behavior in offline/CDN-blocked envs (the MotionPathPlugin CDN fetch).
• That the hashchange listener in useStudioUrlState cleanly transfers selection across cross-composition deep links (only same-project path covered in code review).
• useEnableKeyframes happy path against a real fixture with the update-meta extend on an arc — only unit-tested branches.

— Rames D Jusso

[vanceingalls]

Reviewed at 7268f634. Concur with @james-russo-rames-d-jusso — big PR by LOC but decomposes cleanly into orthogonal fixes (keyframe %-math, gesture-record runtime connect, URL state, selection refinements), each with focused tests.

Verified at HEAD:

• STUDIO_KEYFRAMES_ENABLED flag was introduced upstack in #1610 and remains default-false; this PR doesn't flip it. No new gating flag introduced — decorative-gate-pattern N/A. Consistent gating at every entry point Rames enumerated (TimelineToolbar, App.tsx, StudioPreviewArea, PropertyPanel, propertyPanel3dTransform, TimelineCanvas).
• All four new exports tested: clipToTweenPercentage (3 tests), resolveNewTweenRange (4), animatedProps (4), isPlayheadWithinTween (3), buildExtendedKeyframes (2). Auxiliary findTimelineIdByAncestor, coversComposition, expanded-children hash-form keying also covered.
• hashchange listener at useStudioUrlState.ts has matching removeEventListener cleanup (line 1192). No leak.

Concur with @james-russo-rames-d-jusso (verified each at HEAD):

• useEnableKeyframes.ts:339-341 stale-cache fetch-fallback skip: when selectedGsapAnimations is non-empty, no re-fetch on the no-match branches. Stale cache → wrong shape. Same bug class this PR is fixing. NIT-level follow-up; Rames's fix shape (re-fetch once in the no-match else before falling through to the create path) is right.
• useStudioUrlState.ts:152-174 selection-hydration on every playhead tick: hydratedSelectionRef.current early-return saves it, but the effect graph runs through on each render. Top-level if (hydratedSelectionRef.current) return; + dropping currentTime from deps tightens the hot path. Cosmetic.
• useGestureRecording.ts:399-401 silent runtime-disconnect: applyRuntimePreview failure → r.runtime = null with no signal to the user. One-shot console.warn would surface this in dev/QA.
• playerStore.ts:374-379 window.__playerStore unconditional exposure: comparable to iframe globals, not a security concern, but worth gating behind import.meta.env.DEV or namespacing under window.__hf.studio to match other studio globals.

Rest of Rames's nits also concur (mapped[0]! non-null asserts unnecessary given length < 2 guard; RenderQueue.tsx copy change; index.html data-hf-id="hf-aph5" question).

Note on facade-await regression carryover (cross-stack):

useEnableKeyframes.ts adds new session.commitMutation calls in this PR. Each is followed by return; with no post-action that depends on save-complete semantics. So this PR's NEW callsites don't exercise the HF #1558 shape. But the existing useGestureCommit.ts:127-212 (touched as part of the gesture-recording rewrite stack-wide, not this PR specifically) DOES — see my #1608 review for the broken-facade scope expansion. Not introduced by #1611; not fixed by #1611. Fix lands at #1608.

Verdict: NIT. No band-aid blockers, no test-touch-without-test-add, dispatch chain clean. The four follow-up items are improvements not regressions.

Review by Via