diff --git a/packages/studio/src/hooks/useDomEditSession.test.ts b/packages/studio/src/hooks/useDomEditSession.test.ts deleted file mode 100644 index 040d83b3b0..0000000000 --- a/packages/studio/src/hooks/useDomEditSession.test.ts +++ /dev/null @@ -1,41 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { shouldUseSdkCutover } from "../utils/sdkCutover"; -import type { PatchOperation } from "../utils/sourcePatcher"; - -const styleOp = (property: string, value: string): PatchOperation => ({ - type: "inline-style", - property, - value, -}); - -const attrOp = (property: string, value: string): PatchOperation => ({ - type: "attribute", - property, - value, -}); - -describe("shouldUseSdkCutover", () => { - it("returns false when flag is disabled", () => { - expect(shouldUseSdkCutover(false, true, "hf-abc", [styleOp("color", "red")])).toBe(false); - }); - - it("returns false when no SDK session", () => { - expect(shouldUseSdkCutover(true, false, "hf-abc", [styleOp("color", "red")])).toBe(false); - }); - - it("returns false when selection has no hfId", () => { - expect(shouldUseSdkCutover(true, true, null, [styleOp("color", "red")])).toBe(false); - expect(shouldUseSdkCutover(true, true, undefined, [styleOp("color", "red")])).toBe(false); - }); - - it("returns false when ops array is empty", () => { - expect(shouldUseSdkCutover(true, true, "hf-abc", [])).toBe(false); - }); - - it("returns true when all conditions met with supported op types", () => { - expect(shouldUseSdkCutover(true, true, "hf-abc", [styleOp("color", "red")])).toBe(true); - expect( - shouldUseSdkCutover(true, true, "hf-abc", [styleOp("color", "red"), attrOp("data-x", "1")]), - ).toBe(true); - }); -}); diff --git a/packages/studio/src/hooks/useDomEditSession.test.tsx b/packages/studio/src/hooks/useDomEditSession.test.tsx new file mode 100644 index 0000000000..4a70a3f0d0 --- /dev/null +++ b/packages/studio/src/hooks/useDomEditSession.test.tsx @@ -0,0 +1,277 @@ +// @vitest-environment happy-dom +import { act } from "react"; +import { createRoot } from "react-dom/client"; +import { describe, expect, it, vi } from "vitest"; +import { shouldUseSdkCutover } from "../utils/sdkCutover"; +import type { PatchOperation } from "../utils/sourcePatcher"; +import type { Composition } from "@hyperframes/sdk"; +import type { UseDomEditSessionParams } from "./useDomEditSession"; + +const styleOp = (property: string, value: string): PatchOperation => ({ + type: "inline-style", + property, + value, +}); + +const attrOp = (property: string, value: string): PatchOperation => ({ + type: "attribute", + property, + value, +}); + +describe("shouldUseSdkCutover", () => { + it("returns false when flag is disabled", () => { + expect(shouldUseSdkCutover(false, true, "hf-abc", [styleOp("color", "red")])).toBe(false); + }); + + it("returns false when no SDK session", () => { + expect(shouldUseSdkCutover(true, false, "hf-abc", [styleOp("color", "red")])).toBe(false); + }); + + it("returns false when selection has no hfId", () => { + expect(shouldUseSdkCutover(true, true, null, [styleOp("color", "red")])).toBe(false); + expect(shouldUseSdkCutover(true, true, undefined, [styleOp("color", "red")])).toBe(false); + }); + + it("returns false when ops array is empty", () => { + expect(shouldUseSdkCutover(true, true, "hf-abc", [])).toBe(false); + }); + + it("returns true when all conditions met with supported op types", () => { + expect(shouldUseSdkCutover(true, true, "hf-abc", [styleOp("color", "red")])).toBe(true); + expect( + shouldUseSdkCutover(true, true, "hf-abc", [styleOp("color", "red"), attrOp("data-x", "1")]), + ).toBe(true); + }); +}); + +// ── onReorderShadow source filter (Fix 3) ──────────────────────────────────── +// +// useDomEditSession composes ~9 sub-hooks; the only one relevant to this fix is +// useDomEditCommits, which receives the onReorderShadow callback built inline. +// Every other sub-hook is stubbed to a minimal shape so the hook under test can +// render without pulling in unrelated preview/GSAP/selection machinery. + +const recordResolverParity = vi.fn<(...args: unknown[]) => Promise>(async () => {}); +const capturedOnReorderShadow: { fn: ((targets: string[]) => void) | undefined } = { + fn: undefined, +}; + +vi.mock("../utils/sdkResolverShadow", () => ({ + runResolverShadow: vi.fn(), + recordResolverParity: (...args: unknown[]) => recordResolverParity(...args), +})); +vi.mock("./useDomEditCommits", () => ({ + useDomEditCommits: (params: { onReorderShadow?: (targets: string[]) => void }) => { + capturedOnReorderShadow.fn = params.onReorderShadow; + return { + resolveImportedFontAsset: vi.fn(), + handleDomStyleCommit: vi.fn(), + handleDomAttributeCommit: vi.fn(), + handleDomAttributeLiveCommit: vi.fn(), + handleDomHtmlAttributeCommit: vi.fn(), + handleDomTextCommit: vi.fn(), + handleDomTextFieldStyleCommit: vi.fn(), + handleDomAddTextField: vi.fn(), + handleDomRemoveTextField: vi.fn(), + handleDomBoxSizeCommit: vi.fn(), + handleDomManualEditsReset: vi.fn(), + handleDomEditElementDelete: vi.fn(), + handleDomZIndexReorderCommit: vi.fn(), + }; + }, +})); +vi.mock("./useDomSelection", () => ({ + useDomSelection: () => ({ + domEditSelection: null, + domEditGroupSelections: [], + domEditHoverSelection: null, + activeGroupElement: null, + domEditSelectionRef: { current: null }, + domEditGroupSelectionsRef: { current: [] }, + setActiveGroupElement: vi.fn(), + applyDomSelection: vi.fn(), + clearDomSelection: vi.fn(), + buildDomSelectionFromTarget: vi.fn(), + resolveDomSelectionFromPreviewPoint: vi.fn(), + resolveAllDomSelectionsFromPreviewPoint: vi.fn(), + updateDomEditHoverSelection: vi.fn(), + buildDomSelectionForTimelineElement: vi.fn(), + handleTimelineElementSelect: vi.fn(), + refreshDomEditSelectionFromPreview: vi.fn(), + applyMarqueeSelection: vi.fn(), + }), +})); +vi.mock("./useAskAgentModal", () => ({ + useAskAgentModal: () => ({ + agentModalOpen: false, + agentModalAnchorPoint: null, + copiedAgentPrompt: null, + agentPromptSelectionContext: null, + setAgentModalOpen: vi.fn(), + setAgentPromptSelectionContext: vi.fn(), + setAgentModalAnchorPoint: vi.fn(), + handleAskAgent: vi.fn(), + handleAgentModalSubmit: vi.fn(), + }), +})); +vi.mock("./useStudioSelectionPublisher", () => ({ + useStudioSelectionPublisher: () => {}, +})); +vi.mock("./useGsapTweenCache", () => ({ + useGsapCacheVersion: () => ({ version: 0, bump: vi.fn() }), +})); +vi.mock("./useGsapScriptCommits", () => ({ + useGsapScriptCommits: () => ({ + commitMutation: vi.fn(), + updateGsapProperty: vi.fn(), + updateGsapMeta: vi.fn(), + deleteGsapAnimation: vi.fn(), + deleteAllForSelector: vi.fn(), + addGsapAnimation: vi.fn(), + addGsapProperty: vi.fn(), + removeGsapProperty: vi.fn(), + updateGsapFromProperty: vi.fn(), + addGsapFromProperty: vi.fn(), + removeGsapFromProperty: vi.fn(), + addKeyframe: vi.fn(), + addKeyframeBatch: vi.fn(), + removeKeyframe: vi.fn(), + moveKeyframe: vi.fn(), + resizeKeyframedTween: vi.fn(), + convertToKeyframes: vi.fn(), + removeAllKeyframes: vi.fn(), + setArcPath: vi.fn(), + updateArcSegment: vi.fn(), + }), +})); +vi.mock("./useGroupCommits", () => ({ + useGroupCommits: () => ({ + groupSelection: vi.fn(), + ungroupSelection: vi.fn(), + }), +})); +vi.mock("./useDomEditWiring", () => ({ + useDomEditWiring: () => ({ + onClickToSource: vi.fn(), + selectedGsapAnimations: [], + gsapMultipleTimelines: false, + gsapUnsupportedTimelinePattern: false, + trackGsapInteractionFailure: vi.fn(), + makeFetchFallback: vi.fn(), + handleGsapUpdateProperty: vi.fn(), + handleGsapUpdateMeta: vi.fn(), + handleGsapDeleteAnimation: vi.fn(), + handleGsapDeleteAllForElement: vi.fn(), + handleGsapAddAnimation: vi.fn(), + handleGsapAddProperty: vi.fn(), + handleGsapRemoveProperty: vi.fn(), + handleGsapUpdateFromProperty: vi.fn(), + handleGsapAddFromProperty: vi.fn(), + handleGsapRemoveFromProperty: vi.fn(), + handleGsapAddKeyframe: vi.fn(), + handleGsapAddKeyframeBatch: vi.fn(), + handleGsapRemoveKeyframe: vi.fn(), + handleGsapMoveKeyframeToPlayhead: vi.fn(), + handleGsapMoveKeyframe: vi.fn(), + handleGsapResizeKeyframedTween: vi.fn(), + handleGsapConvertToKeyframes: vi.fn(), + handleGsapRemoveAllKeyframes: vi.fn(), + handleResetSelectedElementKeyframes: vi.fn(), + }), +})); +vi.mock("./usePreviewInteraction", () => ({ + usePreviewInteraction: () => ({ + handlePreviewCanvasMouseDown: vi.fn(), + handlePreviewCanvasPointerMove: vi.fn(), + handlePreviewCanvasPointerLeave: vi.fn(), + handleBlockedDomMove: vi.fn(), + handleDomManualDragStart: vi.fn(), + }), +})); +vi.mock("./useGsapAwareEditing", () => ({ + useGsapAwareEditing: () => ({ + handleGsapAwarePathOffsetCommit: vi.fn(), + handleGsapAwareGroupPathOffsetCommit: vi.fn(), + handleGsapAwareBoxSizeCommit: vi.fn(), + handleGsapAwareRotationCommit: vi.fn(), + commitAnimatedProperty: vi.fn(), + commitAnimatedProperties: vi.fn(), + handleSetArcPath: vi.fn(), + handleUpdateArcSegment: vi.fn(), + handleUnroll: vi.fn(), + commitMutation: vi.fn(), + }), +})); + +// Tell React this is an act-capable environment so act(...) flushes effects. +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +describe("onReorderShadow source filter", () => { + it("passes a readSource function as the 4th recordResolverParity argument when sdkSession and activeCompPath are set", async () => { + const { useDomEditSession } = await import("./useDomEditSession"); + const readProjectFile = vi.fn(async (path: string) => `content of ${path}`); + // Minimal opaque test double: no sub-hook under test actually calls into the + // session (useDomEditCommits and recordResolverParity are both mocked above), + // so it only needs to flow through as a reference. + const sdkSession = {} as unknown as Composition; + + function Probe() { + const params: UseDomEditSessionParams = { + projectId: "proj-1", + activeCompPath: "index.html", + isMasterView: false, + compIdToSrc: new Map(), + captionEditMode: false, + compositionLoading: false, + previewIframeRef: { current: null }, + timelineElements: [], + setSelectedTimelineElementId: vi.fn(), + setRightCollapsed: vi.fn(), + setRightPanelTab: vi.fn(), + showToast: vi.fn(), + refreshPreviewDocumentVersion: vi.fn(), + queueDomEditSave: vi.fn(async (save: () => Promise) => save()), + readProjectFile, + writeProjectFile: vi.fn(async () => {}), + updateEditingFileContent: vi.fn(), + domEditSaveTimestampRef: { current: 0 }, + editHistory: { recordEdit: vi.fn(async () => {}) }, + fileTree: [], + importedFontAssetsRef: { current: [] }, + projectDir: null, + projectIdRef: { current: "proj-1" }, + previewIframe: null, + refreshKey: 0, + previewDocumentVersion: 0, + rightPanelTab: "design", + applyStudioManualEditsToPreviewRef: { current: async () => {} }, + syncPreviewHistoryHotkey: vi.fn(), + reloadPreview: vi.fn(), + setRefreshKey: vi.fn(), + sdkSession, + forceReloadSdkSession: vi.fn(), + }; + useDomEditSession(params); + return null; + } + + const container = document.createElement("div"); + const root = createRoot(container); + act(() => { + root.render(); + }); + try { + expect(capturedOnReorderShadow.fn).toBeTypeOf("function"); + capturedOnReorderShadow.fn?.(["hf-target"]); + expect(recordResolverParity).toHaveBeenCalledWith( + sdkSession, + "hf-target", + "reorderElements", + expect.any(Function), + ); + } finally { + act(() => root.unmount()); + } + }); +}); diff --git a/packages/studio/src/hooks/useDomEditSession.ts b/packages/studio/src/hooks/useDomEditSession.ts index cb7646e044..185720b0d4 100644 --- a/packages/studio/src/hooks/useDomEditSession.ts +++ b/packages/studio/src/hooks/useDomEditSession.ts @@ -87,7 +87,7 @@ export function useDomEditSession({ showToast, refreshPreviewDocumentVersion, queueDomEditSave, - readProjectFile: _readProjectFile, + readProjectFile, writeProjectFile, updateEditingFileContent, domEditSaveTimestampRef, @@ -111,7 +111,6 @@ export function useDomEditSession({ forceReloadSdkSession, }: UseDomEditSessionParams) { void _setRefreshKey; - void _readProjectFile; // ── Selection ── @@ -295,7 +294,9 @@ export function useDomEditSession({ // the SDK resolves each reordered element (the reorderElements op's targets). onReorderShadow: sdkSession ? (targets: string[]) => { - for (const target of targets) recordResolverParity(sdkSession, target, "reorderElements"); + const reorderSrc = activeCompPath ? () => readProjectFile(activeCompPath) : undefined; + for (const target of targets) + void recordResolverParity(sdkSession, target, "reorderElements", reorderSrc); } : undefined, }); diff --git a/packages/studio/src/utils/sdkCutover.ts b/packages/studio/src/utils/sdkCutover.ts index 90c8af53b8..c1367b5f13 100644 --- a/packages/studio/src/utils/sdkCutover.ts +++ b/packages/studio/src/utils/sdkCutover.ts @@ -8,115 +8,9 @@ import { trackStudioEvent } from "./studioTelemetry"; import { markSelfWrite } from "../hooks/sdkSelfWriteRegistry"; import { patchOpsToSdkEditOps } from "./sdkOpMapping"; import { recordResolverParity, recordAnimationResolverParity } from "./sdkResolverShadow"; -import { isAllowedHtmlAttribute, isSafeAttributeValue } from "./htmlAttrSafety"; +import { shouldDeclineTextCutoverForTarget, shouldUseSdkCutover } from "./sdkCutoverEligibility"; -const CUTOVER_OP_TYPES = new Set([ - "inline-style", - "text-content", - "attribute", - "html-attribute", -]); - -// Mirrors the SDK's RESERVED_ATTRS (mutate.ts): a bare `attribute` op is -// force-prefixed `data-`, so e.g. property "end" → "data-end", which the SDK -// rejects with a throw. Detect that up front and decline the whole batch so it -// takes the server path cleanly, instead of throwing inside the dispatch and -// silently falling back per op. -// ponytail: small mirror of the SDK set; if the SDK adds a reserved attr, a new -// op for it just reverts to the (working) throw→fallback path until synced. -const RESERVED_CUTOVER_ATTRS = new Set([ - "data-hf-id", - "data-composition-id", - "data-width", - "data-height", - "data-start", - "data-end", - "data-track-index", - "data-hold-start", - "data-hold-end", - "data-hold-fill", -]); - -function sdkAttrName(op: PatchOperation): string | null { - if (op.type === "attribute") { - return op.property.startsWith("data-") ? op.property : `data-${op.property}`; - } - if (op.type === "html-attribute") return op.property; - return null; -} - -function mapsToReservedAttr(op: PatchOperation): boolean { - const name = sdkAttrName(op); - // Lowercase to match the SDK's validateSetAttribute (it lowercases before the - // reserved check), so "DATA-START" is declined up front too; covers both - // `attribute` (prefixed) and `html-attribute` (raw) ops. - return name !== null && RESERVED_CUTOVER_ATTRS.has(name.toLowerCase()); -} - -// ─── html-attribute safety ─────────────────────────────────────────────────── - -function hasUnsafeHtmlAttributeOp(ops: PatchOperation[]): boolean { - return ops.some( - (op) => - op.type === "html-attribute" && - (!isAllowedHtmlAttribute(op.property) || - (op.value !== null && !isSafeAttributeValue(op.property, op.value))), - ); -} - -function hasTextContentOp(ops: PatchOperation[]): boolean { - return ops.some((op) => op.type === "text-content"); -} - -function targetChildren(target: unknown): unknown[] | null { - if (!target || typeof target !== "object" || !("children" in target)) return null; - const children = (target as { children?: unknown }).children; - return Array.isArray(children) ? children : null; -} - -function elementTag(element: unknown): string | null { - if (!element || typeof element !== "object" || !("tag" in element)) return null; - const tag = (element as { tag?: unknown }).tag; - return typeof tag === "string" ? tag.toLowerCase() : null; -} - -// Tags that are non-HTML namespace elements in a linkedom-parsed HTML body. -// Mirrors the engine's `isHTMLElementTarget` (model.ts) which uses `instanceof -// HTMLElement` — that runtime check catches the same set, but we can't use it -// here because `target` is a plain SDK object, not a DOM Element. If linkedom -// (or a future parser) surfaces additional foreign-content elements as -// non-HTMLElement, add them here. -const NON_HTML_CHILD_TAGS = new Set(["svg", "math"]); - -function shouldDeclineTextCutoverForTarget(target: unknown, ops: PatchOperation[]): boolean { - if (!hasTextContentOp(ops)) return false; - const children = targetChildren(target); - if (!children) return false; - // Legacy patch-element replaces the whole element for multi-child targets and - // for single non-HTML children. The SDK text patch stream stores a scalar - // inverse, so those shapes cannot be made both byte-identical and undo-safe - // here. Let the server path remain authoritative for them. - if (children.length > 1) return true; - const tag = elementTag(children[0]); - return tag !== null && NON_HTML_CHILD_TAGS.has(tag); -} - -export function shouldUseSdkCutover( - flagEnabled: boolean, - hasSession: boolean, - hfId: string | null | undefined, - ops: PatchOperation[], -): boolean { - return ( - flagEnabled && - hasSession && - !!hfId && - ops.length > 0 && - ops.every((o) => CUTOVER_OP_TYPES.has(o.type)) && - !ops.some(mapsToReservedAttr) && - !hasUnsafeHtmlAttributeOp(ops) - ); -} +export { shouldUseSdkCutover } from "./sdkCutoverEligibility"; export interface CutoverDeps { editHistory: { @@ -271,7 +165,13 @@ export async function sdkTimingPersist( ): Promise { // Resolver tripwire — runs BEFORE the cutover gate (decoupled): records when // the SDK can't resolve a target the server timing path is addressing. - recordResolverParity(sdkSession, hfId, "setTiming"); + const timingSrc = deps.readProjectFile; + void recordResolverParity( + sdkSession, + hfId, + "setTiming", + timingSrc ? () => timingSrc(targetPath) : undefined, + ); // Dark-launch gate: without this, timing cutover runs whenever an SDK session // exists (it always does, for shadow/selection) — flipping the flag OFF would // NOT disable it. Gate here so flag-off routes back to the legacy server path. @@ -313,8 +213,15 @@ export function sdkGsapTweenPersist( // animationId (animation-resolution parity). Done here, not via // dispatchGsapOpAndPersist's resolverTarget, because the gate below returns // before that call when cutover is off. - if (op.kind === "add") recordResolverParity(sdkSession, op.target, "addGsapTween"); - else + if (op.kind === "add") { + const gsapSrc = deps.readProjectFile; + void recordResolverParity( + sdkSession, + op.target, + "addGsapTween", + gsapSrc ? () => gsapSrc(targetPath) : undefined, + ); + } else recordAnimationResolverParity( sdkSession, op.animationId, @@ -576,7 +483,9 @@ export async function sdkDeletePersist( deps: CutoverDeps, ): Promise { // Resolver tripwire — runs BEFORE the cutover gate (decoupled). - recordResolverParity(sdkSession, hfId, "removeElement"); + void recordResolverParity(sdkSession, hfId, "removeElement", () => + Promise.resolve(originalContent), + ); // Dark-launch gate: flag OFF → legacy server delete path. if (!STUDIO_SDK_CUTOVER_ENABLED) return false; if (!sdkSession || !sdkSession.getElement(hfId)) return false; diff --git a/packages/studio/src/utils/sdkCutoverEligibility.ts b/packages/studio/src/utils/sdkCutoverEligibility.ts new file mode 100644 index 0000000000..b16642b5d0 --- /dev/null +++ b/packages/studio/src/utils/sdkCutoverEligibility.ts @@ -0,0 +1,116 @@ +/** + * Cutover eligibility checks: whether a batch of patch ops is safe to route + * through the SDK cutover path instead of the legacy server path. Split out of + * sdkCutover.ts (which hit the packages/studio 600-line filesize cap) — this + * block has no dependency on the persist/dispatch functions there. + */ +import type { PatchOperation } from "./sourcePatcher"; +import { isAllowedHtmlAttribute, isSafeAttributeValue } from "./htmlAttrSafety"; + +const CUTOVER_OP_TYPES = new Set([ + "inline-style", + "text-content", + "attribute", + "html-attribute", +]); + +// Mirrors the SDK's RESERVED_ATTRS (mutate.ts): a bare `attribute` op is +// force-prefixed `data-`, so e.g. property "end" → "data-end", which the SDK +// rejects with a throw. Detect that up front and decline the whole batch so it +// takes the server path cleanly, instead of throwing inside the dispatch and +// silently falling back per op. +// ponytail: small mirror of the SDK set; if the SDK adds a reserved attr, a new +// op for it just reverts to the (working) throw→fallback path until synced. +const RESERVED_CUTOVER_ATTRS = new Set([ + "data-hf-id", + "data-composition-id", + "data-width", + "data-height", + "data-start", + "data-end", + "data-track-index", + "data-hold-start", + "data-hold-end", + "data-hold-fill", +]); + +function sdkAttrName(op: PatchOperation): string | null { + if (op.type === "attribute") { + return op.property.startsWith("data-") ? op.property : `data-${op.property}`; + } + if (op.type === "html-attribute") return op.property; + return null; +} + +function mapsToReservedAttr(op: PatchOperation): boolean { + const name = sdkAttrName(op); + // Lowercase to match the SDK's validateSetAttribute (it lowercases before the + // reserved check), so "DATA-START" is declined up front too; covers both + // `attribute` (prefixed) and `html-attribute` (raw) ops. + return name !== null && RESERVED_CUTOVER_ATTRS.has(name.toLowerCase()); +} + +// ─── html-attribute safety ─────────────────────────────────────────────────── + +function hasUnsafeHtmlAttributeOp(ops: PatchOperation[]): boolean { + return ops.some( + (op) => + op.type === "html-attribute" && + (!isAllowedHtmlAttribute(op.property) || + (op.value !== null && !isSafeAttributeValue(op.property, op.value))), + ); +} + +function hasTextContentOp(ops: PatchOperation[]): boolean { + return ops.some((op) => op.type === "text-content"); +} + +function targetChildren(target: unknown): unknown[] | null { + if (!target || typeof target !== "object" || !("children" in target)) return null; + const children = (target as { children?: unknown }).children; + return Array.isArray(children) ? children : null; +} + +function elementTag(element: unknown): string | null { + if (!element || typeof element !== "object" || !("tag" in element)) return null; + const tag = (element as { tag?: unknown }).tag; + return typeof tag === "string" ? tag.toLowerCase() : null; +} + +// Tags that are non-HTML namespace elements in a linkedom-parsed HTML body. +// Mirrors the engine's `isHTMLElementTarget` (model.ts) which uses `instanceof +// HTMLElement` — that runtime check catches the same set, but we can't use it +// here because `target` is a plain SDK object, not a DOM Element. If linkedom +// (or a future parser) surfaces additional foreign-content elements as +// non-HTMLElement, add them here. +const NON_HTML_CHILD_TAGS = new Set(["svg", "math"]); + +export function shouldDeclineTextCutoverForTarget(target: unknown, ops: PatchOperation[]): boolean { + if (!hasTextContentOp(ops)) return false; + const children = targetChildren(target); + if (!children) return false; + // Legacy patch-element replaces the whole element for multi-child targets and + // for single non-HTML children. The SDK text patch stream stores a scalar + // inverse, so those shapes cannot be made both byte-identical and undo-safe + // here. Let the server path remain authoritative for them. + if (children.length > 1) return true; + const tag = elementTag(children[0]); + return tag !== null && NON_HTML_CHILD_TAGS.has(tag); +} + +export function shouldUseSdkCutover( + flagEnabled: boolean, + hasSession: boolean, + hfId: string | null | undefined, + ops: PatchOperation[], +): boolean { + return ( + flagEnabled && + hasSession && + !!hfId && + ops.length > 0 && + ops.every((o) => CUTOVER_OP_TYPES.has(o.type)) && + !ops.some(mapsToReservedAttr) && + !hasUnsafeHtmlAttributeOp(ops) + ); +} diff --git a/packages/studio/src/utils/sdkResolverShadow.test.ts b/packages/studio/src/utils/sdkResolverShadow.test.ts index 5e6b9da2a4..5f91670796 100644 --- a/packages/studio/src/utils/sdkResolverShadow.test.ts +++ b/packages/studio/src/utils/sdkResolverShadow.test.ts @@ -345,7 +345,7 @@ describe("F. recordResolverParity", () => { it("emits element_not_found when the SDK cannot resolve the target", async () => { mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true; const session = await openComposition(BASE_HTML); - recordResolverParity(session, "hf-missing", "setTiming"); + await recordResolverParity(session, "hf-missing", "setTiming"); const ev = lastShadow(); expect(ev?.mismatchCount).toBe(1); expect(ev?.opLabel).toBe("setTiming"); @@ -355,7 +355,7 @@ describe("F. recordResolverParity", () => { it("emits nothing when the target resolves (parity)", async () => { mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true; const session = await openComposition(BASE_HTML); - recordResolverParity(session, "hf-box", "removeElement"); + await recordResolverParity(session, "hf-box", "removeElement"); expect(trackedEvents.filter((e) => e.event === "sdk_resolver_shadow")).toHaveLength(0); }); @@ -363,7 +363,7 @@ describe("F. recordResolverParity", () => { mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = false; const session = await openComposition(BASE_HTML); const spy = vi.spyOn(session, "getElement"); - recordResolverParity(session, "hf-missing", "setTiming"); + await recordResolverParity(session, "hf-missing", "setTiming"); expect(trackedEvents).toHaveLength(0); expect(spy).not.toHaveBeenCalled(); }); @@ -371,9 +371,58 @@ describe("F. recordResolverParity", () => { it("never mutates the session (read-only resolver check)", async () => { mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true; const session = await openComposition(BASE_HTML); - recordResolverParity(session, "hf-box", "setTiming"); + await recordResolverParity(session, "hf-box", "setTiming"); expect(session.getElement("hf-box")?.inlineStyles.color).toBe("red"); // unchanged }); + + it("suppresses the emit when the hfId is absent from source (runtime node)", async () => { + mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true; + const session = await openComposition(BASE_HTML); + await recordResolverParity(session, "hf-runtime", "setTiming", () => + Promise.resolve('
'), + ); + expect(trackedEvents.filter((e) => e.event === "sdk_resolver_shadow")).toHaveLength(0); + }); + + it("emits with sourceHfIdCount=1 when the hfId IS in source but missing from the session", async () => { + mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true; + const session = await openComposition(BASE_HTML); + await recordResolverParity(session, "hf-ghost", "setTiming", () => + Promise.resolve('
'), + ); + const ev = lastShadow(); + expect(ev?.mismatchCount).toBe(1); + expect(ev?.sourceHfIdCount).toBe(1); + }); + + it("reports sourceHfIdCount=2 for a duplicate-id source (ambiguity)", async () => { + mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true; + const session = await openComposition(BASE_HTML); + await recordResolverParity(session, "hf-dup", "setTiming", () => + Promise.resolve(''), + ); + expect(lastShadow()?.sourceHfIdCount).toBe(2); + }); + + it("emits without sourceHfIdCount when no reader is supplied (status quo)", async () => { + mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true; + const session = await openComposition(BASE_HTML); + await recordResolverParity(session, "hf-missing", "setTiming"); + const ev = lastShadow(); + expect(ev?.mismatchCount).toBe(1); + expect(ev?.sourceHfIdCount).toBeUndefined(); + }); + + it("fails open: a readSource error still emits (no suppression)", async () => { + mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true; + const session = await openComposition(BASE_HTML); + await recordResolverParity(session, "hf-missing", "setTiming", () => + Promise.reject(new Error("read failed")), + ); + const ev = lastShadow(); + expect(ev?.mismatchCount).toBe(1); + expect(ev?.sourceHfIdCount).toBeUndefined(); + }); }); // ─── G. recordAnimationResolverParity (GSAP animationId ops) ────────────────── @@ -442,7 +491,7 @@ describe("H. inlined sub-composition leaf", () => { it("recordResolverParity emits NOTHING for a bare leaf inside a sub-comp", async () => { mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true; const session = await openComposition(INLINED_HTML); - recordResolverParity(session, "hf-leaf", "setTiming"); + await recordResolverParity(session, "hf-leaf", "setTiming"); expect(trackedEvents.filter((e) => e.event === "sdk_resolver_shadow")).toHaveLength(0); }); diff --git a/packages/studio/src/utils/sdkResolverShadow.ts b/packages/studio/src/utils/sdkResolverShadow.ts index a80706eb3a..e3347956f4 100644 --- a/packages/studio/src/utils/sdkResolverShadow.ts +++ b/packages/studio/src/utils/sdkResolverShadow.ts @@ -183,6 +183,9 @@ export function sdkResolverShadowCheck( // a genuine resolver divergence (the v0.6.110 class) — keep emitting that. // ponytail: substring match; biases toward keeping signal on a loose hit. if (sourceContent !== undefined && !sourceContent.includes(hfId)) return []; + // Loose match here vs. countHfIdInSource's strict data-hf-id="..." match in the + // caller (runResolverShadow) means an emitted event can carry sourceHfIdCount: 0 — + // see the comment on that field in runResolverShadow for what 0 means in that case. return [{ kind: "element_not_found", hfId }]; } @@ -275,10 +278,14 @@ export function runResolverShadow( // sessionElementCount === 0 = session is empty/broken (actionable). sessionElementCount: session.getElements().length, // Count of data-hf-id="" occurrences in source for an emitted - // element_not_found (the runtime-node filter already dropped absent-from- - // source ids, so an emitted one is in source ≥1×). >1 = duplicate ids → - // resolver picked the wrong instance; =1 = single static node the SDK - // parse dropped (foreign-content exclusion / sub-comp inlining gap). + // element_not_found. >1 = duplicate ids → resolver picked the wrong + // instance; =1 = single static node the SDK parse dropped (foreign-content + // exclusion / sub-comp inlining gap); =0 = the runtime-node filter above + // uses a loose substring match (biased toward keeping signal) while this + // count uses a strict attribute match — an emitted event with count 0 + // means hfId appeared in source as plain text (e.g. a class name, comment, + // or script string) but never as a data-hf-id attribute. Treat 0 as "not + // a genuine attribute occurrence," not as a contradiction. ...(isElementNotFound && sourceContent !== undefined ? { sourceHfIdCount: countHfIdInSource(sourceContent, hfId) } : {}), @@ -300,19 +307,42 @@ export function runResolverShadow( * * No-op when the shadow flag is off; never throws; never mutates the session. */ -export function recordResolverParity( +export async function recordResolverParity( session: Composition | null | undefined, hfId: string | null | undefined, opLabel: string, -): void { + readSource?: () => Promise, +): Promise { if (!STUDIO_SDK_RESOLVER_SHADOW_ENABLED) return; if (!session || !hfId) return; try { if (resolveSnapshot(session, hfId)) return; // resolves — parity, nothing to record + // Capture BEFORE any await: this call is fire-and-forget (`void recordResolverParity(...)`) + // and the caller runs its own session mutation synchronously right after this call + // returns. getElements() caches and that cache is invalidated on dispatch, so reading + // the count after an await would silently reflect POST-edit state, not the pre-edit + // state this field exists to diagnose. + const sessionElementCount = session.getElements().length; + // Cheap check passed above, so the source read only runs on a real divergence. + let source: string | undefined; + if (readSource) { + try { + source = await readSource(); + } catch { + source = undefined; // fail-open: a read error must not drop a real divergence + } + } + // Runtime-generated node the static parse can't model — suppress (mirrors the dom-edit path). + if (source !== undefined && !source.includes(hfId)) return; trackStudioEvent("sdk_resolver_shadow", { hfId, opLabel, - sessionElementCount: session.getElements().length, + sessionElementCount, + // sourceHfIdCount: strict data-hf-id="..." attribute count. Can be 0 even + // on an emitted (non-suppressed) event — the suppression check above is a + // loose substring match (biased toward keeping signal); see the longer + // comment on this field in runResolverShadow for the full explanation. + ...(source !== undefined ? { sourceHfIdCount: countHfIdInSource(source, hfId) } : {}), mismatchCount: 1, mismatches: JSON.stringify([ { kind: "element_not_found", hfId } satisfies SdkResolverMismatch,