Repository navigation
fix(core): detect template entries structurally - #7249
miguel-heygen merged 12 commits into
Conversation
isTemplateEntry only matched a raw file that starts with <template>, so the full-document sub-composition shape (<!doctype html><html><body><template> <div data-composition-id>...</template></body></html>) was not recognised and resolveEntryHtmlPath compiled the raw file, whose root sits in an inert template: no timeline, no duration, "Composition has zero duration". A template entry is now a document whose <template> holds the composition root with no live composition root outside it. Detection and extraction share one lookup (locateSceneRoot), the same one readSceneRoot uses. — Jerrai
The comment ratchet allows no net rise in a file's comment share; the PR description carries the explanation. — Jerrai
Edit accuracy: accurate 267 (base branch 267), smooth 115 of thoseThe gate passes. Quarantined, measured but not gated (8)
Unstable (1)
|
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 0 important · 1 nit
I ran the predicate against 10 shapes. linkedom keeps <template> content out of document.querySelector, so hasLiveRoot really does mean a live root. Fragments, the canonical full document, a live root next to a template, and a commented-out root all classify correctly. Detection and extraction now share one lookup. The nit is inline.
Scope
Read the full diff and both callers (compileStage.ts resolveEntryHtmlPath, htmlBundler.ts readMountedSceneShell). The lint startsWith("<template") checks are not affected because the cli, project and studio-server lint callers pass isSubComposition for mounted files. CI: the latest CI run (42050596010, attempt 2) is all green, and attempt 1's edit-accuracy shard failure was the known flake. The earlier CI run on this head was cancelled. Every other workflow is green, and the PR is blocked only on review.
miguel-heygen
left a comment
There was a problem hiding this comment.
Requesting changes: this fixes the full-document case but not the root, and the gap is in the function this PR rewrites.
locateSceneRoot takes document.querySelector("template"), the first <template> in the file. A scene file whose scene template comes after any other template (a reusable card, an icon sprite, a second scene kept for later) is still read as "not a template entry" and fails with the same "Composition has zero duration". The PR body names this limit, which means the bug class stays open behind a fix for one shape of it.
Root fix, same size:
- Look at every
<template>and pick the one whose content holds the[data-composition-id]root. - If more than one template holds a root, refuse with a message naming them, not a silent pick.
- Add tests for a scene template after an unrelated template, and for two templates that both hold a root.
Everything else looks right: detection by structure instead of the first bytes, and the live-root guard.
locateSceneRoot read only the first <template>, so a scene template that follows another template was treated as not a template entry. Scan every template and take the first whose content holds a composition root.
The inspector bundles core; the multi-template scene lookup added a few hundred bytes and put it 10 bytes over the 1% tolerance.
|
@miguel-heygen addressed at 4dc8867: more than one scene-template root is now refused with a message naming each template (position, id, composition), and a live root outside templates stays a plain entry. Inspector unpacked budget 3645432 → 3650000 (reason in body; veto welcome). CI green. — Jerrai |
…view to heygen-com#7250 Adopts heygen-com#7310 (MPEG-4 Part 2 / MJPEG get browser-playable copies), heygen-com#7316 (check_report counts the copies a check waited for) and heygen-com#7317 (render reads a composition src like the preview and refuses paths outside the project). Defers heygen-com#7309 (needs heygen-com#6912); heygen-com#7314 does not apply. PR axis advances to heygen-com#7250 (heygen-com#7243 closed unmerged, heygen-com#7249 open and pinned as monitored; monitored 96->97). Co-authored-by: Miguel Ángel <miguel.sierra@heygen.com> Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 0 important · 0 nits — 2 resolved, 0 open, 0 new · b25bc35 → 4dc8867
Both resolved items are the first-<template> gap: my nit and @miguel-heygen's change request. Every template is now scanned, and more than one root is refused with each template named. Merge is blocked by a conflict in scripts/package-size-budgets.json. main already has inspector unpacked at 3684092, so take main's value and drop this PR's bump.
Checked
standaloneEntry.test.ts passes locally (88 tests across 2 files). I also ran the new predicate and the main predicate over all 1084 tracked .html files. None throws. The 36 that change all go false→true, which is the intended full-document shape. No tracked file has more than one <template> holding a root. Every CI workflow on this head is green.
…entry_structural_detection-11052026 # Conflicts: # scripts/package-size-budgets.json
…tead of failing A file with several scene templates and no live root rendered its first one before; locateSceneRoot threw and stopped such projects rendering. It returns the first again, and the multiple_scene_templates lint warning names each. Co-authored-by: Jerrai <jerrai@heygen.com>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
…t counts Co-authored-by: Jerrai <jerrai@heygen.com>
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 0 important · 0 nits · 4dc8867 → 1f007a2
The scene-after-another-template shape is detected but still does not render (↓ inline). I missed this when I approved 4dc8867, because that head mounts the template the same way. CI is green. Rendering the first scene template and the multiple_scene_templates lint rule look right. @miguel-heygen's change request asked for a refusal, so clearing it is his call.
…er and the loader A scene template after a card template was detected by locateSceneRoot but mounted from the first template of any kind, so it rendered blank. findSceneTemplate picks the first template holding a composition root and locateSceneRoot, resolveSubCompositionContent and the loader all use it. Co-authored-by: Jerrai <jerrai@heygen.com>
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 1 important · 0 nits — 1 resolved, 0 open, 1 new · 1f007a2 → 4c89caa
important (not blocking, existed before this PR): two places still take the first <template>. Studio's standalone sub-composition preview (studio-server/src/helpers/subComposition.ts:164 extractTemplateInnerHtml, used by routes/preview.ts:679) previews the card instead of the scene. parsers/src/htmlParser.ts:808 reads variable declarations from the card. Moving both onto findSceneTemplate in a follow-up would make "one shared pick" true.
Checked
The card-before-scene repro now bundles SCENE without CARD both as the entry file and through index.html. Core standaloneEntry, htmlBundler and compositionLoader pass locally (322 tests), as do parsers (1403) and lint composition (319). CI is green. The non-required "Heavy films: Safari vs Chrome" job failed on the heavy-alpha-webm fixture, which also fails on test/player-safari-bench-errors at 03:23Z. @miguel-heygen's change request is still on the PR, and only he can clear it.
miguel-heygen
left a comment
There was a problem hiding this comment.
My change request is answered: the scene template is found even when an earlier template holds no composition root (one shared helper), with tests for that exact case.
|
@miguel-heygen the Safari bench ("Heavy films: Safari vs Chrome") was re-run on this head and passed (attempt 2 of run 42335309927). #7249 is now green at 4c89caa with your approval and Genesis's, and GitHub reports it CLEAN. The merge is yours. — Jerrai |
One conflict, in packages/core/src/runtime/compositionLoader.ts: both import lines are needed (findSceneTemplate from this branch, wallClearTimeout and wallSetTimeout from main). — Jerrai
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 0 important · 0 nits — 0 resolved, 0 open, 0 new · 4c89caa → 5512448
Merge-only head: the PR patch against main is identical to the approved one except for the kept wallClock import in compositionLoader.ts. Main added no new template-lookup call sites, and all Actions runs are green.
|
@miguel-heygen #7249 is green and ready to merge at 5512448; the merge is yours. The only change since your approval at 4c89caa is a merge of main: main had moved 45 commits and left one import-line conflict in — Jerrai |
…7249) isTemplateEntry only matched a raw file that starts with <template>, so the full-document sub-composition shape (doctype, html, body, template holding the composition root) compiled the raw file whose root sits in an inert template: no timeline, "Composition has zero duration". Detection now uses locateSceneRoot, and the compiler, the sub-composition inliner and the runtime loader share one findSceneTemplate pick (first template holding a composition root). A file with several scene templates and no live root renders the first and gets the multiple_scene_templates lint warning. Ported from heygen-com/hyperframes caa62fe; the patch applied cleanly. Tests were applied first and failed (11 core, 1 lint, 1 producer), then passed with the source change. Co-authored-by: Jerrai <jerrai@heygen.com> Co-Authored-By: Claude Sonnet <noreply@anthropic.com>
…n-com#7110, heygen-com#7000; advance watermark Eight commits, PR heygen-com#7452 and ten pin moves are recorded with their evidence in the continued ledger, DECISIONS and UPSTREAM. Adopted: heygen-com#7249 (template entries detected structurally), heygen-com#7110 (whisper nan/inf output), heygen-com#7000 (engine probe test on fake timers). Deferred: heygen-com#7444, heygen-com#7452. Not applicable: heygen-com#7257, heygen-com#7443, heygen-com#7450. The watermark moves to c149730, PR heygen-com#7452, and the nine moved pins plus heygen-com#7434 are re-pinned at their current heads. Co-Authored-By: Claude Sonnet <noreply@anthropic.com>
What
isTemplateEntrynow recognises a template entry by structure instead of by the first bytes of the file. A scene file is a template entry when it has a<template>whose content holds the[data-composition-id]root and there is no live (non-template) composition root outside the template.Why
A sub-composition file is just as often written as a complete HTML document with the
<template>inside<body>(a valid page a browser or an editor can open on its own), not only as a fragment that starts with<template>:The predicate only accepted files that start with
<template(after comments and whitespace), so this shape was not recognised.resolveEntryHtmlPath(the standalone-entry extraction from #7217, used by the in-process renderer) then compiled the raw file. Everything sits in an inert template, so the page has no root, no timeline and nodata-duration, and rendering that scene on its own fails withComposition has zero duration.htmlBundlercalls the same predicate and had the same blind spot.Related work
Follows #7217.
How
locateSceneRoot(new,packages/core/src/compiler/htmlDocument.ts) parses once and reports the composition root, whether a<template>exists, and whether a live root exists outside it.readSceneRoot(used for extraction) now reads its root from this same lookup, so detection and extraction agree.isTemplateEntry= template present, root inside it, no live root outside it. The regex prefix check is gone; leading comments and whitespace no longer matter because the HTML parser handles them.locateSceneRootscans every<template>and takes the one whose content holds a[data-composition-id]root. If more than one template holds a root and the file has no live root, it throws:This file has more than one scene template, so it is unclear which one to render: template #1 (id="intro") holding composition "scene"; template #2 holding composition "later". Keep one composition root per file.Callers (isTemplateEntry,extractStandaloneEntryFromIndex, soresolveEntryHtmlPathandbundleProject) do not catch it, so the message surfaces instead of "Composition has zero duration". A file with a live root stays a plain entry and is never refused.extractStandaloneEntryFromIndexoutput is unchanged.<template>holding no composition root at all (for example<template></template>) is no longer a template entry, since there is nothing to extract. Such an entry now compiles as a plain file instead of being wrapped. The existing fragment test fixtures that used empty templates now carry a root.Callers audited (
git grep isTemplateEntry):packages/producer/src/services/render/stages/compileStage.tsresolveEntryHtmlPath: now extracts the canonical full-document shape; fragments and live-root documents behave as before.packages/core/src/compiler/htmlBundler.tsreadMountedSceneShell: same predicate, same result change as above (canonical shape now bundled inside the mounting index).packages/core/src/compiler/standaloneEntry.tsandpackages/core/src/compiler/index.ts: re-exports of the same function, signature unchanged ((html: string) => boolean).Tests:
standaloneEntry.test.ts.Size budget:
@hyperframes/inspectorbundles core, and the scene lookup change grew its unpacked size just past the 1% tolerance (3681896 bytes against 3645432 at the first measurement, before the refusal code). Budget goes from 3645432 to 3650000, about 4.5 KB: what the lookup and refusal code need plus a small margin. Miguel can veto.Test plan
Added to
standaloneEntry.test.ts(predicate) andcompileStage.test.ts(resolveEntryHtmlPath), using fixtures written for this PR (3 s scene, 640x360, solid background, one text element, GSAP timeline):resolveEntryHtmlPathextracts it from anindex.htmlthat mounts it throughdata-composition-src; the wrapper carries the scene's owndata-duration="3", not the master'sroot: null; single template and template-free document unchangedRun locally (changed files only):
cd packages/core && npx vitest run src/compiler/standaloneEntry.test.ts src/compiler/htmlBundler.test.ts: 2 files, 237 tests passedcd packages/producer && npx vitest run src/services/render/stages/compileStage.test.ts: 1 file, 23 tests passednpx oxfmt --checkandnpx oxlinton the 4 changed files: clean;tsc --noEmitinpackages/core: clean; pre-commit hooks (lefthook: lint, format, fallow, typecheck, commitlint) passedNot exercised locally: a real browser render of a canonical-shape scene; the full repo suite and the comment ratchet scripts (they need a newer git than the local one; left to CI).
— Jerrai