Repository navigation
Ring: reading never goes past a drop, so a Dropout's silence lands where the audio was lost (#734) - #774
Merged
Conversation
…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
enabled auto-merge (squash)
October 16, 2026 05:51
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #734. Not only a flaky test: a real misplacement in the recorder.
Cause.
TakeWriter.drainOnceasksring.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 nextring.consume, the read goes past the drop's position and the drop's silence is written after that audio. IndropoutInTheListthat 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.consumestops at the next unconsumed logged drop. It loadswrittenfirst 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 withtaken200 instead of 100, then green.pendingDropDoesNotRestrictandmanyDropsInTurn(past the log's wraparound) passed on first run; they pin the other cases.fullLogmodeled 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.shpasses. A Code Reviewer checked the ordering argument, stalls, and other callers (only TakeWriter callsconsume); 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