fix(openapi): stop create.model.credential from switching the model's active credential

api/controllers/openapi/model_providers.py

@@ -358,7 +358,7 @@ class ModelCredentialsApi(Resource):
         op="create.model.credential",
         kind=Kind.OBJECT,
         summary="Save a credential for one model you name, for providers such as OpenAI-compatible or Ollama; "
-        "it becomes the model's active credential when the model has none",
+        "the first credential of a model not yet set up becomes its active credential",
         examples=(
             Example(
                 title="A self-hosted model",
@@ -375,9 +375,8 @@ class ModelCredentialsApi(Resource):
         returns=(HTTPStatus.CREATED, CredentialWriteResponse, "Credential saved"),
     )
     def post(self, ctx: Context, workspace_id: str, provider: str, *, body: ModelCredentialCreatePayload):
-        service = ModelProviderService()
         with _model_provider_errors():
-            credential_id = service.create_model_credential(
+            credential_id = ModelProviderService().create_model_credential(
                 tenant_id=ctx.workspace.id,
                 provider=provider,
                 model_type=body.model_type.value,
@@ -385,9 +384,6 @@ class ModelCredentialsApi(Resource):
                 credentials=body.credentials,
                 credential_name=body.name,
             )
-            service.activate_model_credential_if_none(
-                ctx.workspace.id, provider, body.model_type.value, body.model, credential_id
-            )
         response = _provider(ctx.workspace.id, provider)
         return _model_write_response(ctx.workspace.id, response, body, credential_id), HTTPStatus.CREATED
 

api/services/model_provider/service.py

@@ -647,24 +647,6 @@ class ModelProviderService:
             credential_name=credential_name,
         )
 
-    def activate_model_credential_if_none(
-        self, tenant_id: str, provider: str, model_type: str, model: str, credential_id: str
-    ) -> None:
-        """Make the credential the model's active one when the model has none, as a provider's first credential is."""
-        provider_configuration = self._get_provider_configuration(tenant_id, provider)
-        current = next(
-            (
-                m
-                for m in provider_configuration.custom_configuration.models
-                if m.model == model and m.model_type == ModelType(model_type)
-            ),
-            None,
-        )
-        if current is not None and not current.current_credential_id:
-            provider_configuration.switch_custom_model_credential(
-                model_type=ModelType(model_type), model=model, credential_id=credential_id
-            )
-
     def update_model_credential(
         self,
         tenant_id: str,

api/tests/unit_tests/controllers/openapi/test_model_providers.py

@@ -1,13 +1,22 @@
+from collections.abc import Callable
 from types import SimpleNamespace
-from typing import cast
-from unittest.mock import Mock, patch
+from typing import Protocol, cast
+from unittest.mock import Mock
 
 import pytest
+from flask import Flask
 from pydantic import ValidationError
 
-from controllers.openapi._models import ModelListQuery
+from controllers.openapi import model_providers
+from controllers.openapi._models import CredentialWriteResponse, ModelCredentialCreatePayload, ModelListQuery, ModelRef
 from controllers.openapi._search import matches
-from controllers.openapi.model_providers import _provider_write_response, model_rows
+from controllers.openapi.auth.context import Context
+from controllers.openapi.model_providers import (
+    ModelCredentialsApi,
+    _model_write_response,
+    _provider_write_response,
+    model_rows,
+)
 from core.entities.model_entities import ModelStatus, ModelWithProviderEntity, SimpleModelProviderEntity
 from graphon.model_runtime.entities.common_entities import I18nObject
 from graphon.model_runtime.entities.model_entities import FetchFrom, ModelPropertyKey, ModelType
@@ -108,15 +117,64 @@ def test_inactive_credential_hints_replace_active() -> None:
     assert hint.input["credential_id"] == "active-id"
 
 
+class _CreateView(Protocol):
+    __handler__: Callable[..., tuple[CredentialWriteResponse, int]]
+
+
+def _model_provider(*custom_models: SimpleNamespace) -> ProviderResponse:
+    configuration = SimpleNamespace(custom_models=list(custom_models))
+    return cast(ProviderResponse, SimpleNamespace(provider=_OLLAMA, custom_configuration=configuration))
+
+
 def _custom(current: str | None) -> SimpleNamespace:
-    return SimpleNamespace(model="llama3", model_type=ModelType.LLM, current_credential_id=current)
+    return SimpleNamespace(
+        model="llama3",
+        model_type=ModelType.LLM,
+        current_credential_id=current,
+        available_model_credentials=[
+            SimpleNamespace(credential_id="new-id", credential_name="spare"),
+            SimpleNamespace(credential_id="other-id", credential_name="main"),
+        ],
+    )
 
 
-@pytest.mark.parametrize(("current", "switched"), [(None, True), ("other-id", False)])
-def test_new_model_credential_becomes_active_only_when_none_is(current: str | None, switched: bool) -> None:
-    configuration = Mock()
-    configuration.custom_configuration.models = [_custom(current)]
-    service = ModelProviderService()
-    with patch.object(service, "_get_provider_configuration", return_value=configuration):
-        service.activate_model_credential_if_none("ws", "langgenius/ollama/ollama", "llm", "llama3", "new-id")
-    assert configuration.switch_custom_model_credential.called is switched
+_LLAMA = ModelRef(model="llama3", model_type=ModelType.LLM)
+_OLLAMA = "langgenius/ollama/ollama"
+
+
+def test_a_new_models_first_credential_reads_back_active() -> None:
+    response = _model_write_response("ws", _model_provider(_custom("new-id")), _LLAMA, "new-id")
+    assert (response.active, response.name, response.hints) == (True, "spare", [])
+
+
+def test_a_credential_beside_an_active_one_hints_replacing_the_active_one() -> None:
+    response = _model_write_response("ws", _model_provider(_custom("other-id")), _LLAMA, "new-id")
+    [hint] = response.hints
+    assert not response.active
+    assert hint.op == "set.model.credential"
+    assert hint.input["credential_id"] == "other-id"
+
+
+@pytest.mark.parametrize(
+    "provider", [_model_provider(_custom(None)), _model_provider()], ids=["none-active", "unknown"]
+)
+def test_nothing_active_reads_back_inactive_without_a_hint(provider: ProviderResponse) -> None:
+    response = _model_write_response("ws", provider, _LLAMA, "new-id")
+    assert (response.active, response.hints) == (False, [])
+
+
+def test_creating_a_model_credential_never_switches_the_active_one(app: Flask, monkeypatch: pytest.MonkeyPatch) -> None:
+    service = Mock(spec=ModelProviderService)
+    service.create_model_credential.return_value = "new-id"
+    monkeypatch.setattr(model_providers, "ModelProviderService", Mock(return_value=service))
+    monkeypatch.setattr(model_providers, "_provider", lambda *_args: _model_provider(_custom("other-id")))
+    body = ModelCredentialCreatePayload(model="llama3", model_type=ModelType.LLM, credentials={"base_url": "x"})
+    ctx = cast(Context, SimpleNamespace(workspace=SimpleNamespace(id="ws")))
+
+    api = ModelCredentialsApi()
+    with app.test_request_context("/openapi/v1/workspaces/ws/model-providers/x/models/credentials", method="POST"):
+        response, status = cast(_CreateView, api.post).__handler__(api, ctx, "ws", _OLLAMA, body=body)
+
+    assert status == 201
+    assert not response.active
+    assert [call[0] for call in service.method_calls] == ["create_model_credential"]