diff --git a/api/config.py b/api/config.py index 68edaf8..9c4a984 100644 --- a/api/config.py +++ b/api/config.py @@ -367,14 +367,16 @@ _PROVIDER_MODELS = { def resolve_model_provider(model_id: str) -> tuple: - """Resolve bare model name, provider, and base_url for AIAgent. + """Resolve model name, provider, and base_url for AIAgent. - Model IDs from the dropdown may include a provider prefix - (e.g. 'anthropic/claude-sonnet-4.6'). Direct-API providers expect - bare model names, while OpenRouter expects the full provider/model path. + Model IDs from the dropdown can be in several formats: + - 'claude-sonnet-4.6' (bare name, uses config default provider) + - 'anthropic/claude-sonnet-4.6' (OpenRouter format, provider/model) + - '@minimax:MiniMax-M2.7' (explicit provider hint from dropdown) - Also reads base_url from config.yaml so providers with custom endpoints - (e.g. MiniMax, Z.AI) are routed correctly. + The @provider:model format is used for models from non-default provider + groups in the dropdown, so we can route them through the correct provider + via resolve_runtime_provider(requested=provider) instead of the default. Returns (model, provider, base_url) where provider and base_url may be None. """ @@ -389,6 +391,13 @@ def resolve_model_provider(model_id: str) -> tuple: if not model_id: return model_id, config_provider, config_base_url + # @provider:model format — explicit provider hint from the dropdown. + # Route through that provider directly (resolve_runtime_provider will + # resolve credentials in streaming.py). + if model_id.startswith('@') and ':' in model_id: + provider_hint, bare_model = model_id[1:].split(':', 1) + return bare_model, provider_hint, None + if '/' in model_id: prefix, bare = model_id.split('/', 1) # OpenRouter always needs the full provider/model path (e.g. openrouter/free, @@ -402,7 +411,6 @@ def resolve_model_provider(model_id: str) -> tuple: # If prefix does NOT match config provider, the user picked a cross-provider model # from the OpenRouter dropdown (e.g. config=anthropic but picked openai/gpt-5.4-mini). # In this case always route through openrouter with the full provider/model string. - # Never strip the prefix and try a direct-API call to a provider whose key may not exist. if prefix in _PROVIDER_MODELS and prefix != config_provider: return model_id, 'openrouter', None @@ -585,23 +593,23 @@ def get_available_models() -> dict: 'models': [{'id': m['id'], 'label': m['label']} for m in _FALLBACK_MODELS], }) elif pid in _PROVIDER_MODELS: - # For non-default providers, prefix model IDs with provider name - # so resolve_model_provider() can route them correctly (e.g. - # \"minimax/MiniMax-M2.7\" instead of bare \"MiniMax-M2.7\"). + # For non-default providers, prefix model IDs with @provider:model + # so resolve_model_provider() routes through that specific provider + # via resolve_runtime_provider(requested=provider). # The default provider's models keep bare names for direct API routing. - # Guard: only prefix when we have a confirmed active_provider, and - # normalise case before comparing (config.yaml may use 'Anthropic'). raw_models = _PROVIDER_MODELS[pid] _active = (active_provider or '').lower() if _active and pid != _active: - # Shallow copy — don't mutate the shared _PROVIDER_MODELS list. - # Bare IDs get prefixed; already-prefixed IDs pass through as-is. models = [] for m in raw_models: mid = m['id'] - models.append({'id': mid if '/' in mid else f'{pid}/{mid}', 'label': m['label']}) + # Don't double-prefix; use @provider: hint for bare names + if mid.startswith('@') or '/' in mid: + models.append({'id': mid, 'label': m['label']}) + else: + models.append({'id': f'@{pid}:{mid}', 'label': m['label']}) else: - models = list(raw_models) # shallow copy to protect against insert() mutations + models = list(raw_models) groups.append({ 'provider': provider_name, 'models': models, diff --git a/api/routes.py b/api/routes.py index d7ff022..bd218e6 100644 --- a/api/routes.py +++ b/api/routes.py @@ -1061,7 +1061,7 @@ def _handle_chat_sync(handler, body): _api_key = None try: from hermes_cli.runtime_provider import resolve_runtime_provider - _rt = resolve_runtime_provider() + _rt = resolve_runtime_provider(requested=_provider) _api_key = _rt.get("api_key") # Also use runtime provider/base_url if the webui config didn't resolve them if not _provider: diff --git a/api/streaming.py b/api/streaming.py index 55922ae..234f290 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -143,11 +143,12 @@ def _run_agent_streaming(session_id, msg_text, model, workspace, stream_id, atta raise ImportError("AIAgent not available -- check that hermes-agent is on sys.path") resolved_model, resolved_provider, resolved_base_url = resolve_model_provider(model) - # Resolve API key via Hermes runtime provider (matches gateway behaviour) + # Resolve API key via Hermes runtime provider (matches gateway behaviour). + # Pass the resolved provider so non-default providers get their own credentials. resolved_api_key = None try: from hermes_cli.runtime_provider import resolve_runtime_provider - _rt = resolve_runtime_provider() + _rt = resolve_runtime_provider(requested=resolved_provider) resolved_api_key = _rt.get("api_key") if not resolved_provider: resolved_provider = _rt.get("provider") diff --git a/tests/test_model_resolver.py b/tests/test_model_resolver.py index fe502bf..38efd9e 100644 --- a/tests/test_model_resolver.py +++ b/tests/test_model_resolver.py @@ -100,10 +100,38 @@ def test_empty_model_returns_config_defaults(): assert provider == 'anthropic' -# ── Non-default provider prefix routing (Issue #138) ──────────────────── +# ── @provider:model hint routing (Issue #138 v2) ──────────────────────── -def test_prefixed_non_default_provider_routes_through_openrouter(): - """minimax/MiniMax-M2.7 with anthropic as default should route via openrouter.""" +def test_provider_hint_routes_to_specific_provider(): + """@minimax:MiniMax-M2.7 routes to minimax provider directly.""" + model, provider, base_url = _resolve_with_config( + '@minimax:MiniMax-M2.7', provider='anthropic', + ) + assert model == 'MiniMax-M2.7' + assert provider == 'minimax' + assert base_url is None # resolve_runtime_provider will fill this + + +def test_provider_hint_zai(): + """@zai:GLM-5 routes to zai provider directly.""" + model, provider, base_url = _resolve_with_config( + '@zai:GLM-5', provider='openai', + ) + assert model == 'GLM-5' + assert provider == 'zai' + + +def test_provider_hint_deepseek(): + """@deepseek:deepseek-chat routes to deepseek provider.""" + model, provider, base_url = _resolve_with_config( + '@deepseek:deepseek-chat', provider='anthropic', + ) + assert model == 'deepseek-chat' + assert provider == 'deepseek' + + +def test_slash_prefix_non_default_still_routes_openrouter(): + """minimax/MiniMax-M2.7 (old format) still routes through openrouter.""" model, provider, base_url = _resolve_with_config( 'minimax/MiniMax-M2.7', provider='anthropic', ) @@ -111,19 +139,10 @@ def test_prefixed_non_default_provider_routes_through_openrouter(): assert provider == 'openrouter' -def test_prefixed_non_default_provider_zai(): - """zai/GLM-5 with openai as default should route via openrouter.""" - model, provider, base_url = _resolve_with_config( - 'zai/GLM-5', provider='openai', - ) - assert model == 'zai/GLM-5' - assert provider == 'openrouter' - - -# ── get_available_models() prefix behaviour ─────────────────────────────── +# ── get_available_models() @provider: hint behaviour ────────────────────── def _available_models_with_provider(provider): - """Helper: temporarily set active_provider in auth store simulation via config.cfg.""" + """Helper: temporarily set active_provider in config.""" old_cfg = dict(config.cfg) config.cfg['model'] = {'provider': provider} try: @@ -133,48 +152,26 @@ def _available_models_with_provider(provider): config.cfg.update(old_cfg) -def test_non_default_provider_models_are_prefixed(): - """With anthropic as default, minimax model IDs should be prefixed 'minimax/...'.""" +def test_non_default_provider_models_use_hint_prefix(): + """With anthropic as default, minimax model IDs should use @minimax: prefix.""" result = _available_models_with_provider('anthropic') groups = {g['provider']: g['models'] for g in result['groups']} if 'MiniMax' in groups: for m in groups['MiniMax']: - assert m['id'].startswith('minimax/'), ( - f"Expected minimax/ prefix, got: {m['id']!r}" + assert m['id'].startswith('@minimax:'), ( + f"Expected @minimax: prefix, got: {m['id']!r}" ) def test_default_provider_models_not_prefixed(): - """The active provider's _PROVIDER_MODELS entries remain bare (no prefix added).""" + """The active provider's models remain bare (no @prefix added).""" import api.config as _cfg - # The bare IDs as stored in _PROVIDER_MODELS (e.g. 'claude-sonnet-4.6') raw_anthropic_ids = {m['id'] for m in _cfg._PROVIDER_MODELS.get('anthropic', [])} result = _available_models_with_provider('anthropic') groups = {g['provider']: g['models'] for g in result['groups']} if 'Anthropic' in groups: returned_ids = {m['id'] for m in groups['Anthropic']} - # Every bare _PROVIDER_MODELS ID must still appear bare (not turned into 'anthropic/...') for bare_id in raw_anthropic_ids: assert bare_id in returned_ids, ( - f"_PROVIDER_MODELS entry '{bare_id}' is missing from the Anthropic group " - f"(returned: {sorted(returned_ids)})" + f"_PROVIDER_MODELS entry '{bare_id}' is missing from the Anthropic group" ) - - -def test_no_active_provider_models_not_prefixed(): - """With no confirmed active_provider, models should not be prefixed.""" - old_cfg = dict(config.cfg) - config.cfg['model'] = {} # no provider set - try: - result = config.get_available_models() - for g in result['groups']: - for m in g['models']: - # No model should have a double-prefix like 'minimax/minimax/...' - parts = m['id'].split('/') - if len(parts) >= 2: - assert parts[0] != parts[1], ( - f"Double-prefix detected: {m['id']!r}" - ) - finally: - config.cfg.clear() - config.cfg.update(old_cfg)