Merge PR #1976 provider response guards

src/llm/providers/astraflow.py

@@ -15,6 +15,7 @@ from llm.core.interface import (
     RateLimitError,
 )
 from llm.core.types import LLMInput, LLMOutput, ModelInfo, ProviderType, ToolCall
+from llm.providers.constants import EMPTY_FILTERED_RESPONSE_ERROR
 
 ASTRAFLOW_BASE_URL = "https://api.umodelverse.ai/v1"
 ASTRAFLOW_CN_BASE_URL = "https://api.modelverse.cn/v1"
@@ -55,7 +56,7 @@ class _AstraflowBaseProvider(LLMProvider):
         env_model = os.environ.get(self.model_env)
         fallback_model = os.environ.get(self.fallback_model_env) if self.fallback_model_env else None
         self.default_model = default_model or env_model or fallback_model or DEFAULT_ASTRAFLOW_MODEL
-        self.client = OpenAI(api_key=self.api_key, base_url=self.base_url)
+        self.client = OpenAI(api_key=self.api_key, base_url=self.base_url, _enforce_credentials=False)
         self._models = [
             ModelInfo(
                 name=self.default_model,
@@ -79,6 +80,8 @@ class _AstraflowBaseProvider(LLMProvider):
                 params["tools"] = [tool.to_openai_tool() for tool in llm_input.tools]
 
             response = self.client.chat.completions.create(**params)
+            if not response.choices or response.choices[0].message is None:
+                raise ValueError(EMPTY_FILTERED_RESPONSE_ERROR)
             choice = response.choices[0]
 
             tool_calls = None

src/llm/providers/constants.py

@@ -0,0 +1,3 @@
+"""Shared provider constants."""
+
+EMPTY_FILTERED_RESPONSE_ERROR = "LLM returned empty or filtered response"

src/llm/providers/openai.py

@@ -1,114 +1,125 @@
-"""OpenAI provider adapter."""
+"""OpenAI provider adapter."""
-
+
-from __future__ import annotations
+from __future__ import annotations
-
+
-import json
+import json
-import os
+import os
-from typing import Any
+from typing import Any
-
+
-from openai import OpenAI
+from openai import OpenAI
-
+
-from llm.core.interface import (
+from llm.core.interface import (
-    AuthenticationError,
+    AuthenticationError,
-    ContextLengthError,
+    ContextLengthError,
-    LLMProvider,
+    LLMProvider,
-    RateLimitError,
+    RateLimitError,
-)
+)
-from llm.core.types import LLMInput, LLMOutput, Message, ModelInfo, ProviderType, ToolCall
+from llm.core.types import LLMInput, LLMOutput, Message, ModelInfo, ProviderType, ToolCall
-
+from llm.providers.constants import EMPTY_FILTERED_RESPONSE_ERROR
-
+
-class OpenAIProvider(LLMProvider):
+
-    provider_type = ProviderType.OPENAI
+class OpenAIProvider(LLMProvider):
-
+    provider_type = ProviderType.OPENAI
-    def __init__(self, api_key: str | None = None, base_url: str | None = None) -> None:
+
-        self.client = OpenAI(api_key=api_key or os.environ.get("OPENAI_API_KEY"), base_url=base_url)
+    def __init__(self, api_key: str | None = None, base_url: str | None = None) -> None:
-        self._models = [
+        self.client = OpenAI(
-            ModelInfo(
+            api_key=api_key or os.environ.get("OPENAI_API_KEY"),
-                name="gpt-4o",
+            base_url=base_url,
-                provider=ProviderType.OPENAI,
+            _enforce_credentials=False,
-                supports_tools=True,
+        )
-                supports_vision=True,
+        self._models = [
-                max_tokens=4096,
+            ModelInfo(
-                context_window=128000,
+                name="gpt-4o",
-            ),
+                provider=ProviderType.OPENAI,
-            ModelInfo(
+                supports_tools=True,
-                name="gpt-4o-mini",
+                supports_vision=True,
-                provider=ProviderType.OPENAI,
+                max_tokens=4096,
-                supports_tools=True,
+                context_window=128000,
-                supports_vision=True,
+            ),
-                max_tokens=4096,
+            ModelInfo(
-                context_window=128000,
+                name="gpt-4o-mini",
-            ),
+                provider=ProviderType.OPENAI,
-            ModelInfo(
+                supports_tools=True,
-                name="gpt-4-turbo",
+                supports_vision=True,
-                provider=ProviderType.OPENAI,
+                max_tokens=4096,
-                supports_tools=True,
+                context_window=128000,
-                supports_vision=True,
+            ),
-                max_tokens=4096,
+            ModelInfo(
-                context_window=128000,
+                name="gpt-4-turbo",
-            ),
+                provider=ProviderType.OPENAI,
-            ModelInfo(
+                supports_tools=True,
-                name="gpt-3.5-turbo",
+                supports_vision=True,
-                provider=ProviderType.OPENAI,
+                max_tokens=4096,
-                supports_tools=True,
+                context_window=128000,
-                supports_vision=False,
+            ),
-                max_tokens=4096,
+            ModelInfo(
-                context_window=16385,
+                name="gpt-3.5-turbo",
-            ),
+                provider=ProviderType.OPENAI,
-        ]
+                supports_tools=True,
-
+                supports_vision=False,
-    def generate(self, input: LLMInput) -> LLMOutput:
+                max_tokens=4096,
-        try:
+                context_window=16385,
-            params: dict[str, Any] = {
+            ),
-                "model": input.model or "gpt-4o-mini",
+        ]
-                "messages": [msg.to_dict() for msg in input.messages],
+
-                "temperature": input.temperature,
+    def generate(self, input: LLMInput) -> LLMOutput:
-            }
+        try:
-            if input.max_tokens:
+            params: dict[str, Any] = {
-                params["max_tokens"] = input.max_tokens
+                "model": input.model or "gpt-4o-mini",
-            if input.tools:
+                "messages": [msg.to_dict() for msg in input.messages],
+                "temperature": input.temperature,
+            }
+            if input.max_tokens:
+                params["max_tokens"] = input.max_tokens
+            if input.tools:
                 params["tools"] = [tool.to_openai_tool() for tool in input.tools]
-
+
-            response = self.client.chat.completions.create(**params)
+            response = self.client.chat.completions.create(**params)
-            choice = response.choices[0]
+            if not response.choices or response.choices[0].message is None:
-
+                raise ValueError(EMPTY_FILTERED_RESPONSE_ERROR)
-            tool_calls = None
+            choice = response.choices[0]
-            if choice.message.tool_calls:
+
-                tool_calls = [
+            tool_calls = None
-                    ToolCall(
+            if choice.message.tool_calls:
-                        id=tc.id or "",
+                tool_calls = [
-                        name=tc.function.name,
+                    ToolCall(
-                        arguments={} if not tc.function.arguments else json.loads(tc.function.arguments),
+                        id=tc.id or "",
-                    )
+                        name=tc.function.name,
-                    for tc in choice.message.tool_calls
+                        arguments={} if not tc.function.arguments else json.loads(tc.function.arguments),
-                ]
+                    )
-
+                    for tc in choice.message.tool_calls
-            return LLMOutput(
+                ]
-                content=choice.message.content or "",
+
-                tool_calls=tool_calls,
+            usage = None
-                model=response.model,
+            if response.usage:
-                usage={
+                usage = {
-                    "prompt_tokens": response.usage.prompt_tokens,
+                    "prompt_tokens": response.usage.prompt_tokens,
-                    "completion_tokens": response.usage.completion_tokens,
+                    "completion_tokens": response.usage.completion_tokens,
-                    "total_tokens": response.usage.total_tokens,
+                    "total_tokens": response.usage.total_tokens,
-                },
+                }
-                stop_reason=choice.finish_reason,
+
-            )
+            return LLMOutput(
-        except Exception as e:
+                content=choice.message.content or "",
-            msg = str(e)
+                tool_calls=tool_calls,
-            if "401" in msg or "authentication" in msg.lower():
+                model=response.model,
-                raise AuthenticationError(msg, provider=ProviderType.OPENAI) from e
+                usage=usage,
-            if "429" in msg or "rate_limit" in msg.lower():
+                stop_reason=choice.finish_reason,
-                raise RateLimitError(msg, provider=ProviderType.OPENAI) from e
+            )
-            if "context" in msg.lower() and "length" in msg.lower():
+        except Exception as e:
-                raise ContextLengthError(msg, provider=ProviderType.OPENAI) from e
+            msg = str(e)
-            raise
+            if "401" in msg or "authentication" in msg.lower():
-
+                raise AuthenticationError(msg, provider=ProviderType.OPENAI) from e
-    def list_models(self) -> list[ModelInfo]:
+            if "429" in msg or "rate_limit" in msg.lower():
-        return self._models.copy()
+                raise RateLimitError(msg, provider=ProviderType.OPENAI) from e
-
+            if "context" in msg.lower() and "length" in msg.lower():
-    def validate_config(self) -> bool:
+                raise ContextLengthError(msg, provider=ProviderType.OPENAI) from e
-        return bool(self.client.api_key)
+            raise
-
+
-    def get_default_model(self) -> str:
+    def list_models(self) -> list[ModelInfo]:
-        return "gpt-4o-mini"
+        return self._models.copy()
+
+    def validate_config(self) -> bool:
+        return bool(self.client.api_key)
+
+    def get_default_model(self) -> str:
+        return "gpt-4o-mini"

tests/test_provider_tools.py

@@ -1,7 +1,10 @@
 from types import SimpleNamespace
 
+import pytest
+
 from llm.core.types import LLMInput, Message, Role, ToolDefinition
 from llm.providers.claude import ClaudeProvider
+from llm.providers.constants import EMPTY_FILTERED_RESPONSE_ERROR
 from llm.providers.openai import OpenAIProvider
 
 
@@ -14,21 +17,20 @@ def _tool() -> ToolDefinition:
 
 
 class _OpenAICompletions:
-    def __init__(self) -> None:
+    def __init__(self, response: SimpleNamespace | None = None) -> None:
         self.params = None
+        self.response = response
 
     def create(self, **params):
         self.params = params
-        return SimpleNamespace(
+        if self.response:
-            choices=[SimpleNamespace(message=SimpleNamespace(content="ok", tool_calls=None), finish_reason="stop")],
+            return self.response
-            model=params["model"],
+        return _openai_response(model=params["model"])
-            usage=SimpleNamespace(prompt_tokens=1, completion_tokens=1, total_tokens=2),
-        )
 
 
 class _OpenAIClient:
-    def __init__(self) -> None:
+    def __init__(self, response: SimpleNamespace | None = None) -> None:
-        self.completions = _OpenAICompletions()
+        self.completions = _OpenAICompletions(response=response)
         self.chat = SimpleNamespace(completions=self.completions)
 
 
@@ -52,6 +54,16 @@ class _AnthropicClient:
         self.api_key = "test"
 
 
+def _openai_response(**overrides) -> SimpleNamespace:
+    defaults = {
+        "choices": [SimpleNamespace(message=SimpleNamespace(content="ok", tool_calls=None), finish_reason="stop")],
+        "model": "gpt-4o-mini",
+        "usage": SimpleNamespace(prompt_tokens=1, completion_tokens=1, total_tokens=2),
+    }
+    defaults.update(overrides)
+    return SimpleNamespace(**defaults)
+
+
 def test_openai_provider_serializes_tools_for_chat_completions():
     provider = OpenAIProvider(api_key="test")
     client = _OpenAIClient()
@@ -72,6 +84,36 @@ def test_openai_provider_serializes_tools_for_chat_completions():
     ]
 
 
+def test_openai_provider_can_be_constructed_without_credentials(monkeypatch):
+    monkeypatch.delenv("OPENAI_API_KEY", raising=False)
+
+    provider = OpenAIProvider()
+
+    assert provider.validate_config() is False
+
+
+def test_openai_provider_rejects_empty_or_filtered_responses():
+    provider = OpenAIProvider(api_key="test")
+
+    for response in [
+        _openai_response(choices=[]),
+        _openai_response(choices=[SimpleNamespace(message=None, finish_reason="content_filter")]),
+    ]:
+        provider.client = _OpenAIClient(response=response)
+        with pytest.raises(ValueError, match=EMPTY_FILTERED_RESPONSE_ERROR):
+            provider.generate(LLMInput(messages=[Message(role=Role.USER, content="hi")]))
+
+
+def test_openai_provider_allows_missing_usage():
+    provider = OpenAIProvider(api_key="test")
+    provider.client = _OpenAIClient(response=_openai_response(usage=None))
+
+    output = provider.generate(LLMInput(messages=[Message(role=Role.USER, content="hi")]))
+
+    assert output.content == "ok"
+    assert output.usage is None
+
+
 def test_claude_provider_serializes_tools_for_messages_api():
     provider = ClaudeProvider(api_key="test")
     client = _AnthropicClient()