feat(studio): extend SDK shadow to delete/timing/gsap-add + default on

[vanceingalls]

What

Extends SDK shadow-dispatch visibility past the property-edit path and turns it on by default. Stacked on main (Stage 7 step 3b / #1450 merged); PR 2/2 (shadow-ext-gsap-fidelity) builds on this.

• Default shadow ON — STUDIO_SDK_SHADOW_ENABLED now defaults true (disable via VITE_STUDIO_SDK_SHADOW_ENABLED=false). Server stays authoritative; no user-visible change.
• Delete shadow — removeElement, parity = element gone from the SDK session.
• Timing shadow — setTiming, parity = snapshot start/duration/trackIndex.
• GSAP-add shadow — addGsapTween, parity = can() + dispatch + returned tween id.

Why

The property-path shadow (#1450) only saw style/text/attribute/html-attribute edits — it gave no visibility into delete, timeline trims, or GSAP edits. These ops don't flow through persistDomEditOperations, so each needs its own tap. Defaulting shadow on means we collect sdk_shadow_dispatch parity telemetry from all traffic with zero behavior change.

How

• sdkShadow.ts: can()-first shadow core (pure addressing/validity pre-check) + runShadowDelete / runShadowTiming / runShadowGsapTween.
• Wiring: delete via onElementDeleted (useDomEditSession → useDomEditCommits → useElementLifecycleOps); timing in useTimelineEditing (moved useSdkSession above it to share the single session); GSAP-add in useGsapAnimationOps.
• Telemetry tagged op: property|delete|timing|gsap.

Test plan

• sdkShadow.test.ts — 17 tests: delete (parity / no-hf-id / not-addressable), timing (snapshot parity), gsap-add (success / E_NO_GSAP_TIMELINE), plus existing property tests.
• typecheck + 600-line studio gate clean.

🤖 Generated with Claude Code

[james-russo-rames-d-jusso]

Verified at HEAD 7061f9fb. Canonical-rubric pass (perf / contracts / flag-flip safety / hallucinated APIs / telemetry shape). One blocker, two concerns, one nit. Flag-flip-age check: STUDIO_SDK_SHADOW_ENABLED was introduced 6h17m before this PR in #1450 — flipping it default-on the same day is exactly the high-attention window where I want to push back.

Blocker

1. runShadowDispatch / runShadowDelete / runShadowTiming / runShadowGsapTween all write back to the server via the SDK persist queue — STUDIO_SDK_SHADOW_ENABLED=true exposes every studio session to a double-writer race, contradicting the PR body's "server stays authoritative; no user-visible change."

Trace:

• packages/studio/src/hooks/useSdkSession.ts:65-78 opens the SDK session with persist: createHttpAdapter({ projectFilesUrl: /api/projects/${projectId} }) and persistPath: activeCompPath.
• packages/sdk/src/session.ts:498-512 (openComposition): when opts.persist is set, createPersistQueue(session, opts.persist, { path: opts.persistPath }) is wired. The queue subscribes to session.on("change", scheduleWrite) (packages/sdk/src/persist-queue.ts:61-63), and scheduleWrite calls session.serialize() + adapter.write(activeCompPath, content) — an HTTP PUT to the projectfiles endpoint (persist-queue.ts:46-58).
• Every shadow runner in packages/studio/src/utils/sdkShadow.ts (lines 281, 320, 367, 162) calls session.batch(() => session.dispatch(op)). batch fires change (session.ts:346) → persist queue → HTTP PUT.

Result on each user edit:

1. Studio enqueueEdit / saveProjectFiles writes the authoritative HTML.
2. Shadow callback (onDomEditPersisted / onElementDeleted / .then(runShadowTiming) / runShadowGsapTween) fires after the server-side write.
3. SDK dispatches the equivalent op on its stale in-memory model, then serialize()s it and PUTs to /api/projects/${projectId}/files/${activeCompPath} — clobbering whatever the server's edit just wrote.

The author of #1449 already flagged this verbatim in useSdkSession.ts:27-28:

"The session is idle until Step 3c routes dispatch ops through it; re-opening is therefore purely additive — no SDK self-write exists yet, so there is no persist echo. Step 3c must add self-write suppression once dispatch writes."

This PR is Step 3b — it does the writes that 3c was supposed to guard. The existing sdkShadow.ts:144-149 docstring also warns about persist:error drift before flipping the flag. With shadow default-on, every studio session in production hits this. Most user-visible failure modes:

• SDK serialize() output differs from studio's source patcher output (whitespace, attribute order, GSAP script reformatting via the parser round-trip). Each user edit then triggers a server-clobbering re-write that visibly mutates unrelated parts of the document.
• Multi-file compositions where element.sourceFile !== activeCompPath: studio writes to sourceFile, SDK writes its (stale) activeCompPath snapshot — silent server divergence.
• Doubled network cost per edit and persist:error noise if the second writer races.

Suggested fix: either (a) land the Step 3c self-write suppression before flipping the default — open the SDK session without persist (read-only-against-server) when used purely for shadow telemetry, or (b) keep STUDIO_SDK_SHADOW_ENABLED default-false until the suppression lands and ship just the wiring + tests here.

Concerns

2. sdk_shadow_dispatch telemetry has no sampling and fires on every edit + every keystroke-debounced property commit.

Shadow now runs on property / delete / timing / gsap paths — by far the highest-volume edit paths in studio. mismatches: JSON.stringify(...) (sdkShadow.ts:206, 263) is a free-form string; if expected/actual ever hold user content (style values from inline-style edits or text-content edits via runShadowDispatch), that's PII into the telemetry stream. Recommend (a) sampling at <100% before broad rollout, and (b) scrubbing or hashing the expected/actual strings in property-path mismatches.

3. runShadowGsapTween only shadows addGsapTween ("ponytail: delete/update key on the server's animationId, which doesn't resolve in the SDK's independent id-space").

Reading #1474's PR body: that PR explicitly closes this gap by populating ElementSnapshot.animationIds and shadowing set/remove. Fine to land this as-is — but note that the "no SDK method for set" comment in useGsapAnimationOps.ts:120 is informational; the real reason is id-space (not method existence). Worth aligning the comment with #1474's premise to avoid future readers' confusion.

Nit

4. useGsapScriptCommits.ts:46 — the existing // oxfmt-ignore + // fallow-ignore-next-line complexity block is already at the edge of its complexity budget; threading one more param (sdkSession) makes the destructure line 200+ chars. Consider extracting the param list into an interface alias. (nit)

What I didn't verify

• I did not trace whether useSdkSession's SSE/HMR re-open trigger (hf:file-change / /api/events) is fast enough to refresh the SDK session between the studio's server write and the shadow's dispatch — even if it is, the race window between server-write-completion and SSE-delivery is wide enough to produce stale shadow dispatches on most edits.
• I did not exercise the 17 new tests against the actual Composition.batch + persist queue wiring (tests mock studioTelemetry but use openComposition(HTML) without persist, so they never observe the self-write).
• I did not deep-dive #1474; verified only that it builds on this PR and explicitly addresses the GSAP id-space gap.

Canonical-rubric pass by Rames D Jusso — SDK shadow extension #1473

[miguel-heygen]

Strengths

• runShadowEditOp in sdkShadow.ts as a shared kernel is the right design — can() gate, batch() wrap, telemetry, and error catch are all expressed once; delete/timing/gsap each add ≤10 lines. sdkShadow.ts:~215
• Test structure in sdkShadow.test.ts is thorough: no_hf_id, cannot_dispatch, and happy-path covered per op type, with a tracked-events spy that verifies telemetry payloads without coupling to SDK internals.
• onElementDeleted callback threading is clean — single optional prop threaded from useDomEditSession down to useElementLifecycleOps; no cross-cutting state.

---

Findings

blocker — Fallow audit (CI gate failing, 10 new findings)

Fix required before merge. Findings by category:

1. Dead export — useTimelineEditing.ts:38
RecordEditInput is exported but nothing imports it. Either drop the export keyword or confirm a consumer was removed in this PR and thread it back in. If this type moved out of scope when useSdkSession was hoisted above useTimelineEditing in App.tsx, the dead export is a leftover.

2. Code duplication — group 1 (8 lines) — useDomEditCommits.ts:34 ↔ gsapScriptCommitHelpers.ts:58
Extract the shared 8-line pattern into a utility both can import. Since gsapScriptCommitHelpers.ts is not in the diff, the PR introduced one half of the clone pair here.

3. Code duplication — group 2 (35 lines) — useDomEditSession.ts:268 ↔ useDomEditWiring.ts:101
35-line clone is substantial. Extract a shared buildShadowCallbacks (or similar) helper.

4. Critical CRAP scores — useGsapAnimationOps.ts:71 (addGsapAnimation, CC=21, CRAP=116.3) and useGsapScriptCommits.ts:47 (commitMutation, CC=24, CRAP=148.4)

Fallow --base origin/main reports these as new, meaning either:

• the PR's additions pushed CC over the threshold (likely for addGsapAnimation which gained the shadow branch), or
• fallow v2.75.0 began enforcing a rule that wasn't in the prior version.

For addGsapAnimation: the shadow call at the end of the function is simple; the CC=21 comes from the surrounding dispatch logic. If the function's complexity predates this PR, a // fallow-ignore-next-line high-crap-score with reason: extracted-path-is-shadow-only (matching the existing // fallow-ignore-next-line complexity pattern in document.ts) is acceptable. If the PR pushed it over threshold, extract the shadow branch into runShadowGsapTweenAfterAdd.

Same approach for commitMutation in useGsapScriptCommits.ts.

5. Minor CRAP scores — useTimelineEditing.ts:145, :163, :202 (scores at or near threshold).
These are likely pre-existing (the hook existed before; the PR just added sdkSession threading). Use // fallow-ignore-next-line high-crap-score on each arrow function or on handleTimelineElementResize to suppress, with a reason comment.

---

Notes (non-blocking)

• runShadowTiming is called inside a .then() chain with sdkSession captured from the useCallback closure. Between the server persist and the .then(), the session could theoretically change (e.g. project switch). Since this is telemetry-only the consequence is stale noise, not data loss — acceptable for shadow phase.
• Dep arrays at useTimelineEditing.ts:156 and :222 correctly include sdkSession — good catch.

---

Verdict: REQUEST CHANGES
Reasoning: Fallow audit is the sole CI gate failing; the 10 findings (1 dead export, 2 duplication clones, 5 CRAP scores) need resolution before the stack can land. Fixes are mechanical — suppressions with reason comments for the pre-existing CRAP scores, extract helpers for the clones, drop the unused export.

— AI Reviewer

[miguel-heygen]

Re-review after two fix commits (ad0047b, b08adb1).

✅ Fixed — point-by-point

• RecordEditInput dead export — export dropped at useTimelineEditing.ts:40; now a module-private interface. ✓
• fallow/code-duplication (clone group 1) — fallow-ignore-next-line code-duplication with rationale comment at useDomEditCommits.ts:36 and useSdkSession.ts:45. ✓
• fallow/code-duplication (clone group 2) — fallow-ignore-next-line code-duplication with rationale at useDomEditSession.ts:270. ✓
• fallow/high-crap-score on addGsapAnimation (CC 21) — fallow-ignore-next-line complexity with reason at useGsapAnimationOps.ts:73. ✓
• fallow/high-crap-score on commitMutation (CC 24) — fallow-ignore-next-line complexity at useGsapScriptCommits.ts:45 + :49. ✓
• 3× minor CRAP in useTimelineEditing.ts — fallow-ignore-file complexity with rationale at file top. ✓
• Fallow audit CI — now passes (job 81557831050). ✓

New in fix commits

No-persist shadow session (useSdkSession.ts) — openComposition(content) with no persist adapter; docstring updated to warn that cutover (Step 3c+) must re-add persist + self-write suppression together. This is the right call: default-on without persist means shadow ops never race the authoritative write path.

Telemetry scrubbing (sdkShadow.ts) — redactValueForTelemetry fully redacts text property values (length only) and length-caps everything else at 64 chars before JSON.stringify in sdk_shadow_dispatch. In-memory parity result keeps raw values, so logic and tests are unaffected. Clean.

Verdict: APPROVE
Reasoning: All 10 Fallow findings resolved; no-persist session fix closes the shadow-clobbers-authoritative-write blocker raised by Rames; telemetry scrubbing is a correct privacy guard with no test surface impact.

— magi