fix(stt): pass language parameter to Groq Whisper API - #55713
fix(stt): pass language parameter to Groq Whisper API#55713alaamohanad169-ship-it wants to merge 3 commits into
Conversation
Hermes Agent Review — PR #55713Verdict: no blocking issues found. Scope reviewed: Checks performed:
Notes:
Ad-hoc probe result: Reviewed by Hermes Agent hourly commander. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused fix. Current main still lacks language in the Groq request at tools/transcription_tools.py:1297-1301, so the reported behavior remains reproducible by code inspection.
Problems
tools/transcription_tools.py:1294in this PR is not null-safe:stt.groq: nullmakes.get("groq", {})returnNone, then.get("language")raises before the function'stryblock. Current main's provider reads use.get("section") or {}after null-guard hardening in3a394210ffae0358100e9d14cf1639f764d7eab7.- The PR changes only
tools/transcription_tools.py; no regression test covers this new configuration-resolution behavior.
Suggested changes
- Load the STT config once and use null-safe
groqandlocalsection lookups. - Add tests for Groq override, local fallback, no configured language, and
stt.groq: null.
Automated hermes-sweeper review.
|
|
||
| # Language: config.yaml (stt.groq.language > stt.local.language) > auto-detect. | ||
| language = ( | ||
| _load_stt_config().get("groq", {}).get("language") |
There was a problem hiding this comment.
stt.groq has no default subsection, so a user configuration containing stt.groq: null makes this expression call .get() on None before the surrounding try block. Please match current main's null-safe provider pattern: (stt_config.get("groq") or {}).get("language").
The Groq STT provider ignored stt.groq.language and stt.local.language from config.yaml, forcing auto-detection. This garbled non-English audio (e.g. Hebrew detected as English). Read language from config (stt.groq.language > stt.local.language) and pass it to client.audio.transcriptions.create() when set. Fixes #55551
… lookup
stt_config.get("groq", {}).get("language") crashes when stt.groq is
explicitly null in config because .get(key, {}) only returns {} when the
key is MISSING, not when it's present-but-None. The or {} pattern matches
commit 3a39421 which fixed sibling sites throughout the file but
missed the groq lookup.
Also caches the _load_stt_config() call to a local var instead of calling
it twice.
Tests: Groq override present, local fallback, no language configured, and
stt.groq: null (the crash case).
52c49f7 to
258586d
Compare
…+ mock file I/O The test was patching 'tools.transcription_tools.OpenAI' but the code imports OpenAI from 'openai' module inside the function. Fixed by patching 'openai.OpenAI' instead. Also added mock for file I/O since the function actually tries to open the audio file.
|
👋 All CI checks are passing (fixed test mocking for Groq STT language config). Could a maintainer merge this when you have a moment? Thanks! |
|
Resolved by PR #73067, which unified language resolution across all STT providers (stt..language > global stt.language > env > auto-detect). The Groq language parameter fix from #23161 (earliest submitter, credited) plus the OpenAI/local halves were salvaged there with authorship preserved. Your analysis of the Groq auto-detect failure was spot on — thanks for the work, and sorry we couldn't take both duplicate implementations. |
The Groq STT provider ignored stt.groq.language and stt.local.language from config.yaml, forcing auto-detection. This garbled non-English audio (e.g. Hebrew detected as English).
Read language from config (stt.groq.language > stt.local.language) and pass it to client.audio.transcriptions.create() when set.
Fixes #55551