diff --git a/routes/model_routes.py b/routes/model_routes.py index b88fa3ef1..e53a23552 100644 --- a/routes/model_routes.py +++ b/routes/model_routes.py @@ -123,6 +123,21 @@ def _clear_user_pref_endpoint_refs(all_prefs: dict, ep_id: str) -> int: return cleared_users +def _default_endpoint_needs_assignment(current_default_id: str, enabled_endpoint_ids) -> bool: + """Whether the global default chat endpoint should be (re)assigned. + + True when nothing is configured yet, or the configured default no longer + resolves to an enabled endpoint (e.g. the user disabled it). Without the + second case, adding a new endpoint after disabling the previous default + leaves `default_endpoint_id` pointing at the disabled endpoint, so features + that read the raw setting (Memory → Tidy) fail with "No default model + configured" even though an enabled endpoint exists. See #3586. + """ + if not current_default_id: + return True + return current_default_id not in enabled_endpoint_ids + + # Loopback hosts a user might type for a local model server (LM Studio, # llama.cpp, vLLM, …). Inside Docker these point at the *container*, not the # host the server actually runs on. @@ -1727,12 +1742,19 @@ def setup_model_routes(model_discovery): ) db.add(ep) db.commit() - # Auto-set as default chat endpoint if none configured yet. Seed - # the first CHAT model (not raw model_ids[0]) so we don't pin the - # global default to an embedding/tts/etc. entry a provider happens - # to list first. + # Auto-set as default chat endpoint when none is usable yet — either + # nothing is configured, or the configured default points at an + # endpoint that is now missing/disabled (#3586). Seed the first CHAT + # model (not raw model_ids[0]) so we don't pin the global default to + # an embedding/tts/etc. entry a provider happens to list first. settings = _load_settings() - if not settings.get("default_endpoint_id"): + enabled_ids = { + e.id + for e in db.query(ModelEndpoint).filter( + ModelEndpoint.is_enabled == True # noqa: E712 + ).all() + } + if _default_endpoint_needs_assignment(settings.get("default_endpoint_id") or "", enabled_ids): from src.endpoint_resolver import _first_chat_model settings["default_endpoint_id"] = ep.id settings["default_model"] = _first_chat_model(model_ids) or "" diff --git a/tests/test_model_routes.py b/tests/test_model_routes.py index 3b23123ef..ee1a53912 100644 --- a/tests/test_model_routes.py +++ b/tests/test_model_routes.py @@ -54,6 +54,7 @@ with preserve_import_state("core.database", "src.database", "core.session_manage _endpoint_settings_using_endpoint, _clear_endpoint_settings_for_endpoint, _clear_user_pref_endpoint_refs, + _default_endpoint_needs_assignment, _PROVIDER_CURATED, ) from src.llm_core import ANTHROPIC_MODELS @@ -154,6 +155,26 @@ def test_endpoint_cleanup_updates_scoped_and_legacy_user_prefs(): assert legacy["default_model_fallbacks"] == [] +# ── _default_endpoint_needs_assignment (add-endpoint auto-default) ── + +def test_default_assignment_when_none_configured(): + # Nothing configured yet → first added endpoint should become the default. + assert _default_endpoint_needs_assignment("", {"a", "b"}) is True + + +def test_default_assignment_when_current_default_disabled(): + # #3586: the configured default points at an endpoint that is no longer + # enabled (the user disabled it). Adding a new endpoint must reassign the + # default — otherwise Memory → Tidy keeps failing with "No default model + # configured" even though an enabled endpoint exists. + assert _default_endpoint_needs_assignment("disabled-ep", {"new-ep"}) is True + + +def test_default_preserved_when_current_default_enabled(): + # Normal case: the configured default is still enabled → leave it alone. + assert _default_endpoint_needs_assignment("live-ep", {"live-ep", "new-ep"}) is False + + # ── _match_provider_curated ── class TestMatchProviderCurated: @@ -966,16 +987,21 @@ def _create_form_kwargs(**overrides): return kwargs -def _patch_create_deps(monkeypatch, db): +def _patch_create_deps(monkeypatch, db, settings=None): import src.auth_helpers as auth_helpers + # Shared, in-memory settings so the auto-default write path stays hermetic + # (no real settings.json). Returned so tests can assert what was persisted. + settings = {"default_endpoint_id": "exists"} if settings is None else settings monkeypatch.setattr(model_routes, "SessionLocal", lambda: db) monkeypatch.setattr(model_routes, "require_admin", lambda request: None) monkeypatch.setattr(model_routes, "ModelEndpoint", _RecordingEndpoint) monkeypatch.setattr(model_routes, "_normalize_base", lambda b: b) monkeypatch.setattr(model_routes, "_rewrite_loopback_for_docker", lambda b, **k: b) - monkeypatch.setattr(model_routes, "_load_settings", lambda: {"default_endpoint_id": "exists"}) + monkeypatch.setattr(model_routes, "_load_settings", lambda: settings) + monkeypatch.setattr(model_routes, "_save_settings", lambda s: settings.update(s)) monkeypatch.setattr(endpoint_resolver, "resolve_url", lambda u: u) monkeypatch.setattr(auth_helpers, "get_current_user", lambda req: None) + return settings def test_list_model_endpoints_returns_key_fingerprint(monkeypatch): @@ -1091,6 +1117,48 @@ def test_post_same_base_url_different_api_key_creates_distinct_endpoint(monkeypa assert db.added[0].api_key == "key-two" +def test_post_reassigns_default_when_current_default_disabled(monkeypatch): + # #3586: the configured default points at a now-disabled endpoint. Adding a + # new endpoint must promote it to the default, otherwise raw-setting readers + # (Memory → Tidy) keep failing with "No default model configured". + disabled = _make_endpoint(id="dead", base_url="http://old-host/v1", is_enabled=False) + db = _PinnedFakeDb([disabled]) + settings = _patch_create_deps( + monkeypatch, db, settings={"default_endpoint_id": "dead", "default_model": "stale"} + ) + create = _get_route("/api/model-endpoints", "POST") + + create( + _PinnedFakeRequest(), + base_url="http://new-host:1234/v1", + **_create_form_kwargs(), + ) + + new_id = db.added[0].id + assert settings["default_endpoint_id"] == new_id + assert settings["default_endpoint_id"] != "dead" + + +def test_post_keeps_default_when_current_default_enabled(monkeypatch): + # Counter-case: an enabled default must be left untouched when another + # endpoint is added. + live = _make_endpoint(id="live", base_url="http://live-host/v1", is_enabled=True) + db = _PinnedFakeDb([live]) + settings = _patch_create_deps( + monkeypatch, db, settings={"default_endpoint_id": "live", "default_model": "live-model"} + ) + create = _get_route("/api/model-endpoints", "POST") + + create( + _PinnedFakeRequest(), + base_url="http://another-host:1234/v1", + **_create_form_kwargs(), + ) + + assert settings["default_endpoint_id"] == "live" + assert settings["default_model"] == "live-model" + + def test_post_same_base_url_same_api_key_still_dedupes(monkeypatch): existing = _make_endpoint( base_url="https://api.example.test/v1",