feat(studio): a blocked GSAP edit records which of seven causes it hit (#4214)

`edit_blocked` is the largest interaction failure in Studio: 575 users in 14
days, 17% of everyone who touches an editing control, and 41% of them are
blocked on their first or second attempt. Only 30% ever commit anything
afterwards.

Almost all of it is one message. "This animation is computed at runtime. Edit
the animation in the Code tab." covers ~460 of those users and is thrown from
seven different places — runtime-dynamic provenance, unresolved keyframes, an
unresolved selector, a geometry commit over unresolved source, and three
live-tween-without-a-source-match cases across position and rotation. The
telemetry carried only that string, so the classes were indistinguishable.

The copy is also only literally true for the first. The rest are parser or
source-matching limits where the animation may be plain authored source, and
we send the user to the Code tab to rewrite something that could be fine.

Adds a `detail` beside the coarse reason, set at each of the seven origins and
emitted as `block_detail`. Telemetry only; nothing branches on it, and the
user-facing copy is unchanged. Which cause dominates decides whether the fix
is better copy, a parser gap, or an Unroll-style recovery path — and guessing
that without the split is what this is meant to avoid.

`directEditOutcomeForProperties` rebuilt blocked outcomes from `reason` alone,
so it dropped the detail for the three causes that flow through it; it now
carries it. Mutation testing caught that, not review.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Vance Ingalls
2026-09-20 07:42:37 +00:00
committed by GitHub
co-authored by Claude Opus 5
parent c204b3c757
commit 05d5abb926
8 changed files with 245 additions and 21 deletions
+1 -1
View File
@@ -135,7 +135,7 @@ export async function materializeIfDynamic(
void iframe;
void commitMutation;
void selection;
throw new GsapEditBlockedError("source-uneditable");
throw new GsapEditBlockedError("source-uneditable", "geometry-unresolved-source");
}
// ── Drag → GSAP position math ──────────────────────────────────────────────
@@ -0,0 +1,115 @@
import type { GsapAnimation } from "@hyperframes/core/gsap-parser";
import { describe, expect, it } from "vitest";
import { buildStudioSaveFailureProperties } from "../utils/studioSaveDiagnostics";
import {
GsapEditBlockedError,
assertGsapEditPersisted,
directEditOutcomeForProperties,
} from "./gsapEditOutcome";
function blockDetailFor(error: unknown): unknown {
return buildStudioSaveFailureProperties({ source: "gsap_commit", error }).block_detail;
}
/** Minimal animation that writes `x`, so the editability check actually runs. */
function animation(overrides: Partial<GsapAnimation> = {}): GsapAnimation {
return {
id: "a1",
targetSelector: "#box",
method: "to",
position: 0,
properties: { x: 100 },
...overrides,
} as GsapAnimation;
}
const POSITION = new Set(["x"]);
describe("a blocked GSAP edit names which of the nine causes it hit", () => {
// The three causes inside assertGsapAnimationDirectlyEditable share one
// message, so only the detail distinguishes them. Asserted through the
// exported wrapper because that is the path a real edit takes — and the
// wrapper used to rebuild the outcome from `reason` alone, dropping it.
it("reports a runtime-dynamic provenance", () => {
const outcome = directEditOutcomeForProperties(
[animation({ provenance: { kind: "runtime-dynamic" } })],
POSITION,
);
expect(outcome).toEqual({
status: "blocked",
reason: "source-uneditable",
detail: "provenance-runtime-dynamic",
});
});
it("reports unresolved keyframes separately from provenance", () => {
const outcome = directEditOutcomeForProperties(
[animation({ hasUnresolvedKeyframes: true })],
POSITION,
);
expect(outcome).toEqual({
status: "blocked",
reason: "source-uneditable",
detail: "unresolved-keyframes",
});
});
it("reports an unresolved selector separately from provenance", () => {
const outcome = directEditOutcomeForProperties(
[animation({ hasUnresolvedSelector: true })],
POSITION,
);
expect(outcome).toEqual({
status: "blocked",
reason: "source-uneditable",
detail: "unresolved-selector",
});
});
it("still routes a helper or loop to unroll, which has its own message", () => {
const outcome = directEditOutcomeForProperties(
[animation({ provenance: { kind: "loop" } })],
POSITION,
);
expect(outcome).toEqual({ status: "blocked", reason: "unroll-required", detail: undefined });
});
it("leaves a literal tween editable", () => {
expect(directEditOutcomeForProperties([animation()], POSITION)).toEqual({
status: "persisted",
});
});
it("carries the detail through a returned outcome, not just a thrown one", () => {
// gsapRuntimeBridge returns blocked outcomes rather than throwing.
expect(() =>
assertGsapEditPersisted({
status: "blocked",
reason: "source-uneditable",
detail: "no-position-tween",
}),
).toThrow(
expect.objectContaining({ name: "GsapEditBlockedError", detail: "no-position-tween" }),
);
});
it("surfaces the detail on the telemetry payload", () => {
expect(
blockDetailFor(
new GsapEditBlockedError("source-uneditable", "live-rotation-no-source-tween"),
),
).toBe("live-rotation-no-source-tween");
});
it("omits the detail for a block that has no specific cause", () => {
expect(blockDetailFor(new GsapEditBlockedError("no-selector"))).toBeUndefined();
});
it("ignores a detail-shaped field on an error that is not a block", () => {
// Guards the name check: any error can carry a `detail`, and reporting a
// save failure's detail as a block cause would be a silent mislabel.
const impostor = Object.assign(new Error("network down"), { detail: "unresolved-selector" });
expect(blockDetailFor(impostor)).toBeUndefined();
expect(blockDetailFor("not an error")).toBeUndefined();
});
});
+39 -10
View File
@@ -2,6 +2,27 @@ import { editabilityForProvenance, type GsapAnimation } from "@hyperframes/core/
export type GsapEditBlockReason = "no-selector" | "unroll-required" | "source-uneditable";
/**
* Which of the nine situations produced a block. The user-facing `reason` stays
* coarse — three messages — but "source-uneditable" alone covers nine distinct
* causes, and `edit_blocked` telemetry could not tell them apart. That matters
* because the copy ("This animation is computed at runtime") is only literally
* true for `provenance-runtime-dynamic`; the others are parser or source-match
* limits, where the animation may well be plain authored source.
*
* Telemetry only. Nothing branches on it.
*/
export type GsapEditBlockDetail =
| "provenance-runtime-dynamic"
| "unresolved-keyframes"
| "unresolved-selector"
| "geometry-unresolved-source"
| "live-position-no-source-tween"
| "no-position-tween"
| "live-rotation-no-source-tween"
| "live-resize-no-source-tween"
| "zero-duration-tween";
export type GsapEditOutcome =
| {
status: "persisted";
@@ -22,7 +43,7 @@ export type GsapEditOutcome =
*/
ownsDragOffset?: boolean;
}
| { status: "blocked"; reason: GsapEditBlockReason };
| { status: "blocked"; reason: GsapEditBlockReason; detail?: GsapEditBlockDetail };
const COPY: Record<GsapEditBlockReason, string> = {
"no-selector": "This layer needs a stable selector before Studio can save the edit.",
@@ -32,25 +53,32 @@ const COPY: Record<GsapEditBlockReason, string> = {
};
export class GsapEditBlockedError extends Error {
constructor(readonly reason: GsapEditBlockReason) {
constructor(
readonly reason: GsapEditBlockReason,
readonly detail?: GsapEditBlockDetail,
) {
super(COPY[reason]);
this.name = "GsapEditBlockedError";
}
}
export function assertGsapEditPersisted(outcome: GsapEditOutcome): void {
if (outcome.status === "blocked") throw new GsapEditBlockedError(outcome.reason);
if (outcome.status === "blocked") throw new GsapEditBlockedError(outcome.reason, outcome.detail);
}
function assertGsapAnimationDirectlyEditable(animation: GsapAnimation): void {
const editability = editabilityForProvenance(animation.provenance);
if (editability === "unroll") throw new GsapEditBlockedError("unroll-required");
if (
editability === "source" ||
animation.hasUnresolvedKeyframes ||
animation.hasUnresolvedSelector
) {
throw new GsapEditBlockedError("source-uneditable");
// Same message for all three, but they are different problems: only the first
// is genuinely a runtime-computed value.
if (editability === "source") {
throw new GsapEditBlockedError("source-uneditable", "provenance-runtime-dynamic");
}
if (animation.hasUnresolvedKeyframes) {
throw new GsapEditBlockedError("source-uneditable", "unresolved-keyframes");
}
if (animation.hasUnresolvedSelector) {
throw new GsapEditBlockedError("source-uneditable", "unresolved-selector");
}
}
@@ -84,7 +112,8 @@ export function directEditOutcomeForProperties(
}
return { status: "persisted" };
} catch (error) {
if (isGsapEditBlockedError(error)) return { status: "blocked", reason: error.reason };
if (isGsapEditBlockedError(error))
return { status: "blocked", reason: error.reason, detail: error.detail };
throw error;
}
}
@@ -180,11 +180,15 @@ it("reuses the ownership parse instead of fetching a resolved size group twice",
);
});
it("blocks when runtime size motion exists but the authored tween cannot be resolved", async () => {
function makeBoxSelection(): { el: HTMLElement; selection: DomEditSelection } {
const el = document.createElement("div");
el.id = "box";
document.body.append(el);
const selection = { id: "box", selector: "#box", element: el } as DomEditSelection;
return { el, selection: { id: "box", selector: "#box", element: el } as DomEditSelection };
}
it("blocks when runtime size motion exists but the authored tween cannot be resolved", async () => {
const { el, selection } = makeBoxSelection();
const liveSizeTween = {
targets: () => [el],
vars: { width: 300, duration: 1 },
@@ -199,7 +203,42 @@ it("blocks when runtime size motion exists but the authored tween cannot be reso
await expect(
tryGsapResizeIntercept(selection, { width: 344, height: 344 }, [], iframe, commitMutation),
).resolves.toEqual({ status: "blocked", reason: "source-uneditable" });
).resolves.toEqual({
status: "blocked",
reason: "source-uneditable",
detail: "live-resize-no-source-tween",
});
expect(commitMutation).not.toHaveBeenCalled();
});
it("blocks a tween with no positive duration instead of committing into it", async () => {
const { selection } = makeBoxSelection();
// A zero-duration `from` is not an instant hold (those are `set`/`to`), so it
// reaches the duration check: a resize has no timeline position to land on.
const zeroDurationTween = {
id: "#box-size",
targetSelector: "#box",
propertyGroup: "size",
method: "from",
position: 0,
duration: 0,
properties: { width: 150, height: 150 },
} as unknown as GsapAnimation;
const commitMutation = vi.fn();
await expect(
tryGsapResizeIntercept(
selection,
{ width: 344, height: 344 },
[zeroDurationTween],
null,
commitMutation,
),
).resolves.toEqual({
status: "blocked",
reason: "source-uneditable",
detail: "zero-duration-tween",
});
expect(commitMutation).not.toHaveBeenCalled();
});
@@ -137,7 +137,12 @@ export async function tryGsapResizeIntercept(
? hasNonHoldTweenForElement(iframe, liveSelector, undefined, [...resizeProperties])
: false;
if (!anim && hasLiveResizeTween) {
return { status: "blocked", reason: "source-uneditable" };
// Third twin of the position/rotation cases: a live tween with no source match.
return {
status: "blocked",
reason: "source-uneditable",
detail: "live-resize-no-source-tween",
};
}
logResize("intercept-enter", {
hasScaleGroup,
@@ -194,7 +199,12 @@ export async function tryGsapResizeIntercept(
}
const tweenDuration = resolveTweenDuration(anim);
if (tweenDuration <= 0) return { status: "blocked", reason: "source-uneditable" };
if (tweenDuration <= 0) {
// The tween exists in source but has no positive duration, so there is no
// timeline position at which a resize could land — a different cause from
// the live-tween-without-a-source-match cases above.
return { status: "blocked", reason: "source-uneditable", detail: "zero-duration-tween" };
}
const { activeKeyframePct, setActiveKeyframePct } = usePlayerStore.getState();
const pct = activeKeyframePct ?? computeCurrentPercentage(selection, anim);
@@ -94,7 +94,11 @@ describe("tryGsapDragIntercept — stale-parse guard (no resurrection after dele
vi.fn().mockResolvedValue([]),
);
expect(result).toEqual({ status: "blocked", reason: "source-uneditable" });
expect(result).toEqual({
status: "blocked",
reason: "source-uneditable",
detail: "live-position-no-source-tween",
});
expect(commitMutation).not.toHaveBeenCalled();
});
@@ -354,7 +358,11 @@ describe("tryGsapRotationIntercept — instant holds", () => {
fakeIframe("puck-b", [liveRotation]),
commitMutation,
),
).resolves.toEqual({ status: "blocked", reason: "source-uneditable" });
).resolves.toEqual({
status: "blocked",
reason: "source-uneditable",
detail: "live-rotation-no-source-tween",
});
expect(commitMutation).not.toHaveBeenCalled();
});
+14 -3
View File
@@ -156,7 +156,13 @@ async function preflightGsapDragIntercept(
const hasLivePosition = hasNonHoldTweenForElement(iframe, selector, undefined, POSITION_CHANNELS);
if (hasLivePosition && !posAnim) {
return { status: "blocked", reason: "source-uneditable" };
// GSAP is visibly moving this element but the parser found no position
// tween for it — a source-match gap, not necessarily computed source.
return {
status: "blocked",
reason: "source-uneditable",
detail: "live-position-no-source-tween",
};
}
if (!posAnim && !writeTargetSelector(selection)) {
return { status: "blocked", reason: "no-selector" };
@@ -268,7 +274,7 @@ export async function tryGsapDragIntercept(
}
if (!posAnim) {
return { status: "blocked", reason: "source-uneditable" };
return { status: "blocked", reason: "source-uneditable", detail: "no-position-tween" };
}
// Verify the anim ID is still valid in the current file. The React-state
@@ -353,7 +359,12 @@ export async function tryGsapRotationIntercept(
? hasNonHoldTweenForElement(iframe, liveSelector, undefined, ROTATION_CHANNELS)
: false;
if (!anim && hasLiveRotationTween) {
return { status: "blocked", reason: "source-uneditable" };
// Rotation twin of the position case above: live tween, no source match.
return {
status: "blocked",
reason: "source-uneditable",
detail: "live-rotation-no-source-tween",
};
}
// `angle` is the ABSOLUTE target rotation resolved by the gesture (gsap base +
@@ -165,9 +165,21 @@ export function buildStudioSaveFailureProperties(
target_id: input.targetId ?? undefined,
target_selector: input.targetSelector ?? undefined,
target_source_file: input.targetSourceFile ?? undefined,
block_detail: gsapBlockDetail(input.error),
};
}
/**
* The specific cause behind a blocked GSAP edit. `error_message` only carries
* the coarse copy, and one of those strings covers six different situations,
* so the message alone cannot say which one a user hit.
*/
function gsapBlockDetail(error: unknown): string | undefined {
if (!(error instanceof Error) || error.name !== "GsapEditBlockedError") return undefined;
const detail = (error as { detail?: unknown }).detail;
return typeof detail === "string" ? detail : undefined;
}
export function trackStudioSaveFailure(input: StudioSaveFailureInput): void {
trackStudioEvent("save_failure", buildStudioSaveFailureProperties(input));
}