Merge pull request #2670 from heygen-com/codex/fix-flat-keyframe-retime

fix(studio): retime flat tween keyframe diamonds

packages/studio/src/components/editor/keyframeRetime.test.ts

@@ -9,6 +9,7 @@ const KEYFRAMES: RetimeKeyframe[] = [
   { percentage: 100, properties: { x: 100 }, ease: "power2.in" },
 ];
 const WINDOW = { tweenStart: 2, tweenDuration: 4 };
+const LEFT_BOUNDARY_DROP = { ...WINDOW, dropAbsTime: 0.5 };
 
 describe("resolveKeyframeRetime — move (within the tween window)", () => {
   it("re-keys an interior keyframe to the tween-% of the drop", () => {
@@ -32,15 +33,53 @@ describe("resolveKeyframeRetime — move (within the tween window)", () => {
     expect(r.kind).toBe("noop");
   });
 
-  it("moves a flat (keyframe-less) tween without needing the keyframes array", () => {
+  it("shortens a flat tween when its synthesized end diamond moves left", () => {
     const r = resolveKeyframeRetime({
       ...WINDOW,
       keyframes: [],
       draggedTweenPct: 100,
-      dropAbsTime: 5, // (5-2)/4 = 75%
+      dropAbsTime: 5,
     });
-    expect(r.kind).toBe("move");
+    expect(r.kind).toBe("resize");
-    expect(r.toTweenPct).toBeCloseTo(75, 5);
+    expect(r.position).toBe(2);
+    expect(r.duration).toBe(3);
+    expect(r.pctRemap).toEqual([]);
+  });
+
+  it("moves a flat tween's start while preserving its absolute end", () => {
+    const r = resolveKeyframeRetime({
+      ...WINDOW,
+      keyframes: [],
+      draggedTweenPct: 0,
+      dropAbsTime: 3,
+    });
+    expect(r.kind).toBe("resize");
+    expect(r.position).toBe(3);
+    expect(r.duration).toBe(3);
+    expect(r.pctRemap).toEqual([]);
+  });
+
+  it("no-ops an unexpected interior percentage on a flat tween", () => {
+    expect(
+      resolveKeyframeRetime({
+        ...WINDOW,
+        keyframes: [],
+        draggedTweenPct: 50,
+        dropAbsTime: 5,
+      }).kind,
+    ).toBe("noop");
+  });
+
+  it("no-ops instead of rounding a sub-10ms flat tween to zero", () => {
+    expect(
+      resolveKeyframeRetime({
+        tweenStart: 2,
+        tweenDuration: 4,
+        keyframes: [],
+        draggedTweenPct: 100,
+        dropAbsTime: 2.0004,
+      }).kind,
+    ).toBe("noop");
   });
 });
 
@@ -67,10 +106,9 @@ describe("resolveKeyframeRetime — resize (past the tween boundary)", () => {
 
   it("extends the FIRST keyframe before the start, shifting position earlier", () => {
     const r = resolveKeyframeRetime({
-      ...WINDOW,
+      ...LEFT_BOUNDARY_DROP,
       keyframes: KEYFRAMES,
       draggedTweenPct: 0,
-      dropAbsTime: 0.5, // before start (2) → move position back + grow duration
     });
     expect(r.kind).toBe("resize");
     expect(r.position).toBeCloseTo(0.5, 5);
@@ -102,10 +140,9 @@ describe("resolveKeyframeRetime — single keyframe (both first and last)", () =
 
   it("resizes left before the start", () => {
     const r = resolveKeyframeRetime({
-      ...WINDOW,
+      ...LEFT_BOUNDARY_DROP,
       keyframes: lone,
       draggedTweenPct: 100,
-      dropAbsTime: 0.5,
     });
     expect(r.kind).toBe("resize");
     expect(r.position).toBeCloseTo(0.5, 5);
@@ -127,14 +164,16 @@ describe("resolveKeyframeRetime — guards", () => {
     ).toBe("noop");
   });
 
-  it("no-ops a boundary drop on a flat tween (nothing to remap)", () => {
+  it("extends a flat tween when its synthesized end diamond moves right", () => {
-    expect(
+    const r = resolveKeyframeRetime({
-      resolveKeyframeRetime({
+      ...WINDOW,
-        ...WINDOW,
+      keyframes: [],
-        keyframes: [],
+      draggedTweenPct: 100,
-        draggedTweenPct: 100,
+      dropAbsTime: 8,
-        dropAbsTime: 8,
+    });
-      }).kind,
+    expect(r.kind).toBe("resize");
-    ).toBe("noop");
+    expect(r.position).toBe(2);
+    expect(r.duration).toBe(6);
+    expect(r.pctRemap).toEqual([]);
   });
 });

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

@@ -51,11 +51,40 @@ export interface KeyframeRetimeResult {
 const NOOP_EPSILON_PCT = 0.1;
 /** Slack (seconds) for the within-tween boundary test. */
 const EPSILON_TIME = 1e-4;
+/** Smallest authored tween window; avoids sub-millisecond/round-to-zero durations. */
+const MIN_TWEEN_DURATION = 0.01;
 
 const round3 = (n: number) => Math.round(n * 1000) / 1000;
 const round1 = (n: number) => Math.round(n * 10) / 10; // 0.1% precision
 const clamp = (n: number, lo: number, hi: number) => Math.max(lo, Math.min(hi, n));
 
+/** Resolve timing for a flat tween's synthesized start/end diamond. */
+function resolveFlatTweenBoundaryRetime(opts: {
+  keyframeCount: number;
+  draggedTweenPct: number;
+  tweenStart: number;
+  tweenEnd: number;
+  dropAbsTime: number;
+}): KeyframeRetimeResult | null {
+  const { keyframeCount, draggedTweenPct, tweenStart, tweenEnd, dropAbsTime } = opts;
+  if (keyframeCount > 0) return null;
+  // Flat tweens have only synthesized boundary diamonds. Never delegate an
+  // unexpected interior percentage to the authored-keyframe move path.
+  if (draggedTweenPct !== 0 && draggedTweenPct !== 100) return { kind: "noop" };
+  const draggedTime = draggedTweenPct === 0 ? tweenStart : tweenEnd;
+  if (Math.abs(dropAbsTime - draggedTime) <= EPSILON_TIME) return { kind: "noop" };
+  const newStart = draggedTweenPct === 0 ? dropAbsTime : tweenStart;
+  const newEnd = draggedTweenPct === 100 ? dropAbsTime : tweenEnd;
+  const newDuration = newEnd - newStart;
+  if (newDuration < MIN_TWEEN_DURATION) return { kind: "noop" };
+  return {
+    kind: "resize",
+    position: round3(newStart),
+    duration: round3(newDuration),
+    pctRemap: [],
+  };
+}
+
 /**
  * Decide move vs resize for a dragged keyframe and, for resize, return the new
  * tween window + remapped keyframes.
@@ -76,20 +105,35 @@ export function resolveKeyframeRetime(opts: {
   if (tweenDuration <= 0) return { kind: "noop" };
   const tweenEnd = tweenStart + tweenDuration;
 
-  // Within the tween window → plain move (re-key the tween-%). This branch never
+  // A flat tween's synthesized diamonds are its real start/end boundaries —
-  // touches the keyframes array, so it still works for synthesized flat tweens.
+  // there is no authored keyframe node to re-key. Moving either boundary must
+  // therefore resize the tween window while keeping the opposite endpoint at
+  // its absolute time. Returning `resize` with an empty remap lets the caller
+  // update the flat tween's position/duration instead of sending a keyframe
+  // mutation that would silently no-op.
+  const flatBoundary = resolveFlatTweenBoundaryRetime({
+    keyframeCount: keyframes.length,
+    draggedTweenPct,
+    tweenStart,
+    tweenEnd,
+    dropAbsTime,
+  });
+  if (flatBoundary) return flatBoundary;
+
+  // Within the tween window → plain move (re-key the tween-%).
   if (dropAbsTime >= tweenStart - EPSILON_TIME && dropAbsTime <= tweenEnd + EPSILON_TIME) {
     const toTweenPct = clamp(((dropAbsTime - tweenStart) / tweenDuration) * 100, 0, 100);
     if (Math.abs(toTweenPct - draggedTweenPct) < NOOP_EPSILON_PCT) return { kind: "noop" };
     return { kind: "move", toTweenPct };
   }
 
-  // Boundary resize needs the real keyframes to remap; a flat tween has none here.
+  // Boundary resize needs the real keyframes to remap. Flat start/end boundaries
+  // were handled above.
   if (keyframes.length === 0) return { kind: "noop" };
 
   const newStart = Math.min(dropAbsTime, tweenStart);
   const newEnd = Math.max(dropAbsTime, tweenEnd);
-  const newDuration = Math.max(0.01, newEnd - newStart);
+  const newDuration = Math.max(MIN_TWEEN_DURATION, newEnd - newStart);
 
   // The dragged keyframe is the one whose tween-% is closest to draggedTweenPct.
   let draggedIdx = 0;

packages/studio/src/components/nle/useTimelineEditCallbacks.test.ts

@@ -0,0 +1,85 @@
+import { describe, expect, it } from "vitest";
+import { resolveTimelineKeyframeTarget } from "./useTimelineEditCallbacks";
+
+const FLAT_ANIMATION = {
+  id: "#title-fromTo-2900",
+  propertyGroup: null,
+};
+
+describe("resolveTimelineKeyframeTarget", () => {
+  it("resolves a synthesized diamond when one mixed flat tween is selected", () => {
+    expect(
+      resolveTimelineKeyframeTarget(
+        8.75,
+        [{ percentage: 8.75, tweenPercentage: 100 }],
+        [FLAT_ANIMATION],
+      ),
+    ).toEqual({ animId: "#title-fromTo-2900", tweenPct: 100 });
+  });
+
+  it("keeps multiple ungrouped flat tweens unresolved instead of guessing", () => {
+    expect(
+      resolveTimelineKeyframeTarget(
+        8.75,
+        [{ percentage: 8.75, tweenPercentage: 100 }],
+        [FLAT_ANIMATION, { id: "#title-to-2900", propertyGroup: null }],
+      ),
+    ).toBeNull();
+  });
+
+  it("keeps multiple flat tweens in the same property group unresolved", () => {
+    expect(
+      resolveTimelineKeyframeTarget(
+        50,
+        [{ percentage: 50, tweenPercentage: 100, propertyGroup: "position" }],
+        [
+          { id: "position-a", propertyGroup: "position" },
+          { id: "position-b", propertyGroup: "position" },
+        ],
+      ),
+    ).toBeNull();
+  });
+
+  it("keeps a keyframed and flat tween in the same property group unresolved", () => {
+    expect(
+      resolveTimelineKeyframeTarget(
+        50,
+        [{ percentage: 50, tweenPercentage: 25, propertyGroup: "position" }],
+        [
+          { id: "position-keyframed", propertyGroup: "position", keyframes: {} },
+          { id: "position-flat", propertyGroup: "position" },
+        ],
+      ),
+    ).toBeNull();
+  });
+
+  it("keeps multiple ungrouped keyframed tweens unresolved", () => {
+    expect(
+      resolveTimelineKeyframeTarget(
+        50,
+        [{ percentage: 50, tweenPercentage: 25 }],
+        [
+          { id: "ungrouped-a", keyframes: {} },
+          { id: "ungrouped-b", keyframes: {} },
+        ],
+      ),
+    ).toBeNull();
+  });
+
+  it("does not infer an animation when the rendered diamond misses the cache", () => {
+    expect(resolveTimelineKeyframeTarget(60, [], [FLAT_ANIMATION])).toBeNull();
+  });
+
+  it("resolves the sole candidate in an explicit property group", () => {
+    expect(
+      resolveTimelineKeyframeTarget(
+        50,
+        [{ percentage: 50, tweenPercentage: 25, propertyGroup: "position" }],
+        [
+          { id: "opacity", propertyGroup: "opacity" },
+          { id: "position", propertyGroup: "position" },
+        ],
+      ),
+    ).toEqual({ animId: "position", tweenPct: 25 });
+  });
+});

packages/studio/src/components/nle/useTimelineEditCallbacks.ts

@@ -36,6 +36,41 @@ export interface TimelineEditCallbackDeps {
   handleRazorSplitAll: (splitTime: number) => Promise<void> | void;
 }
 
+interface TimelineKeyframeTargetAnimation {
+  id: string;
+  propertyGroup?: string | null;
+  keyframes?: unknown;
+}
+
+interface TimelineCachedKeyframe {
+  percentage: number;
+  tweenPercentage?: number;
+  propertyGroup?: string;
+}
+
+/**
+ * Resolve a rendered timeline diamond back to the animation that authored it.
+ * Flat tweens use synthesized diamonds, so a mixed flat tween may have neither
+ * a property group nor real keyframes. The cache currently carries a property
+ * group, not an animation id, so resolution is safe only when that group has a
+ * single candidate. Ambiguous candidates remain unresolved rather than
+ * retiming an arbitrary tween.
+ */
+export function resolveTimelineKeyframeTarget(
+  pct: number,
+  keyframes: ReadonlyArray<TimelineCachedKeyframe>,
+  animations: ReadonlyArray<TimelineKeyframeTargetAnimation>,
+): { animId: string; tweenPct: number } | null {
+  const kf = keyframes.find((item) => Math.abs(item.percentage - pct) < 0.2);
+  if (!kf) return null;
+  const group = kf?.propertyGroup;
+  const candidates = group
+    ? animations.filter((animation) => animation.propertyGroup === group)
+    : animations.filter((animation) => !animation.propertyGroup);
+  const animation = candidates.length === 1 ? candidates[0] : undefined;
+  return animation ? { animId: animation.id, tweenPct: kf.tweenPercentage ?? pct } : null;
+}
+
 /**
  * Builds the timeline edit callback bag (move/resize/split/razor plus the
  * keyframe-diamond callbacks) provided to `<Timeline>` via TimelineEditProvider.
@@ -76,12 +111,7 @@ export function useTimelineEditCallbacks({
     // fallow-ignore-next-line complexity
     (pct: number): { animId: string; tweenPct: number } | null => {
       const cached = usePlayerStore.getState().keyframeCache.get(domEditSelection?.id ?? "");
-      const kf = cached?.keyframes.find((k) => Math.abs(k.percentage - pct) < 0.2);
+      return resolveTimelineKeyframeTarget(pct, cached?.keyframes ?? [], selectedGsapAnimations);
-      const group = kf?.propertyGroup;
-      const anim =
-        (group ? selectedGsapAnimations.find((a) => a.propertyGroup === group) : undefined) ??
-        selectedGsapAnimations.find((a) => a.keyframes);
-      return anim ? { animId: anim.id, tweenPct: kf?.tweenPercentage ?? pct } : null;
     },
     [domEditSelection?.id, selectedGsapAnimations],
   );
@@ -154,12 +184,22 @@ export function useTimelineEditCallbacks({
           decision.position != null &&
           decision.duration != null
         ) {
-          handleGsapResizeKeyframedTween(
+          if (anim.keyframes) {
-            target.animId,
+            handleGsapResizeKeyframedTween(
-            decision.position,
+              target.animId,
-            decision.duration,
+              decision.position,
-            decision.pctRemap,
+              decision.duration,
-          );
+              decision.pctRemap,
+            );
+          } else {
+            // resize-keyframed-tween requires an authored `keyframes` AST node
+            // and intentionally no-ops for a flat tween. Update its real tween
+            // window through the metadata writer (and SDK cutover path) instead.
+            handleGsapUpdateMeta(target.animId, {
+              position: decision.position,
+              duration: decision.duration,
+            });
+          }
         }
       },
       onChangeKeyframeEase: (_elId: string, _pct: number, ease: string) => {