docs(product-launch-video): catch motion jumps at frame cuts - #2880
Merged
jrusso1020 merged 3 commits intoJul 29, 2026
Conversation
jrusso1020
approved these changes
Jul 29, 2026
jrusso1020
left a comment
Collaborator
There was a problem hiding this comment.
thanks for the contribution @petergyang !
# Conflicts: # skills-manifest.json
WaterrrForever
added a commit
that referenced
this pull request
Jul 30, 2026
…end to end Found while running the full product-launch-video workflow twice against a real site (linear.app) to verify PRs #2880/#2881/#2882. All three are independent of those PRs. **Scraped SVGs were unusable as files.** `assetDownloader` wrote an inline `<svg>`'s `outerHTML` straight to `assets/svgs/*.svg`. An inline SVG inherits its namespace from the HTML parser, so `outerHTML` omits `xmlns` — valid pasted back into HTML, but not a standalone document, and `<img src="logo-abc.svg">` renders a broken-image icon. That is exactly how these assets get consumed. `toStandaloneSvg` now declares the namespace on the way to disk (plus `xmlns:xlink`, but only when an `xlink:` attribute is actually used). The filename hash moved to the bytes that land on disk so it still cannot drift from content. **`sfx: none` became a cue named "none".** `fetch-sfx` split the storyboard's `sfx:` list and dropped only empty strings, so the absence marker reached the engine as a real cue that could not resolve. The absence spellings are part of the storyboard vocabulary; drop them. **`bgm_pending` was lost translating neutral meta to product-launch meta.** A detached Lyria/MusicGen generate leaves `bgm: null, bgm_pending: true` until the track lands. `toProductLaunchMeta` returned only `{bgm, voices, sfx}`, so "not ready yet" became indistinguishable from "silent by design" — and because `fetch-sfx` rewrites `audio_meta.json` from the sidecar, a still-generating bed was snapshotted away with nothing to signal it. The flag now survives, and `fetch-sfx` warns when it snapshots a pending bed instead of leaving a silent film that the storyboard claims has music. Not included, deliberately: `assemble-index.mjs` rewrites `index.html` wholesale and so discards the block `transitions.mjs inject` wrote, meaning any Step 6 rework silently loses transitions. Fixing that means deciding whether assemble preserves an injected block or inject becomes re-appliable — it touches both scripts and the Step 5/6 ordering in SKILL.md, so it deserves its own change. Validation: `node --test skills/product-launch-video/scripts/audio.test.mjs` (13 pass, 5 new) · `vitest run src/capture` (85 pass, 5 new) · `bun run lint:skills` · oxlint/oxfmt clean · `tsc --noEmit` clean
WaterrrForever
added a commit
that referenced
this pull request
Jul 30, 2026
…end to end Found while running the full product-launch-video workflow twice against a real site (linear.app) to verify PRs #2880/#2881/#2882. All three are independent of those PRs. **Scraped SVGs were unusable as files.** `assetDownloader` wrote an inline `<svg>`'s `outerHTML` straight to `assets/svgs/*.svg`. An inline SVG inherits its namespace from the HTML parser, so `outerHTML` omits `xmlns` — valid pasted back into HTML, but not a standalone document, and `<img src="logo-abc.svg">` renders a broken-image icon. That is exactly how these assets get consumed. `toStandaloneSvg` now declares the namespace on the way to disk (plus `xmlns:xlink`, but only when an `xlink:` attribute is actually used). The filename hash moved to the bytes that land on disk so it still cannot drift from content. **`sfx: none` became a cue named "none".** `fetch-sfx` split the storyboard's `sfx:` list and dropped only empty strings, so the absence marker reached the engine as a real cue that could not resolve. The absence spellings are part of the storyboard vocabulary; drop them. **`bgm_pending` was lost translating neutral meta to product-launch meta.** A detached Lyria/MusicGen generate leaves `bgm: null, bgm_pending: true` until the track lands. `toProductLaunchMeta` returned only `{bgm, voices, sfx}`, so "not ready yet" became indistinguishable from "silent by design" — and because `fetch-sfx` rewrites `audio_meta.json` from the sidecar, a still-generating bed was snapshotted away with nothing to signal it. The flag now survives, and `fetch-sfx` warns when it snapshots a pending bed instead of leaving a silent film that the storyboard claims has music. Not included, deliberately: `assemble-index.mjs` rewrites `index.html` wholesale and so discards the block `transitions.mjs inject` wrote, meaning any Step 6 rework silently loses transitions. Fixing that means deciding whether assemble preserves an injected block or inject becomes re-appliable — it touches both scripts and the Step 5/6 ordering in SKILL.md, so it deserves its own change. Validation: `node --test skills/product-launch-video/scripts/audio.test.mjs` (13 pass, 5 new) · `vitest run src/capture` (85 pass, 5 new) · `bun run lint:skills` · oxlint/oxfmt clean · `tsc --noEmit` clean
WaterrrForever
added a commit
that referenced
this pull request
Jul 30, 2026
…handoff fields binding Two follow-ups from the same end-to-end runs, now that #2880 and #2881 have landed and their sentences exist to edit. **The scroll shot pointed at an artifact that did not exist.** #2881 said "use a 2x full-page capture and animate the viewport over it". Neither half held: capture emitted no full-page image, and 2x on a long marketing page passes Chrome's 16384px screenshot cap precisely on the pages that most want a scroll shot. Both runs watched the agent go looking, not find it, and improvise — once by re-capturing 2x strips per section, once by using the native 1920x1080 tiles full-bleed. This PR's capture commit adds the 1x plate, so the sentence can now name something real: the plate, its absence on pages too tall to capture in one piece, the tile fallback, and why pushing in past 1:1 still wants a region capture of its own. **A constant field was being read as an absent one.** #2880 asks for x/y, scale, opacity and direction/speed on every handoff. Across two runs on the same model, `opacity` went 0/12 then 12/12 — when the value never changes, leaving it out is a reasonable reading of the instruction. But downstream an omission and "there is no handoff here" are the same thing, so the field set has to be stated as binding even when constant. Same clause added to the worker's side of the contract. Validation: `bun run lint:skills`
WaterrrForever
added a commit
that referenced
this pull request
Jul 30, 2026
…nd to end (#2892) * fix(capture,audio): three defects found running product-launch-video end to end Found while running the full product-launch-video workflow twice against a real site (linear.app) to verify PRs #2880/#2881/#2882. All three are independent of those PRs. **Scraped SVGs were unusable as files.** `assetDownloader` wrote an inline `<svg>`'s `outerHTML` straight to `assets/svgs/*.svg`. An inline SVG inherits its namespace from the HTML parser, so `outerHTML` omits `xmlns` — valid pasted back into HTML, but not a standalone document, and `<img src="logo-abc.svg">` renders a broken-image icon. That is exactly how these assets get consumed. `toStandaloneSvg` now declares the namespace on the way to disk (plus `xmlns:xlink`, but only when an `xlink:` attribute is actually used). The filename hash moved to the bytes that land on disk so it still cannot drift from content. **`sfx: none` became a cue named "none".** `fetch-sfx` split the storyboard's `sfx:` list and dropped only empty strings, so the absence marker reached the engine as a real cue that could not resolve. The absence spellings are part of the storyboard vocabulary; drop them. **`bgm_pending` was lost translating neutral meta to product-launch meta.** A detached Lyria/MusicGen generate leaves `bgm: null, bgm_pending: true` until the track lands. `toProductLaunchMeta` returned only `{bgm, voices, sfx}`, so "not ready yet" became indistinguishable from "silent by design" — and because `fetch-sfx` rewrites `audio_meta.json` from the sidecar, a still-generating bed was snapshotted away with nothing to signal it. The flag now survives, and `fetch-sfx` warns when it snapshots a pending bed instead of leaving a silent film that the storyboard claims has music. Not included, deliberately: `assemble-index.mjs` rewrites `index.html` wholesale and so discards the block `transitions.mjs inject` wrote, meaning any Step 6 rework silently loses transitions. Fixing that means deciding whether assemble preserves an injected block or inject becomes re-appliable — it touches both scripts and the Step 5/6 ordering in SKILL.md, so it deserves its own change. Validation: `node --test skills/product-launch-video/scripts/audio.test.mjs` (13 pass, 5 new) · `vitest run src/capture` (85 pass, 5 new) · `bun run lint:skills` · oxlint/oxfmt clean · `tsc --noEmit` clean * feat(capture): re-add the full-page plate a scroll shot needs, at 1x `product-launch-video` tells a scroll shot to animate a viewport over a full-page capture. No such file existed: capture emits 15 viewport-sized scroll-position tiles, and a plate is not substitutable by tiles — a viewport travelling down one continuous image is the whole point. An earlier `full-page.png` was dropped in 62b5517 because 1/8 agents read it and the contact sheet covered the same ground. That measured it as a *comprehension* artifact, on an eval where nothing was building scroll shots. The scroll shot is a different consumer, so this brings the plate back — but not as it was, because two things have to hold for it to be worth having: - **Taken last.** After the scroll traversal, so lazy images have loaded and scroll-triggered reveals have fired. A plate shot on arrival is full of blank bands, which is a good reason for an agent to look once and never again. - **Sticky chrome neutralised.** `fullPage` bakes a fixed header in at one position, freezing a nav across the middle of the plate. The viewport tiles keep sticky on purpose (natural browsing state); the plate cannot. Positions are recorded and restored in a `finally`, so the extraction passes that run afterwards see an unmodified DOM. **1x, deliberately.** 2x is what you'd want to push in without softening text, but doubling a long marketing page passes Chrome's 16384px screenshot cap precisely on the pages that most want a scroll shot (linear.app: 10962 CSS px → 21924 at 2x). At 1x a 1920-wide plate is pixel-exact for a 1920x1080 viewport. A frame that needs headroom captures its own region at 2x instead. Pages over the cap get no plate rather than a silently clipped one, and the caller falls back to the tiles. Validation: `vitest run src/capture` — 90 pass (5 new) · oxlint/oxfmt clean · `tsc --noEmit` clean * docs(product-launch-video): point the scroll shot at the plate, make handoff fields binding Two follow-ups from the same end-to-end runs, now that #2880 and #2881 have landed and their sentences exist to edit. **The scroll shot pointed at an artifact that did not exist.** #2881 said "use a 2x full-page capture and animate the viewport over it". Neither half held: capture emitted no full-page image, and 2x on a long marketing page passes Chrome's 16384px screenshot cap precisely on the pages that most want a scroll shot. Both runs watched the agent go looking, not find it, and improvise — once by re-capturing 2x strips per section, once by using the native 1920x1080 tiles full-bleed. This PR's capture commit adds the 1x plate, so the sentence can now name something real: the plate, its absence on pages too tall to capture in one piece, the tile fallback, and why pushing in past 1:1 still wants a region capture of its own. **A constant field was being read as an absent one.** #2880 asks for x/y, scale, opacity and direction/speed on every handoff. Across two runs on the same model, `opacity` went 0/12 then 12/12 — when the value never changes, leaving it out is a reasonable reading of the instruction. But downstream an omission and "there is no handoff here" are the same thing, so the field set has to be stated as binding even when constant. Same clause added to the worker's side of the contract. Validation: `bun run lint:skills` * fix(capture,audio): close the three contract gaps raised in review Review on #2892 (Rames, Magi) found the fixes correct inside the changed files but incomplete at the contract level. All three hold up against source; two of the three were reachable in production, and the plate one was self-inflicted by this PR. **The plate guard checked a stale height.** `scrollHeight` was measured before the scroll traversal and handed to the guard, but the plate is deliberately shot *after* it so lazy content has loaded — and lazy loading grows the document. The guard's input therefore read low on exactly the long pages it exists for, letting the check pass and a clipped plate through, undetectable downstream because the skill only teaches the tile fallback when the file is *absent*. `captureFullPagePlate` now measures the height itself at call time, and verifies what Chrome actually produced by reading the PNG's IHDR before writing, since the capture can trigger another round of loading. Over the cap, nothing is emitted. **Assembly dropped the flag again.** `bgm_pending` survived into `audio_meta.json` but `assemble-index.mjs` rebuilt its audio object from three named keys, so at the step that actually builds the film "not ready yet" still looked like "silent by design" — this PR's own framing of the defect, one layer further down. The flag rides along now, and a pending bed with no file raises an anomaly instead of quietly assembling a silent cut against a storyboard that promises music. **The sibling adapters had both audio bugs, and there were two of them.** The review named `faceless-explainer`; `pr-to-video` carries the same file. Its own test asserts the two are byte-identical ("intentionally identical across the reusing skills"), so fixing one alone broke that test — which is what caught the second copy. Both now carry the absence-sentinel filter and the surviving `bgm_pending`, and `faceless-explainer` gets the same five regression tests. Also from review (Miga): the sticky-restore in `finally` is wrapped, so a page that broke mid-capture cannot replace the real error with a cleanup one. Validation: `vitest run src/capture` — 95 pass (5 new) · product-launch audio 13 pass · faceless-explainer audio 10 pass (5 new, incl. the byte-identity contract) · `bun run lint:skills` · oxlint/oxfmt clean · `tsc --noEmit` clean * fix(capture,audio): meet the two review asks I under-delivered on Follow-up to 194fb69. Re-read Magi's review body rather than working from the summary, and two of the three blockers were addressed in spirit but not to the letter. **The plate probed before neutralisation, not after.** 194fb69 moved the measurement off the caller's stale value and into the function, but took it before forcing fixed/sticky elements to `static`. The review called this out specifically and is right: dropping those elements back into flow grows the document, so the probe could still read under the cap on a page that is over it once neutralised. The probe now runs after neutralisation and before the shot, inside the same `try` so restoration still happens on the early return. Added the exact case asked for — initial height under the cap, final height over it — asserting no screenshot is taken, no file is written, and the page is still handed back unmodified. **Assembly warned where the review asked it to refuse.** An anomaly in a list is not enforcement: assemble is re-run on Step 6 rework, long after the audio step's warning scrolled past, and a warning still lets a silent film out the door over a snapshot whose own JSON says the bed is generating. `assemble-index.mjs` now dies on `bgm_pending && !bgm`, with `--allow-pending-bgm` as the deliberate escape for previewing mid-generate. Pinned with three tests in a new `assemble-index.test.mjs`: refusal writes no index.html, the escape assembles and says so, and a film that is silent *by design* still assembles untouched — the distinction the flag exists to make. Validation: `vitest run src/capture` — 96 pass (6 new) · product-launch audio 13 pass · assemble-index 3 pass (new file) · faceless-explainer audio 10 pass · `bun run lint:skills` · oxlint/oxfmt clean · `tsc --noEmit` clean * fix(audio): carry the bgm_pending gate into the sibling assemblers The remaining blocker, and one this PR created: the previous commit made all three copies of the audio adapter *emit* bgm_pending, but only product-launch-video's assembler *reads* it. So faceless-explainer and pr-to-video would do exactly what this PR set out to stop — parse an audio_meta.json that says the bed is still generating and assemble the silent film without a word. Producer fixed in three places, consumer in one, is worse than neither: before this PR there was no flag to drop. Both siblings now get the same three changes product-launch-video got — the flag carried through the audio object, `die` on `bgm_pending && !bgm`, and `--allow-pending-bgm` as the deliberate escape — plus the same three tests: refusal writes no index.html, the escape assembles and says so, and a film that is silent *by design* still assembles untouched. That last one is the one worth having; it proves the flag restored a distinction rather than just adding a gate. Applied as three separate patches rather than a file copy: these assemblers have diverged (pr-to-video validates a bare `<template>` fragment where product-launch takes a `<div>` root, which its fixture reflects). `music-to-video` has the fourth copy of this assembler and is deliberately untouched: it has no audio producer, and its assembler reads `{ voices: [] }` with no bgm path at all, so the flag can never reach it. Validation: product-launch / faceless-explainer / pr-to-video assemble-index — 3 pass each · product-launch audio 13 pass · faceless-explainer audio 10 pass · `vitest run src/capture` 96 pass · `bun run lint:skills` · oxlint/oxfmt clean · `tsc --noEmit` clean
dahans-msft2
pushed a commit
to dahans-msft2/hyperframes
that referenced
this pull request
Aug 6, 2026
…com#2880) * docs(product-launch-video): verify frame seams * chore(skills): regenerate skills manifest --------- Co-authored-by: James <james.russo@heygen.com>
dahans-msft2
pushed a commit
to dahans-msft2/hyperframes
that referenced
this pull request
Aug 6, 2026
…nd to end (heygen-com#2892) * fix(capture,audio): three defects found running product-launch-video end to end Found while running the full product-launch-video workflow twice against a real site (linear.app) to verify PRs heygen-com#2880/heygen-com#2881/heygen-com#2882. All three are independent of those PRs. **Scraped SVGs were unusable as files.** `assetDownloader` wrote an inline `<svg>`'s `outerHTML` straight to `assets/svgs/*.svg`. An inline SVG inherits its namespace from the HTML parser, so `outerHTML` omits `xmlns` — valid pasted back into HTML, but not a standalone document, and `<img src="logo-abc.svg">` renders a broken-image icon. That is exactly how these assets get consumed. `toStandaloneSvg` now declares the namespace on the way to disk (plus `xmlns:xlink`, but only when an `xlink:` attribute is actually used). The filename hash moved to the bytes that land on disk so it still cannot drift from content. **`sfx: none` became a cue named "none".** `fetch-sfx` split the storyboard's `sfx:` list and dropped only empty strings, so the absence marker reached the engine as a real cue that could not resolve. The absence spellings are part of the storyboard vocabulary; drop them. **`bgm_pending` was lost translating neutral meta to product-launch meta.** A detached Lyria/MusicGen generate leaves `bgm: null, bgm_pending: true` until the track lands. `toProductLaunchMeta` returned only `{bgm, voices, sfx}`, so "not ready yet" became indistinguishable from "silent by design" — and because `fetch-sfx` rewrites `audio_meta.json` from the sidecar, a still-generating bed was snapshotted away with nothing to signal it. The flag now survives, and `fetch-sfx` warns when it snapshots a pending bed instead of leaving a silent film that the storyboard claims has music. Not included, deliberately: `assemble-index.mjs` rewrites `index.html` wholesale and so discards the block `transitions.mjs inject` wrote, meaning any Step 6 rework silently loses transitions. Fixing that means deciding whether assemble preserves an injected block or inject becomes re-appliable — it touches both scripts and the Step 5/6 ordering in SKILL.md, so it deserves its own change. Validation: `node --test skills/product-launch-video/scripts/audio.test.mjs` (13 pass, 5 new) · `vitest run src/capture` (85 pass, 5 new) · `bun run lint:skills` · oxlint/oxfmt clean · `tsc --noEmit` clean * feat(capture): re-add the full-page plate a scroll shot needs, at 1x `product-launch-video` tells a scroll shot to animate a viewport over a full-page capture. No such file existed: capture emits 15 viewport-sized scroll-position tiles, and a plate is not substitutable by tiles — a viewport travelling down one continuous image is the whole point. An earlier `full-page.png` was dropped in 62b5517 because 1/8 agents read it and the contact sheet covered the same ground. That measured it as a *comprehension* artifact, on an eval where nothing was building scroll shots. The scroll shot is a different consumer, so this brings the plate back — but not as it was, because two things have to hold for it to be worth having: - **Taken last.** After the scroll traversal, so lazy images have loaded and scroll-triggered reveals have fired. A plate shot on arrival is full of blank bands, which is a good reason for an agent to look once and never again. - **Sticky chrome neutralised.** `fullPage` bakes a fixed header in at one position, freezing a nav across the middle of the plate. The viewport tiles keep sticky on purpose (natural browsing state); the plate cannot. Positions are recorded and restored in a `finally`, so the extraction passes that run afterwards see an unmodified DOM. **1x, deliberately.** 2x is what you'd want to push in without softening text, but doubling a long marketing page passes Chrome's 16384px screenshot cap precisely on the pages that most want a scroll shot (linear.app: 10962 CSS px → 21924 at 2x). At 1x a 1920-wide plate is pixel-exact for a 1920x1080 viewport. A frame that needs headroom captures its own region at 2x instead. Pages over the cap get no plate rather than a silently clipped one, and the caller falls back to the tiles. Validation: `vitest run src/capture` — 90 pass (5 new) · oxlint/oxfmt clean · `tsc --noEmit` clean * docs(product-launch-video): point the scroll shot at the plate, make handoff fields binding Two follow-ups from the same end-to-end runs, now that heygen-com#2880 and heygen-com#2881 have landed and their sentences exist to edit. **The scroll shot pointed at an artifact that did not exist.** heygen-com#2881 said "use a 2x full-page capture and animate the viewport over it". Neither half held: capture emitted no full-page image, and 2x on a long marketing page passes Chrome's 16384px screenshot cap precisely on the pages that most want a scroll shot. Both runs watched the agent go looking, not find it, and improvise — once by re-capturing 2x strips per section, once by using the native 1920x1080 tiles full-bleed. This PR's capture commit adds the 1x plate, so the sentence can now name something real: the plate, its absence on pages too tall to capture in one piece, the tile fallback, and why pushing in past 1:1 still wants a region capture of its own. **A constant field was being read as an absent one.** heygen-com#2880 asks for x/y, scale, opacity and direction/speed on every handoff. Across two runs on the same model, `opacity` went 0/12 then 12/12 — when the value never changes, leaving it out is a reasonable reading of the instruction. But downstream an omission and "there is no handoff here" are the same thing, so the field set has to be stated as binding even when constant. Same clause added to the worker's side of the contract. Validation: `bun run lint:skills` * fix(capture,audio): close the three contract gaps raised in review Review on heygen-com#2892 (Rames, Magi) found the fixes correct inside the changed files but incomplete at the contract level. All three hold up against source; two of the three were reachable in production, and the plate one was self-inflicted by this PR. **The plate guard checked a stale height.** `scrollHeight` was measured before the scroll traversal and handed to the guard, but the plate is deliberately shot *after* it so lazy content has loaded — and lazy loading grows the document. The guard's input therefore read low on exactly the long pages it exists for, letting the check pass and a clipped plate through, undetectable downstream because the skill only teaches the tile fallback when the file is *absent*. `captureFullPagePlate` now measures the height itself at call time, and verifies what Chrome actually produced by reading the PNG's IHDR before writing, since the capture can trigger another round of loading. Over the cap, nothing is emitted. **Assembly dropped the flag again.** `bgm_pending` survived into `audio_meta.json` but `assemble-index.mjs` rebuilt its audio object from three named keys, so at the step that actually builds the film "not ready yet" still looked like "silent by design" — this PR's own framing of the defect, one layer further down. The flag rides along now, and a pending bed with no file raises an anomaly instead of quietly assembling a silent cut against a storyboard that promises music. **The sibling adapters had both audio bugs, and there were two of them.** The review named `faceless-explainer`; `pr-to-video` carries the same file. Its own test asserts the two are byte-identical ("intentionally identical across the reusing skills"), so fixing one alone broke that test — which is what caught the second copy. Both now carry the absence-sentinel filter and the surviving `bgm_pending`, and `faceless-explainer` gets the same five regression tests. Also from review (Miga): the sticky-restore in `finally` is wrapped, so a page that broke mid-capture cannot replace the real error with a cleanup one. Validation: `vitest run src/capture` — 95 pass (5 new) · product-launch audio 13 pass · faceless-explainer audio 10 pass (5 new, incl. the byte-identity contract) · `bun run lint:skills` · oxlint/oxfmt clean · `tsc --noEmit` clean * fix(capture,audio): meet the two review asks I under-delivered on Follow-up to 194fb69. Re-read Magi's review body rather than working from the summary, and two of the three blockers were addressed in spirit but not to the letter. **The plate probed before neutralisation, not after.** 194fb69 moved the measurement off the caller's stale value and into the function, but took it before forcing fixed/sticky elements to `static`. The review called this out specifically and is right: dropping those elements back into flow grows the document, so the probe could still read under the cap on a page that is over it once neutralised. The probe now runs after neutralisation and before the shot, inside the same `try` so restoration still happens on the early return. Added the exact case asked for — initial height under the cap, final height over it — asserting no screenshot is taken, no file is written, and the page is still handed back unmodified. **Assembly warned where the review asked it to refuse.** An anomaly in a list is not enforcement: assemble is re-run on Step 6 rework, long after the audio step's warning scrolled past, and a warning still lets a silent film out the door over a snapshot whose own JSON says the bed is generating. `assemble-index.mjs` now dies on `bgm_pending && !bgm`, with `--allow-pending-bgm` as the deliberate escape for previewing mid-generate. Pinned with three tests in a new `assemble-index.test.mjs`: refusal writes no index.html, the escape assembles and says so, and a film that is silent *by design* still assembles untouched — the distinction the flag exists to make. Validation: `vitest run src/capture` — 96 pass (6 new) · product-launch audio 13 pass · assemble-index 3 pass (new file) · faceless-explainer audio 10 pass · `bun run lint:skills` · oxlint/oxfmt clean · `tsc --noEmit` clean * fix(audio): carry the bgm_pending gate into the sibling assemblers The remaining blocker, and one this PR created: the previous commit made all three copies of the audio adapter *emit* bgm_pending, but only product-launch-video's assembler *reads* it. So faceless-explainer and pr-to-video would do exactly what this PR set out to stop — parse an audio_meta.json that says the bed is still generating and assemble the silent film without a word. Producer fixed in three places, consumer in one, is worse than neither: before this PR there was no flag to drop. Both siblings now get the same three changes product-launch-video got — the flag carried through the audio object, `die` on `bgm_pending && !bgm`, and `--allow-pending-bgm` as the deliberate escape — plus the same three tests: refusal writes no index.html, the escape assembles and says so, and a film that is silent *by design* still assembles untouched. That last one is the one worth having; it proves the flag restored a distinction rather than just adding a gate. Applied as three separate patches rather than a file copy: these assemblers have diverged (pr-to-video validates a bare `<template>` fragment where product-launch takes a `<div>` root, which its fixture reflects). `music-to-video` has the fourth copy of this assembler and is deliberately untouched: it has no audio producer, and its assembler reads `{ voices: [] }` with no bgm path at all, so the flag can never reach it. Validation: product-launch / faceless-explainer / pr-to-video assemble-index — 3 pass each · product-launch audio 13 pass · faceless-explainer audio 10 pass · `vitest run src/capture` 96 pass · `bun run lint:skills` · oxlint/oxfmt clean · `tsc --noEmit` clean
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
Two frames can each look correct on their own and still jump when they are stitched together. The current workflow checks frame midpoints, so it can miss a logo, card, or screenshot that suddenly changes position, size, opacity, or direction at the cut.
What this changes
Why merge this
This catches a common visual defect before the final render. It makes parallel frame work more reliable without adding a dependency or changing the runtime.
Validation
bun run lint:skillsgit diff --check