mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(studio): extend resolver-shadow runtime-node filter to timing/GSAP/reorder chokepoints (#1839)
* fix(studio): source-filter recordResolverParity (async + sourceHfIdCount) * refactor(studio): extract cutover-eligibility checks to unblock filesize cap * fix(studio): pass on-disk source to recordResolverParity at timing/gsap/delete chokepoints * fix(studio): void the reorderElements recordResolverParity call (now async) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(studio): resolve max-review findings (telemetry race, await, reorder filter, count invariant) --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
e5ec4cf532
commit
c215a20c8e
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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<void>>(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<void>) => 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(<Probe />);
|
||||
});
|
||||
try {
|
||||
expect(capturedOnReorderShadow.fn).toBeTypeOf("function");
|
||||
capturedOnReorderShadow.fn?.(["hf-target"]);
|
||||
expect(recordResolverParity).toHaveBeenCalledWith(
|
||||
sdkSession,
|
||||
"hf-target",
|
||||
"reorderElements",
|
||||
expect.any(Function),
|
||||
);
|
||||
} finally {
|
||||
act(() => root.unmount());
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -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,
|
||||
});
|
||||
|
||||
@@ -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<PatchOperation["type"]>([
|
||||
"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<string>([
|
||||
"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<boolean> {
|
||||
// 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<boolean> {
|
||||
// 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;
|
||||
|
||||
@@ -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<PatchOperation["type"]>([
|
||||
"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<string>([
|
||||
"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)
|
||||
);
|
||||
}
|
||||
@@ -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('<div data-hf-id="hf-other"></div>'),
|
||||
);
|
||||
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('<div data-hf-id="hf-ghost"></div>'),
|
||||
);
|
||||
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('<a data-hf-id="hf-dup"></a><b data-hf-id="hf-dup"></b>'),
|
||||
);
|
||||
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);
|
||||
});
|
||||
|
||||
|
||||
@@ -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="<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<string | undefined>,
|
||||
): Promise<void> {
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user