From 9397f2016e7e12dea6fd8d23714807ac0f29bee3 Mon Sep 17 00:00:00 2001 From: xh123637 <3324733895@qq.com> Date: Mon, 2 Nov 2026 19:06:33 +0800 Subject: [PATCH] fix: send max_completion_tokens to GPT-5 and o-series models 1. Rename max_tokens to max_completion_tokens in OpenAICompatibleProvider for GPT-5 family and o-series models, which reject max_tokens on Chat Completions 2. Drop a non-default temperature for the o-series and the original gpt-5/gpt-5-mini/gpt-5-nano, which only accept the default temperature 3. Apply the normalization in both chat() and chat_stream() and add request-shape regression tests --- .../infra/provider/openai_compatible.py | 61 ++++++++ tests/unit/provider/test_openai_compatible.py | 147 +++++++++++++++++- 2 files changed, 207 insertions(+), 1 deletion(-) diff --git a/src/llm4ad/infra/provider/openai_compatible.py b/src/llm4ad/infra/provider/openai_compatible.py index 4ce3151..822f36c 100644 --- a/src/llm4ad/infra/provider/openai_compatible.py +++ b/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] = { diff --git a/tests/unit/provider/test_openai_compatible.py b/tests/unit/provider/test_openai_compatible.py index 9516bf0..07d9e30 100644 --- a/tests/unit/provider/test_openai_compatible.py +++ b/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