fix: keep GPT-5 compatibility handling narrow
This commit is contained in:
parent
f7138e09fb
commit
e67a52e80f
|
|
@ -10,17 +10,6 @@ from __future__ import annotations
|
||||||
from typing import Any, Dict, List, Optional
|
from typing import Any, Dict, List, Optional
|
||||||
|
|
||||||
|
|
||||||
UNSUPPORTED_PARAM_HINTS = (
|
|
||||||
"unsupported",
|
|
||||||
"not supported",
|
|
||||||
"does not support",
|
|
||||||
"unknown parameter",
|
|
||||||
"unexpected keyword",
|
|
||||||
"extra fields",
|
|
||||||
"only supported",
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
def is_gpt5_family(model: Optional[str]) -> bool:
|
def is_gpt5_family(model: Optional[str]) -> bool:
|
||||||
"""Return True when model belongs to GPT-5 family aliases/snapshots."""
|
"""Return True when model belongs to GPT-5 family aliases/snapshots."""
|
||||||
if not model:
|
if not model:
|
||||||
|
|
@ -28,18 +17,6 @@ def is_gpt5_family(model: Optional[str]) -> bool:
|
||||||
return model.strip().lower().startswith("gpt-5")
|
return model.strip().lower().startswith("gpt-5")
|
||||||
|
|
||||||
|
|
||||||
def _is_unsupported_param_error(message: str, param_name: str) -> bool:
|
|
||||||
msg = (message or "").lower()
|
|
||||||
if param_name.lower() not in msg:
|
|
||||||
return False
|
|
||||||
return any(hint in msg for hint in UNSUPPORTED_PARAM_HINTS)
|
|
||||||
|
|
||||||
|
|
||||||
def _extract_error_message(error: Exception) -> str:
|
|
||||||
# openai.BadRequestError string usually includes the API message; keep it generic
|
|
||||||
return str(error)
|
|
||||||
|
|
||||||
|
|
||||||
def create_chat_completion(
|
def create_chat_completion(
|
||||||
client: Any,
|
client: Any,
|
||||||
*,
|
*,
|
||||||
|
|
@ -48,16 +25,15 @@ def create_chat_completion(
|
||||||
temperature: Optional[float] = None,
|
temperature: Optional[float] = None,
|
||||||
max_tokens: Optional[int] = None,
|
max_tokens: Optional[int] = None,
|
||||||
response_format: Optional[Dict[str, Any]] = None,
|
response_format: Optional[Dict[str, Any]] = None,
|
||||||
extra_params: Optional[Dict[str, Any]] = None,
|
|
||||||
max_attempts: int = 4,
|
|
||||||
) -> Any:
|
) -> Any:
|
||||||
"""
|
"""
|
||||||
Create a chat completion with adaptive parameter fallback.
|
Create a chat completion with model-specific request parameters.
|
||||||
|
|
||||||
Compatibility strategy:
|
Compatibility strategy:
|
||||||
- For GPT-5 family, avoid sending temperature by default.
|
- For GPT-5 family, avoid sending temperature by default.
|
||||||
- For token limit, try `max_completion_tokens` on GPT-5, `max_tokens` otherwise.
|
- For token limit, use `max_completion_tokens` on GPT-5, `max_tokens` otherwise.
|
||||||
- On parameter-support errors, adapt and retry without changing caller behavior.
|
- Preserve the legacy request shape for every non-GPT-5 model/provider.
|
||||||
|
- Propagate provider errors unchanged instead of guessing from message text.
|
||||||
"""
|
"""
|
||||||
kwargs: Dict[str, Any] = {
|
kwargs: Dict[str, Any] = {
|
||||||
"model": model,
|
"model": model,
|
||||||
|
|
@ -67,69 +43,18 @@ def create_chat_completion(
|
||||||
if response_format is not None:
|
if response_format is not None:
|
||||||
kwargs["response_format"] = response_format
|
kwargs["response_format"] = response_format
|
||||||
|
|
||||||
# GPT-5 family rejects temperature unless reasoning effort is explicitly `none`.
|
gpt5_family = is_gpt5_family(model)
|
||||||
if temperature is not None and not is_gpt5_family(model):
|
|
||||||
|
if temperature is not None and not gpt5_family:
|
||||||
kwargs["temperature"] = temperature
|
kwargs["temperature"] = temperature
|
||||||
|
|
||||||
if max_tokens is not None:
|
if max_tokens is not None:
|
||||||
if is_gpt5_family(model):
|
if gpt5_family:
|
||||||
kwargs["max_completion_tokens"] = max_tokens
|
kwargs["max_completion_tokens"] = max_tokens
|
||||||
else:
|
else:
|
||||||
kwargs["max_tokens"] = max_tokens
|
kwargs["max_tokens"] = max_tokens
|
||||||
|
|
||||||
if extra_params:
|
return client.chat.completions.create(**kwargs)
|
||||||
kwargs.update(extra_params)
|
|
||||||
|
|
||||||
attempted_signatures = set()
|
|
||||||
unsupported_params = set()
|
|
||||||
last_error: Optional[Exception] = None
|
|
||||||
|
|
||||||
for _ in range(max_attempts):
|
|
||||||
signature = tuple(sorted(kwargs.keys()))
|
|
||||||
if signature in attempted_signatures:
|
|
||||||
break
|
|
||||||
attempted_signatures.add(signature)
|
|
||||||
|
|
||||||
try:
|
|
||||||
return client.chat.completions.create(**kwargs)
|
|
||||||
except Exception as error:
|
|
||||||
last_error = error
|
|
||||||
error_msg = _extract_error_message(error)
|
|
||||||
changed = False
|
|
||||||
|
|
||||||
if _is_unsupported_param_error(error_msg, "temperature") and "temperature" in kwargs:
|
|
||||||
kwargs.pop("temperature", None)
|
|
||||||
unsupported_params.add("temperature")
|
|
||||||
changed = True
|
|
||||||
|
|
||||||
if _is_unsupported_param_error(error_msg, "response_format") and "response_format" in kwargs:
|
|
||||||
kwargs.pop("response_format", None)
|
|
||||||
unsupported_params.add("response_format")
|
|
||||||
changed = True
|
|
||||||
|
|
||||||
if _is_unsupported_param_error(error_msg, "max_tokens") and "max_tokens" in kwargs:
|
|
||||||
token_value = kwargs.pop("max_tokens")
|
|
||||||
unsupported_params.add("max_tokens")
|
|
||||||
if "max_completion_tokens" not in unsupported_params:
|
|
||||||
kwargs["max_completion_tokens"] = token_value
|
|
||||||
changed = True
|
|
||||||
|
|
||||||
if (
|
|
||||||
_is_unsupported_param_error(error_msg, "max_completion_tokens")
|
|
||||||
and "max_completion_tokens" in kwargs
|
|
||||||
):
|
|
||||||
token_value = kwargs.pop("max_completion_tokens")
|
|
||||||
unsupported_params.add("max_completion_tokens")
|
|
||||||
if "max_tokens" not in unsupported_params:
|
|
||||||
kwargs["max_tokens"] = token_value
|
|
||||||
changed = True
|
|
||||||
|
|
||||||
if not changed:
|
|
||||||
raise
|
|
||||||
|
|
||||||
if last_error:
|
|
||||||
raise last_error
|
|
||||||
raise RuntimeError("Chat completion failed with unknown error.")
|
|
||||||
|
|
||||||
|
|
||||||
def extract_chat_completion_text(response: Any) -> str:
|
def extract_chat_completion_text(response: Any) -> str:
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,123 @@
|
||||||
|
from types import SimpleNamespace
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from app.utils.openai_chat_compat import (
|
||||||
|
create_chat_completion,
|
||||||
|
extract_chat_completion_text,
|
||||||
|
is_gpt5_family,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class CompletionRecorder:
|
||||||
|
def __init__(self, result=None, error=None):
|
||||||
|
self.result = result or object()
|
||||||
|
self.error = error
|
||||||
|
self.calls = []
|
||||||
|
|
||||||
|
def create(self, **kwargs):
|
||||||
|
self.calls.append(kwargs)
|
||||||
|
if self.error is not None:
|
||||||
|
raise self.error
|
||||||
|
return self.result
|
||||||
|
|
||||||
|
|
||||||
|
def client_for(recorder):
|
||||||
|
return SimpleNamespace(chat=SimpleNamespace(completions=recorder))
|
||||||
|
|
||||||
|
|
||||||
|
def test_gpt5_uses_completion_token_limit_without_temperature():
|
||||||
|
recorder = CompletionRecorder()
|
||||||
|
messages = [{"role": "user", "content": "hello"}]
|
||||||
|
|
||||||
|
result = create_chat_completion(
|
||||||
|
client_for(recorder),
|
||||||
|
model="gpt-5-2025-08-07",
|
||||||
|
messages=messages,
|
||||||
|
temperature=0.2,
|
||||||
|
max_tokens=123,
|
||||||
|
response_format={"type": "json_object"},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result is recorder.result
|
||||||
|
assert recorder.calls == [
|
||||||
|
{
|
||||||
|
"model": "gpt-5-2025-08-07",
|
||||||
|
"messages": messages,
|
||||||
|
"max_completion_tokens": 123,
|
||||||
|
"response_format": {"type": "json_object"},
|
||||||
|
}
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def test_legacy_model_preserves_original_request_shape():
|
||||||
|
recorder = CompletionRecorder()
|
||||||
|
messages = [{"role": "user", "content": "hello"}]
|
||||||
|
|
||||||
|
create_chat_completion(
|
||||||
|
client_for(recorder),
|
||||||
|
model="third-party-chat-model",
|
||||||
|
messages=messages,
|
||||||
|
temperature=0.7,
|
||||||
|
max_tokens=456,
|
||||||
|
response_format={"type": "json_object"},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert recorder.calls == [
|
||||||
|
{
|
||||||
|
"model": "third-party-chat-model",
|
||||||
|
"messages": messages,
|
||||||
|
"temperature": 0.7,
|
||||||
|
"max_tokens": 456,
|
||||||
|
"response_format": {"type": "json_object"},
|
||||||
|
}
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def test_provider_error_is_propagated_without_guessing_or_retrying():
|
||||||
|
provider_error = RuntimeError("unsupported max_tokens due to a server outage")
|
||||||
|
recorder = CompletionRecorder(error=provider_error)
|
||||||
|
|
||||||
|
with pytest.raises(RuntimeError) as captured:
|
||||||
|
create_chat_completion(
|
||||||
|
client_for(recorder),
|
||||||
|
model="legacy-model",
|
||||||
|
messages=[],
|
||||||
|
max_tokens=10,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert captured.value is provider_error
|
||||||
|
assert len(recorder.calls) == 1
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("model", "expected"),
|
||||||
|
[
|
||||||
|
("gpt-5", True),
|
||||||
|
(" GPT-5.1-mini ", True),
|
||||||
|
("gpt-4.1", False),
|
||||||
|
("my-gpt-5-proxy", False),
|
||||||
|
(None, False),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_gpt5_family_detection(model, expected):
|
||||||
|
assert is_gpt5_family(model) is expected
|
||||||
|
|
||||||
|
|
||||||
|
def test_extracts_text_from_supported_content_shapes():
|
||||||
|
response = SimpleNamespace(
|
||||||
|
choices=[
|
||||||
|
SimpleNamespace(
|
||||||
|
message=SimpleNamespace(
|
||||||
|
content=[
|
||||||
|
{"text": {"value": "first"}},
|
||||||
|
{"content": " second"},
|
||||||
|
SimpleNamespace(text=" third"),
|
||||||
|
]
|
||||||
|
)
|
||||||
|
)
|
||||||
|
]
|
||||||
|
)
|
||||||
|
|
||||||
|
assert extract_chat_completion_text(response) == "first second third"
|
||||||
|
assert extract_chat_completion_text(SimpleNamespace(choices=[])) == ""
|
||||||
|
|
@ -1435,6 +1435,7 @@
|
||||||
"resolved": "https://registry.npmjs.org/d3-selection/-/d3-selection-3.0.0.tgz",
|
"resolved": "https://registry.npmjs.org/d3-selection/-/d3-selection-3.0.0.tgz",
|
||||||
"integrity": "sha512-fmTRWbNMmsmWq6xJV8D19U/gw/bwrHfNXxrIN+HfZgnzqTHp9jOmKMhsTUjXOJnZOdZY9Q28y4yebKzqDKlxlQ==",
|
"integrity": "sha512-fmTRWbNMmsmWq6xJV8D19U/gw/bwrHfNXxrIN+HfZgnzqTHp9jOmKMhsTUjXOJnZOdZY9Q28y4yebKzqDKlxlQ==",
|
||||||
"license": "ISC",
|
"license": "ISC",
|
||||||
|
"peer": true,
|
||||||
"engines": {
|
"engines": {
|
||||||
"node": ">=12"
|
"node": ">=12"
|
||||||
}
|
}
|
||||||
|
|
@ -1912,6 +1913,7 @@
|
||||||
"integrity": "sha512-QP88BAKvMam/3NxH6vj2o21R6MjxZUAd6nlwAS/pnGvN9IVLocLHxGYIzFhg6fUQ+5th6P4dv4eW9jX3DSIj7A==",
|
"integrity": "sha512-QP88BAKvMam/3NxH6vj2o21R6MjxZUAd6nlwAS/pnGvN9IVLocLHxGYIzFhg6fUQ+5th6P4dv4eW9jX3DSIj7A==",
|
||||||
"dev": true,
|
"dev": true,
|
||||||
"license": "MIT",
|
"license": "MIT",
|
||||||
|
"peer": true,
|
||||||
"engines": {
|
"engines": {
|
||||||
"node": ">=12"
|
"node": ">=12"
|
||||||
},
|
},
|
||||||
|
|
@ -2051,6 +2053,7 @@
|
||||||
"integrity": "sha512-ITcnkFeR3+fI8P1wMgItjGrR10170d8auB4EpMLPqmx6uxElH3a/hHGQabSHKdqd4FXWO1nFIp9rRn7JQ34ACQ==",
|
"integrity": "sha512-ITcnkFeR3+fI8P1wMgItjGrR10170d8auB4EpMLPqmx6uxElH3a/hHGQabSHKdqd4FXWO1nFIp9rRn7JQ34ACQ==",
|
||||||
"dev": true,
|
"dev": true,
|
||||||
"license": "MIT",
|
"license": "MIT",
|
||||||
|
"peer": true,
|
||||||
"dependencies": {
|
"dependencies": {
|
||||||
"esbuild": "^0.25.0",
|
"esbuild": "^0.25.0",
|
||||||
"fdir": "^6.5.0",
|
"fdir": "^6.5.0",
|
||||||
|
|
@ -2125,6 +2128,7 @@
|
||||||
"resolved": "https://registry.npmjs.org/vue/-/vue-3.5.25.tgz",
|
"resolved": "https://registry.npmjs.org/vue/-/vue-3.5.25.tgz",
|
||||||
"integrity": "sha512-YLVdgv2K13WJ6n+kD5owehKtEXwdwXuj2TTyJMsO7pSeKw2bfRNZGjhB7YzrpbMYj5b5QsUebHpOqR3R3ziy/g==",
|
"integrity": "sha512-YLVdgv2K13WJ6n+kD5owehKtEXwdwXuj2TTyJMsO7pSeKw2bfRNZGjhB7YzrpbMYj5b5QsUebHpOqR3R3ziy/g==",
|
||||||
"license": "MIT",
|
"license": "MIT",
|
||||||
|
"peer": true,
|
||||||
"dependencies": {
|
"dependencies": {
|
||||||
"@vue/compiler-dom": "3.5.25",
|
"@vue/compiler-dom": "3.5.25",
|
||||||
"@vue/compiler-sfc": "3.5.25",
|
"@vue/compiler-sfc": "3.5.25",
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue