From 110ea12597053f867a48a9e85355eb269fc7f698 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Wed, 1 Apr 2026 02:59:22 +0200 Subject: [PATCH] fix(core,engine,producer): handle id-less media in sub-composition renders (#96) ## What Move the id-less media fix into the shared timing compiler so producer can resolve durations for sub-composition videos before inlining, then carry the merged result through engine parsing, regression coverage, and the regression Docker image used in CI. This PR now does five concrete things: - assigns stable ids to id-less media in core `compileTimingAttrs()` so unresolved duration injection can target them - keeps the engine-side `parseVideoElements()` support for `video[src]` plus the newer `data-duration` / natural-duration fallback from `main` - makes producer prefer sub-composition media metadata over the later inlined-document parse when the same media id appears in both places - makes `sub-composition-video` a runnable regression test by fixing its metadata and checking in the missing `output/compiled.html` snapshot - removes the stale `pnpm-workspace.yaml` copy step from `Dockerfile.test`, so regression CI builds the Bun-based test image from the current workspace layout ## Why - media without an explicit `id` could not participate in unresolved-duration resolution early enough - producer could lose the resolved sub-composition timing by overwriting it with the later inlined parse - the regression fixture intended to cover this case was not actually running in CI because its `meta.json` was incomplete and the required compiled snapshot was missing - the regression image definition still expected a deleted `pnpm-workspace.yaml`, so GitHub Actions failed before the test shard could start Putting the id-generation step in core makes the behavior reusable instead of relying on producer-only HTML patching. ## How ### Shared compiler - core `compileTimingAttrs()` now auto-assigns stable ids to id-less `video` / `audio` tags - those generated ids are returned in `unresolved`, so `injectDurations()` can add `data-duration` and `data-end` to the same media element later in the pipeline - added core tests that cover auto-id assignment and duration injection for generated ids ### Producer - when producer combines `subVideos` / `subAudios` with the media re-parsed from the final inlined HTML, it now lets the sub-composition metadata win - this preserves the resolved/clamped timing already computed for nested media instead of overwriting it with the later parse - `sub-composition-video` now has valid regression metadata and a checked-in `output/compiled.html` snapshot so CI actually executes it ### Engine - resolved the merge conflict in `videoFrameExtractor` by keeping the broader `video[src]` parsing from this branch and the `data-duration` / natural-duration fallback that landed on `main` - added a focused engine unit test for videos without ids ### CI image - `Dockerfile.test` now copies only `package.json` and `bun.lock` at the workspace root before `bun install --frozen-lockfile` - this matches the current monorepo layout and removes the obsolete pnpm-era dependency on `pnpm-workspace.yaml` ## Test plan - [x] `bun run --filter @hyperframes/core test` - [x] `bun run --filter @hyperframes/engine test` - [x] `bun run --filter @hyperframes/producer test --update --sequential sub-composition-video` - [x] `bun run --filter @hyperframes/producer test --sequential sub-composition-video` - [x] Browser check with `agent-browser` against the compiled fixture page (`http://127.0.0.1:8123/compiled.html`) - [x] Clean tracked-only Docker build of `Dockerfile.test` with the PR version of the file applied ## Notes - Latest regression workflow is green on `main`, but before this PR the `sub-composition-video` fixture was being skipped by the harness rather than exercised end to end. - The CI Docker fix was validated from a tracked-only export to avoid local untracked worktree artifacts affecting the result. --- Dockerfile.test | 2 +- .../core/src/compiler/timingCompiler.test.ts | 21 ++ packages/core/src/compiler/timingCompiler.ts | 14 +- .../src/services/videoFrameExtractor.test.ts | 34 +++ .../src/services/videoFrameExtractor.ts | 22 +- .../producer/src/services/htmlCompiler.ts | 32 +- .../tests/sub-composition-video/meta.json | 2 + .../output/compiled.html | 283 ++++++++++++++++++ .../sub-composition-video/output/output.mp4 | 4 +- 9 files changed, 368 insertions(+), 46 deletions(-) create mode 100644 packages/engine/src/services/videoFrameExtractor.test.ts create mode 100644 packages/producer/tests/sub-composition-video/output/compiled.html diff --git a/Dockerfile.test b/Dockerfile.test index 87b952d50..6f040dd7c 100644 --- a/Dockerfile.test +++ b/Dockerfile.test @@ -65,7 +65,7 @@ RUN curl -fsSL https://bun.sh/install | bash ENV PATH="/root/.bun/bin:$PATH" # Install dependencies (full, including devDependencies for tsx + test harness) -COPY package.json bun.lock pnpm-workspace.yaml ./ +COPY package.json bun.lock ./ COPY packages/core/package.json packages/core/package.json COPY packages/engine/package.json packages/engine/package.json COPY packages/producer/package.json packages/producer/package.json diff --git a/packages/core/src/compiler/timingCompiler.test.ts b/packages/core/src/compiler/timingCompiler.test.ts index ae2a29239..687b8278d 100644 --- a/packages/core/src/compiler/timingCompiler.test.ts +++ b/packages/core/src/compiler/timingCompiler.test.ts @@ -35,6 +35,18 @@ describe("compileTimingAttrs", () => { expect(unresolved[0].start).toBe(1); }); + it("auto-assigns ids to id-less videos so unresolved duration resolution can target them", () => { + const html = '