feat(studio): add timeline property lanes

[miguel-heygen]

What

Adds expandable per-property keyframe lanes and segment easing controls.

Why

A clip-level summary alone cannot expose which property owns a keyframe or which segment ease is being edited.

How

Renders property-specific diamonds, connecting segments, accessible ease controls, and navigation affordances from the canonical lane model. This is B4 of the Family B draft Graphite stack.

Test plan

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

Exact Family B tip validation: 2,865 Studio tests, 67 Studio Server route tests, both package typechecks, formatting, lint, diff check, file-size gate, and Fallow audit passed.

Deferred review findings

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

• 🟡 packages/studio/src/player/components/useAutoExpandKeyframedClips.ts:14 — "Later user collapse sticks and never bounces back open" only holds within one hook-component lifetime
• 🟡 packages/studio/src/player/components/TimelinePropertyLanes.tsx:141 — globalEase fallback for a group is taken from animations[0] regardless of which animation a keyframe belongs to
• 🟢 packages/studio/src/player/components/useAutoExpandKeyframedClips.ts:20 — Project switch that reuses the previous project's Map identity silently leaves new project fully collapsed
• 🟢 packages/studio/src/player/components/TimelinePropertyLanes.tsx:41 — synthesizeFlatTweenKeyframes runs twice per flat-tween animation per render

---

Supersedes #2686, 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

Approved with no high findings. The 5 low/informational findings are parked in .scratch/studio-timeline-family-b/issues/09-family-b-v2-r1-deferred.md.

[miguel-heygen]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• feat(studio): wire expanded keyframe timeline lanes #2791
• fix(studio): harden keyframe editing semantics #2787
• feat(studio): consolidate timeline editor callbacks #2786
• feat(studio): add keyframe track headers #2785
• feat(studio): add timeline property lanes #2784 👈 (View in Graphite)
• feat(studio): add timeline keyframe retiming interactions #2783
• feat(studio): add variable timeline timing and layout #2782
• feat(studio): add keyframe timeline state #2781
• main

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

[vanceingalls]

Verdict: APPROVE — additive surface (+712/-1) integrates cleanly, tests are thorough (13 rendering assertions + a per-project auto-expand scenario), and every P2/P3 candidate I found is either pre-declared in the body or deferred by design to later stack PRs.

Adversarial pass at PR head 6bc8d545 against base codex/studio-timeline-b-keyframe-retiming-v2. Prior reviews: 0. Prior line comments: 0.

Adversarial lenses

• Lens A — Property-lane taxonomy. Groups derive from classifyPropertyGroup + PropertyGroupName (core-defined, closed union) — no adapter injection surface. Lane order comes from Array.from(sourceGroups(...)) where sourceGroups is a Map<PropertyGroupName, GsapAnimation[]> filled by iteration order of the input animations array (TimelinePropertyLanes.tsx:41-49). i.e. display order = caller-supplied animation order. Test returns both flat and authored keyframe property groups (test.tsx:143) asserts ["position", "visual"] by inputting them in that order — the assertion is tautological. P3 (deferred): whichever PR wires the consumer must decide whether to sort by canonical group order or leave to upstream. Not blocking here — no consumer exists yet in this PR.
• Lens B — Empty-state UX. TimelinePropertyLanes.tsx:108,111 return null for narrow clip and no lanes; there is no click-to-add affordance in the lane track region. Adding keyframes happens elsewhere (segment button flow through TimelineDiamondLane), and un-keyframed clips remain collapsed via useAutoExpandKeyframedClips — so an "empty lane region" is unreachable by design. Intentional, no action.
• Lens C — Lane height & density. Fixed LANE_H per lane, no compact toggle, no virtualization. Realistic upper bound is the count of distinct PropertyGroupName values (~6-8), so DOM churn is bounded. P3 (informational): no ceiling enforced, but the closed taxonomy makes explosion unlikely.
• Lens D — Additive claim veracity. Verified. The -1 is TimelineClipDiamonds.tsx:15 interface TimelineDiamondKeyframe → export interface TimelineDiamondKeyframe — pure visibility widening, no functional delta. Neither TimelinePropertyLanes nor useAutoExpandKeyframedClips is invoked by any file in this PR — this is stage code that later stack PRs wire in. No existing render path branches on new state; no consumer is accidentally opted-in.
• Lens E — Stack cohesion with #2785 (track headers). Clean shape: each lane div carries data-property-group={group}, data-timeline-property-lane="", data-timeline-lane-top={getTimelineLaneTop(laneIndex)} (TimelinePropertyLanes.tsx:117-120). No orphaned header slot, no half-placeholder. #2785 has a clear DOM keying surface to attach to.

Findings

PR body's 4 pre-declared deferred findings — verified real, correctly severity-marked:

• 🟡 useAutoExpandKeyframedClips.ts:14 — collapse-sticky invariant is per hook-component lifetime (seen is a useRef). Confirmed — unmount-remount loses the memory. Deferred .scratch/…/04-…md. OK.
• 🟡 TimelinePropertyLanes.tsx:141 — globalEase fallback = groupAnimations[0]?.keyframes?.easeEach ?? groupAnimations[0]?.ease ?? "none", ignoring the actual animation each keyframe belongs to. Confirmed — when a single group has multiple animations, easeEach from animation index >0 is dropped. Deferred. OK.
• 🟢 useAutoExpandKeyframedClips.ts:20 — project switch that reuses previous project's Map identity silently skips expansion (sourceChanged guard, line 21). Confirmed. Deferred. OK.
• 🟢 TimelinePropertyLanes.tsx:41 — synthesizeFlatTweenKeyframes runs twice per flat-tween per render (once inside animationContributesLane in sourceGroups, again inside groupKeyframes). Confirmed via code path trace. Deferred. OK.

Additional adversarial observations (all P3 / informational, none blocking):

• P3 — Auto-expand useEffect is state-syncing. useAutoExpandKeyframedClips.ts:19-34 writes to usePlayerStore from an effect. Not a violation of the "no useEffect for state syncing" rule as strictly written (Zustand store is an external system, not local React state), and it can't cleanly become useMemo because it's a side-effect. Flag only if the rule is being read maximally.
• P3 — getTimelineLaneTop defensive clamp (timelineLayout.ts:12-14) — Math.max(0, Math.trunc(laneIndex)) silently normalizes negatives/fractionals. Fine as-is; worth a doc line if callers ever pass indices from a filtered array.
• P3 — data-timeline-lane-top duplicates style.top (TimelinePropertyLanes.tsx:118-121). Presumably intentional for test/consumer readback of the resolved integer without parsing a CSS pixel string. OK, but if #2785 doesn't consume the attribute, it can go.

Signal for the stack

• PR body/message hygiene is strong — the "Supersedes #2686" note plus deferred-findings file makes downstream review much cheaper.
• TimelineDiamondLane already existed on the head branch (from an earlier stack PR); this PR wraps it with groupAware per lane. Interface is stable.
• The exported TimelineDiamondKeyframe interface is now module-public — trivially small blast radius today (2 importers, both in-PR), but a future ABI change becomes a cross-file ripple.

Ship it. My additions above are pre-emptive notes for the consumer PR (#2785+), not this one.

— Via

[james-russo-rames-d-jusso]

Reviewed at 6bc8d545accf.

R2 adversarial pass on the expandable per-property lanes slice. Two orange items on the render path: globalEase = groupAnimations[0]?.keyframes?.easeEach ?? ... misrepresents ease for sibling animations in the same group (segments show the wrong curve while clicks still route to the correct animationId — displayed and edited ease diverge); and a fresh keyframesData={{ format, keyframes }} object literal per render + un-memoized getTimelinePropertyLanes recomputation defeat TimelineDiamondLane's React.memo — every playhead tick re-renders every diamond in every expanded lane.

🟢 Verified clean

• TimelineClipDiamonds.tsx: only exports TimelineDiamondKeyframe — no behavior change
• timelineLayout.ts: getTimelineLaneTop is pure additive helper with defensive Math.max/Math.trunc guarding
• TimelinePropertyLanes.test.tsx: uses act() correctly, cleans DOM in afterEach; keyframe target payload assertion covers group-aware selection identity
• useAutoExpandKeyframedClips.test.tsx: correctly resets playerStore + shell mock between cases and covers sticky-collapse within a project

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

[james-russo-rames-d-jusso]

🟠 globalEase from groupAnimations[0] misrepresents ease for other animations in the same group

globalEase={
  groupAnimations[0]?.keyframes?.easeEach ?? groupAnimations[0]?.ease ?? "none"
}

A property group (e.g. 'position') can aggregate multiple GsapAnimations (sourceGroups pushes any tween whose propertyGroup matches). TimelineDiamondLane's segment-ease renderer uses const ease = kf.ease ?? globalEase (TimelineClipDiamonds.tsx:234). For synthesized/flat tweens, kf.ease is undefined (groupKeyframes only spreads ...keyframe, never injects animation-level ease), so every segment in the lane falls back to the SAME globalEase from the FIRST animation. Segments belonging to sibling animations with different ease/easeEach display the wrong curve on MiniCurveSvg, and the aria-label/title (Edit ${ease} easing) lies — while the click still routes to the correct animationId via keyframeTarget, so displayed ease and edited ease diverge.

Fix: In groupKeyframes, populate ease from the containing animation when the keyframe lacks one: ease: keyframe.ease ?? animation.keyframes?.easeEach ?? animation.ease. Then drop the shared globalEase prop (or keep it strictly as a last-resort 'none' fallback).

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟠 Inline keyframesData object literal defeats TimelineDiamondLane memoization

keyframesData={{ format: "percentage", keyframes }}

TimelineDiamondLane is wrapped in React.memo (TimelineClipDiamonds.tsx:114). Passing a fresh { format, keyframes } object literal on every render — plus a freshly-constructed keyframes array from getTimelinePropertyLanes(...) recomputed unconditionally on line 120 — guarantees the memo comparator fails every render, so every diamond in every expanded property lane re-renders on each playhead tick (currentPercentage changes at animation frame rate).

Fix: Wrap getTimelinePropertyLanes(animations, clipStart, clipDuration) in useMemo keyed on [animations, clipStart, clipDuration], and derive/cache keyframesData inside the same memo so reference is stable across playhead ticks.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟡 Property lane container carries no ARIA identity (data- only)*

{lanes.map(({ group, animations: groupAnimations, keyframes }, laneIndex) => (
  <div
    key={group}
    data-property-group={group}
    data-timeline-property-lane=""
    data-timeline-lane-top={getTimelineLaneTop(laneIndex)}

Lane wrapper only carries data-* attributes — not ARIA — so AT has no way to distinguish a Position lane from a Visual lane. Individual ease-button has aria-label={Edit ${ease} easing} but lane grouping is opaque.

Fix: Add role="group" and aria-label={${group} keyframes} (or aria-labelledby pointing at a visually-hidden label) to the lane container.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟠 getTimelinePropertyLanes re-runs (with nested synthesizeFlatTweenKeyframes calls) every render

if (clipWidthPx < 20 || clipDuration <= 0) return null;
const lanes = getTimelinePropertyLanes(animations, clipStart, clipDuration);

getTimelinePropertyLanes → sourceGroups → animationContributesLane calls animationKeyframes (which calls synthesizeFlatTweenKeyframes) once per animation, and groupKeyframes calls animationKeyframes AGAIN inside its loop — so every animation without native keyframes is synthesized twice per render. No useMemo, so pipeline runs on every parent re-render including 60Hz currentPercentage update. Compounds finding #2.

Fix: Memoize lanes on [animations, clipStart, clipDuration]. In animationContributesLane, compute and cache synthesized keyframes once (return array, reuse in groupKeyframes) instead of re-synthesizing.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟡 useAutoExpandKeyframedClips never shrinks seen.clips — deleted-then-restored clip won't auto-expand again

const fresh: string[] = [];
for (const [key, animations] of gsapAnimations) {
  if (seen.current.clips.has(key)) continue;
  if (animations.some(animationContributesLane)) fresh.push(key);
}
if (fresh.length === 0) return;
for (const key of fresh) seen.current.clips.add(key);

Within a project session seen.current.clips is append-only. If a keyframed clip is removed from gsapAnimations (delete, cut, tween loses its last keyframe) and then reappears with the same clipId (undo/redo, paste with preserved id), the effect finds it in seen.clips and skips auto-expanding it — even though from user's perspective it's a 'new' keyframed clip. Docstring says 'clips added later still auto-expand'; this bug contradicts for id-preserving reinsertions.

Fix: Prune keys from seen.current.clips no longer present in gsapAnimations before the fresh-clip scan (intersect seen.clips with current key set) so a clipId that fully disappeared and reappeared is treated as fresh again.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟢 Project switch drops sticky-collapse state on return

if (seen.current.projectId !== projectId) {
  const sourceChanged = seen.current.source !== gsapAnimations;
  seen.current = { projectId, source: gsapAnimations, clips: new Set() };
  if (!sourceChanged) return;
} else {
  seen.current.source = gsapAnimations;
}

seen.current.clips scoped to current project only — switching projectId nukes it. User who collapsed a keyframed clip in Project A, briefly opened Project B, and switched back finds every keyframed clip auto-expanded again — per-clip collapse decisions did not persist. Doc claims 'per-clip tracking so a later user collapse sticks and never bounces back open' — that's per-project, not across round-trip.

Fix: Either persist collapse decisions across projectId changes (keyed globally by clipId, not by project) or document explicitly that collapse only sticks for duration of a project session.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟢 Redundant clipWidthPx < 20 gate at the wrapper

if (clipWidthPx < 20 || clipDuration <= 0) return null;

TimelineDiamondLane already returns null when clipWidthPx < 20 (TimelineClipDiamonds.tsx:175). Two sources of truth for 'too narrow to render' threshold — future changes silently miss the wrapper's copy, and any 20px-boundary bugs must be fixed in both.

Fix: Drop the width check here (let TimelineDiamondLane own the too-narrow policy), or extract threshold into a shared constant in timelineLayout.ts.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟢 tweenDuration silently falls back to clipDuration when animation.duration undefined

const tweenDuration = animation.duration ?? clipDuration;

Tween with undefined duration is silently treated as spanning entire clip when computing absolute keyframe times. A tween with real keyframes always has a duration in practice — hitting this fallback likely means an upstream parser bug, and masking it as 'span the whole clip' places keyframes at arbitrary positions instead of surfacing missing data.

Fix: Skip animations with undefined duration in groupKeyframes (or sourceGroups) rather than defaulting to clipDuration, and log/report missing duration so upstream parser can be fixed.

— Review by Rames D Jusso