fix(studio): make SDK shadow telemetry fire + be correct (5 fixes, E2E-verified) (#1491)

* fix(studio): open SDK shadow session in master view (was never opening) useSdkSession(projectId, activeCompPath) received activeCompPath=null in the master/entry view — the studio's convention where null means index.html (isMasterView = !activeCompPath || activeCompPath === "index.html"). The hook's guard `if (!projectId || !activeCompPath) return` then bailed, so the SDK session never opened in the default editing surface. Result: sdkSession was null there → every shadow tap (onDomEditPersisted, onElementDeleted, timing, gsap) was undefined/no-op → zero sdk_shadow_dispatch telemetry for master-view edits (the common case). Shadow only fired when a sub-comp was explicitly opened (which sets activeCompPath). Resolve null → "index.html" (matching the existing convention used by isMasterView and blockInstaller) so the session opens in master view. Verified live: instrumenting the hook showed phase "skipped_no_ids" (activeCompPath null) before, "opened" after. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(studio): shadow property parity — read camelCase style key, not kebab The inline-style parity resolver read flat.styles[op.property] with the kebab-case PatchOperation key ("background-color"), but ElementSnapshot inlineStyles are camelCase ("backgroundColor"), so the read-back was always null → a false value_mismatch on every hyphenated CSS property. Single-word props (color, opacity) coincide, so unit tests missed it. Found live: a color edit on a box emitted op:property mismatchCount:1 with {property:"background-color", expected:"rgb(255,79,88)", actual:null}. Convert kebab→camel for the read-back (fall back to the raw key). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(studio): emit op:delete shadow for timeline-clip deletes The Delete/Backspace hotkey routes to handleTimelineElementDelete whenever a timeline element is selected (useAppHotkeys: `if (selectedElementId) { handleTimelineElementDelete(el); return; }`), returning before the shadow-wired handleDomEditElementDelete. Every clip is a timeline element, so clip deletes — the common case — emitted no op:delete; the delete shadow only fired for a non-timed DOM selection. Add runShadowDelete(sdkSession, element.hfId) to handleTimelineElementDelete's success path, mirroring the move/resize timing taps. Verified live (browser-use): deleting a clip now emits sdk_shadow_dispatch op:delete dispatched:true mismatchCount:0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(studio): match GSAP fidelity tweens by resolved element, not raw selector gsap_fidelity keyed tweens by id (targetSelector-method-position). On tween ADD, the SDK writer emits [data-hf-id="X"] selectors while the server emits class selectors (.x) for the same element — different ids → false present/absent mismatch (mc:2) on every add. Update/remove were clean (the tween already existed with one consistent selector). Key by resolved element (selector → data-hf-id via the pre-op DOM) + method + position, so equivalent tweens match and only real value drift registers. Falls back to raw selector when resolution isn't possible. Found live (browser-use): adding a tween emitted gsap_fidelity mc:2 with {[data-hf-id="hf-b"]-to-0 present-only} + {.b-to-0 present-only}. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(studio): don't shadow studio-internal data-hf-* marker attributes Property-path parity false-mismatched on canvas-drag (path-offset) edits, which emit attribute ops like {property:"data-hf-studio-path-offset"}: 1. The name was built as `data-${op.property}` → double-prefix "data-data-hf-studio-path-offset". 2. The SDK model excludes all data-hf-* attributes, so even the right name reads back null → false value_mismatch. attrName() prefixes only when needed; isShadowableOp() drops data-hf-* attribute ops (studio-internal markers the SDK can't represent), filtered in sdkShadowDispatch before dispatch + parity. Code-confirmed via handleDomPathOffsetCommit → commitPositionPatchToHtml → persistDomEditOperations → onDomEditPersisted; live repro blocked because the test comp's elements were GSAP-animated (drags route to the GSAP path). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(studio): document tweenKey + selector-resolver ceilings (PR review) Per review (Rames, non-blocking): name the two silent fail-modes in the GSAP fidelity diff rather than build speculative disambiguators (not observed in studio-emitted templates). - tweenKey: coincident tweens (same element+method+position) collapse, last wins. Props can't join the key — a matched pair must share a key for the field-diff to run. Upgrade path: property-name hash. - makeSelectorResolver: first-match heuristic; ambiguous shared-class selectors may misunify. Upgrade path: querySelectorAll + uniqueness. 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-16 01:56:12 -07:00
parent d9dc88ff60
commit 42696f0af4
5 changed files with 158 additions and 26 deletions
@@ -108,6 +108,20 @@ describe("sdkShadowDispatch (integration)", () => {
    expect(session.getElement("hf-box")?.inlineStyles.color).toBe("#00f");
  });

+  it("does NOT false-mismatch a hyphenated style property (kebab op vs camelCase snapshot)", async () => {
+    const { sdkShadowDispatch } = await import("./sdkShadow");
+    const session = await openComposition(BASE_HTML);
+
+    const ops: PatchOperation[] = [
+      { type: "inline-style", property: "background-color", value: "rgb(255, 79, 88)" },
+    ];
+    const result = sdkShadowDispatch(session, "hf-box", ops);
+
+    expect(result.dispatched).toBe(true);
+    expect(result.mismatches).toHaveLength(0); // was 1 before the kebab→camel read-back fix
+    expect(session.getElement("hf-box")?.inlineStyles.backgroundColor).toBe("rgb(255, 79, 88)");
+  });
+
  it("returns dispatched:false when hfId not found in session", async () => {
    const { sdkShadowDispatch } = await import("./sdkShadow");
    const session = await openComposition(BASE_HTML);
@@ -143,6 +157,21 @@ describe("sdkShadowDispatch (integration)", () => {
    expect(session.getElement("hf-box")?.attributes["data-name"]).toBe("hero");
  });

+  // fallow-ignore-next-line code-duplication
+  it("does NOT false-mismatch studio-internal data-hf-* marker attributes", async () => {
+    const { sdkShadowDispatch } = await import("./sdkShadow");
+    const session = await openComposition(BASE_HTML);
+
+    // path-offset drags emit these already-data-prefixed, SDK-excluded markers.
+    const ops: PatchOperation[] = [
+      { type: "attribute", property: "data-hf-studio-path-offset", value: "true" },
+    ];
+    const result = sdkShadowDispatch(session, "hf-box", ops);
+
+    expect(result.dispatched).toBe(true);
+    expect(result.mismatches).toHaveLength(0); // filtered, not double-prefixed + flagged
+  });
+
  it("returns dispatch_error when dispatch throws — does not propagate", async () => {
    const { sdkShadowDispatch } = await import("./sdkShadow");
    const session = await openComposition(BASE_HTML);
@@ -301,6 +330,21 @@ tl.to("[data-hf-id=\\"hf-box\\"]", { opacity: "1", duration: 0.5 }, 0);
 window.__timelines["t"] = tl;`;
    expect(gsapFidelityMismatches(numeric, stringy)).toEqual([]);
  });
+
+  it("matches the same element across different selector forms when a resolver is given", () => {
+    // SDK writes [data-hf-id="hf-x"], server writes .x — same element, same tween.
+    const sdk = `var tl = gsap.timeline({ paused: true });
+tl.to("[data-hf-id=\\"hf-x\\"]", { x: 200, duration: 0.8 }, 0.5);
+window.__timelines["t"] = tl;`;
+    const server = `var tl = gsap.timeline({ paused: true });
+tl.to(".x", { x: 200, duration: 0.8 }, 0.5);
+window.__timelines["t"] = tl;`;
+    const resolve = (sel: string) => (/hf-x|\.x/.test(sel) ? "hf-x" : sel);
+    // Without a resolver: selector-form divergence → present/absent mismatch.
+    expect(gsapFidelityMismatches(sdk, server).length).toBeGreaterThan(0);
+    // With a resolver: matched by element → no mismatch.
+    expect(gsapFidelityMismatches(sdk, server, resolve)).toEqual([]);
+  });
 });

 describe("runShadowGsapFidelity", () => {
@@ -23,6 +23,22 @@ import type { PatchOperation } from "./sourcePatcher";
 * Multiple inline-style ops are coalesced into a single setStyle (SDK batches
 * style changes naturally). One SDK op is emitted per non-style op.
 */
+// "attribute" PatchOperations carry the data- attribute NAME. Studio passes
+// some already prefixed (e.g. "data-hf-studio-path-offset") and some bare
+// (e.g. "name"); prefix only when needed, never double-prefix.
+function attrName(property: string): string {
+  return property.startsWith("data-") ? property : `data-${property}`;
+}
+
+// The SDK element model excludes data-hf-* attributes (document.ts skips them),
+// so shadowing studio-internal markers (data-hf-studio-path-offset, etc.) can
+// never match — drop those ops from the shadow instead of false-mismatching.
+function isShadowableOp(op: PatchOperation): boolean {
+  if (op.type === "attribute") return !attrName(op.property).startsWith("data-hf-");
+  if (op.type === "html-attribute") return !op.property.startsWith("data-hf-");
+  return true;
+}
+
 export function patchOpsToSdkEditOps(hfId: string, ops: PatchOperation[]): EditOp[] {
  const result: EditOp[] = [];
  const styles: Record<string, string | null> = {};
@@ -38,7 +54,7 @@ export function patchOpsToSdkEditOps(hfId: string, ops: PatchOperation[]): EditO
      result.push({
        type: "setAttribute",
        target: hfId,
-        name: `data-${op.property}`,
+        name: attrName(op.property),
        value: op.value,
      });
    } else if (op.type === "html-attribute") {
@@ -98,17 +114,24 @@ function flattenSnapshot(snap: ElementSnapshot): FlatSnapshot {

 type OpFieldResolver = (op: PatchOperation, flat: FlatSnapshot) => OpFields;

+// Snapshot inlineStyles are camelCase (CSSStyleDeclaration convention); PatchOperation
+// style properties are kebab-case ("background-color"). Convert for read-back, else
+// every hyphenated property false-mismatches against a null actual.
+function kebabToCamel(prop: string): string {
+  return prop.replace(/-([a-z])/g, (_, c: string) => c.toUpperCase());
+}
+
 const OP_FIELD_RESOLVERS: Record<string, OpFieldResolver> = {
  "inline-style": (op, flat) => ({
    property: op.property,
    expected: op.value,
-    actual: flat.styles[op.property] ?? null,
+    actual: flat.styles[kebabToCamel(op.property)] ?? flat.styles[op.property] ?? null,
  }),
  "text-content": (op, flat) => ({ property: "text", expected: op.value ?? "", actual: flat.text }),
  attribute: (op, flat) => ({
-    property: `data-${op.property}`,
+    property: attrName(op.property),
    expected: op.value ?? null,
-    actual: flat.attrs[`data-${op.property}`] ?? null,
+    actual: flat.attrs[attrName(op.property)] ?? null,
  }),
  "html-attribute": (op, flat) => ({
    property: op.property,
@@ -157,8 +180,11 @@ export function sdkShadowDispatch(
  if (!session.getElement(hfId)) {
    return { dispatched: false, mismatches: [{ kind: "element_not_found", hfId }] };
  }
+  // Drop studio-internal markers the SDK model can't represent (data-hf-*), so
+  // canvas-drag/path-offset edits don't false-mismatch on bookkeeping attrs.
+  const shadowable = ops.filter(isShadowableOp);
  try {
-    const sdkOps = patchOpsToSdkEditOps(hfId, ops);
+    const sdkOps = patchOpsToSdkEditOps(hfId, shadowable);
    session.batch(() => {
      for (const op of sdkOps) session.dispatch(op);
    });
@@ -169,7 +195,7 @@ export function sdkShadowDispatch(
    };
  }
  const flat = flattenSnapshot(session.getElement(hfId));
-  const mismatches = ops
+  const mismatches = shadowable
    .map((op) => checkOpParity(op, flat, hfId))
    .filter((m): m is SdkShadowMismatch => m !== null);
  return { dispatched: true, mismatches };
@@ -36,10 +36,35 @@ function extractGsapScript(html: string): string | null {
  return null;
 }

-function animById(script: string): Map<string, GsapAnimation> {
+function posKey(position: unknown): string {
+  if (typeof position === "number") return String(position);
+  const n = Number(position);
+  return Number.isNaN(n) ? String(position) : String(n);
+}
+
+// Key a tween by its RESOLVED target element (not raw selector) + method +
+// position. The SDK writer emits [data-hf-id="X"] selectors while the server
+// emits class/other selectors for the SAME element; keying by resolved element
+// matches them so the diff compares values instead of flagging present/absent.
+//
+// ponytail: one-tween-per-(element, method, position) assumption — coincident
+// tweens (same element+method+position, different props) collapse, last wins,
+// so the diff under-reports them. Props can't go in the key (a matched pair
+// must share a key for the field-diff to run; raw props would split real value
+// drift into present/absent). Not seen in studio-emitted templates; add a
+// property-NAME hash to the key if coincident tweens show up in the wild.
+function tweenKey(anim: GsapAnimation, resolveSelector?: (sel: string) => string): string {
+  const sel = resolveSelector ? resolveSelector(anim.targetSelector) : anim.targetSelector;
+  return `${sel}|${anim.method}|${posKey(anim.position)}`;
+}
+
+function animByKey(
+  script: string,
+  resolveSelector?: (sel: string) => 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);
+  for (const anim of parsed.animations) map.set(tweenKey(anim, resolveSelector), anim);
  return map;
 }

@@ -73,37 +98,41 @@ function canonicalProps(obj: Record<string, unknown> | undefined): string {
 }

 /**
- * 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.
+ * Structurally diff two GSAP scripts. Tweens are matched by resolved target
+ * element + method + position (see tweenKey), so the SDK's [data-hf-id]
+ * selectors and the server's class selectors for the same element don't
+ * false-flag present/absent. Reports a tween present in one but not the other,
+ * and per-field value drift (duration, ease, properties, fromProperties).
+ * Comparison is canonical so writer formatting differences don't register.
+ *
+ * Pass resolveSelector (selector → canonical element id) to enable the
+ * element-based matching; without it, matching falls back to raw selector.
 */
 // fallow-ignore-next-line complexity
 export function gsapFidelityMismatches(
  sdkScript: string,
  serverScript: string,
+  resolveSelector?: (sel: string) => string,
 ): SdkShadowMismatch[] {
-  const sdk = animById(sdkScript);
-  const server = animById(serverScript);
+  const sdk = animByKey(sdkScript, resolveSelector);
+  const server = animByKey(serverScript, resolveSelector);
  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);
+  const keys = new Set([...sdk.keys(), ...server.keys()]);
+  for (const key of keys) {
+    const a = sdk.get(key);
+    const b = server.get(key);
    if (!a || !b) {
      mismatches.push({
        kind: "value_mismatch",
-        hfId: id,
+        hfId: key,
        property: "tween",
        expected: b ? "present" : "absent",
        actual: a ? "present" : "absent",
      });
      continue;
    }
-    // [property, sdk-value, server-value, equal?]
+    // method + position are part of the key (already equal); compare values.
    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],
      [
@@ -123,7 +152,7 @@ export function gsapFidelityMismatches(
      if (!equal) {
        mismatches.push({
          kind: "value_mismatch",
-          hfId: id,
+          hfId: key,
          property,
          expected: bv == null ? null : JSON.stringify(bv),
          actual: av == null ? null : JSON.stringify(av),
@@ -158,6 +187,33 @@ export function resolveGsapFidelityArgs(
  return { before, op: shadowGsapOp, serverScript };
 }

+// Resolve a CSS selector to a canonical element id (data-hf-id) using the pre-op
+// document, so tweens that target the same element via different selectors
+// ([data-hf-id="X"] vs .X) match in the fidelity diff. Falls back to the raw
+// selector when it can't resolve (DOMParser unavailable, no match, bad selector).
+//
+// ponytail: first-match heuristic — querySelector returns the FIRST match, so an
+// ambiguous selector (e.g. .x shared by two elements) may map to a different id
+// than the SDK side's [data-hf-id] target and still flag present/absent. Safe
+// for studio templates (one tween per data-hf-id); upgrade to querySelectorAll +
+// uniqueness check if ambiguous selectors appear.
+function makeSelectorResolver(html: string): (sel: string) => string {
+  let doc: Document | null = null;
+  try {
+    doc = new DOMParser().parseFromString(html, "text/html");
+  } catch {
+    doc = null;
+  }
+  return (sel) => {
+    if (!doc) return sel;
+    try {
+      return doc.querySelector(sel)?.getAttribute("data-hf-id") ?? sel;
+    } catch {
+      return sel;
+    }
+  };
+}
+
 /**
 * 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
@@ -189,7 +245,11 @@ export async function runShadowGsapFidelity(
      });
      return;
    }
-    const mismatches = gsapFidelityMismatches(sdkScript, serverScript);
+    const mismatches = gsapFidelityMismatches(
+      sdkScript,
+      serverScript,
+      makeSelectorResolver(beforeHtml),
+    );
    trackStudioEvent("sdk_shadow_dispatch", {
      op: "gsap_fidelity",
      dispatched: true,