fix(sdk): include template GSAP scripts in resolver parity

[vanceingalls]

What

Fix SDK resolver-parity false positives for GSAP animations declared inside composition templates.

The SDK already models elements inside <template data-composition-id> sub-compositions, but GSAP script discovery only scanned top-level <script> tags. As a result, animation IDs from template-contained scripts were missing from both element snapshots and getAllAnimationIds().

Why

Resolver-shadow telemetry showed animation_not_found divergences for otherwise valid animation IDs, including unresolved and position-derived GSAP IDs. This was especially visible in compositions using nested/template-based content.

The SDK and Studio preview use template contents as part of the live composition, so script discovery must use the same traversal boundary.

How

• Added deep GSAP script discovery through data-composition-id templates.
• Preserved plain <template> behavior for runtime clone sources.
• Applied all discovered scripts when populating per-element animationIds.
• Made getAllAnimationIds() union IDs across all discovered GSAP scripts.
• Kept the existing first-script behavior for write-path operations.

Test plan

• Added a regression test for GSAP animations declared inside a composition template.
• Full SDK test suite: 457 tests passed.
• SDK typecheck passed.
• Oxlint and oxfmt passed.
• Manual testing
• Documentation updated: not applicable

[miga-heygen]

Review — #2189 fix(sdk): include template GSAP scripts in resolver parity

What this does

GSAP script discovery only scanned top-level <script> tags, so animation IDs from scripts inside <template data-composition-id> sub-compositions were missing from both element snapshots and getAllAnimationIds() — causing animation_not_found resolver-parity false positives.

Fix: findScriptElementsDeep walks the DOM tree and descends into composition templates (but NOT plain <template> runtime clone sources). getGsapScripts uses this traversal and returns all matching script texts. Both buildAnimationIdMap and getAllAnimationIds now iterate over ALL discovered scripts rather than just the first.

SSOT analysis

This PR actually improves SSOT by consolidating script discovery:

Before (3 separate traversals, inconsistent markers):

• extractGsapScript (document.ts) — flat querySelectorAll("script"), checked gsap + __timelines + ScrollTrigger
• findGsapScriptElement (model.ts) — flat querySelectorAll("script"), checked gsap + ScrollTrigger (missing __timelines!)
• buildAnimationIdMap — delegated to extractGsapScript (single script only)

After (shared traversal, aligned markers):

• findScriptElementsDeep (model.ts) — single deep traversal, composition-template-aware
• getGsapScripts (model.ts) — uses shared traversal, canonical marker check
• extractGsapScript — now just getGsapScripts(doc)[0] ?? null (first-script for write path)
• findGsapScriptElement — uses shared traversal, marker check now includes __timelines (fixed)
• buildAnimationIdMap — iterates all scripts via getGsapScripts
• getAllAnimationIds — unions IDs across all scripts via getGsapScripts

The __timelines addition to findGsapScriptElement is a quiet fix — compositions that registered timelines without referencing gsap or ScrollTrigger by name would have been invisible to the write-path script finder before this PR.

One nit

The GSAP marker predicate (text.includes("gsap") || text.includes("__timelines") || text.includes("ScrollTrigger")) still appears in two places: getGsapScripts (line ~101) and findGsapScriptElement (line ~113). They're always going to need to agree. A tiny isGsapScriptBody(text) helper would make that canonical — but it's one line, both are in the same file, and they serve different return types (text[] vs Element), so this is a nit, not a blocker.

CI

SDK tests passed. Fallow audit, Windows render/tests, and global install smoke are failing — these look infrastructure-related (Windows CI on HF is frequently flaky per prior reviews). Core checks (Lint, Format, SDK, Runtime contract) are green; Build, Test, Typecheck still pending.

No blockers. Ship it.

— Miga

[james-russo-rames-d-jusso]

Reviewed at 46602d75.

Traced the fix through the resolver-shadow call graph: sdkResolverShadow.ts runs getAllAnimationIds() + per-element animationIds against the SDK's parsed document, both of which are now unioned across all template-discovered scripts via getGsapScripts. Consistent with the marker triple gsap / __timelines / ScrollTrigger that findGsapScriptElement also picked up (findGsapScriptElement's pre-PR marker set was missing __timelines, so this PR also silently fixes a real asymmetry between it and document.ts:extractGsapScript — worth noting in the body, since it's a real bug pre-PR, not just a refactor). linkedom's template.children exposing children directly (unlike browser DOM) is what makes the walk(child) recursion work — matches the pattern in packages/parsers/src/hfIds.ts:126-132 and is explicitly Node-only surface. Regression test covers the primary bug case cleanly.

Three non-blocking observations:

🟠 Sibling parity gap: getElementTimings still reads a single script. packages/sdk/src/session.ts:333 does getGsapScript(this.parsed.document) (singular) and feeds that into extractGsapLabels, which extracts tl.addLabel("name", position) calls used to resolve data-start="labelName + N" references. If a composition defines its labels inside a <template data-composition-id> sub-comp (the same pattern this PR is fixing on the animationIds axis), those labels never reach allLabels, so any element referencing them via data-start will fall to the parseFloat NaN path and effectively resolve to 0. Structurally the same shape as the primary bug — top-level-only script discovery — just on the timing-labels axis rather than the animation-ids axis. Doesn't show up in the resolver-shadow's animation_not_found telemetry (that measures ID resolution, not label resolution), but arguably part of the "SDK models elements inside template sub-compositions" claim in the PR body. Two shapes: (a) extend the fix to loop template-discovered scripts here too — cache invalidation gets a bit noisier since _gsapLabelCache keys on script text, but it's still solvable — or (b) scope the PR body claim explicitly to "animation IDs" and file the labels case as a follow-up.

🟡 DRY: marker triple duplicated. packages/sdk/src/engine/model.ts:400 (getGsapScripts filter) and packages/sdk/src/engine/model.ts:407 (findGsapScriptElement inline check) both hardcode the same text.includes("gsap") || text.includes("__timelines") || text.includes("ScrollTrigger") triple. A future marker addition (e.g. SplitText, ScrollSmoother) will need to be applied in two places — exactly the shape of the pre-PR asymmetry that this PR is (silently) fixing on __timelines. Cheap to extract to an isGsapScriptText(text: string): boolean predicate and have both call sites go through it.

🟡 Sibling extractGsapScript in the parsers package untouched. packages/parsers/src/htmlParser.ts:864 still uses doc.querySelectorAll("script") (which per linkedom's semantics does not descend into templates, same reason as the SDK bug this PR is fixing) and is called by validateComposition at packages/parsers/src/htmlParser.ts:850. Compositions whose only GSAP script is inside a data-composition-id template will skip GSAP validation entirely — no error, no warning, just quiet skip. If validation is intended to see template scripts too (my read is that it should, since the served composition unwraps them), it needs the same lift. If not, worth a comment there explaining that the validator's scope is deliberately narrower than the resolver's.

LGTM from my side — leaving as a comment. Primary bug is fixed cleanly and the test pins it.

— Review by Rames D Jusso

[vanceingalls]

Addressed in commit a61f7de8:

• getElementTimings() now parses labels from every discovered GSAP script, including scripts inside composition templates, with an ordered multi-script cache and regression coverage.
• The SDK and parser now share composition-template-aware DOM traversal, with browser HTMLTemplateElement.content and linkedom support.
• Parser GSAP extraction, validation, and removal now see scripts inside composition templates; added parser regression coverage.
• Centralized the GSAP marker predicate used by both read and write-path script discovery.
• Split buildAnimationIdMap() matching into a helper, clearing the new fallow complexity finding.

Verification: 48 parser tests passed, 53 focused SDK/template tests passed, SDK and parser typechecks passed, oxlint passed, oxfmt passed, and fallow reports no new issues. The remaining timing-resolver clone is inherited and excluded by the audit gate.

[james-russo-rames-d-jusso]

Reviewed delta 46602d75 → a61f7de8.

All three R1 items addressed cleanly, with test coverage on each:

• getElementTimings labels axis — session.ts now unions labels across every discovered GSAP script (scripts.flatMap(script => extractGsapLabels(script))), and the cache key upgraded from a single script string to an ordered list with length + element-wise equality — so a template script rename, reorder, or addition invalidates the parse correctly. Cache clone via [...scripts] is a nice defensive touch. Regression test in session.timings.test.ts pins the exact case (addLabel("template-label", 1.5) inside a data-composition-id template resolves via getElementTimings()).

• DRY marker triple — isGsapScriptText extracted in engine/model.ts; both getGsapScripts and findGsapScriptElement route through it. Marker additions are now a one-line change.

• parsers/htmlParser.ts sibling — extractGsapScript (singular) → extractGsapScripts (plural), all three call sites (parseHtml, cascadeRemoveGsapById, validateCompositionHtml) now use findScriptElementsDeep — mirrored to walkCompositionDescendants in the parsers package for the shared traversal. Three tests added: parseHtml discovers template scripts, removeElementFromHtml strips tweens from template scripts, and validateCompositionHtml validates template GSAP. Full triangle.

Bonus catch worth naming: the new getChildElements(parent) helper in parsers/hfIds.ts falls back to template.content.children when parent.children is empty on a template — so the shared walker is safe under browser DOM semantics too, not just linkedom's flatter template model. I didn't flag that in R1, but it hardens the previously Node-only assumption should any browser-side consumer ever adopt the walker. Nice preemptive lift.

Consolidating the walker itself (SDK's findScriptElementsDeep and parsers' findScriptElementsDeep both delegating to the exported walkCompositionDescendants) is exactly the right shape — one place to keep the "descend through composition templates, stay out of plain templates" invariant. Any future adjustment lands once.

LGTM from my side — leaving as a comment. Clean R2.

— Review by Rames D Jusso

[miga-heygen]

Re-review — #2189

Significant and welcome changes since v1. The new commit a61f7de8 fix(sdk): align template script traversal addresses my nit and goes further:

1. Shared traversal extracted to parsers — walkCompositionDescendants now lives in packages/parsers/src/hfIds.ts as a proper export, with a getChildElements helper that handles linkedom's template content access. Both the SDK (findScriptElementsDeep in model.ts) and the parsers (findScriptElementsDeep in htmlParser.ts) now delegate to it — single traversal owner, no duplication.

2. isGsapScriptText predicate extracted — my nit about the duplicated marker check is now resolved. Both getGsapScripts and findGsapScriptElement use the same isGsapScriptText(text) helper.

3. Parser coverage extended — parseHtml, cascadeRemoveGsapById, and validateCompositionHtml all now discover scripts from composition templates via the shared traversal. Three new parser tests confirm: GSAP script extraction, tween stripping, and onComplete validation all work through composition templates.

4. Label cache updated to multi-script — _gsapLabelCache in session.ts now tracks an array of scripts instead of a single script, with proper ordered comparison for cache validity. Labels are extracted via flatMap across all scripts.

5. appendAnimationIdsForSelector extracted — the inner loop of buildAnimationIdMap is now a named helper, making the multi-script iteration cleaner.

6. getGsapScript (singular) removed from imports — session.ts now only imports getGsapScripts (plural). Clean.

SSOT is better than v1:

• One traversal implementation (walkCompositionDescendants in parsers)
• One GSAP marker predicate (isGsapScriptText in model.ts)
• Both SDK and parsers converge on the same traversal

CI all green (34 checks passing). No blockers. Ship it.

— Miga

[miguel-heygen]

Final approval pass at current head a61f7de8d79255da5a79730959a8ac5614415db9.

Re-verified the R2 fixes: shared composition-template traversal, canonical GSAP marker predicate, multi-script label cache/invalidation, parser extraction/validation/cascade coverage, and browser-template child fallback. No new drift; all required checks pass and no unresolved review threads remain.

Residual notes: none blocking.

[jrusso1020]

APPROVE ✅

Verified current head a61f7de8d (the R2-reviewed head, no post-review push): CI green, mergeStateStatus CLEAN. The three R1 items are all addressed with test coverage: getElementTimings labels now union across template scripts (cache key upgraded to an ordered script-list eq check), isGsapScriptText extracted to dedupe the marker triple, and extractGsapScripts (plural) routed through the shared walkCompositionDescendants walker across parseHtml / cascadeRemoveGsapById / validateCompositionHtml — plus the browser-template getChildElements fallback. Clean SSOT improvement over v1; thoroughly reviewed by Miga + Rames-D across R1/R2.

Approving per Vance's request. — Rames Jusso