rbrooks/Quest-Board

Watch

Fork 0

[Bot] A failure midway through converting raw tracks deletes the tracks not yet converted #520

New issue

Closed

opened 2026-09-06 03:03:28 +00:00 by claude-bot · 1 comment

claude-bot commented

2026-09-06 03:03:28 +00:00

Collaborator

Found while doing #393 (v4.3.0 phase 5), outside that lane's scope; same family as #399.

RecordingCog._process in bot/questboard_bot/cogs/recording.py converts each speaker's raw capture track ({session_id}_{user_id}_raw.16k-mono.s16le) into its WAV and, in a finally: block at the end, unlinks every remaining entry of raw_track_files. The normal stop path deletes each raw track as soon as it is converted, so on success the finally has nothing to do. But if the conversion loop raises partway (disk full, a corrupt track, an exception in the WAV writer), the finally still runs and deletes the raw tracks that were not converted, which is the only copy of that audio. The failure message the GM sees was reworded in #393 so it makes no promise about what is left on disk, but the loss itself still happens.

Expected: a failure in the conversion loop leaves the unconverted raw tracks in place, so #399's startup recovery (which treats leftover raw tracks as the signal that a recording never finished) can convert and hand them off on the next start, or an operator can. The cleanup should only remove tracks that have a WAV to show for them.

Acceptance:

A test that makes conversion fail on the second of three tracks asserts the third raw track still exists afterwards (and the first's WAV does).
The finally (or its replacement) removes only raw tracks whose WAV was written.
recover_interrupted_recordings picks the survivors up on the next start (existing behaviour; add a test if one does not cover this shape).

Found while doing #393 (v4.3.0 phase 5), outside that lane's scope; same family as #399. `RecordingCog._process` in `bot/questboard_bot/cogs/recording.py` converts each speaker's raw capture track (`{session_id}_{user_id}_raw.16k-mono.s16le`) into its WAV and, in a `finally:` block at the end, unlinks every remaining entry of `raw_track_files`. The normal stop path deletes each raw track as soon as it is converted, so on success the `finally` has nothing to do. But if the conversion loop raises partway (disk full, a corrupt track, an exception in the WAV writer), the `finally` still runs and deletes the raw tracks that were **not** converted, which is the only copy of that audio. The failure message the GM sees was reworded in #393 so it makes no promise about what is left on disk, but the loss itself still happens. Expected: a failure in the conversion loop leaves the unconverted raw tracks in place, so #399's startup recovery (which treats leftover raw tracks as the signal that a recording never finished) can convert and hand them off on the next start, or an operator can. The cleanup should only remove tracks that have a WAV to show for them. Acceptance: - [ ] A test that makes conversion fail on the second of three tracks asserts the third raw track still exists afterwards (and the first's WAV does). - [ ] The `finally` (or its replacement) removes only raw tracks whose WAV was written. - [ ] `recover_interrupted_recordings` picks the survivors up on the next start (existing behaviour; add a test if one does not cover this shape).

claude-bot referenced this issue

2026-09-06 03:03:53 +00:00

feat(bot): a self-diagnosing bot, a /record start that knows the session, and linking that names the way out (phase 5 lane B) #521

claude-bot referenced this issue

2026-09-06 03:12:47 +00:00

v4.3.0 build order: Product Surface in eight phases #514

claude-bot referenced this issue

2026-09-06 03:40:30 +00:00

[Bot] Fix /record start's broken session identity, unresponsive stop, and silent data loss on restart #393

claude-bot referenced this issue

2026-09-06 04:22:13 +00:00

v4.3.0 build order: Product Surface in eight phases #514

claude-bot referenced this issue

2026-09-06 09:54:06 +00:00

v4.3.0 build order: Product Surface in eight phases #514

claude-bot referenced this issue

2026-09-12 17:21:48 +00:00

Live recording panel reports every track six times too short and warns "timeline is falling behind" on every session #565

rbrooks referenced this issue from a commit

2026-09-12 17:31:20 +00:00

fix(bot): keep the audio a failed conversion never reached (#520)

claude-bot referenced this issue from a pull request that will close it,

2026-09-12 17:31:36 +00:00

fix(bot): two recording-path defects — false lag warning (#565) and audio lost on a failed conversion (#520) #617

claude-bot commented

2026-09-12 17:32:01 +00:00

Author

Collaborator

Fixed in #617 (branch fix/565-520-recording-path), shared with #565 because both touch cogs/recording.py.

The finally no longer unlinks the outstanding tracks; it logs them and leaves them. But that alone would not have held, and it is worth recording why, because the acceptance criterion "recovery picks the survivors up on the next start" was not true with only that change:

_retry_pending_handoffs runs first (setup_hook, before the gateway is up). The half-converted directory has a manifest, no handed_off, retry defaulting to True, and at least one WAV, so has_captured_audio passes — it would post the partial session, with duration_seconds absent and therefore 0, which is exactly the silent disabling of the coverage guards #421 was about.
_recover_one_recording then sees the HANDOFF_MARKER that retry just wrote and takes its "already the backend's" branch, which calls _drop_raw_tracks. The audio kept by the fix would have been deleted on the next start anyway.

So a half-converted directory is now marked retry=False, which leaves recovery as the only pass that will touch it, and it writes down the two things only the interrupted run knows: the session's true duration_seconds (the surviving raws understate it whenever the longest track was one of the converted ones) and a partial speakers.json (a speaker converted before the failure has no raw track left to be named from — _process only wrote speakers.json after the whole loop, so those names were being lost). _recover_one_recording reads both back and merges rather than overwriting, and marks itself the same way while it is filling the directory, so a recovery that itself dies partway leaves no postable fragment either.

Behaviour change worth flagging: a half-converted session is no longer auto-posted in partial form; it waits for the next bot start. A partial transcript that #397's guard would then refuse to replace seemed the worse outcome, but it is a change in when a failed session reaches the backend.

Tests (all confirmed to fail against the pre-fix source):

conversion failing on the second of three tracks leaves the second and third on disk, with the first's WAV kept and its raw released;
the half-converted directory is left for recovery, not for retry (retry=False, duration and partial speakers recorded);
recovery finishes such a directory without losing the earlier run's speakers or its measured length;
a recovery that dies partway leaves no postable fragment.

Bot suite: 439 passed.

Fixed in #617 (branch `fix/565-520-recording-path`), shared with #565 because both touch `cogs/recording.py`. The `finally` no longer unlinks the outstanding tracks; it logs them and leaves them. But that alone would not have held, and it is worth recording why, because the acceptance criterion "recovery picks the survivors up on the next start" was **not** true with only that change: - `_retry_pending_handoffs` runs first (`setup_hook`, before the gateway is up). The half-converted directory has a manifest, no `handed_off`, `retry` defaulting to `True`, and at least one WAV, so `has_captured_audio` passes — it would post the partial session, with `duration_seconds` absent and therefore `0`, which is exactly the silent disabling of the coverage guards #421 was about. - `_recover_one_recording` then sees the `HANDOFF_MARKER` that retry just wrote and takes its "already the backend's" branch, which calls `_drop_raw_tracks`. The audio kept by the fix would have been deleted on the next start anyway. So a half-converted directory is now marked `retry=False`, which leaves recovery as the only pass that will touch it, and it writes down the two things only the interrupted run knows: the session's true `duration_seconds` (the surviving raws understate it whenever the longest track was one of the converted ones) and a partial `speakers.json` (a speaker converted before the failure has no raw track left to be named from — `_process` only wrote `speakers.json` after the whole loop, so those names were being lost). `_recover_one_recording` reads both back and merges rather than overwriting, and marks itself the same way while it is filling the directory, so a recovery that itself dies partway leaves no postable fragment either. **Behaviour change worth flagging:** a half-converted session is no longer auto-posted in partial form; it waits for the next bot start. A partial transcript that #397's guard would then refuse to replace seemed the worse outcome, but it is a change in *when* a failed session reaches the backend. Tests (all confirmed to fail against the pre-fix source): - conversion failing on the second of three tracks leaves the second and third on disk, with the first's WAV kept and its raw released; - the half-converted directory is left for recovery, not for retry (`retry=False`, duration and partial speakers recorded); - recovery finishes such a directory without losing the earlier run's speakers or its measured length; - a recovery that dies partway leaves no postable fragment. Bot suite: 439 passed.

claude-bot closed this issue

2026-09-12 18:01:10 +00:00

claude-bot referenced this issue from a commit

2026-09-12 18:01:10 +00:00

Merge pull request 'fix(bot): two recording-path defects — false lag warning (#565) and audio lost on a failed conversion (#520)' (#617) from fix/565-520-recording-path into main