-
Notifications
You must be signed in to change notification settings - Fork 3.4k
fix(openai realtime): treat create_response=False as client-side turn taking #6642
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -453,10 +453,23 @@ def __init__( | |
| ) | ||
|
|
||
| modalities = modalities if is_given(modalities) else ["text", "audio"] | ||
| resolved_turn_detection = to_turn_detection(turn_detection) | ||
| if ( | ||
| resolved_turn_detection is not None | ||
| and resolved_turn_detection.create_response is False | ||
| and resolved_turn_detection.interrupt_response is not False | ||
| ): | ||
| logger.warning( | ||
| "create_response=False hands turn taking to the client, but the server still " | ||
| "cancels its response on user speech, pass interrupt_response=False as well" | ||
| ) | ||
|
|
||
| super().__init__( | ||
| capabilities=llm.RealtimeCapabilities( | ||
| message_truncation=True, | ||
| turn_detection=turn_detection is not None, | ||
| # create_response=False leaves the reply to the client: client-side turn taking | ||
| turn_detection=resolved_turn_detection is not None | ||
| and resolved_turn_detection.create_response is not False, | ||
|
longcw marked this conversation as resolved.
|
||
| can_disable_turn_detection=not is_given(turn_detection), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. turn_detection is given when using server_vad (create_response=False, interrupt_response=False,) VAD, but its still client driven turn taking, so I think can_disable_turn_detection should be True not False when create_response=False?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have turn_detection = ServerVad() , i.e. specified, and have client controller turn detection (c.f. on #6635 (comment) "Note — why not LiveKit "manual" turn detection with no VAD" for why) Which means I think can_disable_turn_detection = not isGiven(True) = False
Is
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
in your case,
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bml1g12 since you mentioned the flag is hard to place, here is your exact config traced through this PR — it should do what you want, and With
And because Worth keeping the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see thanks for the detailed explanation, I misunderstood what the parameter can_disable_turn_detection meant |
||
| user_transcription=input_audio_transcription is not None, | ||
| auto_tool_reply_generation=False, | ||
|
|
@@ -501,7 +514,7 @@ def __init__( | |
| modalities=modalities, | ||
| input_audio_transcription=to_audio_transcription(input_audio_transcription), | ||
| input_audio_noise_reduction=to_noise_reduction(input_audio_noise_reduction), | ||
| turn_detection=to_turn_detection(turn_detection), | ||
| turn_detection=resolved_turn_detection, | ||
| api_key=api_key, | ||
| base_url=base_url_val, | ||
| is_azure=is_azure, | ||
|
|
@@ -2246,6 +2259,12 @@ def _handle_error(self, event: RealtimeErrorEvent) -> None: | |
| if event.error.message.startswith("Cancellation failed"): | ||
| return | ||
|
|
||
| if event.error.code == "input_audio_buffer_commit_empty" and ( | ||
| self._opts.turn_detection is not None | ||
| ): | ||
| # the server VAD commits each segment itself, ours lands on an emptied buffer | ||
| return | ||
|
|
||
| provider_label = self._realtime_model._provider_label | ||
| logger.error( | ||
| f"{provider_label} returned an error: {event.error}", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import logging | ||
| from unittest.mock import MagicMock | ||
|
|
||
| import pytest | ||
|
|
||
| from livekit.agents import Agent, AgentSession, TurnHandlingOptions, llm | ||
| from livekit.agents.voice.agent_activity import AgentActivity | ||
| from livekit.agents.voice.speech_handle import SpeechHandle | ||
|
|
||
| from .fake_realtime import FakeRealtimeModel, fake_capabilities | ||
| from .fake_vad import FakeVAD | ||
|
|
||
| pytestmark = pytest.mark.unit | ||
|
|
||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def _livekit_keys(monkeypatch: pytest.MonkeyPatch) -> None: | ||
| # the eager TurnDetector() default reads these; keep construction hermetic | ||
| monkeypatch.setenv("LIVEKIT_API_KEY", "k") | ||
| monkeypatch.setenv("LIVEKIT_API_SECRET", "s") | ||
|
|
||
|
|
||
| def _activity(*, server_turn_detection: bool, allow_interruptions: bool = True) -> AgentActivity: | ||
| session = AgentSession( | ||
| llm=FakeRealtimeModel( | ||
| capabilities=fake_capabilities( | ||
| turn_detection=server_turn_detection, can_disable_turn_detection=False | ||
| ) | ||
| ), | ||
| vad=FakeVAD(fake_user_speeches=[]), | ||
| turn_handling=TurnHandlingOptions(interruption={"enabled": allow_interruptions}), | ||
| ) | ||
| return AgentActivity(Agent(instructions="test"), session) | ||
|
|
||
|
|
||
| def _speech_started(activity: AgentActivity, *, allow_interruptions: bool) -> SpeechHandle: | ||
| handle = SpeechHandle.create(allow_interruptions=allow_interruptions) | ||
| activity._current_speech = handle | ||
| activity._rt_session = MagicMock() | ||
| activity._on_input_speech_started(llm.InputSpeechStartedEvent()) | ||
| return handle | ||
|
|
||
|
|
||
| def test_allow_interruptions_false_rejected_with_server_turn_detection() -> None: | ||
| # the server cancels its own response on user speech, so the local speech can't opt out | ||
| with pytest.raises(ValueError, match="allow_interruptions cannot be False"): | ||
| _activity(server_turn_detection=True, allow_interruptions=False) | ||
|
|
||
|
|
||
| def test_allow_interruptions_false_allowed_with_client_turn_taking() -> None: | ||
| # server VAD with create_response=False reports no server-side turn detection, so a | ||
| # turn-taking agent can disable interruptions (issue #6635) | ||
| activity = _activity(server_turn_detection=False, allow_interruptions=False) | ||
|
|
||
| assert activity._rt_turn_detection_enabled is False | ||
|
|
||
|
|
||
| def test_input_speech_started_keeps_uninterruptible_speech( | ||
| caplog: pytest.LogCaptureFixture, | ||
| ) -> None: | ||
| # the model still reports user speech (server VAD is on to commit and transcribe audio), but | ||
| # neither side interrupts: no response.cancel, no local interruption, and no error log since | ||
| # this is a valid configuration | ||
| activity = _activity(server_turn_detection=False, allow_interruptions=False) | ||
|
|
||
| with caplog.at_level(logging.ERROR, logger="livekit.agents"): | ||
| handle = _speech_started(activity, allow_interruptions=False) | ||
|
|
||
| assert handle.interrupted is False | ||
| activity._rt_session.interrupt.assert_not_called() | ||
| assert not caplog.records | ||
|
|
||
|
|
||
| def test_input_speech_started_interrupts_interruptible_speech() -> None: | ||
| activity = _activity(server_turn_detection=True) | ||
|
|
||
| handle = _speech_started(activity, allow_interruptions=True) | ||
|
|
||
| assert handle.interrupted is True | ||
| activity._rt_session.interrupt.assert_called_once() |
Uh oh!
There was an error while loading. Please reload this page.