fix(studio): resize an element whose scale is an instant hold

[miguel-heygen]

What

Three faults in resizing an element whose size is driven by a scale animation. The first two are the same element shape, an instant hold: a duration: 0 tween carrying scale, which is how a composition pins a static scale.

1. The resize wrote size into the tween carrying scale. That left one tween spanning two property groups. The parser classifies such a tween as neither, so it loses its group suffix and its id along with it: #card-to-0-scale becomes plain #card-to-0. Every later edit went looking for a scale tween and a size tween, found a tween with no group at all, and had nothing it could address. The element failed to save with animation not found and could not be edited again.

2. The drag offset was withheld from a resize that never moved the element. The size saved and the element then snapped back to its authored position on release, every drag.

3. A near-uniform drag silently lost up to one percent on one axis. Drop a 630x408 box at 326x213 and it came back 326x211.

Why

1. The tween resolved for the resize's group is, for a scale-driven element, the one carrying scale. When it is an instant hold the code handed it straight to the size commit, which wrote width and height into it. Size and scale are separate property groups for a reason; putting both in one tween makes it addressable as neither.

2. Whether the caller persists the drag offset was inferred from the element's tweens: a scale-group tween meant "the resize settles its own position, hold the offset back". That is true of the scale route, which commits a scale and then measures where centre-scaling put the box. It is not true of an element whose scale is an instant hold, which has a scale-group tween and still commits width/height, moving nothing. So the offset was withheld, nobody wrote it, and the position tween re-asserted the authored value a frame later.

3. The test for "is this drag uniform enough to commit one scale value" was a fixed 0.01 of scale. That is invisible on a 40px box and two pixels of height on a 408px one, and the question was never about scale: it is only ever whether using one value for both axes would move an edge.

How

1. Size goes to a size hold of its own; the scale hold is left alone.

2. The resize outcome carries the answer instead of the caller guessing. GsapEditOutcome gains ownsDragOffset on the persisted case: the three scale-route returns that run the centre-scaling correction set it, and every other route leaves it unset so the caller forwards the offset exactly as it does for a plain size resize.

3. The uniformity test asks its real question, in pixels, against the axis a collapse would distort.

Where fault 1 has already damaged a composition it is repairable without hand-editing: splitting the mixed tween into property groups gives back a scale tween and a size tween.

Test plan

How was this tested?

• Unit tests added/updated
• Manual testing performed
• Documentation updated (if applicable)

Three targeted tests in gsapResizeMixedTween.test.ts, driving the real intercept against the five-tween shape that surfaced this, with a server stand-in that rejects an id it does not have. Each verified to fail with its fix reverted:

• a resize does not write width/height into the tween carrying scale
• it never sends an animation id the source does not have
• it leaves the drag offset to the caller when it commits size rather than scale

Two sweeps, because both faults were found one composition at a time and that is a bad way to find the third:

• gsapResizeSweep.test.ts: 108 shapes, the cross-product of scale absent / instant hold / tween / longhands, size absent / hold / tween, position absent / static hold / tween, and either nothing extra, the 3D-and-rotation set a card carries, or a tween that already spans two groups. Every run held to two rules: never address an animation the source does not have, never leave a tween spanning two property groups. Plus six playheads across two scale tweens, pinning which tween a resize edits when the element has several; three of the six fail against a stubbed selection that always takes the first.
• gsapResizeGeometrySweep.test.ts: 120 runs over the five routes, six rotations from none to 180 degrees, and four drops from near-zero to an aspect flip, checking the committed scale or size reproduces the rendered box that was dropped, and that the box lands on the drop point where the resize reports it owns the offset. Six failed before fix 3, all of them the near-uniform shrink, at every rotation including none. Rotation was the suspect and came out clean.

Verified in Studio against a real composition, and A/B'd rather than assumed: with the fix in, the element holds its drop point; with it reverted, the snap-back to the authored transform returns on every drag.

Suites green: studio 3507, studio-server 416.

Not covered

• Real pointer gestures. Everything here drives the commit path directly.
• Corner behaviour under rotation is out of scope by construction: this resize is centre-anchored, and the gesture layer already sweeps rotations for both the size math and the centre pin.
• Undo after a resize is covered by composition rather than by a new test: one gesture takes one coalesce key with an infinite window, and both the key injection and the history folding rule have their own tests.

[vanceingalls]

APPROVE @ 1aa0fdb

Three targeted fixes to the resize intercept, each with a matching test that would fail with the fix reverted. The sizeSet guard (fix 1), the ownsDragOffset outcome channel (fix 2), and the pixel-scoped uniformity check (fix 3) are all correct in their primary scenarios, and the coordinate contract in computeDraggedGsapPosition is untouched so preview/commit still agree by construction. New sweeps (gsapResizeSweep.test.ts structural + gsapResizeGeometrySweep.test.ts geometric) harvest commitMutation.mock.calls and reconstruct the AABB against the drop point — CORRECT persisted-state assertions, guard semantics not presence.

1. [NOTE] packages/studio/src/hooks/gsapResizeIntercept.ts:432,544,566 — the three ownsDragOffset: true returns fire regardless of resizeGroup. PR body says "the three scale-route returns that run the centre-scaling correction set it, and every other route leaves it unset", but the returns are reached whenever anim is a real non-instant-hold tween — including the size-tween route (element has no scale group, only a real width/height tween). In that case scaleDraftEl is null, finalizeScaleResizeCommit is a no-op, and the caller in useGsapAwareEditing.ts:275 will now withhold the drag offset — regressing the pre-existing behaviour where scaleRoute=false forwarded the offset through tryGsapDragIntercept. settle() still writes to the live DOM, so the release frame looks right; the persisted state does not, so a re-seek reverts. Neither gsapResizeGeometrySweep.test.ts (mock's live.pos starts at drop.x/y, and nothing in the size branch moves it, so the position check passes trivially) nor useGsapAwareEditing.test.tsx (both new cases use a { propertyGroup: "scale" } fixture) exercises this. Easiest guard: const ownsDragOffset = resizeGroup === "scale" and spread it into the three returns. Size-tween-only elements are uncommon in HeyGen compositions, so this is a follow-up rather than a blocker for the instant-hold win.

2. [NIT] packages/studio/src/hooks/gsapResizeIntercept.ts:261-262 — the pixel-drift check uses cssH on the assumption the shorthand takes newScaleX (verified at :266 and :282-284 — correct). Nothing wrong, but a one-line comment tying the choice of cssH back to that assumption would save the next reader the derivation ("shorthand = newScaleX applied to Y, so the collapse distorts the Y edge by |Δscale| × cssH").

CI (required per ruleset 14211637 — 8 checks): Semantic PR title / Build / regression / Test: runtime contract green; Typecheck / Test / Render on windows-latest / Tests on windows-latest still pending. Non-required Lint/Format/Preflight/Producer/SDK/Studio-load/File-size all green.

— Via

[james-russo-rames-d-jusso]

Reviewed at 1aa0fdba6.

Three faults, each traced to a real corruption on real compositions, each fix pointed and understood; the fault-1 diagnosis (one tween misclassified because it spans two groups, ID suffix and repair path both) and the fault-3 reframing (the question was never scale, it was pixels) are especially clean. The two sweep suites (108 structural shapes × 120 geometry cases across five routes / six rotations / four drops) turn "found one composition at a time" into a guardrail. This is the shape I want more HF fixes to take.

One blocker on fault 2's inverse. The three return { status: "persisted", ownsDragOffset: true } sites at gsapResizeIntercept.ts:432, :544, and :566 are all reachable from BOTH the scale route AND the size route. For a real (non-hold) size tween — auto-keyframe on or off, in-range or outside-range — resizeGroup === "size", scaleDraftEl stays null, and finalizeScaleResizeCommit is a no-op, but ownsDragOffset: true is returned anyway. The caller at useGsapAwareEditing.ts:275 then skips tryGsapDragIntercept(offset), and the anchor never lands in the script — same class of "an offset nobody wrote" the PR body flags as the fault-2 anti-pattern, just inverted (declared instead of inferred). Any corner-drag on a size-tween element persists the new width/height at the OLD anchor position after soft-reload. Inline at gsapResizeIntercept.ts:432 with the trace + suggested fix; the same shape covers the other two returns.

One concern — the geometry sweep silently accepts this. runCase initializes live.pos to the drop point (gsapResizeGeometrySweep.test.ts:203) and never has commitMutation update it (the vi.fn is a bare stub), so the "committed size or scale reproduces the rendered box" assertion is met even when the commit persists no position, and the "when ownsDragOffset is true, the box lands on the drop point" assertion is met trivially because live.pos was pre-positioned there. Same-file inline with a modelling suggestion.

What lands cleanly:

• Fault 1's diagnosis is exact and the repair path lands in the same call. gsapResizeIntercept.ts:120-122 detects any animation without a propertyGroup and hands the split-capable fetch to resolveGroupTween; the #card reproduction in gsapResizeMixedTween.test.ts:57-102 drives an actual 404-throwing server stand-in against the exact five-tween shape and verifies no stale id reaches it. This is the "found this by A/B'ing on a real composition" work that Miguel's memory writes reward. Every "damaged composition is auto-repaired" claim in the PR body maps to a specific line.
• sizeSet guard at :162-165 — when the resize is scale-route (resizeGroup === "scale"), the resolved anim (the scale hold) is deliberately NOT reused as the sizeSet target; findSizeSetAnimation runs and falls back to commitStaticGsapSize creating a fresh add if nothing matches (gsapDragCommit.ts — verified). This is the fault-1 fix's actual mechanism, and the "size hold of its own" comment matches the code.
• Pixel uniformity check at :261-263 — uniformDrift = |ΔScale| × cssH > 0.5 is the RIGHT question given the code commits scale: newScaleX at :266. The commit's height error IS cssH × |newScaleX - newScaleY|, so the threshold is measuring exactly what the collapse would distort. And > 0.5 matches the position-rounding tolerance elsewhere in the file — a distortion below rounding is invisible by construction. The bug-report numbers (630×408 dropped at 326×213 coming back 326×211) prove the old 0.01-of-scale threshold was scale-dimensional; the new one is pixel-dimensional.
• useScaleLonghands triggers the full-rewrite path at :472 — the "GSAP animates each name independently" note in the comment matches how a {scale, scaleX, scaleY} mix silently discards the shorthand. Normalizing every existing keyframe's scale to scaleX/scaleY at :501-508 (only when nonUniformScale) keeps a purely-uniform drag on the shorthand and only rewrites when the axes actually diverge.
• Playhead-under-cursor tween pinning at gsapResizeSweep.test.ts:239-274 — six playheads across two scaleAt tweens, three per side of the boundary at t=3/3.1. This is the "the resize did nothing because it edited the moment you weren't looking at" bug expressed as a test, and the sweep failed 3-of-6 against a stubbed selection that always takes the first — a real regression guard.
• ownsDragOffset as reported truth vs. inferred guess is the right shape. The caller comment at useGsapAwareEditing.ts:261-265 is worth its ink; keeping scaleRoute around only for the LIVE settle() step (:239) and reading the reported flag for the persist decision is the correct split. The regression above is the wiring of the flag, not the shape.
• Idempotence-by-construction is present in each of the three fault mechanisms — the split-repair, the size-hold-of-its-own path, and the pixel-uniformity check all leave a re-applied resize deterministic. The suites cover it as a rule rather than a test each.

The blocker is small — one predicate on each of three returns, or lift the flag into finalizeScaleResizeCommit's output — and the sweep concern is a shape suggestion, not a rewrite. With those closed, this is a clean piece of work.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🔴 Blocker — fault 2's inverse. The three ownsDragOffset: true return sites are reachable from the size route, so a resize on a real (non-hold) size tween drops the anchor offset.

The three sites — this line, :544, and :566 — are shared between the scale-route path and the size-route path. Trace for a size-tween element (selectedGsapAnimations = [size-tween with duration > 0], no scale group, auto-keyframe on, drag with offset={x, y}):

1. hasScaleGroup = false at :110 → resizeGroup = "size".
2. anim resolves to the size tween. isInstantHold(anim) = false, so we pass over the fault-1 branch at :149.
3. resizeGroup === "scale" at :218 is false → we take the size branch at :295: resizeProps = { width, height }, and scaleDraftEl stays null (only set inside the scale branch at :277).
4. Fall through to the add-keyframe path at :554 (or :472 outside-range, or this line if auto-keyframe is off).
5. finalizeScaleResizeCommit runs but scaleDraftEl === null short-circuits it at :314 — no position write.
6. This line returns ownsDragOffset: true.

The caller then reads outcome.ownsDragOffset === true at useGsapAwareEditing.ts:266, sees !ownsDragOffset = false at :275, and skips tryGsapDragIntercept(offset). The gesture's live setElementGsapPosition at :246 renders the offset in the iframe's GSAP runtime — but as the comment on elementGsap.ts:6-7 calls out, that's a live touch, not persistence. The softReload: true on the commit at :563 re-runs the timeline from the source script, which still holds the OLD position — the live gsap.set is overwritten. Element sits at the pre-gesture position after every drag. Same UX symptom as fault 2 (element "snaps back to authored position"), same class of "an offset nobody wrote".

The PR body says it correctly:

the three scale-route returns that run the centre-scaling correction set it, and every other route leaves it unset so the caller forwards the offset exactly as it does for a plain size resize.

That's the intent, but the code doesn't distinguish. All three returns set the flag regardless of route.

Reproduction case in the geometry sweep:

• Route "size tween" (gsapResizeGeometrySweep.test.ts:94) plus rotation 0 plus shrink drop ({w: 326, h: 213, x: 60, y: 40}).
• Real element (no commitMutation stub applying mutations): base position stays at whatever the pre-gesture value was, commit writes only {width, height}, no gsap.set(x, y) reaches the timeline, soft-reload lands the box at the OLD anchor.

The sweep doesn't catch it because runCase at gsapResizeGeometrySweep.test.ts:200-204 pre-positions live.pos to the drop point — inline on that separately.

Suggested fix. The simplest safe form is to key the flag off whether the scale draft actually ran, which is already tracked by scaleDraftEl. At each of the three returns:

return { status: "persisted", ownsDragOffset: scaleDraftEl !== null };

Equivalent: resizeGroup === "scale" — but reading it off scaleDraftEl keeps the flag colocated with the mutation it describes ("did finalize's correction actually run?") rather than with a routing input.

A slightly nicer version threads the flag out of finalizeScaleResizeCommit's return (currently void), so the return-site can't get out of sync with what finalize actually did. Not required — one predicate per return is fine.

Missing test. No existing test catches this because useGsapAwareEditing.test.tsx mocks tryGsapResizeIntercept end-to-end. The two new tests at :316 and :334 cover mocked ownsDragOffset: true and ownsDragOffset: undefined respectively, but nothing wires up an unmocked resize of a size-tween-with-duration to verify the flag comes back false. One test would fail today against the real intercept:

it("does not claim the anchor when the size route commits width/height", async () => {
  document.body.innerHTML = "";
  const el = document.createElement("div");
  el.id = "el";
  el.setAttribute("data-hf-studio-original-box-width", "300");
  el.setAttribute("data-hf-studio-original-box-height", "200");
  document.body.append(el);

  const sizeTween = tween("#el-size", { width: 300, height: 200 }, 2);
  const outcome = await tryGsapResizeIntercept(
    { id: "el", selector: "#el", element: el } as DomEditSelection,
    { width: 400, height: 260 },
    [sizeTween],
    null,
    vi.fn() as never,
    async () => [sizeTween],
  );

  expect(outcome.status).toBe("persisted");
  expect(outcome.status === "persisted" && outcome.ownsDragOffset).not.toBe(true);
});

Same shape as the third gsapResizeMixedTween.test.ts test at :149 — natural next-door test.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟡 Concern — the geometry sweep pre-positions live.pos to the drop point, so the sweep passes even when the commit persists no position.

runCase initializes live.pos at this line to {x: testCase.drop.x, y: testCase.drop.y}, which is where the DRAFT would have visually placed the element in a real gesture. commitMutation is a bare vi.fn() (:209), so it never dispatches to the fake gsap.set in mountCase (:174-178) that would otherwise update live.pos — the only way live.pos moves during the run.

That means:

• settled.x/y = renderRect({pos: live.pos, ...}).x/y at :222 is computed from the pre-set drop point.
• dropped.x/y = el.getBoundingClientRect().x/y at :207 is computed from the SAME live.pos (same fake getBoundingClientRect closure).
• off(settled.x, dropped.x) is trivially false.

So the ownership assertion at :234 ("when the resize reports ownsDragOffset, the box must also land on the drop point") is satisfied whenever live.pos was pre-set to the drop point — which is always — REGARDLESS of whether the commit actually wrote a position to the script. The blocker above is a live regression that the sweep declares clean.

To make the sweep model persistence:

1. Initialize live.pos to the BASE position (testCase.base.x/y), not the drop point — that's where the element sits before the gesture's draft renders.
2. Have commitMutation apply position writes: {x, y} in the properties (or in the keyframes[].properties), plus the specific add-with-gsap.set and commitStaticGsapPosition-shaped mutations that the finalize step actually emits. Then live.pos moves only when the commit actually persists a position.
3. Optionally cross-check by asserting the SIZE route with a size tween sets ownsDragOffset: false — the routes list already carries the intent ("size tween" should not own the drop point).

Alternatively (lighter-touch): keep live.pos initialized to the drop point but ADD an assertion — when owns === false, some position mutation must have reached commitMutation.mock.calls, or the harness accepts that the caller (not the intercept) owns the offset and doesn't assert on landing. As-is, the sweep can't distinguish "commit put the box on the drop point" from "harness pre-put the box on the drop point and the commit did nothing".

The commitMutation.mock.calls inspection already exists in committed() at :126-151 — extending it to catch position writes is small.

Non-blocking, but worth closing: this is the invariant the sweep advertises, and today it doesn't hold.

— Review by Rames D Jusso

[vanceingalls]

APPROVE @ 85c796b6eb0e059b57789f49cfc12d89f2f77614 — R2 (delta on top of R1 1aa0fdba6).

One new commit — 85c796b, "only claim the drop point on the route that settles it" — folds in the R1 NOTE cleanly. Two files touched: gsapResizeIntercept.ts (+15 / -12) and gsapResizeGeometrySweep.test.ts (+53 / -11). No other delta.

1. NOTE fold-in — polarity & coverage verified. finalizeScaleResizeCommit now returns Promise<boolean> — false when scaleDraftEl == null (size-only route) or when the finite / draft-point / selector safety-nets bail, true when the scale route actually settled the drop (including the "already on drop point" short-circuit, residual < 0.5, where nothing gets written but scale still owns the anchor — Miguel's comment reasons that correctly). All three previously-literal ownsDragOffset: true return sites (now lines 437 / 548 / 569, previously 432 / 544 / 566) propagate that boolean: return { status: "persisted", ownsDragOffset: await finalizeScaleResizeCommit() };. This is equivalent to the suggested resizeGroup === "scale" guard — scaleDraftEl is set only on the scale route — and arguably tidier because the runtime signal is the same authority the finalizer already keys off. Consumer at useGsapAwareEditing.ts:266-275 reads outcome.ownsDragOffset === true (strict equal), so false and undefined collapse to the same "caller forwards the offset" branch — clean.

2. Test hits the exact critical property from the NOTE. ROUTES gained per-route settles: boolean metadata — scale tween / scale longhands = true; scale instant hold / size tween / size instant hold = false. The judge asserts owns === testCase.settles and fails with ownsDragOffset <actual>, expected <settles>. The size tween case (real non-instant-hold size tween, 6 rotations × 3 drops = 18 shapes) is exactly the R1 NOTE case — pre-fold that route reported ownsDragOffset: true and the caller withheld an offset nobody wrote; post-fold it reports false and the drag intercept persists the anchor. Not just an "instant hold" cover.

3. No other regressions in the delta. computeDraggedGsapPosition untouched. commitMutation mock behaviour untouched. The test-scaffolding change is a data-shape widen (() => Anim[] → { animations, settles }) plus extracting the judge into a named function — the off tolerance, the persistedPositions / persistedScale / renderRect reconstruction, and the box-size assertion path are all preserved. Nothing shrunk. GsapEditOutcome.ownsDragOffset stays optional, so the type also permits false in the "persisted" variant.

4. CI: Preflight, Lint, Format, Fallow audit, Producer unit tests, Studio load smoke, Studio timeline viewport gate, SDK unit+contract+smoke, Semantic PR title, player-perf, Analyze (actions/python) all green. Build, Typecheck, CLI smoke, Producer integration tests, regression-shards, Test: runtime contract, Preview parity, Analyze (js-ts), Tests/Render on windows-latest still pending (freshly pushed). No failures.

R1 NOTE folded cleanly, tests catch it, no new issues. Ship.

— Via

[james-russo-rames-d-jusso]

Reviewed at 85c796b6e — delta from 1aa0fdba6: gsapResizeIntercept.ts (net +4 lines across three return sites and one helper signature) + gsapResizeGeometrySweep.test.ts (route-model widened, judge rewritten).

Both R1 findings closed, and each with a shape stronger than I asked for.

🔴 Blocker — closed at gsapResizeIntercept.ts:437 / :548 / :569. Rather than reading a call-site proxy (scaleDraftEl !== null) the way I suggested, finalizeScaleResizeCommit now returns Promise<boolean> and each of the three finalize-capable return sites reads it directly: return { status: "persisted", ownsDragOffset: await finalizeScaleResizeCommit() }. That's the right place for the truth to live — the helper knows whether it actually wrote a position or bailed out — and it catches two failure modes my one-liner would have missed:

• Non-finite residual (:357). With scaleDraftEl set (scale route) but residual.x or residual.y non-finite, the pre-PR code returned void and the caller still declared ownership; the new code returns false so ownership follows the write.
• "Already on the drop point" (:361). Scale route, residual absorbed under half a pixel — the helper skips the commit but still returns true, and the comment (Settled, with nothing to write. Still ours: forwarding the drag offset on top would move the box off the point it is already sitting on.) names exactly why. That's the case a naive predicate would have gotten wrong in the other direction.

Between those two, the flag now reflects "did finalize actually settle the drop point", not "does this call site look like the scale route" — the distinction I was groping at in R1 stated cleanly in code.

🟡 Concern — closed at gsapResizeGeometrySweep.test.ts:83, :98-115, :270-278. ROUTES now declares settles: boolean per route, buildCases propagates it onto every Case, and judge asserts owns === testCase.settles before the position check runs — so a run whose ownsDragOffset disagrees with the route's declared truth fails the sweep even when live.pos starts at the drop. The comment at :270-275 calls out exactly the fixture shape I flagged (the fixture's live pose starts at the drop, which is where the gesture leaves it, so a size route trivially "lands" there). Ownership is a first-class invariant in the sweep now instead of a byproduct.

Two smaller touches worth naming:

• "scale instant hold" is marked settles: false, which is the right encoding of fault-1 from this PR (the instant-hold scale tween commits via the size route). If a future fix accidentally routes it through scale, the sweep will catch the drift.
• The extracted judge function (:255-283) makes each shard-check independent, which will read cleanly if this fixture grows.

Runtime contract + Windows tests still finishing at the time of this review; regression shards were green on the previous SHA so I don't expect surprises. LGTM from my side.

— Review by Rames D Jusso