diff --git a/backend/app/utils/openai_chat_compat.py b/backend/app/utils/openai_chat_compat.py index 44ee941a..f0ce9374 100644 --- a/backend/app/utils/openai_chat_compat.py +++ b/backend/app/utils/openai_chat_compat.py @@ -10,17 +10,6 @@ from __future__ import annotations 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: """Return True when model belongs to GPT-5 family aliases/snapshots.""" if not model: @@ -28,18 +17,6 @@ def is_gpt5_family(model: Optional[str]) -> bool: 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( client: Any, *, @@ -48,16 +25,15 @@ def create_chat_completion( temperature: Optional[float] = None, max_tokens: Optional[int] = None, response_format: Optional[Dict[str, Any]] = None, - extra_params: Optional[Dict[str, Any]] = None, - max_attempts: int = 4, ) -> Any: """ - Create a chat completion with adaptive parameter fallback. + Create a chat completion with model-specific request parameters. Compatibility strategy: - For GPT-5 family, avoid sending temperature by default. - - For token limit, try `max_completion_tokens` on GPT-5, `max_tokens` otherwise. - - On parameter-support errors, adapt and retry without changing caller behavior. + - For token limit, use `max_completion_tokens` on GPT-5, `max_tokens` otherwise. + - 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] = { "model": model, @@ -67,69 +43,18 @@ def create_chat_completion( if response_format is not None: kwargs["response_format"] = response_format - # GPT-5 family rejects temperature unless reasoning effort is explicitly `none`. - if temperature is not None and not is_gpt5_family(model): + gpt5_family = is_gpt5_family(model) + + if temperature is not None and not gpt5_family: kwargs["temperature"] = temperature if max_tokens is not None: - if is_gpt5_family(model): + if gpt5_family: kwargs["max_completion_tokens"] = max_tokens else: kwargs["max_tokens"] = max_tokens - if extra_params: - 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.") + return client.chat.completions.create(**kwargs) def extract_chat_completion_text(response: Any) -> str: diff --git a/backend/tests/test_openai_chat_compat.py b/backend/tests/test_openai_chat_compat.py new file mode 100644 index 00000000..fe08a62a --- /dev/null +++ b/backend/tests/test_openai_chat_compat.py @@ -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=[])) == "" diff --git a/frontend/package-lock.json b/frontend/package-lock.json index fdab7ac4..3e56d752 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -1435,6 +1435,7 @@ "resolved": "https://registry.npmjs.org/d3-selection/-/d3-selection-3.0.0.tgz", "integrity": "sha512-fmTRWbNMmsmWq6xJV8D19U/gw/bwrHfNXxrIN+HfZgnzqTHp9jOmKMhsTUjXOJnZOdZY9Q28y4yebKzqDKlxlQ==", "license": "ISC", + "peer": true, "engines": { "node": ">=12" } @@ -1912,6 +1913,7 @@ "integrity": "sha512-QP88BAKvMam/3NxH6vj2o21R6MjxZUAd6nlwAS/pnGvN9IVLocLHxGYIzFhg6fUQ+5th6P4dv4eW9jX3DSIj7A==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -2051,6 +2053,7 @@ "integrity": "sha512-ITcnkFeR3+fI8P1wMgItjGrR10170d8auB4EpMLPqmx6uxElH3a/hHGQabSHKdqd4FXWO1nFIp9rRn7JQ34ACQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.5.0", @@ -2125,6 +2128,7 @@ "resolved": "https://registry.npmjs.org/vue/-/vue-3.5.25.tgz", "integrity": "sha512-YLVdgv2K13WJ6n+kD5owehKtEXwdwXuj2TTyJMsO7pSeKw2bfRNZGjhB7YzrpbMYj5b5QsUebHpOqR3R3ziy/g==", "license": "MIT", + "peer": true, "dependencies": { "@vue/compiler-dom": "3.5.25", "@vue/compiler-sfc": "3.5.25",