Skip to content

Ring: reading never goes past a drop, so a Dropout's silence lands where the audio was lost (#734) - #774

Merged
Skeyelab merged 1 commit into
mainfrom
fix/734-ring-drop-position
Oct 16, 2026
Merged

Skeyelab merged 1 commit into
mainfrom
fix/734-ring-drop-position

Conversation

@Skeyelab

Copy link
Copy Markdown
Member

Closes #734. Not only a flaky test: a real misplacement in the recorder.

Cause. TakeWriter.drainOnce asks ring.nextDrop() whether a drop is ready. A drop is not ready until audio has been stored after it, so it returns nil. If that audio arrives before the next ring.consume, the read goes past the drop's position and the drop's silence is written after that audio. In dropoutInTheList that put the Dropout Marker at 5280 frames (0.11 s) instead of 4800 (0.1 s), only when the writer thread happened to run in that window. In a real Take the Stems carry the silence and the audio in the wrong order.

Fix. SampleRing.consume stops at the next unconsumed logged drop. It loads written first and the drop log second (both acquire), so a drop that comes before audio it sees is also seen: the producer logs a drop before it stores later audio. The producer and the real-time path are untouched (ADR 0002). Cost: at most one extra 5 ms writer sleep per drop.

Tests. Written red first: readStopsAtTheDrop (deterministic, no timing) failed with taken 200 instead of 100, then green. pendingDropDoesNotRestrict and manyDropsInTurn (past the log's wraparound) passed on first run; they pin the other cases. fullLog modeled a reader that ignores drops, so it now lets the drops pile up (same intent, new numbers). I did not widen any tolerance. Full package suite passes (1012 tests); VERIFY_QUICK=1 ./scripts/verify.sh passes. A Code Reviewer checked the ordering argument, stalls, and other callers (only TakeWriter calls consume); no blockers.

Tag gate. This changes the ring on the real-time audio path's consumer side. Per the tag gate it needs a real-device Take (with a deliberate Dropout if possible) from a TestFlight build before External testers. Not checked on a device: only tests.

https://claude.ai/code/session_01Wx6Spsz6BcTiKDzUUP9CzN

…ere the audio was lost (#734)

TakeWriter asks the ring whether a drop is ready, then reads. A drop is not ready until
audio is stored after it, so when that audio arrived between the two steps the read ran
past the drop and the silence went in after it: the Dropout Marker sat at 0.11 s, not
0.1 s (the MarkerListTests.dropoutInTheList flake), and the Stems carried the silence
and the audio in the wrong order. SampleRing.consume now stops at the next logged drop.

Written red first (readStopsAtTheDrop, deterministic: no timing). fullLog modeled a reader
that ignores drops, so it now lets the drops pile up instead; same intent.

Claude-Session: https://claude.ai/code/session_01Wx6Spsz6BcTiKDzUUP9CzN
Assisted-by: Claude Code:claude-sonnet-5-5
@Skeyelab
Skeyelab enabled auto-merge (squash) October 16, 2026 05:51
@codecov

codecov Bot commented Oct 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Skeyelab
Skeyelab merged commit c6aa479 into main Oct 16, 2026
11 checks passed
Skeyelab added a commit that referenced this pull request Oct 16, 2026
The interruption banner naming its cause (#779), the Dropout silence landing where the audio was lost (#774), and the launch price under Buy Pro (#777), plus everything from 1.1.10, which hasn't gone to external testers yet.

Claude-Session: https://claude.ai/code/session_01K4ihr1i6m1uuUswAENvJKJ
Assisted-by: Claude Code:claude-opus-5-5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky: MarkerListTests.dropoutInTheList, Dropout Marker at 0.11 s instead of 0.1 s (Release)

1 participant