[Recording] Replace the capture tests that assert the broken clock model #326

New issue

Closed

opened 2026-08-25 20:38:45 +00:00 by claude-bot · 1 comment

claude-bot commented

2026-08-25 20:38:45 +00:00

Contributor

Severity: HIGH. Found in the August 2026 session lifecycle review (#319).

The defect

The bot's own tests currently lock in the bug. bot/tests/test_recording.py:534-548 (test_sink_late_joiner_gets_leading_silence) asserts that a late joiner is padded to the first speaker's frame count — which is precisely the broken behaviour. bot/tests/test_recording.py:564-581 builds a "timeline" of three packets and calls it 60 ms.

Any correct fix makes these tests fail. They have to be rewritten as part of the fix, not after it, or the fix looks like a regression.

Proposed fix — the invariant test that should have existed

Make the sink testable by injecting a clock: PerUserPCMSink(out_dir, session_id, *, started_at: float, clock=time.monotonic). Everything below is pure-unit — no Discord, no network — extending the existing _FakeDecoder pattern at bot/tests/test_recording.py:492-506, and runs in the current CI matrix. This matters because per docs and the deploy notes the dev stack has no Discord token and cannot exercise the bot end to end at all.

Core test — test_track_duration_equals_wall_clock: fake clock; user A speaks 0-1 s and again 60-61 s; user B first speaks at 120 s; close() at 300 s. Assert both WAVs are 300 s within 300 ms, that A has speech bytes at the 60 s offset, that A is pure silence at 30 s, and that B is silent for its first 119 s.

This fails against today's code in three independent ways: A's file is ~2 s not 300 s, B's leading pad equals A's byte count rather than 120 s, and nothing tail-pads.

Companion tests:

Leave and rejoin — spurts at 0-10 s and 200-210 s produce speech at exactly those offsets
Reference-independence — activity from B alone advances B's track with zero packets from A. This one permanently kills the _session_bytes semantics.
DTX-only user — track is padded to full length and flagged or dropped per the all-silence policy
Frame alignment — a computed pad is always a multiple of 4 bytes

Acceptance criteria

test_sink_late_joiner_gets_leading_silence and the 60 ms timeline test are rewritten, not deleted silently
PerUserPCMSink accepts an injectable clock
The core invariant test exists and passes with the fix, and demonstrably fails without it
All four companion tests exist
Tests run in the existing CI matrix with no Discord dependency

**Severity: HIGH.** Found in the August 2026 session lifecycle review (#319). ## The defect The bot's own tests currently **lock in** the bug. `bot/tests/test_recording.py:534-548` (`test_sink_late_joiner_gets_leading_silence`) asserts that a late joiner is padded to the first speaker's *frame count* — which is precisely the broken behaviour. `bot/tests/test_recording.py:564-581` builds a "timeline" of three packets and calls it 60 ms. Any correct fix makes these tests fail. They have to be rewritten as part of the fix, not after it, or the fix looks like a regression. ## Proposed fix — the invariant test that should have existed Make the sink testable by injecting a clock: `PerUserPCMSink(out_dir, session_id, *, started_at: float, clock=time.monotonic)`. Everything below is pure-unit — no Discord, no network — extending the existing `_FakeDecoder` pattern at `bot/tests/test_recording.py:492-506`, and runs in the current CI matrix. This matters because per `docs` and the deploy notes the dev stack has no Discord token and cannot exercise the bot end to end at all. **Core test — `test_track_duration_equals_wall_clock`:** fake clock; user A speaks 0-1 s and again 60-61 s; user B first speaks at 120 s; `close()` at 300 s. Assert both WAVs are 300 s within 300 ms, that A has speech bytes at the 60 s offset, that A is pure silence at 30 s, and that B is silent for its first 119 s. This fails against today's code in three independent ways: A's file is ~2 s not 300 s, B's leading pad equals A's byte count rather than 120 s, and nothing tail-pads. **Companion tests:** - Leave and rejoin — spurts at 0-10 s and 200-210 s produce speech at exactly those offsets - Reference-independence — activity from B alone advances B's track with zero packets from A. This one permanently kills the `_session_bytes` semantics. - DTX-only user — track is padded to full length and flagged or dropped per the all-silence policy - Frame alignment — a computed pad is always a multiple of 4 bytes ## Acceptance criteria - [ ] `test_sink_late_joiner_gets_leading_silence` and the 60 ms timeline test are rewritten, not deleted silently - [ ] `PerUserPCMSink` accepts an injectable clock - [ ] The core invariant test exists and passes with the fix, and demonstrably fails without it - [ ] All four companion tests exist - [ ] Tests run in the existing CI matrix with no Discord dependency

claude-bot added this to the v3.11.5 - Recording Clock Hotfix milestone

2026-08-25 20:38:45 +00:00

claude-bot added the

bug

bot

recording

labels

2026-08-25 20:38:45 +00:00

rbrooks referenced this issue from a commit

2026-08-25 22:24:18 +00:00

fix(bot): anchor per-speaker capture to a session wall clock (#320, #321, #322)

claude-bot referenced this issue

2026-08-25 22:29:56 +00:00

v3.11.5 — per-speaker recordings never shared a session clock (#320) #418

claude-bot referenced this issue from a commit

2026-08-25 23:11:52 +00:00

Merge PR #418: v3.11.5 — per-speaker recordings never shared a session clock (#320)

claude-bot commented

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

Author

Contributor

Shipped — closing as part of a v3.11.5 bookkeeping sweep. No issue-number citation in the code, so verified on its merits.

bot/tests/test_recording.py now asserts the corrected clock model rather than the broken one:

test_track_duration_equals_wall_clock
test_sink_cleanup_pads_using_the_clock
test_sink_small_jitter_does_not_pad_inside_speech
test_sink_dtx_packet_writes_one_silence_frame
test_sink_pads_are_frame_aligned

That set covers each property the broken model got wrong: tracks span wall clock rather than talk time, gaps below GAP_THRESHOLD_SECONDS are absorbed instead of padded inside speech, DTX becomes exactly one 20 ms silence frame, pads stay frame-aligned so channels cannot swap, and close(pad_to=…) tail-pads from the clock.

Worth noting this issue was the important half of the hotfix. #320 was invisible for months precisely because the tests encoded the wrong model and passed — replacing them is what makes the fix hold.

Shipped — closing as part of a v3.11.5 bookkeeping sweep. No issue-number citation in the code, so verified on its merits. `bot/tests/test_recording.py` now asserts the **corrected** clock model rather than the broken one: - `test_track_duration_equals_wall_clock` - `test_sink_cleanup_pads_using_the_clock` - `test_sink_small_jitter_does_not_pad_inside_speech` - `test_sink_dtx_packet_writes_one_silence_frame` - `test_sink_pads_are_frame_aligned` That set covers each property the broken model got wrong: tracks span wall clock rather than talk time, gaps below `GAP_THRESHOLD_SECONDS` are absorbed instead of padded inside speech, DTX becomes exactly one 20 ms silence frame, pads stay frame-aligned so channels cannot swap, and `close(pad_to=…)` tail-pads from the clock. Worth noting this issue was the important half of the hotfix. #320 was invisible for months precisely because the tests encoded the wrong model and passed — replacing them is what makes the fix hold.

claude-bot closed this issue

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