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

* feat(sdk,studio): populate animationIds; shadow GSAP update/delete Closes the GSAP shadow gaps. The server's animationId was assumed to live in a separate id-space — it does not: the studio-api read path (T6e) and the SDK both derive tween ids as targetSelector-method-position from the same acorn parser, so server ids are dispatchable in the SDK as-is. SDK: populate ElementSnapshot.animationIds (was a hardcoded stub) from parseGsapScriptAcornForWrite().located, resolving each tween's targetSelector to element hf-ids. Makes the snapshot truthful and enables real GSAP parity. Studio: shadow deleteGsapAnimation (removeGsapTween) and updateGsapMeta (setGsapTween) using the server animationId directly. GSAP add/remove parity now verifies via animationIds (present after add, gone after remove). set is existence-only — the SDK still has no per-tween property reader (value fidelity would need serialize()-script round-trip diffing). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(studio): GSAP value fidelity via serialize round-trip diff Closes the last shadow gap: GSAP value fidelity. Existence parity confirmed a tween was created/removed but not that its values (duration/ease/position/ properties) matched the server, since the SDK has no per-tween property reader. runShadowGsapFidelity opens a fresh SDK doc from the server's pre-op file (result.before), applies the same typed op, serializes, and structurally diffs the SDK's GSAP script against the server's resulting script (result.scriptText). Both are re-parsed via parseGsapScriptAcorn, so formatting/whitespace never produces false positives — only real value drift does. gsapFidelityMismatches reports per-field drift and tween presence/absence. Wired at the commitMutation chokepoint (the only place with the server's before+after scripts); handlers pass the typed ShadowGsapOp via CommitMutationOptions.shadowGsapOp. Emits sdk_shadow_dispatch op:gsap_fidelity. Complements the existing live existence shadow (op:gsap). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(studio,sdk): address shadow code-review findings - gsapFidelityMismatches: canonical comparison (sort property keys, numeric- coerce position/duration/values). Server (addAnimationToScript) and SDK (gsapWriterAcorn) are different writers; non-canonical compare flagged key-order / number-vs-string differences as false value drift. - document.ts buildAnimationIdMap: memoize the acorn parse by script text (single-entry). getElements() invalidates on every dispatch, so shadow's frequent dispatches were re-parsing the full GSAP AST each rebuild. Selector resolution still runs per-call (depends on live DOM). - runShadowGsapFidelity: early-bail when serverScript/beforeHtml is empty — skip the costly openComposition. - useSafeGsapCommitMutation: import the shared CommitMutationOptions/ CommitMutation instead of a stale local duplicate (was missing shadowGsapOp). - align extractGsapScript marker set across sdkShadow.ts and document.ts (gsap || __timelines || ScrollTrigger) so both pick the same script. Tests: +2 canonical-compare cases (key-order, number-vs-string → no drift). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(studio,sdk): fallow gate for #1474 (fidelity diff + test clones) - suppress moderate CRAP on gsapFidelityMismatches and the runShadowGsapTween parity arrow (comparison/parity functions are inherently branchy) - suppress two pre-existing test clones in session.test.ts surfaced by the added animationIds tests (TestPreviewAdapter stub, selectionchange setup) Rebased onto the updated #1473 (no-persist shadow session); inherits the persist-race fix and prior fallow suppressions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(studio): extract GSAP fidelity to its own module (file-size gate) sdkShadow.ts hit 602 lines (CI File size check: max 600). Move the GSAP value-fidelity diff (gsapFidelityMismatches, runShadowGsapFidelity, and their private helpers) into sdkShadowGsapFidelity.ts; re-export from sdkShadow.ts so the import surface is unchanged. sdkShadow.ts now 430 lines. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(studio,sdk): address #1474 review feedback - CodeQL js/bad-tag-filter: the GSAP <script> extraction + test regexes now match </script\s*> (whitespace-before-close variant). 3 alerts resolved. - Wiring (Miguel): extract resolveGsapFidelityArgs — a pure, narrowing gate for the commitMutation chokepoint (no non-null assertions) — and unit-test the fire/skip conditions (session, op, before, scriptText). Replaces the inline guard so the wiring decision is covered without rendering the hook. - Property-handler scope (Rames): comment at the chokepoint documenting that only meta-level ops (add/update-meta/delete) carry shadowGsapOp today; per-property and keyframe handlers are a deliberate follow-up. Also why scriptText can be null. - Test coverage (Rames): multi-tween-per-element and shared-selector cross-element animationIds cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(studio): CodeQL js/bad-tag-filter — match </script[^>]*> close tags `</script\s*>` still tripped CodeQL on attribute-junk closes like `</script foo>` (HTML5 ignores junk before `>`). Widen the close-tag match to `</script[^>]*>` in the GSAP-script extraction and the test regexes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-09-03 04:38:33 +00:00 · 2026-06-15 22:40:28 -07:00
parent 5f6ced116d
commit 7593aac5ef
9 changed files with 570 additions and 64 deletions
@@ -4,8 +4,12 @@ import {
  runShadowDelete,
  runShadowTiming,
  runShadowGsapTween,
+  runShadowGsapFidelity,
+  gsapFidelityMismatches,
+  resolveGsapFidelityArgs,
  SdkShadowMismatch,
 } from "./sdkShadow";
+import type { ShadowGsapOp } from "./sdkShadow";
 import type { PatchOperation } from "./sourcePatcher";
 import { openComposition } from "@hyperframes/sdk";

@@ -219,13 +223,24 @@ describe("runShadowTiming", () => {
 });

 describe("runShadowGsapTween", () => {
-  it("dispatches add against a real timeline and reports success", async () => {
+  it("add reports success and the new tween lands on the target's animationIds", async () => {
    const session = await openComposition(GSAP_HTML);
+    const before = session.getElement("hf-box")?.animationIds.length ?? 0;
    runShadowGsapTween(session, {
      kind: "add",
      target: "hf-box",
      tween: { method: "to", properties: { x: 100 }, duration: 0.5 },
    });
+    expect(session.getElement("hf-box")!.animationIds.length).toBe(before + 1);
+    expect(lastShadow()).toMatchObject({ op: "gsap", dispatched: true, mismatchCount: 0 });
+  });
+
+  it("remove drops the tween from animationIds and reports parity", async () => {
+    const session = await openComposition(GSAP_HTML);
+    const animationId = session.getElement("hf-box")?.animationIds[0];
+    expect(animationId).toBeDefined();
+    runShadowGsapTween(session, { kind: "remove", animationId: animationId! });
+    expect(session.getElement("hf-box")?.animationIds ?? []).not.toContain(animationId);
    expect(lastShadow()).toMatchObject({ op: "gsap", dispatched: true, mismatchCount: 0 });
  });

@@ -244,3 +259,118 @@ describe("runShadowGsapTween", () => {
    });
  });
 });
+
+const SCRIPT_A = `var tl = gsap.timeline({ paused: true });
+tl.to("[data-hf-id=\\"hf-box\\"]", { opacity: 1, duration: 0.5 }, 0.2);
+window.__timelines["t"] = tl;`;
+
+describe("gsapFidelityMismatches", () => {
+  it("returns no mismatches for identical scripts", () => {
+    expect(gsapFidelityMismatches(SCRIPT_A, SCRIPT_A)).toEqual([]);
+  });
+
+  it("flags a per-field value drift (duration)", () => {
+    const drifted = SCRIPT_A.replace("duration: 0.5", "duration: 0.9");
+    const mismatches = gsapFidelityMismatches(drifted, SCRIPT_A);
+    expect(mismatches.some((m) => m.property === "duration")).toBe(true);
+  });
+
+  it("flags a tween present in one script but not the other", () => {
+    const empty = `var tl = gsap.timeline({ paused: true });
+window.__timelines["t"] = tl;`;
+    const mismatches = gsapFidelityMismatches(empty, SCRIPT_A);
+    expect(mismatches.some((m) => m.property === "tween")).toBe(true);
+  });
+
+  it("does NOT flag property key-order differences (canonical compare)", () => {
+    const ab = `var tl = gsap.timeline({ paused: true });
+tl.to("[data-hf-id=\\"hf-box\\"]", { x: 10, y: 20, duration: 0.5 }, 0);
+window.__timelines["t"] = tl;`;
+    const ba = `var tl = gsap.timeline({ paused: true });
+tl.to("[data-hf-id=\\"hf-box\\"]", { y: 20, x: 10, duration: 0.5 }, 0);
+window.__timelines["t"] = tl;`;
+    expect(gsapFidelityMismatches(ab, ba)).toEqual([]);
+  });
+
+  it("does NOT flag number-vs-string-equivalent property values", () => {
+    const numeric = `var tl = gsap.timeline({ paused: true });
+tl.to("[data-hf-id=\\"hf-box\\"]", { opacity: 1, duration: 0.5 }, 0);
+window.__timelines["t"] = tl;`;
+    const stringy = `var tl = gsap.timeline({ paused: true });
+tl.to("[data-hf-id=\\"hf-box\\"]", { opacity: "1", duration: 0.5 }, 0);
+window.__timelines["t"] = tl;`;
+    expect(gsapFidelityMismatches(numeric, stringy)).toEqual([]);
+  });
+});
+
+describe("runShadowGsapFidelity", () => {
+  const BEFORE_HTML = `<div data-hf-id="hf-stage" data-hf-root style="width:1280px;height:720px">
+  <div data-hf-id="hf-box" style="opacity:0"></div>
+  <script>var tl = gsap.timeline({ paused: true });
+window.__timelines["t"] = tl;</script>
+</div>`;
+
+  it("reports zero mismatches when the SDK output matches the server script", async () => {
+    // Produce the "server" script by applying the same op via the SDK, so a
+    // faithful SDK writer must reproduce it exactly.
+    const ref = await openComposition(BEFORE_HTML);
+    const op = {
+      kind: "add",
+      target: "hf-box",
+      tween: { method: "to", properties: { x: 100 }, duration: 0.5 },
+    } as const;
+    ref.addGsapTween(op.target, op.tween);
+    const serverScript =
+      ref.serialize().match(/<script\b[^>]*>([\s\S]*?)<\/script[^>]*>/i)?.[1] ?? "";
+
+    await runShadowGsapFidelity(BEFORE_HTML, op, serverScript);
+    expect(lastShadow()).toMatchObject({ op: "gsap_fidelity", dispatched: true, mismatchCount: 0 });
+  });
+
+  it("reports mismatches when the server script diverges", async () => {
+    const op = {
+      kind: "add",
+      target: "hf-box",
+      tween: { method: "to", properties: { x: 100 }, duration: 0.5 },
+    } as const;
+    const ref = await openComposition(BEFORE_HTML);
+    ref.addGsapTween(op.target, op.tween);
+    const serverScript = (
+      ref.serialize().match(/<script\b[^>]*>([\s\S]*?)<\/script[^>]*>/i)?.[1] ?? ""
+    ).replace("100", "999");
+
+    await runShadowGsapFidelity(BEFORE_HTML, op, serverScript);
+    const ev = lastShadow();
+    expect(ev).toMatchObject({ op: "gsap_fidelity", dispatched: true });
+    expect(ev?.mismatchCount as number).toBeGreaterThan(0);
+  });
+});
+
+describe("resolveGsapFidelityArgs (chokepoint wiring)", () => {
+  const op: ShadowGsapOp = { kind: "remove", animationId: "a-1" };
+  const session = {} as object;
+
+  it("returns narrowed args when session, op, before, and serverScript are all present", () => {
+    expect(resolveGsapFidelityArgs(session, op, "<html>before</html>", "tl.to(...)")).toEqual({
+      before: "<html>before</html>",
+      op,
+      serverScript: "tl.to(...)",
+    });
+  });
+
+  it("returns null when no session (shadow not wired)", () => {
+    expect(resolveGsapFidelityArgs(null, op, "before", "script")).toBeNull();
+  });
+
+  it("returns null when no shadowGsapOp (non-meta edit, e.g. property/keyframe)", () => {
+    expect(resolveGsapFidelityArgs(session, undefined, "before", "script")).toBeNull();
+  });
+
+  it("returns null when serverScript is null (composition has no GSAP script)", () => {
+    expect(resolveGsapFidelityArgs(session, op, "before", null)).toBeNull();
+  });
+
+  it("returns null when before is null", () => {
+    expect(resolveGsapFidelityArgs(session, op, null, "script")).toBeNull();
+  });
+});
@@ -367,12 +367,15 @@ export type ShadowGsapOp =
  | { kind: "remove"; animationId: string };

 /**
- * Shadow a GSAP tween mutation. Snapshot value-parity is NOT available: the
- * tween lives in the GSAP <script>, and ElementSnapshot.animationIds is a stub
- * (always [] — see sdk document.ts). So the signal here is can() addressing /
- * validity + dispatch-didn't-throw, plus (for add) that the SDK returned a
- * non-empty tween id. Full fidelity needs serialize()-script round-trip diffing,
- * out of scope for shadow. // ponytail: upgrade when animationIds is populated.
+ * Shadow a GSAP tween mutation (add / set / remove). The server's animationId
+ * shares the SDK's id-space (both derive `targetSelector-method-position` from
+ * the same acorn parser — see sdk assignStableIds), so it is dispatchable as-is.
+ *
+ * Parity via the now-populated ElementSnapshot.animationIds:
+ *   add    → the returned tween id is present on the target element
+ *   remove → the id is gone from every element
+ *   set    → existence only (the SDK exposes no per-tween property reader; value
+ *            fidelity would need serialize()-script round-trip diffing).
 */
 export function runShadowGsapTween(session: Composition, gsapOp: ShadowGsapOp): void {
  if (!STUDIO_SDK_SHADOW_ENABLED) return;
@@ -382,23 +385,50 @@ export function runShadowGsapTween(session: Composition, gsapOp: ShadowGsapOp):
      : gsapOp.kind === "set"
        ? { type: "setGsapTween", animationId: gsapOp.animationId, properties: gsapOp.properties }
        : { type: "removeGsapTween", animationId: gsapOp.animationId };
+  // fallow-ignore-next-line complexity
  runShadowEditOp(session, op, "gsap", () => {
    let newId: string | undefined;
    session.batch(() => {
      if (gsapOp.kind === "add") newId = session.addGsapTween(gsapOp.target, gsapOp.tween);
      else session.dispatch(op);
    });
-    if (gsapOp.kind === "add" && !newId) {
-      return [
-        {
-          kind: "value_mismatch",
-          hfId: gsapOp.target,
-          property: "tweenId",
-          expected: "non-empty",
-          actual: null,
-        },
-      ];
+    if (gsapOp.kind === "add") {
+      const onTarget = session.getElement(gsapOp.target)?.animationIds ?? [];
+      if (!newId || !onTarget.includes(newId)) {
+        return [
+          {
+            kind: "value_mismatch",
+            hfId: gsapOp.target,
+            property: "animationIds",
+            expected: newId ?? "non-empty",
+            actual: onTarget.join(",") || null,
+          },
+        ];
+      }
+    } else if (gsapOp.kind === "remove") {
+      const stillPresent = session
+        .getElements()
+        .some((el) => el.animationIds.includes(gsapOp.animationId));
+      if (stillPresent) {
+        return [
+          {
+            kind: "value_mismatch",
+            hfId: gsapOp.animationId,
+            property: "animationIds",
+            expected: "removed",
+            actual: "present",
+          },
+        ];
+      }
    }
    return [];
  });
 }
+
+// GSAP value-fidelity diff lives in its own module to keep this file under the
+// 600-line studio cap; re-exported here so the shadow surface stays in one place.
+export {
+  gsapFidelityMismatches,
+  resolveGsapFidelityArgs,
+  runShadowGsapFidelity,
+} from "./sdkShadowGsapFidelity";
@@ -0,0 +1,208 @@
+/**
+ * GSAP value-fidelity shadow (serialize round-trip diff). Split out of
+ * sdkShadow.ts to keep that file under the 600-line studio cap.
+ *
+ * Existence parity (sdkShadow.ts) confirms a tween was created/removed, but not
+ * that its VALUES (duration / ease / position / properties) match the server.
+ * The SDK exposes no per-tween property reader, so we compare the two writers'
+ * output: apply the same op to a fresh SDK doc opened from the server's pre-op
+ * file, then structurally diff the SDK's GSAP script against the server's
+ * resulting script. Both are re-parsed, so formatting/whitespace differences
+ * never produce false positives — only real value drift does.
+ */
+
+import { openComposition } from "@hyperframes/sdk";
+import { parseGsapScriptAcorn } from "@hyperframes/core/gsap-parser-acorn";
+import type { GsapAnimation } from "@hyperframes/core/gsap-parser";
+import { STUDIO_SDK_SHADOW_ENABLED } from "../components/editor/manualEditingAvailability";
+import { trackStudioEvent } from "./studioTelemetry";
+import type { SdkShadowMismatch, ShadowGsapOp } from "./sdkShadow";
+
+// Marker set must match document.ts extractGsapScript so both pick the same
+// <script> from any given composition.
+function isGsapScriptBody(body: string): boolean {
+  return body.includes("gsap") || body.includes("__timelines") || body.includes("ScrollTrigger");
+}
+
+function extractGsapScript(html: string): string | null {
+  // Close tag is `</script[^>]*>` (not just `</script>`) — HTML5 ignores junk
+  // before the `>`, e.g. `</script >` or `</script foo>` (CodeQL js/bad-tag-filter).
+  const scripts = html.match(/<script\b[^>]*>([\s\S]*?)<\/script[^>]*>/gi);
+  if (!scripts) return null;
+  for (const block of scripts) {
+    const body = block.replace(/^<script\b[^>]*>/i, "").replace(/<\/script[^>]*>$/i, "");
+    if (isGsapScriptBody(body)) return body;
+  }
+  return null;
+}
+
+function animById(script: string): Map<string, GsapAnimation> {
+  const map = new Map<string, GsapAnimation>();
+  const parsed = parseGsapScriptAcorn(script);
+  for (const anim of parsed.animations) map.set(anim.id, anim);
+  return map;
+}
+
+// The server (addAnimationToScript) and SDK (gsapWriterAcorn) are DIFFERENT
+// writers, so the same tween can serialize with different property key order or
+// number-vs-string forms. Compare canonically — sort keys, coerce numeric
+// strings — so only real value drift registers, not formatting differences.
+
+function numericEqual(a: unknown, b: unknown): boolean {
+  if (a === b) return true;
+  const na = typeof a === "string" ? Number(a) : a;
+  const nb = typeof b === "string" ? Number(b) : b;
+  return (
+    typeof na === "number" &&
+    typeof nb === "number" &&
+    !Number.isNaN(na) &&
+    !Number.isNaN(nb) &&
+    na === nb
+  );
+}
+
+function canonicalProps(obj: Record<string, unknown> | undefined): string {
+  if (!obj) return "{}";
+  const out: Record<string, unknown> = {};
+  for (const key of Object.keys(obj).sort()) {
+    const v = obj[key];
+    // normalize "0.5" → 0.5 so a number/string writer difference isn't drift
+    out[key] = typeof v === "string" && v.trim() !== "" && !Number.isNaN(Number(v)) ? Number(v) : v;
+  }
+  return JSON.stringify(out);
+}
+
+/**
+ * Structurally diff two GSAP scripts by tween id. Reports a tween present in
+ * one but not the other, and per-field value drift (method, position, duration,
+ * ease, properties, fromProperties). Comparison is canonical (see above) so
+ * writer formatting differences do not produce false mismatches.
+ */
+// fallow-ignore-next-line complexity
+export function gsapFidelityMismatches(
+  sdkScript: string,
+  serverScript: string,
+): SdkShadowMismatch[] {
+  const sdk = animById(sdkScript);
+  const server = animById(serverScript);
+  const mismatches: SdkShadowMismatch[] = [];
+  const ids = new Set([...sdk.keys(), ...server.keys()]);
+  for (const id of ids) {
+    const a = sdk.get(id);
+    const b = server.get(id);
+    if (!a || !b) {
+      mismatches.push({
+        kind: "value_mismatch",
+        hfId: id,
+        property: "tween",
+        expected: b ? "present" : "absent",
+        actual: a ? "present" : "absent",
+      });
+      continue;
+    }
+    // [property, sdk-value, server-value, equal?]
+    const fields: Array<[string, unknown, unknown, boolean]> = [
+      ["method", a.method, b.method, a.method === b.method],
+      ["position", a.position, b.position, numericEqual(a.position, b.position)],
+      ["duration", a.duration, b.duration, numericEqual(a.duration, b.duration)],
+      ["ease", a.ease, b.ease, a.ease === b.ease],
+      [
+        "properties",
+        a.properties,
+        b.properties,
+        canonicalProps(a.properties) === canonicalProps(b.properties),
+      ],
+      [
+        "fromProperties",
+        a.fromProperties,
+        b.fromProperties,
+        canonicalProps(a.fromProperties) === canonicalProps(b.fromProperties),
+      ],
+    ];
+    for (const [property, av, bv, equal] of fields) {
+      if (!equal) {
+        mismatches.push({
+          kind: "value_mismatch",
+          hfId: id,
+          property,
+          expected: bv == null ? null : JSON.stringify(bv),
+          actual: av == null ? null : JSON.stringify(av),
+        });
+      }
+    }
+  }
+  return mismatches;
+}
+
+export interface GsapFidelityArgs {
+  before: string;
+  op: ShadowGsapOp;
+  serverScript: string;
+}
+
+/**
+ * Wiring gate for the commitMutation chokepoint: return the narrowed fidelity
+ * args only when there is a live session, a typed shadow op, and both the
+ * pre-op file and the server's resulting script to diff against (scriptText is
+ * null when the composition has no GSAP script). Returns null otherwise. Pure +
+ * narrowing so the wiring decision is unit-testable without rendering the hook
+ * and the caller needs no non-null assertions.
+ */
+export function resolveGsapFidelityArgs(
+  sdkSession: unknown,
+  shadowGsapOp: ShadowGsapOp | undefined,
+  before: string | null | undefined,
+  serverScript: string | null | undefined,
+): GsapFidelityArgs | null {
+  if (!sdkSession || !shadowGsapOp || before == null || serverScript == null) return null;
+  return { before, op: shadowGsapOp, serverScript };
+}
+
+/**
+ * Shadow GSAP value fidelity: open a fresh SDK doc from the server's pre-op
+ * file, apply the same tween op, serialize, and diff the SDK's GSAP script
+ * against the server's resulting script. Emits sdk_shadow_dispatch op:
+ * "gsap_fidelity". Async, fire-and-forget; server stays authoritative.
+ */
+export async function runShadowGsapFidelity(
+  beforeHtml: string,
+  gsapOp: ShadowGsapOp,
+  serverScript: string,
+): Promise<void> {
+  if (!STUDIO_SDK_SHADOW_ENABLED) return;
+  // No server script to diff against → skip the (costly) openComposition.
+  if (!serverScript || !beforeHtml) return;
+  try {
+    const session = await openComposition(beforeHtml);
+    session.batch(() => {
+      if (gsapOp.kind === "add") session.addGsapTween(gsapOp.target, gsapOp.tween);
+      else if (gsapOp.kind === "set") session.setGsapTween(gsapOp.animationId, gsapOp.properties);
+      else session.removeGsapTween(gsapOp.animationId);
+    });
+    const sdkScript = extractGsapScript(session.serialize());
+    if (sdkScript == null) {
+      trackStudioEvent("sdk_shadow_dispatch", {
+        op: "gsap_fidelity",
+        dispatched: false,
+        reason: "no_sdk_script",
+        mismatchCount: 0,
+      });
+      return;
+    }
+    const mismatches = gsapFidelityMismatches(sdkScript, serverScript);
+    trackStudioEvent("sdk_shadow_dispatch", {
+      op: "gsap_fidelity",
+      dispatched: true,
+      mismatchCount: mismatches.length,
+      mismatches: JSON.stringify(mismatches),
+    });
+  } catch (err) {
+    trackStudioEvent("sdk_shadow_dispatch", {
+      op: "gsap_fidelity",
+      dispatched: false,
+      reason: "fidelity_error",
+      error: String(err),
+      mismatchCount: 0,
+    });
+  }
+}