From 685b1e3d9b1eabb4eed2fb8aa31cc7be05d0eefc Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Sat, 9 May 2026 16:59:58 -0700 Subject: [PATCH] fix(console): preserve cs=None fallback in /v1/api/models placeholder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review feedback on #500. The original ``list_available_models`` had an implicit cs=None branch where the placeholder still advertised ``registry.default`` (filtered against enabled rows) when ``app.state.config_store`` was None but ``coord_registry`` was bound — useful in the rare degraded state where lifespan wired the registry but the ConfigStore failed to initialise. The PR #500 refactor accidentally dropped that branch: the helper requires a config_store, so the cs=None case fell out as "blank coordinator default". Add an explicit ``elif coord_registry is not None`` branch that mirrors the helper's tier 3 with the placeholder's enabled-rows filter applied. New test exercises this path by passing ``config_store=False`` to the test fixture. --- tests/test_console_available_models.py | 20 +++++++++++++++++++- turnstone/console/server.py | 8 ++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/tests/test_console_available_models.py b/tests/test_console_available_models.py index f8adef54..70330508 100644 --- a/tests/test_console_available_models.py +++ b/tests/test_console_available_models.py @@ -88,13 +88,15 @@ def _make_client( *, settings: dict[str, str] | None = None, registry_default: str = "", + config_store: bool = True, ) -> TestClient: app = Starlette( routes=[Route("/v1/api/models", list_available_models)], middleware=[Middleware(_AuthMiddleware)], ) app.state.auth_storage = storage - app.state.config_store = _FakeConfigStore(dict(settings or {})) + if config_store: + app.state.config_store = _FakeConfigStore(dict(settings or {})) # ``coord_registry`` is always set in production after lifespan # startup; mirror that here. ``has_alias`` answers from the same # enabled-rows set the handler filters against. @@ -219,6 +221,22 @@ def test_coordinator_skips_registry_default_when_alias_disabled( assert body["coordinator_default_alias"] == "" +def test_coordinator_falls_back_to_registry_default_when_config_store_missing( + storage: SQLiteBackend, +) -> None: + """Edge case from PR #500 review: lifespan can leave + ``app.state.config_store`` as None (e.g. a startup exception) while + ``coord_registry`` still binds successfully. The placeholder must + still advertise ``registry.default`` (filtered against enabled rows) + rather than going blank — otherwise the home composer is uselessly + empty in a degraded-but-recoverable state.""" + _seed_model(storage, definition_id="m1", alias="primary") + body = _get_models(_make_client(storage, registry_default="primary", config_store=False)) + assert body["default_alias"] == "" + assert body["coordinator_default_alias"] == "primary" + assert body["judge_default_alias"] == "primary" + + # --------------------------------------------------------------------------- # Judge resolution # --------------------------------------------------------------------------- diff --git a/turnstone/console/server.py b/turnstone/console/server.py index efe7df36..c5f5f265 100644 --- a/turnstone/console/server.py +++ b/turnstone/console/server.py @@ -1699,6 +1699,14 @@ async def list_available_models(request: Request) -> JSONResponse: registry=coord_registry, alias_filter=lambda a: a in enabled_aliases, ) + elif coord_registry is not None: + # ConfigStore failed lifespan but coord_registry is still bound — + # fall through to ``registry.default`` (filtered) so the home + # composer isn't blank. Mirrors the helper's tier 3 with the + # placeholder's enabled-rows filter applied. + registry_default = getattr(coord_registry, "default", "") or "" + if registry_default in enabled_aliases: + coordinator_default_alias = registry_default # Judge falls back to the resolved coordinator alias when # ``judge.model`` is empty *or* not a registered alias — judge.model # is alias-only (matches IntentJudge.__init__), so an unknown value is