From e68c1b92a42fc55d5943fd6cfb2bc1d2e2a80c06 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Thu, 9 Apr 2026 18:33:24 -0700 Subject: [PATCH] fix: do not build phantom Custom group when active provider is set (#206) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: do not build phantom "Custom" group when active provider is set When model.provider is a real provider (e.g. openai-codex) and model.base_url is configured, hermes_cli reports 'custom' as an authenticated provider. The WebUI model picker was building a separate "Custom" group for it and parking the configured default_model there instead of under the active provider's group — diverging from the TUI which correctly shows the model under its configured provider. Two fixes in api/config.py get_available_models(): 1. Discard 'custom' from detected_providers when active_provider is set and isn't 'custom' itself. The base_url belongs to the active provider. 2. Replace the substring-based default-model injection check with an exact match against _PROVIDER_DISPLAY. The old check `active_provider.lower() in g.get('provider', '').lower()` silently failed for hyphenated IDs like 'openai-codex' vs display name 'OpenAI Codex' (hyphen vs. space), falling through to groups[0] and landing the model in the alphabetical first group instead. Adds two regression tests in tests/test_model_resolver.py covering both conditions. * fix: do not build phantom Custom group when active provider is set Two bugs in get_available_models(): 1. Phantom Custom group: hermes_cli reports 'custom' as authenticated whenever model.base_url is set. With provider=openai-codex + base_url, detected_providers contained both 'openai-codex' and 'custom', producing a duplicate group. Fixed by discarding 'custom' from detected_providers when the active provider is any real named provider. 2. Hyphen/space mismatch in default_model injection: the substring check 'openai-codex' in 'openai codex' is False (hyphen vs space), causing the default model to fall through to groups[0] (alphabetically first provider) instead of the active provider group. Fixed by using _PROVIDER_DISPLAY for exact display-name comparison. Also fixes test helper _available_models_with_full_cfg to clear model env vars during the call, preventing real hermes profile env from leaking into the test assertions. --------- Co-authored-by: mbac Co-authored-by: Nathan Esquenazi --- api/config.py | 22 +++++++- tests/test_model_resolver.py | 105 +++++++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 2 deletions(-) diff --git a/api/config.py b/api/config.py index d0490dd..2f6e13c 100644 --- a/api/config.py +++ b/api/config.py @@ -680,6 +680,14 @@ def get_available_models() -> dict: _seen_custom_ids.add(_cp_model) detected_providers.add('custom') + # If the user configured a real model.provider, the base_url belongs to + # THAT provider, not to a separate "Custom" group. hermes_cli reports + # 'custom' as authenticated whenever base_url is set, which would otherwise + # build a phantom "Custom" bucket next to the real provider's group. Drop + # it unless the user explicitly chose 'custom' as their active provider. + if active_provider and active_provider != 'custom': + detected_providers.discard('custom') + # 5. Build model groups if detected_providers: for pid in sorted(detected_providers): @@ -741,11 +749,21 @@ def get_available_models() -> dict: _norm = lambda mid: mid.split('/', 1)[-1] if '/' in mid else mid all_ids_norm = {_norm(m['id']) for g in groups for m in g.get('models', [])} if _norm(default_model) not in all_ids_norm: - # Determine which group to inject into + # Determine which group to inject into. Compare against the + # provider's display name from _PROVIDER_DISPLAY rather than + # doing a substring match on active_provider — substring + # matching breaks on hyphenated provider IDs like 'openai-codex' + # vs display name 'OpenAI Codex' (hyphen vs. space), which + # silently falls through to groups[0] and lands the model in + # the wrong group. label = default_model.split('/')[-1] if '/' in default_model else default_model + target_display = ( + _PROVIDER_DISPLAY.get(active_provider, active_provider or '').lower() + if active_provider else '' + ) injected = False for g in groups: - if active_provider and active_provider.lower() in g.get('provider', '').lower(): + if target_display and g.get('provider', '').lower() == target_display: g['models'].insert(0, {'id': default_model, 'label': label}) injected = True break diff --git a/tests/test_model_resolver.py b/tests/test_model_resolver.py index 16bcf2a..d9713d2 100644 --- a/tests/test_model_resolver.py +++ b/tests/test_model_resolver.py @@ -221,3 +221,108 @@ def test_default_provider_models_not_prefixed(): assert bare_id in returned_ids, ( f"_PROVIDER_MODELS entry '{bare_id}' is missing from the Anthropic group" ) + + +# ── get_available_models(): phantom "Custom" group regression ───────────── +# +# When the user has model.provider set to a real provider (e.g. openai-codex) +# AND a model.base_url set, hermes_cli reports the 'custom' pseudo-provider as +# authenticated. The WebUI picker must NOT build a separate "Custom" group in +# that case — the base_url belongs to the active provider. + +def _available_models_with_full_cfg(provider, default, base_url): + """Helper: set model.provider, model.default, model.base_url at once. + + Clears model-override env vars (HERMES_MODEL, OPENAI_MODEL, LLM_MODEL) + during the call so the real hermes profile environment doesn't leak into + the test and override the fixture's default model. + """ + import os + import api.config as _cfg + old_cfg = dict(_cfg.cfg) + _cfg.cfg['model'] = { + 'provider': provider, + 'default': default, + 'base_url': base_url, + } + # Clear model-override env vars to prevent the real profile from leaking in + _model_env_keys = ('HERMES_MODEL', 'OPENAI_MODEL', 'LLM_MODEL') + _saved_env = {k: os.environ.pop(k, None) for k in _model_env_keys} + try: + return _cfg.get_available_models() + finally: + _cfg.cfg.clear() + _cfg.cfg.update(old_cfg) + for k, v in _saved_env.items(): + if v is not None: + os.environ[k] = v + + +def test_no_phantom_custom_group_when_active_provider_is_set(monkeypatch): + """Issue: with provider=openai-codex + base_url set, gpt-5.4 was landing + under a phantom "Custom" group instead of the "OpenAI Codex" group.""" + import sys, types + + # Force hermes_cli to report both the real provider and the phantom + # 'custom' as authenticated, simulating what list_available_providers() + # returns when base_url is configured. + fake_mod = types.ModuleType('hermes_cli.models') + fake_mod.list_available_providers = lambda: [ + {'id': 'openai-codex', 'authenticated': True}, + {'id': 'custom', 'authenticated': True}, + ] + fake_auth = types.ModuleType('hermes_cli.auth') + fake_auth.get_auth_status = lambda pid: {'key_source': 'env'} + monkeypatch.setitem(sys.modules, 'hermes_cli.models', fake_mod) + monkeypatch.setitem(sys.modules, 'hermes_cli.auth', fake_auth) + + result = _available_models_with_full_cfg( + provider='openai-codex', + default='gpt-5.4', + base_url='https://chatgpt.com/backend-api/codex', + ) + group_names = [g['provider'] for g in result['groups']] + assert 'Custom' not in group_names, ( + f"Phantom 'Custom' group present; full groups: {group_names}" + ) + + +def test_default_model_lands_under_active_provider_group(monkeypatch): + """The configured default_model must appear under the active provider's + display group, even when the model isn't in _PROVIDER_MODELS[provider] + AND the active provider isn't the alphabetical first detected provider. + + Regression guard for a hyphen-vs-space bug in the "ensure default_model + appears" post-pass: the substring check `active_provider.lower() in + g.get('provider', '').lower()` was failing for 'openai-codex' vs + display name 'OpenAI Codex' (hyphen vs. space), silently falling back + to groups[0] — which, when another provider sorted earlier + alphabetically (e.g. 'anthropic'), placed gpt-5.4 in the WRONG group. + """ + import sys, types + fake_mod = types.ModuleType('hermes_cli.models') + fake_mod.list_available_providers = lambda: [ + {'id': 'anthropic', 'authenticated': True}, # sorts before openai-codex + {'id': 'openai-codex', 'authenticated': True}, + {'id': 'custom', 'authenticated': True}, + ] + fake_auth = types.ModuleType('hermes_cli.auth') + fake_auth.get_auth_status = lambda pid: {'key_source': 'env'} + monkeypatch.setitem(sys.modules, 'hermes_cli.models', fake_mod) + monkeypatch.setitem(sys.modules, 'hermes_cli.auth', fake_auth) + + result = _available_models_with_full_cfg( + provider='openai-codex', + default='gpt-5.4', + base_url='https://chatgpt.com/backend-api/codex', + ) + groups = {g['provider']: [m['id'] for m in g['models']] for g in result['groups']} + assert 'OpenAI Codex' in groups, f"OpenAI Codex group missing: {list(groups)}" + assert 'gpt-5.4' in groups['OpenAI Codex'], ( + f"gpt-5.4 not in OpenAI Codex group; contents: {groups['OpenAI Codex']}" + ) + # And crucially, it must NOT have landed in the alphabetically-first + # group (Anthropic) via the fallback path. + assert 'gpt-5.4' not in groups.get('Anthropic', []), ( + f"gpt-5.4 leaked into Anthropic group via fallback: {groups.get('Anthropic')}" + )