mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-14 18:01:20 +08:00
James Russo
cf573f7f3f
* 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>
137 lines
5.6 KiB
TypeScript
137 lines
5.6 KiB
TypeScript
/**
|
|
* Shared "is this sub-composition file usable?" check.
|
|
*
|
|
* `data-composition-src` files are authored by AI agents far more often than
|
|
* by humans clicking a UI. The dominant real-world failure is a scene worker
|
|
* that dies mid-write (or a step that references a scene before writing it),
|
|
* leaving an empty or partial `compositions/scene-*.html` on disk. Historically
|
|
* this surfaced in three different ways depending on which code path touched
|
|
* the file first:
|
|
*
|
|
* 1. A raw crash inside linkedom's `Document.head` getter — destructuring
|
|
* `firstElementChild` off a `null` `documentElement` — when the file is
|
|
* empty or contains no parseable markup.
|
|
* 2. An actionable-but-late `Error` thrown deep inside the render compiler
|
|
* (see git history: #1364), which aborted the whole render.
|
|
* 3. A silent skip (see git history: #1678) that drops the scene from the
|
|
* output with only a `console.warn`, producing a materially broken
|
|
* video (missing scene, no error surfaced anywhere) with no clear
|
|
* signal to the caller.
|
|
*
|
|
* This module gives every consumer (lint, render pre-flight, the tolerant
|
|
* inliner) a single, shared definition of "usable" so they can never
|
|
* disagree about whether a given file would render something. It lives in
|
|
* `@hyperframes/parsers` (rather than `@hyperframes/core`, where it
|
|
* originated) because `@hyperframes/lint` needs it too, and `lint` cannot
|
|
* depend on `core` — `core` already depends on `lint` — so this shared,
|
|
* dependency-free check lives in the common ancestor package both `core`
|
|
* and `lint` already depend on.
|
|
*
|
|
* `inlineSubCompositions.ts` (in `@hyperframes/core`) intentionally stays
|
|
* tolerant (skip + continue) for the preview/studio bundling path, where
|
|
* partial content while iterating is expected. `lint` and the render
|
|
* pre-flight check (`packages/producer/src/services/htmlCompiler.ts`) use
|
|
* this helper to fail loudly and name the exact offending file, because a
|
|
* render that silently drops a scene is strictly worse than a render that
|
|
* refuses to start.
|
|
*/
|
|
|
|
export type SubCompositionValidityReason =
|
|
| "empty"
|
|
| "unparsable"
|
|
| "no-content"
|
|
| "no-composition-root";
|
|
|
|
export interface SubCompositionValidity {
|
|
ok: boolean;
|
|
/** Present when `ok` is false. */
|
|
reason?: SubCompositionValidityReason;
|
|
/** Human-readable detail suitable for direct inclusion in an error message. */
|
|
detail?: string;
|
|
}
|
|
|
|
/** Minimal shape both linkedom's `Document` and `happy-dom`'s satisfy. */
|
|
export interface ParsableDocumentLike {
|
|
documentElement: { outerHTML?: string } | null;
|
|
body?: { innerHTML?: string | null } | null;
|
|
querySelector(selector: string): { innerHTML?: string | null } | null;
|
|
}
|
|
|
|
/**
|
|
* Check whether `html` (the raw file contents resolved for a
|
|
* `data-composition-src` reference) is non-empty and parses to a document
|
|
* that actually contains renderable content.
|
|
*
|
|
* Mirrors the content-detection steps in `inlineSubCompositions` exactly
|
|
* (resolve → parse → find `<template>` or `<body>` content → parse that →
|
|
* confirm a `[data-composition-id]` root exists in it), so a file that
|
|
* passes this check is guaranteed to produce non-empty output from the
|
|
* inliner, and a file that fails it is guaranteed to hit one of the
|
|
* inliner's `onMissingComposition` branches.
|
|
*
|
|
* @param html Raw file contents, or `null`/`undefined` if the file could not
|
|
* be read (e.g. missing from disk). Callers should distinguish "missing"
|
|
* from "empty" in their own error message using a separate existence
|
|
* check — this function only inspects content.
|
|
* @param parseHtml Parse an HTML string into a document. Pass linkedom's
|
|
* `parseHTML(html).document` or the core bundler's `parseHTMLContent`.
|
|
*/
|
|
export function checkSubCompositionUsability(
|
|
html: string | null | undefined,
|
|
parseHtml: (html: string) => ParsableDocumentLike,
|
|
): SubCompositionValidity {
|
|
if (html == null || !html.trim()) {
|
|
return {
|
|
ok: false,
|
|
reason: "empty",
|
|
detail: "the file is empty (0 bytes or whitespace-only)",
|
|
};
|
|
}
|
|
|
|
const compDoc = parseHtml(html);
|
|
if (!compDoc.documentElement) {
|
|
return {
|
|
ok: false,
|
|
reason: "unparsable",
|
|
detail: "the file's contents could not be parsed as HTML",
|
|
};
|
|
}
|
|
|
|
// Find content: prefer <template>, fall back to <body> — same precedence
|
|
// inlineSubCompositions uses when extracting the sub-composition's markup.
|
|
const contentRoot = compDoc.querySelector("template");
|
|
const contentHtml = contentRoot ? contentRoot.innerHTML || "" : compDoc.body?.innerHTML || "";
|
|
if (!contentHtml.trim()) {
|
|
return {
|
|
ok: false,
|
|
reason: "no-content",
|
|
detail: "the file has no <template> or <body> content to render",
|
|
};
|
|
}
|
|
|
|
const contentDoc = parseHtml(contentHtml);
|
|
if (!contentDoc.documentElement) {
|
|
return {
|
|
ok: false,
|
|
reason: "unparsable",
|
|
detail: "the file's <template>/<body> contents could not be parsed as HTML",
|
|
};
|
|
}
|
|
|
|
// The content must contain an actual composition root — the element the
|
|
// inliner looks for (`contentDoc.querySelector("[data-composition-id]")`)
|
|
// to know what to inject into the host. Well-formed but marker-free HTML
|
|
// (e.g. an AI-authored placeholder like `<body><p>TODO</p></body>`) parses
|
|
// fine and has non-empty content, but has nothing for the inliner to find.
|
|
if (!contentDoc.querySelector("[data-composition-id]")) {
|
|
return {
|
|
ok: false,
|
|
reason: "no-composition-root",
|
|
detail:
|
|
"the file's <template>/<body> content has no element with a data-composition-id attribute",
|
|
};
|
|
}
|
|
|
|
return { ok: true };
|
|
}
|