From 168bb4f1ad2a010c24c433913a4d0d604dcc7bcd Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Tue, 16 Jun 2026 11:01:46 -0700 Subject: [PATCH 1/2] fix(studio): serialize GSAP script commits per file (shadow request race) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rapid GSAP edits (ease/duration/keyframe/property) fired overlapping read-modify-write POSTs to one script file — coalesceKey only dedupes edit history, not requests. The gsap_fidelity shadow then diffed an op against whichever POST's scriptText resolved, which could predate that op → false "expected null, actual power2.out" mismatches. Server persists correctly; a pure client request-pairing race. Adds createKeyedSerializer (per-key promise chain, rejection-safe, self- cleaning). commitMutation now serializes every GSAP-script commit per target file by default (key `gsap-file:`) — covering all op types and all animations, not just one meta family — so same-file POSTs can't interleave. Distinct files run concurrently; an explicit serializeKey still overrides. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../studio/src/hooks/gsapScriptCommitTypes.ts | 8 ++ .../studio/src/hooks/serializeByKey.test.ts | 83 +++++++++++++++++++ packages/studio/src/hooks/serializeByKey.ts | 33 ++++++++ .../studio/src/hooks/useGsapAnimationOps.ts | 6 +- .../studio/src/hooks/useGsapScriptCommits.ts | 24 +++++- 5 files changed, 151 insertions(+), 3 deletions(-) create mode 100644 packages/studio/src/hooks/serializeByKey.test.ts create mode 100644 packages/studio/src/hooks/serializeByKey.ts diff --git a/packages/studio/src/hooks/gsapScriptCommitTypes.ts b/packages/studio/src/hooks/gsapScriptCommitTypes.ts index 202a83add..afc949256 100644 --- a/packages/studio/src/hooks/gsapScriptCommitTypes.ts +++ b/packages/studio/src/hooks/gsapScriptCommitTypes.ts @@ -20,6 +20,14 @@ export interface CommitMutationOptions { softReload?: boolean; skipReload?: boolean; beforeReload?: () => void; + /** + * Serialize this commit against others sharing the same key. Used to chain + * per-animationId GSAP meta updates so overlapping read-modify-write POSTs to + * one file can't interleave — which would pair the shadow fidelity diff with a + * stale server result and report false ease mismatches. Commits without a key + * (and under distinct keys) run concurrently as before. + */ + serializeKey?: string; /** Stage 7 Step 3b: typed SDK equivalent of this mutation for value-fidelity shadow. */ shadowGsapOp?: ShadowGsapOp; /** Typed SDK equivalent of a keyframe mutation for keyframe value-fidelity shadow (gsap_keyframe). */ diff --git a/packages/studio/src/hooks/serializeByKey.test.ts b/packages/studio/src/hooks/serializeByKey.test.ts new file mode 100644 index 000000000..2b023118a --- /dev/null +++ b/packages/studio/src/hooks/serializeByKey.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it } from "vitest"; +import { createKeyedSerializer } from "./serializeByKey"; + +function deferred() { + let resolve!: (value: T) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +describe("createKeyedSerializer", () => { + it("runs same-key tasks strictly in order (second awaits the first)", async () => { + const run = createKeyedSerializer(); + const order: string[] = []; + const first = deferred(); + + const p1 = run("k", async () => { + order.push("1-start"); + await first.promise; + order.push("1-end"); + }); + const p2 = run("k", async () => { + order.push("2-start"); + }); + + // Second task must not start until the first finishes. + await Promise.resolve(); + expect(order).toEqual(["1-start"]); + + first.resolve(); + await Promise.all([p1, p2]); + expect(order).toEqual(["1-start", "1-end", "2-start"]); + }); + + it("does not block tasks under different keys", async () => { + const run = createKeyedSerializer(); + const order: string[] = []; + const blockA = deferred(); + + const pa = run("a", async () => { + order.push("a-start"); + await blockA.promise; + order.push("a-end"); + }); + const pb = run("b", async () => { + order.push("b-start"); + order.push("b-end"); + }); + + // Different key runs to completion while "a" is still blocked. + await pb; + expect(order).toEqual(["a-start", "b-start", "b-end"]); + + blockA.resolve(); + await pa; + expect(order).toEqual(["a-start", "b-start", "b-end", "a-end"]); + }); + + it("does not wedge a key when a prior task rejects", async () => { + const run = createKeyedSerializer(); + const order: string[] = []; + + const p1 = run("k", async () => { + order.push("1"); + throw new Error("boom"); + }); + await expect(p1).rejects.toThrow("boom"); + + const p2 = run("k", async () => { + order.push("2"); + }); + await p2; + expect(order).toEqual(["1", "2"]); + }); + + it("propagates the task's resolved value to its caller", async () => { + const run = createKeyedSerializer(); + await expect(run("k", async () => 42)).resolves.toBe(42); + }); +}); diff --git a/packages/studio/src/hooks/serializeByKey.ts b/packages/studio/src/hooks/serializeByKey.ts new file mode 100644 index 000000000..878a2c4c6 --- /dev/null +++ b/packages/studio/src/hooks/serializeByKey.ts @@ -0,0 +1,33 @@ +/** + * Per-key task serializer. Tasks sharing a key run strictly in order: a new + * task for a key awaits the prior task for that key before starting, so their + * effects (e.g. overlapping read-modify-write POSTs to one file) can't + * interleave. Tasks under different keys are independent and never block each + * other. + * + * Used to serialize GSAP meta-update commits per animationId so the shadow + * fidelity diff always pairs an op with the server result that includes it — + * without globally serializing unrelated commits. + */ +export function createKeyedSerializer() { + const inFlight = new Map>(); + + return function run(key: string, task: () => Promise): Promise { + const prior = inFlight.get(key) ?? Promise.resolve(); + // Chain onto the prior task regardless of how it settled; a rejected prior + // commit must not wedge the key forever. + const next = prior.then(task, task); + inFlight.set(key, next); + // Once this task settles, drop it from the map if nothing newer replaced it, + // so completed keys don't leak. + void next.then( + () => { + if (inFlight.get(key) === next) inFlight.delete(key); + }, + () => { + if (inFlight.get(key) === next) inFlight.delete(key); + }, + ); + return next; + }; +} diff --git a/packages/studio/src/hooks/useGsapAnimationOps.ts b/packages/studio/src/hooks/useGsapAnimationOps.ts index a184807ac..aa953fb2d 100644 --- a/packages/studio/src/hooks/useGsapAnimationOps.ts +++ b/packages/studio/src/hooks/useGsapAnimationOps.ts @@ -40,10 +40,14 @@ export function useGsapAnimationOps({ animationId, properties: { duration: updates.duration, ease: updates.ease, position: updates.position }, }; + // coalesceKey groups rapid meta edits into one history entry. Request + // serialization is now handled per-file at the commitMutation chokepoint + // (useGsapScriptCommits), so no per-op serializeKey is needed here. + const metaKey = `gsap:${animationId}:meta`; commitMutationSafely( selection, { type: "update-meta", animationId, updates }, - { label: "Edit GSAP animation", coalesceKey: `gsap:${animationId}:meta`, shadowGsapOp }, + { label: "Edit GSAP animation", coalesceKey: metaKey, shadowGsapOp }, ); if (sdkSession) runShadowGsapTween(sdkSession, shadowGsapOp); }, diff --git a/packages/studio/src/hooks/useGsapScriptCommits.ts b/packages/studio/src/hooks/useGsapScriptCommits.ts index 49e57991c..1e4b4c7d9 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.ts +++ b/packages/studio/src/hooks/useGsapScriptCommits.ts @@ -1,10 +1,11 @@ -import { useCallback } from "react"; +import { useCallback, useRef } from "react"; import { findUnsafeMutationValues } from "@hyperframes/core/studio-api/finite-mutation"; import type { DomEditSelection } from "../components/editor/domEditingTypes"; import { applySoftReload } from "../utils/gsapSoftReload"; import { resolveGsapFidelityArgs, runShadowGsapFidelity } from "../utils/sdkShadowGsapFidelity"; import { runShadowGsapKeyframeFidelity } from "../utils/sdkShadowGsapKeyframe"; import { updateKeyframeCacheFromParsed } from "./gsapKeyframeCacheHelpers"; +import { createKeyedSerializer } from "./serializeByKey"; import { GsapMutationHttpError, formatGsapMutationRejectionToast, @@ -46,10 +47,16 @@ async function mutateGsapScript( // oxfmt-ignore // fallow-ignore-next-line complexity export function useGsapScriptCommits({ projectIdRef, activeCompPath, previewIframeRef, editHistory, domEditSaveTimestampRef, reloadPreview, onCacheInvalidate, onFileContentChanged, showToast, sdkSession }: GsapScriptCommitsParams) { + // Serializer for per-key commits (options.serializeKey). Keyed by + // `gsap:${animationId}:meta`, it chains a meta commit onto the prior one for + // the same animationId so their POSTs can't interleave — which is what made + // the shadow fidelity diff pair an op with a stale server result and report + // false ease mismatches. Held in a ref so the chain survives re-renders. + const serializerRef = useRef(createKeyedSerializer()); // Pre-existing complexity (server mutate + history + reload branches); this PR // adds only a guarded shadow-fidelity dispatch. // fallow-ignore-next-line complexity - const commitMutation = useCallback(async (selection: DomEditSelection, mutation: Record, options: CommitMutationOptions) => { + const runCommit = useCallback(async (selection: DomEditSelection, mutation: Record, options: CommitMutationOptions) => { const pid = projectIdRef.current; if (!pid) return; const unsafeFields = findUnsafeMutationValues(mutation); @@ -105,6 +112,19 @@ export function useGsapScriptCommits({ projectIdRef, activeCompPath, previewIfra } onCacheInvalidate(); }, [projectIdRef, activeCompPath, previewIframeRef, editHistory, domEditSaveTimestampRef, reloadPreview, onCacheInvalidate, onFileContentChanged, showToast, sdkSession]); + // Every GSAP-script commit is a read-modify-write of one file. Overlapping + // commits to the SAME file (any op type, any animation) interleave server-side + // and make the shadow fidelity diff pair an op with a stale server result — + // the false ease/value mismatches this serializer exists to prevent. So + // serialize per target file by default; an explicit serializeKey overrides. + const commitMutation = useCallback( + (selection: DomEditSelection, mutation: Record, options: CommitMutationOptions) => { + const file = selection.sourceFile || activeCompPath || "index.html"; + const key = options.serializeKey ?? `gsap-file:${file}`; + return serializerRef.current(key, () => runCommit(selection, mutation, options)); + }, + [runCommit, activeCompPath], + ); const trackGsapSaveFailure = useGsapSaveFailureTelemetry(activeCompPath); const commitMutationSafely = useSafeGsapCommitMutation(commitMutation, trackGsapSaveFailure, showToast); const propertyOps = useGsapPropertyDebounce(commitMutationSafely); From 1b02fcd86b4235d6b30f37ffbe5f11a54e851600 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Tue, 16 Jun 2026 12:12:38 -0700 Subject: [PATCH 2/2] refactor(studio): dedup shadow numeric-equal + GSAP script extraction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-review cleanup (no behavior change): - Extract the relative-epsilon float compare into shared sdkShadowNumeric.relEqual, used by both timing parity (sdkShadow) and GSAP value fidelity (numericEqual) — was duplicated verbatim, risking divergent tuning. - Export extractGsapScript from sdkShadowGsapFidelity and import it in the keyframe shadow instead of the byte-identical clone (the regex + marker set must stay in sync with document.ts; one copy is safer). Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/studio/src/utils/sdkShadow.ts | 17 ++++---------- .../studio/src/utils/sdkShadowGsapFidelity.ts | 16 +++++-------- .../studio/src/utils/sdkShadowGsapKeyframe.ts | 23 ++++--------------- packages/studio/src/utils/sdkShadowNumeric.ts | 11 +++++++++ 4 files changed, 27 insertions(+), 40 deletions(-) create mode 100644 packages/studio/src/utils/sdkShadowNumeric.ts diff --git a/packages/studio/src/utils/sdkShadow.ts b/packages/studio/src/utils/sdkShadow.ts index 631eba20b..08ed05b08 100644 --- a/packages/studio/src/utils/sdkShadow.ts +++ b/packages/studio/src/utils/sdkShadow.ts @@ -12,6 +12,7 @@ import type { Composition } from "@hyperframes/sdk"; import type { EditOp, GsapTweenSpec } from "@hyperframes/sdk"; import { STUDIO_SDK_SHADOW_ENABLED } from "../components/editor/manualEditingAvailability"; import { trackStudioEvent } from "./studioTelemetry"; +import { relEqual } from "./sdkShadowNumeric"; import type { DomEditSelection } from "../components/editor/domEditingTypes"; import type { PatchOperation } from "./sourcePatcher"; @@ -388,24 +389,16 @@ 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. +// start/duration tolerate float-precision drift (SDK computes them +// arithmetically, server stores a rounded literal) via the shared relative +// epsilon; trackIndex (integer track slot) is compared exactly. function timingFieldEqual( key: keyof ShadowTiming, actual: number | null | undefined, expected: number, ): boolean { if (typeof actual === "number" && key !== "trackIndex") { - return timingValuesEqual(actual, expected); + return relEqual(actual, expected); } return actual === expected; } diff --git a/packages/studio/src/utils/sdkShadowGsapFidelity.ts b/packages/studio/src/utils/sdkShadowGsapFidelity.ts index cad5f08e0..45f800d7f 100644 --- a/packages/studio/src/utils/sdkShadowGsapFidelity.ts +++ b/packages/studio/src/utils/sdkShadowGsapFidelity.ts @@ -16,6 +16,7 @@ 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 { relEqual } from "./sdkShadowNumeric"; import type { SdkShadowMismatch, ShadowGsapOp } from "./sdkShadow"; // Marker set must match document.ts extractGsapScript so both pick the same @@ -24,7 +25,7 @@ function isGsapScriptBody(body: string): boolean { return body.includes("gsap") || body.includes("__timelines") || body.includes("ScrollTrigger"); } -function extractGsapScript(html: string): string | null { +export function extractGsapScript(html: string): string | null { // Close tag is `]*>` (not just ``) — HTML5 ignores junk // before the `>`, e.g. `` or `` (CodeQL js/bad-tag-filter). const scripts = html.match(/]*>([\s\S]*?)<\/script[^>]*>/gi); @@ -73,12 +74,9 @@ function animByKey( // number-vs-string forms. Compare canonically — sort keys, coerce numeric // strings — so only real value drift registers, not formatting differences. -// Relative-epsilon compare: the two writers round-trip durations through JS -// number formatting, so a value like 3.1 can come back as 3.0999999999999996. -// An exact `===` flags that sub-ULP delta as drift. Treat values as equal when -// they're within 1e-6 * max(1, |a|, |b|) of each other — tight enough that a -// real 2 vs 1 (or 0.5 vs 0.49) drift still flags, loose enough to absorb -// float-formatting noise. +// Coerce string operands to numbers, then compare with the shared relative +// epsilon (relEqual) so float-formatting noise (3.1 vs 3.0999999999999996) +// isn't flagged as drift while a real 2 vs 1 still is. function numericEqual(a: unknown, b: unknown): boolean { if (a === b) return true; const na = typeof a === "string" ? Number(a) : a; @@ -86,9 +84,7 @@ function numericEqual(a: unknown, b: unknown): boolean { if (typeof na !== "number" || typeof nb !== "number" || Number.isNaN(na) || Number.isNaN(nb)) { return false; } - if (na === nb) return true; - const tolerance = 1e-6 * Math.max(1, Math.abs(na), Math.abs(nb)); - return Math.abs(na - nb) <= tolerance; + return relEqual(na, nb); } function canonicalProps(obj: Record | undefined): string { diff --git a/packages/studio/src/utils/sdkShadowGsapKeyframe.ts b/packages/studio/src/utils/sdkShadowGsapKeyframe.ts index 7a657c23d..38a633162 100644 --- a/packages/studio/src/utils/sdkShadowGsapKeyframe.ts +++ b/packages/studio/src/utils/sdkShadowGsapKeyframe.ts @@ -31,7 +31,11 @@ import type { GsapPercentageKeyframe } from "@hyperframes/core/gsap-parser"; import { STUDIO_SDK_SHADOW_ENABLED } from "../components/editor/manualEditingAvailability"; import { trackStudioEvent } from "./studioTelemetry"; import type { SdkShadowMismatch } from "./sdkShadow"; -import { gsapFidelityMismatches, makeSelectorResolver } from "./sdkShadowGsapFidelity"; +import { + extractGsapScript, + gsapFidelityMismatches, + makeSelectorResolver, +} from "./sdkShadowGsapFidelity"; // Match the GSAP writer's percentage equality tolerance so a remove resolves to // the same keyframe the server would pick (writer rounds to ~3 decimals). @@ -46,23 +50,6 @@ export type ShadowKeyframeOp = } | { kind: "remove"; animationId: string; percentage: number }; -// ─── Script helpers (mirror sdkShadowGsapFidelity's extraction) ─────────────── - -function isGsapScriptBody(body: string): boolean { - return body.includes("gsap") || body.includes("__timelines") || body.includes("ScrollTrigger"); -} - -function extractGsapScript(html: string): string | null { - // Close tag is `]*>` (HTML5 ignores junk before `>`). - const scripts = html.match(/]*>([\s\S]*?)<\/script[^>]*>/gi); - if (!scripts) return null; - for (const block of scripts) { - const body = block.replace(/^]*>/i, "").replace(/<\/script[^>]*>$/i, ""); - if (isGsapScriptBody(body)) return body; - } - return null; -} - // ─── percentage → SDK op mapping ────────────────────────────────────────────── function findAnimationKeyframes( diff --git a/packages/studio/src/utils/sdkShadowNumeric.ts b/packages/studio/src/utils/sdkShadowNumeric.ts new file mode 100644 index 000000000..bf8ecd136 --- /dev/null +++ b/packages/studio/src/utils/sdkShadowNumeric.ts @@ -0,0 +1,11 @@ +/** + * Relative-epsilon numeric equality shared by the shadow diffs (timing parity + + * GSAP value fidelity). Both writers round-trip durations/positions through JS + * number formatting, so a value like 3.1 can read back as 3.0999999999999996. + * Treat values within 1e-6 * max(1, |a|, |b|) as equal — tight enough that a + * real 2 vs 1 (or 0.5 vs 0.49) still flags, loose enough to absorb float noise. + */ +export function relEqual(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)); +}