rbrooks/Quest-Board

Watch

Fork 0

[Backend] The silence guard can delete a quiet player from a full-length session #425

New issue

Closed

opened 2026-08-27 23:25:20 +00:00 by claude-bot · 2 comments

claude-bot commented

2026-08-27 23:25:20 +00:00

Collaborator

Severity: CRITICAL. Blocks the v4.0.0 merge. Found while verifying #348's acceptance criteria before closing it.

The defect

drop_silent_tracks (app/services/audio_service.py:1305) runs unconditionally on every session (app/tasks/reminder_tasks.py:2158) and drops any track whose _peak_amplitude falls below the floor. That function samples the file rather than scanning it:

per_window = max(1, min(frames // max(1, windows), 16_000))   # capped at 1 second
for i in range(windows):                                       # windows = 32
    wf.setpos(min(frames - 1, (frames // windows) * i))

Reads are capped at 16,000 frames — one second — while the spacing between them scales with file length. So coverage collapses as sessions get longer:

file	window spacing	audio actually read	coverage
60 s	1.9 s	32 s	53% (windows overlap; speech cannot be missed)
4,639 s (77 min, real)	145 s	32 s	0.69%

Stated exactly: the guard only guarantees detection for a speaker whose track contains one contiguous non-silent stretch longer than the window spacing — ~145 s on a 77-minute session. Anything shorter is caught only if a 1-second probe happens to land on it.

Why this is the failure the guard exists to prevent

Sol_Invictus spoke 163 seconds out of 4,639 in the 2026-08-26 session. That is above 145 s in total, but only safe if it was one unbroken block. A quiet player's speech is not one block — it is a few words every several minutes, which is precisely the distribution this sampling cannot see.

The consequence is silent: the track is dropped before transcription, the player vanishes from the transcript, the summary is generated without them, and the pipeline reports success. The GM is told nothing.

The test does not test this

test_one_brief_utterance_is_enough_to_keep_a_track (tests/test_silent_tracks.py:76) cites this exact production incident in its docstring — "Sol_Invictus in the 2026-08-26 session spoke for 163 seconds out of 4639" — and then builds a 60-second file. At that length windows are 1.9 s apart and 1 s wide, so coverage is total and the assertion cannot fail regardless of the bug.

It is green, and it validates nothing. Same shape as the double corrected in 6ba6202: a test that models an impossible case will eventually assert that the impossible is fine.

Not the bot-side guard

The bot's own drop logic is sound and is not in scope here. It uses speech_bytes_written() — a running counter of decoded audio accumulated during capture (bot/questboard_bot/cogs/recording.py:296) — against a 0.5 s floor, so it has no sampling blind spot, and it reports dropped users through uncaptured_member_ids. bot/tests/test_recording.py:1006 covers the 163-of-4639 case properly.

This issue is only about the backend's second line of defence, which is currently more dangerous than the thing it defends against.

Acceptance criteria

Detection does not degrade with file length — either scan proportionally, or use a measure that does not depend on where probes land (accumulated energy, or reuse the VAD that already runs)
A test at realistic session length (60–90 min) with speech scattered as many short utterances, not one contiguous block, and it must fail against the current implementation before it passes against the fix
A dropped track reaches the GM through the same uncaptured_member_ids / attendance surface the bot-side path uses, rather than a log line
#348's fourth criterion — "GM can see which participants were present but never captured" — holds for backend-dropped tracks too

Notes

VAD already computes real per-track speech duration on the VAD path (SPEAKER_MISSING_MIN_SPEECH_SECONDS, audio_service.py:1246). Reusing that measure would remove the sampling question rather than tune it.
Consider whether this guard should run at all on tracks the current bot uploaded, given the bot already applies a sounder check. Its stated purpose is legacy recordings and version-skew, but it runs on everything.
Found by verifying #348 rather than by a test or an incident, which is an argument for the acceptance-criteria pass being worth its cost.

**Severity: CRITICAL. Blocks the v4.0.0 merge.** Found while verifying #348's acceptance criteria before closing it. ## The defect `drop_silent_tracks` (`app/services/audio_service.py:1305`) runs unconditionally on every session (`app/tasks/reminder_tasks.py:2158`) and drops any track whose `_peak_amplitude` falls below the floor. That function samples the file rather than scanning it: ```python per_window = max(1, min(frames // max(1, windows), 16_000)) # capped at 1 second for i in range(windows): # windows = 32 wf.setpos(min(frames - 1, (frames // windows) * i)) ``` Reads are capped at 16,000 frames — **one second** — while the *spacing* between them scales with file length. So coverage collapses as sessions get longer: | file | window spacing | audio actually read | coverage | |---|---|---|---| | 60 s | 1.9 s | 32 s | **53%** (windows overlap; speech cannot be missed) | | 4,639 s (77 min, real) | **145 s** | 32 s | **0.69%** | Stated exactly: **the guard only guarantees detection for a speaker whose track contains one contiguous non-silent stretch longer than the window spacing** — ~145 s on a 77-minute session. Anything shorter is caught only if a 1-second probe happens to land on it. ## Why this is the failure the guard exists to prevent Sol_Invictus spoke **163 seconds out of 4,639** in the 2026-08-26 session. That is above 145 s *in total*, but only safe if it was one unbroken block. A quiet player's speech is not one block — it is a few words every several minutes, which is precisely the distribution this sampling cannot see. The consequence is silent: the track is dropped before transcription, the player vanishes from the transcript, the summary is generated without them, and the pipeline reports success. The GM is told nothing. ## The test does not test this `test_one_brief_utterance_is_enough_to_keep_a_track` (`tests/test_silent_tracks.py:76`) cites this exact production incident in its docstring — *"Sol_Invictus in the 2026-08-26 session spoke for 163 seconds out of 4639"* — and then builds a **60-second** file. At that length windows are 1.9 s apart and 1 s wide, so coverage is total and the assertion cannot fail regardless of the bug. It is green, and it validates nothing. Same shape as the double corrected in `6ba6202`: a test that models an impossible case will eventually assert that the impossible is fine. ## Not the bot-side guard The bot's own drop logic is sound and is not in scope here. It uses `speech_bytes_written()` — a running counter of decoded audio accumulated during capture (`bot/questboard_bot/cogs/recording.py:296`) — against a 0.5 s floor, so it has no sampling blind spot, and it reports dropped users through `uncaptured_member_ids`. `bot/tests/test_recording.py:1006` covers the 163-of-4639 case properly. This issue is only about the backend's second line of defence, which is currently more dangerous than the thing it defends against. ## Acceptance criteria - [ ] Detection does not degrade with file length — either scan proportionally, or use a measure that does not depend on where probes land (accumulated energy, or reuse the VAD that already runs) - [ ] A test at realistic session length (60–90 min) with speech **scattered** as many short utterances, not one contiguous block, and it must fail against the current implementation before it passes against the fix - [ ] A dropped track reaches the GM through the same `uncaptured_member_ids` / attendance surface the bot-side path uses, rather than a log line - [ ] #348's fourth criterion — "GM can see which participants were present but never captured" — holds for backend-dropped tracks too ## Notes - VAD already computes real per-track speech duration on the VAD path (`SPEAKER_MISSING_MIN_SPEECH_SECONDS`, `audio_service.py:1246`). Reusing that measure would remove the sampling question rather than tune it. - Consider whether this guard should run at all on tracks the current bot uploaded, given the bot already applies a sounder check. Its stated purpose is legacy recordings and version-skew, but it runs on everything. - Found by verifying #348 rather than by a test or an incident, which is an argument for the acceptance-criteria pass being worth its cost.

claude-bot added this to the v4.0.0 - Session Pipeline Accuracy milestone

2026-08-27 23:25:23 +00:00

claude-bot added the

backend

label

2026-08-27 23:25:24 +00:00

claude-bot referenced this issue

2026-08-27 23:26:09 +00:00

[Recording] Guard against re-submitting audio for a session that already has GM-edited content #397

claude-bot referenced this issue

2026-08-27 23:27:47 +00:00

[Backend] The audio retry endpoint disarms both duration guards #421

claude-bot referenced this issue

2026-08-27 23:28:53 +00:00

[Backend] /ask is unprotected on every provider — no context budget, no num_ctx, no truncation check #426

rbrooks referenced this issue from a commit

2026-08-28 00:22:01 +00:00

fix(backend): scan for speech instead of sampling for it (#425)

rbrooks referenced this issue from a commit

2026-08-28 00:22:01 +00:00

fix(backend): put the overwrite guard where the overwrite happens (#397)

claude-bot commented

2026-08-28 00:22:15 +00:00

Author

Collaborator

Fixed in aa0b6c9.

has_audible_speech reads the file sequentially and stops at the first sample above the floor. No sampling, so no length-dependent degradation — and faster in the case that matters, since anyone who speaks ends the loop as soon as they do. Only a genuinely silent track is read to the end, which is the one file worth being certain about.

The bug was worse than this issue described

I wrote it up as "coverage collapses on long files", which is true but understates it. Measuring the old implementation against realistic speech shapes, survival came down to whether a player's speech happened to align with the probe positions:

speech shape	old guard
40 utterances × 4s — 163s total	kept
20 × 4s — 80s total	DROPPED
10 × 3s — 30s total	DROPPED
5 × 2s — 10s total	kept
3 × 2s — 6s total	kept
8 × 1s — 8s total	DROPPED
one 0.5s word in an hour	DROPPED

Eighty seconds of speech deleted while six seconds survives. Not monotonic in how much someone spoke, and not a threshold anyone chose — an alignment artefact between two evenly-spaced sequences. Sol_Invictus surviving the real session was luck.

I nearly shipped the same broken test

My first replacement used 40 utterances of 4 seconds at real session length. It looks like the harder case, and it passes — but so does the old code, because evenly-spaced speech resonates with evenly-spaced probes. A test built from it would have passed before and after the fix and proved nothing, which is exactly what the test this issue criticises did.

I caught it by rebuilding the old implementation and running the candidate fixtures against it, rather than assuming a longer file was automatically a better test. The shapes that ship (20 × 4s, 8 × 1s, one word late in an hour) are ones measured to fail before the fix.

Criteria

Detection does not degrade with file length
A test at realistic session length with scattered speech, verified to fail against the old implementation
A dropped track reaches the GM through the attendance surface
#348's fourth criterion holds for backend-dropped tracks

The two GM-visibility criteria are not done and I am not closing this. They are the same problem as #424 — the pipeline knowing something the GM cannot see — and better solved there than bolted on here. The data-loss half, which is what made this a merge blocker, is fixed.

1,227 passing before, 1,233 after.

Fixed in `aa0b6c9`. `has_audible_speech` reads the file sequentially and stops at the first sample above the floor. No sampling, so no length-dependent degradation — and *faster* in the case that matters, since anyone who speaks ends the loop as soon as they do. Only a genuinely silent track is read to the end, which is the one file worth being certain about. ## The bug was worse than this issue described I wrote it up as "coverage collapses on long files", which is true but understates it. Measuring the old implementation against realistic speech shapes, survival came down to whether a player's speech happened to align with the probe positions: | speech shape | old guard | |---|---| | 40 utterances × 4s — 163s total | kept | | **20 × 4s — 80s total** | **DROPPED** | | **10 × 3s — 30s total** | **DROPPED** | | 5 × 2s — 10s total | kept | | 3 × 2s — 6s total | kept | | **8 × 1s — 8s total** | **DROPPED** | | **one 0.5s word in an hour** | **DROPPED** | **Eighty seconds of speech deleted while six seconds survives.** Not monotonic in how much someone spoke, and not a threshold anyone chose — an alignment artefact between two evenly-spaced sequences. Sol_Invictus surviving the real session was luck. ## I nearly shipped the same broken test My first replacement used 40 utterances of 4 seconds at real session length. It *looks* like the harder case, and it passes — but so does the old code, because evenly-spaced speech resonates with evenly-spaced probes. A test built from it would have passed before and after the fix and proved nothing, which is exactly what the test this issue criticises did. I caught it by rebuilding the old implementation and running the candidate fixtures against it, rather than assuming a longer file was automatically a better test. The shapes that ship (20 × 4s, 8 × 1s, one word late in an hour) are ones **measured to fail before the fix**. ## Criteria - [x] Detection does not degrade with file length - [x] A test at realistic session length with scattered speech, verified to fail against the old implementation - [ ] A dropped track reaches the GM through the attendance surface - [ ] #348's fourth criterion holds for backend-dropped tracks The two GM-visibility criteria are **not** done and I am not closing this. They are the same problem as #424 — the pipeline knowing something the GM cannot see — and better solved there than bolted on here. The data-loss half, which is what made this a merge blocker, is fixed. 1,227 passing before, 1,233 after.

claude-bot referenced this issue

2026-08-28 00:22:27 +00:00

[Recording] Guard against re-submitting audio for a session that already has GM-edited content #397

rbrooks referenced this issue from a commit

2026-08-28 16:47:37 +00:00

fix(backend): tell the GM when the backend drops a silent speaker (#425)

claude-bot commented

2026-08-28 16:47:54 +00:00

Author

Collaborator

Verified against acceptance criteria. Two were met, two were not — the second pair is fixed in 540b0a2.

Criteria

Detection does not degrade with file length — has_audible_speech scans sequentially and stops at the first audible sample. No probing, no spacing that grows with the file. It is also faster in the case that matters: anyone who speaks at all ends the loop when they do, and only a genuinely silent track is read to the end.
A test at realistic session length with speech scattered as many short utterances — three of them, on 4,639-second files: test_a_quiet_player_survives_a_full_length_session, test_speech_scattered_as_single_seconds_survives, test_a_single_word_late_in_a_long_session_is_still_found.

And they do fail against the old implementation. I did not take the docstrings' word for it — I restored the pre-fix sampler (32 windows, reads capped at 16,000 frames) into audio_service and re-ran the file: those three fail, the other twelve pass. Given this issue exists because test_one_brief_utterance_is_enough_to_keep_a_track cited a real incident and then built a 60-second file, that check seemed worth doing rather than assuming.
→ [x] A dropped track reaches the GM through the same uncaptured_member_ids / attendance surface — was not met. Fixed in 540b0a2.
→ [x] #348's fourth criterion holds for backend-dropped tracks too — was not met. Same fix.

What was missing

drop_silent_tracks returned its silent list into a logger.info and nothing else (reminder_tasks.py:2207-2213). uncaptured_member_ids was built solely from presence.json (reminder_tasks.py:2454-2463), which carries only the members the bot declined to upload.

So the backend guard was half a guard. It stopped Whisper inventing speech under a quiet person's name — #348's concern — and then let that person disappear from the transcript with a worker log as the only trace, which is this issue's concern. The GM gets a summary written without them and a pipeline reporting success.

The gap sat exactly where the guard is most needed: drop_silent_tracks exists for recordings the current bot did not produce — an older session, a hand-placed directory, a bot a version behind — and those are precisely the cases with no bot-side report to fall back on.

Track.owner_id is already the Discord user id, so the fix is a merge, not new plumbing. Merged rather than assigned, mirroring the bot's own merge at recording.py:975: the two paths reach the same state differently and either can find someone the other did not.

Extracted as merge_uncaptured_members rather than left inline — process_audio has no test harness, and building one to assert four lines would have been disproportionate.

Note for #348

Its first criterion — "_peak_amplitude scans the full file, not the first 0.5 s" — is unmet as worded: it still samples five 64 KB windows (recording.py:374-400). But it is no longer the drop decision. That now uses speech_bytes_written(), an exact running counter with no sampling blind spot, and _peak_amplitude survives only as a log diagnostic — its docstring says so. The letter is unmet; the intent is superseded by something stronger. Flagging rather than quietly ticking it.

1,296 → 1,300 passing, lint clean. Closing.

**Verified against acceptance criteria. Two were met, two were not — the second pair is fixed in `540b0a2`.** ## Criteria - [x] **Detection does not degrade with file length** — [`has_audible_speech`](webapp/backend/app/services/audio_service.py#L1358-L1389) scans sequentially and stops at the first audible sample. No probing, no spacing that grows with the file. It is also *faster* in the case that matters: anyone who speaks at all ends the loop when they do, and only a genuinely silent track is read to the end. - [x] **A test at realistic session length with speech scattered as many short utterances** — three of them, on 4,639-second files: `test_a_quiet_player_survives_a_full_length_session`, `test_speech_scattered_as_single_seconds_survives`, `test_a_single_word_late_in_a_long_session_is_still_found`. **And they do fail against the old implementation.** I did not take the docstrings' word for it — I restored the pre-fix sampler (32 windows, reads capped at 16,000 frames) into `audio_service` and re-ran the file: those three fail, the other twelve pass. Given this issue exists *because* `test_one_brief_utterance_is_enough_to_keep_a_track` cited a real incident and then built a 60-second file, that check seemed worth doing rather than assuming. - [ ] → [x] **A dropped track reaches the GM through the same `uncaptured_member_ids` / attendance surface** — **was not met.** Fixed in `540b0a2`. - [ ] → [x] **#348's fourth criterion holds for backend-dropped tracks too** — **was not met.** Same fix. ## What was missing `drop_silent_tracks` returned its silent list into a `logger.info` and nothing else ([reminder_tasks.py:2207-2213](webapp/backend/app/tasks/reminder_tasks.py#L2207-L2213)). `uncaptured_member_ids` was built **solely** from `presence.json` ([reminder_tasks.py:2454-2463](webapp/backend/app/tasks/reminder_tasks.py#L2454-L2463)), which carries only the members the *bot* declined to upload. So the backend guard was half a guard. It stopped Whisper inventing speech under a quiet person's name — #348's concern — and then let that person disappear from the transcript with a worker log as the only trace, which is this issue's concern. The GM gets a summary written without them and a pipeline reporting success. The gap sat exactly where the guard is most needed: `drop_silent_tracks` exists for recordings the current bot did not produce — an older session, a hand-placed directory, a bot a version behind — and those are precisely the cases with no bot-side report to fall back on. `Track.owner_id` is already the Discord user id, so the fix is a merge, not new plumbing. Merged rather than assigned, mirroring the bot's own merge at [recording.py:975](bot/questboard_bot/cogs/recording.py#L975): the two paths reach the same state differently and either can find someone the other did not. Extracted as `merge_uncaptured_members` rather than left inline — `process_audio` has no test harness, and building one to assert four lines would have been disproportionate. ## Note for #348 Its first criterion — *"`_peak_amplitude` scans the full file, not the first 0.5 s"* — is **unmet as worded**: it still samples five 64 KB windows ([recording.py:374-400](bot/questboard_bot/cogs/recording.py#L374-L400)). But it is no longer the drop decision. That now uses `speech_bytes_written()`, an exact running counter with no sampling blind spot, and `_peak_amplitude` survives only as a log diagnostic — its docstring says so. The letter is unmet; the intent is superseded by something stronger. Flagging rather than quietly ticking it. 1,296 → 1,300 passing, lint clean. Closing.

claude-bot referenced this issue

2026-08-28 16:48:10 +00:00

[Backend] Drop all-silence tracks before transcription #348

claude-bot closed this issue

2026-08-28 16:48:13 +00:00

claude-bot referenced this issue

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

[Backend] #425's uncaptured-member fix is overridden — a backend-dropped silent speaker is still proposed as "spoke" #432

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

rbrooks referenced this issue from a commit