Skip to content

test(core): add T6a GSAP parser golden baselines (Recast/Babel snapshot)#1263

Merged
vanceingalls merged 5 commits into
mainfrom
06-07-test_core_add_t6a_gsap_parser_golden_baselines_recast_babel_snapshot_
Jun 8, 2026
Merged

test(core): add T6a GSAP parser golden baselines (Recast/Babel snapshot)#1263
vanceingalls merged 5 commits into
mainfrom
06-07-test_core_add_t6a_gsap_parser_golden_baselines_recast_babel_snapshot_

Conversation

@vanceingalls
Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls commented Jun 7, 2026

Summary

  • Adds packages/core/src/parsers/gsapParser.golden.test.ts with 6 toMatchFileSnapshot tests across 3 representative GSAP scripts (minimal/moderate/complex)
  • Golden files are checked in at src/parsers/__goldens__/ and capture both parseGsapScript JSON output and serializeGsapAnimations string output under the current Recast/Babel parser
  • Time-sensitive: must be merged before the Recast → Meriyah swap (R? branch) so the swap has a regression gate — any parser behavior change will surface as a golden diff rather than a silent behavioral change
  • Excludes __goldens__/ from fallow (ignorePatterns) and oxfmt (.prettierignore) since vitest owns formatting of snapshot files

Corpus:

  • minimal — 2 tl.to calls, numeric selectors (macos-notification block)
  • moderate — 6 tl.to calls, multiple CSS-id selectors (yt-lower-third block)
  • complex — stagger, chained .from() calls, const + timeline defaults (vpn-youtube-spot block)

Test plan

  • bun run --cwd packages/core test -- gsapParser.golden.test.ts — 6 pass, 0 failures
  • bun run --cwd packages/core test -- -u gsapParser.golden.test.ts after any parser change shows diffs in __goldens__/ files

miguel-heygen
miguel-heygen previously approved these changes Jun 8, 2026
Copy link
Copy Markdown
Collaborator

@miguel-heygen miguel-heygen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Golden baseline approach is correct for a parser that transforms JS source — toMatchFileSnapshot gives diff-friendly failure output on regression, and gating on Recast before the Meriyah swap is the right sequencing.

A few things worth confirming:

Sorted emission order in complex.serialized.js — animations come out sorted by position ascending (0.05, 0.08, 0.16, 0.2), not declaration order. This is captured in the golden but not narrated in a test description. If the sort is an intentional design decision, a comment in gsapParser.golden.test.ts noting it would help a future Meriyah swapper know this is expected, not an artifact.

__raw: prefix convention — stagger values appear as "__raw:0.08" and "__raw:0.055" in the parsed JSON, indicating they're kept as raw JS expressions. The golden captures this correctly. Same note: a brief comment in the test file or the golden would clarify this is by design.

.fallowrc.jsonc and .prettierignore — both updated. Consistent with the repo's pattern for excluding generated files. No issues.

P2 suggestion (not a blocker): the parseAndSerialize helper returns { parsed: string, serialized: string }. If either the parser or serializer throws, the test just fails with an unguarded error rather than a useful "parsed step failed" message. Adding a try/catch with labeled rethrowing would make failures in large corpus inputs much easier to diagnose. Low priority for characterization tests, but worth doing before T6b/c.

Approved — goldens look correct, infrastructure changes are clean.

Copy link
Copy Markdown

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round-1 review of T6a (GSAP parser golden baselines for the Recast → Meriyah swap). +321/-0, 9 files: 6 golden snapshot files (3 scripts × 2 functions), 1 test file, 2 ignore-file updates. Walked the diff with the lens of (1) coverage vs. parser surface, (2) snapshot vs. assertion trade-off, (3) corpus selection, (4) fallow + oxfmt ignoring, (5) the swap-PR-diff-readability claim.

What's strong

Snapshot-baseline shape is the right call here. The PR body argues this explicitly and I agree — the goal is "diff the parser swap against the current behavior" rather than "lock in correctness." Explicit assertions across 3 scripts × parseGsapScript + serializeGsapAnimations would be ~6 huge toEqual blocks and would obscure WHICH fields changed when Meriyah lands. toMatchFileSnapshot produces a reviewable file-level diff that's the actual review artifact for the swap PR.

__goldens__/ excluded from both fallow and oxfmt. Both .fallowrc.jsonc:38-39 and .prettierignore:5-6 updated with the right rationale ("vitest owns formatting"). ✓ Without these, the goldens would get reformatted on commit and the snapshot diffs would become unreadable.

Inline corpus scripts (not registry-coupled). Lines 25-66 define MINIMAL_SCRIPT / MODERATE_SCRIPT / COMPLEX_SCRIPT as const strings rather than reading from the registry. Right call — if a registry script gets updated for unrelated reasons (animation tweak, copy change), the goldens shouldn't churn.

The describe → beforeAll → it × 2 pattern. Each describe block parses once, then asserts parsed and serialized snapshots independently. So if the parser breaks but the serializer is fine (or vice versa), the failure narrows the blast radius automatically. Tighter than running both assertions in one it.

Concerns

[important — corpus undercount vs. parser support surface] gsapParser.ts:41:

const GSAP_METHODS = new Set<string>(["set", "to", "from", "fromTo"]);

The parser supports four methods. The corpus exercises only tl.to and tl.fromgsap.set and tl.fromTo are unrepresented. So the Meriyah swap could subtly break tl.fromTo parsing or gsap.set capture, and the goldens won't catch it.

Additionally, looking at the parser's scope-binding + literal-resolution machinery (gsapParser.ts:57-115), there are several shapes the swap could affect that the corpus doesn't exercise:

  • tl.fromTo(target, fromVars, toVars, position) — distinct three-object signature. Different node shape, different traversal in the AST visitor.
  • gsap.set(target, vars) at module scope. Looking at MINIMAL_SCRIPT line 30 (gsap.set(notification, ...)) — the golden output for minimal shows only the 2 tl.to animations, no gsap.set. So the parser filters out gsap.set calls. The "set" branch (GSAP_METHODS.has("set")) is reachable but apparently no-op. Worth either documenting in the test why set isn't visible in the golden, or adding a corpus script that does surface a gsap.set-derived animation if there's a code path that should.
  • Math.ceil(7 / 2.4) - 1 in COMPLEX_SCRIPT:51 — but breatheRepeats is never used. The scope-binding resolution at resolveNode:90-105 handles BinaryExpression with arithmetic operators; if Meriyah changes how arithmetic literals fold, this would silently drift. Unfortunately breatheRepeats doesn't reach a tl.* call so the corpus doesn't exercise the binding lookup at all. Worth either using breatheRepeats in a tl.repeat() or similar, or dropping it from the script as dead.
  • Negative numbers / UnaryExpression with - operator. The parser handles -5 at resolveNode:88-91. No corpus script has negative property values or positions.
  • Template literals (UnaryExpression arm at line 112). No corpus script uses backticks.
  • Labels (tl.addLabel("start") + tl.to(…, "start")) or callback adds (tl.add(() => {…}, 1.0)). If the parser handles these — and given the AST visitor's breadth I'd guess it tries — they're a high-risk path under a parser swap. (If it explicitly doesn't handle them, that's worth a negative test too.)

Not all of these need new corpus scripts. Three additions I'd specifically suggest:

  1. Add a fromTo corpus script — any meaningful coverage of the third parser method.
  2. Either use breatheRepeats to feed a tl.repeat or drop it; otherwise the AST-binding paths through resolveNode aren't covered.
  3. Add at least one negative number (e.g. { y: -46 }) somewhere — the UnaryExpression arm is unrepresented.

The fourth method (set) is parsed-but-filtered behavior; a comment in the test explaining why goldens don't show gsap.set outputs would close that loop.

[concern — snapshot ratcheting risk on parser-touching PRs] The Meriyah swap is the intended golden diff. But this baseline now also captures behavior for every other PR that touches gsapParser.ts or gsapSerialize.ts between now and then. A contributor who lands an incidental parser tweak and regenerates goldens with --update-snapshots makes the change invisible in the next reviewer's eye.

Mitigation: a .github/CODEOWNERS rule on packages/core/src/parsers/__goldens__/** requiring an extra reviewer on golden updates, or a CONTRIBUTING.md note saying "if you touch a __goldens__ file, justify the regenerated diff in the PR body." Not blocking — discipline-by-convention works fine for a small team — but worth a one-line note somewhere.

[nit — trailing-no-newline on golden files] The \ No newline at end of file markers on each golden output (e.g. minimal.serialized.js:6 has no trailing \n). This is whatever parseGsapScript + JSON.stringify produce — vitest captures the bytes literally. Not a bug, but the inconsistency between "has trailing whitespace" and "no trailing newline" can confuse editors that auto-add \n on save. Worth a .gitattributes entry like packages/**/__goldens__/** -text to disable line-ending normalization on these files, or just be aware that someone editing a golden by accident will get a noise diff.

Nits

  • g = (name: string) => join(__goldens__, name) (line 21) is a one-call helper used 6x — fine, but if the corpus grows past one parser module, consider just inlining join(__goldens__, "minimal.parsed.json") for readability.
  • beforeAll(() => { ({ parsed, serialized } = parseAndSerialize(MINIMAL_SCRIPT)); }) — could be beforeAll(() => Object.assign(this, parseAndSerialize(…))) if you wanted to avoid the destructuring assign pattern, but the current shape is fine.

Plan-doc alignment

T6a = "Recast/Babel snapshot baseline before parser swap." Scope matches. The follow-up R-branch is the Meriyah swap which expects this golden to be present as a regression gate.

Verdict

Right shape for the goal. The corpus-coverage gap (4 supported methods, 2 exercised) is the only concern worth surfacing as something to address before this lands as the swap-PR's regression gate — even one additional fromTo corpus script materially tightens the safety net. Leaving as a comment.

Review by Rames D Jusso

Extends manualEditsDomPatches.test.ts with rotation and motion pairs.
Same 4-pattern structure: populated, empty, clear restores originals,
build/clear symmetry. Merges duplicate manualEditsTypes import block.
…terization

Fixes four gaps identified in max-setting code review:
- Box-size clear: replace arrayContaining with full ordered toEqual (30 ops)
- Box-size / pathOffset / rotation clear: add empty-string coercion tests
  (origVal||null must produce null, not set property to "")
- Rotation clear: add test for absent STUDIO_ORIGINAL_ROTATION_TRANSFORM_ORIGIN_ATTR
- Motion clear: prove input-independence by calling with both empty and populated
  element and asserting identical output
… (TU)

Deduplicate helpers shared by T1 (htmlParser.roundtrip.test.ts) and T2
(stableIds.test.ts). Both files inline identical implementations; extract
to test-utils.ts so future parser tests (T6a…) import one copy.

Also fix lefthook fallow command to unset GIT_DIR+GIT_INDEX_FILE before
running — those vars are set by git in worktree hook context and block
fallow’s internal temp-worktree creation.
All 14 tests are it.todo, following the T4 pattern. The stubs define the
full createPreviewAdapter interface — elementAtPoint (root exclusion,
hf-id ancestor walk, opacity filter), applyDraft/revertDraft (draft
marker lifecycle), commitPreview (patch derivation), and getElementTimings
(data-start/data-end reader).

createPreviewAdapter does not exist yet; R7 implements it and converts
these stubs to real assertions.
@vanceingalls vanceingalls force-pushed the 06-07-test_core_add_t10_previewadapter_contract_stubs_spec_for_r7_ branch from b911db9 to 2146fea Compare June 8, 2026 02:09
@vanceingalls vanceingalls force-pushed the 06-07-test_core_add_t6a_gsap_parser_golden_baselines_recast_babel_snapshot_ branch from d4a165d to 534b77d Compare June 8, 2026 02:20
6 toMatchFileSnapshot tests across 3 representative scripts (minimal,
moderate, complex). Captures parseGsapScript + serializeGsapAnimations
output before the Recast → Meriyah swap so any parser change is detected
as a golden diff rather than a silent behavioral regression.

Goldens live in src/parsers/__goldens__/ and are checked in. Add
__goldens__/** to fallow ignorePatterns (data files, not modules) and to
.prettierignore so oxfmt does not reformat vitest-written snapshot files.
@vanceingalls vanceingalls force-pushed the 06-07-test_core_add_t6a_gsap_parser_golden_baselines_recast_babel_snapshot_ branch from 534b77d to 4095bfc Compare June 8, 2026 02:21
Base automatically changed from 06-07-test_core_add_t10_previewadapter_contract_stubs_spec_for_r7_ to main June 8, 2026 02:22
@vanceingalls vanceingalls dismissed miguel-heygen’s stale review June 8, 2026 02:22

The base branch was changed.

vanceingalls added a commit that referenced this pull request Jun 8, 2026
T6a (#1263): add fromTo corpus script (third parser method) with negative
position values to exercise UnaryExpression arm; drop unused breatheRepeats
from COMPLEX_SCRIPT; generate fromto.parsed.json + fromto.serialized.js goldens.

T10 (#1262): split applyDraft move/resize into two stubs; add applyDraft edge-
case describe block (concurrent gestures, idempotent revert, playhead-change
stability, nested sub-composition root); add getElementTimings stub for absent
data-start/end on a data-hf-id element.

T7 (#1267): expand to full parity with T3 — add text-content, attribute, and
fallthrough stubs (Core sourceMutation supports all patch types via
patchElementInHtml).
@vanceingalls vanceingalls merged commit 184ef03 into main Jun 8, 2026
37 checks passed
@vanceingalls vanceingalls deleted the 06-07-test_core_add_t6a_gsap_parser_golden_baselines_recast_babel_snapshot_ branch June 8, 2026 02:23
vanceingalls added a commit that referenced this pull request Jun 8, 2026
T6a (#1263): add fromTo corpus script (third parser method) with negative
position values to exercise UnaryExpression arm; drop unused breatheRepeats
from COMPLEX_SCRIPT; generate fromto.parsed.json + fromto.serialized.js goldens.

T10 (#1262): split applyDraft move/resize into two stubs; add applyDraft edge-
case describe block (concurrent gestures, idempotent revert, playhead-change
stability, nested sub-composition root); add getElementTimings stub for absent
data-start/end on a data-hf-id element.

T7 (#1267): expand to full parity with T3 — add text-content, attribute, and
fallthrough stubs (Core sourceMutation supports all patch types via
patchElementInHtml).
vanceingalls added a commit that referenced this pull request Jun 8, 2026
T6a (#1263): add fromTo corpus script (third parser method) with negative
position values to exercise UnaryExpression arm; drop unused breatheRepeats
from COMPLEX_SCRIPT; generate fromto.parsed.json + fromto.serialized.js goldens.

T10 (#1262): split applyDraft move/resize into two stubs; add applyDraft edge-
case describe block (concurrent gestures, idempotent revert, playhead-change
stability, nested sub-composition root); add getElementTimings stub for absent
data-start/end on a data-hf-id element.

T7 (#1267): expand to full parity with T3 — add text-content, attribute, and
fallthrough stubs (Core sourceMutation supports all patch types via
patchElementInHtml).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants