fix(studio): harden keyframe drag commit (review follow-ups)

- pickKeyframeTween no longer falls back to ALL animations on a selector
  mismatch — it only picks among the dragged element's own tweens, so a
  class/compound-selector mismatch can't edit a different element. No
  match → no-op.
- computeKeyframeMovePlan bails to a no-op when a keyframe-array tween's
  dragged keyframe can't be located (stale cache / precision drift) instead
  of falling through to an end-point resize that silently rescaled the
  whole tween and re-timed every keyframe.
- usePopulateKeyframeCacheForFile clipPct now uses 0.001% precision
  (matching useGsapAnimationsForElement) so beat-snapped keyframes from the
  file-wide cache also center on the dot and the two caches agree.
- The optimistic drag hold only releases once the cache reflects the
  committed position (a keyframe near the held %), so an unrelated cache
  rebuild no longer flashes the diamond back to its old spot.
- A drag's document listeners are cleaned up on unmount, so an unmount
  mid-drag (clip delete / comp switch / zoom-out) no longer leaks them.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Co-authored-by: Miguel Ángel <miguel07alm@protonmail.com>

Author: vanceingalls

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

@@ -33,6 +33,11 @@ describe("pickKeyframeTween", () => {
   it("returns undefined when there are no tweens", () => {
     expect(pickKeyframeTween([], el, 1, undefined)).toBeUndefined();
   });
+
+  it("returns undefined rather than editing another element on a selector mismatch", () => {
+    const anims = [flat("a", "#other", 0, 5), flat("b", ".unrelated", 2, 3)];
+    expect(pickKeyframeTween(anims, el, 3, undefined)).toBeUndefined();
+  });
 });
 
 describe("computeKeyframeMovePlan — flat tween", () => {
@@ -85,4 +90,12 @@ describe("computeKeyframeMovePlan — keyframe-array tween", () => {
     const mid = plan.adds.find((a) => a.properties.x === 50);
     expect(mid?.pct).toBeCloseTo(37.5, 1);
   });
+
+  it("is a no-op when the dragged keyframe can't be located (stale cache)", () => {
+    // tweenOldPct 33 matches no keyframe (0/50/100) → must NOT resize the tween.
+    const plan = computeKeyframeMovePlan(anim, 33, el, 70);
+    expect(plan.meta).toBeUndefined();
+    expect(plan.removes).toEqual([]);
+    expect(plan.adds).toEqual([]);
+  });
 });

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

@@ -85,10 +85,12 @@ export function pickKeyframeTween<T extends TweenLike>(
 ): T | undefined {
   const selectors = [el.domId ? `#${el.domId}` : null, el.selector].filter(Boolean);
   const forEl = anims.filter((a) => selectors.includes(a.targetSelector));
-  const pool = forEl.length > 0 ? forEl : anims;
-  const groupPool = group ? pool.filter((a) => a.propertyGroup === group) : [];
-  const candidates = groupPool.length > 0 ? groupPool : pool;
-  if (candidates.length === 0) return undefined;
+  // Only ever pick among THIS element's tweens. Don't fall back to all
+  // animations — a selector mismatch (e.g. a class/compound-selector tween)
+  // would otherwise edit a different element's keyframes. No match → no-op.
+  if (forEl.length === 0) return undefined;
+  const groupPool = group ? forEl.filter((a) => a.propertyGroup === group) : [];
+  const candidates = groupPool.length > 0 ? groupPool : forEl;
   const dist = (a: T): number => {
     const { start, dur } = tweenWindow(a);
     if (origAbsTime >= start && origAbsTime <= start + dur) return 0;
@@ -119,6 +121,11 @@ export function computeKeyframeMovePlan(
     : null;
   const idx = kfs ? kfs.findIndex((k) => Math.abs(k.percentage - tweenOldPct) < 0.5) : -1;
 
+  // Keyframe-array tween but the dragged keyframe couldn't be located (stale
+  // cache / precision drift): no-op rather than falling through to an end-point
+  // resize that would silently rescale the whole tween and re-time every key.
+  if (kfs && idx === -1) return { removes: [], adds: [] };
+
   if (kfs && idx > 0 && idx < kfs.length - 1) {
     const movedPct = tweenDur > 0 ? clampPct(((newAbsTime - tweenStart) / tweenDur) * 100) : 0;
     return { removes: [tweenOldPct], adds: [{ pct: movedPct, properties: kfs[idx]!.properties }] };

packages/studio/src/hooks/useGsapTweenCache.ts

@@ -383,9 +383,12 @@ export function usePopulateKeyframeCacheForFile(
         const elDuration = timelineEl?.duration ?? 1;
         const clipKeyframes = kfData.keyframes.map((kf) => {
           const absTime = toAbsoluteTime(tweenPos, tweenDur, kf.percentage);
+          // 0.001% precision (matching useGsapAnimationsForElement above) so a
+          // beat-snapped keyframe centers exactly on the beat dot and the two
+          // caches agree on a keyframe's percentage.
           const clipPct =
             elDuration > 0
-              ? Math.round(((absTime - elStart) / elDuration) * 1000) / 10
+              ? Math.round(((absTime - elStart) / elDuration) * 100000) / 1000
               : kf.percentage;
           return {
             ...kf,

packages/studio/src/player/components/TimelineClipDiamonds.tsx

@@ -66,16 +66,35 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({
   // until the cache reflects the change (the file round-trip rewrites
   // keyframesData), so it doesn't flash back to the old spot in between.
   const pendingRef = useRef(false);
+  const pendingHeldPctRef = useRef<number | null>(null);
   const pendingTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+  // Cleanup for an in-flight drag's document listeners, so an unmount mid-drag
+  // (clip deleted, comp switch, zoom-out → early return) doesn't leak them.
+  const dragCleanupRef = useRef<(() => void) | null>(null);
 
   useEffect(() => {
     if (!pendingRef.current) return;
+    // Only release the optimistic hold once the cache actually reflects the
+    // committed position (a keyframe near the held %). An unrelated cache
+    // rebuild (e.g. elementCount change) rebuilds keyframesData with the SAME
+    // percentages — releasing then would flash the diamond back to the old spot.
+    const held = pendingHeldPctRef.current;
+    if (held != null && !keyframesData.keyframes.some((k) => Math.abs(k.percentage - held) < 0.3)) {
+      return;
+    }
     pendingRef.current = false;
+    pendingHeldPctRef.current = null;
     if (pendingTimerRef.current) clearTimeout(pendingTimerRef.current);
     setDrag(null);
   }, [keyframesData]);
 
-  useEffect(() => () => clearTimeout(pendingTimerRef.current ?? undefined), []);
+  useEffect(
+    () => () => {
+      clearTimeout(pendingTimerRef.current ?? undefined);
+      dragCleanupRef.current?.();
+    },
+    [],
+  );
 
   if (clipWidthPx < 20) return null;
 
@@ -119,17 +138,20 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({
     const handleUp = () => {
       document.removeEventListener("pointermove", handleMove);
       document.removeEventListener("pointerup", handleUp);
+      dragCleanupRef.current = null;
       const d = dragRef.current;
       dragRef.current = null;
       const willCommit = !!(d && d.moved && Math.abs(d.pct - d.origPct) > 0.5);
       if (willCommit && d) {
         // Hold the dropped position optimistically; the effect clears it once the
         // cache round-trip lands (fallback timeout in case it never does).
         pendingRef.current = true;
+        pendingHeldPctRef.current = d.pct;
         setDrag({ origPct: d.origPct, pct: d.pct });
         if (pendingTimerRef.current) clearTimeout(pendingTimerRef.current);
         pendingTimerRef.current = setTimeout(() => {
           pendingRef.current = false;
+          pendingHeldPctRef.current = null;
           setDrag(null);
         }, 2000);
         onDragKeyframeRef.current?.(d.origPct, d.pct);
@@ -138,6 +160,11 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({
       }
     };
 
+    dragCleanupRef.current = () => {
+      document.removeEventListener("pointermove", handleMove);
+      document.removeEventListener("pointerup", handleUp);
+    };
+
     document.addEventListener("pointermove", handleMove);
     document.addEventListener("pointerup", handleUp);
   };