feat(gsap): read timelines authored inline (acorn read path)

[miguel-heygen]

Edit inline-authored GSAP timelines (3/8)

Read and edit timelines authored inline as window.__timelines["id"] = gsap.timeline() (member-expression reference), not just the canonical const tl = gsap.timeline(); window.__timelines[id] = tl form.

What's here

• acorn read path for inline/member timelines — core/src/parsers/gsapParserAcorn.ts
• recast writer support (default server write path) — inline edits/adds/deletes/keyframes round-trip
• round-trip write coverage test
• studio enable for static inline timelines

Independent of the groups work (parser layer); stacked here to keep gsapWriterAcorn edits in order.

[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
• feat(gsap): read timelines authored inline (acorn read path) #1760 👈 (View in Graphite)
• 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]

Reviewed at b04cee7 — canonical / test-coverage lane. HF-runtime-interop + player semantics deferred to Via.

The TimelineRef model is a clean read-path generalization, and the read tests are genuine (non-reflexive — they parse and assert against literal-fixture shapes, distinct quote forms, multi-timeline non-cross-attribution). Writer round-trip coverage for inline tween edit/add/remove/keyframes looks solid. One ⚠️ load-bearing gap, a handful of 🟡 follow-ups.

---

⚠️ Label-write APIs silently broken for inline timelines — gsapWriterAcorn.ts:277 (isTimelineRooted) only matches Identifier nodes. For inline-rooted timelines, parsed.timelineVar is the source string window.__timelines["scene"], so:

function isTimelineRooted(node, timelineVar) {
  if (node?.type === "Identifier") return node.name === timelineVar;  // never matches inline
  if (node?.type === "CallExpression") return isTimelineRooted(node.callee?.object, timelineVar);
  return false;
}

The Identifier branch can never match a MemberExpression-rooted call. Knock-on:

• isTimelineMethodCall → isAddLabelCall → findLabelStatements (l.1562/1572/1581) always returns [] for inline timelines.
• addLabelToScript (l.1592) silently violates its dedup contract: a second addLabel("x", …) on the same inline timeline APPENDS rather than MOVES, producing two window.__timelines["scene"].addLabel("x", …); statements.
• removeLabelFromScript (l.1618) becomes a no-op for inline timelines — never finds the targets, deletes nothing.
• gsapWriterParity.corpus.test.ts:580-650 exhaustively covers the canonical var tl = … form but doesn't exercise inline, so this regresses silently.

This affects the SDK label-write path consumed at packages/sdk/src/engine/mutate.ts:1333,1342. extractGsapLabels (read) was updated for member refs (parser l.1271-1275) — the asymmetry is what makes this silent. Suggested shape: teach isTimelineRooted about the TimelineRef too (or thread ref through the writer's label helpers and switch on kind), then port one inline-form variant of each of the 7 correctness — addLabelToScript / removeLabelFromScript test cases.

---

🟡 Single-quote preamble regex may go through recast.print()'s default quote choice — gsapParser.ts:1186 (timelineVar = timelineRootSource(ref) → recast.print(ref.node).code). Recast preserves the original node's quote style when it has one, but if the source has both single- and double-quoted occurrences of the same slot (window.__timelines["a"] declaration, then window.__timelines['a'].to(…)), timelineVar matches the declaration's form while lastIndexOf(\${timelineVar}.`)searches the source with that one form — the postamble extraction can come up short by missing the other-quote callsite. Thepreserves single-quote member form on writerecast test only exercises a single-form consistent fixture. Probably low blast radius (mixed-quote authored timelines are rare) but worth a one-line test assertingparseGsapScript` over mixed-quote returns the full tween set with empty postamble even when forms cross.

---

🟡 recast.print of identifier returns a name, of member returns multi-char source — lastIndexOf window narrows — for timelineVar = window.__timelines["scene"], script.lastIndexOf(\${timelineVar}.`)looks for the literal 32-char prefix. Fine for the test fixtures. Edge case: if the source useswindow . __timelines [ "scene" ] .to(...)with extra whitespace, the AST still parses butlastIndexOf(which is source-string-literal) won't find it, andpostamblebecomes empty. Recast / acornprint()produces no-whitespace canonical output, so this is an authored-input edge only. Worth ignoring unlessunsupportedTimelinePatternis set; right now the postamble is silently empty, which is benign but mismatches the canonical-form behavior. Same shape exists in the existing identifier path (a tween withtl . to(...)` would also miss), so this isn't a regression — just an inherited fragility surfaced by the more-complex root token.

---

🟡 Multi-timeline first-wins behavior on inline form — findTimelineVar keeps the first-seen ref and increments timelineCount for siblings (parser test "does not cross-attribute" l.371 confirms). The studio banner shows multipleTimelines, suppressing the per-tween UI (GsapAnimationSection.tsx:54). But: when a source has two inline timelines window.__timelines["a"] + window.__timelines["b"], the editor sees only timeline a's tweens. For canonical-form sources, this was already the established behavior. For inline, this is now more likely to occur in the wild (multi-comp HTMLs author both inline). Verify there's no studio path that lets the user "select" which timeline to edit — if not, no action; if so, the inline form needs the same selector. Likely fine, but worth Via or Miguel confirming explicitly.

---

🟡 hasTimeline semantics shift — gsapParserAcorn.ts:1163 now reports hasTimeline: detection.ref !== null. Previously: detection.timelineVar !== null. The new field includes the inline form, the old field excluded it. findInsertionPoint (writer l.295) uses hasTimeline to decide whether tl.xxx() would be undefined-rendered. With inline, hasTimeline=true and the fallback path findTimelineDeclarationStatement returns null (it only walks VariableDeclaration), so insertionPoint = ast.end. This means a label/keyframe write on an EMPTY inline timeline appends to the very end of the script (works — addAnimationToScript test "adds the first tween to an empty inline timeline" confirms). Just flagging that the writer's fallback insertion isn't anchored at the inline declaration the way it is for canonical — findTimelineDeclarationStatement doesn't have a member variant. Not blocking; the appearance-at-end behavior is correct for an empty script.

---

🟢 Non-findings / verified

• Parity-test reflexivity: tests read AST output and assert against literal-fixture shapes (target selectors, counts, banner flags). Not the f(x) === f(x) shape — these are genuine assertions.
• Duration-source asymmetry: no duration/timing parity claim in the new tests; both parsers compute positions through resolveTimelinePositions independently. N/A.
• Cross-repo coupling: greped heygen-cli + pacific for parseGsapScript / unsupportedTimelinePattern / timelineVar consumers — none. Stays in-monorepo; extractGsapLabels is the only outward-facing surface touched, used by packages/sdk/src/session.ts and correctly updated.
• Dispatch-map silent-drop: PR uses a kind union switch (identifier vs member), not a string-keyed map; both branches handled at every site touched.
• Feature-flag silent revert: no flag-defining files touched.
• Acorn dynamic-import race: acorn is statically imported throughout the parser; no let parser; import().then(m => parser = m.parse) shapes added or modified.
• Facade async drop: no facade rewrite; parseGsapScript* are sync.
• Status-machine in-flight rows: pre-PR projects authored inline that were unreadable before (unsupportedTimelinePattern=true) become readable after this PR. No data shape changes — the same script reads new animations directly. No backfill needed.
• Sibling asymmetry / CSP: read path is pure AST inspection, doesn't eval or Function-construct user input. Comparable to existing identifier-mode path.
• Sub-composition compatibility with #1761: #1761 doesn't touch gsapParser*.ts / gsapWriterAcorn.ts. It consumes animations + timelineVar via existing read API; the inline form returns a syntactically-valid timelineVar source string. #1761's expansion logic reads animations[].targetSelector and tween position, both shape-stable. Cross-coherent.
• Banner copy retarget at GsapAnimationSection.tsx:47-51: change is accurate — unsupportedTimelinePattern is now reserved for computed-key timelines (per parser l.1232-1233 which fires only when timelineCount>0 && ref===null). Copy correctly names the surviving genuinely-unsupported case.

---

📋 Verdict

• Blockers: none.
• Request before merge: address ⚠️ above (inline label-write parity) — silent regression in the SDK label path is the main miss. The fix is small (≤10 LOC + a handful of test cases) and matches the PR's own thesis of "edit inline timelines."
• Defer to Via for player-semantics + runtime-interop sign-off (preview/render parity, paused-master-timeline interaction, the <Iframe> consumer surface — outside my lane this round).
• Stamp posture: hold mine pending the label-write fix; once that lands, happy to stamp.

— Rames D Jusso

[vanceingalls]

PR #1760 — feat(gsap): read timelines authored inline (acorn read path)

Reviewed at b04cee78 · Lane: runtime-interop / SDK / player semantics (Via)
Verdict: Hold — converge with Rames on the inline-form label-write asymmetry (🚫 below). Runtime-interop is clean; render-determinism is clean; the gap is one missed-dispatch chain in the writer's label helpers that silently regresses an SDK consumer path.

TimelineRef is a tasteful generalization — the read path threads the union through every site that previously assumed an Identifier name, and the tests are non-reflexive (parse → assert against literal-fixture shapes, distinct quote forms, multi-timeline non-cross-attribution). My lane finds one chain Rames also caught (so we converge), plus two runtime-interop verifications worth recording.

---

🚫 Inline label-write helpers silently no-op / dedup-broken

Same chain Rames flagged. Confirmed independently from the consumer-surface side.

packages/core/src/parsers/gsapWriterAcorn.ts:277 (isTimelineRooted) only matches an Identifier:

function isTimelineRooted(node: Node, timelineVar: string): boolean {
  if (node?.type === "Identifier") return node.name === timelineVar; // can never match a MemberExpression
  if (node?.type === "CallExpression") return isTimelineRooted(node.callee?.object, timelineVar);
  return false;
}

For inline-rooted timelines, parsed.timelineVar is the source slice window.__timelines["scene"] (per the new timelineRootSource(ref, script) at gsapParserAcorn.ts:444). The Identifier branch never matches a MemberExpression callee object, so:

• isTimelineMethodCall (l.1562) → isAddLabelCall (l.1572) → findLabelStatements (l.1582) → always returns [] for inline timelines.
• addLabelToScript (l.1592) silently violates its own dedup contract: calling it twice with the same name on an inline-form composition produces two window.__timelines["scene"].addLabel("intro", …); statements. The PR's own preamble (l.1596-1599 in the writer file) calls out this anti-pattern explicitly — "Two same-named addLabel statements make removeLabel over-remove" — and the inline form now triggers it on every duplicate add.
• removeLabelFromScript (l.1618) becomes a no-op for inline timelines — never finds the targets, deletes nothing.

This is asymmetric: extractGsapLabels (read) was updated for member refs (parser l.1274-1278, properly switching on ref.kind), but the write side wasn't. The asymmetry is what makes it silent — the editor reads labels back, sees no problem, then the next write produces a duplicate or no-op without surfacing anything.

SDK consumer impact (the surface Vai/Magi flagged into my lane): extractGsapLabels feeds packages/sdk/src/session.ts; the dual write APIs (addLabelToScript / removeLabelFromScript) feed packages/sdk/src/engine/mutate.ts:1333,1342. End-to-end mutation on an inline-form composition: read sees N labels, addLabel("x", t) silently appends, re-read sees N+1. removeLabel("x") finds nothing, re-read still sees N+1. Compounds across edit sessions until the script becomes structurally invalid (duplicate label names → GSAP runtime warns, then non-deterministic seek).

Suggested shape: teach isTimelineRooted about TimelineRef (same sameMemberAccess predicate the parser already uses), or thread ref through the writer's label helpers and switch on kind. The fix is small (≤10 LOC + a handful of test cases mirroring the existing correctness tests for the canonical form). The PR's thesis — "edit timelines authored inline" — names label-edits inside that scope.

This is a band-aid bar fail: silent scope gap in the same merged feature (extractGsapLabels updated, label-write not). Worth fixing in this PR rather than as a follow-up so the inline-form story ships coherent.

---

🟡 Empty-inline-timeline insertion fallback is unanchored

findInsertionPoint (writer l.288) — when the inline timeline has no tweens yet, parsed.located.length === 0, parsed.hasTimeline === true, so it calls findTimelineDeclarationStatement(ast, parsed.timelineVar). That helper only matches VariableDeclaration nodes with decl.id?.name === timelineVar (l.115-134), so for inline form (timelineVar is the multi-char source string) it returns null, and findInsertionPoint falls back to parsed.ast.end.

The test at gsapWriterAcorn.inline.test.ts:730-742 confirms this works when the assignment is the last statement in the script. But if a user's authored script has trailing statements after the inline assignment (e.g. a tl.pause() call, registration in another slot, document.fonts.ready epilogue), the new tween appends at end-of-script rather than after the timeline declaration. The canonical-form path anchors at tlDecl.end; the inline path doesn't. Not a regression in any practical fixture I checked, but the asymmetry is worth a findTimelineDeclarationStatement extension that handles the AssignmentExpression case — same shape as the parser's findTimelineVar already does.

---

🟡 Studio banner copy and unsupportedTimelinePattern semantics: verified

GsapAnimationSection.tsx:48-52 retargets the banner from "any window.__timelines[...]" to "computed-key only". Verified against the parser change at gsapParserAcorn.ts:1232-1233: the flag now fires only when timelineCount > 0 && detection.ref === null, which is precisely the "no statically-resolvable timeline ref found" case (computed key, dynamic property, or non-MemberExpression LHS like array index). Copy accurately names the surviving genuinely-unsupported case. The static-literal-key inline form correctly enters the editable path. No false-positive on the new form.

---

🟡 Helper-built inline timelines: documented limitation but author's comment is mildly wrong

gsapParserAcorn.ts:572-580 skips inlineComputedTimelines for member-form refs with the comment "inline member timelines have nothing to inline, so skip (avoids mis-rooting on the member)". The "nothing to inline" claim isn't quite right: an inline timeline can still have helper-built tweens (function add(sel, x) { window.__timelines["scene"].to(sel, {x}, 0); }; add("#a", 100);). The skip is defensive — inlineComputedTimelines walks identifier-rooted patterns (gsapInline.ts:178, timelineRootName returns Identifier.name only) so feeding it the inline form would be a no-op anyway. The comment would be more accurate as "inline member helpers aren't supported by inlineComputedTimelines, so skip". Nit, low-impact — direct tween calls in inline form still parse correctly via findAllTweenCalls.

---

💭 Recast-path quote-style drift on first add to single-quote inline form

gsapParser.ts:1186 computes timelineVar = recast.print(ref.node).code. Recast generally preserves source quote style for nodes with original attached, but on member expressions with literal string properties the print can default to double quotes. The new tween statement built by buildTweenStatementCode(parsed.timelineVar, animation) (line 1532) would then emit window.__timelines["scene"].to(...) even in a source authored with ['scene']. The test fixture at gsapParser.inline.test.ts:92-97 covers UPDATE on single-quote (works because MagicString operates on existing nodes), but not ADD on single-quote. Coverage gap, very low blast radius. The acorn writer doesn't have this issue — timelineRootSource(ref, script) slices verbatim from source (gsapParserAcorn.ts:444) so quote style is preserved by construction.

---

🟢 Runtime-interop sign-off (my lane)

• HF_EARLY_STUB tween-batching interceptor (packages/producer/src/generated/hf-early-stub-inline.ts): intercepts gsap.timeline() by replacing the returned timeline with a proxy v() that queues .to/.from/.fromTo/.set/.add until first transport call (.pause(), .play(), .seek(), etc.) drains them. Both canonical and inline forms go through this path identically: the proxy IS what gets assigned to window.__timelines["scene"] in the inline form (vs. assigned to tl then to __timelines["root"] in the canonical form). No new races. The hf-timelines-built event fires after the queue drains, unchanged. Render-determinism preserved.
• Composition load timing (init.ts:2245-2258): the __hfTimelinesBuilding gate waits for hf-timelines-built before publishing render-ready. Inline form sets __timelines[id] synchronously at the assignment point (vs. canonical's "set after build" pattern), but both arrive at render-ready post-drain. No earlier-than-canonical render-ready leak.
• Sub-composition registry (init.ts:2289-2313, seekStandaloneRegisteredTimelines): consumes window.__timelines as a registry of RuntimeTimelineLike. Both forms produce a registry entry pointing at the same proxy. No shape difference.
• Lint compatibility (lint/utils.ts:26-27 WINDOW_TIMELINE_ASSIGN_PATTERN): permissive regex — matches the inline form (captures gsap as the "RHS identifier" thanks to .test() substring semantics). gsap_timeline_not_registered does NOT false-positive on inline form. gsap_timeline_registered_before_async_build will lint-fire on inline-form timelines registered before document.fonts.ready (the empty-timeline-registered-early failure mode) — but this is the documented authoring contract, not a new regression. Authors will see the same error they would for __timelines["x"] = tl; before fonts-ready.
• Acorn ECMAScript level: parser uses ecmaVersion: "latest". Static-string-literal member expressions and dot-identifier member expressions are both supported on every modern target; the parser's staticMemberKey (l.413) correctly handles Literal (string) and Identifier (dot-access) cases, and correctly refuses (returns null → unsupportedTimelinePattern) on dynamic-key Identifiers like [id].
• Cross-PR coupling with #1762: PR #1762 also touches gsapWriterAcorn.ts, but in a disjoint function (updateKeyframeInScript at l.776 — the 3D keyframe merge fix). No textual conflict, no semantic interaction with TimelineRef. Stacks cleanly.

🟢 Render-determinism

recast.print(ref.node).code and script.slice(ref.node.start, ref.node.end) are deterministic for a given input — neither introduces sort-by-set or floating-point ordering. buildTweenStatementCode is pure string concatenation. The new code paths add no Map/Set traversal that would surface insertion-order dependence. Renders byte-stable across runs.

---

CI

24/25 checks green; regression-shards (shard-4) still in flight at review time (no failure signal). No blocking signal from CI.

Prior reviews

• Rames (james-russo-rames-d-jusso, b04cee7): converged on the same ⚠️ inline-label-write gap as my 🚫. He defers runtime-interop / player semantics to me. Verdict: hold pending the label-write fix.

Posture

Holding stamp pending the inline-form label-write fix. Runtime-interop + render-determinism + lint compatibility verified clean. Cross-stack with #1762 is non-overlapping. Once the label helpers learn TimelineRef (or isTimelineRooted switches on ref.kind), happy to stamp.

Review by Via

[miguel-heygen]

Fixed the inline label-write asymmetry: isTimelineRooted (+ the isTimelineMethodCall → isAddLabelCall → findLabelStatements chain) now matches member-rooted callees by source, threading script through. addLabelToScript dedups (moves, not duplicates) and removeLabelFromScript finds + deletes on inline timelines — verified with a round-trip test (2 adds → 1, moved; remove → 0). Emission was already correct (uses parsed.timelineVar); only the match path was Identifier-only. The recast default path was already member-aware via TimelineRef, so no change needed there.

[vanceingalls]

PR #1760 R2 — feat(gsap): read timelines authored inline (acorn read path)

Verdict: ✅ LGTM — BLOCK addressed end-to-end, all yellows landed, every R1 ask has matching test coverage.

Reviewed at 031f52f4 (was b04cee78). Delta +39 net / +488/-67 total. Old size +449/-57.

Prior reviewer state at this HEAD: none yet — both Rames and I are still at b04cee78. No converging or diverging R2 signal to weigh.

CI at 031f52f4: Preflight (lint + format) green ×4, Perf/Preview parity green ×8, player-perf / preview-regression green. 16 regression-shards still in-progress, no failures.

---

R1 finding status

🚫 → ✅ BLOCK: inline label-write asymmetry (gsapWriterAcorn.ts:277)

Fixed properly. isTimelineRooted now handles MemberExpression by comparing the source slice:

if (node?.type === "Identifier") return node.name === timelineVar;
if (node?.type === "MemberExpression")
  return script.slice(node.start, node.end) === timelineVar;
if (node?.type === "CallExpression")
  return isTimelineRooted(node.callee?.object, timelineVar, script);

The cleverness checks out: timelineVar is timelineRootSource(ref, script), which for the member case IS script.slice(ref.node.start, ref.node.end) — so a callee MemberExpression whose verbatim source matches the timeline reference's verbatim source is structurally the same access. script is threaded through isTimelineMethodCall → isAddLabelCall → findLabelStatements so the chain is intact.

Test coverage for the BLOCK: packages/core/src/parsers/gsapParser.inline.test.ts:107 exercises BOTH halves of the asymmetry on an inline timeline:

it("dedups addLabel (moves, not duplicates) and removes it on an inline timeline", () => {
  let s = addLabelToScript(src, "intro", 0.5);
  s = addLabelToScript(s, "intro", 0.9);
  expect((s.match(/addLabel\(/g) ?? []).length).toBe(1);  // dedup, no duplicate
  expect(s).toContain('addLabel("intro", 0.9)');           // moved to new position
  expect((removeLabelFromScript(s, "intro").match(/addLabel\(/g) ?? []).length).toBe(0); // actually removes
});

That's the exact mis-behavior R1 + Rames called out (addLabel duplicates / removeLabel no-ops on inline form) — proven gone.

✅ Yellow #1 — empty-inline-timeline insertion fallback

gsapWriterAcorn.inline.test.ts:746 exercises adding the first tween to an empty inline timeline (window.__timelines["scene"] = gsap.timeline({ paused: true }); with no calls yet). Output is window.__timelines["scene"].to("#a", ...), re-read finds 1 animation.

Mechanism: findInsertionPoint falls back to parsed.ast.end as number when findTimelineDeclarationStatement returns null (it only matches VariableDeclaration, not member-form AssignmentExpression). For all corpus cases where the inline assignment is the last top-level statement, AST-end equals "right after the assignment" — works.

Latent watchpoint (not blocking): if a future inline corpus appears with a trailing wrapper (IIFE close, post-decl side-effect statements), the new tween would land at end-of-file instead of immediately after the decl. Audit pre-existing for the same fragility against the const form (likely shares it). Not a regression introduced by this PR — flagging for the next inline expansion.

✅ Yellow #2 — recast quote-style drift

gsapParser.inline.test.ts:93 and gsapWriterAcorn.inline.test.ts:719 BOTH assert single-quote member form is preserved through update:

const sq = `window.__timelines['scene'] = gsap.timeline();\n…`;
const out = updateAnimationInScript(sq, id, { properties: { x: 9 } });
expect(out).toContain("window.__timelines['scene']");

The acorn writer uses verbatim script.slice; the recast writer uses recast.print(ref.node).code, which reproduces unmodified node source verbatim. Both tested.

✅ Yellow #3 — banner copy + unsupportedTimelinePattern

GsapAnimationSection.tsx:48-52: banner copy now distinguishes the two states. Old copy claimed "the window.__timelines[...] pattern" was unsupported (false after this PR). New copy:

This timeline uses a computed key (window.__timelines[variable]) the editor can't resolve statically. Use a string-literal key (window.__timelines["id"]) or a variable declaration (const tl = gsap.timeline()) to enable editing.

Surface flag unsupportedTimelinePattern is now only set when detection.timelineCount > 0 && detection.ref === null (i.e. a timeline was detected by isGsapTimelineCall but no static ref — that's computed-key, dynamic-LHS, etc.). Static-member form correctly flows through as a supported timeline.

✅ R1 "thought": runtime-interop

Unchanged from R1 — read-path-only, no SDK-shape or player-side write. The new structural ref is internal-only; surface stays timelineVar: string. No interop concern.

Coverage strength

The test corpus is comprehensive for the read path:

• Read: double-quote, single-quote, dot-access, computed-key (correctly unsupported), cross-attribution (two member slots → multipleTimelines), canonical const tl regression guard.
• Write (acorn): edit / add / remove / keyframes (add+remove) / single-quote preservation / empty-timeline first-tween / no-op stability.
• Write (recast): edit / add / remove / keyframes / single-quote preservation.
• Labels: inline dedup-and-remove (the BLOCK).

Suggestions (not blocking, post-merge if appetite)

1. findTimelineDeclarationStatement member-form support. When the inline form ships beyond the "decl-is-last-statement" corpus, this helper will need an AssignmentExpression case to insert tweens immediately after the assignment rather than at AST-end. Easy follow-up — leave a TODO breadcrumb if you want.
2. extractGsapLabels parity test. The label-READ path in gsapParserAcorn.ts:1271-1275 was updated to handle member form, but I didn't see a direct inline-form label-extract test. If extractGsapLabels is consumed by the label inspector, an inline-form fixture there would close the symmetry.

Nothing else stood out. Nicely modeled refactor — TimelineRef discriminated union, sameMemberAccess for structural equality, staticMemberKey quote-insensitive: this is the shape it should have, and threading script for the writer-side source-slice compare keeps the acorn writer honest about what "rooted at the timeline" means under the new model.

R2 by Via

[james-russo-rames-d-jusso]

R2 verification — 031f52f4 (acorn read path)

R1 ⚠️ inline label-write asymmetry: resolved ✅. Canonical/test-coverage lane only — runtime-interop sign-off remains Via's call (deferring on whether the writer touch needs any player-side acknowledgment beyond what Via already verified at R1).

---

✅ ⚠️ R1 carry-over — isTimelineRooted + label-helper chain

gsapWriterAcorn.ts:277 now matches MemberExpression callees by source-string equality against parsed.timelineVar:

function isTimelineRooted(node: Node, timelineVar: string, script: string): boolean {
  if (node?.type === "Identifier") return node.name === timelineVar;
  if (node?.type === "MemberExpression") return script.slice(node.start, node.end) === timelineVar;
  if (node?.type === "CallExpression")
    return isTimelineRooted(node.callee?.object, timelineVar, script);
  return false;
}

script is now threaded through isTimelineMethodCall (l.1566) → isAddLabelCall (l.1581) → findLabelStatements (l.1591) → both public addLabelToScript (l.1613) and removeLabelFromScript (l.1635). Verified all 5 internal call sites carry script through. No stale isTimelineRooted(…, timelineVar) 2-arg calls left (gsapInline.ts:178 has a separate same-named local function — unaffected, identifier-only by design per R1's helper-built skip comment).

The match is by script.slice(node.start, node.end) === timelineVar where both sides derive from the SAME source string via timelineRootSource(ref, script) at gsapParserAcorn.ts:444. Char-for-char stable. No alias / canonicalization risk.

Test coverage: gsapParser.inline.test.ts:101-107 — addLabelToScript(src, "intro", 0.5) → addLabelToScript(s, "intro", 0.9) produces exactly one addLabel(…) statement at the moved position (0.9), and removeLabelFromScript brings it back to zero. Genuine assertion (counts addLabel( occurrences in the output text). Covers the full dedup-then-remove contract on inline form. One canonical fixture, not a 7-case mirror of the corpus suite, but the canonical bug class is now covered.

Sibling-asymmetry probe (per feedback_sibling_asymmetry_as_security_evidence): extractGsapLabels (read, gsapParserAcorn.ts:1271-1275) and addLabel/removeLabel (write) now BOTH switch on the TimelineRef member form. Read/write symmetry restored — the silent-asymmetry condition that caused the bug class is gone.

🟡 Mixed-quote authored timelines still under-covered (R1 carry-over, unchanged)

The string-equality match means window.__timelines["scene"] = …; window.__timelines['scene'].addLabel("intro", 0) would still under-match (capture from declaration is double-quote source, callee site is single-quote source). Same blast-radius profile as R1: rare authored shape, no new regression. Worth a follow-up findTimelineVar normalization that compares the resolved static key rather than raw source, but not blocking — the canonical inline form (consistent quote style) is what the editor will produce on every round-trip.

🟡 Empty-inline-timeline insertion anchored at ast.end (R1 carry-over, unchanged)

findInsertionPoint (writer l.292) still falls back to parsed.ast.end when the timeline is empty and findTimelineDeclarationStatement returns null (it only walks VariableDeclaration, not AssignmentExpression). Inline-form empty timelines with trailing statements append the new tween at end-of-script. Both R1 reviews flagged; not addressed in R2. Low blast radius for the canonical fixture (assignment-as-last-statement) — flag again as a 🟡 follow-up not blocking this merge.

🟢 R2-NEW scan (verified clean)

• No dispatch-map silent-drop: writer uses a single recursive isTimelineRooted; both Identifier and MemberExpression branches covered, CallExpression recurses, default returns false. No untyped TYPE_TO_HANDLER map; no new union-tag to miss.
• Race-fix classification: this isn't a race fix; nothing introduced (per feedback_race_fix_preempt_vs_narrow).
• Author claim/diff mismatch (per feedback_author_claim_diff_mismatch): Miguel's comment claims "match path was Identifier-only" → diff confirms the Identifier→MemberExpression branch was the only missing case. Claim matches diff at HEAD.
• No silent revert of recently-merged flag/sibling: git diff b04cee78 031f52f4 against the PR's own 7 files = exactly the writer change + one test. No unrelated reverts.
• Stale call-site grep: grep -rn "isTimelineRooted\|isTimelineMethodCall\|isAddLabelCall\|findLabelStatements" packages/ returns only the writer's internal calls (all updated) + the parser's separately-named isTimelineRootedCall (unrelated function, ref-aware by construction). No orphaned 2-arg callers.
• CI: 25 checks, regression-shards still in flight (no failure signal), one Perf: ${{ matrix.shard }} zero-second failure that looks like a workflow matrix-expansion artifact rather than a real failure (the per-shard Perf jobs all passed).
• Cross-PR (#1762): per Via's R1, no textual conflict — #1762 touches updateKeyframeInScript at writer l.776. Unchanged in R2.

🟢 Read/write symmetry verified end-to-end

End-to-end inline-form lifecycle now works:

1. parseGsapScript("window.__timelines[\"scene\"] = gsap.timeline(); …addLabel(\"intro\", 0.5);") reads the label via extractGsapLabels (member-aware since R1).
2. addLabelToScript(script, "intro", 0.9) finds the existing call via findLabelStatements (member-aware in R2), moves the position to 0.9.
3. removeLabelFromScript(script, "intro") finds and deletes — covered by the new test.

The SDK consumer path at packages/sdk/src/engine/mutate.ts:1333,1342 is unblocked.

---

Verdict

• Blockers: none.
• Concerns: none new; two 🟡 R1 carry-overs (mixed-quote match, empty-inline-timeline insertion anchor) — both pre-existing, both low blast radius, neither blocking.
• Stamp posture: from the canonical / test-coverage lane I'm satisfied; deferring runtime-interop sign-off to Via for the writer-touched paths. Once Via re-confirms (the writer fix is small and local, so unlikely to change his R1 verdict), happy to stamp.

R2 verification by Rames D Jusso

[james-russo-rames-d-jusso]

R3 verification — 473fa7b2 (acorn read path)

R2 fixes hold; R2→R3 diff does not touch any reviewed critical path.

What moved between R2 (031f52f4) and R3 (473fa7b2)

packages/studio/src/components/editor/domEditingLayers.ts | 21 ----
packages/studio/src/hooks/useDomSelection.ts              | 23 ----
2 files changed, 44 deletions(-)

Purely the removal of three [HF-DBG] … console.log debug statements (resolveDomEditSelection entry, resolveDomEditSelection → resolved, applyDomSelection). Clean removals — control flow intact, no orphaned blocks, no comments referencing the dropped logs. eslint-disable-next-line no-console pragmas removed with their statements.

git diff 031f52f4..473fa7b2 -- 'packages/core/**' is empty — the acorn read/write path is byte-identical to R2.

R2 fixes re-verified at HEAD

• ✅ isTimelineRooted at packages/core/src/parsers/gsapWriterAcorn.ts:277 — MemberExpression callee match via script.slice(node.start, node.end) === timelineVar intact.
• ✅ script threaded through the label-write call chain: addLabelToScript / removeLabelFromScript → findLabelStatements(parsed, name, script) → isAddLabelCall(..., script) → isTimelineMethodCall(..., script) → isTimelineRooted(node, timelineVar, script). No intermediate swallowing.
• ✅ Round-trip test at packages/core/src/parsers/gsapParser.inline.test.ts:101-107 intact: dedup-on-second-add and clean-removal on window.__timelines["scene"] inline form.

R3-NEW findings

None. The diff is debug-log cleanup on the studio side, well outside the acorn / parser / round-trip surface.

Carry-overs (still non-blocking, unchanged from R2)

• 🟡 Mixed-quote authored match — pre-existing, low blast radius.
• 🟡 Empty-inline-timeline insertion anchor — pre-existing.

Out of scope for this lane

HF-runtime-interop, SDK/player semantics, and runtime sign-off defer to https://github.com/vance-heygen|@vance-heygen per HF lane split.

R3 verdict: ✅ approve (canonical / test-coverage lane).

R3 verification by Rames D Jusso

[vanceingalls]

PR #1760 R3 — feat(gsap): read timelines authored inline (acorn read path)

Verdict: LGTM-sustained — R3 is a pure-rebase pickup, no semantic change in PR scope.
Reviewed at 473fa7b2 against R2 base 031f52f4 · Stack: eg-03-inline-timeline → eg-02-groups-ui
Lane: runtime-interop / SDK / player semantics

---

Delta classification

gh api compare/031f52f4...473fa7b2 reports status: "diverged", ahead_by: 5, behind_by: 5, merge-base eed9e1dc. Triggered a per-file blob-SHA check across the 7 PR files at both tips:

SAME  packages/core/src/parsers/gsapParser.ts
SAME  packages/core/src/parsers/gsapParserAcorn.ts
SAME  packages/core/src/parsers/gsapWriterAcorn.ts
SAME  packages/studio/src/components/editor/GsapAnimationSection.tsx
SAME  packages/core/src/parsers/gsapParser.inline.test.ts
SAME  packages/core/src/parsers/gsapParserAcorn.inline.test.ts
SAME  packages/core/src/parsers/gsapWriterAcorn.inline.test.ts

Every file in PR-1760's scope is byte-identical between R2 and R3 (additions: 488, deletions: 67, changed_files: 7 unchanged). The "delta" surfaced by compare is the base branch eg-02-groups-ui advancing 5 commits — f4090b8d fix(studio): hoverable group interior + non-sticky drill-in plus four siblings — which are NOT in this PR. They review under their own PRs in the stack.

The verification-vs-action snapshot drift family (feedback_verify_r2_nits_across_stack) is the right lens here: this is the intra-stack base-advance shape, not a content change. R2 verification stays valid because the headRefOid changed only via rebase, and every file blob-SHA in scope is stable.

R1 → R3 carry-over status

• R1 inline label-write asymmetry: ✅ resolved at R2, ✅ still resolved at R3 (gsapWriterAcorn.ts blob-stable).
• R1 isTimelineRooted MemberExpression case: ✅ still present (l.277-285).
• R1 extractGsapLabels member-aware: ✅ still present (gsapParserAcorn.ts:1271-1280).
• R1 5/5 items addressed: ✅ stable.
• R2 non-blocking watchpoints (carry-overs from Rames):
  • 🟡 Mixed-quote inline timelines — would still under-match ("scene" decl vs 'scene' callee). Unchanged. Not blocking; canonical round-trip is single-quote style.
  • 🟡 Empty-inline-timeline insertion anchored at ast.end — findTimelineDeclarationStatement still walks VariableDeclaration only, falls back to ast.end for member-form empty timelines. Tests cover this case ("adds the first tween to an empty inline timeline") — behavior is correct for canonical fixture (assignment-as-last-statement). Unchanged. Not blocking.
  • 🟡 No direct extractGsapLabels inline-form read symmetry test — covered indirectly via the round-trip addLabel → addLabel → removeLabel test in gsapParser.inline.test.ts:101-112. Unchanged. Not blocking.

No new concerns introduced by R3 (because there are no new changes in scope).

CI

@473fa7b2: Preflight (lint + format) ✅, Detect changes ✅, player-perf ✅, preview-regression ✅, Perf (load/scrub/parity/drift/fps) all ✅, Preview parity ✅. Regression-shards 1-8 in flight (status: in_progress, conclusion: null) — same flight-pattern Rames noted at R2. No failure signal.

Cross-reviewer state at SHA

Rames last review (2026-06-27T04:31:41Z) was at R2 SHA 031f52f4, not R3 — he hasn't re-verified at the new HEAD. Given the blob-identity finding, his R2 LGTM (canonical lane) carries forward unchanged.

Verdict

• Blockers: none.
• New concerns: none.
• R1 asks: 5/5 still addressed.
• Stamp posture: ready. Awaiting Vai for the stamp click per default routing.

R3 by Via

[github-actions]

Fallow audit report

Found 16 findings.

Duplication (6)

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)

Health (10)

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)
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.