fix: validate /model values before saving
This commit is contained in:
parent
932d596466
commit
9d3a44e0e8
4 changed files with 330 additions and 11 deletions
44
cli.py
44
cli.py
|
|
@ -2020,14 +2020,44 @@ class HermesCLI:
|
|||
# Use original case so model names like "Anthropic/Claude-Opus-4" are preserved
|
||||
parts = cmd_original.split(maxsplit=1)
|
||||
if len(parts) > 1:
|
||||
new_model = parts[1]
|
||||
self.model = new_model
|
||||
self.agent = None # Force re-init
|
||||
# Save to config
|
||||
if save_config_value("model.default", new_model):
|
||||
print(f"(^_^)b Model changed to: {new_model} (saved to config)")
|
||||
new_model = parts[1].strip()
|
||||
|
||||
from hermes_cli.auth import resolve_provider
|
||||
from hermes_cli.models import validate_requested_model
|
||||
|
||||
try:
|
||||
provider_for_validation = resolve_provider(
|
||||
self.requested_provider,
|
||||
explicit_api_key=self._explicit_api_key,
|
||||
explicit_base_url=self._explicit_base_url,
|
||||
)
|
||||
except Exception:
|
||||
provider_for_validation = self.provider or self.requested_provider
|
||||
|
||||
validation = validate_requested_model(
|
||||
new_model,
|
||||
provider_for_validation,
|
||||
base_url=self.base_url,
|
||||
)
|
||||
|
||||
if not validation.get("accepted"):
|
||||
print(f"(^_^) Warning: {validation.get('message')}")
|
||||
print(f"(^_^) Current model unchanged: {self.model}")
|
||||
else:
|
||||
print(f"(^_^) Model changed to: {new_model} (session only)")
|
||||
self.model = new_model
|
||||
self.agent = None # Force re-init
|
||||
|
||||
if validation.get("persist"):
|
||||
if save_config_value("model.default", new_model):
|
||||
print(f"(^_^)b Model changed to: {new_model} (saved to config)")
|
||||
else:
|
||||
print(f"(^_^) Model changed to: {new_model} (session only)")
|
||||
else:
|
||||
print(f"(^_^) Model changed to: {new_model} (session only)")
|
||||
|
||||
message = validation.get("message")
|
||||
if message:
|
||||
print(f" Warning: {message}")
|
||||
else:
|
||||
print(f"Current model: {self.model}")
|
||||
print(" Usage: /model <model-name> to change")
|
||||
|
|
|
|||
|
|
@ -1,10 +1,15 @@
|
|||
"""
|
||||
Canonical list of OpenRouter models offered in CLI and setup wizards.
|
||||
Canonical model catalogs and lightweight validation helpers.
|
||||
|
||||
Add, remove, or reorder entries here — both `hermes setup` and
|
||||
`hermes` provider-selection will pick up the change automatically.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from difflib import get_close_matches
|
||||
from typing import Any, Optional
|
||||
|
||||
# (model_id, display description shown in menus)
|
||||
OPENROUTER_MODELS: list[tuple[str, str]] = [
|
||||
("anthropic/claude-opus-4.6", "recommended"),
|
||||
|
|
@ -14,17 +19,64 @@ OPENROUTER_MODELS: list[tuple[str, str]] = [
|
|||
("openai/gpt-5.3-codex", ""),
|
||||
("google/gemini-3-pro-preview", ""),
|
||||
("google/gemini-3-flash-preview", ""),
|
||||
("qwen/qwen3.5-plus-02-15", ""),
|
||||
("qwen/qwen3.5-35b-a3b", ""),
|
||||
("qwen/qwen3.5-plus-02-15", ""),
|
||||
("qwen/qwen3.5-35b-a3b", ""),
|
||||
("stepfun/step-3.5-flash", ""),
|
||||
("z-ai/glm-5", ""),
|
||||
("moonshotai/kimi-k2.5", ""),
|
||||
("minimax/minimax-m2.5", ""),
|
||||
]
|
||||
|
||||
_PROVIDER_MODELS: dict[str, list[str]] = {
|
||||
"zai": [
|
||||
"glm-5",
|
||||
"glm-4.7",
|
||||
"glm-4.5",
|
||||
"glm-4.5-flash",
|
||||
],
|
||||
"kimi-coding": [
|
||||
"kimi-k2.5",
|
||||
"kimi-k2-thinking",
|
||||
"kimi-k2-turbo-preview",
|
||||
"kimi-k2-0905-preview",
|
||||
],
|
||||
"minimax": [
|
||||
"MiniMax-M2.5",
|
||||
"MiniMax-M2.5-highspeed",
|
||||
"MiniMax-M2.1",
|
||||
],
|
||||
"minimax-cn": [
|
||||
"MiniMax-M2.5",
|
||||
"MiniMax-M2.5-highspeed",
|
||||
"MiniMax-M2.1",
|
||||
],
|
||||
}
|
||||
|
||||
_PROVIDER_LABELS = {
|
||||
"openrouter": "OpenRouter",
|
||||
"openai-codex": "OpenAI Codex",
|
||||
"nous": "Nous Portal",
|
||||
"zai": "Z.AI / GLM",
|
||||
"kimi-coding": "Kimi / Moonshot",
|
||||
"minimax": "MiniMax",
|
||||
"minimax-cn": "MiniMax (China)",
|
||||
"custom": "custom endpoint",
|
||||
}
|
||||
|
||||
_PROVIDER_ALIASES = {
|
||||
"glm": "zai",
|
||||
"z-ai": "zai",
|
||||
"z.ai": "zai",
|
||||
"zhipu": "zai",
|
||||
"kimi": "kimi-coding",
|
||||
"moonshot": "kimi-coding",
|
||||
"minimax-china": "minimax-cn",
|
||||
"minimax_cn": "minimax-cn",
|
||||
}
|
||||
|
||||
|
||||
def model_ids() -> list[str]:
|
||||
"""Return just the model-id strings (convenience helper)."""
|
||||
"""Return just the OpenRouter model-id strings."""
|
||||
return [mid for mid, _ in OPENROUTER_MODELS]
|
||||
|
||||
|
||||
|
|
@ -34,3 +86,137 @@ def menu_labels() -> list[str]:
|
|||
for mid, desc in OPENROUTER_MODELS:
|
||||
labels.append(f"{mid} ({desc})" if desc else mid)
|
||||
return labels
|
||||
|
||||
|
||||
def normalize_provider(provider: Optional[str]) -> str:
|
||||
"""Normalize provider aliases to Hermes' canonical provider ids."""
|
||||
normalized = (provider or "openrouter").strip().lower()
|
||||
return _PROVIDER_ALIASES.get(normalized, normalized)
|
||||
|
||||
|
||||
def provider_model_ids(provider: Optional[str]) -> list[str]:
|
||||
"""Return the best known model catalog for a provider."""
|
||||
normalized = normalize_provider(provider)
|
||||
if normalized == "openrouter":
|
||||
return model_ids()
|
||||
if normalized == "openai-codex":
|
||||
from hermes_cli.codex_models import get_codex_model_ids
|
||||
|
||||
return get_codex_model_ids()
|
||||
return list(_PROVIDER_MODELS.get(normalized, []))
|
||||
|
||||
|
||||
def validate_requested_model(
|
||||
model_name: str,
|
||||
provider: Optional[str],
|
||||
*,
|
||||
base_url: Optional[str] = None,
|
||||
) -> dict[str, Any]:
|
||||
"""
|
||||
Validate a `/model` value for the active provider.
|
||||
|
||||
Returns a dict with:
|
||||
- accepted: whether the CLI should switch to the requested model now
|
||||
- persist: whether it is safe to save to config
|
||||
- recognized: whether it matched a known provider catalog
|
||||
- message: optional warning / guidance for the user
|
||||
"""
|
||||
requested = (model_name or "").strip()
|
||||
normalized = normalize_provider(provider)
|
||||
if normalized == "openrouter" and base_url and "openrouter.ai" not in base_url:
|
||||
normalized = "custom"
|
||||
|
||||
if not requested:
|
||||
return {
|
||||
"accepted": False,
|
||||
"persist": False,
|
||||
"recognized": False,
|
||||
"message": "Model name cannot be empty.",
|
||||
}
|
||||
|
||||
if any(ch.isspace() for ch in requested):
|
||||
return {
|
||||
"accepted": False,
|
||||
"persist": False,
|
||||
"recognized": False,
|
||||
"message": "Model names cannot contain spaces.",
|
||||
}
|
||||
|
||||
known_models = provider_model_ids(normalized)
|
||||
if requested in known_models:
|
||||
return {
|
||||
"accepted": True,
|
||||
"persist": True,
|
||||
"recognized": True,
|
||||
"message": None,
|
||||
}
|
||||
|
||||
suggestion = get_close_matches(requested, known_models, n=1, cutoff=0.6)
|
||||
suggestion_text = f" Did you mean `{suggestion[0]}`?" if suggestion else ""
|
||||
provider_label = _PROVIDER_LABELS.get(normalized, normalized)
|
||||
|
||||
if normalized == "custom":
|
||||
return {
|
||||
"accepted": True,
|
||||
"persist": True,
|
||||
"recognized": False,
|
||||
"message": None,
|
||||
}
|
||||
|
||||
if normalized == "openrouter":
|
||||
if "/" not in requested or requested.startswith("/") or requested.endswith("/"):
|
||||
return {
|
||||
"accepted": False,
|
||||
"persist": False,
|
||||
"recognized": False,
|
||||
"message": (
|
||||
"OpenRouter model IDs should use the `provider/model` format "
|
||||
"(for example `anthropic/claude-opus-4.6`)."
|
||||
f"{suggestion_text}"
|
||||
),
|
||||
}
|
||||
return {
|
||||
"accepted": True,
|
||||
"persist": False,
|
||||
"recognized": False,
|
||||
"message": (
|
||||
f"`{requested}` is not in Hermes' curated {provider_label} model list. "
|
||||
"Using it for this session only; config unchanged."
|
||||
f"{suggestion_text}"
|
||||
),
|
||||
}
|
||||
|
||||
if normalized == "nous":
|
||||
return {
|
||||
"accepted": True,
|
||||
"persist": False,
|
||||
"recognized": False,
|
||||
"message": (
|
||||
f"Could not validate `{requested}` against the live {provider_label} catalog here. "
|
||||
"Using it for this session only; config unchanged."
|
||||
f"{suggestion_text}"
|
||||
),
|
||||
}
|
||||
|
||||
if known_models:
|
||||
return {
|
||||
"accepted": True,
|
||||
"persist": False,
|
||||
"recognized": False,
|
||||
"message": (
|
||||
f"`{requested}` is not in the known {provider_label} model list. "
|
||||
"Using it for this session only; config unchanged."
|
||||
f"{suggestion_text}"
|
||||
),
|
||||
}
|
||||
|
||||
return {
|
||||
"accepted": True,
|
||||
"persist": False,
|
||||
"recognized": False,
|
||||
"message": (
|
||||
f"Could not validate `{requested}` for provider {provider_label}. "
|
||||
"Using it for this session only; config unchanged."
|
||||
f"{suggestion_text}"
|
||||
),
|
||||
}
|
||||
|
|
|
|||
43
tests/hermes_cli/test_model_validation.py
Normal file
43
tests/hermes_cli/test_model_validation.py
Normal file
|
|
@ -0,0 +1,43 @@
|
|||
"""Tests for provider-aware `/model` validation."""
|
||||
|
||||
import os
|
||||
import sys
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", ".."))
|
||||
|
||||
from hermes_cli.models import validate_requested_model
|
||||
|
||||
|
||||
class TestValidateRequestedModel:
|
||||
def test_known_openrouter_model_can_be_saved(self):
|
||||
result = validate_requested_model("anthropic/claude-opus-4.6", "openrouter")
|
||||
|
||||
assert result["accepted"] is True
|
||||
assert result["persist"] is True
|
||||
assert result["recognized"] is True
|
||||
assert result["message"] is None
|
||||
|
||||
def test_openrouter_requires_provider_model_format(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_unknown_codex_model_is_session_only(self):
|
||||
result = validate_requested_model("totally-made-up", "openai-codex")
|
||||
|
||||
assert result["accepted"] is True
|
||||
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",
|
||||
)
|
||||
|
||||
assert result["accepted"] is True
|
||||
assert result["persist"] is True
|
||||
assert result["message"] is None
|
||||
60
tests/test_cli_model_command.py
Normal file
60
tests/test_cli_model_command.py
Normal file
|
|
@ -0,0 +1,60 @@
|
|||
"""Regression tests for the `/model` slash command."""
|
||||
|
||||
import os
|
||||
import sys
|
||||
from unittest.mock import patch
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||
|
||||
from cli import HermesCLI
|
||||
|
||||
|
||||
class TestModelCommand:
|
||||
def _make_cli(self):
|
||||
cli_obj = HermesCLI.__new__(HermesCLI)
|
||||
cli_obj.model = "anthropic/claude-opus-4.6"
|
||||
cli_obj.agent = object()
|
||||
cli_obj.provider = "openrouter"
|
||||
cli_obj.requested_provider = "openrouter"
|
||||
cli_obj.base_url = "https://openrouter.ai/api/v1"
|
||||
cli_obj._explicit_api_key = None
|
||||
cli_obj._explicit_base_url = None
|
||||
return cli_obj
|
||||
|
||||
def test_invalid_model_does_not_change_current_model(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": False,
|
||||
"persist": False,
|
||||
"recognized": False,
|
||||
"message": "OpenRouter model IDs should use the `provider/model` format.",
|
||||
}), \
|
||||
patch("cli.save_config_value") as save_mock:
|
||||
cli_obj.process_command("/model invalid-model")
|
||||
|
||||
output = capsys.readouterr().out
|
||||
assert "Current model unchanged" in output
|
||||
assert cli_obj.model == "anthropic/claude-opus-4.6"
|
||||
assert cli_obj.agent is not None
|
||||
save_mock.assert_not_called()
|
||||
|
||||
def test_unknown_model_stays_session_only(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": False,
|
||||
"recognized": False,
|
||||
"message": "Using it for this session only; config unchanged.",
|
||||
}), \
|
||||
patch("cli.save_config_value") as save_mock:
|
||||
cli_obj.process_command("/model anthropic/claude-sonnet-next")
|
||||
|
||||
output = capsys.readouterr().out
|
||||
assert "session only" in output
|
||||
assert cli_obj.model == "anthropic/claude-sonnet-next"
|
||||
assert cli_obj.agent is None
|
||||
save_mock.assert_not_called()
|
||||
Loading…
Add table
Add a link
Reference in a new issue