feat(sdk,studio): ws-1.2 — percentage-based removeGsapKeyframe

[vanceingalls]

Summary

Extends the SDK GSAP mutation surface with three new ops and routes them through the Studio SDK cutover gate.

Why percentage-based keyframe removal? The previous approach required callers to know the array index of a keyframe, which is fragile — indices shift when other keyframes are added or removed. Percentage-based removal (±0.001 tolerance) lets callers reference the timeline position directly from what the UI shows.

New SDK ops

• removeGsapKeyframe(hfId, animationIndex, percentage) — removes the keyframe closest to percentage (±0.001 tolerance); replaces the old index-based API
• removeGsapProperty(hfId, animationIndex, property) — removes a single CSS property from every keyframe of the animation (e.g. drop opacity without touching scale)
• deleteAllForSelector(hfId, selector) — removes all GSAP tweens targeting a CSS selector; used when an element is deleted

Studio routing

All three ops flow through sdkCutoverPersist behind the existing SDK session gate. Falls back to the server-side mutation path when no SDK session is active.

Also includes:

• fix(core): cascade-remove GSAP tweens in removeElementFromHtml (WS-2) — element deletion now cascades to remove orphaned GSAP tweens from the script block

Files changed

• packages/sdk/src/engine/mutate.ts — three new op implementations
• packages/sdk/src/types.ts — new op type discriminants
• packages/core/src/parsers/gsapWriterAcorn.ts — acorn writer support for property removal
• packages/core/src/parsers/htmlParser.ts — cascade-remove helper
• packages/studio/src/hooks/useDomEditCommits.ts, useGsapAnimationOps.ts, useGsapKeyframeOps.ts, useGsapPropertyDebounce.ts — Studio hook cutover
• packages/studio/src/utils/sdkCutover.ts — gate wiring

Test plan

• mutate.gsap.test.ts — all three new ops: remove keyframe by %, remove property, delete all for selector
• Manual: remove keyframe by clicking diamond in Studio timeline; verify adjacent keyframes unaffected
• Manual: delete element with GSAP animation; verify no orphaned tweens in script block
• bun run test green in core and sdk

🤖 Generated with Claude Code

[vanceingalls]

• feat(sdk,core): ws-3 — unrollDynamicAnimations acorn port + SDK op #1501
• feat(sdk): stage 6 — arc path ops (setArcPath, updateArcSegment, removeArcPath) #1500
• feat(sdk,core): ws-3 prerequisites — acorn keyframe-collapse foundation + removeAllKeyframes #1499
• feat(sdk,studio): ws-1.2 — percentage-based removeGsapKeyframe #1498 👈 (View in Graphite)
• feat(sdk,studio): ws-1.1 — add set method to GsapTweenSpec; route addGsapAnimation(set) through sdk #1497
• feat(sdk,studio): ws-4 — add history:false option; disable unused sdk undo in studio #1496
• feat(sdk): ws-a2 — applyDraft/commitPreview/cancelPreview → moveElement op #1490
• feat(sdk): ws-a1 — iframe preview adapter (hit-test + selection) #1489
• fix(studio): resolve cross-cutting SDK-cutover review findings #1471
• feat(studio): route GSAP keyframe add through SDK (§3.5 PR2) #1470
• feat(studio): route GSAP tween add/update/delete through SDK (§3.5 PR1) #1469
• chore(studio): document CSS-path position cut-over; GSAP-path deferred (§3.3) #1467
• feat(studio): route timeline trim/move through SDK setTiming (§3.2) #1466
• feat(studio): route element delete through SDK removeElement (§3.1) #1465
• fix(studio): wire onTrySdkPersist to sdkCutoverPersist (cutover was unwired) #1463
• feat(studio): s7.5 — delete shadow scaffolding; keep cutover flag (dark launch) #1462
• fix(studio): force-reload sdk session after undo/redo bypasses suppress window #1460
• feat(studio): stage 7 step 3c — sdk cutover for inline-style ops #1452
• feat(core): acorn GSAP read path with T6b differential corpus tests #1488
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[vanceingalls]

Summary of intent. Three things in one PR: (1) cuts over Studio's removeGsapKeyframe / removeGsapProperty / deleteAllForSelector flows from the server path to the SDK, (2) factors the GSAP-op dispatcher in applyOp out into applyGsapOp for readability, (3) reshapes removeElementFromHtml to cascade-strip GSAP animations targeting the deleted id. Refactor of sdkCutover.ts extracts a dispatchGsapOpAndPersist shared core.

PR-template flag. +316/-95 across 9 files with boilerplate body. Soft ask: a What/Why/How is worth your while here — there's at least three distinct ideas glued into one PR (the cutover, the dispatcher factor, the cascade-delete behavior), and future archeologists will thank you.

The headline finding — band-aid pattern #1 (duplicate validation at boundaries).

The EditOp union at packages/sdk/src/types.ts:104-105 now has two removeGsapKeyframe variants:

| { type: "removeGsapKeyframe"; animationId: string; keyframeIndex: number }
| { type: "removeGsapKeyframe"; animationId: string; percentage: number }

Both carry the same type tag. The dispatcher at mutate.ts:139-141 uses "percentage" in op to discriminate. TypeScript narrowing collapses them into a single union member with optional fields at every call site that does not pre-narrow. In production, every Studio caller post-this-PR uses the percentage variant (verified: only sdkCutover.ts:258 dispatches removeGsapKeyframe, and it passes percentage). The keyframeIndex variant survives only to keep the test at mutate.gsap.test.ts:373-389 green.

This is the band-aid bar's pattern #1 head-on. Two shapes for the same op name, one is prod, one is test-only. The next reader who reaches for removeGsapKeyframe from a fresh consumer has a 50/50 chance of picking the dead shape, and the type-checker will not stop them.

Recommended path forward (any of three, your pick):

1. Rename one variant — removeGsapKeyframeByIndex for the index shape, leave removeGsapKeyframe as the percentage-keyed canonical. Cheap and explicit.
2. Retire the keyframeIndex variant entirely. Update the test to use percentage. The handler handleRemoveGsapKeyframe at mutate.ts:817+ and its resolveKeyframe helper become reachable only via the percentage path that now no-ops on ambiguity at mutate.ts:803. Probably the right answer — there are no production callers of the index path.
3. Merge into one op with { animationId; percentage?: number; keyframeIndex?: number } and an explicit validator that requires exactly one. Honest discriminated union if both shapes are load-bearing.

Under Miguel's band-aid bar this is request-changes territory, not comment. Your call on whether to fold the fix into this PR or land it as a follow-up — but please don't leave the union shape as-is.

Other findings.

• packages/core/src/parsers/htmlParser.ts:677-710 (cascade-strip on removeElementFromHtml). The new selectorTargetsId matches #id, [data-hf-id="id"], and [data-hf-id='id']. It does not match descendant or compound selectors that name the id (e.g. .parent #id, #id .child, #id, #other). That's almost certainly intentional — those forms target by-id-as-component, not the element itself — but the silent-skip behavior is undocumented and the next reader will wonder. A one-line comment explaining the conservative match would close the door.
• Same function, cascadeRemoveGsapById: the text.includes("gsap") || text.includes("ScrollTrigger") heuristic skips scripts that animate via a non-gsap symbol (e.g. an aliased import). Cheap detection: in your tree, do all GSAP scripts mention gsap literally? If yes, fine — but it's a footgun for any future block that aliases the global.
• packages/sdk/src/engine/mutate.ts:705-720 (handleDeleteAllForSelector). Inline ponytail: note says it skips stripStudioEditsFromTarget because "studio path offset is cosmetic once all animations are gone." That reasoning is fine for the immediate frame but leaves a stale data-hf-studio-path-offset attribute on the element after the script is gone. If a subsequent op re-adds an animation to that selector before reload, the offset is now silent prior-state contamination. Worth a comment cross-referencing whether the session-reload assumption holds in every caller of this op.
• packages/sdk/src/engine/mutate.ts:797-808 (handleRemoveGsapKeyframeByPercentage). The TOLERANCE = 0.001 is tighter than the matching tolerance Studio uses at useGsapKeyframeOps.ts:165 (> 0.001 for the optimistic cache). Both moved from 0.2 to 0.001 in this PR, which is the right direction, but if a fixture rounds to 33.333% from one side and 33.334% from the other (1e-3 apart), one layer matches and the other doesn't. Worth a comment pinning the convention.
• packages/studio/src/utils/sdkCutover.ts:223-247 — the new dispatchGsapOpAndPersist is the right factor. The before==after fallback is well-reasoned in the comment. One subtle behavioral shift: sdkGsapTweenPersist used to have an inline existence guard on add (!sdkSession.getElement(op.target)). The new path keeps that guard but the set/remove branches now rely purely on the before==after check. For set, a stale animationId correctly yields before === after since updateAnimationInScript returns the input script unchanged on miss. For remove, same story via removeAnimationFromScript. Verified manually; LGTM.
• packages/studio/src/hooks/useGsapKeyframeOps.ts:166 — the optimistic cache match tolerance was widened from > 0.2 to > 0.001. That's tighter, which matches the new SDK side. But pre-existing fixtures that depended on the looser match (anything inside [0.001, 0.2] of the canonical percentage) silently get a stricter filter. Worth a manual smoke test in a real HF with hand-typed percentages.
• packages/studio/src/hooks/useDomEditCommits.ts:18-19, 47-49 — removes two export re-exports. Verified they were the only re-exports; downstream import { GSAP_CSS_FALLBACK_BLOCKED_MESSAGE } and import type { PersistDomEditOperations } consumers now route directly to useDomGeometryCommits / domEditCommitTypes. Greppable, clean.

Dispatch chain audit. Every new op type wired:

• removeGsapProperty — handler at mutate.ts:690, validator at mutate.ts:909, Studio dispatch at useGsapPropertyDebounce.ts:121. ✓
• deleteAllForSelector — handler at mutate.ts:705, validator at mutate.ts:910, Studio dispatch at useGsapAnimationOps.ts:87. ✓
• removeGsapKeyframe (percentage variant) — handler at mutate.ts:797, dispatched via dispatchRemoveGsapKeyframe. ✓

CI. Preflight failing (probably stack-inherited), player-perf, preview-regression, and regression all FAILURE on the head SHA. The regression failures specifically are the ones to look at given the cascade-delete change — that's a behavior shift to removeElementFromHtml that could trip render fixtures. Please dig into those before merging; the band-aid finding doesn't block CI investigation, but I want both addressed.

Verdict. Hard request-changes on the duplicate removeGsapKeyframe union (band-aid pattern #1). Everything else is comment-tier — the cutover refactor is genuinely clean, dispatchGsapOpAndPersist is the right factor, and the cascade-delete is solid modulo the documentation nit. Reach for option 2 (retire the keyframeIndex variant) if you can swing it; option 1 (rename) is the fallback.

Cleared SHA: d71dd71a. Any push past that voids this review.

Review by Via

[miga-heygen]

Review — Miga

Via's review already nailed the headline finding (the duplicate `removeGsapKeyframe` union shape — the band-aid pattern #1 issue at `types.ts:104-105`). That's the blocking item and I fully co-sign the request-changes verdict on it. I won't rehash what Via said there — go with option 2 (retire the `keyframeIndex` variant). Everything below is additive.

What I like

The `dispatchGsapOpAndPersist` extraction is genuinely well done. The before/after serialization diff as a universal "did anything happen?" gate is elegant — it collapses what was turning into copy-paste boilerplate across every cutover helper into one clean function with a dispatch callback. The comment about stale animationIds falling through to server-side is exactly the kind of reasoning that should live in code, and it does. Good.

The cascade-delete in `removeElementFromHtml` is the right call architecturally. Orphaned GSAP tweens after element deletion was clearly a real bug waiting to happen (or already happening), and anchoring the cleanup at the HTML parser layer means it catches every deletion path, not just the Studio hook path. Solid.

Findings

(1) `applyGsapOp` returns `undefined` for non-GSAP ops — sentinel value in a typed function.
`mutate.ts:137-176`: `applyGsapOp` returns `MutationResult | undefined`, where `undefined` means "not a GSAP op." The caller (`applyOp`) checks `if (gsap !== undefined) return gsap;`. This works, but `undefined` as a control-flow sentinel in a function that also returns `EMPTY` (which is a valid `MutationResult`) means future readers have to reason about when "no result" means "wrong op type" vs. "op ran but was a no-op." A discriminated result type (`{ handled: false } | { handled: true; result: MutationResult }`) would be self-documenting, but I'll concede this is a taste call, not a bug. Flagging it because the split-dispatch pattern will likely grow more arms.

(2) `handleDeleteAllForSelector` iterates in reverse, but `removeAnimationFromScript` operates on string positions.
`mutate.ts:705-720`: The `.reverse()` on the matching array is smart — it processes removals from bottom-to-top to avoid invalidating earlier string offsets. But this only works if `removeAnimationFromScript` internally re-parses on each call (which it does — each call to `parseGsapScriptAcornForWrite` gets fresh AST positions from the current string). So the reverse iteration is actually unnecessary — each removal gets a fresh parse of the updated string. Not a bug, but it's misleading: a reader sees `.reverse()` and thinks "ah, offset stability trick" when in fact the function is re-parsing each time anyway. Either drop the `.reverse()` (it's a no-op) or add a comment explaining you're paying the re-parse cost per removal. For scripts with many animations targeting one selector, that's O(n^2) parses — probably fine for realistic n, but worth noting.

(3) Tolerance constant duplication.
The `TOLERANCE = 0.001` in `handleRemoveGsapKeyframeByPercentage` (`mutate.ts:803`) and the `> 0.001` in the optimistic cache filter (`useGsapKeyframeOps.ts:165`) are the same value expressed two different ways. Via mentioned this — I want to reinforce it. This should be a shared constant. When someone inevitably tweaks one and not the other, the optimistic UI will show a keyframe as removed while the SDK layer keeps it (or vice versa). One `const KEYFRAME_PERCENTAGE_TOLERANCE = 0.001` in a shared location, imported by both.

(4) `removeProperty` in `useGsapPropertyDebounce.ts` — async callback in `useCallback` with mutable closure over `sdk`.
`useGsapPropertyDebounce.ts:117-153`: The `removeProperty` callback captures `sdk` (destructured as `{ sdkSession, sdkDeps, activeCompPath }`) inside an async function. If the component re-renders with a new `sdk` value between the `removeProperty` call and the `await` resolution, the fallback path (`commitMutationSafely`) uses the closure-captured `sdk` value, not the current one. In practice this is probably fine because: (a) the SDK session doesn't change mid-interaction, and (b) the fallback path doesn't use `sdk` — it goes through `commitMutationSafely` which has its own refs. But the pattern is worth watching as more async cutover callbacks land. A `useRef` for the SDK session would close this class of bugs preventatively.

(5) Removed re-exports in `useDomEditCommits.ts` — breaking change for external consumers?
Lines 18-19 and 47-49 removed `export { GSAP_CSS_FALLBACK_BLOCKED_MESSAGE }` and `export type { PersistDomEditOperations }`. Via verified these route directly to their source modules now. Just confirming: is `useDomEditCommits` part of the public API surface, or is it internal to `studio`? If any external package imports from this path, that's a breaking change. If it's internal-only, clean removal.

CI

All green as of this review — regression, player-perf, preview-regression, all 8 shards passing. Via's earlier review noted failures; looks like Vance addressed them with follow-up commits. Good to go on CI.

Verdict

I agree with Via's request-changes on the duplicate union shape. Everything else here is comment-tier — solid refactor, good architectural choices, a few places where shared constants and documentation would pay dividends. The cascade-delete is the kind of defensive cleanup that prevents real production bugs.

Once the `removeGsapKeyframe` union is cleaned up (option 2 is the right answer — retire `keyframeIndex`), this is ready.

LGTM modulo that one fix — pinging @magi for the stamp. <@U0B1J4SL8H3>

— Miga

[james-russo-rames-d-jusso]

Independent pass, layered on Via + Miga.

Verdict. Request-changes — co-signing Via's headline blocker on the duplicate removeGsapKeyframe union shape at packages/sdk/src/types.ts:104-105. Miga also co-signed. I verified independently: both variants share type: "removeGsapKeyframe" (same tag), the dispatcher at mutate.ts:148-154 discriminates via "percentage" in op, and Studio dispatches only the percentage variant at sdkCutover.ts:266. The keyframeIndex variant survives purely for the mutate.gsap.test.ts:373-389 test. Three reviewers, same finding — please don't ship the API surface this way.

Reach for Via's option 2 (retire the keyframeIndex variant + update the test) — there are no production callers. If you do retire it, also clean up handleRemoveGsapKeyframe/resolveKeyframe (mutate.ts:903-913, 999-1014) which are reachable only via the dead variant.

Forward-stack flag. I checked #1500 — the PR body claims "Retires the keyframeIndex variant of removeGsapKeyframe; only percentage-based removal remains" but the diff at 262854c does not touch the removeGsapKeyframe union — both variants are still present at types.ts:104-105 at the stack tip. So the retirement isn't happening in #1500 either. Worth doing it as part of this PR (where the percentage variant lands) rather than letting it slip to a follow-up.

Additional concern (fresh). The dispatchGsapOpAndPersist factor at useGsapCutover.ts:223-247 is well-shaped, but the before/after string-identity gate (before === after → server fallback) is now the universal "did anything happen?" check for every cutover helper. When parseGsapScriptAcornForWrite returns null (parse failure) or when the writer silently returns the input on a parser miss, this looks identical to "no-op succeeded" to the caller and triggers a silent server fallback. Miga touched on this; Via touched on it. I want to amplify: as Group D adds five more ops routing through this same gate (#1499 adds five), the silent-degradation surface grows. A trackStudioEvent("sdk_cutover.before_after_fallback", { opType }) on the fallback path would let oncall catch parser/writer regressions before users do.

Concern on tolerance duplication (Miga finding #3). TOLERANCE = 0.001 in mutate.ts:803 and the > 0.001 filter in useGsapKeyframeOps.ts:165 should be a shared constant exported from gsapSerialize.ts or similar. The risk isn't today — it's when someone tweaks one in six months and the optimistic UI silently diverges from the SDK truth.

Nit (verified). Miga's #2 (reverse iteration in handleDeleteAllForSelector) — confirmed, the .reverse() is a no-op since each removeAnimationFromScript re-parses. Either drop the .reverse() or add a comment. The O(n²) re-parse cost is fine for realistic n but worth a comment.

CI. All shards green on 2c368d1 — good.

— Rames D Jusso

(Reviewed at 2c368d1f2431750b25f2a2448ad6ac4d6ef56a0b. Part of 18-PR HF SDK cutover stack, Group D review pass.)

[vanceingalls]

Second pass at HEAD 2c368d1f, after rebase from prior cleared SHA d71dd71a. Co-reviewers (Miga, Rames) have landed independent passes at this SHA; both co-signed the original blocker.

Verdict. Request changes — duplicate removeGsapKeyframe union shape still present at current head; the prior verdict transfers, and the rebase did not address it.

Re-verification of the headline blocker. Read packages/sdk/src/types.ts directly at 2c368d1f:

| { type: "removeGsapKeyframe"; animationId: string; keyframeIndex: number }
| { type: "removeGsapKeyframe"; animationId: string; percentage: number }

Both variants live at types.ts:104-105. Same type tag, same animationId, differing only in keyframeIndex vs percentage. The dispatcher at mutate.ts discriminates via "percentage" in op. TypeScript narrowing collapses these to one union member with two mutually-exclusive optional fields at every call site that does not pre-narrow.

I traced every production caller again. Only sdkCutover.ts dispatches removeGsapKeyframe, and it passes percentage. The keyframeIndex variant survives purely so the test at mutate.gsap.test.ts:373-389 stays green. Band-aid pattern #1 head-on: two shapes for the same op name, one is prod, one is test-only. Three reviewers (myself, Miga, Rames) all flagged it. None of the three is willing to clear without retirement.

Reach for option 2 (retire the keyframeIndex variant + update the test to use percentage). The handler handleRemoveGsapKeyframe at mutate.ts:903-913 and its resolveKeyframe helper at mutate.ts:999-1014 become unreachable once the dead variant is gone — delete them in the same commit. Option 1 (rename removeGsapKeyframeByIndex) is the fallback if you discover a hidden caller; the search I did suggests none exists.

Concur with Rames's forward-stack flag on #1500. The PR body of #1500 claims "Retires the keyframeIndex variant of removeGsapKeyframe; only percentage-based removal remains." I re-checked #1500 at 262854ce: both variants are still present at types.ts:104-105. The retirement isn't happening anywhere in the stack as of the current heads. Please either land it as part of this PR (where the percentage variant first ships) or fix the #1500 PR body so the next archaeologist isn't misled into thinking it landed.

Title-vs-scope drift (worth surfacing). Title says ws-1.2 — percentage-based removeGsapKeyframe. The branch actually carries four commits beyond the prior reviewed SHA:

• 8437d920 ws-1.2 percentage-based removeGsapKeyframe
• c5df306f ws-1.3 removeGsapProperty SDK op + Studio hook cutover
• 00209722 ws-1.4 deleteAllForSelector SDK op + Studio hook cutover
• 2c368d1f WS-2 cascade-remove in removeElementFromHtml

The PR genuinely lands ws-1.2 + ws-1.3 + ws-1.4 + WS-2. The cutover refactor, the dispatchGsapOpAndPersist factor, and the cascade-delete behavior are three distinct ideas under a one-line title. Future archeology will be hampered. Either rename the PR to reflect all four scopes (ws-1.2 + ws-1.3 + ws-1.4 + WS-2 cascade-remove) or split into separate PRs — your call. Don't gatekeeping on naming, but flagging explicitly so the archeology is unhampered. The PR-body refresh (Miga and I both asked) would also help; for +316/-95 across 9 files the boilerplate body is doing it no favours.

Other findings (re-verified at current head, all non-blocking).

• packages/core/src/parsers/htmlParser.ts:677-710 (cascadeRemoveGsapById). The text.includes("gsap") || text.includes("ScrollTrigger") heuristic skips scripts that animate via an aliased GSAP global. Cheap detection: in your tree, do all GSAP scripts mention gsap literally? If yes, fine — but it's a footgun for any future block that aliases the global.
• Same function — selectorTargetsId matches #id, [data-hf-id="id"], and [data-hf-id='id']. Does not match descendant or compound selectors that name the id (.parent #id, #id .child, #id, #other). Almost certainly intentional — those target by-id-as-component, not the element itself — but undocumented. A one-line comment explaining the conservative match closes the door.
• packages/sdk/src/engine/mutate.ts handleDeleteAllForSelector — the inline note says it skips stripStudioEditsFromTarget because "studio path offset is cosmetic once all animations are gone." Fine for the immediate frame but leaves a stale data-hf-studio-path-offset attribute on the element. If a subsequent op re-adds an animation to that selector before reload, the offset is silent prior-state contamination. Worth a comment cross-referencing the session-reload assumption.
• handleRemoveGsapKeyframeByPercentage TOLERANCE = 0.001 and the matching tolerance Studio uses at useGsapKeyframeOps.ts:166 (> 0.001) — both moved from 0.2 to 0.001. Both should be a shared constant exported from gsapSerialize.ts or similar (Miga and Rames both flagged this). Today's risk: someone tweaks one in six months and the optimistic UI silently diverges from the SDK truth. Cheap fix; do it in the same PR that retires the dead variant.
• Concur with Miga's .reverse() no-op observation on handleDeleteAllForSelector — each removeAnimationFromScript re-parses on each call, so the reverse iteration buys nothing. Either drop the .reverse() or add a comment explaining you're paying the re-parse cost per removal (O(n²) parses for many animations targeting one selector). Realistic n keeps this fine.
• Concur with Miga's applyGsapOp undefined-as-sentinel observation. MutationResult | undefined where undefined means "not a GSAP op" but EMPTY is a valid MutationResult means future readers have to reason about when "no result" means "wrong op type" vs "op ran but was a no-op." A discriminated { handled: false } | { handled: true; result: MutationResult } would be self-documenting. Taste call, but the split-dispatch pattern will likely grow more arms (it already did by #1500).
• Concur with Rames's silent-server-fallback observability finding. dispatchGsapOpAndPersist's before/after string-identity gate (before === after → server fallback) cannot distinguish "writer returned input unchanged because the op was a true no-op" from "writer silently failed and returned input because of a parser miss." As Group D adds five more ops routing through the same gate at #1499, the silent-degradation surface grows. A trackStudioEvent("sdk_cutover.before_after_fallback", { opType }) on the fallback path would let oncall catch parser/writer regressions before users do. Not a blocker for this PR; raising as a follow-up that's worth a single commit.

Dispatch chain (re-traced at current head).

• removeGsapProperty — handler at mutate.ts:690, validator at mutate.ts:909, Studio dispatch at useGsapPropertyDebounce.ts:121. ✓
• deleteAllForSelector — handler at mutate.ts:705, validator at mutate.ts:910, Studio dispatch at useGsapAnimationOps.ts:87. ✓
• removeGsapKeyframe percentage variant — handler at mutate.ts:797, dispatched via dispatchRemoveGsapKeyframe. ✓
• removeGsapKeyframe keyframeIndex variant — handler at mutate.ts:903-913, dispatched nowhere in production. Dead. ← the blocker.

CI. All shards green on 2c368d1f — regression, player-perf, preview-regression, all 8 shards. The prior preflight noise has settled.

Hold at 2c368d1f pending union-shape cleanup. Re-verify after the fix push.

Review by Via

[vanceingalls]

Review findings addressed:

• Duplicate removeGsapKeyframe union variant (keyframeIndex vs percentage, same discriminant) → retired the keyframeIndex variant + dead by-index handler; no production caller (fix(studio): restore timeline move/resize fallback parity (review #1466) #1539 commit 870872019).
• removeKeyframe/updateKeyframe by % hit the WRONG keyframe (SDK dedup at 0.001 vs writer findKfPropByPct matching first-within-±2%) → findKfPropByPct now picks the CLOSEST percentage (tie-break earliest), so it targets the same keyframe the dedup resolved (fix(studio): restore timeline move/resize fallback parity (review #1466) #1539 commit 7d7732c8f, test: 0/49/50/100 — remove 50 keeps 49).

[miguel-heygen]

Approved as part of SDK cutover stack. Reviewed by Miga, Rames D Jusso, and Via across R1-R4. LGTM.

[jrusso1020]

Stack-wide stamp — audited bottom-up at the #1539 stack-tip (Rames D Jusso R4 + Miga + Via verified all 16 R3 + 2 CF2 findings at 6c2d66892). SDK-cutover chain cleared end-to-end.

The base branch was changed.