fix(sarvam): use provider speech timing for eos - #1763
rosetta-livekit-bot[bot] wants to merge 14 commits into
Conversation
…1525) Co-authored-by: rosetta-livekit-bot[bot] <282703043+rosetta-livekit-bot[bot]@users.noreply.github.com> Co-authored-by: u9g <jason.lernerman@livekit.io>
Agent.llmNode now returns ReadableStream<ChatChunk | string | FlushSentinel>, but the agent_v2 hook overrides and AgentHookAdapter still declared the narrower ChatChunk | string union, so passing super.llmNode as the fallback failed to type-check. Widen the override return types and the adapter's fallback/return signatures to include FlushSentinel. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Brian Yin <brian.yin@livekit.io> Co-authored-by: rosetta-livekit-bot[bot] <282703043+rosetta-livekit-bot[bot]@users.noreply.github.com> Co-authored-by: u9g <jason.lernerman@livekit.io>
Catch end-call close listener errors to avoid unhandled rejections during shutdown, and make public tool type guards return false for null inputs.
Co-authored-by: rosetta-livekit-bot[bot] <282703043+rosetta-livekit-bot[bot]@users.noreply.github.com>
Co-authored-by: rosetta-livekit-bot[bot] <282703043+rosetta-livekit-bot[bot]@users.noreply.github.com>
Co-authored-by: rosetta-livekit-bot[bot] <282703043+rosetta-livekit-bot[bot]@users.noreply.github.com>
…egment (#1760) Co-authored-by: Cursor <cursoragent@cursor.com>
…#1698) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 4baec50 The changes in this PR will be included in the next version bump. This PR includes changesets to release 34 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 |
| } else if (this.#sendFinalTranscript(td, putMessage)) { | ||
| this.#finalReceivedForUtterance = true; | ||
| } |
There was a problem hiding this comment.
🟡 Missing #eosEmittedForUtterance guard allows FINAL_TRANSCRIPT after END_OF_SPEECH
When the EOS fallback timer fires (because the server sent END_SPEECH but no transcript arrived within 1000ms), #pendingEos is set to false and #eosEmittedForUtterance is set to true. If a late transcript data message subsequently arrives, the code at line 909 takes the else if branch (since #pendingEos is false) and calls #sendFinalTranscript without checking #eosEmittedForUtterance. This emits a FINAL_TRANSCRIPT event after END_OF_SPEECH was already emitted, violating the expected event ordering (START_OF_SPEECH → FINAL_TRANSCRIPT → END_OF_SPEECH). Downstream in audio_recognition.ts:837-897, this late FINAL_TRANSCRIPT updates audioTranscript, triggers preemptive generation, and runs EOU detection again — all after the user turn was already committed at audio_recognition.ts:1047.
Note that #tryCommitUtterance at plugins/sarvam/src/stt.ts:661 correctly guards against this with this.#eosEmittedForUtterance, but the direct #sendFinalTranscript call path at line 909 does not.
| } else if (this.#sendFinalTranscript(td, putMessage)) { | |
| this.#finalReceivedForUtterance = true; | |
| } | |
| } else if (!this.#eosEmittedForUtterance && this.#sendFinalTranscript(td, putMessage)) { | |
| this.#finalReceivedForUtterance = true; | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| const SAMPLE_RATE = 16000; | ||
| const NUM_CHANNELS = 1; | ||
| const EOS_FALLBACK_TIMEOUT = 1000; |
There was a problem hiding this comment.
🚩 EOS_FALLBACK_TIMEOUT of 1000ms may need tuning
The EOS_FALLBACK_TIMEOUT constant is set to 1000ms at line 38. This is the maximum time the system will wait for a transcript after receiving END_SPEECH before emitting END_OF_SPEECH without one. If Sarvam's server processing latency is sometimes >1000ms (e.g., for longer utterances or under load), the fallback could fire prematurely, causing the transcript to arrive after END_OF_SPEECH (which is the scenario in BUG-0001). The Sarvam STT metrics logging at line 894-896 captures processing_latency — monitoring this in production would help determine if the 1000ms timeout is appropriate.
Was this helpful? React with 👍 or 👎 to provide feedback.
| equals(other: ToolContext): boolean { | ||
| if (this._functionToolsMap.size !== other._functionToolsMap.size) { | ||
| return false; | ||
| } | ||
|
|
||
| const tool1 = ctx1[name]; | ||
| const tool2 = ctx2[name]; | ||
| for (const [id, tool] of this._functionToolsMap) { | ||
| if (other._functionToolsMap.get(id) !== tool) { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| if (!tool1 || !tool2) { | ||
| if (this._providerTools.length !== other._providerTools.length) { | ||
| return false; | ||
| } | ||
|
|
||
| if (tool1.description !== tool2.description) { | ||
| // Provider tools compare as identity sets to match Python's `set(id(t) for t in ...)` | ||
| // semantics — order is not significant. | ||
| const otherProviderIds = new Set(other._providerTools); | ||
| for (const tool of this._providerTools) { | ||
| if (!otherProviderIds.has(tool)) { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| if (this._toolsets.length !== other._toolsets.length) { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| const otherToolsets = new Set(other._toolsets); | ||
| for (const ts of this._toolsets) { | ||
| if (!otherToolsets.has(ts)) { | ||
| return false; | ||
| } | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
🚩 Tool context equality semantics changed from description-based to identity-based comparison
The old isSameToolContext at the removed code compared tool contexts by checking that both had the same set of tool names and that each tool's description matched. The new ToolContext.equals() (agents/src/llm/tool_context.ts:486-521) compares function tools by object identity (other._functionToolsMap.get(id) !== tool), provider tools by identity set, and toolsets by identity set.
This changes the semantics in two call sites:
- Preemptive generation validation (
agents/src/voice/agent_activity.ts:2192):preemptive.tools.equals(this.agent._toolCtx)— previously a preemptive generation would be reused if tools had the same names and descriptions; now it requires the exact same tool instances. This is stricter but safer (avoids reusing a generation when a tool'sexecutefunction changed but its description didn't). - Realtime session reuse (
agents/src/voice/agent_activity.ts:616):this.agent._toolCtx.equals(newActivity.agent._toolCtx)— same stricter comparison for deciding whether to reuse a realtime session across agent handoffs.
The new behavior is more correct (identity is a stronger guarantee than description equality), but callers that previously relied on structural equality (e.g., recreating tools with the same description across agent instances) will now see unnecessary session restarts or preemptive generation invalidations.
Was this helpful? React with 👍 or 👎 to provide feedback.
| async updateTools(tools: llm.ToolContext): Promise<void> { | ||
| const newDeclarations = toFunctionDeclarations(tools); | ||
| const currentToolNames = new Set(this.geminiDeclarations.map((f) => f.name)); | ||
| const newToolNames = new Set(newDeclarations.map((f) => f.name)); | ||
|
|
||
| if (!setsEqual(currentToolNames, newToolNames)) { | ||
| this.geminiDeclarations = newDeclarations; | ||
| this._tools = tools; | ||
| this.markRestartNeeded(); | ||
| if (this._tools.equals(tools)) { | ||
| return; | ||
| } | ||
|
|
||
| this._tools = tools; | ||
| this.markRestartNeeded(); | ||
| } |
There was a problem hiding this comment.
🚩 Google realtime session tool update now uses ToolContext.equals() instead of name-set comparison
The old updateTools in plugins/google/src/realtime/realtime_api.ts compared tool sets by extracting function declaration names into Sets and checking set equality via a helper setsEqual. The new code at line 762-765 uses this._tools.equals(tools) which compares by object identity.
This is a stricter comparison: previously, two different tool instances with the same name would be considered equal (no restart needed); now they trigger a restart. This aligns with the framework-wide move to identity-based equality but means that agent handoffs that recreate tools with the same schema will now trigger unnecessary WebSocket reconnections in the Google realtime session. The markRestartNeeded() call tears down and rebuilds the entire session, so false positives here have a real cost.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Tests
Ported from livekit/agents#6052
Original PR description
Problem
The Sarvam streaming STT plugin tried to manufacture an audio-relative speech-end time from two sources that don't actually provide one:
Sarvam's streaming socket genuinely sends no usable word timing (no timestamps array; speech_start/speech_end come back null), so all this machinery produced misleading timestamps.
Change
Aligned Sarvam with how every other STT plugin (Deepgram, AssemblyAI, Google, Azure…) handles this: