Merge pull request #2683 from langbot-app/test/quality-unit-api-providers-20261017

test: strengthen provider service and secret response contracts

tests/unit_tests/api/service/test_agent_service.py

@@ -868,6 +868,8 @@ async def test_event_processor_rejects_invalid_component_and_missing_parameters(
     service = AgentService(app)
     with pytest.raises(ValueError, match='Select an installed'):
         await service.create_agent(WORKSPACE_UUID, {'kind': 'event_processor', 'component_ref': 'invalid:a/b/c'})
+    app.persistence_mgr.execute_async.assert_not_awaited()
+    app.runner_registry.get.assert_not_awaited()
     app.runner_registry = SimpleNamespace(
         get=AsyncMock(
             return_value=SimpleNamespace(
@@ -882,6 +884,8 @@ async def test_event_processor_rejects_invalid_component_and_missing_parameters(
         await service.create_agent(
             WORKSPACE_UUID, {'kind': 'event_processor', 'component_ref': 'plugin:a/b/c', 'parameters': {}}
         )
+    app.persistence_mgr.execute_async.assert_not_awaited()
+    app.runner_registry.get.assert_awaited_once_with(WORKSPACE_UUID, 'plugin:a/b/c')
 
 
 async def test_unavailable_event_processor_can_still_be_renamed():

tests/unit_tests/api/service/test_knowledge_service.py

@@ -390,6 +390,7 @@ class TestGetKnowledgeBase:
         result = await KnowledgeService(app).get_knowledge_base(CONTEXT, 'kb1')
 
         assert result == {'uuid': 'kb1', 'name': 'KB1'}
+        app.rag_mgr.get_knowledge_base_details.assert_awaited_once_with(CONTEXT, 'kb1')
 
     @pytest.mark.asyncio
     async def test_returns_none_when_not_found(self):
@@ -555,7 +556,11 @@ class TestListParsers:
 
         result = await KnowledgeService(app).list_parsers(CONTEXT)
 
-        assert len(result) == 2
+        assert result == [
+            {'id': 'parser1', 'supported_mime_types': ['text/plain']},
+            {'id': 'parser2', 'supported_mime_types': ['application/pdf']},
+        ]
+        app.plugin_connector.list_parsers.assert_awaited_once_with()
 
     @pytest.mark.asyncio
     async def test_filters_by_mime_type(self):
@@ -589,7 +594,8 @@ class TestGetEngineSchemas:
             'author/engine',
         )
 
-        assert 'properties' in result
+        assert result == {'properties': {'name': {'type': 'string'}}}
+        app.plugin_connector.get_rag_creation_schema.assert_awaited_once_with('author/engine')
 
     @pytest.mark.asyncio
     async def test_returns_retrieval_schema(self):
@@ -601,7 +607,8 @@ class TestGetEngineSchemas:
             'author/engine',
         )
 
-        assert 'properties' in result
+        assert result == {'properties': {'top_k': {'type': 'integer'}}}
+        app.plugin_connector.get_rag_retrieval_schema.assert_awaited_once_with('author/engine')
 
     @pytest.mark.asyncio
     async def test_reports_schema_runtime_exception(self):

tests/unit_tests/api/service/test_provider_service.py

@@ -14,12 +14,14 @@ Source: src/langbot/pkg/api/http/service/provider.py
 from __future__ import annotations
 
 import pytest
+import sqlalchemy
+from uuid import UUID
 from contextlib import nullcontext
 from unittest.mock import AsyncMock, Mock
 from types import SimpleNamespace
 
 from langbot.pkg.api.http.service.provider import ModelProviderService
-from langbot.pkg.entity.persistence.model import ModelProvider, LLMModel
+from langbot.pkg.entity.persistence.model import ModelProvider, LLMModel, EmbeddingModel, RerankModel
 from langbot.pkg.workspace.errors import WorkspaceNotFoundError
 
 
@@ -300,7 +302,23 @@ class TestModelProviderServiceCreateProvider:
 
         # Verify - UUID is generated
         assert provider_uuid is not None
-        assert len(provider_uuid) == 36  # UUID format
+        assert UUID(provider_uuid).version == 4
+        expected = {
+            'uuid': provider_uuid,
+            'workspace_uuid': WORKSPACE_UUID,
+            'name': 'New Provider',
+            'requester': 'openai',
+            'base_url': 'https://api.openai.com',
+            'api_keys': ['key'],
+        }
+        ap.persistence_mgr.execute_async.assert_awaited_once()
+        statement = ap.persistence_mgr.execute_async.await_args.args[0]
+        assert statement.is_insert
+        assert statement.table.name == ModelProvider.__tablename__
+        values = statement.compile().params
+        assert {key: values.get(key) for key in expected} == expected
+        ap.model_mgr.load_provider.assert_awaited_once_with(WORKSPACE_UUID, expected)
+        ap.model_mgr.cache_provider.assert_awaited_once_with(WORKSPACE_UUID, runtime_provider)
 
     async def test_create_provider_loads_to_runtime(self):
         """Loads provider to runtime model_mgr."""
@@ -362,7 +380,15 @@ class TestModelProviderServiceUpdateProvider:
         )
 
         # Verify - reload called
-        ap.model_mgr.reload_provider.assert_called_once_with(WORKSPACE_UUID, 'existing-uuid')
+        ap.model_mgr.reload_provider.assert_awaited_once_with(WORKSPACE_UUID, 'existing-uuid')
+        ap.persistence_mgr.execute_async.assert_awaited_once()
+        statement = ap.persistence_mgr.execute_async.await_args.args[0]
+        assert statement.is_update
+        assert 'uuid' not in statement.compile().params
+        assert statement.compile().params.get('name') == 'Updated Name'
+        assert statement.whereclause.compare(
+            sqlalchemy.and_(ModelProvider.uuid == 'existing-uuid', ModelProvider.workspace_uuid == WORKSPACE_UUID)
+        )
 
     async def test_update_provider_reloads_runtime(self):
         """Reloads provider in runtime after update."""
@@ -432,16 +458,20 @@ class TestModelProviderServiceGetProviderModelCounts:
         rerank_result = Mock()
         rerank_result.scalar = Mock(return_value=1)
 
-        call_count = 0
+        results_by_table = {
+            LLMModel.__tablename__: (LLMModel, llm_result),
+            EmbeddingModel.__tablename__: (EmbeddingModel, embedding_result),
+            RerankModel.__tablename__: (RerankModel, rerank_result),
+        }
 
         async def mock_execute(query):
-            nonlocal call_count
-            call_count += 1
-            if call_count == 1:
-                return llm_result
-            elif call_count == 2:
-                return embedding_result
-            return rerank_result
+            model_type, result = results_by_table[query.get_final_froms()[0].name]
+            assert query.whereclause.compare(
+                sqlalchemy.and_(
+                    model_type.provider_uuid == 'provider-uuid', model_type.workspace_uuid == WORKSPACE_UUID
+                )
+            )
+            return result
 
         ap.persistence_mgr.execute_async = AsyncMock(side_effect=mock_execute)
 
@@ -602,7 +632,12 @@ class TestModelProviderServiceFindOrCreateProvider:
         )
 
         # Verify - name should be parsed from URL (api.example.com)
-        ap.model_mgr.load_provider.assert_called_once()
+        ap.model_mgr.load_provider.assert_awaited_once()
+        assert ap.model_mgr.load_provider.await_args.args[0] == WORKSPACE_UUID
+        assert ap.model_mgr.load_provider.await_args.args[1]['name'] == 'api.example.com'
+        statement = ap.persistence_mgr.execute_async.await_args.args[0]
+        assert statement.is_insert
+        assert statement.compile().params.get('name') == 'api.example.com'
         assert result_uuid is not None
 
 
@@ -624,6 +659,16 @@ class TestModelProviderServiceUpdateSpaceModelProviderApiKeys:
         # Execute
         await service.update_space_model_provider_api_keys(WORKSPACE_UUID, 'space-api-key')
 
+        ap.persistence_mgr.execute_async.assert_awaited_once()
+        statement = ap.persistence_mgr.execute_async.await_args.args[0]
+        assert statement.is_update
+        assert statement.compile().params.get('api_keys') == ['space-api-key']
+        assert statement.whereclause.compare(
+            sqlalchemy.and_(
+                ModelProvider.uuid == '00000000-0000-0000-0000-000000000000',
+                ModelProvider.workspace_uuid == WORKSPACE_UUID,
+            )
+        )
         # Verify - update and reload called for Space provider UUID
         ap.model_mgr.reload_provider.assert_called_once_with(
             WORKSPACE_UUID,

tests/unit_tests/api/test_provider_controller_secrets.py

@@ -62,9 +62,7 @@ async def _create_client(role: str):
     application.provider_service = SimpleNamespace(
         get_providers=AsyncMock(side_effect=get_providers),
         get_provider=AsyncMock(side_effect=get_provider),
-        get_provider_model_counts=AsyncMock(
-            return_value={'llm_count': 1, 'embedding_count': 1, 'rerank_count': 1}
-        ),
+        get_provider_model_counts=AsyncMock(return_value={'llm_count': 1, 'embedding_count': 1, 'rerank_count': 1}),
     )
 
     def model_service(list_name: str, get_name: str):
@@ -136,8 +134,15 @@ async def test_default_preserves_secrets_and_explicit_false_redacts_high_permiss
     false_data = await false_response.get_json()
     default_value = default_data['data'].get('providers', default_data['data'].get('models'))[0]
     false_value = false_data['data'].get('providers', false_data['data'].get('models'))[0]
-    assert '***' not in str(default_value)
+    expected = (
+        {**RAW_PROVIDER, 'llm_count': 1, 'embedding_count': 1, 'rerank_count': 1}
+        if endpoint.endswith('/providers')
+        else RAW_MODEL
+    )
+    assert default_value == expected
     assert '***' in str(false_value)
+    assert 'provider-secret' not in str(false_data)
+    assert 'model-secret' not in str(false_data)
 
 
 @pytest.mark.parametrize(
@@ -158,6 +163,8 @@ async def test_explicit_true_does_not_grant_low_permission_reads(endpoint):
     data = await response.get_json()
     value = data['data'].get('providers', data['data'].get('models'))[0]
     assert '***' in str(value)
+    assert 'provider-secret' not in str(data)
+    assert 'model-secret' not in str(data)
 
 
 @pytest.mark.parametrize(