fix(engine,producer): preserve template-wrapped sub-composition media offsets (#476)

## Problem

Template-wrapped sub-compositions could still lose correct parent timing during render in more than one place.

In the validated repros, a host sub-composition starting after the intro (and in one follow-up repro, starting at `20s` after earlier compositions) contained scene-local media inside it. On the broken paths:

- template-wrapped media could be missed during compile and scheduled at raw scene-local time
- already-correct first-pass offsets could be clobbered during `recompileWithResolutions()`
- even after those two fixes, the browser-metadata reconcile step in `executeRenderJob()` could still overwrite a compiled global `end` with a scene-local `data-end` from the inlined DOM, clipping the tail off late-start sub-composition media

## What this fixes

### Template-wrapped media discovery

- `parseVideoElements`, `parseImageElements`, and `parseAudioElements` now unwrap a single top-level `<template>` wrapper before scraping media
- the unwrap helper is DOM-based, not regex-based, so it avoids the CodeQL backtracking warning and only unwraps the exact single-wrapper shape we want
- multiple sibling templates or other top-level content are left untouched instead of being rewritten heuristically

### Offset preservation after duration resolution

- `recompileWithResolutions()` now preserves the first-pass sub-composition media arrays when the already-inlined HTML no longer contains `[data-composition-src]` hosts
- that prevents correctly offset media metadata from being overwritten by scene-local media parsed from the merged DOM

### Browser metadata reconciliation in the compiled time origin

- browser-discovered media can still report scene-local `data-start` / `data-end` from the merged DOM after inlining
- the producer now reprojects browser `end` values into the compiled element's time origin before reconciling them back into `composition.videos` / `composition.audios`
- this prevents late-start sub-composition media from getting truncated back to a scene-local end during the probe phase

### Regression coverage

- adds focused engine tests for the template unwrap helper
- adds producer regression coverage for both the initial compile path and the post-inline `recompileWithResolutions()` path
- adds producer regression coverage for late-start host compositions (`t≈20`) with scene-local media inside them
- adds producer unit coverage for the browser-end reprojection helper used by the reconcile path

## Root cause

There were three distinct renderer failures behind the bug:

### 1. Template contents were invisible to the media scrapers

`parseSubCompositions()` reads raw sub-composition HTML and applies the host offset to discovered media. But the engine media helpers were querying the parsed document directly, and linkedom follows browser semantics here: top-level `<template>` contents live in a `DocumentFragment`, so `querySelectorAll()` never saw those `<video>` / `<audio>` / `<img>` nodes.

That meant template-wrapped sub-compositions could silently produce zero discovered media during the first pass.

### 2. The duration-resolution recompile could clobber already-correct offsets

After the browser resolves composition durations, `recompileWithResolutions()` reparses the already-inlined HTML. By that point the original `[data-composition-src]` hosts are gone, so `parseSubCompositions()` legitimately returns no nested media.

The old code still rebuilt the deduped media arrays from the merged DOM, which let scene-local media parsed from the inlined HTML overwrite the correctly offset first-pass metadata.

### 3. The browser probe reconcile path mixed two timing coordinate systems

`discoverMediaFromBrowser()` reads `data-start` / `data-end` directly from the live DOM after sub-compositions are already inlined. For nested media, those attributes can still be scene-local even though the compiled metadata has already been offset into the parent host timeline.

The old reconcile path compared those values directly and overwrote `existing.end` whenever the numbers differed. For a late-start sub-composition, that could replace a correct global end like `25.5` with a scene-local end like `5.5`, cutting the clip off during render.

## Verification

### Local checks

- `bun test packages/engine/src/utils/htmlTemplate.test.ts`
- `bun test packages/producer/src/services/htmlCompiler.test.ts`
- `bunx vitest run packages/producer/src/services/renderOrchestrator.test.ts`
- `bun run --filter @hyperframes/engine test`
- `bun run --filter @hyperframes/engine typecheck`
- `bun run --filter @hyperframes/producer typecheck`
- `bunx oxlint packages/engine/src/utils/htmlTemplate.ts packages/engine/src/utils/htmlTemplate.test.ts packages/producer/src/services/renderOrchestrator.ts packages/producer/src/services/renderOrchestrator.test.ts packages/producer/src/services/htmlCompiler.test.ts`
- `bunx oxfmt --check packages/engine/src/utils/htmlTemplate.ts packages/engine/src/utils/htmlTemplate.test.ts`
- `bun run build:producer`

### Render / browser verification

Verified against two local repros:

1. **Early offset repro**
   - host starts at `2s`
   - child media is scene-local `0-4s`
   - compiled render summary keeps the child video/audio at `start: 2`
   - browser verification via `agent-browser` confirmed the `2.2s` frame still shows the child clip active in the host timeline

2. **Late offset repro**
   - earlier compositions run first, then the target host starts at `20s`
   - child media starts scene-local at `1.5s` and should remain visible through `24.5s`
   - compiled render summary keeps the child video/audio at `start: 21.5`, `end: 25.5`
   - browser verification via `agent-browser` confirmed the `24.5s` frame still shows the late clip visible, which is the exact tail-clipping case the old reconcile path could break

## Notes

- the `/tmp/hf-pr475-repro` and `/tmp/hf-pr476-late-offset-repro` projects plus their browser-proof artifacts are verification-only and are not part of this PR
- this PR stays narrowly scoped to sub-composition media timing across compile, recompile, and browser probe reconciliation; it does not broaden into general sub-composition HTML normalization beyond the single-wrapper case
This commit is contained in:
Miguel Ángel
2026-04-24 19:00:42 +02:00
committed by GitHub
parent fbec7bb1c4
commit 267ffd3fca
12 changed files with 328 additions and 17 deletions
+7 -1
View File
@@ -24,10 +24,16 @@ jobs:
outputs:
code: ${{ steps.filter.outputs.code }}
steps:
# Force git-based change detection instead of the pull_request REST API.
# The API path can fail the whole workflow on transient listFiles
# timeouts before any real CI work starts.
- uses: actions/checkout@v4
- uses: dorny/paths-filter@v3
with:
fetch-depth: 0
- uses: dorny/paths-filter@v4
id: filter
with:
token: ""
filters: |
code:
- "packages/**"
+6 -1
View File
@@ -17,10 +17,15 @@ jobs:
outputs:
perf: ${{ steps.filter.outputs.perf }}
steps:
# Force git-based change detection instead of the pull_request REST API.
# The API path can fail the perf workflow on transient listFiles timeouts.
- uses: actions/checkout@v4
- uses: dorny/paths-filter@v3
with:
fetch-depth: 0
- uses: dorny/paths-filter@v4
id: filter
with:
token: ""
filters: |
perf:
- "packages/player/**"
+7 -1
View File
@@ -14,10 +14,16 @@ jobs:
outputs:
code: ${{ steps.filter.outputs.code }}
steps:
# Force git-based change detection instead of the pull_request REST API.
# The API path can fail the whole workflow on transient listFiles
# timeouts before any regression shard even starts.
- uses: actions/checkout@v4
- uses: dorny/paths-filter@v3
with:
fetch-depth: 0
- uses: dorny/paths-filter@v4
id: filter
with:
token: ""
filters: |
code:
- "packages/core/**"
+7 -1
View File
@@ -36,10 +36,16 @@ jobs:
outputs:
code: ${{ steps.filter.outputs.code }}
steps:
# Force git-based change detection instead of the pull_request REST API.
# The API path can fail the workflow on transient listFiles timeouts
# before the Windows render jobs even start.
- uses: actions/checkout@v4
- uses: dorny/paths-filter@v3
with:
fetch-depth: 0
- uses: dorny/paths-filter@v4
id: filter
with:
token: ""
filters: |
code:
- "packages/**"
+2 -1
View File
@@ -11,6 +11,7 @@ import { extractAudioMetadata } from "../utils/ffprobe.js";
import { downloadToTemp, isHttpUrl } from "../utils/urlDownloader.js";
import { DEFAULT_CONFIG, type EngineConfig } from "../config.js";
import { runFfmpeg } from "../utils/runFfmpeg.js";
import { unwrapTemplate } from "../utils/htmlTemplate.js";
import type { AudioElement, AudioTrack, MixResult } from "./audioMixer.types.js";
export type { AudioElement, AudioTrack, MixResult } from "./audioMixer.types.js";
@@ -24,7 +25,7 @@ interface ExtractResult {
export function parseAudioElements(html: string): AudioElement[] {
const elements: AudioElement[] = [];
const { document } = parseHTML(html);
const { document } = parseHTML(unwrapTemplate(html));
// Parse <audio> elements
const audioEls = document.querySelectorAll("audio[id][src]");
@@ -18,6 +18,7 @@ import {
import { downloadToTemp, isHttpUrl } from "../utils/urlDownloader.js";
import { runFfmpeg } from "../utils/runFfmpeg.js";
import { DEFAULT_CONFIG, type EngineConfig } from "../config.js";
import { unwrapTemplate } from "../utils/htmlTemplate.js";
import {
FRAME_FILENAME_PREFIX,
ensureCacheEntryDir,
@@ -102,7 +103,7 @@ export interface ExtractionResult {
export function parseVideoElements(html: string): VideoElement[] {
const videos: VideoElement[] = [];
const { document } = parseHTML(html);
const { document } = parseHTML(unwrapTemplate(html));
const videoEls = document.querySelectorAll("video[src]");
let autoIdCounter = 0;
@@ -156,7 +157,7 @@ export interface ImageElement {
export function parseImageElements(html: string): ImageElement[] {
const images: ImageElement[] = [];
const { document } = parseHTML(html);
const { document } = parseHTML(unwrapTemplate(html));
const imgEls = document.querySelectorAll("img[src]");
let autoIdCounter = 0;
@@ -0,0 +1,42 @@
import { describe, expect, it } from "vitest";
import { unwrapTemplate } from "./htmlTemplate.js";
describe("unwrapTemplate", () => {
it("returns the input unchanged when there is no template wrapper", () => {
const html = `<div>hello</div>`;
expect(unwrapTemplate(html)).toBe(html);
});
it("unwraps a bare top-level template fragment", () => {
const inner = `<span>hi</span>`;
const html = `<template id="t" data-x="1">${inner}</template>`;
expect(unwrapTemplate(html)).toBe(inner);
});
it("unwraps a full document whose body only contains a template", () => {
const inner = `<div id="root"><audio id="a" src="a.mp3"></audio></div>`;
const html = `<!doctype html><html><body><template>${inner}</template></body></html>`;
expect(unwrapTemplate(html)).toBe(inner);
});
it("returns the input unchanged when the closing template tag is missing", () => {
const html = `<template><div>broken`;
expect(unwrapTemplate(html)).toBe(html);
});
it("returns an empty string for an empty template", () => {
const html = `<body><template></template></body>`;
expect(unwrapTemplate(html)).toBe("");
});
it("preserves nested templates inside the outer wrapper", () => {
const inner = `outer-before<template>inner-content</template>outer-after`;
const html = `<template>${inner}</template>`;
expect(unwrapTemplate(html)).toBe(inner);
});
it("leaves multiple sibling templates unchanged", () => {
const html = `<template>a</template>middle<template>b</template>`;
expect(unwrapTemplate(html)).toBe(html);
});
});
+50
View File
@@ -0,0 +1,50 @@
import { parseHTML } from "linkedom";
function parseHTMLContent(html: string): Document {
const trimmed = html.trimStart().toLowerCase();
if (trimmed.startsWith("<!doctype") || trimmed.startsWith("<html")) {
return parseHTML(html).document;
}
return parseHTML(`<!DOCTYPE html><html><head></head><body>${html}</body></html>`).document;
}
function getSingleMeaningfulChild(container: Element): Element | null {
let child: Element | null = null;
for (const node of Array.from(container.childNodes)) {
if (node.nodeType === 3 && !(node.textContent || "").trim()) continue;
if (node.nodeType === 8) continue;
if (node.nodeType !== 1) return null;
if (child) return null;
child = node as Element;
}
return child;
}
/**
* Sub-compositions commonly use a single top-level <template> wrapper. Parse
* the HTML and unwrap only that exact shape, rather than pattern-matching the
* raw string. This avoids both regex backtracking risk and accidental rewrites
* of inputs that contain multiple sibling templates or other top-level content.
*/
export function unwrapTemplate(html: string): string {
const lowered = html.toLowerCase();
if (!lowered.includes("<template") || !lowered.includes("</template>")) {
return html;
}
const { body } = parseHTMLContent(html);
if (!body) return html;
let container: Element = body;
const bodyWrapper = getSingleMeaningfulChild(container);
if (bodyWrapper?.tagName === "BODY") {
container = bodyWrapper;
}
const template = getSingleMeaningfulChild(container);
if (template?.tagName !== "TEMPLATE") {
return html;
}
return template.innerHTML ?? html;
}
@@ -7,6 +7,7 @@ import {
compileForRender,
detectRenderModeHints,
inlineExternalScripts,
recompileWithResolutions,
} from "./htmlCompiler.js";
// ── collectExternalAssets ──────────────────────────────────────────────────
@@ -456,3 +457,141 @@ describe("detectRenderModeHints", () => {
}
});
});
describe("template-wrapped sub-composition media offsets", () => {
function writeTemplateWrappedProject(
hostAttrs: string,
mediaAttrs: string = 'data-start="0" data-duration="4"',
): {
projectDir: string;
indexPath: string;
} {
const projectDir = mkdtempSync(join(tmpdir(), "hf-template-offset-"));
const compositionsDir = join(projectDir, "compositions");
mkdirSync(compositionsDir, { recursive: true });
writeFileSync(
join(projectDir, "index.html"),
`<!DOCTYPE html>
<html>
<body>
<div
id="root"
data-composition-id="root"
data-start="0"
data-width="640"
data-height="360"
data-duration="4"
>
<div
id="scene-host"
data-composition-id="scene"
data-composition-src="compositions/scene.html"
${hostAttrs}
></div>
</div>
<script>
window.__timelines = window.__timelines || {};
window.__timelines["root"] = { duration: () => 4 };
</script>
</body>
</html>`,
);
writeFileSync(
join(compositionsDir, "scene.html"),
`<template id="scene-template">
<div
data-composition-id="scene"
data-start="0"
data-width="640"
data-height="360"
data-duration="4"
>
<video
id="scene-video"
src="../assets/clip.mp4"
${mediaAttrs}
data-track-index="0"
></video>
<script>
window.__timelines = window.__timelines || {};
window.__timelines["scene"] = { duration: () => 4 };
</script>
</div>
</template>`,
);
return { projectDir, indexPath: join(projectDir, "index.html") };
}
it("offsets template-wrapped media to the host start during compile", async () => {
const { projectDir, indexPath } = writeTemplateWrappedProject(
'data-start="2" data-duration="2" data-width="640" data-height="360"',
);
const compiled = await compileForRender(projectDir, indexPath, projectDir);
expect(compiled.videos).toHaveLength(1);
expect(compiled.videos[0]).toMatchObject({
id: "scene-video",
start: 2,
end: 6,
});
expect(compiled.audios).toHaveLength(1);
expect(compiled.audios[0]).toMatchObject({
id: "scene-video-audio",
start: 2,
end: 6,
});
});
it("preserves first-pass media offsets when durations are resolved after inlining", async () => {
const { projectDir, indexPath } = writeTemplateWrappedProject(
'data-start="2" data-width="640" data-height="360"',
);
const compiled = await compileForRender(projectDir, indexPath, projectDir);
expect(compiled.videos[0]?.start).toBe(2);
const recompiled = await recompileWithResolutions(
compiled,
[{ id: "scene-host", duration: 2 }],
projectDir,
projectDir,
);
expect(recompiled.videos).toHaveLength(1);
expect(recompiled.videos[0]).toMatchObject({
id: "scene-video",
start: 2,
end: 6,
});
expect(recompiled.audios).toHaveLength(1);
expect(recompiled.audios[0]).toMatchObject({
id: "scene-video-audio",
start: 2,
end: 6,
});
});
it("offsets scene-local media in compositions that start much later on the timeline", async () => {
const { projectDir, indexPath } = writeTemplateWrappedProject(
'data-start="20" data-duration="6" data-width="640" data-height="360"',
'data-start="1.5" data-duration="4"',
);
const compiled = await compileForRender(projectDir, indexPath, projectDir);
expect(compiled.videos).toHaveLength(1);
expect(compiled.videos[0]).toMatchObject({
id: "scene-video",
start: 21.5,
end: 25.5,
});
expect(compiled.audios).toHaveLength(1);
expect(compiled.audios[0]).toMatchObject({
id: "scene-video-audio",
start: 21.5,
end: 25.5,
});
});
});
@@ -1233,9 +1233,10 @@ export async function recompileWithResolutions(
const mainImages = parseImageElements(html);
// Keep inlined sub-composition media authoritative on ID collisions.
const videos = dedupeElementsById([...mainVideos, ...subVideos]);
const audios = dedupeElementsById([...mainAudios, ...subAudios]);
const images = dedupeElementsById([...mainImages, ...subImages]);
const hasSubMedia = subVideos.length > 0 || subAudios.length > 0 || subImages.length > 0;
const videos = hasSubMedia ? dedupeElementsById([...mainVideos, ...subVideos]) : compiled.videos;
const audios = hasSubMedia ? dedupeElementsById([...mainAudios, ...subAudios]) : compiled.audios;
const images = hasSubMedia ? dedupeElementsById([...mainImages, ...subImages]) : compiled.images;
const remaining = compiled.unresolvedCompositions.filter(
(c) => !resolutions.some((r) => r.id === c.id),
@@ -8,6 +8,7 @@ import type { CompiledComposition } from "./htmlCompiler.js";
import {
applyRenderModeHints,
extractStandaloneEntryFromIndex,
projectBrowserEndToCompositionTimeline,
writeCompiledArtifacts,
} from "./renderOrchestrator.js";
import { toExternalAssetKey } from "../utils/paths.js";
@@ -244,3 +245,17 @@ describe("applyRenderModeHints", () => {
expect(log.warn).not.toHaveBeenCalled();
});
});
describe("projectBrowserEndToCompositionTimeline", () => {
it("keeps end unchanged when browser and compiled starts share the same origin", () => {
expect(projectBrowserEndToCompositionTimeline(2, 2, 6)).toBe(6);
});
it("reprojects a scene-local browser end into the compiled host timeline", () => {
expect(projectBrowserEndToCompositionTimeline(4.417, 0, 85.52)).toBeCloseTo(89.937, 6);
});
it("preserves scene-local media offsets inside compositions that start much later", () => {
expect(projectBrowserEndToCompositionTimeline(21.5, 1.5, 5.5)).toBe(25.5);
});
});
@@ -321,6 +321,24 @@ export interface CompositionMetadata {
height: number;
}
const BROWSER_MEDIA_EPSILON = 0.0001;
/**
* Browser-discovered media inside inlined sub-compositions can still report
* scene-local timing from the merged DOM (e.g. start=0, end=85.52) while the
* compiled metadata is already offset into the parent host timeline
* (e.g. start=4.417, end=89.937). Reproject browser end-time into the
* compiled element's time origin before reconciling it back into the render
* metadata.
*/
export function projectBrowserEndToCompositionTimeline(
existingStart: number,
browserStart: number,
browserEnd: number,
): number {
return browserEnd + (existingStart - browserStart);
}
function updateJobStatus(
job: RenderJob,
status: RenderStatus,
@@ -1191,13 +1209,22 @@ export async function executeRenderJob(
if (existing.src !== src) {
existing.src = src;
}
if (el.end > 0 && (existing.end <= 0 || Math.abs(existing.end - el.end) > 0.0001)) {
existing.end = el.end;
const projectedEnd = projectBrowserEndToCompositionTimeline(
existing.start,
el.start,
el.end,
);
if (
projectedEnd > 0 &&
(existing.end <= 0 ||
Math.abs(existing.end - projectedEnd) > BROWSER_MEDIA_EPSILON)
) {
existing.end = projectedEnd;
}
if (
el.mediaStart > 0 &&
(existing.mediaStart <= 0 ||
Math.abs(existing.mediaStart - el.mediaStart) > 0.0001)
Math.abs(existing.mediaStart - el.mediaStart) > BROWSER_MEDIA_EPSILON)
) {
existing.mediaStart = el.mediaStart;
}
@@ -1224,17 +1251,29 @@ export async function executeRenderJob(
if (existing.src !== src) {
existing.src = src;
}
if (el.end > 0 && (existing.end <= 0 || Math.abs(existing.end - el.end) > 0.0001)) {
existing.end = el.end;
const projectedEnd = projectBrowserEndToCompositionTimeline(
existing.start,
el.start,
el.end,
);
if (
projectedEnd > 0 &&
(existing.end <= 0 ||
Math.abs(existing.end - projectedEnd) > BROWSER_MEDIA_EPSILON)
) {
existing.end = projectedEnd;
}
if (
el.mediaStart > 0 &&
(existing.mediaStart <= 0 ||
Math.abs(existing.mediaStart - el.mediaStart) > 0.0001)
Math.abs(existing.mediaStart - el.mediaStart) > BROWSER_MEDIA_EPSILON)
) {
existing.mediaStart = el.mediaStart;
}
if (el.volume > 0 && Math.abs((existing.volume ?? 1) - el.volume) > 0.0001) {
if (
el.volume > 0 &&
Math.abs((existing.volume ?? 1) - el.volume) > BROWSER_MEDIA_EPSILON
) {
existing.volume = el.volume;
}
}