fix(studio): suppress shadow-parity false positives in timing + text (#1508)

runShadowTiming: compare start/duration with a relative epsilon (1e-6) instead of exact equality so float-precision drift (3.1 vs 3.0999999999999996, 21.36 vs 21.360000000000014) no longer flags; a real difference (3.1 vs 3.5) still flags. trackIndex stays exact. property:text resolver: trim both sides (snapshot.text is already trimmed) and collapse empty-string vs absent (null) text so trailing-whitespace and empty-vs-null no longer flag. Genuine text differences are unaffected; the per-keystroke length lag is a caller-side debounce concern. Adds tests for both fixes plus regression tests documenting two REAL SDK divergences the shadow correctly surfaces (transform-origin removal no-op; duplicate-bare-id delete resolution) — flagged, not fixed here. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-09-03 04:38:33 +00:00 · 2026-06-16 12:26:58 -07:00
parent c096ff3afa
commit 5aca3ad770
2 changed files with 165 additions and 10 deletions
@@ -110,6 +110,7 @@ describe("sdkShadowDispatch (integration)", () => {
    expect(session.getElement("hf-box")?.inlineStyles.color).toBe("#00f");
  });

+  // fallow-ignore-next-line code-duplication
  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);
@@ -149,6 +150,65 @@ describe("sdkShadowDispatch (integration)", () => {
    expect(session.getElement("hf-box")?.text).toBe("Updated");
  });

+  // Fix 2: text parity normalization. snapshot.text is trimmed by the SDK, so a
+  // trailing-whitespace-only difference between the op value and the snapshot must
+  // not flag.
+  it("does NOT false-mismatch trailing-whitespace-only text difference", async () => {
+    const { sdkShadowDispatch } = await import("./sdkShadow");
+    const session = await openComposition(BASE_HTML);
+
+    const ops: PatchOperation[] = [{ type: "text-content", property: "text", value: "World   " }];
+    const result = sdkShadowDispatch(session, "hf-box", ops);
+
+    expect(result.dispatched).toBe(true);
+    expect(result.mismatches).toHaveLength(0); // trimmed both sides
+  });
+
+  // Empty-string op value vs an absent (null) snapshot text must collapse to equal
+  // — both mean "no text content".
+  it("treats empty-string text op and null snapshot text as equal", async () => {
+    const { sdkShadowDispatch } = await import("./sdkShadow");
+    const EMPTY_HTML = /* html */ `<!DOCTYPE html>
+<html><body><img data-hf-id="hf-img" src="x.png" /></body></html>`;
+    const session = await openComposition(EMPTY_HTML);
+
+    const ops: PatchOperation[] = [{ type: "text-content", property: "text", value: "" }];
+    const result = sdkShadowDispatch(session, "hf-img", ops);
+
+    expect(result.dispatched).toBe(true);
+    expect(result.mismatches).toHaveLength(0); // "" vs null → both null
+  });
+
+  // Fix 3 verdict (REAL DIVERGENCE, not a readback artifact): the inline-style
+  // read-back already reads only the AUTHORED style attribute (getElementStyles →
+  // parseStyleAttr), never computed styles. The transform-origin event
+  // (expected null actual "center center") is a genuine SDK bug: setStyle removal
+  // of a HYPHENATED property silently no-ops because setElementStyles deletes the
+  // kebab key while the style map is keyed camelCase. The shadow CORRECTLY flags
+  // it; the fix belongs in the SDK (packages/sdk/src/engine/model.ts), not here.
+  it("CORRECTLY flags the SDK transform-origin removal no-op (real divergence)", async () => {
+    const { sdkShadowDispatch } = await import("./sdkShadow");
+    const TO_HTML = /* html */ `<!DOCTYPE html>
+<html><body><div data-hf-id="hf-box" style="transform-origin: center center">x</div></body></html>`;
+    const session = await openComposition(TO_HTML);
+
+    // op intends to REMOVE transform-origin (value null) ...
+    const ops: PatchOperation[] = [
+      { type: "inline-style", property: "transform-origin", value: null },
+    ];
+    const result = sdkShadowDispatch(session, "hf-box", ops);
+
+    // ... but the SDK still has it → genuine value_mismatch, not suppressed.
+    expect(result.dispatched).toBe(true);
+    expect(result.mismatches).toHaveLength(1);
+    expect(result.mismatches[0]).toMatchObject({
+      kind: "value_mismatch",
+      property: "transform-origin",
+      expected: null,
+      actual: "center center",
+    });
+  });
+
  it("applies attribute op and reads back via session.getElement", async () => {
    const { sdkShadowDispatch } = await import("./sdkShadow");
    const session = await openComposition(BASE_HTML);
@@ -239,6 +299,32 @@ describe("runShadowDelete", () => {
      reason: "cannot_dispatch",
    });
  });
+
+  // Fix 4 verdict (REAL SDK id-resolution divergence, NOT a readback bug): when a
+  // bare hf-id collides between a sub-composition element (scopedId
+  // "hf-host/hf-dup") and a top-level sibling (scopedId "hf-dup"), removeElement
+  // resolves the bare id via resolveScoped → querySelector (document-order-first,
+  // removes the INNER instance), but getElement prefers the canonical top-level
+  // match (scopedId === id) which SURVIVES. The shadow then correctly reports
+  // expected "removed" / actual "present". The readback here is correct (it checks
+  // the same id it dispatched); the fix belongs in the SDK's id resolution
+  // (resolveScoped vs getElement agreement), not in this file.
+  const DUP_ID_HTML = /* html */ `<!DOCTYPE html><html><body>
+    <div data-hf-id="hf-root" data-hf-root>
+      <div data-hf-id="hf-host" data-composition-file="sub.html">
+        <div data-hf-id="hf-dup">inner</div>
+      </div>
+      <div data-hf-id="hf-dup">outer</div>
+    </div>
+  </body></html>`;
+
+  it("CORRECTLY reports present/removed for the SDK duplicate-bare-id delete divergence", async () => {
+    const session = await openComposition(DUP_ID_HTML);
+    runShadowDelete(session, "hf-dup");
+    // removeElement dropped the inner instance; the top-level one survives, so the
+    // readback truthfully flags one mismatch (not silently passed).
+    expect(lastShadow()).toMatchObject({ op: "delete", dispatched: true, mismatchCount: 1 });
+  });
 });

 describe("runShadowTiming", () => {
@@ -251,6 +337,38 @@ describe("runShadowTiming", () => {
    expect(el?.trackIndex).toBe(1);
    expect(lastShadow()).toMatchObject({ op: "timing", dispatched: true, mismatchCount: 0 });
  });
+
+  // Fix 1: float-precision tolerance. The SDK computes durations arithmetically
+  // (returning e.g. 3.0999999999999996); the server stores the rounded literal
+  // (3.1). A relative epsilon must treat these as equal, while a real difference
+  // still flags. A fake session returns the imprecise value on read-back.
+  type FakeTiming = { start?: number; duration?: number; trackIndex?: number };
+  function fakeTimingSession(readback: FakeTiming) {
+    return {
+      can: () => ({ ok: true }),
+      batch: (fn: () => void) => fn(),
+      dispatch: () => {},
+      getElement: () => readback,
+    } as unknown as Parameters<typeof runShadowTiming>[0];
+  }
+
+  it("does NOT flag float-precision duration drift (3.1 vs 3.0999999999999996)", () => {
+    const session = fakeTimingSession({ duration: 3.0999999999999996 });
+    runShadowTiming(session, "hf-clip", { duration: 3.1 });
+    expect(lastShadow()).toMatchObject({ op: "timing", dispatched: true, mismatchCount: 0 });
+  });
+
+  it("does NOT flag float-precision start drift (21.36 vs 21.360000000000014)", () => {
+    const session = fakeTimingSession({ start: 21.360000000000014 });
+    runShadowTiming(session, "hf-clip", { start: 21.36 });
+    expect(lastShadow()).toMatchObject({ op: "timing", dispatched: true, mismatchCount: 0 });
+  });
+
+  it("STILL flags a real duration difference (3.1 vs 3.5)", () => {
+    const session = fakeTimingSession({ duration: 3.5 });
+    runShadowTiming(session, "hf-clip", { duration: 3.1 });
+    expect(lastShadow()).toMatchObject({ op: "timing", dispatched: true, mismatchCount: 1 });
+  });
 });

 describe("runShadowGsapTween", () => {
@@ -121,13 +121,29 @@ function kebabToCamel(prop: string): string {
  return prop.replace(/-([a-z])/g, (_, c: string) => c.toUpperCase());
 }

+// Text parity: the SDK snapshot.text is trimmed, so trim the op value too.
+// An empty string and absent text (null) are treated as equivalent (collapsed
+// to null) so "" vs null does not flag — both mean "no text content".
+function normalizeText(value: string | null | undefined): string | null {
+  if (value == null) return null;
+  const trimmed = value.trim();
+  return trimmed === "" ? null : trimmed;
+}
+
 const OP_FIELD_RESOLVERS: Record<string, OpFieldResolver> = {
  "inline-style": (op, flat) => ({
    property: op.property,
    expected: op.value,
    actual: flat.styles[kebabToCamel(op.property)] ?? flat.styles[op.property] ?? null,
  }),
-  "text-content": (op, flat) => ({ property: "text", expected: op.value ?? "", actual: flat.text }),
+  // snapshot.text is already TRIMMED; trim the expected op value to match, so
+  // trailing-whitespace differences don't flag. Empty-vs-absent ("" vs null) is
+  // collapsed in checkOpParity. A genuinely different text value still flags.
+  "text-content": (op, flat) => ({
+    property: "text",
+    expected: normalizeText(op.value),
+    actual: normalizeText(flat.text),
+  }),
  attribute: (op, flat) => ({
    property: attrName(op.property),
    expected: op.value ?? null,
@@ -345,6 +361,28 @@ export interface ShadowTiming {
  trackIndex?: number;
 }

+// Timing start/duration are computed arithmetically by the SDK (e.g. 21.36 -
+// 0 + drag delta) but stored as a rounded literal server-side, so exact compare
+// flags float-precision noise like 3.1 vs 3.0999999999999996 (~1e-16). Compare
+// with a relative epsilon; a genuinely different value (3.1 vs 3.5) still flags.
+// trackIndex is an integer track slot — compared exactly by the caller.
+function timingValuesEqual(a: number, b: number): boolean {
+  if (a === b) return true;
+  return Math.abs(a - b) <= 1e-6 * Math.max(1, Math.abs(a), Math.abs(b));
+}
+
+// start/duration tolerate float-precision drift; trackIndex (integer slot) is exact.
+function timingFieldEqual(
+  key: keyof ShadowTiming,
+  actual: number | null | undefined,
+  expected: number,
+): boolean {
+  if (typeof actual === "number" && key !== "trackIndex") {
+    return timingValuesEqual(actual, expected);
+  }
+  return actual === expected;
+}
+
 /** Shadow a timing edit. Parity: snapshot start/duration/trackIndex match. */
 export function runShadowTiming(
  session: Composition,
@@ -373,15 +411,14 @@ export function runShadowTiming(
    ];
    for (const [key, actual] of fields) {
      const expected = timing[key];
-      if (expected !== undefined && actual !== expected) {
-        mismatches.push({
-          kind: "value_mismatch",
-          hfId,
-          property: key,
-          expected: String(expected),
-          actual: actual == null ? null : String(actual),
-        });
-      }
+      if (expected === undefined || timingFieldEqual(key, actual, expected)) continue;
+      mismatches.push({
+        kind: "value_mismatch",
+        hfId,
+        property: key,
+        expected: String(expected),
+        actual: actual == null ? null : String(actual),
+      });
    }
    return mismatches;
  });