feat(core): add GSAP keyframe + motion-path source mutations

[miguel-heygen]

Stack: GSAP keyframe + motion-path editing — core read/write layer (#1553 → #1561).

What

Add core parser + writer support for keyframe and motion-path source mutations on GSAP tweens: add / remove / update keyframes, convert a flat tween to keyframes, edit motion-path waypoints, and classify tween properties into groups (position / scale / rotation / visual).

Why

The Studio keyframe/motion-path editor needs deterministic, AST-level read and write of GSAP timelines so that edits round-trip through the composition source without reformatting unrelated code.

How

• Parser surfaces keyframes, property groups, and resolved tween timing (gsapParser.ts, gsapConstants.ts).
• Writer-vs-parser parity tests lock emitted source against what the parser reads back.

Note on the write path (recast vs acorn)

The keyframe/motion-path write ops apply through recast, not the acorn writer — recast reprints only the touched nodes, preserving the surrounding source formatting (the property of these mutations we care about). The parity tests assert the recast output round-trips through the acorn-based parser.

There is one known acorn-vs-recast divergence: an array-form keyframe update with partial props — recast replaces the keyframe object, acorn would merge it. This is captured as an it.skip with the ready assertion in gsapWriter.parity.test.ts, pending the writer fix; it does not affect the object-form path used by the Studio editor today.

Test plan

• Unit tests added (parser + writer parity, incl. addKeyframeToScript array-form + updateKeyframeInScript)
• bun run test (core) green

[miguel-heygen]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• feat(core): route motion-path mutations through studio-api + fix clip stamping #1555 : 2 dependent PRs (#1556 , #1607 )
• feat(core): add GSAP keyframe + motion-path source mutations #1554 👈 (View in Graphite)
• chore(producer): shim __filename/__dirname in the CJS banner #1553
• main

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

[jrusso1020]

Approved at c8aee055 per Rames D Jusso "stamp-ready with 1 nit + 1 question." Deferring his specific items (posted in his review) to Miguel for resolution inline — non-blocking from my side.

PR body is the unfilled template; please fill in even a one-paragraph description before merge so the change history names what landed (Rames D Jusso + Via both flagged this as a stack-wide process concern across all 9 PRs).

[terencecho]

Test-coverage + backwards-compat axis (complementary to @rames-jusso's structural pass)

Reviewed with a narrower lens: do the new mutation APIs ship with tests, and is anything signature-broken on the existing parser/writer surface?

Verified

• Test coverage strong on the new surface:
  • gsapParser.test.ts adds units for updateMotionPathPointInScript (4 cases), addMotionPathPointInScript (2 cases), removeMotionPathPointInScript (2 cases), addMotionPathToScript (1 case), syncPositionHoldsBeforeKeyframes (6 cases), addKeyframeToScript backfill on/off (2 cases), _auto exclusion (1 case).
  • gsapWriter.parity.test.ts adds removeKeyframeFromScript array-form parity (3 cases: implicit-% removal, collapse-on-fewer-than-2, no-op on mismatched %).
• Writer parity scope is correct. Motion-path mutations are intentionally recast-only (live in gsapParser.ts, no acorn counterpart) because #1555 routes them through studio-api (server-side). The browser/SDK acorn writer doesn't need them — the studio calls them via HTTP. So the parity test absence for motion-path is by design, not a gap.
• No export signature breaks: every export touched in gsapParser.ts and gsapWriterAcorn.ts is an ADDITION (verified by git diff filter on ^[-+]\s*export function — no - lines).

Note

• The commit message describes the surface well, but the PR body is the unfilled template — see consolidated note on #1561. For the most critical PR in the stack (+651/-4 core mutations), the empty body makes the change harder to justify to a future archaeologist.

— Review by tai (pr-review)

[james-russo-rames-d-jusso]

Reviewed at 4ee9b9fb (stack-aware, layered on tai's R1 test-coverage pass).

✅ Stamp-ready on the structural axis. Foundation PR for the keyframe + motion-path editor — pure addition surface, no signature breaks, parity-tested where it matters. Anchoring on tai's verified test-coverage pass (gsapParser.test.ts + gsapWriter.parity.test.ts cover the new ops at 4/2/2/1/6/2/1 + 3 cases respectively); I'm layering structural + downstream-consumer findings.

Verified

• Pure addition surface. gsapParser.ts +373/-4, gsapWriterAcorn.ts +130/-4 — every new export (updateMotionPathPointInScript, addMotionPathPointInScript, removeMotionPathPointInScript, addMotionPathToScript, syncPositionHoldsBeforeKeyframes) is additive. The 4-line deletions on each side are loosening type checks (the ObjectExpression-only bail) to handle array-form keyframes — the existing object-form path still runs as before.
• Array-form keyframes are the load-bearing fix. keyframes: [{x,y}, …] previously silently no-op'd on update/remove because both writers bailed on the ObjectExpression check. New removeArrayKeyframe + updateArrayKeyframeByPct (acorn) and arrLoc branches in the recast path handle it by even-distribution percentage matching, with convertArrayKeyframesToObject normalizing in-place when a new percentage must be inserted. Parity test at gsapWriter.parity.test.ts:162-211 covers the recast/acorn equivalence — exactly the right shape.
• syncPositionHoldsBeforeKeyframes is the right place for the hold invariant. "Element holds first keyframe's value before tween start" is universal-NLE behavior; implementing it as a tagged tl.set(…, 0) kept in sync via a single re-derive pass is the right primitive. Idempotency test at gsapParser.test.ts covers the canonical regression (re-call after move). isStudioHoldSet re-export through gsapParserExports.ts is the clean way to let studio filter the synthetic sets out of user-visible animation lists.
• Position-drift fallback in locateAnimationWithFallback (gsapParser.ts:1856-1900). Animation ids encode timeline position (#puck-to-1200-position); a gesture that changes position changes the id, so a stale client cache no-ops. Falling back to selector+method+group + nearest-position is the right shape. ANIM_ID_RE is tight (^(.*)-(fromTo|from|to|set)-(\d+)-([a-z]+)$) — selector permissive enough to include # / . / quoted strings, method enum-closed, position numeric, group lowercase-alpha.

Concerns

• gsapConstants.ts:80 — _auto AND data excluded from classification. The exclusion list now has three entries: transformOrigin, _auto, data. The _auto exclusion is well-defended (regression test added: { x: 100, y: 50, _auto: 1 } must classify as position). The data exclusion is justified in the comment as "Studio hold-set tag" but has no regression test. If a future hold-set or annotation ever writes a property literally named data, that classification path silently drops it. Suggest: add a one-line test mirroring the _auto regression for data, or rename the marker to _data for symmetry with _auto.
• splitAnimationsInScript from-tween fix (gsapParser.ts:1700-1716). The fix correctly handles a completed .from() by NOT inheriting its recorded properties (they're the hidden start state). Two questions:
  • .fromTo() is not in the branch — does a completed .fromTo() revert to the from state or end at the to state? GSAP's behavior is the latter (ends at to like .to()), so this is probably fine, but worth a comment line stating it.
  • This only fires for animEnd <= opts.splitTime (animation ends BEFORE split). What about a .from() that ends AFTER the split? Today the inherited-props code path doesn't run for those, so this is silent — but worth pinning with a test that a mid-flight .from() split doesn't inherit hidden-start values.
• addMotionPathToScript 2-anchor default authors (0,0) → (x,y) with position (gsapParser.ts:842-851). If the caller invokes with position > 0, the fresh tween's 0% snaps to (0,0) at the moment the tween starts — i.e. the element jumps from its CSS-baseline to (0,0). If the studio invokes this BEFORE the hold-sync runs (which it does — hold-sync is gated by HOLD_SYNC_MUTATION_TYPES in routes/files.ts:594-611 of #1555, and add-motion-path is NOT in that set), the user sees a one-frame jump. This is the same class of bug #1555 R2 calls out for HOLD_SYNC_MUTATION_TYPES. Suggest landing the hold-sync set expansion in #1555 BEFORE this PR merges, or addMotionPathToScript itself should call syncPositionHoldsBeforeKeyframes on its output.

Nits

• (nit) gsapParser.ts:1856-1862 — ANIM_ID_RE matches set as a method but splitIntoPropertyGroups / position-tween logic typically excludes set. If a set ever lands in the located list, the fallback could pick a set neighbor for a to tween. Probably fine in practice (ids are method-disambiguated) but a comment noting the assumption would help future archaeologists.
• (nit) PR body is well-written here — thank you. The unfilled-template note tai + jrusso1020 flagged earlier no longer applies at the current commit.

Questions

• The new addMotionPathToScript returns { script, id: "" } on parse failure or empty-located+null-timelineVar. id: "" is a sentinel; consumers in #1555 (routes/files.ts:1086-1098) only use result.script, so they don't observe the empty id. But if anything downstream (studio store?) ever calls addMotionPathToScript directly and chains on id, the silent "" will cause a phantom "no animation" branch. Worth either returning { script, id: null } or asserting at the call site.

What I didn't verify

• The acorn-vs-recast parity for the new splitAnimationsInScript from-tween branch — only the array-keyframes ops have explicit parity tests. The from-tween fix is recast-only on the surface I read; if acorn has its own split path, both need the same conditional.
• That _auto: 1 is the only sentinel — if studio ever adds _hold: 1 or similar, the exclusion list needs a regex (_ prefix) rather than a literal list.

— Rames D Jusso

[vanceingalls]

Reviewed at 4ee9b9fb. @jrusso1020's stamp at c8aee055 stands — independently verified that the 28 commits between c8aee055..4ee9b9fb are unrelated merges into the base chain (slideshow #1580-1594, SDK WS-B/C/D #1569-1574, releases v0.6.113/.114), not #1554-attributable. @james-russo-rames-d-jusso's R1 layered concerns are fair; concurring + adding two net-new findings.

Concur with @james-russo-rames-d-jusso:

• gsapConstants.ts:80 — data exclusion has no regression test mirroring the _auto regression. Either add the one-line test or rename to _data for symmetry with the _ sentinel convention. Cheap.
• splitAnimationsInScript from-tween fix at gsapParser.ts:1700-1716: .fromTo() not in the branch + mid-flight .from() split untested.
• addMotionPathToScript 2-anchor default authors (0,0) → (x,y) (gsapParser.ts:842-851); if called at position > 0 without HOLD_SYNC_MUTATION_TYPES covering add-motion-path, the user sees a one-frame jump-to-(0,0). This converges with my own concern on #1555's missing hold-sync types (below) — same bug class, two surface areas. Strongest cross-PR finding in the stack.

Net-new (parity-test discipline gaps — HF #1500 pattern):

gsapWriter.parity.test.ts at HEAD adds the array-form removeKeyframeFromScript parity block (lines 168-214, real expect(modelOf(acornOut)).toEqual(modelOf(recastOut)) assertions). Good. But two sibling array-form ops added in this PR have only acorn output correctness coverage in gsapWriter.acorn.test.ts, NOT a parity: describe block:

• updateKeyframeInScript (array-form, the #shuttle case) — covered at gsapWriter.acorn.test.ts:250-262. No parity-test.
• addKeyframeToScript (array-form normalization) — covered at gsapWriter.acorn.test.ts:264-275. No parity-test.

This is the HF #1500 silent-opt-out pattern — a new op variant added with acorn output correctness only, leaving the recast↔acorn equivalence un-pinned. Cheap fix: mirror the removeKeyframeFromScript block for the other two array-form ops.

Net-new (PR-body framing nit):

Body says "Extend the Acorn-based GSAP writer with keyframe/motion-path ops." But motion-path ops (updateMotionPathPointInScript, addMotionPathPointInScript, removeMotionPathPointInScript, addMotionPathToScript) + syncPositionHoldsBeforeKeyframes ship in gsapParser.ts (recast) only — acorn writer doesn't gain them in this PR. Same for the .from() empty-revert fix. This widens the recast/acorn cutover surface silently. Worth a one-line PR-body clarification ("motion-path ops + sync-position-holds + .from() split-fix are recast-only this PR; acorn cutover follow-up tracked separately") so future archaeologists don't have to re-verify which writer got what.

Stamp stands. Above are NIT-level; not blocking the merge but worth one fix-up pass.

Review by Via

[james-russo-rames-d-jusso]

Layering on @via's review — net-new from my R1:

Parity-test asymmetry on new keyframe ops. updateKeyframeInScript + addKeyframeToScript (array form) ship with acorn output correctness describe blocks but no parity: blocks. Sibling removeKeyframeFromScript did get a proper parity block. Mirror that for the other two — recast↔acorn equivalence is un-pinned for the two ops that the new motion-path / keyframe UX will exercise most heavily.

Scope clarification ask. Motion-path ops + syncPositionHoldsBeforeKeyframes + .from() split-fix appear to be recast-only this PR (acorn cutover follow-up). One-line PR-body note prevents downstream confusion about scope of the acorn cutover work.

Both are non-blocking but worth landing while the ops are fresh.

— Rames D Jusso