Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b08a647f0e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ev.type != SpeechEventType.RECOGNITION_USAGE: | ||
| self._last_event_time = time.perf_counter() |
There was a problem hiding this comment.
When a non-streaming primary is passed, the constructor wraps it in StreamAdapter; that wrapper emits local START_OF_SPEECH and END_OF_SPEECH events before it awaits the real recognize() call (livekit-agents/livekit/agents/stt/stream_adapter.py:127-146). This branch counts those local VAD events as provider activity, so _check_stall can never fire for that utterance if recognize() hangs or returns no transcript, and the documented wrapped-STT fallback leaves the caller muted. Track actual recognition activity for this wrapper rather than its synthetic VAD events.
Useful? React with 👍 / 👎.
| stall_monitor_task = asyncio.create_task(self._stall_monitor_task(vad_stream)) | ||
| try: | ||
| await self._run_streams(vad_stream=vad_stream) |
There was a problem hiding this comment.
If the per-stream VAD fails, VADStream.__anext__ propagates that error (livekit-agents/livekit/agents/vad.py:180-185) and _stall_monitor_task terminates, but _run waits only for _run_streams; later VAD pushes are suppressed as RuntimeErrors. With a connected mute STT, the outer stream then waits indefinitely instead of surfacing the detector failure or failing over, so opting into stall detection is silently ineffective. Wait for or observe both tasks and propagate or handle monitor termination.
Useful? React with 👍 / 👎.
| if vad_stream is not None and isinstance(data, rtc.AudioFrame): | ||
| with contextlib.suppress(RuntimeError): | ||
| vad_stream.push_frame(data) |
There was a problem hiding this comment.
When callers delimit audio with public RecognizeStream.flush(), this condition ignores the _FlushSentinel even though the same sentinel is sent to the main and recovery STTs below. VADStream.flush() is a required hard segment boundary (livekit-agents/livekit/agents/vad.py:155-165), so the detector can carry speaking state across segments; two short segments can be merged past stall_min_speech_duration, and activity from one segment can mask a stall in the next. Forward or otherwise handle the flush boundary in the detector.
Useful? React with 👍 / 👎.
Only transcripts (interim, preflight, final) now count as STT progress: speech boundaries can come from a VAD, e.g. the one of stt.StreamAdapter, before any recognition happened, which hid a stalled recognize call. Forward flush() to the stall detection VAD as a hard segment boundary, like stt.StreamAdapter does, log a failure of the stall monitor (stall detection is then disabled for that stream), and keep a pending stall check alive after end_input() so a stalled STT that stays open doesn't hang the caller. Rename _last_event_time to _last_transcript_time. Addresses review feedback on livekit#7826. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
b08a647 to
8372619
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83726198fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| vad_stream.push_frame(data) | ||
| elif isinstance(data, self._FlushSentinel): | ||
| # a hard segment boundary for the VAD, as in stt.StreamAdapter | ||
| vad_stream.flush() |
There was a problem hiding this comment.
When a caller invokes RecognizeStream.flush() (including via end_input()) while the VAD is still in its speaking state—for example, when an audio clip ends without the detector's minimum trailing silence—this flush hard-resets both shipped VAD implementations without emitting END_OF_SPEECH (livekit-plugins/livekit-plugins-silero/livekit/plugins/silero/vad.py:358-361 and livekit-agents/livekit/agents/inference/vad.py:270-273). Since _stall_monitor_task only arms a check on END_OF_SPEECH, a connected-but-mute primary is never declared stalled in that case and the stream can wait indefinitely; preserve/evaluate the active detected segment at this boundary before resetting it.
Useful? React with 👍 / 👎.
A flush (including the one from end_input()) hard-resets the stall-detection VAD without an END_OF_SPEECH, so an ongoing speech never scheduled a stall check. On a flush, check the ongoing speech as if it ended there, since the STT is flushed too. Use its VAD-reported duration so speech the VAD already dropped can't trigger a later check. Addresses review feedback on livekit#7826. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5362a9fa23
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (speech_start_time := self._speech_start_time) is not None: | ||
| self._speech_start_time = None | ||
| self._schedule_stall_check(speech_start_time, self._speech_duration) |
There was a problem hiding this comment.
When callers enqueue buffered audio and then call flush() or end_input() without yielding, _forward_input_task can drain the unbounded input channel in one turn; vad_stream.push_frame() and vad_stream.flush() only enqueue work, so the monitor has not yet set _speech_start_time when this state is read. The VAD can subsequently emit START/INFERENCE events and then hard-reset on the queued flush without EOS, leaving a mute STT with no scheduled check and a hung stream. This enqueue-versus-monitor race in the revised flush handling is fresh evidence beyond the earlier flush finding; wait for the detector/monitor to consume the boundary before deciding whether to schedule the check.
Useful? React with 👍 / 👎.
| if self._stall_check_task is not None: | ||
| self._stall_check_task.cancel() | ||
| self._stall_check_task = asyncio.create_task(_check_stall()) |
There was a problem hiding this comment.
When two qualifying speech segments end less than stall_timeout apart and the active STT emits nothing, scheduling the second segment cancels the first segment's earlier deadline. Each subsequent retry utterance can therefore restart the timer, allowing a connected-but-mute primary to avoid failover indefinitely while the user keeps saying short phrases; retain the earliest pending unanswered-speech check until a transcript arrives or that deadline fires.
Useful? React with 👍 / 👎.
| SpeechEventType.PREFLIGHT_TRANSCRIPT, | ||
| SpeechEventType.FINAL_TRANSCRIPT, | ||
| ): | ||
| self._last_transcript_time = time.perf_counter() |
There was a problem hiding this comment.
When a provider emits a transcript-typed event with no alternatives or empty text, this updates _last_transcript_time and suppresses failover even though the detected speech produced no usable transcription. This is a concrete path for the built-in inference STT, whose _process_transcript filters empty interim results but still emits empty FINAL_TRANSCRIPT events (livekit-agents/livekit/agents/inference/stt.py:1417-1443); a connected primary returning such finals for long speech can therefore keep the user muted indefinitely. Only count a non-empty alternative as progress, matching the recovery check below.
Useful? React with 👍 / 👎.
- Run the stall detection VAD on a stream per flush segment: ending its input instead of flushing it makes the monitor see the boundary once the VAD processed all the audio before the flush, so a speech it only detects after the flush was forwarded is still checked. Speech starts are clamped to the segment end, as a lagging VAD dates them late. - Keep every pending stall check: a later speech no longer cancels the check of an earlier unanswered one. - Only count transcripts with a non-empty text as STT progress, as empty finals (e.g. of the inference STT) don't show the STT is transcribing. Addresses review feedback on livekit#7826. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 840568f8db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ev.type == VADEventType.START_OF_SPEECH: | ||
| # the VAD only reports speech after some of it was heard, while the STT may | ||
| # already have reacted, so date the speech back to when it began | ||
| speech_start_time = time.perf_counter() - ev.speech_duration |
There was a problem hiding this comment.
When the detector processes audio behind the STT (for example, the shipped Silero path explicitly accumulates slower-than-realtime inference delay in livekit-plugins/livekit-plugins-silero/livekit/plugins/silero/vad.py:454-462), this timestamps the speech as callback time minus only its audio duration, omitting the detector backlog. A valid transcript can therefore be recorded before speech_start_time, causing the check at line 636 to mark a healthy provider stalled. The new per-segment clamp is fresh evidence beyond the earlier flush race: it applies only once a segment has an end_time, and that end can itself postdate a transcript emitted before the flush. Anchor the speech to the segment's audio timeline instead.
Useful? React with 👍 / 👎.
The stall check dated a detected speech from when its START_OF_SPEECH event was consumed minus the speech duration, ignoring the VAD's backlog. A VAD lagging behind the audio (e.g. Silero's slower-than-realtime inference accumulation) dated the speech too late, so a transcript the STT emitted for it earlier didn't count and a healthy STT was failed over. Each VAD segment now records when each frame was pushed against the cumulative audio duration, and a speech starts when the audio at the VAD event's position (timestamp - speech_duration) was pushed. The kept timeline is trimmed as events are consumed. This replaces the clamp to the segment end. The check deadline is unchanged. Addresses review feedback on livekit#7826. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Hi! A note on scope before more review rounds. The review bots have done four rounds on this PR, and each round's fixes have added machinery: a VAD stream per flush segment, per-speech checks, and now an audio-timeline mapping so a lagging VAD dates speech correctly. It's all opt-in ( Before going further I'd like to check the direction with you:
Happy to simplify or close this in favour of a different design. |
| def push_frame(self, frame: rtc.AudioFrame) -> None: | ||
| self.frame_times.append(time.perf_counter()) | ||
| self.audio_duration += frame.duration | ||
| self.frame_ends.append(self.audio_duration) | ||
| self.vad_stream.push_frame(frame) |
There was a problem hiding this comment.
🔴 Silent streams retain unlimited frame history
When a VAD emits no events during silence, push_frame keeps every frame's timing metadata. trim runs only when a VAD event arrives, so a long-lived silent stream grows its history without bound.
Learn more
Stall detection records a wall-clock push time and an audio-timeline end for every incoming frame. A VAD is allowed to omit inference events during silence; the speech-free _FlushCountingVADStream does exactly that. The monitor only calls trim when it receives an event. On an open stream with no event or flush, the two arrays therefore grow for every frame, even though the VAD consumes its input.
Example: A speech-boundary-only VAD receives 100 audio frames each second of an hour-long silent session. No events arrive, so about 360,000 entries remain in each timing list instead of being discarded or compacted.
Recommended fix: Bound or compact the retained timeline even when the VAD emits no events. Preserve enough timing information to date a start-of-speech event that arrives late, without storing one timestamp per frame indefinitely.
Was this helpful? React with 👍 or 👎 to provide feedback.
| i = bisect.bisect_left(self.frame_ends, position) | ||
| return self.frame_times[min(i, len(self.frame_times) - 1)] |
There was a problem hiding this comment.
🟡 Frame-boundary speech misses stall detection
When speech starts at a frame boundary, pushed_at dates it to the preceding frame's push. A transcript between the two pushes then counts for the new speech, masking a silent STT.
Learn more
The timeline stores each frame's end position and wall-clock push time. At a frame boundary, the audio beginning at that position belongs to the next frame, but bisect_left returns the previous frame. The resulting earlier start time makes _schedule_stall_check treat a transcript from before the new speech as its answer.
Example: One frame ends at audio position 1.0s and was pushed at wall time 1.0s. The next frame starts there and is pushed at wall time 1.1s. A transcript at 1.05s precedes that speech, but the check uses 1.0s and considers it answered.
Recommended fix: Use bisect_right to select the next frame when the speech position equals a frame end, retaining the last-frame clamp for positions beyond the stored timeline.
| i = bisect.bisect_left(self.frame_ends, position) | |
| return self.frame_times[min(i, len(self.frame_times) - 1)] | |
| i = bisect.bisect_right(self.frame_ends, position) | |
| return self.frame_times[min(i, len(self.frame_times) - 1)] |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb07e832ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if speech_start_time is not None: | ||
| self._schedule_stall_check(speech_start_time, speech_duration) |
There was a problem hiding this comment.
When a non-streaming primary is auto-wrapped and the caller flushes while the detector still considers a qualifying speech active, StreamAdapterWrapper handles that same boundary by calling vad_stream.flush() and continuing (livekit-agents/livekit/agents/stt/stream_adapter.py:117-121); because a VAD flush hard-resets without emitting END_OF_SPEECH, the wrapper never reaches its _wrapped_stt.recognize() call for that segment. Scheduling a check here therefore labels a healthy wrapped STT unavailable and starts fallback/recovery solely because the wrapper intentionally dropped the segment (the same applies to an explicitly supplied StreamAdapter); skip this flush-derived check for that path or make the wrapper finalize the active segment first.
Useful? React with 👍 / 👎.
A streaming STT can keep its connection open and stop emitting events while audio still flows. The FallbackAdapter only reacted to errors and timeouts, so the user was effectively muted until the provider closed the session. Add an opt-in `stall_timeout` (with `stall_min_speech_duration`) to `stt.FallbackAdapter`. When set, the adapter's VAD runs on the streamed audio, and if the active STT emits no event between the start of a detected speech and `stall_timeout` seconds after its end, the stream fails with a retryable APIConnectionError, so the existing fallback and recovery take over. Disabled by default. Fixes livekit#7693 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Only transcripts (interim, preflight, final) now count as STT progress: speech boundaries can come from a VAD, e.g. the one of stt.StreamAdapter, before any recognition happened, which hid a stalled recognize call. Forward flush() to the stall detection VAD as a hard segment boundary, like stt.StreamAdapter does, log a failure of the stall monitor (stall detection is then disabled for that stream), and keep a pending stall check alive after end_input() so a stalled STT that stays open doesn't hang the caller. Rename _last_event_time to _last_transcript_time. Addresses review feedback on livekit#7826. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A flush (including the one from end_input()) hard-resets the stall-detection VAD without an END_OF_SPEECH, so an ongoing speech never scheduled a stall check. On a flush, check the ongoing speech as if it ended there, since the STT is flushed too. Use its VAD-reported duration so speech the VAD already dropped can't trigger a later check. Addresses review feedback on livekit#7826. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Run the stall detection VAD on a stream per flush segment: ending its input instead of flushing it makes the monitor see the boundary once the VAD processed all the audio before the flush, so a speech it only detects after the flush was forwarded is still checked. Speech starts are clamped to the segment end, as a lagging VAD dates them late. - Keep every pending stall check: a later speech no longer cancels the check of an earlier unanswered one. - Only count transcripts with a non-empty text as STT progress, as empty finals (e.g. of the inference STT) don't show the STT is transcribing. Addresses review feedback on livekit#7826. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The stall check dated a detected speech from when its START_OF_SPEECH event was consumed minus the speech duration, ignoring the VAD's backlog. A VAD lagging behind the audio (e.g. Silero's slower-than-realtime inference accumulation) dated the speech too late, so a transcript the STT emitted for it earlier didn't count and a healthy STT was failed over. Each VAD segment now records when each frame was pushed against the cumulative audio duration, and a speech starts when the audio at the VAD event's position (timestamp - speech_duration) was pushed. The kept timeline is trimmed as events are consumed. This replaces the clamp to the segment end. The check deadline is unchanged. Addresses review feedback on livekit#7826. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bb07e83 to
f8a8dd6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8a8dd626c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if forward_task.done(): | ||
| forward_task.result() # propagate the stream's own failure, if any | ||
| return |
There was a problem hiding this comment.
When end_input() follows a qualifying utterance before its stall deadline, a provider that closes cleanly without emitting text makes forward_task complete, so this branch returns success and _run cancels the VAD monitor and pending checks instead of failing over. This is reachable with shipped streams—for example, Deepgram sends its close message when input ends and treats the resulting expected socket close as a normal return (livekit-plugins/livekit-plugins-deepgram/livekit/plugins/deepgram/stt.py:646-671)—leaving the detected utterance untranscribed and the mute provider marked available; defer successful completion until the detector has concluded that there is no unanswered qualifying speech.
Useful? React with 👍 / 👎.
summary
stall_timeout(andstall_min_speech_duration, default 1.0s) tostt.FallbackAdapterto detect a streaming STT that stays connected but stops transcribing. Today the adapter only reacts to errors/timeouts, so a provider that keeps its WebSocket open but emits nothing mutes the user until it closes the session itself.vadruns on the streamed audio; if the active STT emits no transcript with non-empty text (interim, preflight or final; empty finals, e.g. from the inference STT, and speech boundaries, which can come from a VAD such asstt.StreamAdapter's, don't count;stall_min_speech_durationkeeps short noises out) between the start of a detected speech of at leaststall_min_speech_durationandstall_timeoutseconds after its end, the stream fails with a retryableAPIConnectionError, so the existing fallback, availability events and recovery (which still requires a real final transcript) take over.speech_durationso STT events arriving before the VAD's start-of-speech count; the "last transcript" clock resets on each new leg so speech from before a failover never blames the new STT;flush()is forwarded to the stall detection VAD as a segment boundary; a failure of that VAD is logged and turns off detection for that stream only; a pending check survivesend_input(), so a stalled STT that stays open can't hang the caller.end_input()) during a speech ends it there: the stall VAD runs on a new stream per flush segment and the previous one's input is ended, so the check is made once the VAD processed all the pre-flush audio, never at enqueue time. Cost: onevad.stream()per flush (~9 µs silero, ~29 µs inference; passexecutor=toinference.VADto avoid a thread pool per stream).stall_timeoutafter the VAD reports the end of speech, or after the flush / end of input.None); when off, the stream path is unchanged (no extra task or VAD stream).stall_timeoutafter the user stops talkingtest plan
uv run pytest tests/test_stt_fallback.py(unit, virtual time): new tests cover a connected-but-mute primary failing over to the fallback at EOS + stall_timeout, pure silence (no failover), speech shorter thanstall_min_speech_duration(no failover), a healthy primary transcribing before and after the VAD's end of speech (no failover),stall_timeoutwithout a VAD raisingValueError, speech boundaries alone (no transcript) still failing over,flush()reaching the detector VAD, a VAD failure being logged while transcription continues, and the stall check survivingend_input(); a lagging VAD keeping a healthy STT that emitted an early interim while still failing over a mute one; each new test fails without its fixstall_timeout=None(no transcript reaches the user)ruff format/check; strictmypy -p livekit.agentsFixes #7693
🤖 Generated with Claude Code