[Update] Model versions for Sarvam - #637
Conversation
📝 WalkthroughWalkthroughUpdated Sarvam LLM support to use Merge Risk: 🟡 Moderate · up to The PR updates supported Sarvam model versions across LLM, TTS, and STT, but the current STT changes can miss finalizing some speech turns and can retain a background keepalive task after disconnects. These bounded correctness and resource-lifecycle issues should be fixed or explicitly accepted before merging. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
plugins/sarvam/tests/test_sarvam_stt.py (1)
66-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd return annotations to the new async test methods.
Add
-> Nonetotest_legacy_models_rejectedandtest_mode_included_for_v3.Proposed change
- async def test_legacy_models_rejected(self): + async def test_legacy_models_rejected(self) -> None: ... - async def test_mode_included_for_v3(self): + async def test_mode_included_for_v3(self) -> None:As per coding guidelines, “Use type annotations everywhere.”
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c9d7662f-613a-4276-8ce9-627cc12789d9
📒 Files selected for processing (9)
CHANGELOG.mdplugins/sarvam/README.mdplugins/sarvam/example/sarvam_example.pyplugins/sarvam/tests/test_sarvam_llm.pyplugins/sarvam/tests/test_sarvam_stt.pyplugins/sarvam/tests/test_sarvam_tts.pyplugins/sarvam/vision_agents/plugins/sarvam/llm.pyplugins/sarvam/vision_agents/plugins/sarvam/stt.pyplugins/sarvam/vision_agents/plugins/sarvam/tts.py
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
The default model was pointed at the legacy /speech-to-text/ws contract. Route saaras:v3-realtime through /speech-to-text-realtime/ws and keep v3/v4 on the legacy path. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/sarvam/tests/test_sarvam_stt.py (1)
95-101: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftReplace
FakeWebSocketwith a local WebSocket test server.These local classes are mocks. Use a fixture that starts a local WebSocket server and assert on frames received by that server.
As per coding guidelines,
**/*test*.py: “Never mock in tests”.Also applies to: 122-128
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 83349b5c-8511-4697-ae6e-a6d79908e5d9
📒 Files selected for processing (3)
CHANGELOG.mdplugins/sarvam/tests/test_sarvam_stt.pyplugins/sarvam/vision_agents/plugins/sarvam/stt.py
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
| @@ -91,7 +127,7 @@ def __init__(self) -> None: | |||
| async def send_str(self, message: str) -> None: | |||
| self.sent_messages.append(message) | |||
|
|
|||
| stt = STT(api_key="sk_test") | |||
| stt = STT(api_key="sk_test", model="saaras:v3") | |||
| ws = FakeWebSocket() | |||
| stt._ws = ws | |||
| stt._connection_ready.set() | |||
| @@ -109,6 +145,38 @@ async def send_str(self, message: str) -> None: | |||
| assert message["audio"]["encoding"] == "audio/wav" | |||
| assert message["audio"]["sample_rate"] == 16000 | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align the test name with the asserted codec.
The test name says pcm_s16le, but line 145 asserts audio/wav. Rename the test to describe the WAV assertion, or change the expected codec if PCM was intended.
| self._session: Optional[aiohttp.ClientSession] = None | ||
| self._ws: Optional[aiohttp.ClientWebSocketResponse] = None | ||
| self._receive_task: Optional[asyncio.Task[Any]] = None | ||
| self._keepalive_task: Optional[asyncio.Task[None]] = None |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Replace Any with concrete types.
_receive_loop() returns None, so _receive_task can use asyncio.Task[None]. Model the two outbound payload shapes with a concrete union instead of dict[str, Any].
As per coding guidelines, **/*.py: “Avoid using Any type”.
Also applies to: 214-215
Source: Coding guidelines
| if event == "vad.speech_end": | ||
| self._in_speech = False | ||
| self._audio_start_time = None | ||
| if participant is not None and self.vad_signals: | ||
| self._emit_turn_ended_event(participant) | ||
| return |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
target="plugins/sarvam/vision_agents/plugins/sarvam/stt.py"
printf '%s\n' '--- file outline ---'
ast-grep outline "$target" --view compact || true
printf '%s\n' '--- relevant symbols and references ---'
rg -n -C 4 'vad\.speech_end|transcript\.final|TurnEnded|_keepalive_loop|keepalive|_in_speech|_audio_start_time|def (start|stop|aclose|close)|class ' "$target"
printf '%s\n' '--- file size ---'
wc -l "$target"Repository: GetStream/Vision-Agents
Length of output: 7966
🏁 Script executed:
#!/bin/bash
set -eu
target="plugins/sarvam/vision_agents/plugins/sarvam/stt.py"
printf '%s\n' '--- initialization, start, and audio path ---'
sed -n '90,235p' "$target"
printf '%s\n' '--- realtime and legacy handlers ---'
sed -n '250,425p' "$target"
printf '%s\n' '--- receive loop and close lifecycle ---'
sed -n '230,270p' "$target"
sed -n '419,469p' "$target"
printf '%s\n' '--- related event definitions and consumers ---'
rg -n -C 3 'TurnEnded|turn_ended|transcript.*mode|mode=.*replacement|vad_signals' --glob '*.py' --glob '*.ts' --glob '*.tsx' .
printf '%s\n' '--- Sarvam tests and references ---'
rg -n -C 5 'Sarvam|saaras:v3-realtime|vad\.speech_end|transcript\.final|_handle_realtime_message' --glob '*test*' --glob '*.py' --glob '*.ts' .Repository: GetStream/Vision-Agents
Length of output: 50380
🌐 Web query:
Sarvam AI saaras:v3-realtime WebSocket vad.speech_end transcript.final event ordering
💡 Result:
In the Sarvam AI saaras:v3-realtime WebSocket API, the vad.speech_end event is emitted before the transcript.final event [1]. This ordering is a specific design feature of the saaras:v3-realtime implementation, intended to support low-latency interactions such as preemptive generation [1]. When the model detects the end of speech, it emits the vad.speech_end signal immediately [1]. This allows downstream systems (such as voice agents) to mark the turn as committed and potentially begin speculative processing before the full transcript.final—which arrives shortly after—is received [1]. This sequence is the opposite of the legacy saaras:v3 (non-realtime) WebSocket implementation, where the final transcript was typically received before or coinciding with the end-of-speech signal [1]. Authoritative technical documentation for integrating with frameworks like LiveKit and Pipecat explicitly confirms this order: vad.speech_end precedes transcript.final [1][2].
Citations:
- 1: https://docs.sarvam.ai/api/integration/livekit-production-best-practices
- 2: https://docs.sarvam.ai/api/integration/pipecat-production-guide
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- core STT emission and stream consumption ---'
sed -n '120,180p' agents-core/vision_agents/core/stt/stt.py
rg -n -C 4 'output|TurnEnded|Transcript' agents-core/vision_agents/core --glob '*.py' \
| head -n 220
printf '%s\n' '--- focused Sarvam references ---'
rg -n -C 4 'saaras:v3-realtime|vad\.speech_end|transcript\.final|_handle_realtime_message' \
plugins/sarvam tests --glob '*.py' --glob '*.md' --glob '*.json' || true
printf '%s\n' '--- relevant source lines ---'
nl -ba plugins/sarvam/vision_agents/plugins/sarvam/stt.py | sed -n '255,330p'Repository: GetStream/Vision-Agents
Length of output: 32320
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- STT output consumers ---'
rg -n -C 5 'isinstance\(.*TurnEnded|TurnEnded\)|TurnEnded|turn_ended' \
agents-core vision-agents plugins --glob '*.py' \
| rg -v 'tests|test_' \
| head -n 320
printf '%s\n' '--- transcript final consumers ---'
rg -n -C 5 '\.final|Transcript\)|isinstance\(.*Transcript|transcript' \
agents-core/vision_agents plugins --glob '*.py' \
| rg -v 'tests|test_' \
| head -n 320
printf '%s\n' '--- Sarvam test section ---'
sed -n '120,195p' plugins/sarvam/tests/test_sarvam_stt.py
printf '%s\n' '--- source-derived behavior verifier ---'
python3 - <<'PY'
from pathlib import Path
source = Path("plugins/sarvam/vision_agents/plugins/sarvam/stt.py").read_text()
speech_end = source.index('if event == "vad.speech_end":')
transcript_branch = source.index('if event not in ("transcript.partial", "transcript.final"):')
assert speech_end < transcript_branch
speech_end_block = source[speech_end:transcript_branch]
assert "_emit_turn_ended_event(participant)" in speech_end_block
assert "return" in speech_end_block
events = []
def emit_turn_ended() -> None:
events.append("TurnEnded")
def emit_transcript_final() -> None:
events.append("Transcript(final)")
for event in ("vad.speech_end", "transcript.final"):
if event == "vad.speech_end":
emit_turn_ended()
elif event == "transcript.final":
emit_transcript_final()
assert events == ["TurnEnded", "Transcript(final)"]
print("documented sequence model:", events)
print("source branch emits TurnEnded before later transcript.final")
PYRepository: GetStream/Vision-Agents
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- transcribing flow control ---'
sed -n '304,495p' agents-core/vision_agents/core/agents/inference/transcribing_flow.py
printf '%s\n' '--- Sarvam test assertions ---'
sed -n '130,190p' plugins/sarvam/tests/test_sarvam_stt.py
printf '%s\n' '--- transcript store update semantics ---'
rg -n -C 8 'def update_user_transcript|class TranscriptStore|def flush_user|pending_users' \
agents-core/vision_agents/core/agents/transcript --glob '*.py'Repository: GetStream/Vision-Agents
Length of output: 15996
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- turn detection mode setup ---'
rg -n -C 5 '_no_turn_detection|turn_detection' \
agents-core/vision_agents/core/agents/inference/transcribing_flow.py \
agents-core/vision_agents/core/stt/stt.py \
plugins/sarvam/vision_agents/plugins/sarvam/stt.py
printf '%s\n' '--- deterministic event-state verifier ---'
python3 - <<'PY'
from pathlib import Path
stt_source = Path("plugins/sarvam/vision_agents/plugins/sarvam/stt.py").read_text()
flow_source = Path(
"agents-core/vision_agents/core/agents/inference/transcribing_flow.py"
).read_text()
# Confirm the provider emits a non-eager turn end before handling a later final.
speech_end_start = stt_source.index('if event == "vad.speech_end":')
speech_end_end = stt_source.index(
'if event not in ("transcript.partial", "transcript.final"):',
speech_end_start,
)
speech_end_block = stt_source[speech_end_start:speech_end_end]
assert "_emit_turn_ended_event(participant)" in speech_end_block
assert "return" in speech_end_block
assert 'mode = "final" if event == "transcript.final" else "replacement"' in stt_source
# Confirm the downstream finalization requires both a final buffer and a confirmed turn.
finalization_marker = "if (\n buffer.final"
finalization_start = flow_source.index(finalization_marker)
finalization_block = flow_source[finalization_start : finalization_start + 260]
assert "llm_turn.confirmed" in finalization_block
assert "llm_turn.finalized" in finalization_block
# Minimal model of the relevant downstream state transitions.
buffer_text = ""
buffer_final = False
llm_confirmed = False
llm_started = False
llm_finalized = False
# vad.speech_end arrives before any transcript.
# The non-eager path sees no buffer and cannot create/confirm a turn.
if not buffer_text:
pass
# transcript.final arrives later.
buffer_text = "hello"
buffer_final = True
llm_started = True
# No prior TurnEnded confirmation exists, so this new non-eager turn remains unconfirmed.
if buffer_final and llm_started and llm_confirmed:
llm_finalized = True
assert not llm_confirmed
assert not llm_finalized
print("documented order with no partial:", [
"vad.speech_end",
"transcript.final",
])
print("result: final transcript remains unconfirmed and cannot finalize the LLM turn")
PYRepository: GetStream/Vision-Agents
Length of output: 12855
Handle transcript.final after vad.speech_end.
When no transcript.partial exists, TurnEnded sees an empty buffer and does not confirm an LLMTurn. The later transcript.final starts an unconfirmed turn, so the LLM turn is not finalized. Track the pending turn end and confirm it when the final transcript arrives. Add coverage for this sequence, including the no-partial case.
| while True: | ||
| await asyncio.sleep(KEEPALIVE_INTERVAL_S) | ||
| if self._ws is not None and not self._ws.closed: | ||
| await self._ws.send_str(json.dumps({"event": "ping"})) | ||
| except asyncio.CancelledError: | ||
| pass | ||
| except (aiohttp.ClientError, ConnectionError): | ||
| logger.debug("Sarvam STT keepalive send failed") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Stop the keepalive task after a remote close.
After _receive_loop() exits for a disconnected socket, this loop continues sleeping forever because a closed socket does not end the loop. Each disconnected realtime instance retains a pending task until a caller explicitly invokes close(). Stop the keepalive task when the receive loop terminates, or return when _ws is closed.
🧰 Tools
🪛 ast-grep (0.45.1)
[info] 432-432: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"event": "ping"})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
Why
Changes