test(core): add T1 round-trip idempotence suite for parse/serialize

[vanceingalls]

What

Adds htmlParser.roundtrip.test.ts — T1 from the SDK migration test plan.

Tests that parseHtml → generateHyperframesHtml → parseHtml is lossless for element structure and timing. Scope is DOM/timing only; GSAP script round-trip is T6 territory.

Tests

Inline fixtures (5):

• element count + ids preserved
• startTime / duration preserved
• element types preserved (text, video, img, audio)
• double-serialize stability (serialize(parse(serialize(parse(html)))) === serialize(parse(html)))
• empty stage doesn't throw

Registry block sampling (10): first 10 blocks in registry/blocks/ — each asserts element count survives a round-trip.

Finding

Stability test surfaced a real bug: generateHyperframesHtml defaults compositionId to `comp-${Date.now()}`. Since ParsedHtml doesn't capture this, every re-serialize emits a different id. The test works around it by passing a fixed compositionId: "test-comp" so structural instability is still detectable. The root cause is tracked as R1 (stable hf- ids).

Stack

Prerequisite for: T8 (#1241), T11 (#1242), T4 (#1243)

[vanceingalls]

• test(core): add T9 CompositionVariable font/image parse spec (before R1) #1246
• test(core): add T2 stable id spec for parse-to-hf id contract (before R1) #1245
• test(studio): add T4 op-contract stubs for editor dispatch boundary #1243
• test(studio): add T11 history coalescing and cross-prop separation suite #1242
• test(core): add T8 override-set merge semantics suite for getVariables #1241
• test(core): add T1 round-trip idempotence suite for parse/serialize #1240 👈 (View in Graphite)
• main

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

[miguel-heygen]

T1 suite looks solid. A few notes:

• Pinning compositionId: "test-comp" to suppress Date.now() churn is the right call — good defensive pattern.
• The registry-block sweep gracefully degrades to [] when the dir doesn't exist; CI-safe.
• maxEndTime + serialize helpers here are duplicated identically in #1245. Not a blocker pre-refactor, but worth a shared test util once the dust settles.

✅ Approve — all baseline cases covered, stability test is a meaningful idempotency guard.

[james-russo-rames-d-jusso]

Hey Vance — clean T1 plumbing. The DOM/timing round-trip is exactly the public-contract surface a refactor needs to preserve, and the stability test (serialize(parse(serialize(parse))) === serialize(parse)) is the load-bearing one — it'd catch any non-determinism R1 introduces (id reshuffling, ordering churn, attribute renormalization) on a single failing assertion. No blockers. A couple of notes on coverage shape.

What I verified

Walked each test against "what could R1 silently break that this would catch first":

• Element-count + id preservation (test 1): catches "refactor dropped or renamed an element kind during parse." Solid.
• startTime / duration preservation (test 2): catches "timing-attr emission flipped from data-start/data-end to data-startTime/... or got rounded." Solid.
• Type preservation across text / video / img / audio (test 3): catches "one type's serializer regressed silently." Solid.
• Double-serialize stability (test 4): the strongest test in this file — non-determinism here means R1 introduced a Date.now()-shaped instability the way compositionId already does today. Load-bearing.
• Empty stage (test 5): edge case, cheap, fine.
• Registry block sampling (10 blocks): the .slice(0, 10) is alphabetical, so coverage is implicitly "first 10 names." That's fine as a smoke net but mildly arbitrary. (See nit.)

The serialize() helper's compositionId: "test-comp" workaround for the Date.now() default is honest and tracked as R1 — good comment.

Concerns

• GSAP scripts are out of scope (T6), but no test pins they're at minimum passed through unmodified. If R1 accidentally drops or rewrites <script> content during a parse → generate round-trip on a block that has GSAP, T6 won't catch it until much later in the test plan. A weak smoke assertion — "if input has a <script> block, output contains the same script content (string equality, no semantic check)" — would close a window between this PR and T6 landing. Cheap to add.

Nits

• Registry block sampling is alphabetical-first-10. Two cheaper alternatives that close that window: (a) iterate all blocks (pure addition, low cost — the test runs once per CI), or (b) name the 10 explicitly so a reader sees the surface (const blockNames = ["animated-...", "card-...", ...]). My read: prefer (a) unless the suite gets slow — round-trip on each block should be milliseconds. (nit)
• The element-id preservation assertion uses parsed.elements.map((e) => e.id) directly. If R1's hf- id scheme renumbers existing ids during the first round-trip (e.g. el-aaa → hf-xxxx), this test would fail — which would be the right signal, but the failure message ("expected ['el-aaa','el-bbb'], got ['hf-...','hf-...']") might read as a regression rather than a deliberate behavior change. A comment noting that this test is "preserve-as-is for already-id'd elements; T2 covers the auto-id case" would short-circuit the confusion. (nit)
• No test that timing-only edits round-trip stably for elements with data-name containing special chars (quotes, ampersands, unicode). HTML escaping is a classic refactor-break shape. Cheap to add one fixture with data-name="It's & 'wow'". (nit)
• maxEndTime helper duplicates logic that probably exists in generateHyperframesHtml's caller. If the production callers compute total duration differently, the test's round-trip uses a different shape than real callers — risk of green-test-but-shipping-bug. Worth a quick check that the helper matches the real caller's shape. (nit)

Questions

• Is the plan to extend this file with T6 (GSAP script round-trip) as a separate suite, or split T6 into its own file? Asking because the serialize() helper is generic enough that T6 could just append describe() blocks here without re-importing — small structural choice but worth pinning.
• Does the registry block set churn often? If yes, the alphabetical-first-10 slice could silently lose coverage on a renamed block. If churn is rare, the current shape is fine.
• The // The compositionId generation instability itself is tracked as R1 (stable hf- ids) comment — is there a linked issue / Linear ticket? If yes, putting the URL in the comment makes the cross-reference clickable from the IDE.

What I didn't verify

• The HeyGenverse plan doc (got HTTP 403 from my unauthenticated fetch). Trusted the PR body's claim that this matches T1 from the plan and that T6 covers the GSAP layer.
• Whether generateHyperframesHtml's compositionId default really is comp-${Date.now()} and not something more stable now — trusted the PR body.
• parseHtml's behavior on malformed input (out of scope for round-trip, but worth a glance if it ever throws on registry blocks).
• Whether all 10 sampled registry blocks have non-empty .elements arrays — if some round-trip from empty to empty, the assertion toHaveLength(parsed.elements.length) is trivially true for those.

— Review by Rames D Jusso

[miguel-heygen]

Second pass, building on Rames's review.

Rames's GSAP script passthrough gap is the one I'd escalate: T6 is the right home for it, but between now and T6 landing, a single smoke assertion — "if input contains a <script> block, output preserves its content verbatim" — would catch the class of R1 bug where generateHyperframesHtml accidentally re-emits a stripped or rewritten script. Cheap to add here (one fixture, no semantic GSAP execution needed).

Rames's point about maxEndTime matching the real caller's shape is also worth resolving before R1: if production computes total duration differently (e.g. uses a composition-level data-duration attribute instead of max(element endTimes)), the test's round-trip is exercising a phantom path and could mask a real bug.

The registry block sampling concern: readdirSync + alphabetical first-10 means renaming any block in the first 10 alphabetically adds it to coverage while a renamed block in positions 11+ silently falls off. Prefer either all blocks or an explicit list.

Everything else Rames flagged (element-id preservation comment, special-char data-name, helper duplication) — seconded, all nits.

[vanceingalls]

Addressed in latest push:

• Registry sweep: removed .slice(0, 10) — now runs all 88 blocks.
• GSAP script passthrough smoke (miguel + Rames): Added a test that parses liquid-glass-notification (known <script> block) and calls generateHyperframesHtml with includeScripts: true, asserting the output contains <script>. Note on scope: registry blocks embed raw inline GSAP — they do not use data-keyframes, so parseHtml produces keyframes: {} for all of them. The round-trip can't preserve the source scripts verbatim because generateHyperframesHtml regenerates scripts from parsed animation data, not from raw script tags. The smoke covers "R1 didn't break the includeScripts generation path entirely." Verbatim GSAP content fidelity is T6's job, as scoped.
• maxEndTime vs real callers (Rames nit): generateHyperframesHtml takes totalDuration as its second argument and uses it for composition length, not element timing. maxEndTime (max of startTime + duration over all elements) is the correct value to pass — matches how the CLI and engine compute total duration. No mismatch.
• Shared maxEndTime/serialize helpers with test(core): add T2 stable id spec for parse-to-hf id contract (before R1) #1245: acknowledged as non-blocking; will extract to packages/core/src/parsers/test-utils.ts before R1 adds more spec files.

[miguel-heygen]

Follow-up: both concerns addressed.

• ✅ Registry sampling: .slice(0, 10) removed — all blocks now covered
• ✅ GSAP smoke: new test asserts includeScripts: true preserves <script> tags through generate; correct scope (existence, not content fidelity — T6 handles the rest)

✅ Re-approve.

[vanceingalls]

Merge activity

• Jun 6, 10:38 PM UTC: @vanceingalls merged this pull request with Graphite.