rbrooks/Quest-Board

Watch

Fork 0

[Backend] check_transcript_covers_session destroys a valid transcript when a session has a long silent tail #431

New issue

Closed

opened 2026-08-28 22:16:33 +00:00 by claude-bot · 1 comment

claude-bot commented

2026-08-28 22:16:33 +00:00

Collaborator

Summary

check_transcript_covers_session compares the last segment's end time against the recording's wall clock. Because every track is tail-padded with silence to the full wall clock, that ratio is speech-end ÷ wall-clock — which is not a coverage measure of anything. Any session where play ends before 60% of the elapsed recording time fails, deterministically, and the already-transcribed session is discarded.

Why this is not hypothetical

Nothing stops a recording when the voice channel empties — there is no voice_state_update listener in the recording cog. The only backstop is max_recording_hours, which defaults to 6 (bot/questboard_bot/config.py:39, scheduled at bot/questboard_bot/cogs/recording.py:589). A GM who forgets to /record stop is the expected case, not an exotic one.

Concrete triggers:

2 h recording, 50 min of it after play ended → speech ends at 58% → fails
4 h wall clock that was 2 h of play plus a forgotten stop → exactly 50% → fails

Verified mechanism

Every track is tail-padded to the reported wall clock with exact zeros — rec.sink.close(pad_to=float(duration_s)) at bot/questboard_bot/cogs/recording.py:843, via _write_silence (recording.py:197, :316-341). Track length ≡ wall clock, independent of when speech stopped.
duration_s = int(time.monotonic() - rec.started_at) — pure wall clock (recording.py:768).
On retry/reprocess the denominator cannot shrink: derive_session_duration returns the longest WAV (webapp/backend/app/services/audio_service.py:1421-1448), which is the padded full length. Called at app/routers/admin.py:214 and app/routers/sessions.py:948.
check_transcript_covers_session (app/services/audio_service.py:1477-1496) computes last_end = max(s["end"] for s in all_segments) and raises when last_end < 0.6 * duration_seconds. Trailing padded silence contributes nothing to last_end.

Why the cost is high

It runs at app/tasks/reminder_tasks.py:2284 — after transcribe_with_optional_vad (:2267), and before the merge (:2286) and the TranscriptSegment inserts (:2305).

So at the moment it raises, the GPU work is already paid for and the segments are correct and correctly attributed. The exception path sets audio_processing_status = failed and stores the message; the complete transcript is thrown away. Retrying reproduces it exactly, because the duration is re-derived from the same padded tracks. There is no force-skip flag — the function short-circuits only on duration_seconds <= 600 or empty segments, and all three process_audio call sites pass a real duration.

The error message also asserts a specific wrong cause — "The per-speaker tracks are likely not on a shared session clock" — sending whoever reads it into the capture code, which is not where the problem is.

The counter-argument, and why it fails

The docstring calls this "belt and braces behind check_tracks_cover_session" (audio_service.py:1483). The case it uniquely catches is "tracks are full length but each on its own clock". Since #320, capture derives every track's position from a single _t0 (recording.py:151-156), so the capture code cannot produce that shape.

That is an argument that the guard has low value, not that it is safe. A low-value guard with a high-cost, unrecoverable false positive is the trade worth changing.

Suggested fix

Preferred: compare last_end against the audible extent rather than wall clock. The VAD path already computes per-track speech spans (audio_service.py:1004), so max(span_end) across tracks is free — and it is the number the guard's own error message is pretending to use.

Alternatives:

Gate the raise on "no track has audible speech after 0.6 × duration".
Downgrade to a warning persisted on SummarisationRun. The transcript is complete; the GM can judge. Current behaviour trades a complete transcript for a scary and misleading message.

Acceptance criteria

A recording with a long silent tail (speech ending at ~50% of wall clock, all tracks full length and on a shared clock) processes successfully end to end.
The genuine failure the guard exists for — tracks full length but on mutually incomparable clocks — still raises.
The regression test is run against the current behaviour first and confirmed to fail there.
If the guard still raises in any case, it does so before transcription, or is downgraded so a paid-for transcript is never discarded.
The error message no longer asserts a cause the code has not established.

## Summary `check_transcript_covers_session` compares the last segment's end time against the **recording's wall clock**. Because every track is tail-padded with silence to the full wall clock, that ratio is *speech-end ÷ wall-clock* — which is not a coverage measure of anything. Any session where play ends before 60% of the elapsed recording time fails, deterministically, and the already-transcribed session is discarded. ## Why this is not hypothetical Nothing stops a recording when the voice channel empties — there is no `voice_state_update` listener in the recording cog. The only backstop is `max_recording_hours`, which defaults to **6** (`bot/questboard_bot/config.py:39`, scheduled at `bot/questboard_bot/cogs/recording.py:589`). A GM who forgets to `/record stop` is the expected case, not an exotic one. Concrete triggers: - 2 h recording, 50 min of it after play ended → speech ends at 58% → **fails** - 4 h wall clock that was 2 h of play plus a forgotten stop → exactly 50% → **fails** ## Verified mechanism 1. Every track is tail-padded to the reported wall clock with exact zeros — `rec.sink.close(pad_to=float(duration_s))` at `bot/questboard_bot/cogs/recording.py:843`, via `_write_silence` (`recording.py:197`, `:316-341`). Track length ≡ wall clock, independent of when speech stopped. 2. `duration_s = int(time.monotonic() - rec.started_at)` — pure wall clock (`recording.py:768`). 3. On retry/reprocess the denominator cannot shrink: `derive_session_duration` returns the **longest** WAV (`webapp/backend/app/services/audio_service.py:1421-1448`), which is the padded full length. Called at `app/routers/admin.py:214` and `app/routers/sessions.py:948`. 4. `check_transcript_covers_session` (`app/services/audio_service.py:1477-1496`) computes `last_end = max(s["end"] for s in all_segments)` and raises when `last_end < 0.6 * duration_seconds`. Trailing padded silence contributes nothing to `last_end`. ## Why the cost is high It runs at `app/tasks/reminder_tasks.py:2284` — **after** `transcribe_with_optional_vad` (`:2267`), and **before** the merge (`:2286`) and the `TranscriptSegment` inserts (`:2305`). So at the moment it raises, the GPU work is already paid for and the segments are correct and correctly attributed. The exception path sets `audio_processing_status = failed` and stores the message; the complete transcript is thrown away. Retrying reproduces it exactly, because the duration is re-derived from the same padded tracks. There is no force-skip flag — the function short-circuits only on `duration_seconds <= 600` or empty segments, and all three `process_audio` call sites pass a real duration. The error message also asserts a specific wrong cause — *"The per-speaker tracks are likely not on a shared session clock"* — sending whoever reads it into the capture code, which is not where the problem is. ## The counter-argument, and why it fails The docstring calls this "belt and braces behind `check_tracks_cover_session`" (`audio_service.py:1483`). The case it uniquely catches is "tracks are full length but each on its own clock". Since #320, capture derives every track's position from a single `_t0` (`recording.py:151-156`), so the capture code cannot produce that shape. That is an argument that the guard has **low value**, not that it is safe. A low-value guard with a high-cost, unrecoverable false positive is the trade worth changing. ## Suggested fix Preferred: compare `last_end` against the **audible extent** rather than wall clock. The VAD path already computes per-track speech spans (`audio_service.py:1004`), so `max(span_end)` across tracks is free — and it is the number the guard's own error message is pretending to use. Alternatives: - Gate the raise on "no track has audible speech after `0.6 × duration`". - Downgrade to a warning persisted on `SummarisationRun`. The transcript is complete; the GM can judge. Current behaviour trades a complete transcript for a scary and misleading message. ## Acceptance criteria - [ ] A recording with a long silent tail (speech ending at ~50% of wall clock, all tracks full length and on a shared clock) processes successfully end to end. - [ ] The genuine failure the guard exists for — tracks full length but on mutually incomparable clocks — still raises. - [ ] The regression test is run against the **current** behaviour first and confirmed to fail there. - [ ] If the guard still raises in any case, it does so **before** transcription, or is downgraded so a paid-for transcript is never discarded. - [ ] The error message no longer asserts a cause the code has not established.

claude-bot added this to the v4.0.1 - Retention Safety & Secrets at Rest milestone

2026-08-28 22:17:07 +00:00

claude-bot referenced this issue

2026-08-28 22:45:09 +00:00

[Evals] Synthetic-audio end-to-end harness for the per-speaker transcription pipeline #433

rbrooks referenced this issue from a commit

2026-08-28 23:16:57 +00:00

fix(backend): stop a silent tail destroying a good transcript (#431)

claude-bot commented

2026-08-28 23:17:13 +00:00

Author

Collaborator

Fixed in 54116c2 on feat/v4-deterministic-attribution.

What changed

check_transcript_covers_session now measures the transcript against the audible extent of the tracks instead of the wall clock, via a new last_audible_second — the mirror of the existing has_audible_speech, scanning backward for the same reason that one scans forward: the answer is usually in the first chunk, and only a long silent tail gets walked back through, which is exactly the measurement being asked for.

Two deliberate choices worth recording:

tracks is required, not optional. Defaulting it would leave a caller silently measuring against the wall clock again — the bug this exists to remove.
Unmeasurable skips rather than falling back. When no track can be measured, the guard logs and returns. Falling back to the clock would reintroduce this through the back door, and the guard is not worth a false positive: the shape it uniquely catches — full-length tracks on mutually incomparable clocks — cannot be produced by capture since #320 placed every track against a single t0.

The error message no longer asserts a cause the code has not established. It now reports the audible extent alongside the timeline end, so the reader is pointed at transcription rather than at capture.

One thing worth noting for the release

v4.0.0 made this worse rather than introducing it. The guard predates the release, but admin retry used to pass duration=0 (admin.py:215 on main), which short-circuited both coverage checks — so a long-tailed session that failed could at least be recovered by retrying. derive_session_duration does not exist on main at all; it arrives in this release via 7b91626 and arms the guard on precisely that recovery path. Without this fix, v4.0.0 would have turned "recoverable" into "fails identically every time".

Verification

The regression fixture was run against the pre-fix guard body, taken verbatim from git, before the fix was trusted:

	result
Old guard, silent-tail fixture	raises — "ends at 2950s of a 7200s recording (41% covered)"
New guard, same fixture	passes — 2950s of 3000s audible is 98% covered
New guard, genuinely truncated transcript	still raises — 30% of audible audio covered

That third row was the one worth checking. Widening a guard's denominator is an easy way to disable it by accident, and a suite full of green tests would not have noticed.

Seven new tests in test_duration_guards.py, covering both directions (a silent tail must not destroy a good transcript; a silent tail must not blind the guard to a transcript that genuinely stops short of the speech that is there), plus unit coverage on last_audible_second — including that speech below _SILENT_TRACK_PEAK does not count as audible, using the same threshold as drop_silent_tracks so a track nothing would transcribe cannot extend the window a transcript is expected to cover.

1,369 backend tests pass (up from 1,362), ruff format and check clean, check_version_sync.py OK. CHANGELOG entry added under 4.0.0 and confirmed extractable by re-running the release workflow's own regex against it.

Acceptance criteria

A recording with a long silent tail processes successfully end to end
The genuine failure the guard exists for still raises
The regression test was run against the current behaviour first and confirmed to fail there
A paid-for transcript is no longer discarded on a false positive
The error message no longer asserts a cause the code has not established

Fixed in `54116c2` on `feat/v4-deterministic-attribution`. ## What changed `check_transcript_covers_session` now measures the transcript against the **audible extent of the tracks** instead of the wall clock, via a new `last_audible_second` — the mirror of the existing `has_audible_speech`, scanning backward for the same reason that one scans forward: the answer is usually in the first chunk, and only a long silent tail gets walked back through, which is exactly the measurement being asked for. Two deliberate choices worth recording: - **`tracks` is required, not optional.** Defaulting it would leave a caller silently measuring against the wall clock again — the bug this exists to remove. - **Unmeasurable skips rather than falling back.** When no track can be measured, the guard logs and returns. Falling back to the clock would reintroduce this through the back door, and the guard is not worth a false positive: the shape it uniquely catches — full-length tracks on mutually incomparable clocks — cannot be produced by capture since #320 placed every track against a single `t0`. The error message no longer asserts a cause the code has not established. It now reports the audible extent alongside the timeline end, so the reader is pointed at transcription rather than at capture. ## One thing worth noting for the release v4.0.0 made this **worse** rather than introducing it. The guard predates the release, but admin retry used to pass `duration=0` (`admin.py:215` on `main`), which short-circuited both coverage checks — so a long-tailed session that failed could at least be recovered by retrying. `derive_session_duration` does not exist on `main` at all; it arrives in this release via `7b91626` and arms the guard on precisely that recovery path. Without this fix, v4.0.0 would have turned "recoverable" into "fails identically every time". ## Verification The regression fixture was run against the **pre-fix guard body, taken verbatim from git**, before the fix was trusted: | | result | |---|---| | Old guard, silent-tail fixture | **raises** — "ends at 2950s of a 7200s recording (41% covered)" | | New guard, same fixture | passes — 2950s of 3000s audible is 98% covered | | New guard, genuinely truncated transcript | **still raises** — 30% of audible audio covered | That third row was the one worth checking. Widening a guard's denominator is an easy way to disable it by accident, and a suite full of green tests would not have noticed. Seven new tests in `test_duration_guards.py`, covering both directions (a silent tail must not destroy a good transcript; a silent tail must not blind the guard to a transcript that genuinely stops short of the speech that is there), plus unit coverage on `last_audible_second` — including that speech below `_SILENT_TRACK_PEAK` does not count as audible, using the same threshold as `drop_silent_tracks` so a track nothing would transcribe cannot extend the window a transcript is expected to cover. **1,369 backend tests pass** (up from 1,362), ruff format and check clean, `check_version_sync.py` OK. CHANGELOG entry added under 4.0.0 and confirmed extractable by re-running the release workflow's own regex against it. ## Acceptance criteria - [x] A recording with a long silent tail processes successfully end to end - [x] The genuine failure the guard exists for still raises - [x] The regression test was run against the current behaviour first and confirmed to fail there - [x] A paid-for transcript is no longer discarded on a false positive - [x] The error message no longer asserts a cause the code has not established

claude-bot closed this issue

2026-08-28 23:17:19 +00:00

claude-bot referenced this issue

2026-08-28 23:26:29 +00:00

v4.0.0 — deterministic attribution: the summary is composed from checked events, not asserted #434

claude-bot referenced this issue

2026-08-29 07:57:13 +00:00

[Backend] Fail processing when per-speaker track durations do not match the recorded session length #324

claude-bot referenced this issue

2026-08-29 18:18:19 +00:00

[Review] Document every feature, and validate each doc against actual behaviour #442

rbrooks referenced this issue from a commit