From aa3c5e59d3e837580892bac7165a84facf7c7b4d Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:49:32 -0700 Subject: [PATCH] fix(tools): honor raw stt.provider: local; finish _reconfigure_provider provider-string migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real gaps the CI-red sibling tests exposed: - read_selection() treated EVERY raw stt.provider: local as the legacy DEFAULT_CONFIG seed and reported no-selection — but the seed never reached config.yaml (save_config strips schema defaults), so a picker- or hand-written local pick was silently discarded and the autodetect ladder could route an explicit local user to cloud STT. A raw 'local' is now a genuine selection; the merged-view ambiguity note replaces the over-broad shim (mirror comment updated in nous_subscription._selected_provider and _get_provider). - _reconfigure_provider was half-migrated: the tts/stt/browser/web branches and the managed-category fallthrough still wrote use_gateway flags and vendor names for managed rows. They now write the single provider string ('nous' for managed rows) and pop the legacy key, matching _write_provider_config. --- hermes_cli/nous_subscription.py | 15 +++----- hermes_cli/tools_config.py | 36 +++++++++++++------ tests/tools/test_strict_provider_selection.py | 11 +++--- tools/tool_backend_helpers.py | 14 ++++---- tools/transcription_tools.py | 7 ++-- 5 files changed, 48 insertions(+), 35 deletions(-) diff --git a/hermes_cli/nous_subscription.py b/hermes_cli/nous_subscription.py index 882956fe31..01d10d12f4 100644 --- a/hermes_cli/nous_subscription.py +++ b/hermes_cli/nous_subscription.py @@ -475,16 +475,11 @@ def get_nous_subscription_features( image_selected = _selected_provider(image_gen_cfg) video_selected = _selected_provider(video_gen_cfg) - # Same seeded-value shim as tools.tool_backend_helpers.read_selection: - # legacy DEFAULT_CONFIG seeded ``stt.provider: local`` on every install, - # so that exact value with no picker-written use_gateway key is treated - # as never-configured. - if ( - stt_selected == "local" - and isinstance(stt_cfg, dict) - and "use_gateway" not in stt_cfg - ): - stt_selected = None + # Lockstep with tools.tool_backend_helpers.read_selection: these are + # merged-config sections, so the legacy DEFAULT_CONFIG-seeded + # ``stt.provider: local`` COULD appear here without a user pick on old + # versions. Current DEFAULT_CONFIG no longer seeds it, so a merged + # ``local`` implies the raw file holds it — a genuine selection. # Managed selection flags (replace the legacy use_gateway reads — # use_gateway is now interpreted only inside _selected_provider). diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index a724b4888f..cea0d5f792 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -5084,22 +5084,33 @@ def _reconfigure_provider( ) return + # Selection model (mirrors _write_provider_config): every row writes ONE + # provider string per category — "nous" for managed rows, the vendor name + # for BYOK rows — and drops any legacy use_gateway key so the read-time + # shim (use_gateway: true ⇒ nous) cannot override the fresh pick. if provider.get("tts_provider"): tts_cfg = config.setdefault("tts", {}) - tts_cfg["provider"] = provider["tts_provider"] - tts_cfg["use_gateway"] = bool(managed_feature) + tts_cfg["provider"] = ( + NOUS_MANAGED_PROVIDER if managed_feature else provider["tts_provider"] + ) + tts_cfg.pop("use_gateway", None) _print_success(f" TTS provider set to: {provider['tts_provider']}") if provider.get("stt_provider"): stt_cfg = config.setdefault("stt", {}) - stt_cfg["provider"] = provider["stt_provider"] - stt_cfg["use_gateway"] = bool(managed_feature) + stt_cfg["provider"] = ( + NOUS_MANAGED_PROVIDER if managed_feature else provider["stt_provider"] + ) + stt_cfg.pop("use_gateway", None) _print_success(f" STT provider set to: {provider['stt_provider']}") if "browser_provider" in provider: bp = provider["browser_provider"] browser_cfg = config.setdefault("browser", {}) - if bp == "local": + if managed_feature: + browser_cfg["cloud_provider"] = NOUS_MANAGED_PROVIDER + _print_success(f" Browser cloud provider set to: {bp or 'nous'}") + elif bp == "local": browser_cfg["cloud_provider"] = "local" _print_success(" Browser set to local mode") elif bp: @@ -5107,7 +5118,7 @@ def _reconfigure_provider( _print_success(f" Browser cloud provider set to: {bp}") # Browser Use mode (browser.backend) composes with the provider — # switching providers keeps the driver choice intact. - browser_cfg["use_gateway"] = bool(managed_feature) + browser_cfg.pop("use_gateway", None) if provider.get("browser_backend"): browser_cfg = config.setdefault("browser", {}) @@ -5117,8 +5128,10 @@ def _reconfigure_provider( # Set web search backend in config if applicable if provider.get("web_backend"): web_cfg = config.setdefault("web", {}) - web_cfg["backend"] = provider["web_backend"] - web_cfg["use_gateway"] = bool(managed_feature) + web_cfg["backend"] = ( + NOUS_MANAGED_PROVIDER if managed_feature else provider["web_backend"] + ) + web_cfg.pop("use_gateway", None) _print_success(f" Web backend set to: {provider['web_backend']}") # Set computer_use backend in config if applicable @@ -5132,13 +5145,14 @@ def _reconfigure_provider( if not isinstance(section, dict): section = {} config[managed_feature] = section - section["use_gateway"] = True + section["provider"] = NOUS_MANAGED_PROVIDER + section.pop("use_gateway", None) elif not managed_feature: for cat_key, cat in TOOL_CATEGORIES.items(): if provider in cat.get("providers", []): section = config.get(cat_key) - if isinstance(section, dict) and section.get("use_gateway"): - section["use_gateway"] = False + if isinstance(section, dict): + section.pop("use_gateway", None) break if not env_vars: diff --git a/tests/tools/test_strict_provider_selection.py b/tests/tools/test_strict_provider_selection.py index cbb158e47a..aed8ae9e32 100644 --- a/tests/tools/test_strict_provider_selection.py +++ b/tests/tools/test_strict_provider_selection.py @@ -66,11 +66,14 @@ class TestReadSelection: with self._with_raw({"web": {"backend": ""}}): assert tbh.read_selection("web") is None - def test_seeded_stt_local_is_no_selection(self): - """Legacy DEFAULT_CONFIG seeded stt.provider: local on every - install; that value alone must be treated as never-configured.""" + def test_raw_stt_local_is_a_selection(self): + """A raw config.yaml ``stt.provider: local`` is a genuine pick: the + DEFAULT_CONFIG seed never reached disk (save_config strips schema + defaults), and the current picker's Local Whisper row writes exactly + this shape (provider only, legacy use_gateway popped). Treating it + as no-selection would silently discard the user's choice.""" with self._with_raw({"stt": {"provider": "local"}}): - assert tbh.read_selection("stt") is None + assert tbh.read_selection("stt") == "local" def test_stt_local_with_use_gateway_key_is_a_selection(self): """A picker-written stt section (use_gateway key present) means diff --git a/tools/tool_backend_helpers.py b/tools/tool_backend_helpers.py index f3c23c9c85..0096c72fdf 100644 --- a/tools/tool_backend_helpers.py +++ b/tools/tool_backend_helpers.py @@ -361,13 +361,13 @@ def read_selection(section: str) -> str | None: if "use_gateway" in raw and is_truthy_value(raw.get("use_gateway"), default=False): return NOUS_MANAGED_PROVIDER - # Migration shim: DEFAULT_CONFIG historically seeded ``stt.provider: - # local`` on every install, so that exact value with no picker-written - # use_gateway key is ambiguous. Treat it as never-configured — the - # autodetect ladder prefers local first anyway, and hard-pinning would - # error every seeded install that lacks faster-whisper. - if section == "stt" and name == "local" and "use_gateway" not in raw: - return None + # NOTE on the legacy DEFAULT_CONFIG ``stt.provider: local`` seed: it never + # reached the raw config.yaml (``save_config`` strips schema defaults), + # and the old picker's Local Whisper row always wrote ``use_gateway: + # False`` beside it. A raw ``local`` here therefore IS a user selection — + # hand-written or picker-written — and is honored like any other vendor + # name. The seeded-value ambiguity only exists in DEFAULT_CONFIG-merged + # views, which this function never reads. if name: return name diff --git a/tools/transcription_tools.py b/tools/transcription_tools.py index 77f4cca296..9a38929d84 100644 --- a/tools/transcription_tools.py +++ b/tools/transcription_tools.py @@ -1034,9 +1034,10 @@ def _get_provider(stt_config: dict) -> str: if explicit and provider == "local": # Legacy DEFAULT_CONFIG seeded ``stt.provider: local`` on every # install, so a merged-config "local" is not proof of a user pick. - # ``read_selection`` reads the raw config.yaml and applies the - # seeded-value migration shim; when the raw file holds no stt - # selection, take the autodetect branch (which prefers local first + # ``read_selection`` reads the raw config.yaml: when the raw file + # holds an stt selection (picker- or hand-written ``local``) it is + # honored; when the merged "local" came only from a legacy default + # merge, take the autodetect branch (which prefers local first # anyway, so a genuine local user is unaffected when it's available). try: from tools.tool_backend_helpers import read_selection