feat(studio): expand sub-composition groups + children in the timeline

[miguel-heygen]

Sub-composition timeline expansion (4/8)

Expand sub-composition groups and their children in the studio timeline, with correct keyframe rendering and clip expansion for sub-composition timeline clips.

Stacked on the groups UI.

[miguel-heygen]

• chore(studio): add anonymous usage events #1765
• fix(studio): strip a group's GSAP when ungrouping #1764
• fix(studio): remove keyframe dragging from the timeline #1763
• fix(studio): batch 3D depth commit so perspective + z don't race #1762
• feat(studio): expand sub-composition groups + children in the timeline #1761 👈 (View in Graphite)
• feat(gsap): read timelines authored inline (acorn read path) #1760
• feat(studio): element groups — studio UI #1759
• feat(studio): element groups — source mutations #1758
• main

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

[james-russo-rames-d-jusso]

Reviewing 362195cb as part of the 8-PR groups stack — read alongside foundation #1758/#1759 and sibling #1764. Cross-PR shape with foundation is coherent (data-hf-group wrapper id added in #1764 to make it a first-class clip-manifest node — used here by useTimelineSyncCallbacks's collect to enumerate group descendants). No drift from foundation.

🟠 Concerns

useExpandedTimelineElements.ts:154-160 — mixed manifest/DOM-only siblings: DOM-only ones are silently dropped. The fallback is all-or-nothing:

const fromManifest = manifest.filter(...);
if (fromManifest.length > 0) return fromManifest;
return domSiblingClips(...);

If a sub-comp host contains some children with data-start (manifest clips) and some without (DOM-only pills / freshly inlined group bodies), the manifest branch wins and the DOM-only ones never appear in the expanded timeline row. The new test at useExpandedTimelineElements.test.ts:128-167 only covers the all-DOM-only case. Realistic mixed case: a sub-comp with one keyframed avatar (manifest) + a few static pills (DOM-only) — pills vanish from the timeline expand even though they're authored. Suggest merging the two arrays (manifest takes precedence on id collision) rather than picking one.

useTimelineSyncCallbacks.ts:115-132 — collect recursion has no depth bound and silently overwrites parentMap on duplicate ids. Two failure modes:

1. Unbounded recursion via if (isGroup) collect(child, child.id). The comment on 113-114 says "one level into groups for drill-in" but the code recurses through every nested group. Realistic? Probably not common, but a deeply group-nested sub-comp (group-in-group-in-group via successive ⌘G presses, since #1759 wires that up) pushes the JS stack. Bound it to a fixed depth (3-5) with a // fallow-ignore-next-line if intentional, or update the comment to "all nested groups."
2. Same id appearing under multiple sub-comp hosts. If the same sub-comp is inlined twice on the same scene (two <div data-composition-id="hero">…</div> instances), the iframe DOM has duplicate ids. iframeDoc.getElementById(clip.id) returns the first match only (existing behavior), but the second collect invocation still walks the second host's DOM (because the outer for (const clip of data.clips) iterates both) and the second pass overwrites parentMap.set(child.id, parentId) — the second instance's children get attributed to the first host. The clip manifest dedups by id, but the DOM doesn't. Probably a pre-existing limitation surfaced more visibly by the new code path; worth a one-line comment acknowledging.

useGsapTweenCache.ts:188-205 — resolveClipTimingBasis host fallback hardcodes index.html#.... The host lookup falls back to:

elements.find((el) => el.domId === hostId || (el.key ?? el.id) === `index.html#${hostId}`)

If a sub-comp host lives in a non-index.html source (nested sub-comp authored in scene.html, etc.), the key form index.html#${hostId} won't match and the host falls back to { start: 0, duration: 1 }. The domId === hostId branch will still match for the typical 1-level case, but for deeper nesting this silently regresses to elDuration=1 — the exact bug the docstring at 178-183 says we're fixing. Suggest either omitting the key branch (rely on domId only) or threading the actual sourceFile of the host through.

🟡 Questions

useGsapTweenCache.ts:228-230, 486-488 — domClipChildrenKey selector allocates per call. s.domClipChildren.map(c => \${c.id}<${c.hostId}`).join("|")runs on every selector invocation; Zustand usesObject.isso string-equality elides re-renders, but the join cost is paid on every state read. For sub-comp expands with hundreds of pills this might show in perf flamegraphs. Could the store derive + memoize adomClipChildrenKeyslice once atsetDomClipChildren` time?

useExpandedTimelineElements.ts:154-160 — expand/collapse persistence model. The expansion is implicitly driven by selectedElementId (lines 195-213). Reload, undo/redo, selection cleared → expansion collapses. That looks intentional (sub-comp expand follows the selection drill-in introduced in #1759), but if product wants the expanded state to survive a page reload, the store flush at 399-403 wipes domClipChildren too — confirm that's the intended UX.

🟢 Nits

useTimelineSyncCallbacks.ts:121, 126 — group label fallback to child.id defeats the slugging effort in #1764. When the wrapper has no data-hf-group value (legacy wrappers, or stripped-by-mistake), the label silently becomes the raw id slug (e.g. group-1 instead of Group 1). That's the right default but worth a comment so a future reader doesn't "fix" it.

useTimelineSyncCallbacks.ts:111 — innerRoot fallback to hostEl when [data-hf-inner-root] is absent. Older sub-comps that predate the data-hf-inner-root attribute (compiled before #1758-ish) would have collect iterate hostEl.children directly. That probably works, but consider asserting it in a test fixture so the dual-path isn't silently broken later.

What I didn't verify

• Live drill-in behavior in the studio (no preview env).
• That the domSiblingClips synthesized clip's track: host.track placement actually renders cleanly in TimelineCanvas (the synthesized clips share the host's track index; if TimelineCanvas assumes one element per track, multiple pills on the same host track might overlap).
• The domClipChildrenKey selector identity stability under Zustand's useSyncExternalStore semantics.
• Perf of repeated collect walks on every preview reload — the sync runs on every onPlayerState callback; if that fires per frame, the DOM walk is per-frame.

— Rames D Jusso

[vanceingalls]

Review of #1761 — feat(studio): expand sub-composition groups + children in the timeline

HEAD: 362195cb · Base: eg-03-inline-timeline (#1760) · Files: 5 · Diff: +207/-22
CI: all required green (player-perf, regression, preview-regression, Preview parity, WIP). Graphite mergeability still in progress at read time.
Prior reviewers: Rames COMMENTED at 03:23 UTC — convergent on the manifest-vs-DOM-only-children and string-join perf concerns I raise below (kept his framing for completeness since he posted first). He adds two angles I missed: unbounded collect recursion + same-id-under-multiple-sub-comp-hosts attribution (useTimelineSyncCallbacks.ts:115-132), and a hardcoded index.html#${hostId} fallback that silently regresses for non-index.html-authored sub-comps (useGsapTweenCache.ts:188-205). Both worth addressing. I won't re-raise.

Verdict: approve with comments — solid, well-tested incremental layer on top of the existing expand-sub-comp work. Test coverage for the new DOM-only fallback path is present. No blocking issues from the runtime-interop / player-semantics lane. Three follow-ups worth flagging (one distinct from Rames; two convergent), none merge-blocking.

---

Findings

💭 useTimelineSyncCallbacks.ts:83-139 — widened try surface silently keeps stale clipParentMap / domClipChildren on iframe failure

The pre-PR code only ran setClipParentMap(parentMap) inside the if (clipTree) block of a narrow try. The new code:

• Hoists parentMap above the if (clipTree) (good — needed so the DOM-children walk can extend it).
• Wraps the entire clipTree walk + DOM walk + both setters in one try.
• The catch comment claims "maps stay empty", but if the throw happens between the two getState().set… calls (or anywhere in the DOM walk for the second-or-later composition load), the store keeps the previous composition's clipParentMap and domClipChildren. With setDomClipChildren not memoised on reference, that means stale references survive an iframe failure mid-recompose.

Real-world reachability is low (DOM ops on a same-origin iframe rarely throw mid-walk), and the symptom — stale expand-tree showing yesterday's pills — is recoverable on the next successful sync. Flag for awareness; consider resetting both setters in the catch, or moving the two setState calls outside the try after assembling the locals.

💭 useExpandedTimelineElements.ts:154-160 — manifest-vs-DOM children is exclusive, not additive

The fallback is if (fromManifest.length > 0) return fromManifest; else return domSiblingClips(...). If a sub-comp host has both timed manifest children (e.g. an authored <div data-start="0" data-duration="3">) and DOM-only group/pill children, the DOM-only ones are silently dropped from the expanded view. Probably matches today's authoring convention (a sub-comp is either timed-children or group/pill, not both), but if the convention slips the user sees a partial expansion with no warning. Either:

• Merge: [...fromManifest, ...domSiblingClips(...)] (with id-dedup), or
• Document the convention in the comment so a future reader knows the fallthrough is intentional.

The PR description / future docs should make the "sub-comp internals are all-or-nothing" assumption explicit.

💭 useGsapTweenCache.ts:228-230, 486-488 — string-join store selector recomputes on every Zustand set

const domClipChildrenKey = usePlayerStore((s) =>
  s.domClipChildren.map((c) => `${c.id}<${c.hostId}`).join("|"),
);

Zustand's default equality is referential === on the selector return, so every store set (drag, select, beat edit, anything) re-runs this selector. The string IS stable for stable content, so React bails the re-render — but the O(n) join happens on every set. For deep scenes with many sub-comp descendants it's wasted CPU per click. Two cheaper shapes:

• useShallow (zustand/shallow) over the raw array, or
• A pre-computed key cached at setDomClipChildren time and a plain (s) => s.domClipChildrenKey selector.

Not a regression vs. the rest of the codebase (the elementCount selector is constant-time, but other selectors elsewhere do similar work) and not a blocker — flag as a follow-up if perf tooling flags it.

---

My-lane angles checked & cleared

1. Player-store state shape (playerStore.ts:174-175, 313-314, 402) — new field added with sensible default [], setter wires through set, reset clears it. DomClipChild interface is exported and consumed via type-only imports. ✔
2. Timeline-sync semantics — parent→child time mapping (useExpandedTimelineElements.ts:118-138) — domSiblingClips spans children across the full host bounds (host.start, host.duration), then buildChildElements clamps to the display bounds. expandedParentStart carries the host start, so local-time edits resolve correctly. Test expands DOM-only sub-comp children asserts the math. ✔
3. Sub-composition timing isolation — no window.__timelines[childId] writes from this PR. The studio side stays observer-only (reads clip.id, getElementById, children, never assigns to runtime state). ✔
4. GSAP tween cache invalidation (useGsapTweenCache.ts:226-230, 460, 486-488, 567) — both useGsapAnimationsForElement and usePopulateKeyframeCacheForFile re-key on domClipChildrenKey so when DOM children appear after the initial cache populate, the clip-relative percentages get recomputed against the host bounds. lastFetchKeyRef guard prevents redundant fetches. resolveClipTimingBasis correctly falls back to index.html#<hostId> for cross-file host lookups. ✔
5. Sub-comp host element-visibility contract — this PR does not change init.ts:480-482 semantics. Expanded children are studio-side timeline display only; nothing flows back into runtime visibility. ✔
6. Mid-flight render race — no new window.__player.renderSeek calls; processTimelineMessage flows are unchanged in their RAF/playhead behavior. The new state writes are on the same code path as existing setClipParentMap, which already coexisted with the producer's seek loop. ✔
7. Decorative gate pattern — the kind !== "composition" || !clip.id guard on the for-loop populates domClipChildren correctly for every sub-comp host; no read path checks for entries the populate skipped. ✔
8. Cross-PR with #1763 (drag stripping) — synthesised DOM-only clips reuse createTimelineElementFromManifestClip; whatever drag treatment #1763 applies to manifest clips will apply to these too. No new render path that bypasses the strip. ✔

---

Test coverage

• New unit expands DOM-only sub-comp children (no manifest clip) under the host (useExpandedTimelineElements.test.ts:128-167) asserts the host-relative span, file rebase, and host-row replacement. Good behavioural coverage of the new domSiblingClips branch.
• No new tests for the useTimelineSyncCallbacks DOM-walk path (the iframe DOM walk that produces domClipChildren). Acceptable — that code is a pure DOM enumeration with one observable side effect (the setDomClipChildren call) that the existing integration coverage exercises indirectly.

---

Review by Via

[vanceingalls]

R2 of #1761 — feat(studio): expand sub-composition groups + children in the timeline

Old HEAD: 362195cb · New HEAD: f27e81ee · Delta commit: f27e81ee fix(studio): correct keyframes + expansion for sub-composition timeline clips
Delta size: +137/-22 across 4 files (PR total: +207/-22, 5 files unchanged from R1)
CI: all required green (player-perf, regression, preview-regression, Preview parity, Preflight, WIP). Graphite mergeability_check pending — non-blocking.
Prior reviewer state at this SHA: Via R1 4583938061 COMMENTED @ 362195cb; Rames R1 4583912234 COMMENTED @ 362195cb. No R2 from Rames yet at f27e81ee.

Verdict: approve with comments. Delta is a focused follow-up that introduces resolveClipTimingBasis to fix the elDuration=1 / keyframe-overflow bug for sub-comp internals (pills inside scenes), wires a new domClipChildrenKey dep into both keyframe-cache effects so the cache re-populates once DOM children arrive, and rewrites the useTimelineSyncCallbacks group-walk as a recursive collect that handles id-less structural wrappers + ungrouped descendants + nested groups. Substantive fix, well-scoped. Most prior nits remain open but are still nits.

Per-finding status (Via R1)

1. try-block setState ordering (useTimelineSyncCallbacks.ts:135-139) — 💭 still open. setClipParentMap + setDomClipChildren are still inside the try; catch is a silent swallow with comment "maps stay empty" — no reset on partial-population. Risk unchanged: a throw mid-collect after some pushes leaves a partially-populated map persisted. Still a nit; the catch comment makes the intent clear enough.
2. manifest vs DOM-only exclusive (useExpandedTimelineElements.ts:154-160) — ✅ documented. New comment: "Prefer real manifest children; fall back to DOM-only sub-comp children (groups/pills) that have no data-start and thus never enter the manifest." Behavior unchanged but the convention is now explicit, which was the ask.
3. string-join selector perf (useGsapTweenCache.ts:228-230, 486-488) — 💭 still open. Both usePlayerStore((s) => s.domClipChildren.map(...).join("|")) selectors are unchanged — no useShallow, no memoized key. Re-runs the join on every render of any subscriber to the store. Cost stays low until domClipChildren is large; flag again if it shows up in profiling.

Per-finding status (Rames R1)

4. Mixed manifest / DOM-only convention — ✅ converged with my #2 above; covered by the new comment.
5. Unbounded recursion in collect (useTimelineSyncCallbacks.ts:119-133) — 💭 still open. New collect has two recursive branches (!child.id unwrap, isGroup descend) with no depth bound. Inline comment narrates intent ("one level into groups for drill-in") but doesn't enforce it: a group nested inside a group still recurses without limit, and the id-less unwrap path is unbounded by construction. For the DOM shapes this code targets today (sub-comp body → groups → pills) depth is small, so practical risk is low — but the safety property is implicit in DOM shape, not in code. Adding a depth arg with a cheap cap would be the surest fix; alternatively, a comment promising the DOM invariant would close it.
6. Hardcoded index.html#${hostId} fallback (useGsapTweenCache.ts:204) — 💭 still open. New helper still uses literal index.html#${hostId} for the host key fallback, while the first lookup correctly uses ${sourceFile}#${elementId} (dynamic). Host-key matching assumes the host clip's key is always prefixed with index.html#, which works for top-level scenes hosted in the root composition but not for nested sub-comp hosts authored in another file. For the first-level expansion this PR targets the assumption holds, but it's a land-mine for the nested-sub-comp work that's clearly on the roadmap (the docstring mentions deeper nesting). Source the host's actual file from the manifest (e.g. look up manifest.find(c => c.id === hostId)?.compositionSrc) before this lands in deeper-nesting work.

Notes

• The domClipChildrenKey dep wiring on both useGsapAnimationsForElement and usePopulateKeyframeCacheForFile is the right pattern — it forces a re-run once the DOM-child surface settles, which is exactly when the host bounds become resolvable. Pairs cleanly with the try-guarded population on the sync-callbacks side.
• collect correctly switches from the previous "groups only" walk to "id'd descendants, grouped or ungrouped" with a single-level group drill-in. The data-hf-inner-root lookup is a nice escape valve for the inlined sub-comp body wrapper.
• Test coverage is light for the delta — useExpandedTimelineElements.test.ts only got formatting changes, not new assertions for the deeper recursion paths in collect or the host-bounds-fallback path through resolveClipTimingBasis. Worth a follow-up test if either gets exercised more aggressively.

R2 by Via

[james-russo-rames-d-jusso]

R2 verification of #1761 — feat(studio): expand sub-composition groups + children in the timeline

HEAD reviewed: f27e81ee (R1 SHA: 362195cb) · Base: eg-03-inline-timeline · Files at HEAD vs R1 SHA on this PR's surface (studio/*): bit-identical — git diff 362195cb..f27e81ee -- packages/studio/ returns empty.

Prior reviewer state:

• Rames R1 COMMENTED @ 362195cb (2026-06-27T03:23 UTC).
• Vance R1 COMMENTED @ 362195cb (2026-06-27T03:28 UTC).
• Vance R2 COMMENTED @ f27e81ee (2026-06-27T04:30 UTC, ~22m ago) — converged on my R1 framing + added two angles.

Verdict: comment / R1 findings unchanged. The R1→HEAD delta on this PR's studio files is zero: Miguel's "addressed" claim reflects a Graphite restack that lifted the same fix-up commit (f27e81ee ≡ 362195cb for our purposes) onto the rebased foundation. No code changes to the recursion shape, the try-catch swallow, the manifest/DOM-only exclusivity, the string-join selector, or the hardcoded index.html# fallback. Layering onto Vance R2's per-finding triage rather than re-raising parallel.

---

R1 findings re-checked at f27e81ee

1. ⚠️ Unbounded collect recursion (useTimelineSyncCallbacks.ts:115-132) — STILL OPEN.
Code unchanged. Two recursive branches (!child.id unwrap at L117-120 + isGroup descend at L129) with no depth bound, no cycle guard. Vance R2 marked this 💭 with the right framing — practical risk low because DOM nesting is bounded by sub-comp body → groups → pills today, but safety is implicit in DOM shape, not enforced in code. Adjacent risk that Vance didn't quite spell out: the outer try/catch at L83-139 SWALLOWS any throw from collect into the comment "cross-origin or __clipTree not available — maps stay empty", which is actively misleading — a RangeError: Maximum call stack size exceeded would be silently absorbed and the timeline expansion would just stop working with no log. If you don't add the depth cap, at least log in the catch (console.warn) so a real failure is debuggable instead of mystery-empty.

2. ⚠️ Mixed-siblings handling (useTimelineSyncCallbacks.ts:115-132) — RESOLVED-BY-DOCUMENTATION on the consumer side, still implicit on the producer side.
Re-reading my own R1 against the producer code: collect doesn't actually drop mixed-shape children. It pushes every id'd child and descends through every id-less wrapper. So the original "silent drop" framing was wrong for the producer. The CONSUMER (buildExpandedElements at useExpandedTimelineElements.ts:147-156) IS exclusive (manifest OR DOM-only fallback, never both) — and that's the legitimate concern Vance flagged (his R1 #2 → R2 ✅ documented). I withdraw my R1 mixed-siblings framing as misread. ✅.

3. ⚠️ Same-id-under-multiple-sub-comp-hosts attribution (useTimelineSyncCallbacks.ts:115-132) — STILL OPEN.
The walk pushes {id, parentId, hostId} per child but does NOT dedupe by id across hosts. If two sub-comp instances of the same composition file are present in the scene (e.g. two <div class="scene" data-composition-id="scene" id="scene-host-a"> and scene-host-b cousins), each host's collect() walks the same internal id-set ("group-1", "pill-1", …) — producing duplicate domClipChildren entries with the SAME id but different hostId. Downstream domClipChildrenKey join-string sees both, and parentMap.set(child.id, parentId) at L128 has a last-write-wins race: only the SECOND host's children are reachable via parentMap, breaking the first host's expansion. (Sub-comp instances reusing the same composition file is exactly the case where THIS PR matters — pills inside reused scenes.) Two fixes: (a) namespace the synthesized child id with ${hostId}/${id} everywhere downstream, or (b) refuse to walk a second host that re-uses an id already collected, with a console warning. Worth verifying against a fixture with two instances of the same sub-comp before this lands.

4. ⚠️ Hardcoded index.html#${hostId} fallback (useGsapTweenCache.ts:204) — STILL OPEN.
Code unchanged. Vance R2 marked 💭 with the same framing I had. The first lookup at L201 correctly uses ${sourceFile}#${elementId} (dynamic), but the host-key fallback at L204 hardcodes index.html#${hostId}. For root-composition-hosted sub-comps this is fine; for nested sub-comps hosted in another file it silently regresses to host=undefined and the function returns {elStart: 0, elDuration: 1} — exactly the bug this PR is fixing, just one level deeper. Source the host's actual file from the manifest (manifest.find(c => c.id === hostId)?.compositionSrc) before the deeper-nesting work lands; the patch shape is two lines.

---

Additional cross-cuts (not in R1)

5. 💭 Try-catch silent setState ordering (useTimelineSyncCallbacks.ts:83-139). Converging with Vance R2 #1. The catch swallows mid-collect throws after some children have already been pushed, leaving domClipChildren partially populated AND the previous timeline's stale maps still in the store (because the two setState calls at L135-136 never fire). Either move the two setState calls OUTSIDE the try after assembling locals, or setClipParentMap(new Map()); setDomClipChildren([]) in the catch. Nit — the catch comment misleads more than the actual behavior.

6. 💭 String-join selector perf (useGsapTweenCache.ts:228-230, 486-488). Converging with Vance R2 #3. O(n) join on every Zustand set regardless of which slice changed. Cheap fix: useShallow over the raw array, or pre-compute a key inside setDomClipChildren and select that. Not a blocker; flag if it shows up in profiling.

---

Cross-read with sibling #1764

Verified seam: HF#1761's domClipChildren is re-computed from live DOM on every applyParsedTimeline sync. After HF#1764 strips a group's GSAP + removes the wrapper, the next iframe re-render produces a flat DOM (no <div data-hf-group>), and collect() walks the host's children as id'd-direct (no recursion into groups). The pills end up with parentId = hostId instead of parentId = groupId. Timeline shows them as host-direct rows. ✅ No stale-domClipChildren between the two PRs.

Trust state at HEAD

• Tests added: ✅ expands DOM-only sub-comp children exercises buildExpandedElements consumer path.
• Tests NOT added: the collect() DOM-walk producer in useTimelineSyncCallbacks.ts has no direct unit coverage (Vance R2 noted this too). The recursion + same-id-across-hosts + try-swallow paths above are all untested.

---

What I did not verify

• Whether the studio writers ever currently produce two instances of the same sub-comp file in a single scene (concern #3 hinges on this — if it's impossible by construction, the finding is theoretical).
• Whether the domClipChildrenKey join hits a profiling threshold on real scenes (concern #6 hinges on this).
• Whether data-hf-inner-root is a guaranteed attribute on every sub-comp host or just the first-class ones (affects collect's implicit invariant).

R2 verification by Rames D Jusso

[vanceingalls]

R3 verification of #1761 — feat(studio): expand sub-composition groups + children in the timeline

HEAD reviewed: 8265a4d1 (R2 SHA: f27e81ee, R1 SHA: 362195cb) · Base: eg-03-inline-timeline
Size: unchanged at +207/-22 across 5 PR-scoped files.
CI: all required green at this SHA — player-perf, regression, preview-regression, Preview parity, Preflight (lint + format), WIP. Graphite mergeability_check in_progress (non-blocking).
Prior reviewer state at this SHA: Via R1 @ 362195cb COMMENTED, Via R2 @ f27e81ee COMMENTED, Rames R1 @ 362195cb COMMENTED, Rames R2 @ f27e81ee COMMENTED. No R3 from Rames yet at 8265a4d1.

Verdict: LGTM-sustained (pure base-rebase). The R2→R3 delta (f27e81ee...8265a4d1, 7 commits, +757/-91 across 15 files in compare) is entirely base-branch evolution — sibling-stack work in packages/core/src/parsers/gsap* (parser + writer + new inline tests), GsapAnimationSection.tsx, domEditingLayers.ts, useDomSelection.ts, and studioPreviewHelpers.ts (group-hit-point fallback). None of these are PR-scoped to #1761.

Blob-SHA proof — all 5 PR-touched files are byte-identical between R2 and R3:

File	R2 blob	R3 blob	Match
useTimelineSyncCallbacks.ts	c943680b	c943680b	yes
useGsapTweenCache.ts	97053f63	97053f63	yes
useExpandedTimelineElements.ts	e657c3b9	e657c3b9	yes
playerStore.ts	4f108b26	4f108b26	yes
useExpandedTimelineElements.test.ts	2df7fa9a	2df7fa9a	yes

R3 carries the same single delta commit fix(studio): correct keyframes + expansion for sub-composition timeline clips as R2 (identical message, identical content) — just replayed on a newer base.

Open NITs from R2 — status unchanged

All four R2-open items remain open by construction (no PR-scoped code changed):

1. try-block setState ordering (useTimelineSyncCallbacks.ts) — setClipParentMap + setDomClipChildren still inside the try; catch still silent-swallow with no partial-state reset.
2. string-join selector perf (useGsapTweenCache.ts:228-230, 486-488) — both domClipChildrenKey selectors still join on every render; no useShallow, no memoized key.
3. Unbounded collect recursion (useTimelineSyncCallbacks.ts:119-133) — no depth bound on either recursive branch; safety still implicit in DOM shape, not in code.
4. Hardcoded index.html#${hostId} fallback (useGsapTweenCache.ts:204) — still literal index.html#; will land-mine deeper-nesting work once a sub-comp is hosted from another file.

All four were nits at R2 and remain nits at R3.

Notes

• The non-PR delta in studioPreviewHelpers.ts (new findGroupAtPoint for animated-group hit-testing) is sibling-stack work that pairs well with this PR's group/pill expansion — empty space inside an animated group's overlay now hit-tests to the group itself. Worth a sanity check if it shows up in the regression diff for the timeline-select flow downstream, but it's not in scope here.
• Re-rebase noise like this is exactly the verification-vs-action snapshot drift pattern — the URL-displayed delta is large, but the PR-scoped delta is zero. Worth the per-file blob-SHA check on every rebased R3 against a clean R2.

R3 by Via

[james-russo-rames-d-jusso]

R3 verification of #1761 — feat(studio): expand sub-composition groups + children in the timeline

HEAD reviewed: 8265a4d1 (R2 SHA: f27e81ee) · Base: eg-03-inline-timeline · Diff R2→HEAD on R1-cited files: zero lines (packages/studio/src/player/hooks/useTimelineSyncCallbacks.ts, packages/studio/src/player/hooks/useExpandedTimelineElements.ts, packages/studio/src/hooks/useGsapTweenCache.ts, packages/studio/src/player/store/playerStore.ts — all four byte-identical between f27e81ee and 8265a4d1).

Verdict: Pure Graphite base-rebase, third consecutive restack with zero substantive content delta on this PR's surface. R1 + R2 findings carry through unchanged by construction. Converging with Via's R3 at 8265a4d1 on the verdict and the blob-identical analysis — his per-file blob-SHA table is the cleanest evidence; I confirmed git diff f27e81ee..8265a4d1 -- <R1 files> returns empty.

R2 findings re-verified at HEAD

All four R2 ⚠️ items remain open by definition (the files didn't move). Via enumerated four; I add a fifth he didn't:

#	Finding	R3 status
1	Unbounded collect recursion + silent try-catch swallow at useTimelineSyncCallbacks.ts:115-139	open
2	Same-id-under-multiple-sub-comp-hosts attribution (parentMap.set(child.id, parentId) at useTimelineSyncCallbacks.ts:128 is last-write-wins; reused sub-comp instances collide)	open
3	Hardcoded index.html#${hostId} fallback at useGsapTweenCache.ts:205 (trivial fix: manifest.find(c => c.id === hostId)?.compositionSrc)	open
4	setClipParentMap + setDomClipChildren inside the try block; catch silent-swallows with no partial-state reset (Via R3 #1)	open
5	domClipChildrenKey string-join selectors at useGsapTweenCache.ts:228-230, 486-488 run on every render — no useShallow, no memoized key (Via R3 #2)	open

All five are non-blocker nits — same posture as R1 + R2.

Notes

• I'm not re-opening any R2 finding as a blocker just because the restack didn't pick them up. They were nits at R2; they remain nits at R3.
• The pattern (third restack-no-content cycle) is worth a Slack ping for stack-hygiene if it continues — the URL-displayed delta is large enough that a reviewer who doesn't run git diff per-file would think substantial change is happening. But that's a process note, not a review finding.

Sustaining R2 verdict: non-blocking nits-only. Approve once Miguel decides to land — the open items are all stack-hygiene shape, not blocking the timeline-expansion feature.

R3 verification by Rames D Jusso

[github-actions]

Fallow audit report

Found 21 findings.

Duplication (8)

Severity	Rule	Location	Description
minor	fallow/code-duplication	packages/parsers/src/gsapParserAcorn.inline.test.ts:25	Code clone group 1 (8 lines, 2 instances)
minor	fallow/code-duplication	packages/parsers/src/gsapParserAcorn.inline.test.ts:36	Code clone group 1 (8 lines, 2 instances)
minor	fallow/code-duplication	packages/studio/src/components/editor/manualEditsDom.ts:168	Code clone group 2 (7 lines, 2 instances)
minor	fallow/code-duplication	packages/studio/src/components/editor/propertyPanelHelpers.ts:328	Code clone group 2 (7 lines, 2 instances)
minor	fallow/code-duplication	packages/studio/src/hooks/useDomSelection.ts:71	Code clone group 3 (22 lines, 2 instances)
minor	fallow/code-duplication	packages/studio/src/hooks/usePreviewInteraction.ts:16	Code clone group 3 (22 lines, 2 instances)
minor	fallow/code-duplication	packages/studio/src/player/hooks/useExpandedTimelineElements.test.ts:45	Code clone group 4 (7 lines, 2 instances)
minor	fallow/code-duplication	packages/studio/src/player/hooks/useExpandedTimelineElements.test.ts:68	Code clone group 4 (7 lines, 2 instances)

Health (13)

Severity	Rule	Location	Description
major	fallow/high-crap-score	packages/parsers/src/gsapParserAcorn.ts:392	'sameMemberAccess' has CRAP score 63.6 (threshold: 30.0, cyclomatic 15)
minor	fallow/high-crap-score	packages/studio/src/components/editor/LayersPanel.tsx:151	'seekToLayer' has CRAP score 43.1 (threshold: 30.0, cyclomatic 12)
minor	fallow/high-crap-score	packages/studio/src/components/editor/LayersPanel.tsx:306	'<arrow>' has CRAP score 37.1 (threshold: 30.0, cyclomatic 11)
critical	fallow/high-crap-score	packages/studio/src/components/editor/domEditingDom.ts:202	'escapeCssIdentifier' has CRAP score 148.4 (threshold: 30.0, cyclomatic 24)
critical	fallow/high-crap-score	packages/studio/src/components/editor/useDomEditOverlayRects.ts:115	'update' has CRAP score 299.6 (threshold: 30.0, cyclomatic 35)
critical	fallow/high-crap-score	packages/studio/src/hooks/useAppHotkeys.ts:156	'dispatchModifierKey' has CRAP score 420.0 (threshold: 30.0, cyclomatic 20)
critical	fallow/high-crap-score	packages/studio/src/hooks/useAppHotkeys.ts:359	'applyHistory' has CRAP score 156.0 (threshold: 30.0, cyclomatic 12)
minor	fallow/high-crap-score	packages/studio/src/hooks/useAppHotkeys.ts:430	'handleAppKeyDown' has CRAP score 30.0 (threshold: 30.0, cyclomatic 5)
major	fallow/high-crap-score	packages/studio/src/hooks/useAppHotkeys.ts:490	'syncPreviewHistoryHotkey' has CRAP score 56.0 (threshold: 30.0, cyclomatic 7)
critical	fallow/high-crap-score	packages/studio/src/player/hooks/useTimelineSyncCallbacks.ts:60	'processTimelineMessage' has CRAP score 148.4 (threshold: 30.0, cyclomatic 24)
critical	fallow/high-crap-score	packages/studio/src/player/hooks/useTimelineSyncCallbacks.ts:209	'initializeAdapter' has CRAP score 238.6 (threshold: 30.0, cyclomatic 31)
minor	fallow/high-crap-score	packages/studio/src/player/hooks/useTimelineSyncCallbacks.ts:320	'onMessage' has CRAP score 31.6 (threshold: 30.0, cyclomatic 10)
minor	fallow/high-crap-score	packages/studio/src/utils/studioPreviewHelpers.ts:89	'findGroupAtPoint' has CRAP score 37.1 (threshold: 30.0, cyclomatic 11)

Generated by fallow.