rbrooks/Quest-Board

Watch

Fork 0

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

New issue

Closed

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

claude-bot commented

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

Contributor

Summary

merge_uncaptured_members correctly adds the owner of a backend-dropped silent track to uncaptured_member_ids — and the value is then discarded. Because the speakers dict is built from all discovered WAVs and is never narrowed after drop_silent_tracks, that same owner is still counted as a captured speaker, gets AttendanceProposalSource.spoke assigned first, and the in_channel_silent assignment is skipped.

The GM sees someone who never made a sound proposed as having spoken — the opposite of what #425 set out to fix.

Verified mechanism

speakers is built from every discovered WAV, before silent tracks are dropped:
webapp/backend/app/tasks/reminder_tasks.py:2150-2153 — speakers = {wav.stem: resolved_speaker_name(...) for wav in wav_files}.
Grepped every assignment: the only other one is speakers = {} at :2135. It is never narrowed after drop_silent_tracks.
merge_uncaptured_members(uncaptured_member_ids, silent_tracks) at :2490-2492 does its job correctly — the dropped owner is added to the list.
Both are passed to generate_attendance_proposals at :2496-2502.

In webapp/backend/app/services/attendance_service.py:212-229:

speaker_ids = list(speakers.keys())
...
for discord_id in speaker_ids:
    uid = discord_to_user.get(discord_id)
    if uid is not None:
        proposed[uid] = AttendanceProposalSource.spoke

for discord_id in uncaptured_member_ids:
    uid = discord_to_user.get(discord_id)
    if uid is not None and uid not in proposed:      # ← never true for this owner
        proposed[uid] = AttendanceProposalSource.in_channel_silent

The dropped owner is in speaker_ids, so proposed[uid] is already spoke by the time the second loop runs.

This directly contradicts the stated intent in the comment three lines above the call site (reminder_tasks.py:2485-2489) and in merge_uncaptured_members' own docstring (app/services/audio_service.py:1345-1350).

Blast radius

Only the backend-drop route is affected — an older recording, a hand-placed directory, or a bot running a version behind the backend. The bot's own pre-upload drops (#348) never appear in speakers at all, so those still resolve correctly.

That is precisely the route drop_silent_tracks exists for, and precisely the case where the bot's own report is absent — which is the reason #425 was opened.

How this got through

The #425 work added merge_uncaptured_members and tested that function in isolation, where it behaves correctly. Nothing exercised the composition of merge_uncaptured_members with generate_attendance_proposals against an un-narrowed speakers dict, so the override was invisible. A guard that was never watched fire, and it doesn't.

Suggested fix

Narrow speakers to the surviving tracks after drop_silent_tracks — the dict exists to describe captured speakers, and after the drop the dropped owners are no longer that. Alternatively, let uncaptured_member_ids win over spoke in generate_attendance_proposals, since a member explicitly reported as uncaptured is better evidence than their mere presence in the file list.

Narrowing the dict is preferable: it fixes the cause rather than the symptom, and speakers is also used for the unmatched display-name pass at attendance_service.py:231+, which has the same wrong input today.

Acceptance criteria

A session containing a full-length, entirely silent track whose owner is a linked campaign member proposes that member as in_channel_silent, not spoke.
The regression test is run against the current behaviour first and confirmed to fail there.
The bot-reported uncaptured path (#114) still resolves to in_channel_silent as before.
A speaker who genuinely spoke is still proposed as spoke.
The unmatched display-name pass is checked against the same narrowing.

## Summary `merge_uncaptured_members` correctly adds the owner of a backend-dropped silent track to `uncaptured_member_ids` — and the value is then discarded. Because the `speakers` dict is built from **all** discovered WAVs and is never narrowed after `drop_silent_tracks`, that same owner is still counted as a captured speaker, gets `AttendanceProposalSource.spoke` assigned first, and the `in_channel_silent` assignment is skipped. The GM sees someone who never made a sound proposed as having spoken — the opposite of what #425 set out to fix. ## Verified mechanism 1. `speakers` is built from every discovered WAV, **before** silent tracks are dropped: `webapp/backend/app/tasks/reminder_tasks.py:2150-2153` — `speakers = {wav.stem: resolved_speaker_name(...) for wav in wav_files}`. Grepped every assignment: the only other one is `speakers = {}` at `:2135`. It is never narrowed after `drop_silent_tracks`. 2. `merge_uncaptured_members(uncaptured_member_ids, silent_tracks)` at `:2490-2492` does its job correctly — the dropped owner **is** added to the list. 3. Both are passed to `generate_attendance_proposals` at `:2496-2502`. 4. In `webapp/backend/app/services/attendance_service.py:212-229`: ```python speaker_ids = list(speakers.keys()) ... for discord_id in speaker_ids: uid = discord_to_user.get(discord_id) if uid is not None: proposed[uid] = AttendanceProposalSource.spoke for discord_id in uncaptured_member_ids: uid = discord_to_user.get(discord_id) if uid is not None and uid not in proposed: # ← never true for this owner proposed[uid] = AttendanceProposalSource.in_channel_silent ``` The dropped owner is in `speaker_ids`, so `proposed[uid]` is already `spoke` by the time the second loop runs. This directly contradicts the stated intent in the comment three lines above the call site (`reminder_tasks.py:2485-2489`) and in `merge_uncaptured_members`' own docstring (`app/services/audio_service.py:1345-1350`). ## Blast radius Only the **backend-drop** route is affected — an older recording, a hand-placed directory, or a bot running a version behind the backend. The bot's own pre-upload drops (#348) never appear in `speakers` at all, so those still resolve correctly. That is precisely the route `drop_silent_tracks` exists for, and precisely the case where the bot's own report is absent — which is the reason #425 was opened. ## How this got through The #425 work added `merge_uncaptured_members` and tested that function in isolation, where it behaves correctly. Nothing exercised the composition of `merge_uncaptured_members` with `generate_attendance_proposals` against an un-narrowed `speakers` dict, so the override was invisible. A guard that was never watched fire, and it doesn't. ## Suggested fix Narrow `speakers` to the surviving tracks after `drop_silent_tracks` — the dict exists to describe captured speakers, and after the drop the dropped owners are no longer that. Alternatively, let `uncaptured_member_ids` win over `spoke` in `generate_attendance_proposals`, since a member explicitly reported as uncaptured is better evidence than their mere presence in the file list. Narrowing the dict is preferable: it fixes the cause rather than the symptom, and `speakers` is also used for the `unmatched` display-name pass at `attendance_service.py:231+`, which has the same wrong input today. ## Acceptance criteria - [ ] A session containing a full-length, entirely silent track whose owner is a linked campaign member proposes that member as `in_channel_silent`, not `spoke`. - [ ] The regression test is run against the **current** behaviour first and confirmed to fail there. - [ ] The bot-reported uncaptured path (#114) still resolves to `in_channel_silent` as before. - [ ] A speaker who genuinely spoke is still proposed as `spoke`. - [ ] The `unmatched` display-name pass is checked against the same narrowing.

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

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

2026-08-29 00:26:25 +00:00

fix(backend): never delete raw audio inline (#427, #432)

claude-bot commented

2026-08-29 00:26:57 +00:00

Author

Contributor

Fixed in d4881b9 on fix/v4.0.1-retention-safety, alongside #427 (same call path).

The fix

Narrowing extracted as narrow_speakers_to_captured, placed directly beside merge_uncaptured_members in audio_service, and called from process_audio right after drop_silent_tracks.

Extracting it rather than inlining the dict comprehension was deliberate. The reason this defect survived a release is that the missing step would have lived only inside process_audio — a task no test drives end to end. A step that exists only inline in an untested task is a step nothing can catch. As a named function with its own tests, it is reachable; as three lines in the middle of a 400-line task, it was not.

The two functions are now documented as a pair, with each docstring pointing at the other and explaining that changing one without the other is exactly what happened here.

Tests

Three unit tests on the pair, including the invariant that actually matters:

assert not set(narrowed) & set(uncaptured)

A dropped owner must leave one collection and enter the other, and appear in neither both nor neither. That overlap is the defect — stated as an invariant rather than as two separate assertions that could both pass while the composition stayed broken.

Plus two service-level tests in test_attendance_proposals.py. The second one is unusual and worth flagging: it asserts the unfixed behaviour of generate_attendance_proposals — that passing a dropped owner in both arguments still resolves to spoke. That pins why the narrowing lives in the caller. If someone later "fixes" it a second time by making uncaptured win inside that function, this test fails and points them at the caller-side narrowing rather than leaving two competing fixes in the codebase.

Acceptance criteria

A session containing a full-length, entirely silent track whose owner is a linked campaign member proposes that member as in_channel_silent, not spoke
The regression test was run against the current behaviour first and confirmed to fail there
The bot-reported uncaptured path (#114) still resolves to in_channel_silent — unchanged, since those members never had a WAV and so were never in speakers
A speaker who genuinely spoke is still proposed as spoke
The unmatched display-name pass is checked against the same narrowing — it iterates the same dict, so it now sees the narrowed set, and the test asserts a dropped owner does not surface as an unmatched name either

1,378 backend tests pass.

Fixed in `d4881b9` on `fix/v4.0.1-retention-safety`, alongside #427 (same call path). ## The fix Narrowing extracted as `narrow_speakers_to_captured`, placed directly beside `merge_uncaptured_members` in `audio_service`, and called from `process_audio` right after `drop_silent_tracks`. Extracting it rather than inlining the dict comprehension was deliberate. The reason this defect survived a release is that the missing step would have lived only inside `process_audio` — a task **no test drives end to end**. A step that exists only inline in an untested task is a step nothing can catch. As a named function with its own tests, it is reachable; as three lines in the middle of a 400-line task, it was not. The two functions are now documented as a pair, with each docstring pointing at the other and explaining that changing one without the other is exactly what happened here. ## Tests Three unit tests on the pair, including the invariant that actually matters: ```python assert not set(narrowed) & set(uncaptured) ``` A dropped owner must leave one collection and enter the other, and appear in **neither both nor neither**. That overlap *is* the defect — stated as an invariant rather than as two separate assertions that could both pass while the composition stayed broken. Plus two service-level tests in `test_attendance_proposals.py`. The second one is unusual and worth flagging: it asserts the **unfixed** behaviour of `generate_attendance_proposals` — that passing a dropped owner in both arguments still resolves to `spoke`. That pins *why* the narrowing lives in the caller. If someone later "fixes" it a second time by making `uncaptured` win inside that function, this test fails and points them at the caller-side narrowing rather than leaving two competing fixes in the codebase. ## Acceptance criteria - [x] A session containing a full-length, entirely silent track whose owner is a linked campaign member proposes that member as `in_channel_silent`, not `spoke` - [x] The regression test was run against the current behaviour first and confirmed to fail there - [x] The bot-reported uncaptured path (#114) still resolves to `in_channel_silent` — unchanged, since those members never had a WAV and so were never in `speakers` - [x] A speaker who genuinely spoke is still proposed as `spoke` - [x] The `unmatched` display-name pass is checked against the same narrowing — it iterates the same dict, so it now sees the narrowed set, and the test asserts a dropped owner does not surface as an unmatched name either 1,378 backend tests pass.

claude-bot referenced this issue

2026-08-29 05:58:03 +00:00

v4.0.1 — retention safety and secrets at rest #435

claude-bot referenced this issue from a commit

2026-08-29 06:58:09 +00:00

Merge PR #435: v4.0.1 — retention safety and secrets at rest

claude-bot closed this issue

2026-08-29 07:09:34 +00:00

claude-bot referenced this issue

2026-08-29 18:17:59 +00:00

[Review] Audit the test suite for tests that cannot fail #441

claude-bot referenced this issue