Skip to content

fix(media-use): reconcile disk-only narration WAVs into the audio ledger - #4000

Open
miga-heygen wants to merge 3 commits into
mainfrom
fix-narration-ledger-reconciliation
Open

miga-heygen wants to merge 3 commits into
mainfrom
fix-narration-ledger-reconciliation

Conversation

@miga-heygen

@miga-heygen miga-heygen commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • A narration WAV can land in assets/voice/<id>.wav without ever going through this engine's own TTS synthesis loop: hand-placed, copied over from another project, or left behind by a run that wrote the file but didn't finish registering it. audio_meta.json's voices[] only ever grows inside that synthesis loop, so a file like this was invisible to every consumer that trusts voices[] (each workflow's assemble-index.mjs <audio> emission, captions.mjs) — the render succeeds and simply plays without that narration line, with zero warning.
  • skills/media-use/audio/scripts/audio.mjs is the one shared engine backing this (its own header: "ONE implementation... workflows do NOT vendor a copy"), used by product-launch-video, faceless-explainer, and pr-to-video. Added a reconciliation step there — the single correct fix location — that scans assets/voice/ on every invocation (independent of --only, so a bgm/sfx-only re-run still catches drift) and backfills any file whose id isn't already known but is still requested by the current script with non-empty text. A stale file from a since-edited or since-cleared line is left alone, not resurrected.
  • Placed before the existing hasVoice/totalDuration computation so a reconciled voice correctly participates in the BGM sizing/retrieval decision downstream, not just the final ledger output.
  • Extracted the "does this line have real text" check (previously only inline in the synthesis loop) into a shared lineText() helper so the two call sites can't drift on that decision.

Test plan

  • New test file with 5 cases, spawning the real engine script end-to-end (no mocks): a disk-only WAV gets backfilled with a real ffprobe-read duration; an already-registered voice isn't duplicated; a WAV with no matching current script line is left alone; a WAV for a line whose text was since cleared is left alone; a project with no assets/voice/ directory is an unaffected no-op.
  • Confirmed genuine RED on the unmodified code (2 of the 5 cases fail with 0 backfilled voices instead of 1), then GREEN after restoring the fix.
  • All 5 tests pass locally where ffmpeg/ffprobe are on PATH. In the Test: skills CI job specifically (no ffmpeg on that runner), these 5 tests currently {skip: !HAS_FFMPEG}, the same as the pre-existing BGM loop-fade test in this same skill — a pre-existing gap in that job's own design, not something introduced by this change.
  • All 3 consuming workflows' own test suites (194 tests) and the full media-use test tree (351 tests) pass with no regressions.

miga-heygen and others added 3 commits September 16, 2026 13:31
A narration WAV can land in assets/voice/<id>.wav without ever going
through this engine's own TTS synthesis loop: hand-placed, copied over
from another project, or left behind by a run that wrote the file but
didn't finish registering it. audio_meta.json's voices[] only grows
inside that loop, so a file like this was invisible to every consumer
that trusts voices[] (each skill's assemble-index.mjs <audio> emission,
captions.mjs) with zero warning — the render succeeds and simply plays
without that narration line.

Reconcile assets/voice/ against voices[] on every engine invocation,
independent of --only, before the BGM sizing decision that depends on
voices[]. Only backfills ids the current request still asks for with
non-empty text, so a stale file from a since-edited or since-cleared
script line is left untouched, not resurrected. Extracted the line's
"has real text" check into a shared lineText() helper so the synthesis
loop and the new reconciliation pass can't drift on that decision.

Co-Authored-By: Miguel Ángel <miguel.sierra@heygen.com>
knownIds.add(id) inside the reconciliation loop could never affect
anything: id is derived uniquely from each disk filename, and readdirSync
never returns the same filename twice in one pass, so no iteration could
ever re-observe an id it just added. Removed it along with the unused
extraArgs parameter on the test's runEngine helper, and switched the test
fixtures to the house mkdtempSync/t.after(cleanup) pattern already used
elsewhere in this skill (skills/media-use/scripts/dither.test.mjs) instead
of leaking a temp directory per test run.

Co-Authored-By: Miguel Ángel <miguel.sierra@heygen.com>
Co-Authored-By: Miguel Ángel <miguel.sierra@heygen.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant