Commit Graph
3 Commits
Author SHA1 Message Date
James RussoandClaude Opus 4.8 cf573f7f3f fix(core,producer,cli): pre-flight validation for empty/malformed sub-compositions (#1831)
* fix(core,producer,cli): pre-flight validation for empty/malformed sub-compositions

The #1 render failure bucket in production telemetry (PostHog project 356858,
dashboard 1783183 "HyperFrames — Bottom-Line & Activation"; ~65-69K
occurrences / ~27-28K affected users over 30 days, ~80% via AI-agent
authoring flows) is a `data-composition-src` reference pointing at a scene
file that is empty, malformed, or missing.

Root cause, traced end-to-end:
- The literal error "Composition HTML is empty or could not be parsed: <path>"
  is real (not a PostHog paraphrase) — thrown by a since-reverted guard in
  packages/core/src/compiler/inlineSubCompositions.ts (#1364), then changed to
  a silent skip in #1678 to avoid aborting renders on partial content during
  authoring. #1629 added per-assembler guards for 3 skill workflows
  (product-launch-video, faceless-explainer, pr-to-video), but general-video
  and hand-authored flows — where the dominant filename `scene-title.html`
  (40K+/68K of the bucket) originates — have no assembler and thus no guard.
  #1678 assumed the assembler guards from #1629 covered this pre-render; they
  only covered 3 of the many authoring flows.
- On current `main`, an empty/malformed data-composition-src file no longer
  crashes or throws during render — it's silently dropped by the tolerant
  inliner. Reproduced locally: `hyperframes render` on a project with an
  empty scene-title.html "succeeds" after ~93s (two 45s
  pollSubCompositionTimelines timeouts) with the scene silently missing from
  the output video. `hyperframes validate` also reports "No console errors"
  for the same broken project.
- The raw `Cannot destructure property 'firstElementChild' of
  'documentElement' as it is null` crash reproduces directly against
  linkedom (the DOMParser polyfill packages/cli/src/utils/dom.ts installs in
  the real CLI runtime) for empty and non-HTML input — confirmed with a
  standalone repro script, not just inferred. jsdom/happy-dom (used in this
  repo's own test environment) are spec-compliant and never produce a null
  documentElement, which is why this needed a linkedom-specific test file.

Fix:
- New shared helper `checkSubCompositionUsability`
  (packages/core/src/compiler/subCompositionValidity.ts) is the single
  source of truth for "is this data-composition-src file usable" — mirrors
  the inliner's own parse/template/body logic so all callers agree.
- `inlineSubCompositions.ts` (preview/studio bundling) now uses the shared
  helper internally but keeps its #1678 tolerant skip-and-continue behavior
  unchanged — mid-authoring iteration on a partial project must keep
  working. `onMissingComposition` now also receives a human-readable reason.
- New render-only pre-flight (`assertSubCompositionsUsable` in
  packages/producer/src/services/htmlCompiler.ts) walks every
  data-composition-src reference (including nested ones, root-relative,
  matching parseSubCompositions' own resolution) before any compilation
  work starts, and throws naming every offending file at once. This is
  unconditional — not gated behind --strict — because a render that
  silently drops a scene is strictly worse than one that refuses to start.
  Confirmed locally: render now fails in ~0.4s with an actionable message
  instead of "succeeding" after 93s with a missing scene.
- New `hyperframes lint` rule `missing_or_empty_sub_composition`
  (packages/cli/src/utils/lintProject.ts) surfaces the same check as a
  file-scoped, actionable lint error (already unconditional — lint exits 1
  on any error).
- `hyperframes validate` now also runs this check before launching a
  browser, so it no longer reports "No console errors" for a project with a
  broken sub-composition.
- `packages/core/src/parsers/htmlParser.ts`: guarded every
  `documentElement`-may-be-null access (parseHtml, updateElementInHtml,
  addElementToHtml, removeElementFromHtml, extractCompositionMetadata,
  validateCompositionHtml) with a new typed `CompositionHtmlParseError` (or,
  for validateCompositionHtml's collect-and-report contract, a typed
  validation failure) instead of a raw crash.

Tests: empty file, whitespace-only, malformed/non-HTML, missing file, nested
sub-compositions (both happy path and broken-grandchild), and the happy path
— at the shared-helper, lint, and render pre-flight layers.

Not changed: the AI-agent authoring skills (skills/*). general-video and
hand-authored flows have no assemble-index.mjs equivalent to guard, so the
fix is at the CLI/render layer instead — flow-agnostic, covers every
authoring path, and the skills' existing "run lint/validate and stop on
failure" guidance now actually catches this class of mistake once run.

Not run in this environment: the producer package's full regression-harness
test suite (`bun test` in packages/producer) — it performs heavy real
rendering (S3 asset downloads, Google Fonts fetches, full video encodes) and
did not complete in a reasonable time in this sandbox. Verified instead via
the targeted test file for all touched code (76/76 passing), whole-repo
typecheck/build/oxlint, `fallow audit` (complexity/duplication/dead-code
gate, clean), and manual end-to-end CLI runs (render/lint/validate) against
reproduction projects, including a nested sub-composition scenario. CI
should run the full producer suite before merge.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* refactor(parsers,lint): port empty-composition pre-flight to extracted packages

Rebased onto main, which extracted @hyperframes/lint from core (lint depends
only on parsers, not core). Relocate checkSubCompositionUsability from core to
@hyperframes/parsers so both core (inliner) and lint can consume it without a
core<->lint cycle; core keeps a @deprecated re-export shim.

Correctness fixes from code review:
- checkSubCompositionUsability now returns "no-composition-root" when the
  <template>/<body> content has no [data-composition-id] element (previously
  a marker-free placeholder body passed both guards).
- lint's missing/empty sub-composition rule now only checks files reachable
  via data-composition-src from the root (matching render pre-flight), instead
  of a raw filesystem walk that false-positived on orphaned files.
- drop `as string` cast in inlineSubCompositions in favor of an explicit
  null guard (per CLAUDE.md).

Review-comment items:
- move EmptyCompositionError JSDoc above the class (was above the adapter fn).
- correct stale circular-ref comment to match actual silent-skip behavior.
- rewrite self-contradicting lint message ("silently drop") to describe the
  new loud render-pre-flight abort.
- add the __PLACEHOLDER__ (/^__[A-Z_]+__$/) skip to the render pre-flight so
  it agrees with lint.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-01 14:25:44 -07:00
Miguel Ángel 6aaab32ccb refactor: make @hyperframes/lint depend only on parsers (#1773)
* refactor: make @hyperframes/lint depend only on parsers, not core

Relocates the leaf utilities lint pulled from core — URL/asset-path helpers,
font aliases, and the slideshow manifest parser — into the standalone
@hyperframes/parsers base, and drops @hyperframes/core from lint's
dependencies. Core keeps back-compat re-export stubs at the old paths, so
producer/studio/cli are unchanged.

Why: lint was the lightweight validator from #1749, but depending on core
transitively pulled studio-server (hono) and bpm-detective — irrelevant to
linting. Now installing @hyperframes/lint pulls only parsers + postcss, and
the core<->lint dependency cycle is gone.

- parsers main entry stays browser-safe (pure utils only); the node:path
  asset helpers live behind the new @hyperframes/parsers/asset-paths subpath
- slideshow parser exposed via @hyperframes/parsers/slideshow

* feat(lint): add browser entry; harden CSS url() regex (ReDoS)

@hyperframes/lint/browser — a fully client-side rule engine (lintHyperframeHtml,
lintMediaUrls, shouldBlockRender) with zero node: builtins, so browser-only
editors can validate compositions with no Node.js and no server round-trip.
Closes the browser-validation ask on #1749.

- shouldBlockRender extracted from the fs-bound project.ts into its own pure
  module so the browser entry stays node-free
- pure composition primitives (data types, font aliases, URL helper) exposed via
  a new recast-free @hyperframes/parsers/composition subpath, so the browser
  bundle tree-shakes out the GSAP/recast machinery (verified: esbuild
  platform=browser bundles with 0 node builtins)
- lint built with a platform:browser tsup pass — compile-time guarantee the
  browser entry never pulls a node builtin
- harden CSS_URL_RE against polynomial ReDoS (CodeQL js/polynomial-redos);
  behavior-preserving, verified against existing tests + an old/new parity check
- parsers/lint marked sideEffects:false
2026-06-27 13:51:21 -04:00
Miguel Ángel cdf9c817e1 refactor: extract @hyperframes/parsers from core (#1755)
## Summary

Extracts the GSAP parser/writer suite, HTML parser, hf-ids, spring-ease, and the shared composition data types out of `@hyperframes/core/src/parsers/` into a new, independently-publishable **`@hyperframes/parsers`** package.

This is the foundation of the [#1749](https://github.com/heygen-com/hyperframes/issues/1749) effort: make HyperFrames' parsing/linting/validation usable as plain libraries in a Node app, without shelling out to the CLI. Parsers is the standalone base every other extracted package builds on.

**Part 1 of 3** — splits #1754 into independently-reviewable pieces. Parts 2 (lint) and 3 (studio-server) stack on this branch.

## What moves

| | |
|---|---|
| Source moved out of core | **~9,900 LOC** (`src/parsers/` → `packages/parsers/src/`) |
| Total lines removed from core (incl. tests + goldens) | ~19,600 |
| Files relocated | 39 |
| Tests carried over | **660 passing** (5 skipped, 3 todo) |

The big movers: `gsapParser` / `gsapParserAcorn` (the recast + acorn dual parsers), `gsapWriterAcorn`, `gsapSerialize`, `gsapUnroll`, `htmlParser`, `hfIds`, `springEase`, `stableIds`, plus the `__goldens__` corpus.

## Bundle footprint of the new package

| Artifact | Size |
|---|---|
| `dist/` (unpacked) | 1.7 MB |
| npm tarball (packed) | 409 KB |
| `dist/index.js` | 90 KB (**~21 KB gzipped**) |
| Heaviest entries | `gsapWriterAcorn.js` 93 KB · `gsapParser.js` 91 KB |

Most of the weight is the GSAP AST machinery (recast/babel/acorn). It's tree-shakeable via subpath entries (`@hyperframes/parsers/hf-ids`, `/gsap-constants`, etc.) so a consumer that only needs `hf-ids` (2 KB) doesn't pull the parsers.

## How `@hyperframes/core` changes

The interesting part: **core sheds its entire AST toolchain.**

| core `dependencies` | before | after |
|---|---|---|
| count | 9 | 6 |
| removed | — | `@babel/parser`, `acorn`, `acorn-walk`, `magic-string`, `recast` |
| added | — | `@hyperframes/parsers`, `linkedom` |

Before this PR, importing `@hyperframes/core` at all dragged in babel + recast + acorn just to construct types. Now those live behind `@hyperframes/parsers`, and a consumer that only wants core's runtime/compiler types never resolves the parser stack. Core keeps thin `@deprecated` re-export stubs at the old subpaths (`@hyperframes/core/gsap-parser`, `/gsap-constants`, …) so nothing downstream breaks.

## Design notes

- **`"bun"` export condition before `"node"`** in every package export. Bun resolves the TypeScript source directly (no pre-built `dist/`), while Node/tsx/Docker contexts fall through to `"node"` → `dist/`. This keeps the dev loop zero-build while published artifacts stay Node-consumable.
- `@hyperframes/parsers` is **standalone** — zero `@hyperframes/*` dependencies — so it can be the base of the stack.

## Test plan

- [x] `bun run --filter @hyperframes/parsers test` — 660 tests pass
- [x] `bun run --filter @hyperframes/sdk test` — 382 tests pass
- [x] `bun run build` — full monorepo build succeeds
- [x] Fallow audit passes on CI
2026-06-27 00:46:26 -04:00