fix(voice): stop dropping turns whose playback starts after audio forwarding completes (combines #1910 + #1960)#1966
Conversation
…warding completes Combines #1910 (false-interruption pause before first frame, #1909) and #1960 (deferred lk.playback_started from DataStream avatar outputs) into a single change: the PLAYBACK_STARTED listener moves to performAudioForwarding so it outlives the forwarding task, forwardAudio no longer settles firstFrameFut, and all reply paths (pipeline, realtime, and say()) settle a still-pending future via settleFirstFrameFut once the playout window ends. A reported non-zero playback position on interruption is also honored as evidence of partial playback. Co-authored-by: enrique <enrique.espaillat@gydehealth.ai> Co-authored-by: Jared Friedman <jared@ycombinator.com> Co-authored-by: Cursor <cursoragent@cursor.com>
🦋 Changeset detectedLatest commit: 4de626d The changes in this PR will be included in the next version bump. This PR includes changesets to release 35 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (speechHandle.interrupted) { | ||
| replyAbortController.abort(); | ||
| await cancelAndWait(tasks, AgentActivity.REPLY_TASK_CANCEL_TIMEOUT); | ||
| if (audioOutput) { | ||
| audioOutput.clearBuffer(); | ||
| await audioOutput.waitForPlayout(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔍 The say() path lacks the new partial-playback-position heuristic
The ttsTask method (used by session.say()) at agents/src/voice/agent_activity.ts:2551-2558 handles interruption by aborting and clearing the buffer, but unlike the pipeline (forwardSegment at line 2925-2928) and realtime (processOneMessage at line 3493-3496) paths, it does NOT check startedForwardingAt or playbackPosition > 0 as a fallback for detecting partial playback when firstFrameFut hasn't resolved. This means the say() path still relies solely on firstFrameFut resolving (via the late PLAYBACK_STARTED listener) to enter the 'speaking' state and record metrics. For avatar outputs where the playback-started RPC races with interruption, say() may still miss the speaking-state transition. This is a pre-existing gap that the PR's new heuristic doesn't cover for this path, though the say() path always commits to chat context regardless (line 2560-2583), so the dropped-turn symptom doesn't apply here.
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| @@ -2895,7 +2915,18 @@ export class AgentActivity implements RecognitionHooks { | |||
| if (audioOutput) { | |||
| audioOutput.clearBuffer(); | |||
| const interruptedPlaybackEv = await audioOutput.waitForPlayout(); | |||
There was a problem hiding this comment.
should we use something like
// generation.ts — performAudioForwarding(): record the baseline
const out: _AudioOut = {
audio: [], firstFrameFut: new Future(), _hasCapturedOwnFrame: false,
capturedSegmentsBefore: audioOutput.capturedPlayoutSegments,
};
// agent_activity.ts — the interrupted branch (both the segment and say paths)
- const forwardedAudio = output.audioOut?.startedForwardingAt !== undefined;
+ const playedOwnFrame =
+ output.audioOut !== null &&
+ audioOutput.capturedPlayoutSegments > output.audioOut.capturedSegmentsBefore;
if ((firstFrameFut.done && !firstFrameFut.rejected) ||
(playedOwnFrame && playbackEv.playbackPosition > 0)) {
output.played = 'partial'; // now only when a frame really played
...
}basically forwarding check is replaced with captured check
There was a problem hiding this comment.
I can do it, but it will no longer have parity match with python code-level, tho I think its fine.
…ment count startedForwardingAt is set before AudioOutput.captureFrame resolves, so a frame that bails at a pause/interrupt gate (e.g. ParticipantAudioOutput while paused) still looked like playback evidence, letting a stale playback position from a previous segment commit a never-heard reply. Record the output's capturedPlayoutSegments when forwarding is set up and only trust a non-zero playback position when the count has since increased — the same condition under which waitForPlayout returns this segment's event rather than a stale one. Suggested by @chenghao-mou in #1966 review. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
2 participants
Summary
This PR combines #1910 (@enriqueespaillat-gyde) and #1960 (@snowmaker) into a single change. Both PRs fix the same root cause —
forwardAudiorejectingfirstFrameFut(and detaching itsPLAYBACK_STARTEDlistener) whenever forwarding finished before playback started — but they edit the exact same lines ingeneration.tsandagent_activity.tsand would conflict whichever lands first. Rather than asking one author to rebase over the other, we consolidated both fixes here to simplify coordination. Full credit to both authors: the commit carriesCo-authored-bytrailers for each.The two scenarios fixed
Both hit the same window — no frame has played by the time forwarding finishes — and both ended with the turn silently removed from the chat context (phantom utterance) while the agent never entered the
speakingstate:waitPlaybackStart: true(e.g. LemonSlice) accept frames faster than real time and send thelk.playback_startedRPC ~1s after forwarding already finished; the future was rejected before the RPC arrived.What this PR does
Takes #1910's structure as the base (it covers all three reply paths, including
say()), plus #1960's playback-position evidence and avatar-shaped tests:PLAYBACK_STARTEDlistener moves toperformAudioForwardingso it outlives the forwarding task;forwardAudiono longer settlesfirstFrameFut; a sharedAgentActivity.settleFirstFrameFuthelper settles a still-pending future once the playout window ends in all three reply paths —forwardSegment(pipeline),processOneMessage(realtime), andttsTask(say(), in atry/finallyso the listener can't leak on the shared output even if playout throws).DeferredStartOutputinagent_activity_interrupted_commit.test.ts) and the stale-playback-position guard test._AudioOut._hasCapturedOwnFrame(initialized in one place, so noundefinedwindow), and a listener-lifecycle regression test asserting the shared output returns to zeroPLAYBACK_STARTEDlisteners after both the resolve and reject paths settle.Python parity note: this mirrors
livekit/agents(see #5039), where the playback-started listener is owned by the caller and only the reply task settles the first-frame future after playout.Replaces
performAudioForwarding+ caller-side settle helper +say()coverage) is the backbone of this change.We opened this combined PR because the two originals conflict line-for-line and neither is a strict subset of the other; landing them separately would force a painful rebase on whoever goes second. Both of you are credited as co-authors on the commit. Please take a look and flag anything that misrepresents your original intent.
Fixes #1909.
Test plan
pnpm build:agentspassesvitest run src/voice/generation_interrupt_before_first_frame.test.ts src/voice/generation_tts_timeout.test.ts src/voice/agent_activity_interrupted_commit.test.ts— 14/14 passsrc/voicesuite: only pre-existingamd.test.tsfailures (also fail on pristinemain)waitPlaybackStart: trueavatar (LemonSlice) — interrupt mid-reply, confirm the partial turn stays in chat contextMade with Cursor