diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index 35b758c45d259..5bf6f5fe765a6 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -916,6 +916,37 @@ def _model_sort_key(model_id: str, prefix: str) -> tuple: return version_key + (suffix_rank, suffix) + date_key +class AmbiguousAliasError(Exception): + """Alias family-matches multiple catalog models; caller must disambiguate. + + Raised by :func:`resolve_alias` instead of silently picking one candidate + via version-sort heuristics. ``candidates`` is sorted best-guess-first + (see :func:`_model_sort_key`) for display purposes only. + """ + + def __init__(self, alias: str, provider: str, candidates: list[str]): + self.alias = alias + self.provider = provider + self.candidates = candidates + super().__init__( + f"alias {alias!r} matches {len(candidates)} models on {provider}" + ) + + +def _ambiguous_alias_message(err: "AmbiguousAliasError") -> str: + """User-facing disambiguation list for an ambiguous alias.""" + shown = err.candidates[:10] + lines = "\n".join(f" {i}. {m}" for i, m in enumerate(shown, 1)) + more = "" + if len(err.candidates) > len(shown): + more = f"\n … and {len(err.candidates) - len(shown)} more" + return ( + f"'{err.alias}' matches {len(err.candidates)} models on " + f"{err.provider} — not switching automatically:\n{lines}{more}\n" + f"Pick one with /model ." + ) + + def resolve_alias( raw_input: str, current_provider: str, @@ -987,9 +1018,15 @@ def resolve_alias( if not matches: return None - # Sort by version descending — prefer the latest/highest version + # Sort by version descending (best guess first) for display, but NEVER + # silently pick among multiple candidates: version-sort heuristics have + # repeatedly guessed wrong (dated snapshots outranking point releases, + # suffix tiebreaks landing on the cheapest tier). One match = resolve; + # several = make the user choose. prefix_for_sort = f"{vendor}/{family}" if aggregator else family matches.sort(key=lambda m: _model_sort_key(m, prefix_for_sort)) + if len(matches) > 1: + raise AmbiguousAliasError(key, current_provider, matches) return (current_provider, matches[0], key) @@ -1026,6 +1063,9 @@ def _resolve_alias_fallback( """ providers = authenticated_providers or ("openrouter", "nous") for provider in providers: + # AmbiguousAliasError propagates: the alias exists on this provider, + # the user just has to choose — trying the next provider instead + # would silently switch them somewhere they didn't ask to go. result = resolve_alias(raw_input, provider) if result is not None: return result @@ -1422,7 +1462,15 @@ def switch_model( ) # Resolve alias on the TARGET provider - alias_result = resolve_alias(new_model, target_provider) + try: + alias_result = resolve_alias(new_model, target_provider) + except AmbiguousAliasError as err: + return ModelSwitchResult( + success=False, + target_provider=target_provider, + is_global=is_global, + error_message=_ambiguous_alias_message(err), + ) if alias_result is not None: _, new_model, resolved_alias = alias_result @@ -1444,8 +1492,21 @@ def switch_model( alias_result = None else: alias_result = resolve_alias(raw_input, current_provider) + except AmbiguousAliasError as err: + return ModelSwitchResult( + success=False, + is_global=is_global, + error_message=_ambiguous_alias_message(err), + ) except Exception: - alias_result = resolve_alias(raw_input, current_provider) + try: + alias_result = resolve_alias(raw_input, current_provider) + except AmbiguousAliasError as err: + return ModelSwitchResult( + success=False, + is_global=is_global, + error_message=_ambiguous_alias_message(err), + ) # --- Step a: Try alias resolution on current provider --- @@ -1466,7 +1527,14 @@ def switch_model( user_providers=user_providers, custom_providers=custom_providers, ) - fallback_result = _resolve_alias_fallback(raw_input, authed) + try: + fallback_result = _resolve_alias_fallback(raw_input, authed) + except AmbiguousAliasError as err: + return ModelSwitchResult( + success=False, + is_global=is_global, + error_message=_ambiguous_alias_message(err), + ) if fallback_result is not None: target_provider, new_model, resolved_alias = fallback_result logger.debug( diff --git a/tests/hermes_cli/test_ollama_cloud_auth.py b/tests/hermes_cli/test_ollama_cloud_auth.py index d6eba2a1c8272..c9b88a66c5e7b 100644 --- a/tests/hermes_cli/test_ollama_cloud_auth.py +++ b/tests/hermes_cli/test_ollama_cloud_auth.py @@ -283,36 +283,82 @@ class TestResolveAliasEdgeCases: # --------------------------------------------------------------------------- class TestResolveAliasSorting: - """resolve_alias must pick the highest-version model, correctly - ignoring YYYYMMDD snapshot stamps that would otherwise dwarf real - version numbers (e.g. claude-opus-4-20250514 vs claude-opus-4-8).""" + """Aliases matching multiple catalog models must NOT silently pick one — + resolve_alias raises AmbiguousAliasError with candidates sorted + best-guess-first (dated snapshots demoted below real point versions).""" + + def test_anthropic_opus_ambiguous_lists_candidates(self, monkeypatch): + """Multiple family matches surface a choice instead of auto-picking; + the display ordering demotes date-stamped snapshots.""" + import pytest - def test_anthropic_opus_picks_latest(self, monkeypatch): - """Date-stamped snapshot ID must not outrank a higher point version.""" import hermes_cli.model_switch as ms + monkeypatch.setattr("hermes_cli.models._PROVIDER_MODELS", {}) monkeypatch.setattr(ms, "_ensure_direct_aliases", lambda: None) monkeypatch.setattr(ms, "DIRECT_ALIASES", {}) monkeypatch.setattr(ms, "list_provider_models", lambda p: ["claude-opus-4-1", "claude-opus-4-7", "claude-opus-4-8", "claude-opus-4-20250514"]) - result = ms.resolve_alias("opus", "anthropic") - assert result is not None and result[1] == "claude-opus-4-8" + with pytest.raises(ms.AmbiguousAliasError) as exc: + ms.resolve_alias("opus", "anthropic") + assert exc.value.candidates[0] == "claude-opus-4-8" + assert set(exc.value.candidates) == { + "claude-opus-4-1", "claude-opus-4-7", + "claude-opus-4-8", "claude-opus-4-20250514", + } + + def test_unsynced_new_model_sorts_first(self, monkeypatch): + """A just-released model missing from models.dev still ranks above + older, dated siblings in the candidate ordering.""" + import pytest - def test_unsynced_new_model_wins(self, monkeypatch): - """A just-released model missing from models.dev still outranks - older, dated siblings (static _PROVIDER_MODELS merge scenario).""" import hermes_cli.model_switch as ms + monkeypatch.setattr("hermes_cli.models._PROVIDER_MODELS", {}) monkeypatch.setattr(ms, "_ensure_direct_aliases", lambda: None) monkeypatch.setattr(ms, "DIRECT_ALIASES", {}) monkeypatch.setattr(ms, "list_provider_models", lambda p: ["claude-opus-4-7", "claude-opus-4-8", "claude-opus-4-20250514", "claude-opus-4-9"]) + with pytest.raises(ms.AmbiguousAliasError) as exc: + ms.resolve_alias("opus", "anthropic") + assert exc.value.candidates[0] == "claude-opus-4-9" + + def test_single_match_resolves_without_error(self, monkeypatch): + """Exactly one family match still resolves automatically.""" + import hermes_cli.model_switch as ms + + monkeypatch.setattr("hermes_cli.models._PROVIDER_MODELS", {}) + monkeypatch.setattr(ms, "_ensure_direct_aliases", lambda: None) + monkeypatch.setattr(ms, "DIRECT_ALIASES", {}) + monkeypatch.setattr(ms, "list_provider_models", + lambda p: ["claude-opus-4-8", "claude-sonnet-4-6"]) result = ms.resolve_alias("opus", "anthropic") - assert result is not None and result[1] == "claude-opus-4-9" + assert result is not None and result[1] == "claude-opus-4-8" + + def test_switch_model_surfaces_ambiguity_message(self, monkeypatch): + """switch_model returns a failure result listing the candidates + instead of switching to a heuristic guess.""" + import hermes_cli.model_switch as ms + + monkeypatch.setattr("hermes_cli.models._PROVIDER_MODELS", {}) + monkeypatch.setattr(ms, "_ensure_direct_aliases", lambda: None) + monkeypatch.setattr(ms, "DIRECT_ALIASES", {}) + monkeypatch.setattr(ms, "list_provider_models", + lambda p: ["claude-opus-4-8", + "claude-opus-4-20250514"]) + result = ms.switch_model( + "opus", + current_provider="anthropic", + current_model="claude-sonnet-4-6", + ) + assert result.success is False + assert "claude-opus-4-8" in result.error_message + assert "claude-opus-4-20250514" in result.error_message + assert "not switching automatically" in result.error_message # ---------------------------------------------------------------------------