feat(sdk,studio): populate animationIds; shadow GSAP update/delete + value fidelity

[vanceingalls]

What

PR 2/2 of the shadow-extension stack (on top of #1473). Closes the GSAP shadow gaps.

• SDK: populate ElementSnapshot.animationIds (was a hardcoded [] stub) from the acorn parser's located tweens, resolving each tween's targetSelector to element hf-ids.
• GSAP update/delete shadow — deleteGsapAnimation → removeGsapTween, updateGsapMeta → setGsapTween, using the server's animationId directly.
• GSAP value fidelity — runShadowGsapFidelity: open a fresh SDK doc from the server's pre-op file, apply the same op, serialize, and structurally diff the SDK's GSAP script against the server's resulting script.

Why

The premise that server and SDK had separate id-spaces was wrong: T6e moved the studio-api read path to the same acorn parser the SDK uses, and both derive tween ids as targetSelector-method-position. So the server's animationId is dispatchable in the SDK as-is — unblocking update/delete shadow. Populating animationIds makes the snapshot truthful and gives real existence parity. Value fidelity (duration/ease/position/properties) needs comparing the two writers' output, since the SDK has no per-tween property reader.

How

• document.ts: buildAnimationIdMap parses the GSAP script and maps targetSelector → hf-id → tween ids; threaded into buildElement.
• sdkShadow.ts: gsapFidelityMismatches (re-parses both scripts, diffs per-field — formatting never causes false positives) + runShadowGsapFidelity.
• Wired at the commitMutation chokepoint (only place with the server's before+after scripts); handlers pass the typed op via CommitMutationOptions.shadowGsapOp. Telemetry op: gsap_fidelity (complements op: gsap existence).

Test plan

• SDK: session.test.ts — animationIds populated, untargeted empty, populated id dispatchable via removeGsapTween (221 total pass).
• Studio: sdkShadow.test.ts — add/remove via animationIds parity, gsapFidelityMismatches (identical / value drift / tween presence), runShadowGsapFidelity (match / divergence) — 23 tests.
• typecheck + studio gate clean.

🤖 Generated with Claude Code

[james-russo-rames-d-jusso]

Verified at HEAD 51abc5e89cd492fb683ad1326e277bbddef530b7. Canonical-rubric pass on the stacked PR (on top of #1473).

T6e premise — VERIFIED (load-bearing)

Both id-generators implement assignStableIds byte-identically:

• SDK / browser: packages/core/src/parsers/gsapParserAcorn.ts:1030-1044 — ${targetSelector}-${method}-${posKey}${groupSuffix} with posKey = Math.round(position * 1000).
• Server / legacy recast path: packages/core/src/parsers/gsapParser.ts:1068-1082 — same shape, same posKey quantization, same collision-counter (-${count} suffix when count > 1).
• studio-api read path is on acorn now: packages/core/src/studio-api/routes/files.ts:27 imports parseGsapScriptAcorn — both getCompositionAnimations (524) and bulk-update validators (587, 1171, 1236) read through it. The recast parseGsapScript is gone from the read path.

So server.animationId === sdk.animationId for any tween with the same (targetSelector, method, position, propertyGroup) and a shared pre-op file. The PR's premise holds; update/delete shadow via server's id is structurally sound.

What I checked structurally

• buildAnimationIdMap (packages/sdk/src/document.ts:66-87): multi-match selectors (e.g. .button → many) correctly fan tween ids out to every matched element. Unparseable selector → skipped via try/catch around querySelectorAll. Unparseable script → empty map → animationIds: [] (truthful stub fallback).
• gsapFidelityMismatches (sdkShadow.ts:629-682): per-field, set-symmetric-difference over ids; canonicalProps sorts keys + coerces numeric strings ("0.5" → 0.5); numericEqual covers position/duration writer differences. Tests at sdkShadow.test.ts:398-435 lock both edge cases (key-order, number-vs-string-equivalent). The diff is structurally correct — won't false-positive on writer formatting.
• removeGsapTween / setGsapTween / addGsapTween exist on Composition (packages/sdk/src/types.ts:237-239, session.ts:134-145). No hallucinated API.
• Telemetry dims: op: gsap (existence) and op: gsap_fidelity (value) are distinct. No dashboard collision with #1473's gsap op — same op string, but they fire from different code paths and runShadowGsapFidelity adds mismatches payload that existence doesn't.

Concerns (non-blocking)

1. shadowGsapOp coverage gap on property-level edits. The chokepoint is wired in useGsapScriptCommits.ts:70, but only useGsapAnimationOps.ts (createGsap / updateGsapMeta / deleteGsap) actually passes shadowGsapOp. useGsapPropertyDebounce.ts (update-property, add-property, remove-property, update-from-property, add-from-property, remove-from-property) and useGsapKeyframeOps.ts all pass through commitMutationSafely with NO shadowGsapOp — so the property-scrub path (the most frequent edit + the one most likely to surface writer divergence on properties / fromProperties) is silently un-shadowed. The PR body's "value fidelity (duration/ease/position/properties)" claim is honored at the meta level but not at per-property. Suggest either (a) a follow-up PR enumerating the property handlers and synthesizing shadowGsapOp: { kind: "set", ... } from { animationId, property, value }, or (b) a comment at the property handlers documenting the deliberate scope omission so the next reader doesn't read the gap as a bug.

2. Module-level cache state in document.ts:46-47. gsapLocatedCacheKey / gsapLocatedCacheVal are module-scoped mutable singletons across the entire SDK realm. Two parallel compositions with different script texts will thrash (each buildRoots invalidates the other's entry) — bounded but no longer "covers the hot path" the comment promises. Bigger latent issue: if the SDK is ever consumed from a Worker pool or repeated buildRoots on different docs concurrently, the single-entry cache becomes a correctness footgun (parsed structures captured against the wrong script text are unlikely because the key check is by reference-equality on the script string, but it's a sharp edge). Lifting the cache onto the Document (WeakMap keyed by script string) or onto the SdkDocument instance would be both faster (no cross-comp thrash) and structurally safer. (nit-leaning concern.)

3. runShadowGsapFidelity open-cost. openComposition(beforeHtml) per edit + two parseGsapScriptAcorn calls per diff. Fire-and-forget void, gated by STUDIO_SDK_SHADOW_ENABLED + shadowGsapOp presence, so the prod-default-on flag in #1473 means this runs on every meta-level edit. Fine for the current handler set (low-rate add/delete + a debounced update-meta), but worth a back-of-envelope before broadening to property scrubs — the budget could double or triple. No action this PR.

Nits

• isGsapScriptBody duplicated logic. sdkShadow.ts:422 and document.ts:101 both need the same marker set (gsap / __timelines / ScrollTrigger). The comment on each says "must match the other" — true today but bit-rot-prone. A shared core/gsap-script-detection.ts helper would lock the invariant. (nit.)
• runShadowGsapFidelity extractGsapScript is regex-based on raw serialize() HTML, while document.ts uses linkedom + DOM querySelectorAll("script"). Both work for the test fixtures, but a <script> with attributes containing </script> text or nested CDATA would diverge. Not a real scenario for GSAP scripts, but worth knowing if the script-emission shape ever changes. (nit.)

Test coverage

23 new tests on sdkShadow.test.ts + 3 on SDK session.test.ts covering the load-bearing premise (id dispatchable as removeGsapTween target — session.test.ts:451-456), untargeted-empty (session.test.ts:446-449), and the writer-formatting false-positive guards (sdkShadow.test.ts:415-434). Missing: multi-tween-per-element coverage (one element with two tos and one from) and shared-selector cross-element coverage — both follow from the implementation but aren't asserted. Would be nice to lock down in a follow-up.

Recommendation

Greenlight from me. The T6e architectural unification is real and validated, the value-fidelity diff is canonical (writer-formatting won't false-positive), and the existence-parity logic is tight. Concern 1 (property-handler coverage gap) is the only one with a real product implication — flag for the author to confirm intent (deliberate scope-limit vs follow-up).

Tagging @rames-d-jusso for stamp if the author confirms the property-handler coverage is intentional and there are no other blockers from peer reviewers.

Canonical-rubric pass by Rames D Jusso — SDK shadow GSAP fidelity #1474, stacked on #1473

[miguel-heygen]

Stacked on #1473 — blocked on that PR landing. CI here is CLEAN; this review covers the diff above the base.

Strengths

• gsapFidelityMismatches is the right abstraction: purely functional, takes two script strings, returns per-field diffs. Completely testable in isolation from any HTTP call or session state.
• Canonical compare (numericEqual, key-order-sorted canonicalProps) is good defensive engineering — the server writer and SDK writer can disagree on 1 vs "1" and key insertion order without generating false-positive mismatches. Test cases for both cases are solid. sdkShadow.ts:~440
• runShadowGsapFidelity is correctly fire-and-forget (void at callsite, sdkShadow.ts line cited in useGsapScriptCommits.ts:68) and doesn't block the user path. Server authoritative boundary is respected.
• Memoized parseLocatedCached in document.ts:~50 is a worthwhile optimization — acorn parse is the hot path on getElements() rebuilds and the cache is bounded.

---

Findings

important — gsapFidelityMismatches / runShadowGsapFidelity are exported but have no existence tests in the live session path

runShadowGsapFidelity opens a fresh composition with openComposition(beforeHtml), applies the op, serializes, and diffs. The two canonical-compare test cases in PR #1474's test additions (sdkShadow.test.ts:279–295) are good. However, there is no test asserting that runShadowGsapFidelity is actually called at the commitMutation chokepoint — i.e. that the wiring from useGsapScriptCommits.ts:68 through to telemetry fires on a real addGsapAnimation call. The runShadowGsapTween tests in the same file give existence coverage; runShadowGsapFidelity has integration tests but not wiring tests. Low stakes for shadow-only code, but worth noting before this path is used to gate anything.

nit — result.before != null && result.scriptText != null guards at useGsapScriptCommits.ts:68

The before and scriptText nullchecks are correct (both are string | null | undefined from CommitMutationResult). A comment explaining why scriptText can be null (no GSAP script in the composition) would help the next reader. One line, optional.

nit — parseLocatedCached single-entry cache key in document.ts:~55

The single-entry LRU is fine for the stated hot path. If a composition alternates between two script strings on rapid undo/redo the cache thrashes, but that's an unlikely hot path for shadow telemetry. Leave as-is.

---

Notes

• buildAnimationIdMap correctly handles missing script (returns empty map) and gracefully catches querySelector throws on malformed selectors. No change needed.
• Marker set alignment between document.ts:extractGsapScript and sdkShadow.ts:isGsapScriptBody (gsap || __timelines || ScrollTrigger) is explicit with a comment on both sides — good cross-file documentation.
• useSafeGsapCommitMutation.ts type-alias removal (local CommitMutationOptions/CommitMutation → shared import from gsapScriptCommitTypes.ts) is a clean dedup; the previously-shadowed shadowGsapOp field was the concrete bug this fixes.

---

Verdict: APPROVE (pending #1473)
Reasoning: Implementation is solid, tests are present for the new primitives, and the commitMutation wiring is correct. The one important finding is a wiring-test gap on runShadowGsapFidelity — low stakes for telemetry-only code but worth a follow-up before the path is used to gate anything.

— AI Reviewer

The base branch was changed.