feat(core): parse-parity suite for acorn parser (T6d)

[vanceingalls]

Summary

Adds a full parse-parity test suite that runs every scenario from the existing gsapParser.test.ts against the new acorn parser, and fixes a percentage calculation bug exposed by those tests. This is the third and final parser PR (T6b → T6c → T6d).

Why

T6b's differential corpus tests only covered scenarios from an earlier, narrower test file. The full gsapParser.test.ts suite has 60 scenarios including edge cases (variable targets, three-level keyframe easing, chained calls, stagger round-trips) that the differential file didn't exercise. Running that suite against the acorn parser revealed one real bug.

What changed

packages/core/src/parsers/gsapParserAcorn.ts

• Bug fix: object-array keyframe percentage calculation had cumulative duration and percentage computation in the wrong order vs the recast parser. Fix: accumulate duration first, then compute percentage, matching recast behavior.

packages/core/src/parsers/gsapParserAcorn.full.test.ts (new)

• 60-scenario parse-parity suite ported from gsapParser.test.ts:
  • Basic .to() / .from() / .set() / .fromTo()
  • resolvedStart position resolution (relative, absolute, implicit, chained)
  • Timeline defaults inheritance
  • Property group classification
  • Stagger / yoyo / repeat round-trips
  • Unresolvable value round-trips
  • Variable-target resolution (querySelector, getElementById, querySelectorAll)
  • Array targets, chained tween calls, gsap.utils.toArray
  • Lexical scoping, forEach/map callback targets
  • Native GSAP keyframes (percentage-object, object-array, simple-array, three-level easing)
• 4 tests are it.skip'd — write-path scenarios already covered by gsapWriter.acorn.test.ts

Test plan

• bun run test packages/core → 60 pass, 4 skip in full suite; T6b + T6c tests unaffected
• Stacked on: T6c (write path)
• Stack above: Phase 3b (SDK mutation ops)

[vanceingalls]

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

• refactor(core): swap studio-api read path from recast to acorn parser (T6e) #1392
• feat(sdk,core): phase 3b — 8 gsap/label ops + setClassStyle #1379
• feat(core): parse-parity suite for acorn parser (T6d) #1370 👈 (View in Graphite)
• feat(core): acorn GSAP write path — magic-string offset-splice (T6c) #1369
• feat(core): acorn GSAP read path with T6b differential corpus tests #1368
• main

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

[james-russo-rames-d-jusso]

LGTM with one legit gap worth fixing before T6d closes, and a methodology nuance worth knowing. The one-line bug fix in gsapParserAcorn.ts:632 is real and the object-array keyframes test at :833-850 is a clean regression guard for it.

What the suite covers:

• 60 active tests + 4 it.skip'd (write-path, correctly deferred to gsapWriter.acorn.test.ts) across 13 describe blocks.
• Corpus: .to/.from/.set/.fromTo, position resolution (numeric / += / < / implicit / negative-clamp), defaults inheritance, property-group classification, stagger/yoyo/repeat round-trip, unresolvable values, variable-target resolution (querySelector / getElementById / querySelectorAll / gsap.utils.toArray), array targets, chained calls, lexical scoping across IIFE scopes, forEach/map callback targets, all 4 native keyframe formats (percentage / object-array / simple-array / three-level easing).
• Methodology: assertions are hand-coded against acorn output, not a differential-by-construction against the recast parser. T6b (gsapParser.acorn.test.ts) already does true golden-snapshot differential parity for 4 corpus scripts — this PR is "coverage extension" rather than "parity by construction." Worth understanding when reading the PR title.

Concerns (non-blocking-ish):

• gsapParserAcorn.full.test.ts — motionPath is entirely absent from the parity suite. The acorn parser implements motionPath (see parseMotionPathNode and motionPathResult plumbing in gsapParserAcorn.ts lines ~750-940), and the recast gsapParser.test.ts has 6 motionPath tests (describe("motionPath parsing") at L1761 — waypoint array + curviness, cubic + control points, autoRotate, keyframe merge, <2-waypoint skip, no-motionPath identity). None of these run against acorn anywhere — not in T6b differential, not here. If acorn's motionPath path regressed silently, you wouldn't catch it until a real composition fails to render. Recommend porting the 6 motionPath cases into this suite before T6d lands, or adding them to T6b as goldens. Either is fine.
• gsapParserAcorn.full.test.ts:7-13 — re-asserting expectations against the acorn parser only (parseGsapScript = parseGsapScriptAcorn) means a recast-side assertion bug in the original suite would get faithfully reproduced into the acorn expectations. T6b already covers true differential for 4 scripts; consider noting in the file header that this suite trusts the recast-baseline assertions and the real parity guarantee is T6b's golden snapshots. Doc-only; not a blocker.

Bug fix:

• gsapParserAcorn.ts:629-641 — flipping cumulative += entry.duration to happen before the percentage calc is verbatim recast parity (cross-checked against gsapParser.ts:670-679). Old behavior would have produced [0, 22, 65] for object-array durations [0.5, 1, 0.8]; new behavior gives [22, 65, 100] matching the test at gsapParserAcorn.full.test.ts:847-849. Good catch.

Nits:

• gsapParserAcorn.full.test.ts:75 — REAL_WORLD_SCRIPT is defined but I only see it referenced once (the "real-world IIFE composition" test at L671). Fine to keep — naming + comment make it clearly an intentional fixture — just flagging that it reads like it might be meant for broader reuse.
• Header could say "coverage extension" rather than "parse-parity" since T6b is the actual parity harness. Or leave it — internal vocabulary is fine.

Things NOT in the diff worth checking:

• scrollTrigger — not parsed by the acorn (or recast) read path at all. Not a regression from this PR, but if any consumer in #1379 expects scrollTrigger configs to round-trip, they'll silently drop. Out of scope here, just flagging.
• Negative-case coverage already lives in T6b (returns empty result on syntax error) — no gap.

Cross-stack coherence:

• #1379 adds addLabel/removeLabel writer ops, but addLabel is not parsed by gsapParserAcorn.ts (zero matches in the file) and the writer at the T6c commit doesn't handle it either. That's expected — label support is being added in #1379. But it means #1379 needs to carry its own label parse + write coverage; this T6d suite won't catch label-path regressions. Worth a heads-up to whoever reviews #1379.
• Per the Graphite-hoist pattern: I checked just this PR's incremental diff. If a fix gets hoisted from here to the tip during review iteration, ping me.

Review by Rames D Jusso

[vanceingalls]

Thanks @james-russo-rames-d-jusso. The motionPath gap is addressed:

6 motionPath tests added to gsapParserAcorn.full.test.ts in the current Phase 3b commit (065446e45): waypoint array + curviness, cubic + control points, autoRotate, keyframe merge, <2-waypoint skip, no-motionPath identity. All 6 are direct ports of the recast cases at gsapParser.test.ts:1761–1893. All pass.

Methodology note acknowledged — the file header could be clearer that this suite trusts the recast-baseline assertions and T6b's goldens are the actual parity guarantee. Will add a note.

[james-russo-rames-d-jusso]

R2 verification — HF#1370 (T6d parse-parity)

PR: #1370
Author: Vance Ingalls
HEAD at R2: ef2dc8ef2b5156659c7627f03836bc8c0db1bbb7 (single commit, authored 2026-06-12T07:57:43Z, committed 2026-06-12T18:05:42Z — likely a Graphite restack)
Round-1 commit reviewed: 51633ad69fb691b0d4f766681b3608bb031a75ae
Round-1 review submitted: 2026-06-12T17:19:43Z
Round-2 ping from Vance: 2026-06-12T20:45:15Z

Verdict

⚠️ R1-C1 is OPEN — the motionPath tests were not added to this PR. Vance's R2 comment is in good faith but technically wrong: the 6 motionPath tests live in commit 065446e45 on stack-tip PR #1379, NOT in #1370. The HEAD of #1370 (ef2dc8e) has zero motionPath references in either gsapParserAcorn.full.test.ts or gsapParserAcorn.ts.

This is a textbook Graphite stack fix-hoist pattern — Vance landed the fix at the stack tip rather than the originating PR. Operational impact depends on whether #1370 lands separately from #1379 or as part of one merge-stack train.

Findings — verbatim verification

R1-C1: motionPath gap → ❌ STILL OPEN (with caveat)

Claim from author: "6 motionPath tests added to gsapParserAcorn.full.test.ts in the current Phase 3b commit (065446e45)."

Verified state:

• gsapParserAcorn.full.test.ts at HEAD of #1370 (ef2dc8e): 0 motionPath references, 914 lines, ends at the native-keyframes block.
• gsapParserAcorn.full.test.ts at HEAD of #1379 (065446e): 1051 lines, contains describe("motionPath parsing", ...) at L918 with all 6 ports (waypoint+curviness L919, cubic+control L950, autoRotate L985, keyframe merge L1001, <2-waypoint skip L1028, no-motionPath identity L1040). Confirmed direct ports of recast tests.
• gh api repos/.../commits?path=gsapParserAcorn.full.test.ts&sha=ef2dc8e returns ONLY the original T6d commit — no R1-response commit on this branch.

Caveat — alternative resolution rubric (per James's feedback memory): Operational goal is "acorn motionPath has parity coverage somewhere in the test corpus before T6d closes." The tests do exist — just on the stack-tip PR. If the stack merges together via Graphite (which the bot-comment graphite warning indicates is the intended mode: "merge this PR as a stack on Graphite"), the motionPath coverage lands at the same moment T6d does, and the goal is met.

However: the R1 finding explicitly said "before T6d lands" and offered TWO acceptable homes ("port into this suite OR add to T6b as goldens"). Neither happened on #1370's branch. If for any reason #1370 lands without #1379 (rollback, merge-stack split, cherry-pick), there is a window where acorn ships motionPath parsing without parity coverage.

Recommendation to Vance: either (a) cherry-pick 065446e45's motionPath-test additions down into #1370 so the fix lives at the originating PR, or (b) explicitly confirm the stack will land atomically and document the cross-PR coupling in #1370's body.

R1-N1: header trust-relationship note → ❌ OPEN

Claim from author: "Will add a note."

Verified state: File header at L1-13 is unchanged from R1:

// fallow-ignore-file duplication
/**
 * T6d: parse-parity suite — runs the full gsapParser.test.ts parse scenarios
 * against parseGsapScriptAcorn.  Write-path tests are it.skip'd; those live
 * in gsapWriter.acorn.test.ts.
 */

No mention of "trusts recast-baseline assertions" or "T6b carries the real parity guarantee." This is a nit, but Vance committed to it. Low priority — flagging for transparency, not a blocker.

R1-N2: REAL_WORLD_SCRIPT defined-but-single-use → N/A

Defined at L58, used once at L672. Cosmetic. R1 marked as "fine to keep." No action expected.

R1-N3: "coverage extension" vs "parse-parity" rename → N/A

Title unchanged ("feat(core): parse-parity suite for acorn parser (T6d)"). R1 marked as "or leave it — internal vocabulary is fine." No action expected.

R1-Bugfix: cumulative-vs-percentage ordering → ✅ STILL IN PLACE

Verified at gsapParserAcorn.ts HEAD lines ~626-642:

const totalDuration = raw.reduce((sum, r) => sum + (r.duration ?? 0), 0);
const keyframes: GsapPercentageKeyframe[] = [];
if (totalDuration > 0) {
  let cumulative = 0;
  for (const entry of raw) {
    cumulative += entry.duration ?? 0;                            // ← accumulate FIRST
    const percentage = Math.round((cumulative / totalDuration) * 100);  // ← then percentage
    ...

This is verbatim recast parity. The regression-guard test at gsapParserAcorn.full.test.ts:847-849 (expectKeyframe(kfs[0], 22, ...); expectKeyframe(kfs[1], 65, ...); expectKeyframe(kfs[2], 100, ...);) is intact at HEAD. Load-bearing fix preserved.

New findings (R2)

🆕 Cross-stack confusion in author's response. Vance's R2 comment says the motionPath tests are "in the current Phase 3b commit" — but Phase 3b is #1379, not #1370. The Slack-facing reader (James) will assume the work landed on #1370. Worth gently flagging the lane confusion so Vance either hoists the tests down to #1370 or rewrites the comment to acknowledge the cross-PR location explicitly. This is the kind of cross-stack-fix coupling that gets lost if the PRs land separately.

🆕 CI is green on the substantive checks. All regression-shards, Perf: drift/fps/load/parity/scrub, Preflight, Preview parity, player-perf are pass. The lone red is Perf: ${{ matrix.shard }} on run 27438552110, which is a canceled stale run superseded by a higher-priority newer request (annotation: "Canceling since a higher priority waiting request for player-perf-refs/pull/1370/merge exists"). Not a real failure.

🆕 Graphite stack-tip risk acknowledged in R1. R1's "Cross-stack coherence" footer already called out the fix-hoist risk: "if a fix gets hoisted from here to the tip during review iteration, ping me." This is exactly what happened — the motionPath addition was hoisted to the tip. So the warning fired correctly, but the author then closed the loop with a Slack comment claiming the fix landed here, which masks the hoist.

Resolved vs open count

Status	Count	Items
✅ Resolved	1	R1-Bugfix (still in place)
❌ Open (but covered at stack tip)	1	R1-C1 (motionPath at #1379, not #1370)
❌ Open (committed but not done)	1	R1-N1 (header note)
N/A (R1 marked optional)	2	R1-N2, R1-N3
🆕 New	3	Cross-stack confusion, CI cancel noise, stack-hoist confirmed

CI status

All substantive checks pass. Only red is a canceled stale run. Mergeability blocked by Graphite downstack policy (expected — stack-merge mode).

Suggested R2 response to Vance

Friendly note that the motionPath tests are at #1379's tip, not #1370 — confirm whether stack-merge is the intended ship mode (in which case it's fine to leave as-is, with a one-line note in #1370's body about the cross-PR coupling) OR whether he wants to cherry-pick them down. Mention this is the Graphite fix-hoist pattern flagged in R1's footer. Skip blocking on it — operational goal will be met at stack-merge time — but flag the lane confusion so future reviewers don't get tripped up.

File path: /tmp/r2_1370.md

[vanceingalls]

R2 response:

• R1-C1 (motionPath tests): Confirmed fix-hoist — the 6 motionPath tests are in the Phase 3b commit (04172071b, PR feat(sdk,core): phase 3b — 8 gsap/label ops + setClassStyle #1379), not in feat(core): parse-parity suite for acorn parser (T6d) #1370's commit. This is intentional: the acorn motionPath parser itself lands in Phase 3b, so the parity tests live there too. The stack is designed to merge atomically, so coverage lands at the same time as the parser. Added a note to the file header (commit a3ea6ada2) documenting this cross-PR coupling explicitly.

• R1-N1 (header trust-relationship note): Added in commit a3ea6ada2. The header now documents the recast-baseline trust model and notes where motionPath tests live.

All CI checks green. Happy to cherry-pick the motionPath tests down to this commit if you'd prefer them here rather than at stack tip — just say the word.

[jrusso1020]

LGTM.

[miguel-heygen]

T6d parse-parity suite — review

Pre-check summary

• CI: all regression shards ✅, all Perf:* ✅. Perf: ${{ matrix.shard }} failure is the known infra artifact (expired log, job is skipped on main). Graphite pending is expected for a stacked PR. No prior approve/reject reviews.
• Existing comments from james-russo-rames-d-jusso: 2 COMMENTED (no approval or blocking state).

---

packages/core/src/parsers/gsapParserAcorn.ts — the code change

The bugfix is correct and elegantly minimal.

-      const percentage = Math.round((cumulative / totalDuration) * 100);
+      cumulative += entry.duration ?? 0;
+      const percentage = Math.round((cumulative / totalDuration) * 100);
       ...
-      cumulative += entry.duration ?? 0;

Moving the accumulation before the percentage computation means each keyframe's percentage now represents the end of its segment, not the start. For [{dur:0.5}, {dur:1}, {dur:0.8}] (total 2.3s):

• Before fix → 0%, 22%, 65% (first frame always pinned to 0%)
• After fix → 22%, 65%, 100% (CSS convention: each stop = where the segment ends)

The test at full.test.ts:813 (expectKeyframe(kfs[0], 22, …)) locks this in correctly. ✓

---

packages/core/src/parsers/gsapParserAcorn.full.test.ts — coverage analysis

914 lines across 12 describe blocks. Coverage is thorough on the happy path. Findings below.

---

important — missing >, <+offset, <-offset position specifiers

full.test.ts:249–267 (resolves += and < positions) and full.test.ts:228–247 (chained -=0.3 pattern) cover the most common relative-position tokens. However, the following real-world GSAP variants are absent:

• ">" — start at end of previous tween
• "<+0.5" — start at beginning of previous + 0.5
• "<-0.2" — start at beginning of previous - 0.2
• ">-0.3" — end of previous - 0.3 (overlap onto the tail)

These appear in production compositions regularly. If resolvePosition does not handle them, parsing degrades silently to __raw: or 0. Suggest adding at least one test per token family in a follow-up (not a blocker since they live outside this task's scope).

---

important — standalone gsap.set() exclusion is implicit only

full.test.ts:667–681 (REAL_WORLD_SCRIPT fixture) asserts exactly 3 animations, implicitly verifying that the two gsap.set(kicker, …) and gsap.set(glyph, …) standalone calls are excluded. But there is no explicit test with the expectation written out, e.g.:

it("excludes standalone gsap.set() from animations", () => {
  const script = `gsap.set("#el", { opacity: 0 });
                  const tl = gsap.timeline({ paused: true });
                  tl.to("#el", { opacity: 1, duration: 0.5 }, 0);`;
  const result = parseGsapScript(script);
  expect(result.animations).toHaveLength(1);          // not 2
  expect(result.animations[0].method).toBe("to");
});

If the implicit coverage is intentional, a comment calling it out would help future readers.

---

important — no test for gsap.timeline() without paused: true

Every fixture in the suite uses gsap.timeline({ paused: true }). The defaults-only variant ({ defaults: { ease: … } }) is covered, but a bare gsap.timeline() call (no options at all, common in player-controlled timelines) has no coverage. If the parser regex or AST walk for timeline detection keys on the options object structure, this could be a latent failure mode.

---

nit — implicitPosition only asserted for true; never for absence

full.test.ts:288–310 asserts implicitPosition: true on sequential tweens. There is no assertion that an explicitly-positioned tween (e.g. position 0 or 2) does not set implicitPosition. A single complementary expect(anim.implicitPosition).toBeUndefined() on an explicit tween would close that contract.

// full.test.ts:278 (resolves numeric positions)
expect(result.animations[0].implicitPosition).toBeUndefined(); // ← missing

---

nit — object-array keyframe test could document the "end-of-segment" intent

full.test.ts:807–818 tests kfs[0] at 22%, but a brief inline comment would clarify why the first keyframe is not at 0% (i.e., because the percentage marks the END of the segment, not the start). This is important context for anyone reading this test after the off-by-one in the source has long been forgotten.

---

nit — stagger serialised as "__raw:0.1" — consider round-trip numeric assertion

full.test.ts:486 asserts extras!.stagger === "__raw:0.1". The __raw: encoding is deliberate, but there is no test asserting that serializeGsapAnimations renders a numeric stagger back as stagger: 0.1 (not stagger: "__raw:0.1"). The existing parseAndSerialize round-trip at line 497 covers the complex object stagger path; a numeric-stagger round-trip would complete the picture.

---

Overall assessment

The test suite is impressively thorough: 12 describe blocks, all major target-resolution paths (querySelector, querySelectorAll, getElementById, inline calls, forEach, arrays, toArray, lexical IIFE scopes), all three keyframe formats, timeline defaults, ID generation, and collision disambiguation. The write-path it.skip stubs are correctly deferred with clear comments. The single code change is a well-scoped, correct fix.

The important findings above are genuinely worth filing as follow-up issues but none are blockers for landing this task (T6d scope is parse-parity; write-path and edge-position specifiers belong to subsequent tasks or separate coverage tickets).

Verdict: APPROVE
Reasoning: The bugfix is correct, the test suite delivers comprehensive parse-parity coverage for the acorn parser, CI is clean, and the gaps identified are out-of-scope edge cases rather than correctness failures in the delivered surface.

— magi