Repository navigation
fix(deepgram): forward enable_diarization and tags to open streams - #7821
Atharvsinh-codez wants to merge 1 commit into
Conversation
STT.update_options() stored enable_diarization and tags on the STT but left them out of the call that updates the open streams. Each stream keeps its own copy of the options, so it reconnected with the old values: turning diarization on mid-session never sent diarize=true, and new tags never reached Deepgram.
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}
| sample_rate=sample_rate, | ||
| no_delay=no_delay, | ||
| endpointing_ms=endpointing_ms, | ||
| enable_diarization=enable_diarization, |
There was a problem hiding this comment.
🟡 Enabled diarization remains unavailable to adapters
After update_options(enable_diarization=True), open streams emit speaker IDs but capabilities.diarization stays false. MultiSpeakerAdapter rejects the STT even though diarization is active.
Learn more
The constructor initializes capabilities.diarization from the initial option in STT.init. The newly forwarded update changes the configuration of open sockets without updating that capability. MultiSpeakerAdapter gates construction on the capability, so an STT can produce speaker IDs while the adapter refuses it.
Example: Create STT(api_key="test-key"), open a stream, then call update_options(enable_diarization=True). The next Deepgram socket requests diarize=true, but MultiSpeakerAdapter(stt=instance) raises ValueError.
Recommended fix: Synchronize self._capabilities.diarization with an explicitly supplied enable_diarization in STT.update_options and cover both enabling and disabling with a test. Consider how existing wrappers cache the capabilities at construction.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Folded into #7828 together with the other two STT fixes and a fix for the flaky unit-tests failure. Closing this one. |
Summary
deepgram.STT.update_options()savesenable_diarizationandtagson the STT, but the call that updates the open streams leaves both out. Each stream keeps its own copy of the options, so it reconnects with the old values. That has two effects:stt.update_options(enable_diarization=True)during a session reconnects the stream withoutdiarize=true. No speaker IDs come back, and nothing reports an error.tagsnever reach Deepgram for the open stream.SpeechStream.update_options()already accepts both parameters, andstt_v2already forwardstags. Only these two arguments were missing from the forwarding call.Test plan
test_stt_update_options_reaches_open_streams_diarization_and_tags. It opens a stream, callsSTT.update_options(enable_diarization=True, tags=["prod"]), and checks the URL of the reconnected socket. It fails onmain(nodiarizein the query) and passes with this change.uv run pytest tests/test_plugin_deepgram_stt.py --plugin deepgram: 42 passed.livekit.plugins.deepgram) are clean.