fix(interruption): keep the overlap armed when agent speech restarts mid-interrupt - #2117
Conversation
🦋 Changeset detectedLatest commit: 696f954 The changes in this PR will be included in the next version bump. This PR includes changesets to release 39 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![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: 37bb370aaf
ℹ️ 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".
| // Deliberately no overlap-state gate here: whether a slice belongs to an overlap is decided | ||
| // once, upstream, when the slice is cut. Re-reading the flag after the await above would | ||
| // drop audio the pipeline already committed to, since `overlap-speech-ended`, | ||
| // `agent-speech-ended` and `bargein_detected` can all clear it in that window. Late | ||
| // responses are harmless — handleMessage() ignores anything outside an open overlap. |
There was a problem hiding this comment.
When a reconnect stalls this transform, the first overlap can end before its committed slice is sent. If a second overlap begins before the server answers that late request, handleMessage() sees the new overlap's global overlapSpeechStarted flag and accepts the old created_at even though its cache entry was cleared, potentially emitting a stale bargein_detected verdict that falsely interrupts the agent during the second overlap. Associate requests with an overlap generation or discard responses whose request no longer belongs to the active overlap.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, but pre-existing and not caused by this change — tracked separately as #2119 rather than fixed here.
handleMessage() is byte-identical to main across this PR and #2117 (git diff origin/main..HEAD -- ws_transport.ts touches only transform()). Response→overlap attribution has always been "is some overlap open right now", and the send gate never protected it: the misattribution you describe also happens for a slice that was sent perfectly legitimately inside its own overlap, purely from response latency —
- overlap A open, slice sent at
T; - user pauses,
overlap-speech-endedclears the flag; - user resumes,
overlap-speech-startedopens overlap B and clears the cache; - the response for
Tlands,state.overlapSpeechStartedistrue, and it is accepted as overlap B's verdict.
No reconnect needed, and the gap between two overlaps in one agent turn is routinely a few hundred ms (see the existing keeps making requests across repeated short overlaps in one agent turn test).
You're also right that cache.clear() is not a filter — setOrUpdate() recreates the entry — so I can't fix this by requiring existing !== undefined either. That would be actively wrong: BoundedCache holds 10 entries and can evict a live request during a long overlap, which would suppress real interruptions.
What this PR does add is the hook for the real fix: slices now carry overlapGeneration (see the separate commit addressing the request-accounting half of this), so sendAudioData() can stamp the generation onto the entry and handleMessage() can reject responses from a closed generation. That needs a store surviving cache.clear(), which is a bigger change than belongs in a bugfix for the send boundary — hence #2119.
One correction on scope: this PR does make one new variant reachable — a slice parked on an in-flight reconnect and sent after its overlap closed — but a reconnect only happens on updateOptions(), so that path is far rarer than the latency path that already exists on main.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
37bb370 to
7159bb7
Compare
…mid-interrupt `overlapSpeechStarted` is the gate that lets a user's overlapping audio reach the interruption model, and the only thing that raises it is a VAD start-of-speech. Two events cleared it while the user was still talking. Because VAD never re-announces speech already under way, nothing could re-arm it for the rest of the agent turn: every remaining frame was dropped, no inference request was made, and the agent talked straight through the interruption. A transport failover rebuilds InterruptionStreamBase from scratch — all of its state lives in a setupTransform() closure, unlike Python where the equivalent flags are instance attributes a reconnect leaves alone. Replaying `agent-speech-started` restored only half of it. Hand the in-progress overlap to the replacement stream instead, via a distinct `agent-speech-resumed` sentinel so the real one keeps meaning "new turn, reset everything". A second speech segment in one turn (a queued SpeechHandle, or the reply after a tool call) raises `agent-speech-started` again with no `agent-speech-ended` in between, because onPipelineReplyDone only reports the end once the speech queue drains. Preserve an open overlap across that; a genuine new turn still resets the overlap, audio buffer, cache and counters. Also marks the forwarding task's rejection handled at creation: it rejects as soon as the stream is torn down, but the `finally` only attaches a handler after the retry backoff, so the rejection surfaced as an unhandled rejection. Co-authored-by: Cursor <cursoragent@cursor.com>
7159bb7 to
696f954
Compare
|
Superseded by #2126, which carries this change in full (both halves: the multi-segment guard and the Verified there before and after: on Closing in favour of #2126. |
…gment starts One agent turn can span several speech segments: a reply that produces tool calls skips _on_end_of_agent_speech, so the follow-up tool-response reply (or a queued say()) raises _AgentSpeechStartedSentinel again with no end sentinel in between. _forward_data treated every start sentinel as a new turn and reset the stream state, clearing _overlap_started while the user was still mid-interrupt. From that point the interruption was silently swallowed: _overlap_started gates audio forwarding to the bargein gateway, and only a fresh VAD speech onset - which never comes for speech already under way - can raise it again, so no verdict was ever emitted and the agent talked straight through the user. Skip the reset when the stream already believes the agent is speaking and an overlap is open, treating the sentinel as a continuation of the same turn. A genuine new turn (an end sentinel was seen) still wipes the overlap, the cache and the counters. Ports the stream-side guard from livekit/agents-js#2117 (the transport-failover half of that PR does not apply here: these flags live on self and _run() reconnects in place on the same stream object). Fixes livekit#6548
|
Redirect: this fix now lives in #2131 (stack 2/5, based on #2130). It was folded into #2126 for a while; that consolidation is closed and split back apart, because bundling five independent defects made each one hard to judge. #2131 carries both mechanisms — the transport-failover |
1 participant
The gate
overlapSpeechStartedininterruption_stream.tsis what lets a user's overlapping audio reach the interruption model at all:The only thing that ever raises it is an
overlap-speech-startedsentinel, which comes from a VAD start-of-speech. VAD does not re-announce speech that is already under way. So if the flag is cleared while the user is mid-interrupt, nothing can re-arm it for the rest of that agent turn: every remaining frame is dropped,numRequestsstays 0, nooverlapping_speechevent is emitted, and the agent talks straight through the interruption.Two things cleared it without the user's turn having ended.
1. Transport failover rebuilds the stream and loses the overlap
createInterruptionTaskbuilds a brand newInterruptionStreamBaseon every retry, and all of that class's state (agentSpeechStarted,overlapSpeechStarted,overlapCount,startIdx,cache) lives in asetupTransform()closure. Python keeps the equivalent flags as instance attributes and only reconnects the socket inside_run(), so they survive:The existing JS mitigation replayed
agent-speech-startedafter a retry, which restores only half the picture — the agent is known to be speaking again, but the overlap is disarmed, and that sentinel's handler additionally wipes the cache and counters.This PR hands the in-progress overlap to the replacement stream through a new internal
agent-speech-resumedsentinel. A separate sentinel (rather than teachingagent-speech-startedabout resumes) keeps the real one meaning exactly what it says: a new agent turn began, reset everything.Any retryable transport error reaches this path — a socket
errorevent or a server{"type":"error"}frame both become retryableAPIErrors — so an ordinary transient gateway blip during an interruption is enough.2. A second speech segment in the same turn is treated as a new turn
One user-perceived agent turn routinely contains several speech segments: a queued
SpeechHandle, or the reply that follows a tool call.AgentActivity.onPipelineReplyDoneonly reportsonEndOfAgentSpeechonce the speech queue has drained, and the tool-call path moves straight tothinkingwithout reporting it at all:So the second segment raises
agent-speech-startedwith noagent-speech-endedin between, and the handler reset an overlap the user was in the middle of. An open overlap is now carried across; everything else is unchanged.Note this second one is shared with Python, whose
_AgentSpeechStartedSentinelhandler calls_reset_state()unconditionally. This is a deliberate divergence, and worth porting back.Preserving the legitimate reset
The reset is skipped only when the stream already believes the agent is speaking and an overlap is open. Every other case — a first segment, a segment after
agent-speech-ended, a resume after a paused false interruption — still clears the overlap, audio buffer, cache and counters exactly as before.resets overlap state on a new agent turnpins that, and passed both before and after the change.Also
forwardTaskwas a floating promise. It rejects the moment its stream is torn down, but thefinallythat attaches a handler only runs after the 2s retry backoff, so the rejection was reported as an unhandled rejection. It is now marked handled at creation; thefinallystill awaits and logs it.Test plan
Four tests added to
interruption_pipeline.test.ts, driving real 48 kHz audio throughAudioRecognitioninto the mocked gateway.Verified red before the fix, and verified each test is coupled to its own change by reverting them individually:
keeps sending user audio after the transport fails over mid-overlapkeeps sending user audio when a second speech segment starts mid-overlap,still reports a bargein after a second speech segment starts mid-overlapThe
resets overlap state on a new agent turnguard test passes in every configuration.pnpm build— exit 0pnpm vitest run agents/src/inference/interruption agents/src/voice— 55 files, 564 tests passingpnpm lint— no new findings on changed filespnpm format:checkpnpm api:check— fails identically onmain(pre-existingexport * as ___issue, 13 unrelated packages). No public API change here:interruption_stream.tsis not re-exported from any package index.Honest scope
This is a plausible mechanism that matches a user report of adaptive interruption always classifying interruptions as backchannel, not a confirmed root cause of it — no live reproduction was available. Both mechanisms above produce the user's primary symptom (the agent never stops speaking, and their audio never reaches the gateway) and are reproduced deterministically here. Neither, however, produces an emitted
overlapping_speechevent carryingnumRequests: 0, which is what that report cited: theoverlap-speech-endedhandler only emits whenoverlapSpeechStartedis true, so these paths emit no event at all. If the reporter's instrumentation genuinely showed emitted events withnumRequests: 0, there is at least one more mechanism still unaccounted for.Related: #2118 tracks
remote_session.tsdroppingprobability/probabilities/numRequestswhen mapping toAgentSessionEvent_OverlappingSpeech, which is why distributed-agent users cannot see these diagnostics at all.Made with Cursor