fix: send max_completion_tokens to GPT-5 and o-series models

src/llm4ad/infra/provider/openai_compatible.py

@@ -6,6 +6,7 @@
 
 import asyncio
 import json
+import re
 import time
 from collections.abc import AsyncIterator
 from typing import Any
@@ -79,6 +80,60 @@ def _should_use_json_object_mode(self) -> bool:
             or model.startswith("deepseek")
         )
 
+    def _bare_model_name(self) -> str:
+        """Return the configured model id without any provider path prefix."""
+        return (self.model or "").rsplit("/", 1)[-1].lower()
+
+    def _is_reasoning_model(self) -> bool:
+        """Return whether the model rejects ``max_tokens`` on Chat Completions.
+
+        OpenAI's GPT-5 family and o-series only accept
+        ``max_completion_tokens``; the ``gpt-5-chat`` variants are plain
+        chat models and keep accepting ``max_tokens``.
+        """
+        name = self._bare_model_name()
+        if name.startswith("gpt-5-chat"):
+            return False
+        return name.startswith("gpt-5") or bool(re.match(r"^o\d", name))
+
+    def _rejects_non_default_temperature(self) -> bool:
+        """Return whether the model only accepts the default temperature.
+
+        The o-series and the original GPT-5 generation (``gpt-5``,
+        ``gpt-5-mini``, ``gpt-5-nano``, including dated snapshots) reject
+        any temperature other than 1; later GPT-5.x models accept it.
+        """
+        name = self._bare_model_name()
+        if re.match(r"^o\d", name):
+            return True
+        return bool(re.match(r"^gpt-5(?:-(?:mini|nano))?(?:-\d|$)", name))
+
+    def _normalize_request_kwargs(self, kwargs: dict[str, Any]) -> dict[str, Any]:
+        """Adapt caller kwargs to the parameters the configured model accepts.
+
+        Reasoning models get their token budget under
+        ``max_completion_tokens`` instead of the rejected ``max_tokens``,
+        and models pinned to the default temperature have a non-default
+        ``temperature`` dropped so the request is not rejected on the
+        next parameter. Every other model keeps the caller's kwargs
+        exactly as passed.
+
+        Args:
+            kwargs: Generation parameters supplied by the caller.
+
+        Returns:
+            The kwargs to hand to the OpenAI client.
+        """
+        normalized = dict(kwargs)
+        if self._is_reasoning_model() and "max_tokens" in normalized:
+            budget = normalized.pop("max_tokens")
+            normalized.setdefault("max_completion_tokens", budget)
+        if self._rejects_non_default_temperature():
+            temperature = normalized.get("temperature")
+            if temperature is not None and temperature != 1:
+                normalized.pop("temperature")
+        return normalized
+
     @classmethod
     def _build_schema_instruction(cls, schema_json: dict[str, Any]) -> str:
         """Build instructions that ask for a JSON instance, not a JSON Schema."""
@@ -385,6 +440,9 @@ async def chat(
         # Pop LLM4AD-internal kwargs that must not reach the OpenAI SDK
         request_stage = kwargs.pop("request_stage", "")
 
+        # Rename/drop parameters the configured model would reject
+        kwargs = self._normalize_request_kwargs(kwargs)
+
         # Convert internal ChatMessage format to OpenAI format
         openai_messages = self._convert_messages(messages)
 
@@ -610,6 +668,9 @@ async def chat_stream(
         Returns:
             StreamResponse that yields text chunks and captures tool calls.
         """
+        # Rename/drop parameters the configured model would reject
+        kwargs = self._normalize_request_kwargs(kwargs)
+
         openai_messages = self._convert_messages(messages)
 
         create_kwargs: dict[str, Any] = {

tests/unit/provider/test_openai_compatible.py

@@ -901,7 +901,6 @@ class PersonInfo(BaseModel):
 @pytest.mark.asyncio
 async def test_chat_with_list_schema(provider):
     """Test chat with schema that returns a list."""
-
     class Item(BaseModel):
         id: int
         name: str
@@ -938,3 +937,149 @@ class ItemsList(BaseModel):
     assert len(result.parsed.items) == 2
     assert result.parsed.items[0].name == "A"
     assert result.parsed.items[1].name == "B"
+
+
+def _mock_completion(model: str) -> ChatCompletion:
+    """Build a minimal successful ChatCompletion for request-shape tests."""
+    return ChatCompletion(
+        id="shape-id",
+        created=1234567890,
+        model=model,
+        choices=[
+            Choice(
+                finish_reason="stop",
+                index=0,
+                message=ChatCompletionMessage(role="assistant", content="ok"),
+            )
+        ],
+        usage={"prompt_tokens": 5, "completion_tokens": 3, "total_tokens": 8},
+        object="chat.completion",
+    )
+
+
+def _provider_for_model(provider_config, model: str) -> OpenAICompatibleProvider:
+    """Create a provider for ``model`` with a mocked client."""
+    provider = OpenAICompatibleProvider({**provider_config, "model": model})
+    provider.client = AsyncMock()
+    provider.client.chat.completions.create.return_value = _mock_completion(model)
+    return provider
+
+
+@pytest.mark.asyncio
+async def test_chat_reasoning_model_renames_max_tokens(provider_config):
+    """Reasoning models get max_completion_tokens instead of max_tokens.
+
+    GPT-5.x reasoning models reject max_tokens; the budget must be
+    renamed to max_completion_tokens while temperature stays.
+    """
+    provider = _provider_for_model(provider_config, "gpt-5.4-mini")
+
+    await provider.chat(
+        [ChatMessage(role="user", content="sort numbers")],
+        max_tokens=4096,
+        temperature=0.7,
+    )
+
+    call_kwargs = provider.client.chat.completions.create.call_args.kwargs
+    assert "max_tokens" not in call_kwargs
+    assert call_kwargs["max_completion_tokens"] == 4096
+    assert call_kwargs["temperature"] == 0.7
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("model", ["o3", "o4-mini", "gpt-5", "gpt-5-mini", "gpt-5-nano"])
+async def test_chat_fixed_temperature_models_drop_temperature(provider_config, model):
+    """Fixed-temperature models drop a non-default temperature.
+
+    The o-series and the original GPT-5 generation reject a temperature
+    other than 1, so it must be dropped along with the max_tokens rename.
+    """
+    provider = _provider_for_model(provider_config, model)
+
+    await provider.chat(
+        [ChatMessage(role="user", content="sort numbers")],
+        max_tokens=4096,
+        temperature=0.7,
+    )
+
+    call_kwargs = provider.client.chat.completions.create.call_args.kwargs
+    assert "max_tokens" not in call_kwargs
+    assert call_kwargs["max_completion_tokens"] == 4096
+    assert "temperature" not in call_kwargs
+
+
+@pytest.mark.asyncio
+async def test_chat_fixed_temperature_model_keeps_default_temperature(provider_config):
+    """A temperature of exactly 1 is the models' own default and is kept."""
+    provider = _provider_for_model(provider_config, "gpt-5")
+
+    await provider.chat(
+        [ChatMessage(role="user", content="sort numbers")],
+        max_tokens=100,
+        temperature=1,
+    )
+
+    call_kwargs = provider.client.chat.completions.create.call_args.kwargs
+    assert call_kwargs["temperature"] == 1
+    assert call_kwargs["max_completion_tokens"] == 100
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("model", ["gpt-4o", "gpt-5-chat-latest", "deepseek-chat"])
+async def test_chat_non_reasoning_models_keep_request_shape(provider_config, model):
+    """Non-reasoning models keep the caller's request shape.
+
+    Models outside the reasoning families keep the caller's kwargs
+    exactly: max_tokens and temperature pass through unchanged.
+    """
+    provider = _provider_for_model(provider_config, model)
+
+    await provider.chat(
+        [ChatMessage(role="user", content="sort numbers")],
+        max_tokens=4096,
+        temperature=0.7,
+    )
+
+    call_kwargs = provider.client.chat.completions.create.call_args.kwargs
+    assert call_kwargs["max_tokens"] == 4096
+    assert call_kwargs["temperature"] == 0.7
+    assert "max_completion_tokens" not in call_kwargs
+
+
+@pytest.mark.asyncio
+async def test_chat_stream_reasoning_model_renames_max_tokens(provider_config):
+    """The streaming path applies the same parameter normalization."""
+    provider = _provider_for_model(provider_config, "gpt-5.4-mini")
+    chunks = [
+        ChatCompletionChunk(
+            id="stream-shape",
+            created=1234567890,
+            model="gpt-5.4-mini",
+            choices=[ChunkChoice(delta={"content": "ok"}, index=0, finish_reason=None)],
+            object="chat.completion.chunk",
+        ),
+    ]
+
+    async def mock_stream(*args, **kwargs):
+        for chunk in chunks:
+            yield chunk
+
+    captured = {}
+
+    async def mock_create(*args, **kwargs):
+        captured.update(kwargs)
+        return mock_stream()
+
+    provider.client.chat.completions.create = mock_create
+
+    stream = await provider.chat_stream(
+        [ChatMessage(role="user", content="sort numbers")],
+        max_tokens=4096,
+        temperature=0.7,
+    )
+    async for _ in stream:
+        pass
+
+    assert "max_tokens" not in captured
+    assert captured["max_completion_tokens"] == 4096
+    assert captured["temperature"] == 0.7