fix(models): harden LiteLLM request handling (#197)

* fix(models): harden LiteLLM request handling

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(models): preserve legacy function call messages

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: XiaoBoAI

openjudge/models/litellm_chat_model.py

@@ -99,11 +99,7 @@ async def achat(
         """
         import litellm
 
-        if not isinstance(messages, list):
-            raise ValueError(
-                f"LiteLLM `messages` field expected type `list`, got `{type(messages)}` instead.",
-            )
-        messages = [msg.to_dict() if isinstance(msg, ChatMessage) else msg for msg in messages]
+        messages = self._normalize_and_validate_messages(messages, "LiteLLM")
 
         call_kwargs: Dict[str, Any] = {
             "model": self.model,
@@ -155,6 +151,6 @@ async def achat(
 
         response = await litellm.acompletion(**call_kwargs)
 
-        if self.stream:
+        if call_kwargs["stream"]:
             return self._handle_streaming_response(response, structured_model, callback)
         return self._handle_non_streaming_response(response, structured_model, callback)

openjudge/models/openai_chat_model.py

@@ -115,6 +115,38 @@ def __init__(
 
         self.client = AsyncOpenAI(**client_args)
 
+    @staticmethod
+    def _normalize_and_validate_messages(
+        messages: list[dict | ChatMessage],
+        provider_name: str,
+    ) -> list[dict]:
+        """Convert ``ChatMessage`` objects and validate OpenAI message shape."""
+        if not isinstance(messages, list):
+            raise ValueError(
+                f"{provider_name} `messages` field expected type `list`, " f"got `{type(messages)}` instead.",
+            )
+
+        normalized_messages = [msg.to_dict() if isinstance(msg, ChatMessage) else msg for msg in messages]
+
+        def _is_valid_message(msg: dict) -> bool:
+            if not isinstance(msg, dict) or "role" not in msg:
+                return False
+            role = msg["role"]
+            if role == "assistant" and ("tool_calls" in msg or "function_call" in msg):
+                return True
+            if role == "tool":
+                return "tool_call_id" in msg and "content" in msg
+            return "content" in msg
+
+        if not all(_is_valid_message(msg) for msg in normalized_messages):
+            raise ValueError(
+                "Invalid message format. Each message must have 'role' and appropriate fields. "
+                "User/system messages need 'content'. Tool messages need 'tool_call_id' and 'content'. "
+                "Assistant messages with 'tool_calls' or 'function_call' don't require 'content'.",
+            )
+
+        return normalized_messages
+
     async def achat(
         self,
         messages: list[dict | ChatMessage],
@@ -164,34 +196,7 @@ async def achat(
                 The response from the OpenAI chat completions API.
         """
 
-        # checking messages
-        if not isinstance(messages, list):
-            raise ValueError(
-                "OpenAI `messages` field expected type `list`, " f"got `{type(messages)}` instead.",
-            )
-        messages = [msg.to_dict() if isinstance(msg, ChatMessage) else msg for msg in messages]
-
-        # Validate messages - note that for assistant messages with tool_calls,
-        # content can be None or missing (this is valid OpenAI format)
-        def _is_valid_message(msg: dict) -> bool:
-            if not isinstance(msg, dict) or "role" not in msg:
-                return False
-            role = msg["role"]
-            # Assistant messages with tool_calls don't require content
-            if role == "assistant" and "tool_calls" in msg:
-                return True
-            # Tool messages require tool_call_id and content
-            if role == "tool":
-                return "tool_call_id" in msg and "content" in msg
-            # All other messages require content
-            return "content" in msg
-
-        if not all(_is_valid_message(msg) for msg in messages):
-            raise ValueError(
-                "Invalid message format. Each message must have 'role' and appropriate fields. "
-                "User/system messages need 'content'. Tool messages need 'tool_call_id' and 'content'. "
-                "Assistant messages with 'tool_calls' don't require 'content'.",
-            )
+        messages = self._normalize_and_validate_messages(messages, "OpenAI")
 
         # Qwen-omni requires different base64 audio format from openai
         if "omni" in self.model.lower():

pyproject.toml

@@ -82,7 +82,7 @@ verl = [
     "verl"
 ]
 litellm = [
-    "litellm>=1.89.0,<2.0.0"
+    "litellm>=1.96.2,<2.0.0"
 ]
 
 [project.urls]

tests/models/test_litellm_chat_model.py

@@ -161,3 +161,77 @@ async def test_non_list_messages_raises(self):
         model = LiteLLMChatModel(model="gpt-4o")
         with pytest.raises(ValueError):
             await model.achat(messages="not a list")
+
+    async def test_assistant_function_call_without_content_is_accepted(self):
+        _, calls = _install_litellm_stub()
+        model = LiteLLMChatModel(model="gpt-4o")
+        messages = [
+            {
+                "role": "assistant",
+                "function_call": {
+                    "name": "lookup",
+                    "arguments": "{}",
+                },
+            },
+        ]
+
+        await model.achat(messages=messages)
+
+        assert calls[-1]["messages"] == messages
+
+    @pytest.mark.parametrize(
+        "messages",
+        [
+            [{"content": "missing role"}],
+            [{"role": "user"}],
+            [{"role": "tool", "content": "missing tool call id"}],
+            [42],
+        ],
+        ids=["missing_role", "missing_content", "missing_tool_call_id", "non_dict"],
+    )
+    async def test_invalid_message_format_raises(self, messages):
+        _, calls = _install_litellm_stub()
+        model = LiteLLMChatModel(model="gpt-4o")
+
+        with pytest.raises(ValueError, match="Invalid message format"):
+            await model.achat(messages=messages)
+        assert calls == []
+
+    @pytest.mark.parametrize(
+        "instance_stream, call_stream, expected_handler",
+        [
+            (False, True, "streaming"),
+            (True, False, "non_streaming"),
+        ],
+    )
+    async def test_call_stream_override_selects_matching_response_handler(
+        self,
+        monkeypatch,
+        instance_stream,
+        call_stream,
+        expected_handler,
+    ):
+        _, calls = _install_litellm_stub()
+        model = LiteLLMChatModel(model="gpt-4o", stream=instance_stream)
+        streaming_result = object()
+        non_streaming_result = object()
+
+        monkeypatch.setattr(
+            model,
+            "_handle_streaming_response",
+            lambda *_args: streaming_result,
+        )
+        monkeypatch.setattr(
+            model,
+            "_handle_non_streaming_response",
+            lambda *_args: non_streaming_result,
+        )
+
+        result = await model.achat(
+            messages=[{"role": "user", "content": "hi"}],
+            stream=call_stream,
+        )
+
+        assert calls[-1]["stream"] is call_stream
+        expected_result = streaming_result if expected_handler == "streaming" else non_streaming_result
+        assert result is expected_result