Models: avoid hidden models in default fallback

Both get_default_chat and _recover_empty_session_model picked the
first model from cached_models[0] without checking hidden_models.
If the first cached model was hidden (e.g. minimax-m3), it was
returned as the default or used to repair empty session models,
even though the model list endpoints already filter hidden_models.

- Add _visible_models() helper that filters cached_models by
  hidden_models (mirrors the filtering in list_model_endpoints)
- Use _visible_models() in get_default_chat fallback (when no
  explicit default_model is saved)
- Use _visible_models() in _recover_empty_session_model (when
  repairing a session whose model field is empty before chat send)
- Add regression tests for hidden-model filtering in default chat
  resolution, and unit tests for _visible_models helper
This commit is contained in:
Yavor Ivanov
2026-06-02 13:37:14 +02:00
committed by GitHub
parent 8115cb01a2
commit 7cc8fdb2f5
3 changed files with 178 additions and 4 deletions

View File

@@ -28,6 +28,7 @@ from core.database import SessionLocal, get_session_mode, set_session_mode
from core.database import Session as DBSession, ChatMessage as DBChatMessage from core.database import Session as DBSession, ChatMessage as DBChatMessage
from core.database import Document as DBDocument, ModelEndpoint from core.database import Document as DBDocument, ModelEndpoint
from routes.research_routes import _resolve_research_endpoint from routes.research_routes import _resolve_research_endpoint
from routes.model_routes import _visible_models
from routes.chat_helpers import ( from routes.chat_helpers import (
resolve_session_auth, resolve_session_auth,
build_chat_context, build_chat_context,
@@ -130,7 +131,13 @@ def _recover_empty_session_model(sess, session_id: str) -> bool:
cached = [] cached = []
if not cached: if not cached:
return False return False
model = cached[0] try:
visible = _visible_models(cached, getattr(ep, "hidden_models", None))
except Exception:
visible = cached
if not visible:
return False
model = visible[0]
if not isinstance(model, str) or not model.strip(): if not isinstance(model, str) or not model.strip():
return False return False
model = model.strip() model = model.strip()

View File

@@ -479,6 +479,15 @@ def _model_endpoint_error_message(base_url: str, ping: Dict[str, Any] = None) ->
return "No models found for that provider/key." return "No models found for that provider/key."
def _visible_models(cached_models, hidden_models):
"""Filter cached model IDs by hidden_models. Returns list of visible IDs."""
all_models = json.loads(cached_models) if isinstance(cached_models, str) else (cached_models or [])
if not hidden_models:
return all_models
hidden = set(json.loads(hidden_models) if isinstance(hidden_models, str) else (hidden_models or []))
return [m for m in all_models if m not in hidden]
def setup_model_routes(model_discovery): def setup_model_routes(model_discovery):
router = APIRouter(prefix="/api") router = APIRouter(prefix="/api")
@@ -1331,9 +1340,9 @@ def setup_model_routes(model_discovery):
chat_url = build_chat_url(base) chat_url = build_chat_url(base)
if not model and getattr(ep, "cached_models", None): if not model and getattr(ep, "cached_models", None):
try: try:
models = _json.loads(ep.cached_models) if isinstance(ep.cached_models, str) else ep.cached_models visible = _visible_models(ep.cached_models, getattr(ep, "hidden_models", None))
if models: if visible:
model = models[0] model = visible[0]
except Exception: except Exception:
pass pass
return {"endpoint_id": ep.id, "endpoint_url": chat_url, "model": model} return {"endpoint_id": ep.id, "endpoint_url": chat_url, "model": model}

View File

@@ -27,6 +27,7 @@ class _FakeModelEndpoint:
class _FakeDbSession: class _FakeDbSession:
id = _FakeColumn("id")
endpoint_url = _FakeColumn("endpoint_url") endpoint_url = _FakeColumn("endpoint_url")
@@ -44,6 +45,9 @@ class _FakeQuery:
def first(self): def first(self):
return self.rows[0] if self.rows else None return self.rows[0] if self.rows else None
def all(self):
return list(self.rows)
class _FakeDb: class _FakeDb:
def __init__(self, rows): def __init__(self, rows):
@@ -73,16 +77,30 @@ def _install_model_route_import_stubs(monkeypatch):
db_mod.SessionLocal = lambda: _FakeDb([]) db_mod.SessionLocal = lambda: _FakeDb([])
db_mod.ModelEndpoint = _FakeModelEndpoint db_mod.ModelEndpoint = _FakeModelEndpoint
db_mod.Session = _FakeDbSession db_mod.Session = _FakeDbSession
db_mod.Document = MagicMock()
db_mod.DocumentVersion = MagicMock()
db_mod.GalleryImage = MagicMock()
middleware_mod = types.ModuleType("core.middleware") middleware_mod = types.ModuleType("core.middleware")
middleware_mod.require_admin = lambda request: None middleware_mod.require_admin = lambda request: None
multipart_mod = types.ModuleType("python_multipart") multipart_mod = types.ModuleType("python_multipart")
multipart_mod.__version__ = "0.0.13" multipart_mod.__version__ = "0.0.13"
models_mod = types.ModuleType("core.models")
models_mod.ChatMessage = MagicMock()
exceptions_mod = types.ModuleType("core.exceptions")
exceptions_mod.SessionNotFoundError = type("SessionNotFoundError", (Exception,), {})
session_mgr_mod = types.ModuleType("core.session_manager")
session_mgr_mod.SessionManager = MagicMock()
monkeypatch.delitem(sys.modules, "routes.model_routes", raising=False) monkeypatch.delitem(sys.modules, "routes.model_routes", raising=False)
monkeypatch.delitem(sys.modules, "routes.chat_routes", raising=False)
monkeypatch.delitem(sys.modules, "routes.session_routes", raising=False)
monkeypatch.setitem(sys.modules, "core", core_mod) monkeypatch.setitem(sys.modules, "core", core_mod)
monkeypatch.setitem(sys.modules, "core.database", db_mod) monkeypatch.setitem(sys.modules, "core.database", db_mod)
monkeypatch.setitem(sys.modules, "core.middleware", middleware_mod) monkeypatch.setitem(sys.modules, "core.middleware", middleware_mod)
monkeypatch.setitem(sys.modules, "python_multipart", multipart_mod) monkeypatch.setitem(sys.modules, "python_multipart", multipart_mod)
monkeypatch.setitem(sys.modules, "core.models", models_mod)
monkeypatch.setitem(sys.modules, "core.exceptions", exceptions_mod)
monkeypatch.setitem(sys.modules, "core.session_manager", session_mgr_mod)
def _install_core_auth_stub(monkeypatch): def _install_core_auth_stub(monkeypatch):
@@ -483,3 +501,143 @@ async def test_webhook_tool_reuses_private_url_validation():
assert result["exit_code"] == 1 assert result["exit_code"] == 1
assert "private/internal" in result["error"] assert "private/internal" in result["error"]
def test_default_chat_skips_hidden_first_model(monkeypatch):
"""get_default_chat picks first visible model when default_model is empty
and the first cached model is hidden."""
_install_model_route_import_stubs(monkeypatch)
import routes.model_routes as model_routes
import routes.prefs_routes as prefs_routes
ep = SimpleNamespace(
id="ep1",
base_url="http://localhost:11434",
is_enabled=True,
owner="fresh",
cached_models='["hidden-model", "visible-model"]',
hidden_models='["hidden-model"]',
)
monkeypatch.setattr(model_routes, "ModelEndpoint", _FakeModelEndpoint)
monkeypatch.setattr(model_routes, "SessionLocal", lambda: _FakeDb([ep]))
monkeypatch.setattr(model_routes, "_load_settings", lambda: {})
monkeypatch.setattr(model_routes, "owner_filter", lambda q, m, u, **kw: q)
monkeypatch.setattr(model_routes, "_normalize_base", lambda base: base.rstrip("/"))
monkeypatch.setattr(model_routes, "build_chat_url", lambda base: f"{base}/chat/completions")
monkeypatch.setattr(prefs_routes, "_load_for_user", lambda user: {})
request = SimpleNamespace(
state=SimpleNamespace(current_user="fresh"),
app=SimpleNamespace(state=SimpleNamespace(
auth_manager=SimpleNamespace(is_admin=lambda user: False)
)),
)
result = _default_chat_endpoint()(request)
assert result["model"] == "visible-model", f"Expected visible-model, got {result['model']!r}"
def test_default_chat_admin_skips_hidden_first_model(monkeypatch):
"""Admin user with global defaults also skips hidden models in fallback."""
_install_model_route_import_stubs(monkeypatch)
import routes.model_routes as model_routes
ep = SimpleNamespace(
id="ep1",
base_url="http://localhost:11434",
is_enabled=True,
owner=None,
cached_models='["hidden-model", "visible-model"]',
hidden_models='["hidden-model"]',
)
monkeypatch.setattr(model_routes, "ModelEndpoint", _FakeModelEndpoint)
monkeypatch.setattr(model_routes, "SessionLocal", lambda: _FakeDb([ep]))
monkeypatch.setattr(model_routes, "_load_settings", lambda: {})
monkeypatch.setattr(model_routes, "owner_filter", lambda q, m, u, **kw: q)
monkeypatch.setattr(model_routes, "_normalize_base", lambda base: base.rstrip("/"))
monkeypatch.setattr(model_routes, "build_chat_url", lambda base: f"{base}/chat/completions")
request = SimpleNamespace(
state=SimpleNamespace(current_user="admin"),
app=SimpleNamespace(state=SimpleNamespace(
auth_manager=SimpleNamespace(is_admin=lambda user: True)
)),
)
result = _default_chat_endpoint()(request)
assert result["model"] == "visible-model"
def test_default_chat_all_models_hidden_returns_empty_model(monkeypatch):
"""When all cached models are hidden, get_default_chat returns model: ''."""
_install_model_route_import_stubs(monkeypatch)
import routes.model_routes as model_routes
ep = SimpleNamespace(
id="ep1",
base_url="http://localhost:11434",
is_enabled=True,
owner=None,
cached_models='["hidden-a", "hidden-b"]',
hidden_models='["hidden-a", "hidden-b"]',
)
monkeypatch.setattr(model_routes, "ModelEndpoint", _FakeModelEndpoint)
monkeypatch.setattr(model_routes, "SessionLocal", lambda: _FakeDb([ep]))
monkeypatch.setattr(model_routes, "_load_settings", lambda: {})
monkeypatch.setattr(model_routes, "owner_filter", lambda q, m, u, **kw: q)
monkeypatch.setattr(model_routes, "_normalize_base", lambda base: base.rstrip("/"))
monkeypatch.setattr(model_routes, "build_chat_url", lambda base: f"{base}/chat/completions")
request = SimpleNamespace(
state=SimpleNamespace(current_user="admin"),
app=SimpleNamespace(state=SimpleNamespace(
auth_manager=SimpleNamespace(is_admin=lambda user: True)
)),
)
result = _default_chat_endpoint()(request)
assert result["model"] == "", f"Expected empty model, got {result['model']!r}"
def test_visible_models_filters_hidden_first(monkeypatch):
"""_visible_models removes hidden models from the list."""
from routes.model_routes import _visible_models
result = _visible_models(
'["hidden-model", "visible-model"]',
'["hidden-model"]',
)
assert result == ["visible-model"]
def test_visible_models_all_hidden_returns_empty(monkeypatch):
"""_visible_models returns [] when all models are hidden."""
from routes.model_routes import _visible_models
result = _visible_models(
'["hidden-a", "hidden-b"]',
'["hidden-a", "hidden-b"]',
)
assert result == []
def test_visible_models_no_hidden_returns_all(monkeypatch):
"""_visible_models returns full list when no hidden_models."""
from routes.model_routes import _visible_models
result = _visible_models(
'["model-a", "model-b"]',
None,
)
assert result == ["model-a", "model-b"]
def test_visible_models_empty_cached_returns_empty(monkeypatch):
"""_visible_models returns [] for empty cached list."""
from routes.model_routes import _visible_models
result = _visible_models([], None)
assert result == []