Fix: Parsing lang from stt ctor#1028
Conversation
🦋 Changeset detectedLatest commit: 465b52c The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 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 |
📝 WalkthroughWalkthroughParses language from model strings of the form "provider/model:language", computes nextModel and nextLanguage, applies them to STT options, and updates the STT constructor to accept Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.cursor/rules/agent-core.mdc)
Files:
**/*.{ts,tsx}?(test|example|spec)📄 CodeRabbit inference engine (.cursor/rules/agent-core.mdc)
Files:
**/*.{ts,tsx}?(test|example)📄 CodeRabbit inference engine (.cursor/rules/agent-core.mdc)
Files:
🧠 Learnings (2)📚 Learning: 2026-01-16T14:33:39.551ZApplied to files:
📚 Learning: 2026-01-16T14:33:39.551ZApplied to files:
🧬 Code graph analysis (1)agents/src/inference/stt.ts (1)
🪛 GitHub Actions: Buildagents/src/inference/stt.ts[error] 178-178: TS2322: Type 'ModelWithLanguage | undefined' is not assignable to type 'TModel | undefined'. Build step: 'tsc --declaration --emitDeclarationOnly && node ../scripts/copyDeclarationOutput.js' 🔇 Additional comments (1)
✏️ Tip: You can disable this entire section by setting 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 |
| } | ||
|
|
||
| // Parse language from model string if provided: "provider/model:language" | ||
| let nextModel = model; |
There was a problem hiding this comment.
We might want to change the model type to ModelWithLanguage
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
agents/src/inference/stt.ts (1)
158-185:⚠️ Potential issue | 🔴 CriticalFix TS2322 by narrowing
nextModeltoTModel.
The constructor assignsmodel: nextModeltothis.opts(line 178), which expects typeTModel | undefined. However,nextModelstarts asModelWithLanguage | undefined, and TypeScript cannot narrow the type through the conditional reassignment. CastnextModelupfront and parse the originalmodelparameter to ensure proper type narrowing.🔧 Proposed fix
- let nextModel = model; + let nextModel = model as TModel | undefined; let nextLanguage = language; - if (typeof nextModel === 'string') { - const idx = nextModel.lastIndexOf(':'); + if (typeof model === 'string') { + const idx = model.lastIndexOf(':'); if (idx !== -1) { - const languageFromModel = nextModel.slice(idx + 1) as STTLanguages; + const languageFromModel = model.slice(idx + 1) as STTLanguages; if (nextLanguage && nextLanguage !== languageFromModel) { this.#logger.warn( '`language` is provided via both argument and model, using the one from the argument', { language: nextLanguage, model: nextModel }, ); } else { nextLanguage = languageFromModel; } - nextModel = nextModel.slice(0, idx) as TModel; + nextModel = model.slice(0, idx) as TModel; } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
agents/src/inference/stt.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/agent-core.mdc)
Add SPDX-FileCopyrightText and SPDX-License-Identifier headers to all newly added files with '// SPDX-FileCopyrightText: 2025 LiveKit, Inc.' and '// SPDX-License-Identifier: Apache-2.0'
Files:
agents/src/inference/stt.ts
**/*.{ts,tsx}?(test|example|spec)
📄 CodeRabbit inference engine (.cursor/rules/agent-core.mdc)
When testing inference LLM, always use full model names from
agents/src/inference/models.ts(e.g., 'openai/gpt-4o-mini' instead of 'gpt-4o-mini')
Files:
agents/src/inference/stt.ts
**/*.{ts,tsx}?(test|example)
📄 CodeRabbit inference engine (.cursor/rules/agent-core.mdc)
Initialize logger before using any LLM functionality with
initializeLogger({ pretty: true })from '@livekit/agents'
Files:
agents/src/inference/stt.ts
🧠 Learnings (2)
📚 Learning: 2026-01-16T14:33:39.551Z
Learnt from: CR
Repo: livekit/agents-js PR: 0
File: .cursor/rules/agent-core.mdc:0-0
Timestamp: 2026-01-16T14:33:39.551Z
Learning: Applies to **/*.{ts,tsx}?(test|example|spec) : When testing inference LLM, always use full model names from `agents/src/inference/models.ts` (e.g., 'openai/gpt-4o-mini' instead of 'gpt-4o-mini')
Applied to files:
agents/src/inference/stt.ts
📚 Learning: 2026-01-16T14:33:39.551Z
Learnt from: CR
Repo: livekit/agents-js PR: 0
File: .cursor/rules/agent-core.mdc:0-0
Timestamp: 2026-01-16T14:33:39.551Z
Learning: Applies to **/*.{ts,tsx}?(test|example) : Initialize logger before using any LLM functionality with `initializeLogger({ pretty: true })` from 'livekit/agents'
Applied to files:
agents/src/inference/stt.ts
🧬 Code graph analysis (1)
agents/src/inference/stt.ts (1)
agents/src/inference/index.ts (2)
ModelWithLanguage(21-21)STTLanguages(19-19)
🪛 GitHub Actions: Build
agents/src/inference/stt.ts
[error] 178-178: TS2322: Type 'ModelWithLanguage | undefined' is not assignable to type 'TModel | undefined'. Build step: 'tsc --declaration --emitDeclarationOnly && node ../scripts/copyDeclarationOutput.js'
🔇 Additional comments (1)
agents/src/inference/stt.ts (1)
123-132: Constructor signature update looks good.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
Hey @adrian-cowham, what's the status of this PR? Are we still plan to merge it? |
|
Yeah. It's good to go if you approve. |
toubatbrian
left a comment
There was a problem hiding this comment.
Thanks for confirming, could you resolve the build error as well. Once it is resolved, I'll go ahead and merge it
Summary by CodeRabbit