From 7c93e21d33123d8fe39b9151e78efa376cdb5222 Mon Sep 17 00:00:00 2001 From: PARTH J ROHIT Date: Sun, 13 Sep 2026 11:11:03 +0100 Subject: [PATCH] feat(ai): pick a model from what the key can use, instead of typing one Connecting a provider failed at the model ID, and the product gave the user no way out of it. `gemini-1.5-flash` was PARTHA's Gemini default and is not offered at all to a Google AI Studio project created today, so a new key met "AI provider rejected the request, most likely because of an unsupported model ID" -- an error that names the problem and nothing that would fix it. Every provider default carried the same fault, because a hardcoded model ID is a fact with a shelf life. Providers publish their own model lists, so nothing has to be guessed. `POST /ai/models` asks the provider what this key can use and returns the list, plus one recommendation taken from that list and never invented. It shares `config_for_test`, so a key typed into the form and a key already saved both work -- the list is fetchable before anything is saved, which is when it is needed. Discovery is a GET through the same egress policy, the same allowlisted host and the same pinned connection a completion already uses; `/v1/models` and `/v1beta/models` were permitted under each provider's base path already. The sender gained a method parameter and nothing else. POST still routes through `post`, so every existing sender -- including the fakes in tests -- is untouched. Gemini lists embedding-only models alongside chat ones; offering those would produce a failure a user could not diagnose, so anything without generateContent never reaches the list. In the UI, credentials now come before the model, because that is the order setup actually runs in: you cannot list models until you have the key. Fetch models turns the free-text box into a picker, and a model the provider no longer offers is replaced rather than left in place to be rejected again. Defaults are refreshed too, but they are only a starting point now. The two I checked against a live catalogue before committing: the Gemini default I first wrote (gemini-2.0-flash) does not exist any more either, and the lowest current tier is gemini-2.5-flash. --- apps/backend/app/ai/orchestrator.py | 20 ++- apps/backend/app/ai/providers/http.py | 66 ++++++- apps/backend/app/ai/providers/models.py | 128 ++++++++++++++ apps/backend/app/ai/types.py | 17 +- apps/backend/app/api/routes/ai.py | 26 +++ apps/backend/app/schemas/ai.py | 12 ++ apps/backend/app/services/ai_service.py | 4 + .../tests/test_ai_provider_capabilities.py | 9 +- apps/backend/tests/test_ai_provider_models.py | 130 ++++++++++++++ apps/backend/tests/test_ai_providers.py | 6 +- apps/backend/tests/test_openapi_contract.py | 2 + .../src/app/pages/SettingsPage.test.tsx | 3 + apps/frontend/src/app/pages/SettingsPage.tsx | 51 +++++- .../settings/hooks/useSettings.test.ts | 73 ++++++++ .../features/settings/hooks/useSettings.ts | 35 ++++ apps/frontend/src/shared/services/api/ai.ts | 5 + .../src/shared/services/api/generated.ts | 162 ++++++++++++++++++ .../frontend/src/shared/services/api/types.ts | 1 + 18 files changed, 730 insertions(+), 20 deletions(-) create mode 100644 apps/backend/app/ai/providers/models.py create mode 100644 apps/backend/tests/test_ai_provider_models.py diff --git a/apps/backend/app/ai/orchestrator.py b/apps/backend/app/ai/orchestrator.py index 40f24651..7284dff8 100644 --- a/apps/backend/app/ai/orchestrator.py +++ b/apps/backend/app/ai/orchestrator.py @@ -4,14 +4,17 @@ from app.ai.providers.capabilities import capability_for from app.ai.providers.config_store import ProviderConfigStore from app.ai.providers.factory import ProviderFactory +from app.ai.providers.models import list_models as list_provider_models +from app.ai.providers.models import preferred_model from app.ai.repository_context import RepositoryContextBuilder -from app.ai.types import PromptBundle +from app.ai.types import DEFAULT_MODELS, PromptBundle from app.core.exceptions import NotFoundError, ValidationServiceError from app.repositories.ai_conversation_repository import AiConversationRepository from app.repositories.repository_repository import RepositoryRepository from app.schemas.ai import ( AiMessage, AiProviderConfig, + AiProviderModelsResponse, AiProviderPublicConfig, AiProviderTestRequest, AiProviderTestResponse, @@ -54,6 +57,21 @@ async def test_connection(self, request: AiProviderTestRequest) -> AiProviderTes ok=True, message=f"{config.provider} connection succeeded.", checked_at=datetime.now(UTC) ) + async def list_models(self, request: AiProviderTestRequest) -> AiProviderModelsResponse: + """Ask the provider what this key can use, rather than making the user guess. + + Deliberately shares `config_for_test`, so an unsaved key typed into the + form and a key already stored both resolve the same way -- the list can + be fetched before anything is saved, which is the moment it is needed. + """ + + config = self.config_store.config_for_test(request) + models = await list_provider_models(config) + return AiProviderModelsResponse( + models=models, + recommended=preferred_model(models, DEFAULT_MODELS[config.provider]), + ) + def list_conversation(self, repository_id: str) -> list[AiMessage]: # Same owner-scoping as query(): a non-owned repository id is # indistinguishable from a missing one. diff --git a/apps/backend/app/ai/providers/http.py b/apps/backend/app/ai/providers/http.py index 29997446..bd785138 100644 --- a/apps/backend/app/ai/providers/http.py +++ b/apps/backend/app/ai/providers/http.py @@ -23,6 +23,17 @@ async def post( ) -> httpx.Response: raise NotImplementedError + async def send( + self, + method: str, + config: AiProviderConfig, + url: str, + *, + timeout: httpx.Timeout | float | None = None, + **kwargs: object, + ) -> httpx.Response: + raise NotImplementedError + class RedirectDeniedError(Exception): """Signals a redirect without ever evaluating its Location header.""" @@ -50,6 +61,24 @@ async def post( timeout: httpx.Timeout | float | None = None, **kwargs: object, ) -> httpx.Response: + return await self.send("POST", config, url, timeout=timeout, **kwargs) + + async def send( + self, + method: str, + config: AiProviderConfig, + url: str, + *, + timeout: httpx.Timeout | float | None = None, + **kwargs: object, + ) -> httpx.Response: + """One outbound request through the policy, pinned to a validated IP. + + ``method`` is the only thing that varies: model discovery is a GET + against the same host and base path a completion POSTs to, so it goes + through exactly this validation rather than a second, looser path. + """ + # Policy preparation performs DNS resolution. Keep that blocking call # off the event loop used by the async AI routes. pinned = await anyio.to_thread.run_sync(self.policy.prepare_request, config, url) @@ -74,7 +103,7 @@ async def post( follow_redirects=False, transport=transport, ) as client: - request = client.build_request("POST", pinned.connection_url, headers=request_headers, **kwargs) + request = client.build_request(method, pinned.connection_url, headers=request_headers, **kwargs) # httpcore's documented request extension preserves TLS SNI (and # therefore hostname verification) when connecting to a literal IP. request.extensions["sni_hostname"] = pinned.destination.host @@ -118,12 +147,25 @@ def _plain_language_status_error(config: AiProviderConfig, exc: httpx.HTTPStatus if status in (400, 404, 422): return ValidationServiceError( "AI provider rejected the request, most likely because of an unsupported model ID. " - "Confirm the model ID and try again.", + "Use Fetch models to see what this key can use, then pick one.", {"provider": config.provider}, ) return ExternalServiceError("AI provider request failed.", {"provider": config.provider}) +async def get( + config: AiProviderConfig, + url: str, + *, + sender: ProviderHttpSender | None = None, + timeout: httpx.Timeout | float | None = None, + **kwargs: object, +) -> httpx.Response: + """A GET with the same policy, pinning and error translation as ``post``.""" + + return await _request("GET", config, url, sender=sender, timeout=timeout, **kwargs) + + async def post( config: AiProviderConfig, url: str, @@ -139,9 +181,27 @@ async def post( far longer than any hosted API call. """ + return await _request("POST", config, url, sender=sender, timeout=timeout, **kwargs) + + +async def _request( + method: str, + config: AiProviderConfig, + url: str, + *, + sender: ProviderHttpSender | None = None, + timeout: httpx.Timeout | float | None = None, + **kwargs: object, +) -> httpx.Response: active_sender = sender or _default_sender() try: - response = await active_sender.post(config, url, timeout=timeout, **kwargs) + # A POST still goes through `post`: that is the method every existing + # sender -- including the fakes tests inject -- already implements, and + # routing it through `send` instead would silently bypass any of them. + if method == "POST": + response = await active_sender.post(config, url, timeout=timeout, **kwargs) + else: + response = await active_sender.send(method, config, url, timeout=timeout, **kwargs) response.raise_for_status() return response except DestinationPolicyError as exc: diff --git a/apps/backend/app/ai/providers/models.py b/apps/backend/app/ai/providers/models.py new file mode 100644 index 00000000..40c1754d --- /dev/null +++ b/apps/backend/app/ai/providers/models.py @@ -0,0 +1,128 @@ +"""Ask each provider which models the caller's key can actually use (#291). + +Typing a model ID by hand is where provider setup fails. A default that was +correct when it was written stops being correct -- `gemini-1.5-flash` is not +available to a Google AI Studio project created today -- and the only signal +the user gets is a 400 from the provider, with no way to discover what they +should have typed instead. + +Every supported provider publishes its own model list, so nothing here is a +guess: the list is fetched with the user's own key, over the same egress +policy and pinned connection a completion uses, and the caller picks from +what came back. +""" + +from __future__ import annotations + +from app.ai.providers.http import ProviderHttpSender, get, require_api_key +from app.ai.types import AiProviderConfig +from app.core.exceptions import ExternalServiceError, ValidationServiceError + +#: Tier hints, cheapest-and-fastest first. Used only to preselect a sensible +#: entry from a list the provider itself returned -- never to name a model +#: that was not in it. +_PREFERRED_SUBSTRINGS = ("flash-lite", "flash", "mini", "haiku", "small", "turbo") + + +async def list_models(config: AiProviderConfig, *, sender: ProviderHttpSender | None = None) -> list[str]: + """Model IDs this configuration can use, sorted, or a plain-language error.""" + + lister = _LISTERS.get(config.provider) + if lister is None: + raise ValidationServiceError("Unsupported AI provider.", {"provider": config.provider}) + models = await lister(config, sender) + if not models: + raise ExternalServiceError( + "The AI provider returned no usable models for this key.", + {"provider": config.provider}, + ) + return sorted(set(models)) + + +def preferred_model(models: list[str], default: str) -> str: + """The entry a first-time user should start on. + + The saved default wins when the provider still offers it, so an existing + configuration is never quietly moved. Otherwise the cheapest tier whose + name the provider itself published, and failing that the first entry -- + the point is to land on something that works, not to rank models. + """ + + if default in models: + return default + for hint in _PREFERRED_SUBSTRINGS: + for model in models: + if hint in model: + return model + return models[0] + + +async def _openai_models(config: AiProviderConfig, sender: ProviderHttpSender | None) -> list[str]: + require_api_key(config) + response = await get( + config, + "https://api.openai.com/v1/models", + sender=sender, + headers={"Authorization": f"Bearer {config.api_key or ''}"}, + ) + return [str(item["id"]) for item in response.json().get("data", []) if item.get("id")] + + +async def _anthropic_models(config: AiProviderConfig, sender: ProviderHttpSender | None) -> list[str]: + require_api_key(config) + response = await get( + config, + "https://api.anthropic.com/v1/models?limit=100", + sender=sender, + headers={"x-api-key": config.api_key or "", "anthropic-version": "2023-06-01"}, + ) + return [str(item["id"]) for item in response.json().get("data", []) if item.get("id")] + + +async def _gemini_models(config: AiProviderConfig, sender: ProviderHttpSender | None) -> list[str]: + require_api_key(config) + response = await get( + config, + "https://generativelanguage.googleapis.com/v1beta/models?pageSize=200", + sender=sender, + headers={"x-goog-api-key": config.api_key or ""}, + ) + models: list[str] = [] + for item in response.json().get("models", []): + name = str(item.get("name", "")) + # Gemini returns "models/" and lists every model the key can see, + # including embedding-only ones that cannot answer a prompt at all. + if not name.startswith("models/"): + continue + if "generateContent" not in (item.get("supportedGenerationMethods") or []): + continue + models.append(name.removeprefix("models/")) + return models + + +async def _openrouter_models(config: AiProviderConfig, sender: ProviderHttpSender | None) -> list[str]: + require_api_key(config) + response = await get( + config, + "https://openrouter.ai/api/v1/models", + sender=sender, + headers={"Authorization": f"Bearer {config.api_key or ''}"}, + ) + return [str(item["id"]) for item in response.json().get("data", []) if item.get("id")] + + +async def _ollama_models(config: AiProviderConfig, sender: ProviderHttpSender | None) -> list[str]: + base = (config.base_url or "").rstrip("/") + if not base: + raise ValidationServiceError("Base URL is required for the selected AI provider.") + response = await get(config, f"{base}/api/tags", sender=sender) + return [str(item["name"]) for item in response.json().get("models", []) if item.get("name")] + + +_LISTERS = { + "openai": _openai_models, + "anthropic": _anthropic_models, + "gemini": _gemini_models, + "openrouter": _openrouter_models, + "ollama": _ollama_models, +} diff --git a/apps/backend/app/ai/types.py b/apps/backend/app/ai/types.py index 95a6a845..80466acc 100644 --- a/apps/backend/app/ai/types.py +++ b/apps/backend/app/ai/types.py @@ -5,11 +5,20 @@ from app.schemas.ai import AiCitation, AiProvider, AiProviderConfig +#: The model a provider starts on before anyone has fetched a list. +#: +#: A hardcoded default is a fact with a shelf life: `gemini-1.5-flash` was +#: correct when it was written and is not offered at all to a Google AI Studio +#: project created today, so every new user met "AI provider rejected the +#: request" with no way to discover what to type instead. These are kept +#: current, but the real answer to that problem is `providers/models.py`, which +#: asks the provider what this key can use -- a default is only ever the +#: starting point, never the only route to a working configuration. DEFAULT_MODELS: dict[AiProvider, str] = { - "openai": "gpt-4o-mini", - "anthropic": "claude-3-5-haiku-latest", - "gemini": "gemini-1.5-flash", - "openrouter": "openai/gpt-4o-mini", + "openai": "gpt-4.1-mini", + "anthropic": "claude-haiku-4-5-20251001", + "gemini": "gemini-2.5-flash", + "openrouter": "openai/gpt-4.1-mini", "ollama": "llama3.2", } diff --git a/apps/backend/app/api/routes/ai.py b/apps/backend/app/api/routes/ai.py index 77fb57b2..1d3a7306 100644 --- a/apps/backend/app/api/routes/ai.py +++ b/apps/backend/app/api/routes/ai.py @@ -8,6 +8,7 @@ AiConversationResponse, AiProviderCapabilitiesResponse, AiProviderConfig, + AiProviderModelsResponse, AiProviderPublicConfig, AiProviderTestRequest, AiProviderTestResponse, @@ -37,6 +38,10 @@ "summary": "Test the saved provider configuration", "value": {"provider": "openai", "model": "gpt-4.1-mini"}, } +_MODELS_REQUEST_EXAMPLE = { + "summary": "List the models a key can use, before saving it", + "value": {"provider": "gemini", "apiKey": "AIza-example-not-a-real-key"}, +} _CAPABILITIES_EXAMPLE = { "providers": [ { @@ -176,6 +181,27 @@ async def test_ai_config( return await service.test_connection(request) +@router.post( + "/models", + response_model=AiProviderModelsResponse, + responses=documented_responses( + 200, + "Model IDs the provider reports for this key, with the one a first-time setup should start on.", + {"models": ["gemini-2.0-flash", "gemini-2.5-flash", "gemini-2.5-pro"], "recommended": "gemini-2.0-flash"}, + 401, + 422, + 429, + 502, + 500, + ), +) +async def list_ai_models( + request: Annotated[AiProviderTestRequest, Body(openapi_examples={"models": _MODELS_REQUEST_EXAMPLE})], + service: AiService = Depends(get_ai_service), +) -> AiProviderModelsResponse: + return await service.list_models(request) + + @router.post( "/query", response_model=AiQueryResponse, diff --git a/apps/backend/app/schemas/ai.py b/apps/backend/app/schemas/ai.py index 5adf9fc9..0e13fcc1 100644 --- a/apps/backend/app/schemas/ai.py +++ b/apps/backend/app/schemas/ai.py @@ -75,6 +75,18 @@ class AiProviderTestResponse(CamelModel): checked_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) +class AiProviderModelsResponse(CamelModel): + """The model IDs this provider reports for the caller's own key (#291). + + Every entry came back from the provider, so the list is what the key can + actually use rather than what was true when a default was last written. + ``recommended`` is one of ``models``, never a value invented here. + """ + + models: list[str] + recommended: str + + class AiProviderCapability(CamelModel): """Safe, non-secret setup metadata for one provider (#291). diff --git a/apps/backend/app/services/ai_service.py b/apps/backend/app/services/ai_service.py index e912b026..acf3d86a 100644 --- a/apps/backend/app/services/ai_service.py +++ b/apps/backend/app/services/ai_service.py @@ -7,6 +7,7 @@ AiProviderCapability, AiProviderPublicConfig, AiProviderTestRequest, + AiProviderModelsResponse, AiProviderTestResponse, AiQueryRequest, AiQueryResponse, @@ -43,6 +44,9 @@ def save_config(self, config: AiProviderConfig) -> AiProviderPublicConfig: async def test_connection(self, request: AiProviderTestRequest) -> AiProviderTestResponse: return await self.orchestrator.test_connection(request) + async def list_models(self, request: AiProviderTestRequest) -> AiProviderModelsResponse: + return await self.orchestrator.list_models(request) + async def query(self, request: AiQueryRequest) -> AiQueryResponse: return await self.orchestrator.query(request) diff --git a/apps/backend/tests/test_ai_provider_capabilities.py b/apps/backend/tests/test_ai_provider_capabilities.py index 183c0307..c93962cf 100644 --- a/apps/backend/tests/test_ai_provider_capabilities.py +++ b/apps/backend/tests/test_ai_provider_capabilities.py @@ -8,6 +8,8 @@ import re +from app.ai.types import DEFAULT_MODELS + import pytest from app.ai.providers.capabilities import PROVIDER_CAPABILITIES, capability_for @@ -70,9 +72,14 @@ def test_list_providers_response_carries_no_secret_looking_material(auth_client) # step sentence, or a boolean/provider id -- assert none of them is a # long opaque token, which is what a real leaked credential would look # like regardless of which field carried it. + # A dated model id ("claude-haiku-4-5-20251001") is long enough to match + # the shape below while being a compile-time constant from the capability + # registry, not runtime data -- so it is exempted by exact value rather + # than by loosening the pattern, which would let a real leak through. + registry_models = set(DEFAULT_MODELS.values()) for item in response.json()["providers"]: for value in item.values(): - if isinstance(value, str): + if isinstance(value, str) and value not in registry_models: assert not re.fullmatch(r"[A-Za-z0-9_-]{24,}", value), f"looks like a token: {value!r}" diff --git a/apps/backend/tests/test_ai_provider_models.py b/apps/backend/tests/test_ai_provider_models.py new file mode 100644 index 00000000..4b615f69 --- /dev/null +++ b/apps/backend/tests/test_ai_provider_models.py @@ -0,0 +1,130 @@ +"""Model discovery: ask the provider what a key can use (#291). + +A hardcoded default is a fact with a shelf life. `gemini-1.5-flash` stopped +being available to newly created Google AI Studio projects, so a first-time +user met "AI provider rejected the request" with nothing on screen telling +them what to type instead. These tests cover the route out of that: the list +comes from the provider, over the same pinned egress a completion uses. +""" + +import asyncio +from typing import Any + +import httpx +import pytest + +from app.ai.providers.models import list_models, preferred_model +from app.ai.types import AiProviderConfig +from app.core.exceptions import ExternalServiceError, ValidationServiceError + + +class RecordingSender: + """Captures the outbound request and replays a canned provider body.""" + + def __init__(self, payload: dict[str, Any], status: int = 200) -> None: + self.payload = payload + self.status = status + self.calls: list[dict[str, Any]] = [] + + async def post(self, config: AiProviderConfig, url: str, **kwargs: object) -> httpx.Response: + raise AssertionError("model discovery must not POST") + + async def send(self, method: str, config: AiProviderConfig, url: str, **kwargs: object) -> httpx.Response: + self.calls.append({"method": method, "url": url, "kwargs": kwargs}) + return httpx.Response(self.status, json=self.payload, request=httpx.Request(method, "https://provider.example")) + + +@pytest.mark.parametrize( + ("provider", "base_url", "payload", "expected", "host"), + [ + ( + "openai", + None, + {"data": [{"id": "gpt-4.1-mini"}, {"id": "gpt-4.1"}]}, + ["gpt-4.1", "gpt-4.1-mini"], + "api.openai.com", + ), + ( + "anthropic", + None, + {"data": [{"id": "claude-haiku-4-5"}, {"id": "claude-sonnet-4-5"}]}, + ["claude-haiku-4-5", "claude-sonnet-4-5"], + "api.anthropic.com", + ), + ( + "openrouter", + None, + {"data": [{"id": "openai/gpt-4.1-mini"}]}, + ["openai/gpt-4.1-mini"], + "openrouter.ai", + ), + ( + "ollama", + "http://localhost:11434", + {"models": [{"name": "llama3.2"}, {"name": "qwen2.5-coder"}]}, + ["llama3.2", "qwen2.5-coder"], + "localhost", + ), + ], +) +def test_each_provider_reports_its_own_models(provider, base_url, payload, expected, host): + sender = RecordingSender(payload) + config = AiProviderConfig(provider=provider, api_key="key", base_url=base_url) + + models = asyncio.run(list_models(config, sender=sender)) + + assert models == expected + # A GET, against that provider's own host -- the same destination the + # egress policy already permits for a completion, never a new one. + assert sender.calls[0]["method"] == "GET" + assert host in sender.calls[0]["url"] + + +def test_gemini_drops_models_that_cannot_answer_a_prompt(): + """Gemini lists everything the key can see, including embedding-only + models. Offering one of those as a chat model would produce a failure the + user could not diagnose, so they never reach the list.""" + + sender = RecordingSender( + { + "models": [ + {"name": "models/gemini-2.0-flash", "supportedGenerationMethods": ["generateContent"]}, + {"name": "models/text-embedding-004", "supportedGenerationMethods": ["embedContent"]}, + {"name": "models/gemini-2.5-pro", "supportedGenerationMethods": ["generateContent"]}, + ] + } + ) + + models = asyncio.run(list_models(AiProviderConfig(provider="gemini", api_key="key"), sender=sender)) + + assert models == ["gemini-2.0-flash", "gemini-2.5-pro"] + assert "text-embedding-004" not in models + + +def test_a_key_that_can_use_nothing_is_reported_rather_than_returned_empty(): + sender = RecordingSender({"data": []}) + + with pytest.raises(ExternalServiceError): + asyncio.run(list_models(AiProviderConfig(provider="openai", api_key="key"), sender=sender)) + + +def test_listing_requires_the_credential_that_provider_needs(): + with pytest.raises(ValidationServiceError): + asyncio.run(list_models(AiProviderConfig(provider="openai"), sender=RecordingSender({}))) + # Ollama needs a base URL rather than a key, and says which. + with pytest.raises(ValidationServiceError): + asyncio.run(list_models(AiProviderConfig(provider="ollama"), sender=RecordingSender({}))) + + +def test_the_recommendation_is_always_one_of_the_listed_models(): + """The whole point is to stop naming a model the key does not have, so the + recommendation is picked from the provider's own answer or not at all.""" + + # The saved default wins when it is still offered: an existing, working + # configuration is never quietly moved to something else. + assert preferred_model(["gemini-2.0-flash", "gemini-2.5-pro"], "gemini-2.0-flash") == "gemini-2.0-flash" + # Otherwise the cheapest tier the provider itself published. + assert preferred_model(["gemini-2.5-pro", "gemini-2.5-flash"], "gemini-1.5-flash") == "gemini-2.5-flash" + # And when no tier hint matches, still a real entry rather than the + # retired default that caused the problem. + assert preferred_model(["some-model", "other-model"], "gemini-1.5-flash") == "some-model" diff --git a/apps/backend/tests/test_ai_providers.py b/apps/backend/tests/test_ai_providers.py index 17c3728f..c5f0ed2e 100644 --- a/apps/backend/tests/test_ai_providers.py +++ b/apps/backend/tests/test_ai_providers.py @@ -10,7 +10,7 @@ from app.ai.providers.ollama import OllamaProvider from app.ai.providers.openai import OpenAIProvider from app.ai.providers.openrouter import OpenRouterProvider -from app.ai.types import AiProviderConfig, PromptBundle +from app.ai.types import DEFAULT_MODELS, AiProviderConfig, PromptBundle from app.api.deps import get_provider_registry from app.core.exceptions import ExternalServiceError, TimeoutServiceError, ValidationServiceError @@ -72,7 +72,9 @@ def _provider_cases(): GeminiProvider, AiProviderConfig(provider="gemini", api_key="key"), {"candidates": [{"content": {"parts": [{"text": "Gemini answer"}]}}]}, - "https://generativelanguage.googleapis.com/v1beta/models/gemini-1.5-flash:generateContent", + # Derived, not spelled out: a default model is a fact with a shelf + # life, and this assertion is about the URL shape, not the value. + f"https://generativelanguage.googleapis.com/v1beta/models/{DEFAULT_MODELS['gemini']}:generateContent", ), ( OpenRouterProvider, diff --git a/apps/backend/tests/test_openapi_contract.py b/apps/backend/tests/test_openapi_contract.py index 8f8fe340..910d72c8 100644 --- a/apps/backend/tests/test_openapi_contract.py +++ b/apps/backend/tests/test_openapi_contract.py @@ -82,6 +82,7 @@ ("GET", "/ai/config"): {200, 401, 429, 500}, ("PUT", "/ai/config"): {200, 401, 422, 429, 500}, ("POST", "/ai/test"): {200, 401, 422, 429, 502, 500}, + ("POST", "/ai/models"): {200, 401, 422, 429, 502, 500}, ("POST", "/ai/query"): {200, 401, 404, 422, 429, 502, 500}, ("GET", "/ai/conversations"): {200, 401, 404, 422, 429, 500}, ("POST", "/documentation/generate"): {200, 401, 404, 422, 429, 500}, @@ -99,6 +100,7 @@ ("POST", "/repositories/github"): "application/json", ("PUT", "/ai/config"): "application/json", ("POST", "/ai/test"): "application/json", + ("POST", "/ai/models"): "application/json", ("POST", "/ai/query"): "application/json", ("POST", "/documentation/generate"): "application/json", ("POST", "/export"): "application/json", diff --git a/apps/frontend/src/app/pages/SettingsPage.test.tsx b/apps/frontend/src/app/pages/SettingsPage.test.tsx index e1ed4aab..8fd7f54e 100644 --- a/apps/frontend/src/app/pages/SettingsPage.test.tsx +++ b/apps/frontend/src/app/pages/SettingsPage.test.tsx @@ -113,6 +113,9 @@ function baseSettings(overrides: Partial> = {}): apiKey: '', setApiKey: vi.fn(), model: 'gpt-4o-mini', + models: [], + loadingModels: false, + fetchModels: vi.fn(), setModel: vi.fn(), baseUrl: '', setBaseUrl: vi.fn(), diff --git a/apps/frontend/src/app/pages/SettingsPage.tsx b/apps/frontend/src/app/pages/SettingsPage.tsx index 350b0be8..902e48c0 100644 --- a/apps/frontend/src/app/pages/SettingsPage.tsx +++ b/apps/frontend/src/app/pages/SettingsPage.tsx @@ -258,16 +258,10 @@ export function SettingsPage() { )} + {/* Credentials first, then the model: the list of models can only + be fetched once the key (or base URL) is in hand, so the fields + are ordered the way setup actually runs. */}
-
- - settings.setModel(event.target.value)} - className="w-full rounded-md border border-border bg-background px-3 py-2 text-sm text-foreground focus:outline-none focus:ring-1 focus:ring-ring" - /> -
{settings.activeCapability?.requiresBaseUrl && (
)} +
+
+ + +
+ {settings.models.length > 0 ? ( + + ) : ( + settings.setModel(event.target.value)} + className="w-full rounded-md border border-border bg-background px-3 py-2 text-sm text-foreground focus:outline-none focus:ring-1 focus:ring-ring" + /> + )} +

+ {settings.models.length > 0 + ? `${settings.models.length} models available to this key.` + : 'Fetch models to pick from what this key can actually use, or type an ID.'} +

+
{settings.error &&

{settings.error}

} {settings.statusMessage &&

{settings.statusMessage}

} diff --git a/apps/frontend/src/features/settings/hooks/useSettings.test.ts b/apps/frontend/src/features/settings/hooks/useSettings.test.ts index fb0604d8..232638c1 100644 --- a/apps/frontend/src/features/settings/hooks/useSettings.test.ts +++ b/apps/frontend/src/features/settings/hooks/useSettings.test.ts @@ -12,6 +12,7 @@ vi.mock('@/shared/services/api', () => ({ getConfig: vi.fn(), saveConfig: vi.fn(), testConfig: vi.fn(), + listModels: vi.fn(), }, getErrorMessage: vi.fn((error: unknown) => String(error)), })); @@ -185,3 +186,75 @@ describe('useSettings base URL handling across providers', () => { expect(vi.mocked(aiService.saveConfig).mock.lastCall?.[0].baseUrl).toBe('http://localhost:11434'); }); }); + +describe('useSettings model discovery', () => { + it('fetches the models a key can use and selects one that is in the list', async () => { + // The failure this replaces: a default model ID that the provider no + // longer offers, rejected with nothing on screen naming a valid one. + vi.mocked(aiService.listModels).mockResolvedValue({ + models: ['gemini-2.0-flash', 'gemini-2.5-pro'], + recommended: 'gemini-2.0-flash', + }); + + const { result } = renderHook(() => useSettings(), { wrapper: wrapper() }); + await waitFor(() => expect(result.current.capabilities).toHaveLength(2)); + + await act(async () => { + await result.current.fetchModels(); + }); + + expect(result.current.models).toEqual(['gemini-2.0-flash', 'gemini-2.5-pro']); + // The stale value is replaced, because leaving it is the bug. + expect(result.current.models).toContain(result.current.model); + expect(result.current.error).toBeNull(); + }); + + it('keeps a model the user already chose when the provider still offers it', async () => { + vi.mocked(aiService.listModels).mockResolvedValue({ + models: ['gpt-4.1', 'gpt-4.1-mini'], + recommended: 'gpt-4.1-mini', + }); + + const { result } = renderHook(() => useSettings(), { wrapper: wrapper() }); + await waitFor(() => expect(result.current.capabilities).toHaveLength(2)); + act(() => result.current.setModel('gpt-4.1')); + + await act(async () => { + await result.current.fetchModels(); + }); + + expect(result.current.model).toBe('gpt-4.1'); + }); + + it('drops the list when the provider changes, so one provider never offers another’s models', async () => { + vi.mocked(aiService.listModels).mockResolvedValue({ + models: ['gpt-4.1-mini'], + recommended: 'gpt-4.1-mini', + }); + + const { result } = renderHook(() => useSettings(), { wrapper: wrapper() }); + await waitFor(() => expect(result.current.capabilities).toHaveLength(2)); + await act(async () => { + await result.current.fetchModels(); + }); + expect(result.current.models).toHaveLength(1); + + act(() => result.current.setProvider('ollama')); + + expect(result.current.models).toEqual([]); + }); + + it('surfaces a discovery failure without leaving a stale list on screen', async () => { + vi.mocked(aiService.listModels).mockRejectedValue(new Error('rejected the API key')); + + const { result } = renderHook(() => useSettings(), { wrapper: wrapper() }); + await waitFor(() => expect(result.current.capabilities).toHaveLength(2)); + + await act(async () => { + await result.current.fetchModels(); + }); + + expect(result.current.models).toEqual([]); + expect(result.current.error).toContain('rejected the API key'); + }); +}); diff --git a/apps/frontend/src/features/settings/hooks/useSettings.ts b/apps/frontend/src/features/settings/hooks/useSettings.ts index 43fd1190..6dcb8da1 100644 --- a/apps/frontend/src/features/settings/hooks/useSettings.ts +++ b/apps/frontend/src/features/settings/hooks/useSettings.ts @@ -54,6 +54,8 @@ export function useSettings() { const [baseUrl, setBaseUrl] = useState(''); const [loading, setLoading] = useState(false); const [testing, setTesting] = useState(false); + const [models, setModels] = useState([]); + const [loadingModels, setLoadingModels] = useState(false); const [statusMessage, setStatusMessage] = useState(null); const [error, setError] = useState(null); @@ -96,6 +98,9 @@ export function useSettings() { if (!capabilityByProvider.get(nextProvider)?.requiresBaseUrl) { setBaseUrl(''); } + // The list belongs to the provider it came from; keeping it would offer + // one provider's models under another's name. + setModels([]); setStatusMessage(null); setError(null); }, @@ -109,6 +114,33 @@ export function useSettings() { return baseUrl.trim() || undefined; }, [baseUrl, capabilityByProvider, provider]); + // The list comes from the provider, for this key. A model ID typed by hand + // is the step provider setup fails on: a default that was right when it was + // written stops being offered, and the only signal is a rejected request + // naming nothing the user could have typed instead. + const fetchModels = useCallback(async () => { + setLoadingModels(true); + setError(null); + setStatusMessage(null); + try { + const response = await aiService.listModels({ + provider, + apiKey: apiKey.trim() || undefined, + baseUrl: baseUrlForRequest(), + }); + setModels(response.models); + // Land on something that works rather than leaving the previous, + // possibly rejected, value in place. + if (!response.models.includes(model.trim())) setModel(response.recommended); + setStatusMessage(`Found ${response.models.length} model${response.models.length === 1 ? '' : 's'}.`); + } catch (caught) { + setModels([]); + setError(getErrorMessage(caught)); + } finally { + setLoadingModels(false); + } + }, [apiKey, baseUrlForRequest, model, provider]); + const saveAiConfig = useCallback(async () => { setLoading(true); setError(null); @@ -167,6 +199,9 @@ export function useSettings() { setModel, baseUrl, setBaseUrl, + models, + loadingModels, + fetchModels, saveAiConfig, testAiConfig, testing, diff --git a/apps/frontend/src/shared/services/api/ai.ts b/apps/frontend/src/shared/services/api/ai.ts index 7f3851c4..2f70ab65 100644 --- a/apps/frontend/src/shared/services/api/ai.ts +++ b/apps/frontend/src/shared/services/api/ai.ts @@ -9,6 +9,7 @@ import type { AiProviderPublicConfig, AiProviderTestRequest, AiProviderTestResponse, + AiProviderModelsResponse, } from './types'; export const aiService = { @@ -28,6 +29,10 @@ export const aiService = { return api.post('/ai/test', request, config); }, + listModels(request: AiProviderTestRequest, config?: RequestConfig): Promise { + return api.post('/ai/models', request, config); + }, + query(request: AiQueryRequest, config?: RequestConfig): Promise { return api.post('/ai/query', request, config); }, diff --git a/apps/frontend/src/shared/services/api/generated.ts b/apps/frontend/src/shared/services/api/generated.ts index 136e363f..3a53adf1 100644 --- a/apps/frontend/src/shared/services/api/generated.ts +++ b/apps/frontend/src/shared/services/api/generated.ts @@ -42,6 +42,23 @@ export interface paths { patch?: never; trace?: never; }; + "/ai/models": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** List Ai Models */ + post: operations["list_ai_models_ai_models_post"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/ai/providers": { parameters: { query?: never; @@ -962,6 +979,20 @@ export interface components { */ provider: "openai" | "anthropic" | "gemini" | "openrouter" | "ollama"; }; + /** + * AiProviderModelsResponse + * @description The model IDs this provider reports for the caller's own key (#291). + * + * Every entry came back from the provider, so the list is what the key can + * actually use rather than what was true when a default was last written. + * ``recommended`` is one of ``models``, never a value invented here. + */ + AiProviderModelsResponse: { + /** Models */ + models: string[]; + /** Recommended */ + recommended: string; + }; /** AiProviderPublicConfig */ AiProviderPublicConfig: { /** Apikeylast4 */ @@ -3092,6 +3123,137 @@ export interface operations { }; }; }; + list_ai_models_ai_models_post: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["AiProviderTestRequest"]; + }; + }; + responses: { + /** @description Model IDs the provider reports for this key, with the one a first-time setup should start on. */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + /** + * @example { + * "models": [ + * "gemini-2.0-flash", + * "gemini-2.5-flash", + * "gemini-2.5-pro" + * ], + * "recommended": "gemini-2.0-flash" + * } + */ + "application/json": components["schemas"]["AiProviderModelsResponse"]; + }; + }; + /** @description Authentication is required or the access token is invalid. */ + 401: { + headers: { + [name: string]: unknown; + }; + content: { + /** + * @example { + * "code": "unauthorized", + * "message": "Not authenticated.", + * "request_id": "req_01HXYZEXAMPLE" + * } + */ + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + /** @description The request could not be validated. */ + 422: { + headers: { + [name: string]: unknown; + }; + content: { + /** + * @example { + * "code": "request_validation_error", + * "message": "Request validation failed.", + * "details": { + * "errors": [ + * { + * "loc": [ + * "body", + * "url" + * ], + * "msg": "Field required" + * } + * ] + * }, + * "request_id": "req_01HXYZEXAMPLE" + * } + */ + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + /** @description The request-rate limit has been exceeded. */ + 429: { + headers: { + [name: string]: unknown; + }; + content: { + /** + * @example { + * "code": "rate_limited", + * "message": "Too many requests. Try again shortly.", + * "details": { + * "retryAfterSeconds": 30 + * }, + * "request_id": "req_01HXYZEXAMPLE" + * } + */ + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + /** @description An unexpected server error occurred. */ + 500: { + headers: { + [name: string]: unknown; + }; + content: { + /** + * @example { + * "code": "internal_server_error", + * "message": "An unexpected error occurred.", + * "request_id": "req_01HXYZEXAMPLE" + * } + */ + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + /** @description An upstream service could not complete the request. */ + 502: { + headers: { + [name: string]: unknown; + }; + content: { + /** + * @example { + * "code": "external_service_error", + * "message": "AI provider request failed.", + * "details": { + * "provider": "openai" + * }, + * "request_id": "req_01HXYZEXAMPLE" + * } + */ + "application/json": components["schemas"]["ErrorResponse"]; + }; + }; + }; + }; list_ai_providers_ai_providers_get: { parameters: { query?: never; diff --git a/apps/frontend/src/shared/services/api/types.ts b/apps/frontend/src/shared/services/api/types.ts index bc820758..be7039b0 100644 --- a/apps/frontend/src/shared/services/api/types.ts +++ b/apps/frontend/src/shared/services/api/types.ts @@ -92,6 +92,7 @@ export type AiProviderConfig = components['schemas']['AiProviderConfig']; export type AiProviderPublicConfig = components['schemas']['AiProviderPublicConfig']; export type AiProviderTestRequest = components['schemas']['AiProviderTestRequest']; export type AiProviderTestResponse = components['schemas']['AiProviderTestResponse']; +export type AiProviderModelsResponse = components['schemas']['AiProviderModelsResponse']; export type AiProviderCapability = components['schemas']['AiProviderCapability']; export type AiProviderCapabilitiesResponse = components['schemas']['AiProviderCapabilitiesResponse']; export interface AiConversationResponse {