fix: prefix non-default provider model IDs for correct routing (#142)
* fix: prefix non-default provider model IDs for correct routing When multiple providers are configured, models from non-default providers (e.g. MiniMax when Anthropic is default) were sent as bare names without provider context. resolve_model_provider() couldn't determine the target provider and routed them to the default provider's API, which failed. Fix: get_available_models() now prefixes model IDs with the provider name (e.g. minimax/MiniMax-M2.7) for providers that are NOT the active config provider. The default provider's models keep bare names for direct API routing. This matches the existing pattern for OpenRouter models. Added 2 tests to test_model_resolver.py for cross-provider routing. Closes #138 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: model prefix — null-guard, case normalization, mutation safety, tests Four fixes on top of original PR: - active_provider=None guard: without a confirmed provider all models were being prefixed. Only prefix when active_provider is set. - Case normalisation: compare pid against active_provider.lower() so config.yaml entries like 'Anthropic' match pid 'anthropic'. - Mutation safety: default branch used raw reference to _PROVIDER_MODELS[pid]; the default_model injector later calls list.insert() on that reference, permanently mutating the shared constant. Both branches now use a copy. - Already-prefixed model IDs pass through as-is (no double-prefix). Added 3 tests for get_available_models() prefix behaviour: - Non-default provider models are prefixed - Active provider's own entries remain bare - No double-prefix when active_provider is absent --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -585,9 +585,26 @@ 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\").
|
||||
# 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']})
|
||||
else:
|
||||
models = list(raw_models) # shallow copy to protect against insert() mutations
|
||||
groups.append({
|
||||
'provider': provider_name,
|
||||
'models': _PROVIDER_MODELS[pid],
|
||||
'models': models,
|
||||
})
|
||||
else:
|
||||
# Unknown provider -- use auto-detected models if available,
|
||||
|
||||
@@ -98,3 +98,83 @@ def test_empty_model_returns_config_defaults():
|
||||
)
|
||||
assert model == ''
|
||||
assert provider == 'anthropic'
|
||||
|
||||
|
||||
# ── Non-default provider prefix routing (Issue #138) ────────────────────
|
||||
|
||||
def test_prefixed_non_default_provider_routes_through_openrouter():
|
||||
"""minimax/MiniMax-M2.7 with anthropic as default should route via openrouter."""
|
||||
model, provider, base_url = _resolve_with_config(
|
||||
'minimax/MiniMax-M2.7', provider='anthropic',
|
||||
)
|
||||
assert model == 'minimax/MiniMax-M2.7'
|
||||
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 ───────────────────────────────
|
||||
|
||||
def _available_models_with_provider(provider):
|
||||
"""Helper: temporarily set active_provider in auth store simulation via config.cfg."""
|
||||
old_cfg = dict(config.cfg)
|
||||
config.cfg['model'] = {'provider': provider}
|
||||
try:
|
||||
return config.get_available_models()
|
||||
finally:
|
||||
config.cfg.clear()
|
||||
config.cfg.update(old_cfg)
|
||||
|
||||
|
||||
def test_non_default_provider_models_are_prefixed():
|
||||
"""With anthropic as default, minimax model IDs should be prefixed 'minimax/...'."""
|
||||
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}"
|
||||
)
|
||||
|
||||
|
||||
def test_default_provider_models_not_prefixed():
|
||||
"""The active provider's _PROVIDER_MODELS entries 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)})"
|
||||
)
|
||||
|
||||
|
||||
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)
|
||||
|
||||
Reference in New Issue
Block a user