fix(core): render standalone sub-composition previews + surface captured video assets (cli) by WaterrrForever · Pull Request #1631 · heygen-com/hyperframes

WaterrrForever · 2026-06-22T07:14:29Z

What

Two independent source-layer changes that came out of the skills/capture work:

fix(core) — standalone sub-composition previews that rendered blank/black now render correctly. Three distinct failure modes in buildSubCompositionHtml, all of which only surface in a standalone preview (a full composition masks them via its mounting host).
feat(cli) — generateAssetDescriptions now surfaces captured video clips in the asset description list, tagged [video].

Why

Standalone sub-composition previews

A full composition mounts each sub-composition under a data-composition-src host that supplies its size, background, and composition-id binding. A standalone preview has no such host, so any of the following silently produces a blank/black frame:

Digit-leading #id CSS selectors. A CSS identifier cannot start with a digit, so an authored rule like #01-wall-pushes-back { width: 1920px; height: 1080px; background: … } is an invalid selector and the browser drops the whole rule — taking the root's size and background with it. In a full composition the host paints/sizes the frame so the collapse is masked; standalone falls back to height: 0 + transparent → blank.
Regex <template> extraction fooled by comments. The old /<template[^>]*>([\s\S]*)<\/template>/ could latch onto a literal "<template>" appearing inside an HTML comment (e.g. a head note like "the HF runtime clones ONLY <template> contents") and mis-slice the capture, re-wrapping the real content in an inert <template> that the browser never renders → no [data-composition-id] element, no registered timeline → blank.
data-composition-id only on the <template> tag. A common authoring pattern. With no host wrapper to carry the id, the rendered body has no [data-composition-id] element, the runtime never binds the registered GSAP timeline, seeking does nothing, and the frame renders at its pre-animation state (GSAP fromTo pins opacity:0) → blank.

Captured video asset descriptions

For a product/demo capture, the moving clips are usually the strongest hero material, but generateAssetDescriptions only described images/SVGs/fonts — videos were invisible to downstream planners.

How

`packages/core/.../subComposition.ts`

fixDigitLeadingIdSelectors(root) — collects element ids that start with a digit, then rewrites matching #id selectors in <style> blocks to their escaped, valid form (#\30 1-wall-pushes-back, which still matches id="01-wall-pushes-back") so the rule applies and the full declaration block returns. Scoped to ids actually present on elements, matched as #id not followed by another ident char, so hex colors (#1F2BE0) and other values are never touched. Run across the whole document (ids live in <body>, their rules may live in a <head> <style>) and in each content branch of buildSubCompositionHtml.
extractTemplateInnerHtml(rawComp) — locates the wrapping <template> via querySelector("template") (a real element node) instead of a regex, so comment text can't fool it.
promoteTemplateCompositionId(rawComp, body) — when the id was declared only on the <template> tag and the content has no [data-composition-id], carries it onto the content's first rendered root element. No-op when an element already exposes the id, so compositions that already render correctly are untouched.

These compose cleanly with the post-processing added in #1605 (stripEmbeddedRuntimeScripts / tagRootCompositionFile): the new helpers handle content extraction, #1605's run afterward on the extracted body.

`packages/cli/.../contentExtractor.ts`

generateAssetDescriptions reads extracted/video-manifest.json (written earlier by captureVideoManifest, carrying each clip's DOM heading/caption + dims) and emits each downloaded clip first, tagged [video], with a trimmed caption/heading and ~W×H. The videos/ dir is skipped in the image walk (entries come from the manifest, which has the captions the bare files lack).

Test plan

packages/core/.../subComposition.test.ts — 4 new cases, all green (7 passed):

escapes #id selectors whose id starts with a digit so the rule is not dropped;
extracts the real <template> even when a head comment mentions the literal text <template>;
promotes the <template>'s data-composition-id onto the root element when the content lacks one;
does not add a duplicate data-composition-id when the root already has one.
bun run test (core) — 17 passed
bunx oxlint / oxfmt --check / typecheck — clean (lefthook pre-commit)
bun run build — @hyperframes/core + @hyperframes/cli build clean (the unrelated @hyperframes/studio tsup: command not found is a local env issue, untouched by this diff)
No contentExtractor unit test (it reads a capture-time manifest; behaviour is exercised by the capture pipeline)

Rebased on current main (origin/main @ #1605).

…ition preview A CSS identifier cannot start with a digit, so an authored rule like `#1-wall-pushes-back { ... }` is an invalid selector and the browser drops the whole rule — taking the root's size/background with it. A full composition masks this (the host stretches/paints the frame), but a standalone preview has no host, so the root collapses to height:0 + transparent and renders blank. extractFullDocumentParts now rewrites `#<digit-leading-id>` selectors to their escaped valid form (`#\30 1-...`, still matching the element id), scoped to ids actually present and matched only as `#id` not followed by an ident char so hex colors are never touched. Also harden the <template> inner-HTML extraction to use the DOM instead of a greedy regex. Tests added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit e43b377)

generateAssetDescriptions now reads extracted/video-manifest.json and emits each downloaded clip first, tagged [video], with its DOM heading/caption and dimensions — motion clips are usually the strongest hero material and downstream planners key off the [video] marker. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The function was split out into gsapDragPositionCommit.ts in #1605, but the test kept importing it from ./gsapDragCommit, which no longer exports it — yielding `is not a function` at runtime. Import from the correct module to match the production import in gsapRuntimeBridge.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

jrusso1020

Read the full diff + verified the helper logic against tests. The three failure modes are real, the fixes are targeted, and the test coverage pins each one. Posting as COMMENT (not APPROVE) because customer-partner PRs route stamp eligibility through the team's discipline — but on merit the fix is clean.

What I verified

fixDigitLeadingIdSelectors (subComposition.ts:54-67) — escapes #01-foo selectors via \<hex-of-first-char> <rest> (CSS-spec compliant). Scoped to ids that are actually present on elements (the digitIds Set), regex anchored with (?![\w-]) negative lookahead so hex color values (#1F2BE0) and other non-ident tokens are correctly left alone. Run on the whole document so head <style> rules referencing body ids are covered. The escape format \30 1-foo matches the canonical CSS Syntax Level 3 escape for digit-leading identifiers. ✓
extractTemplateInnerHtml — DOM-based extraction (parseHTML(...).querySelector("template")) instead of greedy regex. Comments can't fool querySelector since the HTML parser strips them before tree construction. ✓
promoteTemplateCompositionId — regex parses the template's data-composition-id, then no-ops when any descendant already carries the attribute (body.querySelector("[data-composition-id]") check). NON_RENDERED_TAGS Set (SCRIPT/STYLE/LINK/META/TEMPLATE/NOSCRIPT) keeps setAttribute off non-rendering siblings. Optional-chain on root?.setAttribute handles the edge case where all body children are non-rendering. ✓
Tests — 4 new tests in subComposition.test.ts cover digit-leading-id escape, template-extraction-fooled-by-comment regression, composition-id promotion, and the no-double-id no-op. The negative assertions (hex colors untouched, sibling <style>/<script> don't get data-composition-id) pin the "scoped" intent.
Video manifest in generateAssetDescriptions — reads extracted/video-manifest.json, tags entries [video], places them FIRST in the returned list (deliberate for downstream-planner hero-material framing). Caption fallback chain (caption → heading → "motion clip") is sensible. Fails silently if the manifest doesn't exist, which matches the surrounding try { ... } catch { /* no foo dir */ } style of the function. ✓

One nit — unrelated test refactor

The fourth file in the PR (packages/studio/src/hooks/gsapDragCommit.test.ts, 1/1) moves commitGsapPositionFromDrag's import from ./gsapDragCommit to ./gsapDragPositionCommit. The source file gsapDragPositionCommit.ts exists on this branch (verified via git ls-tree), but it landed via merged PRs in the branch's history that haven't squash-merged-equivalently to main yet — so the test import update is paired with content that's already in main via different SHAs.

Specifically: this branch contains commits 091137e3c (#1605), e0822c6e8 (#1553), f12754f5a (#1628) — all three merged to main via squash with different SHAs. git merge-base says merge-base == current main HEAD, so the merge will be clean per git's recursive heuristic. Just flagging that the test-import file is the only sign this PR's branch carries those upstream squashes — the import refactor reads as "unrelated cleanup" but is actually a necessary downstream of the squash mismatch.

Not a blocker — just worth knowing.

Stamp posture

Per team discipline on customer-partner PRs (Miao is allowlisted but not a trusted-stamper), the review is on-merit and stamp eligibility routes through the team. From my read: ready to stamp once a teammate confirms. CI shows no failures / no pending blockers.

Review by Jerrai

The function was split out into gsapDragPositionCommit.ts in #1605, but the test kept importing it from ./gsapDragCommit, which no longer exports it — yielding 'is not a function' at runtime. Import from the correct module. Inherited main breakage (same fix as #1631); fixes the Test CI check on this branch independently of merge order. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

miga-heygen

Independent Review — Miga

Read the full diff, PR description, and Rames's prior review. Performed independent analysis of CSS escaping correctness, DOM-based extraction safety, composition-id promotion logic, video asset surfacing, and test coverage. Broadly agree with Rames — the three fixes address real failure modes, the implementations are well-scoped, and the tests pin each one. A few findings worth surfacing:

CSS escaping — `fixDigitLeadingIdSelectors`

Verified against CSS Syntax Level 3. The escape form \30 1-foo is spec-correct — the mandatory space after the hex escape prevents ambiguity when the next character is also a hex digit (e.g. id="42" → #\34 2). The negative lookahead (?![\\w-]) correctly handles compound selectors (#01-wall .child, #01-wall:hover, #01-wall.active) since the boundary character is never in [\w-]. Hex color values are safe — they're never collected into the digitIds Set because the scan only queries [id] elements. REGEXP_SPECIALS escaping handles IDs with special characters (parens, brackets, etc.).

One subtlety worth calling out: escapeLeadingDigitIdent only escapes the first digit. For id="123-scene" this produces #\31 23-scene, which is valid — after the escape sequence, the CSS parser consumes 23-scene as the identifier continuation. This is correct but non-obvious; a brief inline comment on that function noting "only the leading digit needs escaping per CSS Syntax L3 §4.3.11" would help future readers. (Nit, not a blocker.)

DOM-based template extraction — `extractTemplateInnerHtml`

The switch from regex to parseHTML().querySelector("template") is the right call. querySelector operates on the parsed DOM tree, so comment text () is invisible to it. No XSS concern here — parseHTML is server-side (linkedom), scripts don't execute during parsing, and the content comes from local composition files. The regex→DOM change is strictly safer.

`promoteTemplateCompositionId` — sibling skip list

NON_RENDERED_TAGS covers SCRIPT, STYLE, LINK, META, TEMPLATE, NOSCRIPT. This is complete for the context: the children being iterated are extracted template content injected into <body>, so head-only elements like TITLE and BASE won't appear as siblings. The optional chain root?.setAttribute handles the edge case where all children are non-rendered — silent no-op is correct since there's no rendered element to tag. The early return when body.querySelector("[data-composition-id]") finds an existing element prevents the double-tag scenario (Test 4 pins this).

Minor note: the regex /<template[^>]*\sdata-composition-id\s*=\s*["']([^"']+)["']/i that extracts the composition id from rawComp uses [^>]* before the attribute. If a preceding attribute's value contains a literal > (valid when quoted), the regex stops early. This is a known regex-HTML limitation but vanishingly unlikely in tooling-generated composition files. Not a practical risk.

Video asset descriptions — `generateAssetDescriptions`

Ordering videos FIRST is a deliberate design choice for downstream planners — makes sense for hero-material framing. Caption fallback chain (caption → heading → "motion clip") is sensible. The 140-char truncation with whitespace normalization is appropriate for summary context.

One suggestion: v.localPath.split("/").pop() to extract the filename works on Linux/macOS but breaks on Windows-style paths (backslash separators). path.basename(v.localPath) handles both — one-line swap, zero risk, strictly better. Whether this matters depends on whether the manifest could ever be generated on Windows. (Low priority.)

The bare catch { } is consistent with surrounding code style and correct for optional enrichment data. If the manifest doesn't exist or is malformed, silently returning no video lines is the right behavior.

Test quality

The 4 new tests are well-targeted — each pins a specific failure mode with tight positive AND negative assertions. Test 3 (digit-leading ID escape) is particularly strong, verifying both the escape output and that hex color values survive untouched.

Gaps worth noting (nice-to-haves, not blockers):

Multiple digit-leading IDs in one document — the iteration logic in fixDigitLeadingIdSelectors processes them in a loop, but no test exercises two IDs simultaneously. A test with both id="01-wall" and id="02-music" in the same composition would pin this.
No-op path through promoteTemplateCompositionId — when <template> has no data-composition-id at all. The function returns early, but no test verifies the no-op doesn't accidentally mutate the DOM.
Compound selector escape — e.g. #01-wall .child > span in a stylesheet. The regex handles this correctly (verified by lookahead analysis), but a test would pin the behavior against regression.

Unrelated file — `gsapDragCommit.test.ts`

Agree with Rames's nit: the import refactor (commitGsapPositionFromDrag moved from ./gsapDragCommit to ./gsapDragPositionCommit) is mechanically correct but unrelated to the PR's scope. Not a blocker — it's a one-line import fixup that follows a module split from a prior PR — but it muddies the diff.

Verdict

The three core fixes are correct, well-motivated, and adequately tested. The video asset surfacing is a clean additive feature. I'd recommend approval once a teammate with stamp authority confirms. The path.basename swap is the only suggestion I'd push for; everything else is nit-level.

Review by Miga

…view Review follow-ups (#1631), all non-blocking polish: - contentExtractor: use path.basename() instead of localPath.split('/').pop() so video filenames resolve correctly on Windows-style paths too. - subComposition: document that only the leading digit needs CSS escaping (CSS Syntax L3 §4.3.11) on escapeLeadingDigitIdent. - tests: pin three previously-uncovered paths — multiple digit-leading ids in one composition, a digit-leading id inside compound/combinator selectors, and the promoteTemplateCompositionId no-op when the <template> has no id. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

jrusso1020

Round-2 — verified the fixes against the new commit.

refactor(core): address review nits on standalone sub-composition preview (8cf535e40):

Windows-path safety — contentExtractor.ts now uses path.basename() instead of localPath.split('/').pop(). Fixes the case where a manifest path is captured on Windows with backslashes. ✓
Spec reference added — escapeLeadingDigitIdent docstring cites CSS Syntax L3 §4.3.11. Makes the regex-escape intent locatable for future readers. ✓
3 new tests — pinning previously-uncovered paths. From the file count (93/0 test additions) the coverage delta is meaningful. ✓

CI: CodeQL pass, no failing checks.

No new concerns. Stamp posture unchanged — routes through the team.

Review by Jerrai

miguel-heygen

Re-review after 8cf535e.

I checked the follow-up patch: contentExtractor.ts now uses path.basename, escapeLeadingDigitIdent documents why only the leading digit is escaped, and the added tests cover multiple digit-leading IDs, compound/combinator selectors, and the no-template-id no-op path. gh pr checks 1631 is green at current head.

Verdict: COMMENT (code-ready; approval/stamp withheld for trusted-stamper path)
Reasoning: The prior nits are addressed and the current CI/CodeQL state is clean; no blockers from my re-review.

The function was split out into gsapDragPositionCommit.ts in #1605, but the test kept importing it from ./gsapDragCommit, which no longer exports it — yielding 'is not a function' at runtime. Import from the correct module. Inherited main breakage (same fix as #1631/#1635); fixes the Test CI check on this branch independently of merge order. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

miga-heygen

Re-review — final round

Checked the follow-up commit 8cf535e against my prior findings:

path.basename — addressed. contentExtractor.ts now imports basename from node:path and uses it instead of split("/").pop(). Cross-platform path handling is correct.
CSS spec reference — addressed. escapeLeadingDigitIdent docstring now cites CSS Syntax Level 3 §4.3.11 explaining why only the leading digit needs escaping.
Test coverage gaps — addressed. Three new tests added:
- Multiple digit-leading IDs in one composition (#01-wall + #02-music)
- Compound/combinator selectors (#01-wall .child > span, #01-wall:hover)
- No-op path when <template> lacks data-composition-id
Unrelated gsapDragCommit.test.ts import refactor — still present, still fine (mechanically correct, not a blocker).

CI is fully green — all checks pass including regression shards, CodeQL, Windows tests, and perf gates.

No new concerns. LGTM on merit.

Review by Miga

WaterrrForever and others added 3 commits June 22, 2026 15:04

jrusso1020 reviewed Jun 22, 2026

View reviewed changes

miga-heygen reviewed Jun 22, 2026

View reviewed changes

jrusso1020 mentioned this pull request Jun 22, 2026

refactor(skills): move product-launch / pr-to-video / faceless-explainer onto the script-driven architecture #1635

Open

4 tasks

jrusso1020 reviewed Jun 22, 2026

View reviewed changes

miguel-heygen reviewed Jun 22, 2026

View reviewed changes

miga-heygen mentioned this pull request Jun 22, 2026

fix(core): register sub-composition timelines after async build + lint rule #1638

Open

3 tasks

miga-heygen reviewed Jun 22, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(core): render standalone sub-composition previews + surface captured video assets (cli)#1631

fix(core): render standalone sub-composition previews + surface captured video assets (cli)#1631
WaterrrForever wants to merge 4 commits into
mainfrom
fix/subcomp-preview-and-video-assets

WaterrrForever commented Jun 22, 2026

Uh oh!

jrusso1020 left a comment

Uh oh!

miga-heygen left a comment

Uh oh!

jrusso1020 left a comment

Uh oh!

miguel-heygen left a comment

Uh oh!

miga-heygen left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

WaterrrForever commented Jun 22, 2026

What

Why

Standalone sub-composition previews

Captured video asset descriptions

How

packages/core/.../subComposition.ts

packages/cli/.../contentExtractor.ts

Test plan

Uh oh!

jrusso1020 left a comment

Choose a reason for hiding this comment

What I verified

One nit — unrelated test refactor

Stamp posture

Uh oh!

miga-heygen left a comment

Choose a reason for hiding this comment

Independent Review — Miga

CSS escaping — fixDigitLeadingIdSelectors

DOM-based template extraction — extractTemplateInnerHtml

promoteTemplateCompositionId — sibling skip list

Video asset descriptions — generateAssetDescriptions

Test quality

Unrelated file — gsapDragCommit.test.ts

Verdict

Uh oh!

jrusso1020 left a comment

Choose a reason for hiding this comment

Uh oh!

miguel-heygen left a comment

Choose a reason for hiding this comment

Uh oh!

miga-heygen left a comment

Choose a reason for hiding this comment

Re-review — final round

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

`packages/core/.../subComposition.ts`

`packages/cli/.../contentExtractor.ts`

CSS escaping — `fixDigitLeadingIdSelectors`

DOM-based template extraction — `extractTemplateInnerHtml`

`promoteTemplateCompositionId` — sibling skip list

Video asset descriptions — `generateAssetDescriptions`

Unrelated file — `gsapDragCommit.test.ts`