feat(audio): require the voiceover carve, default strength 0.8, slower release - #3878
Conversation
…a voice The hyperframes-audio skill described the carve as one option among several, so builds with BGM under narration shipped with only a volume duck. Make it a requirement in the three places an agent reads while assembling audio: the carve section of hyperframes-audio, general-video's Assemble step, and the production loop's Audio stage. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
0.25 left the bed audibly fighting the voice; a build carved at 0.25 had to be redone at 0.8 before it was accepted. Make carve.mjs default to 0.8 and rewrite the strength prose and the sample output to match. The core DEFAULT_CARVE used by the Studio panel is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Second commit: |
At 1.6s the bed audibly came back at every sentence break on narrated builds, which reads as the effect switching off rather than the mix breathing. 2.4s lets it swell back over a breath. Band-filter release is unchanged; a notch closing is inaudible. Test timings move with the constant. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Third commit: level-envelope release 1.6s → 2.4s in core ( |
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at fe53f51a. The behaviour change is well argued and the docs match the code — I verified the numbers rather than taking them. Two things to fix before this goes in, one of them mechanical.
The release change ships with nothing pinning it
This is the finding. DUCK_RELEASE_S 1.6 → 2.4 is the PR's headline, and after this diff no test fails if it is reverted:
DUCK_RELEASE_S = 2.4 (head) → 52 passed
DUCK_RELEASE_S = 1.6 (reverted, head tests) → 52 passed
That is not a pre-existing gap — the suite did discriminate before this PR. Running the head's constant against the base's assertions reds two tests:
× analyseCarveDuck > leaves the bed alone where the voice is silent
→ expected -0.215 to be close to +0
× analyseCarveDuck > lets the bed come back slowly after the voice stops
So the old assertions were load-bearing, and moving the sample points later (15 → 15.9, 7.5 → 8.5, 11 → 13.5) made each one true of both constants. The edits were necessary — the old points genuinely fail at 2.4 — but the replacements landed far enough out on the tail that they stopped distinguishing the two releases at all. Net effect: the one PR that changes this constant is also the one that removes the suite's grip on it, and a future revert is silent.
The envelope has plenty of room to pin it. Same fixture as the existing test, three seconds after the last word:
| t | 2.4s (head) | 1.6s (base) |
|---|---|---|
| 7.5 | −5.34 | −3.84 |
| 9.0 | −2.85 | −1.47 |
| 11.0 | −1.20 | −0.40 |
One line, added to lets the bed come back slowly after the voice stops:
// Three seconds past the last word the bed is still audibly down — at 1.6s it
// is all but back, which is the sentence-break flip this constant exists to stop.
expect(at(duck, 9)).toBeLessThan(-2);I ran it both ways before suggesting it: passes at 2.4, fails at 1.6 with expected -1.4687 to be less than -2. Pick a different point if you prefer, but please pick one that reds on the old value — the current three do not.
skills-manifest.json is stale for hyperframes-audio
Regenerating with the repo's own generator at this head changes the hash:
$ bun packages/cli/scripts/gen-skills-manifest.ts
~ hyperframes-audio (24871fc8a76d942f → 11e9f843798abf49)
Deterministic across runs, on a clean tree, and general-video and hyperframes-core both regenerate to exactly the values you committed — so the generator agrees with you on two of three skills and disagrees on one. Skills: manifest in sync is red for this.
The committed 24871fc8a76d942f matches neither the head's audio content nor the previous commit's (d893452e066a0c38), so it looks like the pre-commit hook ran, then SKILL.md picked up another edit before the commit closed. bun packages/cli/scripts/gen-skills-manifest.ts and commit the result fixes it.
Smaller
The new carve.mjs default is unpinned too (scripts/carve.mjs:47). carve.test.mjs does run in CI, but it is entirely about bed/voice/group detection — no assertion mentions strength, so 0.25 → 0.8 broke nothing and nothing would notice it moving again. Core's side of this is pinned (expect(DEFAULT_CARVE.strength).toBe(0.25)), which is what makes the asymmetry worth a line.
The "Required whenever music plays under a voice" block overlaps the existing Carve by default paragraph a dozen lines below it, which already says a bed under narration is not a polish step. Not wrong, just two voices saying one rule; folding the older one in would read better.
What I checked and found accurate
52/52 pass— exact, and 325 pass across the audio suites with no collateral from the constant.- "Studio's
DEFAULT_CARVEin core stays at 0.25" — true, unchanged in the diff, and pinned by a test. - "Band-filter release unchanged" —
RELEASE_S = 0.25untouched. - The documented sample output, which is the claim I most expected to drift, and it does not.
carveProfile(0.8)returns{maxCutDb: 14.8, bands: 6, q: 2.06, intelligibilityBias: 0.92, duckDb: 19.2, headroomDb: 15.6}, so "six bands … about 7 dB each and 15 dB at 1.6 kHz, with 19 dB of level room" and thefloor -19.2 dBin the sample block are all real numbers from the profile.carveProfile(0.25)likewise gives the6 dB / three bands / 6 dB of roomthe prose now uses for comparison.
I could not reproduce the −26 / −31 / −33 dB recap-build measurement — that needs the build — so I am taking it as reported.
On the other red checks: Test failed at 18:30 because the producer jobs it gates on were cancelled, and those same jobs succeeded at 18:34; regression has both a success and a failure at this head. Neither is attributable to this diff. The manifest one is real, and I reproduced it independently before looking at CI.
Requesting changes on the manifest and the missing pin — both are a few minutes. The change itself reads right to me: a slower release on narrated builds is the correct call, and the docblock explaining why 1.6 failed is the part that will stop someone tuning it back.
— Rames
| // ...noticeably recovered two and a half seconds on... | ||
| expect(at(duck, 8.5)).toBeGreaterThan(atSpeech * 0.5); | ||
| // ...and within a hair of flat several seconds later, rather than held down. | ||
| expect(at(duck, 13.5)).toBeGreaterThan(-0.6); |
There was a problem hiding this comment.
These three points now pass at both 1.6s and 2.4s, so nothing here reds if DUCK_RELEASE_S is reverted. The base versions of these same assertions did fail at 2.4 — the discrimination existed and this edit spent it.
Adding one point that still separates them (verified both ways: passes at 2.4, fails at 1.6 with expected -1.4687 to be less than -2):
expect(at(duck, 9)).toBeLessThan(-2);There was a problem hiding this comment.
Added your 9s probe verbatim (passes at 2.4, reds at 1.6). Kept the 8.5s / 13.5s points as the recovery-shape checks. (in e488871)
| }, | ||
| "hyperframes-audio": { | ||
| "hash": "84035d8ce2a61fbc", | ||
| "hash": "24871fc8a76d942f", |
There was a problem hiding this comment.
Stale. Regenerating at this head gives 11e9f843798abf49; general-video and hyperframes-core both regenerate to exactly what you committed, so it is only this one. It matches neither the head content nor the previous commit’s (d893452e066a0c38) — reads like the hook ran and then SKILL.md took one more edit.
bun packages/cli/scripts/gen-skills-manifest.ts
There was a problem hiding this comment.
Regenerated with gen-skills-manifest.ts after the final SKILL.md edit; the hook had run before a later edit, as you guessed. (in e488871)
|
|
||
| function parseArgs(argv) { | ||
| const args = { strength: 0.25, dryRun: false, voices: [] }; | ||
| const args = { strength: 0.8, dryRun: false, voices: [] }; |
There was a problem hiding this comment.
Nit: nothing pins this. carve.test.mjs runs in CI but never mentions strength — it is all bed/voice/group detection — so this default could move again unnoticed. Core’s equivalent is pinned (expect(DEFAULT_CARVE.strength).toBe(0.25)), which is what makes the gap stand out.
There was a problem hiding this comment.
Pinned: DEFAULT_STRENGTH and parseArgs are exported, and carve.test.mjs asserts 0.8 plus the --strength override. (in e488871)
…quirement, sync the manifest Review at fe53f51: the moved test points passed at both 1.6s and 2.4s, so a revert of DUCK_RELEASE_S was silent. Add the 9s probe that reds on 1.6s. Export DEFAULT_STRENGTH and parseArgs from carve.mjs and pin 0.8 in carve.test.mjs. Fold the new requirement paragraph into the existing 'Carve by default' one so the rule is stated once. Regenerate skills-manifest.json after the last edit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
jrusso1020
left a comment
There was a problem hiding this comment.
Approving at e488871c. This supersedes my own CHANGES_REQUESTED 5182288325 at fe53f51a — noting it because both land under one jrusso1020 identity and GitHub keeps no record that a block existed.
Both items closed, and I re-ran the same checks that found them rather than reading the fix.
1. The constant is pinned now. The check that failed before is the one that matters: revert DUCK_RELEASE_S to 1.6 and the suite reds.
at 2.4: 52 pass, 0 fail
at 1.6: 51 pass, 1 fail
expect(at(duck, 9)).toBeLessThan(-2);
Expected: < -2
Received: -1.4687500000000007
(fail) analyseCarveDuck > lets the bed come back slowly after the voice stops
That -1.469 is the −1.47 dB I measured when I proposed the probe, so it is landing exactly where it was aimed. Before this commit the same revert was 52 pass either way. Keeping 8.5s and 13.5s as recovery-shape checks is the right call — they were never wrong, just not discriminating on their own.
2. The manifest is in sync. The generator is now a no-op on a clean tree:
$ bun packages/cli/scripts/gen-skills-manifest.ts
skills-manifest.json already in sync — no change (20 skills)
sha unchanged, git diff empty. The entry reads 383dbd408b47a7db rather than the 11e9f843798abf49 I computed at fe53f51a, which is what it should be — SKILL.md changed again in this push, so the content hash moved and the manifest was regenerated after that edit. That ordering was the whole original problem, and it is the right way round now.
3. DEFAULT_STRENGTH, which you pinned on your own initiative — worth saying it discriminates too, since exporting a constant and asserting it can easily be a tautology. It isn't: set it back to 0.25 and the test reds on expected: 0.8, actual: 0.25. Asserting parseArgs separately is the part that earns its keep — it pins the wiring, not just the literal, so a default that stops reaching args.strength still fails.
The SKILL.md fold is faithful. I diffed it clause by clause because a "fold" that quietly drops a requirement is the thing to watch for. Every clause survives: the requirement itself, the four voice-track kinds, the command with --bed/--voice, the hyperframes check confirmation of data-fx-carve / data-fx-chain / data-automation before rendering, and the 1–3 kHz duck-is-not-a-mix rationale verbatim. The skip condition even got more correct on the way — "no narration for the music to sit under" became "no voice", which is what the rest of the section says. Net effect is one statement of the rule instead of two that could drift apart.
One note that is not yours: bun test skills/hyperframes-audio/scripts/carve.test.mjs fails locally on cannot load @hyperframes/core from its temp dir. I checked it against fe53f51a and it fails identically there, so it is an environment resolution issue, not this PR. Under node --test the new test passes.
Review by Rames (pr-review session)
|
Approval stands — this is a formatting nit, not a code issue, but it is what is holding the merge so flagging it rather than leaving you to hunt it.
It is from this push: the same file formats clean at -import { DEFAULT_STRENGTH, carveSources, groupSourceRefusal, loadCore, parseArgs } from "./carve.mjs";
+import {
+ DEFAULT_STRENGTH,
+ carveSources,
+ groupSourceRefusal,
+ loadCore,
+ parseArgs,
+} from "./carve.mjs";
(Worth noting for anyone checking locally: this repo formats with oxfmt, not prettier — prettier is not even a devDependency, and running it flags unrelated files that are perfectly fine.) — Rames (pr-review session) |
The pre-commit format hook's glob does not include .mjs, so the new test file reached CI unformatted while oxfmt --check . covers it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Why
A narrated build with background music (recap-ripples, 2026-09-11) shipped with only a volume duck because the hyperframes-audio skill framed the voiceover carve as an option. Carved at the 0.25 default it was still too weak, and at 1.6s release the bed audibly came back at every sentence break.
What (three commits)
skills/hyperframes-audio/SKILL.md, general-video's Assemble step, and the production loop's Audio stage.skills-manifest.jsonis the hook-regenerated hash update.carve.mjs(was 0.25); strength prose and sample output rewritten to match. Studio'sDEFAULT_CARVEin core stays at 0.25, that's the panel's toggle-on behaviour.DUCK_RELEASE_S1.6s → 2.4s. Band-filter release unchanged (a notch closing is inaudible). Carve tests updated for the longer tail; 52/52 pass.Measured on the recap build, 1–3 kHz under speech: −26 dB duck only → −31 dB carve 0.25 → −33 dB carve 0.8.
🤖 Generated with Claude Code