Files
hyperframes/.github/workflows/player-perf.yml
T
Miguel Ángel 267ffd3fca 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
2026-04-24 19:00:42 +02:00

150 lines
4.9 KiB
YAML

name: Player perf
on:
pull_request:
push:
branches: [main]
concurrency:
group: player-perf-${{ github.ref }}
cancel-in-progress: true
jobs:
changes:
name: Detect changes
runs-on: ubuntu-latest
timeout-minutes: 2
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
with:
fetch-depth: 0
- uses: dorny/paths-filter@v4
id: filter
with:
token: ""
filters: |
perf:
- "packages/player/**"
- "packages/core/**"
- "package.json"
- "bun.lock"
- ".github/workflows/player-perf.yml"
perf-shards:
name: "Perf: ${{ matrix.shard }}"
needs: changes
if: needs.changes.outputs.perf == 'true'
runs-on: ubuntu-latest
timeout-minutes: 20
strategy:
fail-fast: false
matrix:
include:
- shard: load
scenarios: load
runs: "5"
- shard: fps
scenarios: fps
runs: "3"
- shard: scrub
scenarios: scrub
runs: "3"
- shard: drift
scenarios: drift
runs: "3"
- shard: parity
scenarios: parity
runs: "3"
steps:
- uses: actions/checkout@v4
- uses: oven-sh/setup-bun@v2
- uses: actions/setup-node@v4
with:
node-version: 22
- run: bun install --frozen-lockfile
# Player perf loads packages/player/dist/hyperframes-player.global.js
# and packages/core/dist/hyperframe.runtime.iife.js, so a full build is required.
- run: bun run build
- name: Set up Chrome (headless shell)
id: setup-chrome
uses: browser-actions/setup-chrome@v1
with:
chrome-version: stable
# The parity scenario shells out to `ffmpeg -lavfi ssim` to score the
# live-playback frame against the sync-seek reference frame. ffmpeg is
# not on the default ubuntu-latest runner image, and a missing binary
# surfaces as ENOENT inside computeSsim() — informative, but cheaper
# to just install it here so the shard never trips on infra.
- name: Install ffmpeg (parity shard only)
if: matrix.shard == 'parity'
run: |
sudo apt-get update
sudo apt-get install -y --no-install-recommends ffmpeg
ffmpeg -version | head -n 1
- name: Run player perf — ${{ matrix.shard }} (measure mode)
working-directory: packages/player
env:
PUPPETEER_EXECUTABLE_PATH: ${{ steps.setup-chrome.outputs.chrome-path }}
run: |
bun run perf \
--mode=measure \
--scenarios=${{ matrix.scenarios }} \
--runs=${{ matrix.runs }}
- name: Upload perf results
if: always()
uses: actions/upload-artifact@v4
with:
name: player-perf-${{ matrix.shard }}
path: packages/player/tests/perf/results/
if-no-files-found: warn
retention-days: 30
# Summary job — matches the required check name in branch protection.
# Logs an explicit "skipped" / "passed" / "failed" line both to stdout and to
# $GITHUB_STEP_SUMMARY so a false skip is obvious in the Checks UI without
# having to dig into the changes-job logs.
player-perf:
runs-on: ubuntu-latest
needs: [changes, perf-shards]
if: always()
steps:
- name: Check results
env:
PERF_FILTER_RESULT: ${{ needs.changes.outputs.perf }}
PERF_SHARDS_RESULT: ${{ needs.perf-shards.result }}
run: |
{
echo "## Player perf gate"
echo ""
echo "- paths-filter \`perf\` matched: \`${PERF_FILTER_RESULT}\`"
echo "- perf-shards result: \`${PERF_SHARDS_RESULT}\`"
echo ""
} >> "$GITHUB_STEP_SUMMARY"
if [ "${PERF_FILTER_RESULT}" != "true" ]; then
echo "::notice title=Player perf::SKIPPED — no changes under packages/player/**, packages/core/**, package.json, bun.lock, or .github/workflows/player-perf.yml. Auto-pass."
echo "**Status:** SKIPPED (no player/core changes — auto-pass)" >> "$GITHUB_STEP_SUMMARY"
exit 0
fi
if [ "${PERF_SHARDS_RESULT}" != "success" ]; then
echo "::error title=Player perf::FAILED — perf-shards result was '${PERF_SHARDS_RESULT}'. See the per-shard logs above."
echo "**Status:** FAILED (perf-shards result: \`${PERF_SHARDS_RESULT}\`)" >> "$GITHUB_STEP_SUMMARY"
exit 1
fi
echo "::notice title=Player perf::PASSED — all perf shards completed successfully."
echo "**Status:** PASSED" >> "$GITHUB_STEP_SUMMARY"