feat(studio): add timeline keyframe retiming interactions by miguel-heygen · Pull Request #2783 · heygen-com/hyperframes

miguel-heygen · 2026-07-25T17:31:46Z

What

Adds direct timeline keyframe retiming interactions.

Why

Editors need to drag keyframes while preserving element identity, clip-relative timing, occupied-destination safety, and deterministic source updates.

How

Routes retime gestures through the shared timing model and mutation callbacks, with focused coverage for drag lifecycle and destination rules. This is B3 of the Family B Graphite stack.

Review fix folded in: onMoveKeyframe returns Promise<boolean> across the whole callback chain (timelineCallbacks -> TimelineLanes -> useTimelineEditCallbacks). This commit is where TimelineClipDiamonds starts awaiting the commit result, so the widening belongs here; without it the intermediate commits do not typecheck.

Test plan

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

Every commit in the stack typechecks independently. Drag-to-retime verified in a browser: dragging a keyframe rewrites its percentage on disk while preserving both its value and its ease.

Deferred review findings

Every blocker and high finding raised on this PR is fixed in the stack. The 3 remaining low/nit findings are parked, verbatim, in .scratch/studio-timeline-family-b/issues/03-pr-2685-deferred-review-findings.md:

🟡 packages/studio/src/player/components/TimelineClipDiamonds.tsx:139 — useEffect clears pending entries by clip-% only, ignoring keyframe identity
🟢 packages/studio/src/player/components/TimelineClipDiamonds.tsx:424 — data-keyframe-percentage attribute is polysemous
🟢 packages/studio/src/player/components/TimelineClipDiamonds.tsx:456 — onPointerCancel clears drag state but never fires the click-fallback

Supersedes #2685, which was closed when main was rewritten to unwind an early landing of this stack. Same head commit, same review history.

R1 review follow-ups

Fixed in this PR:

Escape cancels an in-flight retime, matching clip and element drags. The pointerup that follows is swallowed rather than falling through to the click branch.
Pointer moves are throttled to one preview render per frame, so a 120Hz trackpad no longer re-evaluates every diamond's memo per event.

Scoped out, documented at the dispatch site: multi-select drag would have to move every selected keyframe as one mutation, which the script ops do not express yet.

The 2 remaining low findings (edge auto-scroll, post-drop snap-back window) are parked in .scratch/studio-timeline-family-b/issues/09-family-b-v2-r1-deferred.md.

R2 review follow-ups

Fixed in this PR:

The diamond connector reached its previous keyframe through a non-null assertion. CONTRIBUTING.md asks for a guard clause outside already-checked paths, so the index check and the lookup are now the same guard.

miguel-heygen · 2026-07-25T19:27:33Z

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

vanceingalls

Verdict: COMMENT — retime mechanism is sound (pointer capture correct, cancel path clean, single-write-per-drag, gesture-layer neighbour clamp, rapid-second-drag identity via pendingRetimeRef is well-tested), and the widened Promise<boolean> chain in timelineCallbacks.ts / TimelineLanes.tsx / useTimelineEditCallbacks.ts matches. The observations below are polish, not blockers; the deferred findings in the PR body correctly cover the pending-clear-by-clip-%-only + data-attr semantics, so I'm not repeating them.

Head SHA: 50a90772cabc4709e36d4fdd07758aac3e0a2e57.

Adversarial lenses

Lens A — drag lifecycle — mostly clean. setPointerCapture on pointerdown, releasePointerCapture on both pointerup (TimelineClipDiamonds.tsx:351) and pointercancel (:451); touchAction: "none" on the button (:439) gets touch parity; the grab-offset is captured once on pointerdown via d.startX + d.fromClipPct (:311-315), and previewClipPct is delta-based so mid-drag re-renders don't snap-to-cursor. Two gaps:

P2 — Escape does not cancel an in-flight drag. No key handler is armed on pointerdown. Users can back out of clip/element drags elsewhere in the app with Escape; this diamond can't. Pressing Escape here just leaves the drag running until pointerup commits it. Suggest a document.addEventListener('keydown', …) scoped to dragRef.current !== null that mirrors the pointercancel branch at TimelineClipDiamonds.tsx:444-452 (null the ref, setPreview(null), release capture).
P3 — no auto-scroll when the pointer nears the timeline viewport edge. Long clips scrolled off-screen can't be retimed to their edges without releasing and re-scrolling.

Lens B — constraint enforcement — solid. sortedClipPcts (:190) feeds both previewClipPct and resolveKeyframeDrag, so the gesture layer clamps to immediate neighbours + clip bounds before this file ever sees the drop. The subsequent clipToTweenPercentage extrapolation is defensively clamped to the animation's own tween-% range at TimelineClipDiamonds.tsx:378-385, so a boundary drag can't reselect an out-of-range tween-% (150%) even though the mutation would clamp back. Nice.

Lens C — undo integration — clean. One onMoveKeyframe per drag, fired only on pointerup (:404); setPreview during pointermove is visual-only, never persisted. resolveKeyframeRetime returns noop at keyframeRetime.ts:74/125 when the drop resolves onto the source (< 0.1% tween epsilon or within EPSILON_TIME on flat tweens), so a click with a few px of trackpad jitter can't sneak a phantom undo entry. Nothing to add.

Lens D — multi-select drag parity — flag.

P2 — multi-select retime is not implemented. The lane accepts selectedKeyframes: ReadonlySet<string> and isKfSelected at TimelineClipDiamonds.tsx:300-302 reads it, but onPointerDown/onPointerUp only operate on the pressed diamond (d.kfKey, target). If two or more keyframes are selected and the user drags one, only that one moves; the rest stay put. If Family-B's plan is "multi-select drag comes in a later stack PR" that's fine — just call it out here so reviewers don't assume the shipped selection primitive implies group retime. If multi-drag is intentionally scoped out of this stack, that's worth a short comment near the retime dispatch (:369-415).

Lens E — perf & throttling — mild.

P2 — setPreview on every pointermove without rAF throttling. onPointerMove at TimelineClipDiamonds.tsx:318-337 calls setPreview per event. React 18 batches, but on a 120Hz trackpad + a densely populated expanded lane row (each diamond's React.memo still evaluates its props), this is one render per event. A requestAnimationFrame gate (accumulate the latest clientX, flush once per rAF) would drop most of them and matches how the timeline's clip drag path throttles elsewhere.
P3 — post-drop snap-back window. renderPct at :293 reads only from preview (cleared on pointerup at :350) or kf.percentage (stale cache until the mutation round-trips). Between pointerup and the next cache tick, the diamond visually snaps back to its old %. The pendingRetimeRef map fixes the identity problem for rapid double-drags but doesn't influence rendering. If the GSAP mutation is synchronous in practice this is invisible; if it isn't, adding pendingRetimeRef.current.get(kfKey)?.clipPct into the renderPct fallback would remove the flicker for free. Not a blocker — worth a quick check on a real clip.

Facade blast-radius

onMoveKeyframe widened to Promise<boolean> across four surfaces (timelineCallbacks.ts, TimelineLanes.tsx, useTimelineEditCallbacks.ts, TimelineClipDiamonds.tsx) — the ?? Promise.resolve(false) fallback in the TimelineClipDiamonds facade (:501) preserves the "undefined-callback → non-committing" semantic. handleGsapMoveKeyframe / handleGsapResizeKeyframedTween remain fire-and-forget on the callee side; the boolean is used only by pendingRetimeRef.clearPending and by the test that asserts a false result clears pending. Consistent.

PR-body claims audit

"Every commit in the stack typechecks independently" + "the widening belongs here" — matches: this PR is where TimelineClipDiamonds starts awaiting the commit result (:404), so pulling the return-type change forward into #2781/#2782 would produce a mismatched signature on the callee. Correct placement.
Test coverage claims match diff: 5 new gesture tests (click-armed no-op, retimed reselect, rapid second retime, failed-pending clear, ambiguous ease segment) + 2 flat-tween tests in the pure resolver.
Deferred findings section correctly matches the code (I re-verified :139/:424/:456).

— Via

james-russo-rames-d-jusso

Reviewed at 50a90772cabc.

R2 adversarial pass on the retiming interactions slice. Two blockers flagged: (1) the Promise committed signal introduced in this PR is cosmetic — the mutation is fire-and-forget through commitMutation.catch, so committed always resolves true regardless of whether the server persist actually succeeded; (2) rapid-second retime is silently discarded because resolveKeyframeTarget(fromClipPct) looks up against the still-stale cache — a keyframe with pending clipPct=75 can't be located when the cache still shows 50. These interact: the pendingRetimeRef cleanup depends on !committed, which the first blocker breaks — so a silent persist failure leaves the pending entry permanently set to a nonexistent destination.

🟢 Verified clean

keyframeRetime.ts round3 replacing round1 is consistent with test-file updates (33.3→33.333, 63.6→63.636)
onPointerCancel handler clearing dragRef + preview and releasing pointer capture matches the drag armed by onPointerDown
timelineKeyframeSelectionKey degenerate cases (no propertyGroup / no animationId) round-trip through diamond render and useTimelineKeyframeHandlers.toggleSelectedKeyframe with consistent shape
The easeAmbiguous && animationId !== undefined gate correctly withholds the inline ease button on collided merged segments, per new tests in TimelineClipDiamonds.test.tsx
handleGsapMoveKeyframeToPlayhead / handleGsapRemoveKeyframe are (still) invoked with correct animId+tweenPct pair by useTimelineEditCallbacks for KeyframeDiamondContextMenu delete + move-to-playhead

Complements Via's parallel adversarial pass (Family B rollup). Where Via found 0 P1 / 17 P2 / 18 P3, the adversarial subagent pass here surfaced additional depth on the mutation-authority thread and the Promise chain.

— Review by Rames D Jusso

An ungrouped tween (mixed property groups classify to propertyGroup undefined) fed keyframeCache but was skipped by every gsapAnimations writer, so the collapsed row drew diamonds the expanded lanes had no source animation to render. Drop the property-group gate at all three writers; lane consumers already filter by group. Also route the same-percentage merge in updateKeyframeCacheFromParsed through deduplicateKeyframes so the easeAmbiguous rule has one owner.

Each keyframe-cache writer re-derived a clip-relative percentage inline, and the post-commit writer rounded to 0.1% while the others used 0.001%. Selection keys embed that number, so a commit-time rewrite could orphan a live key. toClipPercentage owns the rounding, toClipKeyframes owns the whole row (percentage plus the tween percentage and animation identity the lanes read), and the parsed write reuses elementCacheKeys instead of open-coding the three key variants.

R3 review follow-ups on the keyframe cache: - clearKeyframeCacheForFile collected ids from the index.html alias prefix too, so a re-scan of one composition file wiped rows a sibling file had just written (several files re-scan concurrently). Only the file's own prefixed keys name the ids now; clearKeyframeCacheForElement still takes the alias and bare key with them. - toClipKeyframes fell back to a fixed 1s tween duration, which put a duration-less tween's keyframes at a percentage no edit path agreed with. It now spans the clip, matching resolveEditableTweenDuration. - collectAnimatableKeyframeProperties takes `object` so call sites drop their `as Record<string, unknown>` casts. Regression tests cover both fixes.

…tack tip The R1/R3 residuals on this PR were fixed at the top of the stack, so they only cleared once every branch above landed. They belong here, next to the code they correct: - `idFromSelector` inverts `idSelector` for both regex readers, so the post-commit cache refresh stops skipping the CSS-unsafe ids `idSelector` exists to support. - `deduplicateKeyframes` drops `ease` when it is ambiguous; the flag was the only honest answer and the last-writer-wins curve belonged to an arbitrary colliding tween. - `isStaticPositionHold` is now the single owner of the hold skip. The `sourceAnimations` filter and the `allKeyframes` filter had diverged on whether `immediateRender` counts as a property. - The keyframe-cache setters no-op when the write changes nothing, instead of handing every subscriber a fresh Map. - `reset()` clears `focusedEaseSegment`. - The test hook `delete`s its window key rather than setting it to undefined, so feature detection still works. - The `toClipKeyframes` fixture uses `as unknown as T` with the justification CONTRIBUTING.md asks for.

Dragging the playhead to the start of the composition needed a very slow drag. The scrub surface begins GUTTER + TRACKS_LEFT_PAD px right of the viewport edge, and both scrub paths bailed out when the pointer sat left of that origin rather than clamping. So the last 80px of the drag toward zero silently did nothing: the playhead stuck at whatever the last in-range sample reported, and only a drag slow enough to sample inside the thin sliver before the origin ever reached 0. Both paths now share getTimelineScrubTime, which clamps to [0, duration]. One owner, so the live-feedback path and the committed-seek path cannot disagree about the edge again.

The local extractIdFromSelector duplicated the `#id`-only regex that idFromSelector replaced, so both DOM-less paths in resolveSelectorElementIds (no-iframe fallback and querySelectorAll-throw recovery) read no id at all for the bracketed `[id="..."]` form writers emit for CSS-unsafe ids. Deleted the duplicate and imported the shared reader; both forms now resolve.

Rows stopped sharing one pixel height when lanes gained expansion, so the only production caller was passing cumulative row coordinates with trackHeight 1 and both scrollTops zeroed. The parameter names described units the values no longer carried. The vertical axis is now a row index and the caller keeps ownership of folding scroll and per-row heights into it.

The getTimelineRowTop docblock had a second copy sitting on TimelineTrackHeightClip, where it describes nothing. Only the one on the function stays.

…view Escape now ends an in-flight diamond drag the way it already ends clip and element drags: the armed gesture is marked cancelled, the preview is dropped, and the pointerup that follows is swallowed instead of falling through to the click branch. The preview also flushes once per animation frame instead of once per pointermove, so a high-rate trackpad no longer re-renders every diamond in the row several times a frame. Single-diamond retime stays the documented scope; multi-select drag needs a batched mutation the script ops do not express yet.

CONTRIBUTING.md asks for a guard clause rather than a non-null assertion outside an already-checked path. The index check and the lookup are now the same guard.

vanceingalls

Reviewed at 6e0118cb (code-review max, delta lens over Rames's pass).

Verdict

COMMENTED. No independent stamp on this slice — Rames's two 🔴 blockers stand at this head, and my own re-verification against useTimelineEditCallbacks.ts:155-207 + TimelineClipDiamonds.tsx:440-457 confirms both mechanisms. The upshot of the review is a Graphite-stack posture check: Miguel has explicitly deferred both blockers to the stack tip 6ee750fee (PR #2791), and file-diff verification against that SHA shows the fixes are structural, not cosmetic — see the "Peer state / stack-tip verification" section below. If the stack ships as one squash-merge, this PR is fine to carry the blockers into #2791; if it were to merge alone, the retime feature ships broken.

Scope

+880/-86 across 11 files, all under packages/studio/src/{components/{editor,nle},player/components}. Adds direct drag-to-retime for keyframe diamonds, an inline segment ease button on hover, an Escape-cancel path for in-flight drags, a per-frame preview throttle, and a Promise<boolean> widening on onMoveKeyframe through timelineCallbacks.ts:69-73 → TimelineLanes.tsx:77 → useTimelineEditCallbacks.ts:155. Net-new: timelineKeyframeIdentity.ts (17 lines — selection-key builder) and useTimelineKeyframeHandlers.ts/.test.tsx (108/108 — an unused hook extraction; see Peer state).

Retiming primitive semantics

Primitive. Move-vs-resize decision is pure in keyframeRetime.ts:96-164. Within-window drop → kind:"move" re-keys the tween-%. Past-boundary drop grows the tween window and remaps every other keyframe by absolute time — value + ease preserved. Flat-tween boundary diamonds get a dedicated resolveFlatTweenBoundaryRetime path so they can never delegate to the authored-keyframe branch.
Round-trip. Drop clip-% → dropAbsTime = elStart + (toClipPct/100)*elDuration (l.171) → decision → handleGsapMoveKeyframe(animId, target.tweenPct, decision.toTweenPct). The clip-% is thrown away past the entry point; the mutation keys on tween-% throughout. The compat shim at TimelineClipDiamonds.tsx:549-555 unwraps target.percentage and drops propertyGroup/animationId/tweenPercentage — that IS the mechanism Rames blocker #2 (identity loss) surfaces through, see below.
Modifier keys. shiftKey on pointerup routes to onShiftClickKeyframe for multi-select — no interaction with drag. No Alt/Cmd affordance for snap-off / precision / constrain. Scoped-out at TimelineClipDiamonds.tsx:400-403 as "single-diamond retime by design"; multi-select drag deferred to a batched mutation the script ops don't express yet. Reasonable given the ops boundary.
Snap / constraint. No snap-to-grid or snap-to-neighbour beyond the neighbour clamp in resolveKeyframeDrag/previewClipPct. Neighbour clamp reads sortedClipPcts (l.222), which is stale-cache-only — Rames flagged this as 🟡 (finding #7).
Transaction grouping. One mutation per pointer-up (l.455). Not per-pointermove. Correct — pointer-move only throttles a setPreview state update via rAF (l.359-375); no writes fire until release.
Mid-drag revert. Escape handler at l.172-184 marks dragRef.current.cancelled=true, cancels the preview rAF, drops preview state, and the subsequent pointerup at l.379-385 swallows the release without firing a click. Clean. pointercancel at l.495-504 mirrors the drop-without-click path.
Boundary policy. Past-boundary drops grow the tween window (resize decision); the neighbour clamp inside the drag helper prevents cross-neighbour reorder (subject to finding #7's stale-cache caveat).
Playback interaction. Not audited — drag is pointer-driven state; the playhead effect at onClickKeyframe (l.461-465) parks selection at the new position but does not pause playback. Out of scope for this PR's thesis.

Editor-UI 12 lenses

Silent-catch. Two .catch-shaped surfaces: (a) the .then(committed => ..., clearPending) promise handler at l.455-457 uses clearPending as the reject handler — the rejection is silent from the user's POV (no toast, no telemetry), only the pending-map entry is dropped; (b) the pointercancel at l.495-504 is silent by design. (a) shipping without observability on a persist failure is intentional (the diamond snaps back to cache) but note it explicitly; Rames finding #5 (🟠) covers the leak case, mine covers the observability case.
Commit semantics. Single-write-per-drag confirmed (l.455 is the one mutation call); rAF-throttled preview does NOT trigger writes.
Key stability. React key on the diamond <button> is ${i}-${kf.percentage} (l.471) — index+percentage. During a rapid retime, the diamond's percentage changes mid-drag ONLY in the preview (visual), not in kf.percentage (source of the key). The button node identity is stable across the drag. OK.
ARIA. Diamonds are <button> with title and aria-label on the ease button (l.290). No role="slider" on the diamond itself — keyboard drag-to-retime is not wired. Rames finding #4 (🟠) is the sibling gap for the ease button; the diamond itself has the same gap but is out of scope for this PR (drag is the new affordance).
Keyboard drag. Not implemented. See Rames #4.
Propagation. stopPropagation on button pointerdown/pointerup (l.339, 394) + suppressNextClick mechanism (l.197-203) documented to defeat the browser's post-pointer synthetic click on ancestor. Clean.
Semantic-vs-symptom. Rames blocker #1 (committed boolean cosmetic) IS the semantic — the caller's .then((committed) => if (!committed) clearPending()) at l.455-457 encodes an "await persist" mental model that the callback signature does not deliver. See Fix-internal audit.
Sibling helpers. onDeleteKeyframe/onMoveKeyframeToPlayhead in useTimelineEditCallbacks.ts:138-146 share the resolveKeyframeTarget(pct) stale-cache lookup pattern with the retime path — the identity-drop mechanism (Rames #2) applies to them too whenever the cache hasn't caught up with a prior write. Not exercised at this PR's scope (delete/move-to-playhead don't run under rapid-fire drag) but the shape is identical.
Parity claim audit. No external-spec parity claim in body — scoped as internal script-op capability. Skip.
Cross-mode. Segment ease button (l.271-316) is hover-only, only visible when hoveredSegment===i; keyboard-tab traversal cannot reach it. Rames #4 (🟠).
Perf audit. sortedClipPcts = sorted.map(k => k.percentage) at l.222 recomputed inline every render — 100-diamond row allocates a fresh array per render; downstream resolveKeyframeDrag/previewClipPct receive a fresh ref each call. Cosmetic; useMemo(() => sorted.map(...), [sorted]) would tighten but the fresh-object allocation is already dwarfed by the surrounding preview-state churn. Non-blocking.
PR-body vs diff. Body claims: Escape cancels + swallows next pointerup (verified l.172-184 + 379-385); one preview render per frame (verified l.359-375); guard-clause replaces non-null on prev-keyframe (verified l.241-243, matches commit 6e0118cb's message); multi-select scoped out with in-code comment (verified l.400-403); Promise<boolean> widening on onMoveKeyframe for typecheck reasons (verified across timelineCallbacks:69-73 → TimelineLanes:77 → useTimelineEditCallbacks:155). Body's committed semantics implicitly (via "starts awaiting the commit result") over-promise the boolean — same shape as blocker #1.

Standards + Spec + Precision + Round-trip

Standards (Standards lens re-run below is the mechanical audit — clean).
Spec forward. All PR-body bullets deliver in-diff. No missing coverage.
Spec reverse. One undisclosed extraction: useTimelineKeyframeHandlers (108 lines) + its test (108 lines) — Rames #6 (🟠). Not called by any file in the PR's own diff; consumers live upstream at the stack tip. Body doesn't mention this addition.
Sibling-precision divergence. Tolerances at HEAD: pending-cleanup effect uses < 0.2 (TimelineClipDiamonds.tsx:151), resolveTimelineKeyframeTarget uses < 0.2 (useTimelineEditCallbacks.ts:64), onClickKeyframe at-playhead uses < 0.5 (TimelineClipDiamonds.tsx:333), context-menu keyframe find uses < 0.5 and < 0.2 (useTimelineKeyframeHandlers.ts:44,86), onToggleKeyframeAtPlayhead uses <= 1 (useTimelineEditCallbacks.ts:222). Five tolerances across four sites for the same "keyframe near this %" question. The <= 1 in onToggleKeyframeAtPlayhead is the widest and the outlier — it deliberately treats "roughly at playhead" more loosely than a fine-grained drag would want. Not new to this PR; pre-existing across the stack. Non-blocking observation, but a candidate for a KEYFRAME_PCT_TOLERANCE constant.
Middle-man wrap/unwrap. TimelineClipDiamonds.tsx:549-555 compat shim: (target, toClipPercentage) => props.onMoveKeyframe?.(props.elementId, target.percentage, toClipPercentage) — unwraps target.percentage, drops propertyGroup/animationId/tweenPercentage. IS the wire-level surface of Rames blocker #2: identity that the diamond composed from pendingRetimeRef is dropped at this boundary; the receiving useTimelineEditCallbacks.onMoveKeyframe (l.155-207) has to re-derive by resolveKeyframeTarget(fromClipPct) off the stale cache, which is the exact silent failure. Miguel's tip fix widens the callback signature to (elId, keyframe: TimelineKeyframeTarget, toClipPct) — the identity survives. Confirms Rames's blocker is a structural middle-man drop, not just a state-timing artifact.

Six-category internal-boundary (adversarial fix-internal audit)

The fix's advertised guarantee is "rapid second retime composes from pending position." The primitives to audit at HEAD:

State discard on error. pendingRetimeRef set at l.449, cleared on !committed || reject at l.450-454. The .then((committed) => if (!committed) clearPending(), clearPending) shape is correct in intent — but if committed is meaningless (blocker #1) the !committed branch never runs, and the useEffect at l.146-155 becomes the sole GC path. That path matches on tolerance-not-identity (Rames finding #8 🟢), so on silent failure the ref leaks. Two-primitive silent-X: cleanup depends on either an honest committed OR an identity-keyed cache reflection; PR delivers neither.
Return-boundary invariant. onMoveKeyframe in useTimelineEditCallbacks.ts:155-207 return true at l.206 after every path except explicit return false. Not tied to handleGsap* returning success. Blocker #1 IS this axis: the primitive's "successful return" says nothing about the underlying persist.
Library defaults. handleGsap* handlers themselves — not audited in this PR's diff. If they use commitMutation.catch(...) fire-and-forget (as Rames's block asserts), the boolean's semantics decompose from "did it persist" to "did we call the branch." Verification requires reading DomEditContext — out of this PR's scope but the receiving contract matters.
Precedence in overlap rules. The pending-cleanup effect at l.146-155 matches by Math.abs(k.percentage - pending.clipPct) < 0.2 on ANY keyframe in the row (Rames finding #8 🟢). In a multi-keyframe row an unrelated sibling near the destination % silently clears the pending entry for the drag that hasn't settled yet. Precedence bug: nearest-neighbour wins over identity.
Session/resource ownership. dragRef.current is nulled at every terminal branch (pointerup, pointercancel, escape) and previewFrameRef is cleared on every cleanup path (cancelPreviewFrame). Correct.
Discovery/enumeration completeness. The pending-cleanup effect enumerates pendingRetimeRef.current (all entries) and checks each against keyframesData.keyframes (all row keyframes) — both directions covered. But the match predicate is asymmetric (tolerance-only, not identity), so completeness of enumeration doesn't compensate for wrong pairing.

React refactor hygiene

Dead props. KeyframeDiamondContextMenu.tsx widens the state interface (+ propertyGroup?, tweenPercentage?, animationId?) and the onDelete/onMoveToPlayhead prop signatures (+3 args). These wider props are gathered into the state (l.5-14) and forwarded to the handlers (l.68-74, l.88-93), but the callers wiring onDeleteKeyframe/onMoveKeyframeToPlayhead at the TimelineLanes/TimelineEditCallbacks boundary receive only (elId, percentage) — the extra 3 args are DROPPED at the callback boundary. Not net-broken (delete-by-percentage still works when the cache is fresh) but the added args have no consumer at this SHA. Ships-with-tip pattern; note.
Falsy-zero. Grep for \|\|.*(percentage|duration|position|start) clean in the changed lines.
Mid-drag useEffect(setDraft, [value]). No such pattern.
useEffect for state syncing. Two useEffects in TimelineClipDiamonds.tsx: l.146 (pending-map GC via cache reflection — external synchronization to store state, legitimate) and l.172 (keydown subscription — legitimate). Neither is state-syncing.
Fast Refresh. All .tsx files export only components + hooks. Clean.
List virtualization. Diamond row length is bounded by the animation's keyframe count (author-driven, small N). Not applicable.
Semantic tokens. Style props use raw hex (#171717, #a3a3a3, rgba(255,255,255,0.14)) inline. This is the studio package, not packages/movio — semantic-token discipline is a movio rule; the studio package uses raw color tokens for the internal editor UI. Not applicable here.
Emojis in UI. Grep clean.
CSS-in-JS. No Emotion. Inline style props are used per package convention.

Element identity / keying

Selection key via timelineKeyframeSelectionKey(elementId, target) at timelineKeyframeIdentity.ts:8-17 — collapsed-row uses ${elementId}:${percentage}, group-aware row uses ${elementId}:${propertyGroup}[:${animationId}]:${percentage}. Stable identity for React and for the selectedKeyframes Set.
React key on the diamond <button> at l.471 is ${i}-${kf.percentage} — index + percentage, mount-stable across the drag (percentage is source, not preview).
pendingRetimeRef keys by kfKey (l.323) — stable identity throughout the drag.
The identity-loss mechanism Rames #2 flags is NOT a keying bug at the React layer; it's an identity-drop at the compat-shim callback wrapper (Middle-man wrap/unwrap section above). Confirmed distinct.

Standards lens re-run

Files audited at 6e0118cb: TimelineClipDiamonds.tsx, useTimelineEditCallbacks.ts, useTimelineKeyframeHandlers.ts, timelineKeyframeIdentity.ts, keyframeRetime.ts, KeyframeDiamondContextMenu.tsx, timelineCallbacks.ts, TimelineLanes.tsx, plus the two test files.

Bare as T assertions (\bas +[A-Z]): 0 hits (one legitimate MouseEvent as ReactMouseEvent type-import rename in useTimelineKeyframeHandlers.ts:1 — not a value cast).
Non-null ! assertions (\w+!\., \w+!\[, \w+!;): 0 hits at HEAD. (One was removed in commit 6e0118cb — the diamond-connector sorted[i-1]! replaced with the guard-clause const prev = sorted[i-1]; if (!prev) return null;. PR body correctly names this.)
.message access without instanceof Error narrowing: 0 hits.
Angle-bracket casts: 0 hits.
Test files: .message on catch: 0 hits.

No residual Standards issues.

Deletions (the -86 lines)

Not purely additive — Miguel replaces existing behavior in three places:

TimelineClipDiamonds.tsx:139 — non-null on prev-keyframe replaced by guard clause (commit 6e0118cb). Delta preserves behavior (same skip when i===0); guard is stricter and matches CONTRIBUTING.md. Verified.
useTimelineEditCallbacks.ts onMoveKeyframe signature widened from Promise<void>|void to Promise<boolean>. Deletions: the old void-return branch. Callers rewired. Verified.
TimelineLanes.tsx:479 — onMoveKeyframe threaded through to TimelineClipDiamonds. One-line prop pass-through, no behavior deleted.

Test deletions (TimelineClipDiamonds.test.tsx -15) are edits around the pointerEvent helper and setup — not test coverage removed, refactored. Sampled the new test at l.223 (composes a rapid second retime from the pending position) and l.302 (clears a failed pending retime when the callback returns false / rejects) — these two directly exercise the pending-map contract Rames blocker #1/#2 falsify at the integration boundary. Unit-mock passes because vi.fn().mockResolvedValue(true) doesn't exercise the stale-cache lookup in useTimelineEditCallbacks.onMoveKeyframe. Symmetric-test discipline gap: the unit test asserts what the CALLER sends, not what the REAL callback resolves against.

Non-blocking observations

sortedClipPcts at TimelineClipDiamonds.tsx:222 allocates a fresh array per render (see Perf audit).
Tolerance divergence for "keyframe near this %" across 5 sites (0.2, 0.2, 0.5, 0.2, <= 1) — pre-existing across the stack, worth consolidating into a named constant.
KeyframeDiamondContextMenu widened interface has no consumer at HEAD; the extra propertyGroup/tweenPercentage/animationId args land upstream (part of Rames #6).

Peer state / stack-tip verification

Rames posted a full R2 pass at 50a90772 (before this head): 2× 🔴 (Promise cosmetic; rapid-second retime silent), 4× 🟠 (segment ease button keyboard-unreachable; pendingRetimeRef leak on silent failure; optimistic seek+select before commit; useTimelineKeyframeHandlers no consumer), 1× 🟡 (neighbour clamp stale-cache), 3× 🟢. All mechanisms re-verified at 6e0118cb; the fix scope hasn't changed.

Miguel replied to every Rames comment with the canned deferral: "Addressed at the stack tip in 6ee750fee (PR #2791). This branch is an ancestor in the same stack, so the fix ships with it rather than appearing in this PR's own diff." Verified against 6ee750fee (fix(studio): close the review findings that survived the stack) and against the tip head edbb6d102c1b:

Blocker #1 (Promise cosmetic). At tip, useTimelineEditCallbacks.onMoveKeyframe returns the mutation's own return: return handleGsapMoveKeyframe(target.animId, target.tweenPct, decision.toTweenPct, sel). The callback now propagates the actual persist result. Structural fix, verified.
Blocker #2 (rapid-second retime identity loss). At tip, onMoveKeyframe signature widens to (elId, keyframe: TimelineKeyframeTarget, toClipPct) and resolveKeyframeTarget short-circuits the stale-cache lookup when the target carriesIdentity (propertyGroup !== undefined || tweenPercentage !== undefined || animationId !== undefined). Identity survives round-trip. Structural fix, verified.
Rames #5, #7, #8 named in the tip commit body: "The pending-retime bookkeeping matches on keyframe identity, not just on 'something is near that percentage'" + "The neighbour clamp composes pending destinations in before sorting, so a second drag can't cross a neighbour that already moved." Verified against diff.
Rames #6 (no consumer): The tip wires useTimelineKeyframeHandlers into the timeline call-site. Consumer verified.

CI at HEAD: Preflight (lint+format) pass, player-perf pass, Preview parity pass, preview-regression pass; Graphite/mergeability_check pending; per-shard Perf/regression-shards skipping (change-gate). No red required checks.

Envelope: 3 commits, no Co-Authored-By: trailer, no 🤖 Generated with Claude Code footer. Clean.

Stamp posture

No stamp — COMMENTED. Rames's blockers stand at this PR's own head, and I concur; stamping here in isolation would rubber-stamp through a real defect. But the stack-tip fix verifies at both the mechanism and file level, so I'm not filing CHANGES_REQUESTED either — Miguel's Graphite-stack posture is honest and the tip's edbb6d102c1b is where the review-stamp gate for the merge belongs. If PR #2791 comes up for review at its own tip, the stamp gate there absorbs this slice's blockers.

— Review by Via

james-russo-rames-d-jusso

Reviewed at 6e0118cb3f5adf43bec8ee08bac89a164b480328 — B3 slice, +880/-86.

Big win in the DiamondLane extraction: making the row group-aware, threading TimelineKeyframeTarget through the whole handler chain, unifying selection keys via timelineKeyframeSelectionKey, adding Escape to cancel an in-flight retime, rAF-throttling the preview, and hoisting the diamond-size decision to LANE_H · DIAMOND_RATIO so the collapsed row and property lanes render at parity. The pendingRetimeRef bridge for rapid second retimes is a nice touch — the retest coverage (composes a rapid second retime from the pending position, clears a failed pending retime when the callback returns false / rejects, cancels an in-flight retime on Escape) is thorough. TimelineDiamondLane at this SHA is only wired via the TimelineClipDiamonds adapter (which forces groupAware=false and routes to the legacy 3-arg callback contract); the group-aware direct-consumer wiring lands in a later slice, so the contract additions here are foundational.

Foundational-contract lens applied. Highlights:

timelineKeyframeSelectionKey builds three key shapes (elId:pct, elId:group:pct, elId:group:animId:pct) — all consumers I could find go through the helper (useTimelineKeyframeHandlers and the adapter's translations), so the key-format change is safe. Good.
easeAmbiguous semantics: the collapsed row hides the inline ease button on segments ending on an ambiguous keyframe or one without animationId. Tests cover both. Correct — a merged-row button can't target a single tween's ease.
isStaticPositionHold unification (from R4-fix-hoist on 2781) still holds — no divergence introduced here.

Concerns

🟡 Promise<boolean> contract semantics are ambiguous — true means "dispatched", not "committed". TimelineDiamondLane reads the promise's false-or-rejection as "retime failed → clear the pending entry" (TimelineClipDiamonds.tsx — void onMoveKeyframe?.(...).then((committed) => { if (!committed) clearPending(); }, clearPending);). The concrete implementation in useTimelineEditCallbacks.ts at line 155 (onMoveKeyframe: async (_elId, fromClipPct, toClipPct) => { ... }) only returns false from pre-check failures (target/sel/anim couldn't resolve, or the decision wasn't move/resize); every success path dispatches through the sync handleGsapMoveKeyframe/handleGsapResizeKeyframedTween/handleGsapUpdateMeta wrappers and then returns true unconditionally. So if moveKeyframe fails async (script-ops write error, network hiccup on remote persistence, etc.), the promise resolves true, the pending entry survives, and the effect at TimelineClipDiamonds.tsx:151 won't clear it because the cache never updates. The next drag on the (visually-unchanged) diamond then uses the phantom pending destination as its fromTarget, and the mutation targets a keyframe that isn't there. Not likely to fire in practice today (the studio's mutation pipeline is fire-and-forget by design), but the coverage tests at lines ~285-315 assert exactly this cleanup path and it's dormant against the real callback. Options: (a) tighten the contract to Promise<void> and document "the pending bridge is a rendering optimization, not a commit guarantee" — the cleanup effect becomes the only path; or (b) plumb the real commit result back from moveKeyframe/resizeKeyframedTween so true/false reflects what actually landed.
🟡 pendingRetimeRef cleanup at TimelineClipDiamonds.tsx:150-159 matches by proximity, not identity. The effect iterates pending entries and clears any whose destination is within 0.2% of any keyframe in keyframesData.keyframes — not the specific keyframe identified by the entry's key. In practice the diamond re-keys once the cache updates (its kfKey shifts from the pre-move percentage to the committed one), so an orphaned entry can't affect the wrong drag — the cache-update effect always fires with the new-shape keyframes, and a stale entry that survives only bloats the ref. But the 0.2% proximity match can also fire early: a densely packed group-aware lane with a neighbor keyframe within 0.2% of the pending destination would clear the entry before the actual commit lands. Consider matching by (animationId, tweenPercentage) when those are set — that pins the check to the specific keyframe the entry references, and the collapsed-row case can still fall back to the current clip-% match.

Nits

round1 → round3 (keyframeRetime.ts:154) — the precision bump propagates into pctRemap values that downstream resizeKeyframedTween consumers rely on for keyframe-key lookup. Not a bug (the consumer does Math.abs(...) < tol matching), but any code path that stringifies + compares (String(pct) as a Map key, say) would silently split what used to be one bucket. I didn't find such a callsite; noting in case one exists in a sibling slice.
Diamond size at TimelineClipDiamonds.tsx:210-213 is now Math.round(LANE_H * DIAMOND_RATIO) for beat-inactive clips (was Math.round(clipHeightPx * DIAMOND_RATIO)). Comment explains this is intentional parity across contexts. Confirm the visual change lands where Figma expects — collapsed clips with clipHeightPx > LANE_H used to render bigger diamonds and now render smaller.

What I didn't verify

Whether any external non-Studio consumer imports TimelineDiamondKeyframe — the interface was previously non-exported and was renamed from KeyframeEntry. git ls-files | xargs grep inside packages/studio was clean, but a cross-repo consumer (docs, storybook, another workspace package) could exist that I didn't sweep.
Whether applyKeyframeRetime-like commit paths have async error signals that could be plumbed back through the promise. That's the "option (b)" in the first concern.

— Review by Rames D Jusso

vanceingalls

Reviewed at 6e0118cb3f5adf43bec8ee08bac89a164b480328 (code-review max, R3, delta vs Rames's fresh pass at 4790113912).

Verdict

APPROVE. Rames re-reviewed at this same SHA one minute after my R2 and downgraded both prior 🔴 blockers to 🟡 concerns, explicitly noting the Promise<boolean> contract mismatch is "not likely to fire in practice today (the studio's mutation pipeline is fire-and-forget by design)." Re-verifying his two remaining concerns at head: both are real but non-functional at this SHA (the code works; the contract narrative overpromises), and both are already fixed structurally at the stack tip 6ee750fee (PR #2791) that Miguel is carrying. No new defect surfaces from my Editor-UI/Standards/Spec/Precision/Round-trip lens sweep. Stamping this slice as ancestor-in-stack.

Rames R3 findings — status at this head

File:line	Claim (🟡)	Verdict	Evidence
`TimelineClipDiamonds.tsx:455` + `useTimelineEditCallbacks.ts:155-207`	`Promise<boolean>` — `true` means "dispatched," not "committed"	Real, non-blocking	Verified at head: `useTimelineEditCallbacks.ts:180` calls `handleGsapMoveKeyframe(...)` fire-and-forget then unconditionally `return true` at l.206. Every success path returns `true` regardless of whether the mutation persisted. Rames's own qualifier ("not likely to fire in practice today") holds — the studio's mutation pipeline is synchronous fire-and-forget. Contract clarity issue, not a live defect. Miguel's stack-tip fix propagates the actual mutation return through the callback (verified against `6ee750fee` in my R2). Ships with the stack.
`TimelineClipDiamonds.tsx:150-159`	`pendingRetimeRef` cleanup matches by proximity, not identity	Real, non-blocking	Verified at head: `keyframesData.keyframes.some((k) => Math.abs(k.percentage - pending.clipPct) < 0.2)` — clears the entry when ANY keyframe in the row sits within 0.2% of the pending destination. Same mechanism as Rames's prior 🟢 finding #8, upgraded to 🟡 in this pass because a densely-packed group-aware lane can shorten the window. Miguel's tip fix (`6ee750fee`) matches by keyframe identity when `carriesIdentity` is set. Ships with the stack.
`keyframeRetime.ts:154` (nit)	`round3` propagates into `pctRemap` — `String(pct)` map key could split buckets	Speculative, no callsite	Grepped: no `String(pct)` / `Map<string>` keyed on `pctRemap.to` value in the changed files or their direct consumers. `resizeKeyframedTween` consumer keys by `Math.abs(...) < tol` (verified — `useTimelineEditCallbacks.ts` uses tolerance matching throughout). Rames flagged as "noting in case one exists in a sibling slice" — clean at this scope.
`TimelineClipDiamonds.tsx:210-213` (nit)	Diamond size change: `LANE_H * DIAMOND_RATIO` vs previous `clipHeightPx * DIAMOND_RATIO`	Intentional design change	Comment at l.209-211 explicitly names the parity goal ("One consistent keyframe-diamond size everywhere (clip bars + property lanes)"). `LANE_H = 28` → 22px diamonds when `!beatsActive`. Tall clips (`clipHeightPx > 28`) now render at the shared 22px. Non-blocker; if Figma expects a taller diamond for tall clips a follow-up can revisit.

Count: 2 real 🟡 concerns (both structurally fixed at stack tip, non-functional at this SHA), 2 nits (one speculative, one intentional).

Miguel's inline comments — status

Miguel posted 10 canned deferral replies at 14:49Z 2026-07-27, one per Rames R1 comment (ids 3658272600-3658275650). All 10 say verbatim: "Addressed at the stack tip in 6ee750fee (PR #2791). This branch is an ancestor in the same stack, so the fix ships with it rather than appearing in this PR's own diff." Each carries typecheck + oxlint + test verification against the tip. Rames's fresh R3 review does not re-litigate the deferral — he re-scopes to the two 🟡 concerns above and the 2 nits, leaving the stack-tip posture in place.

Deferral honesty verified at tip head edbb6d102c1b in my R2 (see Peer state / stack-tip verification there). No residual Miguel comments live in this PR outside the deferral chain.

Fix-internal remaining-silent-X audit

Adversarial internal-boundary sweep on the retime feature at this SHA:

State discard on error. pendingRetimeRef set at l.449, cleared in the .then((committed) => if (!committed) clearPending(), clearPending) handler at l.455-457. On silent async failure the !committed branch does not fire (Rames concern #1). Cleanup then depends solely on the tolerance-match useEffect at l.146-155. Two-primitive silent-X (honest committed OR identity-keyed cache reflection) — at this SHA neither delivers. The ref does not leak to disk, only to the in-memory Map; the ceiling is bounded by row keyframe count. Mitigation ships with 6ee750fee.
Return-boundary invariant. onMoveKeyframe in useTimelineEditCallbacks.ts:155-207 returns true after every success dispatch, false only from pre-check failures. The primitive's return is not tied to the underlying handleGsap* handler's persist result — that IS Rames concern #1. Same mitigation.
Library defaults. handleGsapMoveKeyframe / handleGsapResizeKeyframedTween / handleGsapUpdateMeta are synchronous fire-and-forget wrappers into DomEditActionsContext. Not audited in this PR's diff; the contract is "call it, no return." That matches Rames's "fire-and-forget by design" qualifier.
Precedence in overlap rules. Cleanup effect at l.146-155 matches on ANY keyframe within 0.2%; asymmetric — nearest-neighbour wins over identity. Same as Rames concern #2. Mitigation at tip.
Session / resource ownership. dragRef.current nulled at every terminal branch (pointerup l.395, pointercancel l.500, escape l.175). previewFrameRef cleared via cancelPreviewFrame() on every terminal branch + effect cleanup. pointerCapture released on all terminals. Clean.
Discovery / enumeration completeness. Cleanup effect iterates pendingRetimeRef.current entries (all) and cross-checks against keyframesData.keyframes (all). Both directions covered. The match predicate's asymmetry (see precedence) is the residual defect — completeness alone doesn't compensate.

Net: two axes (state-discard, return-invariant) are the same mechanism as Rames concern #1; one axis (precedence) is Rames concern #2. Session ownership and discovery are clean. Three axes matched to the two 🟡 concerns; no fifth silent-X surfaces.

Standards lens re-run

Files audited at 6e0118cb: TimelineClipDiamonds.tsx, useTimelineEditCallbacks.ts, useTimelineKeyframeHandlers.ts, timelineKeyframeIdentity.ts, keyframeRetime.ts, KeyframeDiamondContextMenu.tsx, timelineCallbacks.ts, TimelineLanes.tsx, plus the two test files.

Bare as T assertions (\bas +[A-Z]): 0 hits in added lines. (One legitimate MouseEvent as ReactMouseEvent type-import rename in useTimelineKeyframeHandlers.ts:1 — not a value cast. Two as HTMLElement casts at TimelineLanes.tsx:362,417 are pre-existing lines, not in this PR's diff.)
Non-null ! assertions (\w+!\., \w+!\[, \w+!;): 0 hits. (The prev-keyframe non-null was already replaced by the guard-clause const prev = sorted[i - 1]; if (!prev) return null; at TimelineClipDiamonds.tsx:242-243.)
.message access without instanceof Error narrowing: 0 hits.
Angle-bracket casts: 0 hits.
Test files: .message on catch: 0 hits.

No residual Standards issues.

Independent findings

Nothing new beyond what my R2 already surfaced (middle-man shim at TimelineClipDiamonds.tsx:549-555; tolerance divergence across 5 sites — pre-existing across the stack). Rames's fresh pass converges on the same mechanisms my R2 identified; no unique defect either of us missed.

One re-observation worth naming: the useEffect at l.146-155 runs on every keyframesData.keyframes re-reference (props identity, not deep). If the parent re-renders and passes a fresh keyframes array without any keyframe-content change, the cleanup effect fires and can clear a pending entry mid-drag when a same-position keyframe already sits in the row. In practice keyframesData is memoized upstream, but this is the second-order axis of Rames concern #2. Non-blocking; ships-with-tip.

Peer state

Rames: fresh R3 at 4790113912 (18:11Z 2026-07-27), COMMENTED — 2× 🟡 (down from prior 🔴), 2 nits. Rames own qualifier: "Not likely to fire in practice today (the studio's mutation pipeline is fire-and-forget by design)." Deferral to stack tip stands.
Miguel: 10 canned deferrals to 6ee750fee (PR #2791) at 14:49Z 2026-07-27, one per Rames R1 comment. Typecheck + oxlint + test verified at tip. No open comments live in this PR outside the deferral chain.
CI: Preflight (lint+format) pass, player-perf pass, Preview parity pass, preview-regression pass, regression pass; per-shard Perf / regression-shards skipping (change-gate). Graphite / mergeability_check in progress (stack orchestration, non-blocking). mergeStateStatus: UNSTABLE reflects Graphite-pending, not a red required check. No red required checks.
Envelope: 3 commits, no Co-Authored-By: trailer, no Generated with Claude Code footer. Clean.

— Review by Via

The base branch was changed.

feat(studio): add keyframe timeline state

e4d7bde

miguel-heygen force-pushed the codex/studio-timeline-b-variable-layout-v2 branch from 1e99115 to 514a219 Compare July 25, 2026 19:45

miguel-heygen force-pushed the codex/studio-timeline-b-keyframe-retiming-v2 branch from 5816e70 to 50a9077 Compare July 25, 2026 19:45

vanceingalls reviewed Jul 25, 2026

View reviewed changes

james-russo-rames-d-jusso reviewed Jul 25, 2026

View reviewed changes

miguel-heygen force-pushed the codex/studio-timeline-b-variable-layout-v2 branch from 514a219 to 97dd041 Compare July 25, 2026 21:17

miguel-heygen force-pushed the codex/studio-timeline-b-keyframe-retiming-v2 branch from 50a9077 to 0779ac9 Compare July 25, 2026 21:17

miguel-heygen force-pushed the codex/studio-timeline-b-variable-layout-v2 branch from 97dd041 to 41b0a68 Compare July 25, 2026 23:51

miguel-heygen force-pushed the codex/studio-timeline-b-keyframe-retiming-v2 branch from 0779ac9 to 32427d9 Compare July 25, 2026 23:51

miguel-heygen force-pushed the codex/studio-timeline-b-variable-layout-v2 branch from 41b0a68 to bb55462 Compare July 27, 2026 14:07

miguel-heygen force-pushed the codex/studio-timeline-b-keyframe-retiming-v2 branch from 32427d9 to 29a9984 Compare July 27, 2026 14:07

miguel-heygen added 2 commits July 27, 2026 19:02

miguel-heygen force-pushed the codex/studio-timeline-b-keyframe-retiming-v2 branch from 29a9984 to e3e3456 Compare July 27, 2026 17:20

miguel-heygen force-pushed the codex/studio-timeline-b-variable-layout-v2 branch from bb55462 to 6f1194d Compare July 27, 2026 17:20

miguel-heygen added 4 commits July 27, 2026 19:51

feat(studio): add variable timeline timing and layout

d518972

refactor(studio): drop the duplicated row-top docblock

4c7703f

The getTimelineRowTop docblock had a second copy sitting on TimelineTrackHeightClip, where it describes nothing. Only the one on the function stays.

miguel-heygen added 3 commits July 27, 2026 19:52

feat(studio): add timeline keyframe retiming interactions

e36fb38

refactor(studio): guard the diamond connector's previous keyframe

6e0118c

CONTRIBUTING.md asks for a guard clause rather than a non-null assertion outside an already-checked path. The index check and the lookup are now the same guard.

miguel-heygen force-pushed the codex/studio-timeline-b-keyframe-retiming-v2 branch from e3e3456 to 6e0118c Compare July 27, 2026 17:54

miguel-heygen force-pushed the codex/studio-timeline-b-variable-layout-v2 branch from 6f1194d to 4c7703f Compare July 27, 2026 17:54

vanceingalls reviewed Jul 27, 2026

View reviewed changes

james-russo-rames-d-jusso reviewed Jul 27, 2026

View reviewed changes

vanceingalls previously approved these changes Jul 27, 2026

View reviewed changes

miguel-heygen changed the base branch from codex/studio-timeline-b-variable-layout-v2 to main July 28, 2026 00:23

miguel-heygen merged commit 6e0118c into main Jul 28, 2026
46 of 56 checks passed

miguel-heygen deleted the codex/studio-timeline-b-keyframe-retiming-v2 branch July 28, 2026 00:37

Navigation Menu

Uh oh!

feat(studio): add timeline keyframe retiming interactions - #2783

feat(studio): add timeline keyframe retiming interactions#2783
miguel-heygen merged 13 commits into
mainfrom
codex/studio-timeline-b-keyframe-retiming-v2

miguel-heygen commented Jul 25, 2026 •

edited

Loading

Uh oh!

miguel-heygen commented Jul 25, 2026 •

edited

Loading

Uh oh!

vanceingalls left a comment

Uh oh!

james-russo-rames-d-jusso left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vanceingalls left a comment

Uh oh!

james-russo-rames-d-jusso left a comment

Uh oh!

vanceingalls left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

miguel-heygen commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

How

Test plan

Deferred review findings

R1 review follow-ups

R2 review follow-ups

Uh oh!

miguel-heygen commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vanceingalls left a comment

Choose a reason for hiding this comment

Adversarial lenses

Facade blast-radius

PR-body claims audit

Uh oh!

james-russo-rames-d-jusso left a comment

Choose a reason for hiding this comment

🟢 Verified clean

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vanceingalls left a comment

Choose a reason for hiding this comment

Verdict

Scope

Retiming primitive semantics

Editor-UI 12 lenses

Standards + Spec + Precision + Round-trip

Six-category internal-boundary (adversarial fix-internal audit)

React refactor hygiene

Element identity / keying

Standards lens re-run

Deletions (the -86 lines)

Non-blocking observations

Peer state / stack-tip verification

Stamp posture

Uh oh!

james-russo-rames-d-jusso left a comment

Choose a reason for hiding this comment

Concerns

Nits

What I didn't verify

Uh oh!

vanceingalls left a comment

Choose a reason for hiding this comment

Verdict

Rames R3 findings — status at this head

Miguel's inline comments — status

Fix-internal remaining-silent-X audit

Standards lens re-run

Independent findings

Peer state

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

miguel-heygen commented Jul 25, 2026 •

edited

Loading

miguel-heygen commented Jul 25, 2026 •

edited

Loading