Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused barge-in fix. The premise is present on current main: tools/tts_tool.py:3611 performs an unbounded prefetch-semaphore acquire, while the playback worker blocks on chunk_queue.get() (tools/tts_tool.py:3532) and cleanup waits for it before signaling completion (tools/tts_tool.py:3763-3775). The stop-aware polling in this PR addresses those waits without adding a new configuration or tool surface.
Problems
tests/tools/test_tts_streaming.py:649only proves three producer generators started. It does not prove the consumer reached the fourth blocked semaphore acquire beforestop.set(), so the regression can pass without exercising the reported failure.
Suggested changes
- Add a deterministic fourth-acquire synchronization point before setting the stop event, then retain the prompt
doneassertion.
Automated hermes-sweeper review.
| daemon=True, | ||
| ) | ||
| consumer.start() | ||
| assert three_prefetches_started.wait(timeout=2.0) |
There was a problem hiding this comment.
This event proves only that three prefetch generators started. The consumer can be descheduled before it reaches the fourth _prefetch_sem.acquire(), allowing stop.set() to make the test pass without exercising the blocked-capacity path. Please synchronize on entry to the fourth acquire (or an equivalent observable) before interrupting.
Adversarial verification — Vox Lockin lane 01 (barge-in echo class)Verified head Live probe
Composition with the echo guard (#75792)#75792 touches Note (out of scope, cross-referenced)The No changes needed. Good fix — the test asserts the actual unblock behavior, not a mocked seam. |
Verification comment (Vox Lockin lane 10 — adversarial check)Verified this PR against current Premise confirmed on main:
Checked against this PR's head
CI: all required checks pass (run 30737994064), mergeable_state CLEAN. Relation to #40010 (Stop TTS on PTT): this is the piece that makes PTT-stop actually responsive during a stalled upstream — the TUI already wires Composition note (lane 10): orthogonal to my truncation work (#78234, touches the same file but different hunks — sentence splitting vs. queue protocol). No conflicts expected. |
What does this PR do?
The per-sentence streaming TTS prefetch pipeline from #71084, salvaged onto current main by #76623, caps in-flight requests with a semaphore and plays each bounded chunk queue in order. If three provider iterators stop yielding, the fourth sentence blocks forever in
Semaphore.acquire(). Barge-in only setsstop_event, so it cannot release that wait; the playback worker can also remain blocked inchunk_queue.get(), and cleanup waits up to 300 seconds while the old pipeline keeps the audio device.This makes every prefetch-capacity and playback-queue wait interruption-aware. Normal completion still drains and joins all prefetches. On barge-in, Hermes abandons stalled upstream iterators, closes playback promptly, sets the done event, and allows the next voice turn to start. Full bounded queues are drained just enough to let producer threads terminate instead of leaking.
Related Issue
No issue filed. Follow-up to #71084 / #76623; open and closed PRs/issues were searched for this behavior.
Fixes #
Type of Change
Changes Made
tools/tts_tool.py: make prefetch semaphore, chunk-queue writes, playback reads, and interrupted cleanup responsive tostop_event.tests/tools/test_tts_streaming.py: reproduce three stalled prefetches plus the blocked fourth sentence and prove barge-in completes promptly.How to Test
stop_eventwhile sentence four is waiting for prefetch capacity.tts_done_eventremains unset; on this branch it is set within one second and the consumer exits.Regression proof:
Focused validation:
The deselected test is an existing Windows-only clock-resolution assertion in
test_hybrid_prefetch_fires_http_immediately; the focused test and all other voice/TTS tests pass.Checklist
Code
pytest tests/ -qand all tests pass - focused voice/TTS suites were run insteadDocumentation & Housekeeping
cli-config.yaml.exampleupdate - N/A; no config keys changedCONTRIBUTING.md/AGENTS.mdupdate - N/AScreenshots / Logs
See the main-vs-branch regression proof and validation output above.