fix(stt): read Groq model from stt.groq.model config before env fallback - #58765
fix(stt): read Groq model from stt.groq.model config before env fallback#58765liuhao1024 wants to merge 1 commit into
Conversation
The Groq STT provider skipped the provider-namespaced config lookup
(stt.groq.model) that every other STT provider (OpenAI, Mistral,
ElevenLabs, local) uses. This forced users to set the STT_GROQ_MODEL
env var instead of using config.yaml like all other providers.
Add groq_cfg lookup matching the established pattern:
model or groq_cfg.get('model', DEFAULT_GROQ_STT_MODEL)
Includes regression test test_config_groq_model_used.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused regression fix. The premise still holds on current main: tools/transcription_tools.py:1761-1763 bypasses stt.groq.model and falls straight back to DEFAULT_GROQ_STT_MODEL.
Problems
- The new
stt_config.get("groq", {})can returnNonefor valid YAMLstt.groq: null, so the subsequent.get("model", ...)would raise. Current sibling branches use.get("<provider>") or {}after the null-subsection hardening in3a394210. - This changes a user-facing configuration contract, but
website/docs/user-guide/configuration.md:1627-1634still describes Groq model overrides as environment-driven only.
Suggested changes
- Use
groq_cfg = stt_config.get("groq") or {}and cover a null subsection in the regression tests. - Document
stt.groq.modeland its precedence overSTT_GROQ_MODEL.
Automated hermes-sweeper review.
| @@ -1671,7 +1671,8 @@ def transcribe_audio(file_path: str, model: Optional[str] = None) -> Dict[str, A | |||
| return _transcribe_local_command(file_path, model_name) | |||
|
|
|||
There was a problem hiding this comment.
stt.groq: null is valid YAML and makes this expression return None, so the next groq_cfg.get(...) raises AttributeError. Please match the null-safe sibling-provider pattern introduced in 3a394210: stt_config.get("groq") or {}, with a regression test for the null subsection.
|
Closing as superseded: this exact fix landed on main via Thanks @liuhao1024 — your patch identified the right resolution order; the merged change matches it. |
What does this PR do?
Fixes the Groq STT provider's model resolution to read from
stt.groq.modelin config.yaml before falling back to theSTT_GROQ_MODELenv var / default. Every other STT provider (OpenAI, Mistral, ElevenLabs, local) already follows this pattern, but the Groq branch skipped the config lookup entirely.Related Issue
Fixes #58734
Type of Change
Changes Made
tools/transcription_tools.py— Addstt_config.get("groq", {})lookup beforeDEFAULT_GROQ_STT_MODELfallback, matching the pattern used by OpenAI/Mistral/ElevenLabs/local providerstests/tools/test_transcription_tools.py— Addtest_config_groq_model_usedregression test verifyingstt.groq.modelis read from configHow to Test
python -m pytest tests/tools/test_transcription_tools.py -k groq -xvs— all 24 Groq-related tests should passpython -m pytest tests/tools/test_transcription_tools.py -x— full suite (108 tests) should passpython -m pytest tests/tools/test_transcription_tools.py::TestTranscribeAudioDispatch::test_config_groq_model_used -xvsshould passstt.groq.model: whisper-large-v3in config.yaml (without settingSTT_GROQ_MODELenv), trigger voice transcription, and confirm Groq useswhisper-large-v3instead of the defaultwhisper-large-v3-turboChecklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/A