Commit cf37ca4
authored
harness-followup(mofa-podcast): preserve per-segment WAVs on success (#59)
* harness-followup(mofa-podcast): preserve per-segment WAVs on success
Refactor `SegmentDirCleanup` from "always wipe on Drop" to
"wipe-on-failure-only" so the octos harness can run a future
`PerFileNonSilent` validator against `<output_dir>/segments/seg_*.wav`
after `generate_podcast` returns. A TTS dropout on one segment is
otherwise padded to silence and slips past the whole-file
`AudioNonSilent` check, leaving the user with a mid-conversation
dropout.
Contract:
- Success path (Ok return): `mark_success()` flips an AtomicBool on
the guard; Drop becomes a no-op and dialogue/placeholder WAVs
survive.
- Failure path (Err return, panic, SIGTERM cancel): flag stays false
and Drop wipes the scratch directory — matches the prior behaviour
so failed runs don't leak partial state.
Filename shape is unchanged (`seg_{NNN}_{voice}.wav` for dialogue;
`pause_after_*`, `pause_line_*`, `bgm_placeholder_line_*` for the
intentionally-silent placeholders) so the future octos validator can
glob `**/segments/seg_*.wav` and skip placeholders.
Tests: +5 unit / integration-style tests around the cleanup contract
(default-wipe, mark-success preserve, idempotency, missing-dir
graceful, full happy-path file shape). Also stabilises three
pre-existing flaky tests that shared a second-resolution timestamp
with each other and could collide under parallel `cargo test`.
Manifest bumped 0.4.5 -> 0.4.6 so the octos harness can key
`PerFileNonSilent` on it once the octos-side PR lands.
cargo check / cargo clippy -- -D warnings / cargo test all green
(matches CI). The `--all-targets` clippy errors are pre-existing on
origin/main and out of scope.
* mofa-podcast: wipe stale segments at run start; correct cancel docs
Address codex review P1s on #59:
P1.1 — Stale-segment reuse (functional regression introduced by this
PR's preserve-on-success behaviour). Previously the always-wipe Drop
guaranteed `<output_dir>/segments/` started empty. Now that the
success path preserves files, a subsequent run reusing the same
output directory could pick up a stale `seg_NNN_voice.wav` from a
prior successful run via the assembly loop's `seg_path.exists()`
check (line ~1640) when its own TTS call fails for that slot.
Failure would be silently masked: errors are recorded but the
consistency check `assembled_dialogue_segments != dialogue_count`
passes against stale audio.
Fix: clear `<output_dir>/segments/` at the start of every
`generate_podcast` invocation, before constructing the guard.
Combined with the in-run failure-path Drop, the directory only ever
holds files from the current run on disk after return.
P1.2 — Documentation inaccuracy. Both `check_cancel()` and the
SIGTERM handler call `std::process::exit(130)`, which bypasses RAII
Drop. The PR's comments overclaimed that Drop runs on SIGTERM —
corrected to explicitly note partial state may survive a cancel
until the next run wipes on entry (long-standing pre-existing
behaviour, out of scope to refactor here).
Test coverage:
- +1 unit/integration: `generate_podcast_wipes_segments_directory_on_error_return`
plants a stale file, calls `generate_podcast` with an Err path, and
asserts both the start-of-run wipe and Err-path Drop removed it.
Integration shell: `scripts/test-integration.sh` 5.5 inverted —
previously asserted `segments/` was cleaned after success; now
asserts `seg_*.wav` are preserved (matching the harness contract).
cargo check / clippy / test / fmt all green; 54 tests (was 53 in
the first commit on this branch).1 parent f28ae0b commit cf37ca4
5 files changed
Lines changed: 327 additions & 21 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
226 | 226 | | |
227 | 227 | | |
228 | 228 | | |
229 | | - | |
230 | | - | |
231 | | - | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
232 | 235 | | |
233 | | - | |
| 236 | + | |
| 237 | + | |
234 | 238 | | |
235 | 239 | | |
236 | 240 | | |
| |||
0 commit comments