fix(studio): switch keyframe ease modes optimistically

[miguel-heygen]

What

Three Studio timeline fixes, verified live in the preview server:

1. Curve, Spring, and Wiggle mode switches commit optimistically for the selected keyframe segment.
2. Right-clicking a motion-path arc waypoint opens the path node menu instead of the browser's own context menu.
3. An expanded sub-composition row keeps its hidden state, so its eye toggle can show a hidden element again.

Why

1. The easing inspector allowed a mode tab to appear selected without reliably applying the corresponding ease mutation. Inspector state and authored GSAP source need to converge immediately.
2. Only keyframe nodes had a context-menu handler, so arc waypoints (motionPath) fell through to Chrome's menu and none of the node actions were reachable.
3. An expanded child row is built from a manifest clip, which carries none of the host element's attributes, so data-hidden never reached it. The eye on that row always reported the element visible: the first click hid it, and every click after wrote data-hidden again instead of removing it. The element could not be shown again, not even after a reload, because the attribute was already in the source.

How

• Route mode changes through the same exact segment mutation path as other easing edits, and update inspector state optimistically while preserving rollback on failure.
• Attach the node context menu to arc waypoints on the same handler keyframe nodes use.
• Inherit hidden, timelineLocked and timelineRole on an expanded child from the flat store element for the same child, which is built with its host element.

This is C4 and the tip of the independent Family C draft Graphite stack.

Test plan

• Unit tests added/updated
• Manual testing performed
• Documentation updated (not applicable)

Validated on this head with the full Studio suite (3,122 passed, 0 failed, 18 todos), package typechecks, oxfmt, oxlint, file-size gates, and Fallow with zero introduced findings. Fixes 2 and 3 were reproduced and re-verified by hand in the preview server against a keyframe/motion-path project.

Not covered here: the remaining families D through G still stack above this branch, and the slow save round trip observed locally when toggling visibility is unrelated to these changes.

[miguel-heygen]

• fix(studio): switch keyframe ease modes optimistically #2695 👈 (View in Graphite)
• fix(studio): publish keyframe cache refresh atomically #2694
• feat(studio): bulk-edit easing for merged keyframes #2693
• fix(studio): target colliding keyframes exactly #2692
• main

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

[james-russo-rames-d-jusso]

Reviewed at 2469850c.

The optimistic-display state machine at packages/studio/src/components/editor/EaseCurveSection.tsx:333-360 is neat: pendingEase = { source, value } where source is the parent-ease prop at commit time, and displayedEase = pendingEase?.source === ease ? pendingEase.value : ease auto-clears the moment the parent's ease prop changes (either to what we committed or to something else entirely). The [ease] effect at line 353 belt-and-braces this with an explicit setPendingEase(null) on any prop change. Two of the new tests pin the invariant well:

• EaseCurveSection.test.tsx:311 — "keeps an optimistic mode visible through its canonical prop round-trip" (parent echoes back the same string we committed → optimistic stays applied because source === ease still holds).
• EaseCurveSection.test.tsx:334 — "replaces an optimistic mode when the canonical prop changes externally" (parent lands on something different → optimistic clears because source !== ease now).

And the AnimationCard.test.tsx:264 .each matrix covers Spring / Wiggle / Curve mode transitions end-to-end through onUpdateKeyframeEase — good coverage of the wiring, not just the local state.

Concerns

• Rollback path assumes the parent always emits an ease change. pendingEase is only cleared when the [ease] effect fires. If onCustomEaseCommit → onUpdateSegmentEase / onUpdateKeyframeEase → gsapCommitMutation fails silently or the mutation lands as a no-op (same string committed as already stored), the store's ease prop never changes, the effect never re-runs, and the optimistic value sticks in the UI even though the authored source didn't change. The description says "preserving rollback behavior on failure", and the round-trip test covers the success echo, but I don't see a test where the commit fails and the display returns to the pre-click state. If the mutation path guarantees an ease-prop change on every commit outcome (success OR rejection), a comment naming that invariant on commitEase would close it out; if not, a rejection-path test or an explicit rollback (clear pendingEase after onCustomEaseCommit returns/rejects) is what makes the rollback claim load-bearing.

• packages/studio/src/components/editor/holdEaseSeek.test.ts reaches across package boundaries and is out of scope for this PR. The import { installStudioCustomEase } from "../../../../core/src/runtime/customEase"; bypasses @hyperframes/core's exports — installStudioCustomEase is not re-exported from packages/core/src/index.ts and there's no ./customEase / ./runtime/customEase entry in packages/core/package.json. installStudioCustomEase already has coverage inside core at packages/core/src/runtime/customEase.test.ts, and the new test's scope (GSAP parseEase("hold") + timeline.seek()) isn't referenced by the PR body. Either move the test to packages/core/src/runtime/ (where the deep import goes away) or add a ./customEase package.json export so this call site can import from "@hyperframes/core/customEase".

What I didn't verify

• Multi-target segment commits (onUpdateSegmentEase bulk-edit): pendingEase is a per-EaseCurveSection local state, and each colliding-target keyframe gets its own instance, so the optimistic display should be per-instance-correct — but I didn't trace whether a bulk commit's completion emits an ease prop change on each mounted EaseCurveSection in the same tick or whether one instance might race the other.

— Review by Rames D Jusso

[vanceingalls]

R1 review — via

Grade: B+
Overall: CORRECT (non-blocking)

Thesis check: "Optimistic" here means the ease inspector paints the newly-selected mode locally the instant the user clicks, instead of waiting for ease prop to round-trip back through the mutation pipeline. The implementation matches: pendingEase = { source: <ease at commit time>, value: <predicted new ease> }, and displayedEase = pendingEase?.source === ease ? pendingEase.value : ease. Everything the inspector derives (mode, springBounce, wiggleConfig, curve, curvePath, showGraph, label, dropdown ease, param field) now reads displayedEase — so the mode-tab, glyph, and param field converge with the click even before the parent's gsapCommitMutation settles. The useEffect [ease] clears both draft and pendingEase when the parent lands (or diverges), which is the "canonical prop round-trip" contract the two new controlled tests pin down. That matches the title.

P0/P1 findings:
None.

P2/P3 findings:

• [P2] packages/studio/src/components/editor/EaseCurveSection.tsx:337-353 — rapid double-click transient flicker. If the user clicks Spring then Wiggle before the parent processes the first commit, the sequence is:

  • Click Spring → pendingEase = { source: X, value: SPRING }, display = SPRING, fires onCustomEaseCommit(SPRING).
  • Click Wiggle (still same render, ease closure = X) → pendingEase = { source: X, value: WIGGLE }, display = WIGGLE, fires onCustomEaseCommit(WIGGLE).
  • Parent lands commit 1 first → ease becomes SPRING → pendingEase.source (X) !== ease (SPRING) so displayedEase = SPRING. Effect fires, clears pendingEase.
  • Parent lands commit 2 → ease becomes WIGGLE, display = WIGGLE.
  • Visible sequence: WIGGLE → SPRING → WIGGLE. That's exactly the mid-motion flicker the fix is trying to eliminate, but only for a single commit in flight.
  • Failure scenario: handleUpdateSegmentEase in useDomEditSession.ts:463 fires void gsapCommitMutation(...) per call — I couldn't confirm whether the parent coalesces vs. serializes rapid mutations to the same segment. If coalesced (last-write-wins), no flicker. If serial, the intermediate render is user-visible on a slow doc pipeline.
  • Mitigation is small: track the predicted terminal value instead of the source-at-click, e.g. hold pendingEase until ease === pendingEase.value (rather than ease !== pendingEase.source). Or explicitly test rapid double-clicks and let ordering guide the design.

• [P2] packages/studio/src/components/editor/EaseCurveSection.tsx:355-360 — fire-and-forget optimistic without a rollback channel. The PR body claims "preserving rollback behavior on failure," but I don't see any signal path from a failed commit back to this component. onCustomEaseCommit is void-called; if the parent's gsapCommitMutation silently drops or rejects (see useDomEditSession.ts:484 — void gsapCommitMutation(...)), ease prop never changes, pendingEase.source === ease stays true forever, and displayedEase = pendingEase.value continues to lie about persistence.

  • Failure scenario: any transient failure inside gsapCommitMutation that doesn't push a corrective ease back through props leaves the user believing the mode switch persisted — but reloading (or serializing the doc) shows the old ease. There's no toast, no revert, no error boundary here.
  • Fix options: (1) verify upstream that the mutation pipeline always pushes an ease change on either success or failure, and note that assumption in the comment above pendingEase; (2) add a timeout that clears pendingEase after N ms if ease hasn't converged; (3) return a promise from onCustomEaseCommit so this component can await + roll back on rejection.
  • This is not a regression (the pre-fix ease === source compare would also silently drop), but the PR body asserts rollback is preserved — the code doesn't demonstrate that. Suggest either wiring failure back or softening the body language.

• [P3] packages/studio/src/components/editor/holdEaseSeek.test.ts:1 — cross-package relative import bypassing the export contract. import { installStudioCustomEase } from "../../../../core/src/runtime/customEase"; is the only file in packages/studio/src that reaches into packages/core/src with a relative path. Everything else uses @hyperframes/core/<subpath> (e.g. @hyperframes/core/gsap-parser, @hyperframes/core/spring-ease). Verified via rg -n 'from "\.\./\.\./\.\./\.\./core/' returning empty otherwise. packages/core/package.json doesn't export ./runtime/custom-ease (grep of customEase|custom-ease returns none), which is why the relative path exists.

  • Failure scenario: on a real build, packages/core publishes only what its exports map declares. A subpath consumer via a deep relative path may work in-workspace (tsc resolves through the workspace filesystem) but breaks external @hyperframes/core consumers and violates the package-subpaths check that the CI's check:package-subpaths script enforces. Also creates a maintenance trap: renaming packages/core/src/runtime/customEase.ts silently breaks studio tests.
  • Fix: either (a) add a "./runtime/custom-ease" subpath to packages/core/package.json and import via @hyperframes/core/runtime/custom-ease, or (b) move the test into packages/core/src/runtime/ where customEase.test.ts already lives (that file already covers hold(0.5) === 0 at line 27; the seek-with-real-gsap variant would live naturally next to it). Note core lacks a direct gsap dep — option (a) is the cleaner fix.

• [P3] packages/studio/src/components/editor/holdEaseSeek.test.ts — scope creep vs PR title/body. The PR title/description scope is "switch keyframe ease modes optimistically" and "cover Curve, Spring, and Wiggle transitions." holdEaseSeek.test.ts covers hold-ease seek semantics through a real gsap timeline, unrelated to mode-switch optimism. Consider splitting.

Nits:

• packages/studio/src/components/editor/EaseCurveSection.tsx:393-401 — same-string commit no longer clears draft. The old handlePointerUp had queueMicrotask(() => setDraft(null)) which cleared draft even if the drag ended on a bit-identical ease string (rare, but possible when the drag returns to origin). The new code relies on useEffect [ease], which won't fire when ease string is unchanged, so draft persists until the next drag or mode switch. Visually indistinguishable (draft matches ease-derived curve), but state lingers and diverges from prior behavior. Worth an explicit test or a note in the effect comment.
• packages/studio/src/components/editor/EaseCurveSection.tsx:365-368 — the two-line comment introduced above curvePath is nice; the trailing comment on line ~371 explaining curve !== null is what keeps Hold handle-free reads a little stranded (it describes showHandles which is already terse). Consider moving next to showHandles.
• packages/studio/src/components/editor/holdEaseSeek.test.ts:2 — import { gsap } from "gsap" in studio tests: fine because studio depends on gsap, but the test's real target is core runtime. Reinforces the P3 above about test placement.

Positive callouts:

• renderControlledSection + parametrized it.each in AnimationCard.test.tsx:263 cover the three-mode transition matrix with a live keyframe animation, canonical-prop round-trip convergence (EaseCurveSection.test.tsx:308-330), and external replacement while an optimistic mode is pending (EaseCurveSection.test.tsx:332-355). Both round-trip AND divergence lanes exercised — solid coverage of the fix's semantics.
• The pendingEase.source === ease gate is a compact way to represent "our prediction is still alive" without commit tokens/IDs.
• The useEffect [ease] clears both draft and pendingEase together — the two locals share a single "external truth changed" signal, which prevents them getting out of sync.
• Comment at EaseCurveSection.tsx:365 calls out why the graph derives from displayedEase — future readers will thank you.
• Delegating the preset dropdown and param field to commitEase (rather than raw onCustomEaseCommit) means every commit path — mode toggle, preset click, param field entry, curve drag, keyboard nudge — goes through the same optimistic gate. No asymmetric paint holes.

Race analysis: Traced three race paths. (a) Single click → commit → round-trip. pendingEase shows locally; when ease prop lands on either the predicted value or any other value, displayedEase picks up ease directly, then the effect clears pendingEase. No inter-render tearing because the fallback (ease) is always defined. Clean. (b) Two rapid mode clicks with serial parent commits. Intermediate-value flicker as described in the P2 above. If gsapCommitMutation coalesces the second write over the first, no flicker; if it serializes, the display flips through the intermediate before landing. (c) External change (undo/redo) mid-flight. pendingEase.source !== new ease, so displayedEase immediately reflects the external value; the effect then clears pendingEase. The pending write still fires and will land later — user transiently sees the undone value, then the pending lands. Acceptable behavior for this class of editor.

Sweep-fix breadth: Verified via rg -n "data-ease-mode" and rg -n 'role="radiogroup"' packages/studio/src/components/editor — the data-ease-mode toggle pattern is unique to EaseCurveSection. No sibling surface has the same "click a mode tab and see the mutation not stick" bug that this fix addresses. No cross-surface inconsistency introduced.

Standards-lens mechanical:

1. Empty-count OK — N/A. Pass.
2. Error boundary — leaf inspector component; parent boundary owns. Pass.
3. Disposal / cleanup — no new subscriptions, listeners, or refs. Pass.
4. Telemetry — inherits trackStudioSegmentEaseEdit at AnimationCard.tsx:315; the new commit path fires through the same callback. Pass.
5. Bare as T — no new casts in source. Test uses mock.lastCall![0] non-null (pre-existing pattern in this test file). Pass.
6. Non-null ! — only in tests. Pass.
7. Untyped catch .message — N/A. Pass.

CI note: Preflight (lint + format) is failing on packages/studio/src/hooks/useDomEditSession.ts (oxfmt), but that file is not in this PR's diff. The failure comes from a downstack PR in the #2692→#2695 stack; preview-regression then cascades because PREVIEW_PARITY_RESULT: skipped. Fix belongs downstack.

— Review by Via

[miguel-heygen]

All three are fixed.

Rollback. The commit path is void end to end, so nothing downstream can report a rejection. Instead of implying a signal that does not exist, the optimistic value is now time-bounded: if the round trip does not arrive within 2s the panel falls back to the committed ease. A write that is rejected or lands as a no-op can no longer pin a curve on screen that was never saved. Covered by a fake-timer test.

Double-click race. The section now keeps a queue of every value it committed and has not yet seen come back. When ease arrives it looks the value up in that queue: a hit means one of its own commits landed, and everything sent before it is retired with it, but a newer commit still in flight keeps painting. That is what stops the wiggle, spring, wiggle flash you describe. A value that is not in the queue is an edit made elsewhere, and the real value still wins immediately, so the existing external-change behaviour is unchanged.

Cross-package import. packages/core now exposes ./runtime/custom-ease the same way it exposes ./runtime/clipTree and the other runtime subpaths (package.json exports, package-subpaths.json, and the tsconfig.json emit list, so dist/runtime/customEase.js is actually built). holdEaseSeek.test.ts imports through it. The test stays in studio on purpose: it pins hold against real gsap, which is a studio dependency, while core's own customEase.test.ts covers the resolver against a stub.

[james-russo-rames-d-jusso]

Delta-reviewed 2469850c..cbd22db3.

Both prior concerns cleanly resolved, and the rollback fix in particular went beyond what I asked for by also solving a race I hadn't named (rapid mode-switching flicker):

• Rollback: pendingEase shape simplified from {source, value} to a plain string, and correctness is now carried by two orthogonal mechanisms:

  1. An inFlightEasesRef queue at packages/studio/src/components/editor/EaseCurveSection.tsx:342 that records every commit and matches arriving ease props against it. [ease] effect at line 355 splices settled commits off the queue and only clears pendingEase when the queue drains — so an older commit's echo cannot repaint over a newer optimistic value (the "wiggle → spring → wiggle" flicker), and an out-of-band ease change (not in the queue) still wins immediately. Test at EaseCurveSection.test.tsx:347 pins the double-switch case.
  2. A PENDING_EASE_TIMEOUT_MS = 2000 fallback at line 378-388 that force-clears the optimistic value if the round-trip never arrives. Test at EaseCurveSection.test.tsx:369 uses vi.useFakeTimers() + advanceTimersByTime(2000) to verify the fallback fires. The docblock ("Long enough for a normal write-reparse-rerender round trip, short enough that a dropped write self-corrects while the author is still looking at the panel") names the trade-off honestly.

• Package boundary: ./runtime/custom-ease export added to packages/core/package.json:122, packages/core/package-subpaths.json:113, and packages/core/tsconfig.json:17. The test's import is now @hyperframes/core/runtime/custom-ease — proper package-boundary crossing — and the new comment explains why the seek test lives in studio (real gsap, not the parseEase stub the core resolver test uses) rather than the intuitive "move it to core" alternative. Fair justification.

The publishKeyframeCache primitive from #2694 rides in as expected; the delta on that surface here is identical to what I signed off on there.

What I didn't verify

• The 2s timeout under a slow real-world commit path. In studio-local editing the round trip is well under 2s, but if this component ever gets rendered against a remote-backed commit (e.g. shared-session editing), 2s could false-positive rollback while a legitimate commit is still in flight. Worth watching if the timeout ever needs a policy override — for now the constant + docblock is the right shape.
• Multi-target segment commits (the bulk-edit path from #2693): each EaseCurveSection instance owns its own inFlightEasesRef, so the queues are independent, which I believe is correct — but I didn't trace whether a bulk commit's completion emits an ease prop change on each mounted section in the same tick.

— Review by Rames D Jusso

[vanceingalls]

R2 review — via

HEAD verified: cbd22db32530bfe30ea53a2ff68284be07fce9cb
Grade: A- (was B+)
Overall: APPROVE

R1 findings delta:

• [P2 fire-and-forget without rollback] — RESOLVED. packages/studio/src/components/editor/EaseCurveSection.tsx:329,389-396 introduces a 2s PENDING_EASE_TIMEOUT_MS fallback: when pendingEase is set, an effect arms setTimeout that clears both inFlightEasesRef and pendingEase if the committed ease prop never comes back. That's option (2) from my R1 (time-bounded clear) — it makes the "preserving rollback behavior on failure" claim in the PR body load-bearing without demanding a rejection channel through onCustomEaseCommit. Locked by EaseCurveSection.test.tsx:353-378 — falls back to the committed ease when the commit never round-trips uses vi.useFakeTimers() + vi.advanceTimersByTime(2000) and asserts aria-checked flips from spring back to curve. Comment on lines 384-388 names the invariant explicitly. Load-bearing test, live rollback path. Closes both the Via and the convergent Rames concern.

• [P2 double-click flicker (WIGGLE→SPRING→WIGGLE)] — RESOLVED. EaseCurveSection.tsx:346,365-382 replaces the "source-at-click vs current-ease" gate with a proper FIFO queue: inFlightEasesRef records every value the section committed, oldest first. When ease lands, indexOf(ease) locates the settled commit and splice(0, landed + 1) retires it plus everything older. If the queue is non-empty, pendingEase stays — so an older commit echoing back mid-flight does NOT repaint the intermediate. Exactly the mitigation I described in R1 ("track the predicted terminal value") generalized to the multi-write case. Locked by EaseCurveSection.test.tsx:331-350 — ignores an older in-flight commit arriving after a newer switch clicks Spring then Wiggle then rolls back ease to spring(0.42) and asserts wiggle stays checked. Externally-driven divergence still short-circuits: indexOf < 0 clears the queue and drops pendingEase (lines 369-374), verified by EaseCurveSection.test.tsx:401-418.

• [P3 4-level relative import bypassing export contract] — RESOLVED. packages/core/package-subpaths.json:113-118, packages/core/package.json:122-128 + 362-365, and packages/core/tsconfig.json:18 add a ./runtime/custom-ease subpath (source + bun + node + dist + types). holdEaseSeek.test.ts:4 now imports via @hyperframes/core/runtime/custom-ease instead of the 4-level relative path. Verified installStudioCustomEase is exported at packages/core/src/runtime/customEase.ts:91. Package-subpaths CI check has a valid target. This is the R1-recommended fix (a) rather than moving the test, which is fine — studio owns the gsap dep and this is the studio adapter surface.

• [P3 test out-of-scope vs PR title/body] — UNCHANGED. holdEaseSeek.test.ts still covers hold-ease seek semantics through a real gsap timeline, not "mode switch." Given (a) the boundary breach is now fixed and (b) this is P3, no longer worth blocking — it's a bundled seek regression net for the hold adapter that happens to ride this stack. Author's call.

• [Rames convergent — rollback path]: RESOLVED, convergent with Via. Same finding, same fix satisfies both.

• [Rames convergent — cross-package boundary]: RESOLVED, convergent with Via. Same finding, same fix satisfies both.

Fresh-pass findings on the new queue + timeout code:

• [P3 nit — draft lingers on 2s-timeout drag failure] EaseCurveSection.tsx:389-396. The pending timeout clears pendingEase and inFlightEasesRef but not draft. If a curve-drag commit is dropped and the 2s falls through, curvePath / showGraph revert to the committed ease (via displayedEase = pendingEase ?? ease), but activeTuple = draft ?? curve still uses the stale draft tuple for the handle positions — so the graph shape reverts while the P1/P2 handle dots stay on the dragged coordinates. Corner case (curve drag + parent drop + 2s+ before another interaction), and the next mouse-down re-seeds draft, so it self-heals. Consider also clearing draft inside the timeout for symmetry. Not blocking.

• [P3 nit — draft not cleared on same-string commit] carried from R1. Unchanged in this pass; already flagged, still visually equivalent since curve equals draft on a same-string commit. Kept as a note, not new.

• [Positive — queue design handles same-value repeats correctly] inFlightEasesRef.indexOf(ease) returns the FIRST occurrence, and splice(0, landed + 1) retires everything up to and including it. So rapid Spring→Wiggle→Spring or Spring→Spring double-clicks resolve cleanly (traced Spring→Wiggle→Spring: after each landing, the queue-oldest-first invariant holds, pendingEase never repaints an intermediate). No race pathology introduced.

• [Positive — multi-target EaseCurveSection instances stay independent] inFlightEasesRef is per-instance state, and each colliding-target commit flows through a distinct EaseCurveSection. Rames flagged this as unverified in R1; the queue approach makes intra-instance ordering correct without cross-instance coordination. Bulk-commit success or failure surfaces the same on every mounted instance since they each hold their own optimistic queue.

• [Positive — useEffect [ease] and [pendingEase] cleanup discipline] Timeout is properly cleared via return handle. Ref does not need cleanup (dies with component). No listeners, no subscriptions.

Positive callouts:

• Queue-based inFlightEasesRef is the right generalization of the R1 mitigation. Named inFlight with the oldest-first comment on line 343-345 makes the invariant scannable.
• The two purpose-built comments on lines 324-328 (PENDING_EASE_TIMEOUT_MS) and 384-388 (fire-and-forget rationale) explain why the fallback timer exists — future readers who wonder "can we just await the promise?" will find their answer in-file.
• Three new tests directly pin the three semantic invariants (canonical round-trip, queue-order flicker suppression, time-bounded rollback). Coverage is unambiguous and pins the R1 concerns.
• packages/core/package-subpaths.json + package.json + tsconfig.json co-updated in the same commit — the subpath export is complete (source + runtime + types + dist target). No maintenance trap.
• openSegment(host, label) helper factored out of AnimationCard.test.tsx cleans up two call sites and reads more clearly.

Standards-lens (mechanical):

1. Empty-count OK — N/A. Pass.
2. Error boundary — leaf component. Parent owns. Pass.
3. Disposal / cleanup — timeout clearTimeout returned. Pass.
4. Telemetry — inherits trackStudioSegmentEaseEdit via commitEase → onCustomEaseCommit. Pass.
5. Bare as T — no new casts. Pass.
6. Non-null ! — test-only, pre-existing. Pass.
7. Untyped catch — N/A. Pass.

Merge readiness: Preflight (lint + format) is now GREEN on this HEAD — the downstack oxfmt issue I flagged in R1 has been cleared by the stack rebase. Regression shards still IN_PROGRESS; Graphite mergeability check IN_PROGRESS. Green all P2/P3, no new P1, PR is ready to land once required regressions finish.

— Review by Via

The base branch was changed.