fix(media-use): reconcile disk-only narration WAVs into the audio ledger - #4000
Open
miga-heygen wants to merge 3 commits into
Open
miga-heygen wants to merge 3 commits into
miga-heygen wants to merge 3 commits into
Conversation
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
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.
Summary
assets/voice/<id>.wavwithout 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'svoices[]only ever grows inside that synthesis loop, so a file like this was invisible to every consumer that trustsvoices[](each workflow'sassemble-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.mjsis the one shared engine backing this (its own header: "ONE implementation... workflows do NOT vendor a copy"), used byproduct-launch-video,faceless-explainer, andpr-to-video. Added a reconciliation step there — the single correct fix location — that scansassets/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.hasVoice/totalDurationcomputation so a reconciled voice correctly participates in the BGM sizing/retrieval decision downstream, not just the final ledger output.lineText()helper so the two call sites can't drift on that decision.Test plan
assets/voice/directory is an unaffected no-op.Test: skillsCI 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.media-usetest tree (351 tests) pass with no regressions.