diff --git a/docs/configuration.md b/docs/configuration.md index bb2c79bb..7608c94b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -583,10 +583,12 @@ equivalent.** Operators who need a runtime toggle should open an issue. | `VECTOR_SYNC_QUEUE_MAX_SIZE` | ⚠️ Optional | `10000` | Max queued documents | | `OLLAMA_BASE_URL` | ⚠️ Optional | - | Ollama API endpoint for embeddings | | `OLLAMA_EMBEDDING_MODEL` | ⚠️ Optional | `nomic-embed-text` | Embedding model to use | +| `OLLAMA_GENERATION_MODEL` | ⚠️ Optional | - | Ollama model for text generation | | `OLLAMA_VERIFY_SSL` | ⚠️ Optional | `true` | Verify SSL certificates | | `OPENAI_API_KEY` | ⚠️ Optional | - | OpenAI API key (selects OpenAI provider) | | `OPENAI_BASE_URL` | ⚠️ Optional | - | OpenAI base URL override (for compatible APIs) | | `OPENAI_EMBEDDING_MODEL` | ⚠️ Optional | `text-embedding-3-small` | OpenAI embedding model | +| `OPENAI_GENERATION_MODEL` | ⚠️ Optional | - | OpenAI model for text generation | | `MISTRAL_API_KEY` | ⚠️ Optional | - | Mistral API key (selects Mistral provider) | | `MISTRAL_EMBEDDING_MODEL` | ⚠️ Optional | `mistral-embed` | Mistral embedding model (1024-dim) | | `MISTRAL_BASE_URL` | ⚠️ Optional | - | Mistral base URL override (proxies, on-prem) | diff --git a/nextcloud_mcp_server/providers/_retry.py b/nextcloud_mcp_server/providers/_retry.py index d1ced096..8fb8011e 100644 --- a/nextcloud_mcp_server/providers/_retry.py +++ b/nextcloud_mcp_server/providers/_retry.py @@ -69,7 +69,8 @@ def retry_on_rate_limit( logger.error( "%s rate limit exceeded after %d attempts", provider_name, MAX_RETRIES ) - assert last_error is not None # The loop above always assigns it. + if last_error is None: # pragma: no cover — loop above always sets this + raise RuntimeError("retry loop exited without capturing an error") raise last_error return wrapper diff --git a/nextcloud_mcp_server/providers/mistral.py b/nextcloud_mcp_server/providers/mistral.py index 26702e93..b5f61584 100644 --- a/nextcloud_mcp_server/providers/mistral.py +++ b/nextcloud_mcp_server/providers/mistral.py @@ -6,6 +6,10 @@ can be added later if needed; see ADR-015. import logging +# mistralai 2.x ships no top-level __init__.py, so `from mistralai import …` +# raises ImportError. The canonical public paths are `mistralai.client` (which +# re-exports the SDK class via `client/__init__.py`) and `mistralai.client.errors` +# (which lazy-loads SDKError). There is no `mistralai.models` subpackage either. from mistralai.client import Mistral from mistralai.client.errors import SDKError diff --git a/nextcloud_mcp_server/providers/openai.py b/nextcloud_mcp_server/providers/openai.py index df0407be..1b044039 100644 --- a/nextcloud_mcp_server/providers/openai.py +++ b/nextcloud_mcp_server/providers/openai.py @@ -10,14 +10,14 @@ import logging from openai import AsyncOpenAI, RateLimitError -from ._retry import retry_on_rate_limit as _retry_factory +from ._retry import retry_on_rate_limit from .base import Provider logger = logging.getLogger(__name__) # OpenAI's RateLimitError is itself a 429-specific class, so the default # is_rate_limit predicate ("always True") matches the previous behavior. -retry_on_rate_limit = _retry_factory(RateLimitError, provider_name="OpenAI") +_retry_429 = retry_on_rate_limit(RateLimitError, provider_name="OpenAI") # Well-known embedding dimensions for OpenAI models @@ -92,7 +92,7 @@ class OpenAIProvider(Provider): """Whether this provider supports text generation.""" return self.generation_model is not None - @retry_on_rate_limit + @_retry_429 async def embed(self, text: str) -> list[float]: """ Generate embedding vector for text. @@ -173,7 +173,7 @@ class OpenAIProvider(Provider): return all_embeddings - @retry_on_rate_limit + @_retry_429 async def _embed_batch_request(self, batch: list[str]) -> list[list[float]]: """Make a single batch embedding request with retry logic.""" assert self.embedding_model is not None # Type narrowing @@ -208,7 +208,7 @@ class OpenAIProvider(Provider): ) return self._dimension - @retry_on_rate_limit + @_retry_429 async def generate(self, prompt: str, max_tokens: int = 500) -> str: """ Generate text from a prompt. diff --git a/tests/unit/providers/test_mistral.py b/tests/unit/providers/test_mistral.py index 29f4392e..d3c58c15 100644 --- a/tests/unit/providers/test_mistral.py +++ b/tests/unit/providers/test_mistral.py @@ -3,11 +3,13 @@ from unittest.mock import AsyncMock, MagicMock import pytest +from mistralai.client.errors import SDKError from nextcloud_mcp_server.providers.mistral import ( BATCH_SIZE, MISTRAL_EMBEDDING_DIMENSIONS, MistralProvider, + _is_rate_limit, ) @@ -196,3 +198,17 @@ async def test_mistral_base_url_passed_to_sdk(mocker): api_key="test-key", server_url="https://example.com/mistral", ) + + +@pytest.mark.unit +def test_mistral_is_rate_limit_predicate(): + """_is_rate_limit returns True only for SDKErrors with status_code == 429.""" + err_429 = MagicMock(spec=SDKError) + err_429.status_code = 429 + err_500 = MagicMock(spec=SDKError) + err_500.status_code = 500 + + assert _is_rate_limit(err_429) is True + assert _is_rate_limit(err_500) is False + # ValueError has no status_code attr → getattr returns None → False. + assert _is_rate_limit(ValueError()) is False diff --git a/tests/unit/providers/test_registry.py b/tests/unit/providers/test_registry.py index dc871834..b9be98f8 100644 --- a/tests/unit/providers/test_registry.py +++ b/tests/unit/providers/test_registry.py @@ -69,8 +69,11 @@ def test_registry_picks_simple_with_custom_dimension(clean_provider_env): @pytest.mark.unit -def test_registry_picks_mistral_when_api_key_set(clean_provider_env): +def test_registry_picks_mistral_when_api_key_set(clean_provider_env, mocker): """MISTRAL_API_KEY alone is enough to select MistralProvider.""" + # MistralProvider eagerly constructs the SDK client in __init__; stub it + # so the test doesn't depend on the SDK accepting arbitrary keys. + mocker.patch("nextcloud_mcp_server.providers.mistral.Mistral") clean_provider_env.setenv("MISTRAL_API_KEY", "test-key") _reload_config()