diff --git a/docs/configuration.md b/docs/configuration.md index 7608c94b..dd71e300 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -593,6 +593,8 @@ equivalent.** Operators who need a runtime toggle should open an issue. | `MISTRAL_EMBEDDING_MODEL` | ⚠️ Optional | `mistral-embed` | Mistral embedding model (1024-dim) | | `MISTRAL_BASE_URL` | ⚠️ Optional | - | Mistral base URL override (proxies, on-prem) | | `AWS_REGION` | ⚠️ Optional | - | AWS region (selects Bedrock provider) | +| `AWS_ACCESS_KEY_ID` | ⚠️ Optional | - | AWS access key (boto3 credential chain fallback) | +| `AWS_SECRET_ACCESS_KEY` | ⚠️ Optional | - | AWS secret key (boto3 credential chain fallback) | | `BEDROCK_EMBEDDING_MODEL` | ⚠️ Optional | - | Bedrock embedding model ID | | `BEDROCK_GENERATION_MODEL` | ⚠️ Optional | - | Bedrock generation model ID | | `SIMPLE_EMBEDDING_DIMENSION` | ⚠️ Optional | `384` | Dimension for the fallback Simple provider | diff --git a/nextcloud_mcp_server/providers/openai.py b/nextcloud_mcp_server/providers/openai.py index 1b044039..536697bd 100644 --- a/nextcloud_mcp_server/providers/openai.py +++ b/nextcloud_mcp_server/providers/openai.py @@ -77,9 +77,12 @@ class OpenAIProvider(Provider): self._dimension = OPENAI_EMBEDDING_DIMENSIONS[embedding_model] logger.info( - f"Initialized OpenAI provider: base_url={base_url or 'default'} " - f"(embedding_model={embedding_model}, generation_model={generation_model}, " - f"dimension={self._dimension})" + "Initialized OpenAI provider: base_url=%s " + "(embedding_model=%s, generation_model=%s, dimension=%s)", + base_url or "default", + embedding_model, + generation_model, + self._dimension, ) @property @@ -123,8 +126,9 @@ class OpenAIProvider(Provider): if self._dimension is None: self._dimension = len(embedding) logger.info( - f"Detected embedding dimension: {self._dimension} " - f"for model {self.embedding_model}" + "Detected embedding dimension: %d for model %s", + self._dimension, + self.embedding_model, ) return embedding @@ -167,8 +171,9 @@ class OpenAIProvider(Provider): if self._dimension is None and batch_embeddings: self._dimension = len(batch_embeddings[0]) logger.info( - f"Detected embedding dimension: {self._dimension} " - f"for model {self.embedding_model}" + "Detected embedding dimension: %d for model %s", + self._dimension, + self.embedding_model, ) return all_embeddings diff --git a/tests/unit/providers/test_mistral.py b/tests/unit/providers/test_mistral.py index d3c58c15..0e38da15 100644 --- a/tests/unit/providers/test_mistral.py +++ b/tests/unit/providers/test_mistral.py @@ -200,6 +200,61 @@ async def test_mistral_base_url_passed_to_sdk(mocker): ) +@pytest.mark.unit +async def test_mistral_embed_raises_on_empty_response_data(mock_mistral_client): + """embed(): empty response.data triggers the defensive RuntimeError guard.""" + empty_response = MagicMock() + empty_response.data = [] + mock_mistral_client.embeddings.create_async = AsyncMock(return_value=empty_response) + + provider = MistralProvider(api_key="test-key", embedding_model="mistral-embed") + with pytest.raises(RuntimeError, match="returned no embedding"): + await provider.embed("test") + + +@pytest.mark.unit +async def test_mistral_embed_raises_on_null_embedding(mock_mistral_client): + """embed(): a single response item with embedding=None is rejected.""" + null_item = MagicMock() + null_item.embedding = None + null_item.index = 0 + null_response = MagicMock() + null_response.data = [null_item] + mock_mistral_client.embeddings.create_async = AsyncMock(return_value=null_response) + + provider = MistralProvider(api_key="test-key", embedding_model="mistral-embed") + with pytest.raises(RuntimeError, match="returned no embedding"): + await provider.embed("test") + + +@pytest.mark.unit +async def test_mistral_batch_raises_on_null_embedding(mock_mistral_client): + """_embed_batch_request: a null embedding inside a batch raises explicitly.""" + good = _make_data([0.1, 0.2], 0) + bad = MagicMock() + bad.embedding = None + bad.index = 1 + response = MagicMock() + response.data = [good, bad] + mock_mistral_client.embeddings.create_async = AsyncMock(return_value=response) + + provider = MistralProvider(api_key="test-key", embedding_model="mistral-embed") + with pytest.raises(RuntimeError, match="null embedding"): + await provider.embed_batch(["a", "b"]) + + +@pytest.mark.unit +async def test_mistral_batch_raises_on_count_mismatch(mock_mistral_client): + """_embed_batch_request: fewer embeddings returned than inputs sent.""" + # Two inputs sent, one embedding returned. + response = _make_response([[0.1, 0.2]]) + mock_mistral_client.embeddings.create_async = AsyncMock(return_value=response) + + provider = MistralProvider(api_key="test-key", embedding_model="mistral-embed") + with pytest.raises(RuntimeError, match="returned 1 embeddings for 2 inputs"): + await provider.embed_batch(["a", "b"]) + + @pytest.mark.unit def test_mistral_is_rate_limit_predicate(): """_is_rate_limit returns True only for SDKErrors with status_code == 429.""" diff --git a/tests/unit/providers/test_registry.py b/tests/unit/providers/test_registry.py index b9be98f8..5d4c3226 100644 --- a/tests/unit/providers/test_registry.py +++ b/tests/unit/providers/test_registry.py @@ -108,8 +108,11 @@ def test_registry_openai_wins_over_mistral_and_ollama(clean_provider_env): @pytest.mark.unit -def test_registry_mistral_wins_over_ollama(clean_provider_env): +def test_registry_mistral_wins_over_ollama(clean_provider_env, mocker): """Mistral takes priority over Ollama when both are configured.""" + # Stub the Mistral SDK constructor for the same reason as the sibling + # picker test — keeps the registry test independent of SDK key validation. + mocker.patch("nextcloud_mcp_server.providers.mistral.Mistral") clean_provider_env.setenv("MISTRAL_API_KEY", "mistral-key") clean_provider_env.setenv("OLLAMA_BASE_URL", "http://localhost:11434") _reload_config()