fix(parsers): exclude per-keyframe duration from object-keyframe properties

[miguel-heygen]

What

Object-form GSAP keyframes ({ "0%": {...}, "50%": {...} }) that carry a per-step duration no longer expose duration as an animatable keyframe property. Both the live acorn read parser (parsePercentageKeyframes) and the retired recast parser now skip duration in the %-keyed extraction — matching the array-form parser (parseObjectArrayKeyframes), which already excludes it.

Why

duration is GSAP array-keyframe segment timing, not an animatable property. When it leaked into a keyframe's properties, the Studio surfaced a bogus "duration" keyframe lane and treated it as editable, corrupting the keyframe round-trip on edit. This is a real, self-contained parser bug and a small hardening.

How

A one-line guard in each %-keyed extraction loop: else if (k === "duration") continue;. No other behavior changes.

Test plan

• New unit tests (acorn + recast) parse an object keyframe with a stray per-step duration and assert: duration is NOT in any keyframe's properties; real props (x) and per-keyframe ease ARE preserved.
• vitest run both parser suites — 270 passed / 4 skipped.
• typecheck, oxlint, oxfmt, fallow, commitlint — green.

Scope note

This is a parser hardening only. It is not the fix for the "Script error." on manual keyframe edits — that turned out to be a separate custom-ease resolution bug in the Studio runtime (custom eases weren't registered in GSAP's internal ease map, so keyframe segments resolved to a non-function ease and threw on render). That fix lives with the keyframe-timeline feature, not on main. This PR stands on its own regardless.

[james-russo-rames-d-jusso]

Reviewed at 492b49deba816a4c2f7771d9a2c952d43bcc927d.

What this does

Both %-keyed keyframe extractors in the parser layer — parsePercentageKeyframes in gsapParserAcorn.ts:854 (live) and its retired sibling in gsapParser.ts:686 — now skip a per-step duration when walking the property record. In GSAP, duration is array-keyframe segment timing (between-keyframe pacing), not an animatable value; a %-keyed step's timing is owned by the % key itself, so a stray duration here is neither timing nor a property. Without this guard, Studio surfaced it as a bogus lane and round-tripped it back as an animatable value, corrupting the tween on next edit.

Verification

• Parity with array-form parser — parseObjectArrayKeyframes already excludes duration in the array-form (per PR body). This closes the divergence between array and %-keyed extractors so a shape-converted keyframe (array → object) doesn't silently gain a phantom lane.
• Per-keyframe ease preserved — the guard is placed inside the else if chain AFTER the ease branch, so {x: 100, duration: 6, ease: "power2.out"} still emits kfEase = "power2.out" and props {x: 100}. Both acorn and retired test cases assert this on the middle step (kfs[1]?.ease === "power2.out"). Load-bearing.
• Real animatable prop preserved — both tests assert kf.properties contains x while lacking duration for all three keyframes. Would fail cleanly if the guard were placed before the ease branch or if the extractor short-circuited the whole iteration.
• Both parser suites covered — new tests at gsapParser.test.ts:659-676 and gsapParserAcorn.full.test.ts:87-102 use identical scripts, keeping the recast/acorn parity explicit.
• Comment quality — the multi-line comment names WHY duration in a %-keyframe is meaningless AND names the failure mode ("bogus lane", "corrupts on next edit"). Future readers get context without git-blame.

Adversarial pass

• What if a GSAP plugin legitimately uses duration as an animatable value? — GSAP core reserves duration universally; any plugin author using it as a property would collide with tween-level duration already. Not a realistic case.
• Top-level tl.to(target, {duration: 1, keyframes: {...}}) — that duration is the tween's own timing, parsed separately by the enclosing tween-parameter reader. This fix scopes only to the values INSIDE each % key. No collision.
• Round-trip destructiveness — if any on-disk composition ships with a %-keyframe containing duration, the parser silently drops it on load. Per the PR framing, that value was meaningless anyway; dropping it is the intended cleanup path. Not a data-loss concern for legitimate content.
• Non-%-keyed object keyframes — the parsePercentageKeyframes code path only fires when keys are %-suffixed literals (validated upstream). Other object-keyframe shapes flow through different extractors. Scope is correctly narrow.
• Merge order noted by Magi — this touches gsapParserAcorn.ts alongside #2413's larger identity-consolidation. Since both add non-overlapping edits (the duration guard here vs. expandedScopeNode capture in #2413), a rebase should be trivial mechanical merge. Behaviorally independent as claimed.

Verdict framing

Small, focused hardening with load-bearing parity tests on both parsers. Nothing to hold up. LGTM from my side.

— Review by Rames D Jusso

[vanceingalls]

Runtime-interop lens (paired with Rames on test/observability, whose review I read in full). Additive — Rames covered the mechanics, ease-branch ordering, comment quality, plugin/top-level/round-trip adversarials, and general merge-order framing. Two verifications to add.

Additive verifications

• #2413 overlap check, hard-grepped. grep -c parsePercentageKeyframes /tmp/pr2413.diff = 0. #2413's hunks on gsapParserAcorn.ts land at lines 39 / 51 / 240 / 264 / 430 / 442 / 1222 / 1324 / 1363 / 1380 / 1818 / 1871 — all in collectScopeBindings / tweenCallToAnimation / top-level exports. parsePercentageKeyframes (this PR's target, line 854) is untouched. Mechanical merge, not just "should be trivial."
• Retired-parser patch is load-bearing, not cosmetic. gsapParser.test.ts:2-3 imports parseGsapScript directly from ./gsapParser — the retired Recast module itself, NOT via the exports redirect at gsapParserExports.ts:47 that reroutes the public parseGsapScript to Acorn. So Recast's own regression + golden + stress + inline test suites (gsapParser.golden.test.ts, gsapParser.stress.test.ts, gsapParser.inline.test.ts) all still exercise this code path. Patching the retired sibling in lockstep keeps that internal parity real, and prevents gsapWriterParity.corpus.test.ts from drifting when the writer round-trips a corpus that carries the fixed shape.

Nit — follow-up, not gating

• Consider a mirror test that walks parse → mutate → serialize → re-parse to pin that a stray duration doesn't sneak back in via the writer. Current tests assert on parser output only; a writer round-trip closes the "Studio surfaced a bogus lane" loop end-to-end. (gsapWriterAcorn.ts is the target; would live alongside the existing gsapWriter.reviewFixes.test.ts.)

Verdict: APPROVE
Reasoning: Small, symmetric parity fix. Both parsers get identical guards plus affirmative regression tests. Adversarial pass cleared (both Rames's + mine, non-overlapping). Retired Recast patch is load-bearing for internal parity, not dead code. No merge collision with #2413.

— Via