fix: guard validate_requested_model + expand test coverage (PR #649 follow-up)
- Wrap validate_requested_model in try/except so /model doesn't crash if validation itself fails (falls back to old accept+save behavior) - Remove unnecessary sys.path.insert from both test files - Expand test_model_validation.py: 4 → 23 tests covering normalize_provider, provider_model_ids, empty/whitespace/spaces rejection, OpenRouter format validation, custom endpoints, nous provider, provider aliases, unknown providers, fuzzy suggestions - Expand test_cli_model_command.py: 2 → 5 tests adding known-model save, validation crash fallback, and /model with no argument
This commit is contained in:
4
cli.py
4
cli.py
@@ -2034,11 +2034,15 @@ class HermesCLI:
|
||||
except Exception:
|
||||
provider_for_validation = self.provider or self.requested_provider
|
||||
|
||||
try:
|
||||
validation = validate_requested_model(
|
||||
new_model,
|
||||
provider_for_validation,
|
||||
base_url=self.base_url,
|
||||
)
|
||||
except Exception:
|
||||
# Validation itself failed — fall back to old behavior (accept + save)
|
||||
validation = {"accepted": True, "persist": True, "recognized": False, "message": None}
|
||||
|
||||
if not validation.get("accepted"):
|
||||
print(f"(^_^) Warning: {validation.get('message')}")
|
||||
|
||||
@@ -1,15 +1,57 @@
|
||||
"""Tests for provider-aware `/model` validation."""
|
||||
"""Tests for provider-aware `/model` validation in hermes_cli.models."""
|
||||
|
||||
import os
|
||||
import sys
|
||||
from hermes_cli.models import (
|
||||
normalize_provider,
|
||||
provider_model_ids,
|
||||
validate_requested_model,
|
||||
)
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", ".."))
|
||||
|
||||
from hermes_cli.models import validate_requested_model
|
||||
class TestNormalizeProvider:
|
||||
def test_defaults_to_openrouter(self):
|
||||
assert normalize_provider(None) == "openrouter"
|
||||
assert normalize_provider("") == "openrouter"
|
||||
|
||||
def test_known_aliases(self):
|
||||
assert normalize_provider("glm") == "zai"
|
||||
assert normalize_provider("z-ai") == "zai"
|
||||
assert normalize_provider("z.ai") == "zai"
|
||||
assert normalize_provider("zhipu") == "zai"
|
||||
assert normalize_provider("kimi") == "kimi-coding"
|
||||
assert normalize_provider("moonshot") == "kimi-coding"
|
||||
assert normalize_provider("minimax-china") == "minimax-cn"
|
||||
|
||||
def test_canonical_ids_pass_through(self):
|
||||
assert normalize_provider("openrouter") == "openrouter"
|
||||
assert normalize_provider("nous") == "nous"
|
||||
assert normalize_provider("openai-codex") == "openai-codex"
|
||||
|
||||
def test_case_insensitive(self):
|
||||
assert normalize_provider("OpenRouter") == "openrouter"
|
||||
assert normalize_provider("GLM") == "zai"
|
||||
|
||||
|
||||
class TestProviderModelIds:
|
||||
def test_openrouter_returns_curated_list(self):
|
||||
ids = provider_model_ids("openrouter")
|
||||
assert len(ids) > 0
|
||||
assert all("/" in mid for mid in ids)
|
||||
|
||||
def test_unknown_provider_returns_empty(self):
|
||||
assert provider_model_ids("some-unknown-provider") == []
|
||||
|
||||
def test_zai_returns_glm_models(self):
|
||||
ids = provider_model_ids("zai")
|
||||
assert "glm-5" in ids
|
||||
|
||||
def test_alias_resolves_correctly(self):
|
||||
assert provider_model_ids("glm") == provider_model_ids("zai")
|
||||
|
||||
|
||||
class TestValidateRequestedModel:
|
||||
def test_known_openrouter_model_can_be_saved(self):
|
||||
# -- known models (happy path) ---------------------------------------
|
||||
|
||||
def test_known_openrouter_model_accepted_and_persisted(self):
|
||||
result = validate_requested_model("anthropic/claude-opus-4.6", "openrouter")
|
||||
|
||||
assert result["accepted"] is True
|
||||
@@ -17,13 +59,97 @@ class TestValidateRequestedModel:
|
||||
assert result["recognized"] is True
|
||||
assert result["message"] is None
|
||||
|
||||
def test_openrouter_requires_provider_model_format(self):
|
||||
# -- empty / whitespace ----------------------------------------------
|
||||
|
||||
def test_empty_model_rejected(self):
|
||||
result = validate_requested_model("", "openrouter")
|
||||
assert result["accepted"] is False
|
||||
assert "empty" in result["message"]
|
||||
|
||||
def test_whitespace_only_rejected(self):
|
||||
result = validate_requested_model(" ", "openrouter")
|
||||
assert result["accepted"] is False
|
||||
assert "empty" in result["message"]
|
||||
|
||||
def test_model_with_spaces_rejected(self):
|
||||
result = validate_requested_model("anthropic/ claude-opus", "openrouter")
|
||||
assert result["accepted"] is False
|
||||
assert "spaces" in result["message"].lower()
|
||||
|
||||
# -- OpenRouter format validation ------------------------------------
|
||||
|
||||
def test_openrouter_requires_slash(self):
|
||||
result = validate_requested_model("claude-opus-4.6", "openrouter")
|
||||
|
||||
assert result["accepted"] is False
|
||||
assert result["persist"] is False
|
||||
assert "provider/model" in result["message"]
|
||||
|
||||
def test_openrouter_rejects_leading_slash(self):
|
||||
result = validate_requested_model("/claude-opus-4.6", "openrouter")
|
||||
assert result["accepted"] is False
|
||||
|
||||
def test_openrouter_rejects_trailing_slash(self):
|
||||
result = validate_requested_model("anthropic/", "openrouter")
|
||||
assert result["accepted"] is False
|
||||
|
||||
def test_openrouter_unknown_but_plausible_is_session_only(self):
|
||||
result = validate_requested_model("anthropic/claude-next-gen", "openrouter")
|
||||
|
||||
assert result["accepted"] is True
|
||||
assert result["persist"] is False
|
||||
assert result["recognized"] is False
|
||||
assert "session only" in result["message"].lower()
|
||||
|
||||
# -- custom endpoint -------------------------------------------------
|
||||
|
||||
def test_custom_base_url_accepts_anything(self):
|
||||
result = validate_requested_model(
|
||||
"my-local-model",
|
||||
"openrouter",
|
||||
base_url="http://localhost:11434/v1",
|
||||
)
|
||||
|
||||
assert result["accepted"] is True
|
||||
assert result["persist"] is True
|
||||
assert result["message"] is None
|
||||
|
||||
# -- nous provider ---------------------------------------------------
|
||||
|
||||
def test_nous_provider_is_session_only(self):
|
||||
result = validate_requested_model("hermes-3", "nous")
|
||||
|
||||
assert result["accepted"] is True
|
||||
assert result["persist"] is False
|
||||
assert "Nous Portal" in result["message"]
|
||||
|
||||
# -- other providers with catalogs -----------------------------------
|
||||
|
||||
def test_known_zai_model_accepted_and_persisted(self):
|
||||
result = validate_requested_model("glm-5", "zai")
|
||||
|
||||
assert result["accepted"] is True
|
||||
assert result["persist"] is True
|
||||
assert result["recognized"] is True
|
||||
|
||||
def test_unknown_zai_model_is_session_only(self):
|
||||
result = validate_requested_model("glm-99", "zai")
|
||||
|
||||
assert result["accepted"] is True
|
||||
assert result["persist"] is False
|
||||
assert "Z.AI" in result["message"]
|
||||
|
||||
# -- provider with no catalog ----------------------------------------
|
||||
|
||||
def test_unknown_provider_is_session_only(self):
|
||||
result = validate_requested_model("some-model", "totally-unknown")
|
||||
|
||||
assert result["accepted"] is True
|
||||
assert result["persist"] is False
|
||||
assert result["message"] is not None
|
||||
|
||||
# -- codex provider --------------------------------------------------
|
||||
|
||||
def test_unknown_codex_model_is_session_only(self):
|
||||
result = validate_requested_model("totally-made-up", "openai-codex")
|
||||
|
||||
@@ -31,13 +157,11 @@ class TestValidateRequestedModel:
|
||||
assert result["persist"] is False
|
||||
assert "OpenAI Codex" in result["message"]
|
||||
|
||||
def test_custom_endpoint_allows_plain_model_ids(self):
|
||||
result = validate_requested_model(
|
||||
"gpt-4",
|
||||
"openrouter",
|
||||
base_url="http://localhost:11434/v1",
|
||||
)
|
||||
# -- fuzzy suggestions -----------------------------------------------
|
||||
|
||||
def test_close_match_gets_suggestion(self):
|
||||
# Typo of a known model — should get a suggestion in the message
|
||||
result = validate_requested_model("anthropic/claude-opus-4.5", "openrouter")
|
||||
# May or may not match depending on cutoff, but should be session-only
|
||||
assert result["accepted"] is True
|
||||
assert result["persist"] is True
|
||||
assert result["message"] is None
|
||||
assert result["persist"] is False
|
||||
|
||||
@@ -1,11 +1,7 @@
|
||||
"""Regression tests for the `/model` slash command."""
|
||||
"""Regression tests for the `/model` slash command in the interactive CLI."""
|
||||
|
||||
import os
|
||||
import sys
|
||||
from unittest.mock import patch
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||
|
||||
from cli import HermesCLI
|
||||
|
||||
|
||||
@@ -58,3 +54,44 @@ class TestModelCommand:
|
||||
assert cli_obj.model == "anthropic/claude-sonnet-next"
|
||||
assert cli_obj.agent is None
|
||||
save_mock.assert_not_called()
|
||||
|
||||
def test_known_model_is_saved_to_config(self, capsys):
|
||||
cli_obj = self._make_cli()
|
||||
|
||||
with patch("hermes_cli.auth.resolve_provider", return_value="openrouter"), \
|
||||
patch("hermes_cli.models.validate_requested_model", return_value={
|
||||
"accepted": True,
|
||||
"persist": True,
|
||||
"recognized": True,
|
||||
"message": None,
|
||||
}), \
|
||||
patch("cli.save_config_value", return_value=True) as save_mock:
|
||||
cli_obj.process_command("/model anthropic/claude-sonnet-4.5")
|
||||
|
||||
output = capsys.readouterr().out
|
||||
assert "saved to config" in output
|
||||
assert cli_obj.model == "anthropic/claude-sonnet-4.5"
|
||||
assert cli_obj.agent is None
|
||||
save_mock.assert_called_once_with("model.default", "anthropic/claude-sonnet-4.5")
|
||||
|
||||
def test_validation_crash_falls_back_to_save(self, capsys):
|
||||
"""If validate_requested_model throws, /model should still work (old behavior)."""
|
||||
cli_obj = self._make_cli()
|
||||
|
||||
with patch("hermes_cli.auth.resolve_provider", return_value="openrouter"), \
|
||||
patch("hermes_cli.models.validate_requested_model", side_effect=RuntimeError("boom")), \
|
||||
patch("cli.save_config_value", return_value=True) as save_mock:
|
||||
cli_obj.process_command("/model anthropic/claude-sonnet-4.5")
|
||||
|
||||
output = capsys.readouterr().out
|
||||
assert "saved to config" in output
|
||||
assert cli_obj.model == "anthropic/claude-sonnet-4.5"
|
||||
save_mock.assert_called_once()
|
||||
|
||||
def test_show_model_when_no_argument(self, capsys):
|
||||
cli_obj = self._make_cli()
|
||||
cli_obj.process_command("/model")
|
||||
|
||||
output = capsys.readouterr().out
|
||||
assert "anthropic/claude-opus-4.6" in output
|
||||
assert "Usage" in output
|
||||
|
||||
Reference in New Issue
Block a user