From 9a1e97112639d31a3b5a0ab86d47f47078280d75 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 17 Mar 2026 10:30:58 -0700 Subject: [PATCH] fix(stt): respect explicit provider config instead of env-var fallback (#1775) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(session): skip corrupt lines in load_transcript instead of crashing Wrap json.loads() in load_transcript() with try/except JSONDecodeError so that partial JSONL lines (from mid-write crashes like OOM/SIGKILL) are skipped with a warning instead of crashing the entire transcript load. The rest of the history loads fine. Adds a logger.warning with the session ID and truncated corrupt line content for debugging visibility. Salvaged from PR #1193 by alireza78a. Closes #1193 * fix(stt): respect explicit provider config instead of env-var fallback Rework _get_provider() to separate explicit config from auto-detect. When stt.provider is explicitly set in config.yaml, that choice is authoritative — no silent cross-provider fallback based on which env vars happen to be set. When no provider is configured, auto-detect still tries: local > groq > openai. This fixes the reported scenario where provider: local + a placeholder OPENAI_API_KEY caused the system to silently select OpenAI and fail with a 401. Closes #1774 --- tests/tools/test_transcription.py | 10 +- tests/tools/test_transcription_tools.py | 140 +++++++++++++++--------- tools/transcription_tools.py | 116 ++++++++++---------- 3 files changed, 151 insertions(+), 115 deletions(-) diff --git a/tests/tools/test_transcription.py b/tests/tools/test_transcription.py index c8daface..0ce3f246 100644 --- a/tests/tools/test_transcription.py +++ b/tests/tools/test_transcription.py @@ -26,13 +26,14 @@ class TestGetProvider: from tools.transcription_tools import _get_provider assert _get_provider({"provider": "local"}) == "local" - def test_local_fallback_to_openai(self, monkeypatch): + def test_explicit_local_no_cloud_fallback(self, monkeypatch): + """Explicit local provider must not silently fall back to cloud.""" monkeypatch.setenv("VOICE_TOOLS_OPENAI_KEY", "sk-test") monkeypatch.delenv("GROQ_API_KEY", raising=False) with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \ patch("tools.transcription_tools._HAS_OPENAI", True): from tools.transcription_tools import _get_provider - assert _get_provider({"provider": "local"}) == "openai" + assert _get_provider({"provider": "local"}) == "none" def test_local_nothing_available(self, monkeypatch): monkeypatch.delenv("VOICE_TOOLS_OPENAI_KEY", raising=False) @@ -47,12 +48,13 @@ class TestGetProvider: from tools.transcription_tools import _get_provider assert _get_provider({"provider": "openai"}) == "openai" - def test_openai_fallback_to_local(self, monkeypatch): + def test_explicit_openai_no_key_returns_none(self, monkeypatch): + """Explicit openai without key returns none — no cross-provider fallback.""" monkeypatch.delenv("VOICE_TOOLS_OPENAI_KEY", raising=False) with patch("tools.transcription_tools._HAS_FASTER_WHISPER", True), \ patch("tools.transcription_tools._HAS_OPENAI", True): from tools.transcription_tools import _get_provider - assert _get_provider({"provider": "openai"}) == "local" + assert _get_provider({"provider": "openai"}) == "none" def test_default_provider_is_local(self): with patch("tools.transcription_tools._HAS_FASTER_WHISPER", True): diff --git a/tests/tools/test_transcription_tools.py b/tests/tools/test_transcription_tools.py index a74fde04..b5c9f977 100644 --- a/tests/tools/test_transcription_tools.py +++ b/tests/tools/test_transcription_tools.py @@ -66,19 +66,12 @@ class TestGetProviderGroq: from tools.transcription_tools import _get_provider assert _get_provider({"provider": "groq"}) == "groq" - def test_groq_fallback_to_local(self, monkeypatch): + def test_groq_explicit_no_fallback(self, monkeypatch): + """Explicit groq with no key returns none — no cross-provider fallback.""" monkeypatch.delenv("GROQ_API_KEY", raising=False) with patch("tools.transcription_tools._HAS_FASTER_WHISPER", True): from tools.transcription_tools import _get_provider - assert _get_provider({"provider": "groq"}) == "local" - - def test_groq_fallback_to_openai(self, monkeypatch): - monkeypatch.delenv("GROQ_API_KEY", raising=False) - monkeypatch.setenv("VOICE_TOOLS_OPENAI_KEY", "sk-test") - with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \ - patch("tools.transcription_tools._HAS_OPENAI", True): - from tools.transcription_tools import _get_provider - assert _get_provider({"provider": "groq"}) == "openai" + assert _get_provider({"provider": "groq"}) == "none" def test_groq_nothing_available(self, monkeypatch): monkeypatch.delenv("GROQ_API_KEY", raising=False) @@ -90,36 +83,25 @@ class TestGetProviderGroq: class TestGetProviderFallbackPriority: - """Cross-provider fallback priority tests.""" + """Auto-detect fallback priority and explicit provider behaviour.""" - def test_local_fallback_prefers_groq_over_openai(self, monkeypatch): - """When local unavailable, groq (free) is preferred over openai (paid).""" + def test_auto_detect_prefers_local(self): + """Auto-detect prefers local over any cloud provider.""" + with patch("tools.transcription_tools._HAS_FASTER_WHISPER", True): + from tools.transcription_tools import _get_provider + assert _get_provider({}) == "local" + + def test_auto_detect_prefers_groq_over_openai(self, monkeypatch): + """Auto-detect: groq (free) is preferred over openai (paid).""" monkeypatch.setenv("GROQ_API_KEY", "gsk-test") monkeypatch.setenv("VOICE_TOOLS_OPENAI_KEY", "sk-test") with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \ patch("tools.transcription_tools._HAS_OPENAI", True): from tools.transcription_tools import _get_provider - assert _get_provider({"provider": "local"}) == "groq" + assert _get_provider({}) == "groq" - def test_local_fallback_to_groq_only(self, monkeypatch): - """When only groq key available, falls back to groq.""" - monkeypatch.setenv("GROQ_API_KEY", "gsk-test") - with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \ - patch("tools.transcription_tools._HAS_OPENAI", True): - from tools.transcription_tools import _get_provider - assert _get_provider({"provider": "local"}) == "groq" - - def test_openai_fallback_to_groq(self, monkeypatch): - """When openai key missing but groq available, falls back to groq.""" - monkeypatch.delenv("VOICE_TOOLS_OPENAI_KEY", raising=False) - monkeypatch.setenv("GROQ_API_KEY", "gsk-test") - with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \ - patch("tools.transcription_tools._HAS_OPENAI", True): - from tools.transcription_tools import _get_provider - assert _get_provider({"provider": "openai"}) == "groq" - - def test_openai_nothing_available(self, monkeypatch): - """When no openai key and no local, returns none.""" + def test_explicit_openai_no_key_returns_none(self, monkeypatch): + """Explicit openai with no key returns none — no cross-provider fallback.""" monkeypatch.delenv("VOICE_TOOLS_OPENAI_KEY", raising=False) monkeypatch.delenv("GROQ_API_KEY", raising=False) with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \ @@ -136,18 +118,83 @@ class TestGetProviderFallbackPriority: from tools.transcription_tools import _get_provider assert _get_provider({}) == "local" - def test_openai_fallback_to_local_command(self, monkeypatch): - monkeypatch.delenv("VOICE_TOOLS_OPENAI_KEY", raising=False) - monkeypatch.delenv("OPENAI_API_KEY", raising=False) + +# ============================================================================ +# Explicit provider config respected (GH-1774) +# ============================================================================ + +class TestExplicitProviderRespected: + """When stt.provider is explicitly set, that choice is authoritative. + No silent fallback to a different cloud provider.""" + + def test_explicit_local_no_fallback_to_openai(self, monkeypatch): + """GH-1774: provider=local must not silently fall back to openai + even when an OpenAI API key is set.""" + monkeypatch.setenv("OPENAI_API_KEY", "sk-real-key-here") monkeypatch.delenv("GROQ_API_KEY", raising=False) + with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \ + patch("tools.transcription_tools._HAS_OPENAI", True): + from tools.transcription_tools import _get_provider + result = _get_provider({"provider": "local"}) + assert result == "none", f"Expected 'none' but got {result!r}" + + def test_explicit_local_no_fallback_to_groq(self, monkeypatch): + monkeypatch.setenv("GROQ_API_KEY", "gsk-test") + with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \ + patch("tools.transcription_tools._HAS_OPENAI", True): + from tools.transcription_tools import _get_provider + result = _get_provider({"provider": "local"}) + assert result == "none" + + def test_explicit_local_uses_local_command_fallback(self, monkeypatch): + """Local-to-local_command fallback is fine — both are local.""" monkeypatch.setenv( "HERMES_LOCAL_STT_COMMAND", "whisper {input_path} --output_dir {output_dir} --language {language}", ) + with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False): + from tools.transcription_tools import _get_provider + result = _get_provider({"provider": "local"}) + assert result == "local_command" + + def test_explicit_groq_no_fallback_to_openai(self, monkeypatch): + monkeypatch.delenv("GROQ_API_KEY", raising=False) + monkeypatch.setenv("OPENAI_API_KEY", "sk-real-key") with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \ patch("tools.transcription_tools._HAS_OPENAI", True): from tools.transcription_tools import _get_provider - assert _get_provider({"provider": "openai"}) == "local_command" + result = _get_provider({"provider": "groq"}) + assert result == "none" + + def test_explicit_openai_no_fallback_to_groq(self, monkeypatch): + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + monkeypatch.delenv("VOICE_TOOLS_OPENAI_KEY", raising=False) + monkeypatch.setenv("GROQ_API_KEY", "gsk-test") + with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \ + patch("tools.transcription_tools._HAS_OPENAI", True): + from tools.transcription_tools import _get_provider + result = _get_provider({"provider": "openai"}) + assert result == "none" + + def test_auto_detect_still_falls_back_to_cloud(self, monkeypatch): + """When no provider is explicitly set, auto-detect cloud fallback works.""" + monkeypatch.setenv("OPENAI_API_KEY", "sk-real-key") + monkeypatch.delenv("GROQ_API_KEY", raising=False) + with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \ + patch("tools.transcription_tools._HAS_OPENAI", True): + from tools.transcription_tools import _get_provider + # Empty dict = no explicit provider, uses DEFAULT_PROVIDER auto-detect + result = _get_provider({}) + assert result == "openai" + + def test_auto_detect_prefers_groq_over_openai(self, monkeypatch): + monkeypatch.setenv("GROQ_API_KEY", "gsk-test") + monkeypatch.setenv("OPENAI_API_KEY", "sk-real-key") + with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \ + patch("tools.transcription_tools._HAS_OPENAI", True): + from tools.transcription_tools import _get_provider + result = _get_provider({}) + assert result == "groq" # ============================================================================ @@ -686,28 +733,19 @@ class TestTranscribeAudioDispatch: assert "faster-whisper" in result["error"] assert "GROQ_API_KEY" in result["error"] - def test_openai_provider_falls_back_to_local_command(self, monkeypatch, sample_ogg): + def test_explicit_openai_no_key_returns_error(self, monkeypatch, sample_ogg): + """Explicit provider=openai with no key returns an error, not a fallback.""" monkeypatch.delenv("VOICE_TOOLS_OPENAI_KEY", raising=False) monkeypatch.delenv("OPENAI_API_KEY", raising=False) - monkeypatch.setenv( - "HERMES_LOCAL_STT_COMMAND", - "whisper {input_path} --model {model} --output_dir {output_dir} --language {language}", - ) with patch("tools.transcription_tools._load_stt_config", return_value={"provider": "openai"}), \ patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \ - patch("tools.transcription_tools._HAS_OPENAI", True), \ - patch("tools.transcription_tools._transcribe_local_command", return_value={ - "success": True, - "transcript": "hello from fallback", - "provider": "local_command", - }) as mock_local_command: + patch("tools.transcription_tools._HAS_OPENAI", True): from tools.transcription_tools import transcribe_audio result = transcribe_audio(sample_ogg) - assert result["success"] is True - assert result["transcript"] == "hello from fallback" - mock_local_command.assert_called_once_with(sample_ogg, "base") + assert result["success"] is False + assert "No STT provider" in result["error"] def test_invalid_file_short_circuits(self): from tools.transcription_tools import transcribe_audio diff --git a/tools/transcription_tools.py b/tools/transcription_tools.py index d279dbd3..bae0893e 100644 --- a/tools/transcription_tools.py +++ b/tools/transcription_tools.py @@ -164,76 +164,72 @@ def _normalize_local_command_model(model_name: Optional[str]) -> str: def _get_provider(stt_config: dict) -> str: """Determine which STT provider to use. - Priority: - 1. Explicit config value (``stt.provider``) - 2. Auto-detect: local > groq (free) > openai (paid) - 3. Disabled (returns "none") + When ``stt.provider`` is explicitly set in config, that choice is + honoured — no silent cloud fallback. When no provider is configured, + auto-detect tries: local > groq (free) > openai (paid). """ if not is_stt_enabled(stt_config): return "none" + explicit = "provider" in stt_config provider = stt_config.get("provider", DEFAULT_PROVIDER) - if provider == "local": - if _HAS_FASTER_WHISPER: - return "local" - if _has_local_command(): - logger.info("faster-whisper not installed, falling back to local STT command") - return "local_command" - # Local requested but not available — fall back to groq, then openai - if _HAS_OPENAI and os.getenv("GROQ_API_KEY"): - logger.info("faster-whisper not installed, falling back to Groq Whisper API") - return "groq" - if _HAS_OPENAI and _resolve_openai_api_key(): - logger.info("faster-whisper not installed, falling back to OpenAI Whisper API") - return "openai" - return "none" + # --- Explicit provider: respect the user's choice ---------------------- - if provider == "local_command": - if _has_local_command(): - return "local_command" - if _HAS_FASTER_WHISPER: - logger.info("Local STT command unavailable, falling back to local faster-whisper") - return "local" - if _HAS_OPENAI and os.getenv("GROQ_API_KEY"): - logger.info("Local STT command unavailable, falling back to Groq Whisper API") - return "groq" - if _HAS_OPENAI and _resolve_openai_api_key(): - logger.info("Local STT command unavailable, falling back to OpenAI Whisper API") - return "openai" - return "none" + if explicit: + if provider == "local": + if _HAS_FASTER_WHISPER: + return "local" + if _has_local_command(): + return "local_command" + logger.warning( + "STT provider 'local' configured but unavailable " + "(install faster-whisper or set HERMES_LOCAL_STT_COMMAND)" + ) + return "none" - if provider == "groq": - if _HAS_OPENAI and os.getenv("GROQ_API_KEY"): - return "groq" - # Groq requested but no key — fall back - if _HAS_FASTER_WHISPER: - logger.info("GROQ_API_KEY not set, falling back to local faster-whisper") - return "local" - if _has_local_command(): - logger.info("GROQ_API_KEY not set, falling back to local STT command") - return "local_command" - if _HAS_OPENAI and _resolve_openai_api_key(): - logger.info("GROQ_API_KEY not set, falling back to OpenAI Whisper API") - return "openai" - return "none" + if provider == "local_command": + if _has_local_command(): + return "local_command" + if _HAS_FASTER_WHISPER: + logger.info("Local STT command unavailable, using local faster-whisper") + return "local" + logger.warning( + "STT provider 'local_command' configured but unavailable" + ) + return "none" - if provider == "openai": - if _HAS_OPENAI and _resolve_openai_api_key(): - return "openai" - # OpenAI requested but no key — fall back - if _HAS_FASTER_WHISPER: - logger.info("OpenAI STT key not set, falling back to local faster-whisper") - return "local" - if _has_local_command(): - logger.info("OpenAI STT key not set, falling back to local STT command") - return "local_command" - if _HAS_OPENAI and os.getenv("GROQ_API_KEY"): - logger.info("OpenAI STT key not set, falling back to Groq Whisper API") - return "groq" - return "none" + if provider == "groq": + if _HAS_OPENAI and os.getenv("GROQ_API_KEY"): + return "groq" + logger.warning( + "STT provider 'groq' configured but GROQ_API_KEY not set" + ) + return "none" - return provider # Unknown — let it fail downstream + if provider == "openai": + if _HAS_OPENAI and _resolve_openai_api_key(): + return "openai" + logger.warning( + "STT provider 'openai' configured but no API key available" + ) + return "none" + + return provider # Unknown — let it fail downstream + + # --- Auto-detect (no explicit provider): local > groq > openai --------- + + if _HAS_FASTER_WHISPER: + return "local" + if _has_local_command(): + return "local_command" + if _HAS_OPENAI and os.getenv("GROQ_API_KEY"): + logger.info("No local STT available, using Groq Whisper API") + return "groq" + if _HAS_OPENAI and _resolve_openai_api_key(): + logger.info("No local STT available, using OpenAI Whisper API") + return "openai" + return "none" # --------------------------------------------------------------------------- # Shared validation