mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-14 23:21:35 +00:00
fix: is_model_gpt_5_model substring match incorrectly excludes gpt-5.X-chat models
The check was a substring match that inadvertently excluded versioned chat models like gpt-5.3-chat and gpt-5.1-chat from the GPT-5 routing path, because "gpt-5-chat" is a substring of "gpt-5.3-chat". Those models were routed to the regular Azure chat path which does not suppress parallel_tool_calls, causing Azure to return finish_reason="stop" together with tool_calls, breaking AI-agent tool-use loops. Replace the substring check with an exact membership test against a set containing only the standalone "gpt-5-chat" model, which is the only model that should take the regular chat path. Fixes: gpt-5.1-chat, gpt-5.2-chat, gpt-5.3-chat, gpt-5.4-chat routing regression.
This commit is contained in:
parent
72a461ba4a
commit
d7a8483570
4 changed files with 250 additions and 69 deletions
|
|
@ -40,9 +40,21 @@ class AzureOpenAIGPT5Config(AzureOpenAIConfig, OpenAIGPT5Config):
|
|||
Accepts both explicit gpt-5 model names and the ``gpt5_series/`` prefix
|
||||
used for manual routing.
|
||||
"""
|
||||
# gpt-5-chat* is a chat model and shouldn't go through GPT-5 reasoning restrictions.
|
||||
# The gpt-5-chat* family (gpt-5-chat, gpt-5-chat-latest, gpt-5-chat-2025-08-07,
|
||||
# …) are regular chat models: they support temperature and tool_choice but NOT
|
||||
# reasoning_effort. They must NOT be routed through the GPT-5 reasoning path.
|
||||
#
|
||||
# Versioned chat models such as gpt-5.3-chat and gpt-5.1-chat ARE reasoning
|
||||
# models and must stay on the GPT-5 path. The distinguishing feature is that
|
||||
# the gpt-5-chat family has a literal "-chat" immediately after "gpt-5"
|
||||
# (i.e. "gpt-5-chat…"), while versioned chat models interpose a minor version
|
||||
# number (i.e. "gpt-5.<digit>-chat").
|
||||
#
|
||||
# A bare "gpt-5-chat" prefix test (without the previous substring check) is
|
||||
# safe because no versioned model name starts with "gpt-5-chat".
|
||||
_normalized = model.split("/")[-1] # strip provider prefix, e.g. "azure/"
|
||||
return (
|
||||
"gpt-5" in model and "gpt-5-chat" not in model
|
||||
"gpt-5" in model and not _normalized.startswith("gpt-5-chat")
|
||||
) or "gpt5_series" in model
|
||||
|
||||
def get_supported_openai_params(self, model: str) -> List[str]:
|
||||
|
|
|
|||
|
|
@ -53,9 +53,20 @@ class OpenAIGPT5Config(OpenAIGPTConfig):
|
|||
|
||||
@classmethod
|
||||
def is_model_gpt_5_model(cls, model: str) -> bool:
|
||||
# gpt-5-chat* behaves like a regular chat model (supports temperature, etc.)
|
||||
# Don't route it through GPT-5 reasoning-specific parameter restrictions.
|
||||
return "gpt-5" in model and "gpt-5-chat" not in model
|
||||
# The gpt-5-chat* family (gpt-5-chat, gpt-5-chat-latest, gpt-5-chat-2025-08-07,
|
||||
# …) are regular chat models: they support temperature and tool_choice but NOT
|
||||
# reasoning_effort. They must NOT be routed through the GPT-5 reasoning path.
|
||||
#
|
||||
# Versioned chat models such as gpt-5.3-chat and gpt-5.1-chat ARE reasoning
|
||||
# models and must stay on the GPT-5 path. The distinguishing feature is that
|
||||
# the gpt-5-chat family has a literal "-chat" immediately after "gpt-5"
|
||||
# (i.e. "gpt-5-chat…"), while versioned chat models interpose a minor version
|
||||
# number (i.e. "gpt-5.<digit>-chat").
|
||||
#
|
||||
# A bare "gpt-5-chat" prefix test (without the previous substring check) is
|
||||
# safe because no versioned model name starts with "gpt-5-chat".
|
||||
_normalized = model.split("/")[-1] # strip provider prefix, e.g. "openai/"
|
||||
return "gpt-5" in model and not _normalized.startswith("gpt-5-chat")
|
||||
|
||||
@classmethod
|
||||
def is_model_gpt_5_search_model(cls, model: str) -> bool:
|
||||
|
|
|
|||
151
tests/test_is_model_gpt_5_model.py
Normal file
151
tests/test_is_model_gpt_5_model.py
Normal file
|
|
@ -0,0 +1,151 @@
|
|||
"""
|
||||
Regression tests for is_model_gpt_5_model() in both OpenAI and Azure GPT-5 config
|
||||
classes.
|
||||
|
||||
Background
|
||||
----------
|
||||
In v1.82.3 a substring check was introduced::
|
||||
|
||||
return "gpt-5" in model and "gpt-5-chat" not in model
|
||||
|
||||
This inadvertently treated versioned chat models like ``gpt-5.3-chat`` and
|
||||
``gpt-5.1-chat`` as *non*-GPT-5 models, because the string ``"gpt-5-chat"`` is
|
||||
a substring of ``"gpt-5.3-chat"``. Those models were then routed through the
|
||||
regular Azure chat path which does not suppress ``parallel_tool_calls``, causing
|
||||
Azure to return ``finish_reason="stop"`` together with tool_calls and breaking
|
||||
n8n AI-agent workflows.
|
||||
|
||||
There are two distinct families:
|
||||
|
||||
* **gpt-5-chat family** (``gpt-5-chat``, ``gpt-5-chat-latest``,
|
||||
``gpt-5-chat-2025-08-07``, …) — regular chat models that support ``temperature``
|
||||
and ``tool_choice`` but NOT ``reasoning_effort``. Must NOT be on the GPT-5
|
||||
reasoning path.
|
||||
|
||||
* **Versioned chat models** (``gpt-5.1-chat``, ``gpt-5.2-chat``,
|
||||
``gpt-5.3-chat``, …) — ARE GPT-5 reasoning models and must stay on the GPT-5
|
||||
path.
|
||||
|
||||
The fix uses a prefix check (``startswith("gpt-5-chat")``) on the normalised model
|
||||
name instead of a substring check, which correctly distinguishes the two families.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from litellm.llms.openai.chat.gpt_5_transformation import OpenAIGPT5Config
|
||||
from litellm.llms.azure.chat.gpt_5_transformation import AzureOpenAIGPT5Config
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Parametrized fixtures
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
# Models that MUST be classified as GPT-5 (routed through GPT-5 reasoning path)
|
||||
GPT5_MODELS = [
|
||||
"gpt-5",
|
||||
"gpt-5.1",
|
||||
"gpt-5.2",
|
||||
"gpt-5.3",
|
||||
"gpt-5.4",
|
||||
"gpt-5.1-chat", # versioned chat — THE KEY REGRESSION CASE
|
||||
"gpt-5.2-chat", # versioned chat — also a regression case
|
||||
"gpt-5.3-chat", # versioned chat — THE KEY REGRESSION CASE
|
||||
"gpt-5.2-chat-latest", # versioned chat with date suffix
|
||||
"gpt-5.1-codex",
|
||||
"gpt-5.1-codex-mini",
|
||||
"gpt-5.1-mini",
|
||||
"gpt-5-nano",
|
||||
"gpt-5-mini",
|
||||
"gpt-5-codex",
|
||||
]
|
||||
|
||||
# Models that must NOT be classified as GPT-5 (regular chat path)
|
||||
NON_GPT5_MODELS = [
|
||||
"gpt-5-chat", # gpt-5-chat family — regular chat path
|
||||
"gpt-5-chat-latest", # gpt-5-chat family with alias suffix
|
||||
"gpt-5-chat-2025-08-07", # gpt-5-chat family with date suffix
|
||||
"gpt-4",
|
||||
"gpt-4o",
|
||||
"gpt-4-turbo",
|
||||
"gpt-3.5-turbo",
|
||||
"o1",
|
||||
"o3",
|
||||
"o3-mini",
|
||||
]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# OpenAIGPT5Config
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestOpenAIGPT5ConfigIsModelGpt5Model:
|
||||
|
||||
@pytest.mark.parametrize("model", GPT5_MODELS)
|
||||
def test_gpt5_models_are_classified_as_gpt5(self, model: str):
|
||||
assert OpenAIGPT5Config.is_model_gpt_5_model(
|
||||
model
|
||||
), f"Expected '{model}' to be classified as a GPT-5 model"
|
||||
|
||||
@pytest.mark.parametrize("model", NON_GPT5_MODELS)
|
||||
def test_non_gpt5_models_are_not_classified_as_gpt5(self, model: str):
|
||||
assert not OpenAIGPT5Config.is_model_gpt_5_model(
|
||||
model
|
||||
), f"Expected '{model}' NOT to be classified as a GPT-5 model"
|
||||
|
||||
def test_versioned_chat_models_are_not_excluded_by_prefix(self):
|
||||
"""Core regression guard: gpt-5-chat prefix must not match versioned models."""
|
||||
versioned_chat_models = ["gpt-5.1-chat", "gpt-5.2-chat", "gpt-5.3-chat"]
|
||||
for model in versioned_chat_models:
|
||||
assert OpenAIGPT5Config.is_model_gpt_5_model(
|
||||
model
|
||||
), f"Regression: '{model}' was incorrectly excluded from GPT-5 path"
|
||||
|
||||
def test_gpt5_chat_family_is_excluded(self):
|
||||
"""gpt-5-chat family should stay on the regular chat path."""
|
||||
for model in ["gpt-5-chat", "gpt-5-chat-latest", "gpt-5-chat-2025-08-07"]:
|
||||
assert not OpenAIGPT5Config.is_model_gpt_5_model(
|
||||
model
|
||||
), f"Expected '{model}' (gpt-5-chat family) NOT to be on the GPT-5 path"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# AzureOpenAIGPT5Config
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestAzureOpenAIGPT5ConfigIsModelGpt5Model:
|
||||
|
||||
@pytest.mark.parametrize("model", GPT5_MODELS)
|
||||
def test_gpt5_models_are_classified_as_gpt5(self, model: str):
|
||||
assert AzureOpenAIGPT5Config.is_model_gpt_5_model(
|
||||
model
|
||||
), f"Expected Azure '{model}' to be classified as a GPT-5 model"
|
||||
|
||||
@pytest.mark.parametrize("model", NON_GPT5_MODELS)
|
||||
def test_non_gpt5_models_are_not_classified_as_gpt5(self, model: str):
|
||||
assert not AzureOpenAIGPT5Config.is_model_gpt_5_model(
|
||||
model
|
||||
), f"Expected Azure '{model}' NOT to be classified as a GPT-5 model"
|
||||
|
||||
def test_versioned_chat_models_are_not_excluded_by_prefix(self):
|
||||
"""Core regression guard: gpt-5-chat prefix must not match versioned models."""
|
||||
versioned_chat_models = ["gpt-5.1-chat", "gpt-5.2-chat", "gpt-5.3-chat"]
|
||||
for model in versioned_chat_models:
|
||||
assert AzureOpenAIGPT5Config.is_model_gpt_5_model(
|
||||
model
|
||||
), f"Regression: Azure '{model}' was incorrectly excluded from GPT-5 path"
|
||||
|
||||
def test_gpt5_chat_family_is_excluded(self):
|
||||
"""gpt-5-chat family should stay on the regular chat path."""
|
||||
for model in ["gpt-5-chat", "gpt-5-chat-latest", "gpt-5-chat-2025-08-07"]:
|
||||
assert not AzureOpenAIGPT5Config.is_model_gpt_5_model(
|
||||
model
|
||||
), f"Expected Azure '{model}' (gpt-5-chat family) NOT to be on the GPT-5 path"
|
||||
|
||||
def test_gpt5_series_routing_prefix_is_always_classified_as_gpt5(self):
|
||||
"""Models using the gpt5_series/ manual-routing prefix must always match."""
|
||||
series_models = ["gpt5_series/my-deployment", "gpt5_series/prod"]
|
||||
for model in series_models:
|
||||
assert AzureOpenAIGPT5Config.is_model_gpt_5_model(
|
||||
model
|
||||
), f"Azure '{model}' with gpt5_series/ prefix should be classified as GPT-5"
|
||||
|
|
@ -31,7 +31,7 @@ class TestPromptVersioning:
|
|||
litellm_params=PromptLiteLLMParams(
|
||||
prompt_id="jack",
|
||||
prompt_integration="dotprompt",
|
||||
dotprompt_content="v1 content"
|
||||
dotprompt_content="v1 content",
|
||||
),
|
||||
prompt_info=PromptInfo(prompt_type="db"),
|
||||
),
|
||||
|
|
@ -40,7 +40,7 @@ class TestPromptVersioning:
|
|||
litellm_params=PromptLiteLLMParams(
|
||||
prompt_id="jack",
|
||||
prompt_integration="dotprompt",
|
||||
dotprompt_content="v2 content"
|
||||
dotprompt_content="v2 content",
|
||||
),
|
||||
prompt_info=PromptInfo(prompt_type="db"),
|
||||
),
|
||||
|
|
@ -49,7 +49,7 @@ class TestPromptVersioning:
|
|||
litellm_params=PromptLiteLLMParams(
|
||||
prompt_id="jane",
|
||||
prompt_integration="dotprompt",
|
||||
dotprompt_content="jane v1"
|
||||
dotprompt_content="jane v1",
|
||||
),
|
||||
prompt_info=PromptInfo(prompt_type="db"),
|
||||
),
|
||||
|
|
@ -58,7 +58,7 @@ class TestPromptVersioning:
|
|||
litellm_params=PromptLiteLLMParams(
|
||||
prompt_id="jack",
|
||||
prompt_integration="dotprompt",
|
||||
dotprompt_content="v3 content"
|
||||
dotprompt_content="v3 content",
|
||||
),
|
||||
prompt_info=PromptInfo(prompt_type="db"),
|
||||
),
|
||||
|
|
@ -120,34 +120,44 @@ class TestPromptVersioning:
|
|||
}
|
||||
|
||||
# Test with base prompt ID - should return latest version
|
||||
assert get_latest_version_prompt_id(
|
||||
prompt_id="jack",
|
||||
all_prompt_ids=all_prompt_ids
|
||||
) == "jack.v3"
|
||||
assert (
|
||||
get_latest_version_prompt_id(
|
||||
prompt_id="jack", all_prompt_ids=all_prompt_ids
|
||||
)
|
||||
== "jack.v3"
|
||||
)
|
||||
|
||||
# Test with versioned prompt ID - should still return latest version
|
||||
assert get_latest_version_prompt_id(
|
||||
prompt_id="jack.v1",
|
||||
all_prompt_ids=all_prompt_ids
|
||||
) == "jack.v3"
|
||||
assert (
|
||||
get_latest_version_prompt_id(
|
||||
prompt_id="jack.v1", all_prompt_ids=all_prompt_ids
|
||||
)
|
||||
== "jack.v3"
|
||||
)
|
||||
|
||||
# Test with single version
|
||||
assert get_latest_version_prompt_id(
|
||||
prompt_id="jane",
|
||||
all_prompt_ids=all_prompt_ids
|
||||
) == "jane.v1"
|
||||
assert (
|
||||
get_latest_version_prompt_id(
|
||||
prompt_id="jane", all_prompt_ids=all_prompt_ids
|
||||
)
|
||||
== "jane.v1"
|
||||
)
|
||||
|
||||
# Test with non-versioned prompt
|
||||
assert get_latest_version_prompt_id(
|
||||
prompt_id="simple_prompt",
|
||||
all_prompt_ids=all_prompt_ids
|
||||
) == "simple_prompt"
|
||||
assert (
|
||||
get_latest_version_prompt_id(
|
||||
prompt_id="simple_prompt", all_prompt_ids=all_prompt_ids
|
||||
)
|
||||
== "simple_prompt"
|
||||
)
|
||||
|
||||
# Test with non-existent prompt
|
||||
assert get_latest_version_prompt_id(
|
||||
prompt_id="nonexistent",
|
||||
all_prompt_ids=all_prompt_ids
|
||||
) == "nonexistent"
|
||||
assert (
|
||||
get_latest_version_prompt_id(
|
||||
prompt_id="nonexistent", all_prompt_ids=all_prompt_ids
|
||||
)
|
||||
== "nonexistent"
|
||||
)
|
||||
|
||||
def test_construct_versioned_prompt_id(self):
|
||||
"""
|
||||
|
|
@ -156,34 +166,34 @@ class TestPromptVersioning:
|
|||
from litellm.proxy.prompts.prompt_endpoints import construct_versioned_prompt_id
|
||||
|
||||
# Test with base prompt ID and version
|
||||
assert construct_versioned_prompt_id(
|
||||
prompt_id="jack_success",
|
||||
version=4
|
||||
) == "jack_success.v4"
|
||||
assert (
|
||||
construct_versioned_prompt_id(prompt_id="jack_success", version=4)
|
||||
== "jack_success.v4"
|
||||
)
|
||||
|
||||
# Test with None version - should return base ID unchanged
|
||||
assert construct_versioned_prompt_id(
|
||||
prompt_id="jack_success",
|
||||
version=None
|
||||
) == "jack_success"
|
||||
assert (
|
||||
construct_versioned_prompt_id(prompt_id="jack_success", version=None)
|
||||
== "jack_success"
|
||||
)
|
||||
|
||||
# Test with existing versioned ID - should replace version
|
||||
assert construct_versioned_prompt_id(
|
||||
prompt_id="jack_success.v2",
|
||||
version=4
|
||||
) == "jack_success.v4"
|
||||
assert (
|
||||
construct_versioned_prompt_id(prompt_id="jack_success.v2", version=4)
|
||||
== "jack_success.v4"
|
||||
)
|
||||
|
||||
# Test with hyphenated prompt ID
|
||||
assert construct_versioned_prompt_id(
|
||||
prompt_id="my-prompt",
|
||||
version=1
|
||||
) == "my-prompt.v1"
|
||||
assert (
|
||||
construct_versioned_prompt_id(prompt_id="my-prompt", version=1)
|
||||
== "my-prompt.v1"
|
||||
)
|
||||
|
||||
# Test with double-digit version
|
||||
assert construct_versioned_prompt_id(
|
||||
prompt_id="test_prompt",
|
||||
version=10
|
||||
) == "test_prompt.v10"
|
||||
assert (
|
||||
construct_versioned_prompt_id(prompt_id="test_prompt", version=10)
|
||||
== "test_prompt.v10"
|
||||
)
|
||||
|
||||
|
||||
class TestPromptVersionsEndpoint:
|
||||
|
|
@ -203,8 +213,7 @@ class TestPromptVersionsEndpoint:
|
|||
|
||||
# Mock user with admin role
|
||||
mock_user = UserAPIKeyAuth(
|
||||
api_key="test_key",
|
||||
user_role=LitellmUserRoles.PROXY_ADMIN
|
||||
api_key="test_key", user_role=LitellmUserRoles.PROXY_ADMIN
|
||||
)
|
||||
|
||||
# Create mock prompt registry with multiple versions
|
||||
|
|
@ -214,7 +223,7 @@ class TestPromptVersionsEndpoint:
|
|||
litellm_params=PromptLiteLLMParams(
|
||||
prompt_id="jack",
|
||||
prompt_integration="dotprompt",
|
||||
dotprompt_content="v1"
|
||||
dotprompt_content="v1",
|
||||
),
|
||||
prompt_info=PromptInfo(prompt_type="db"),
|
||||
),
|
||||
|
|
@ -223,7 +232,7 @@ class TestPromptVersionsEndpoint:
|
|||
litellm_params=PromptLiteLLMParams(
|
||||
prompt_id="jack",
|
||||
prompt_integration="dotprompt",
|
||||
dotprompt_content="v2"
|
||||
dotprompt_content="v2",
|
||||
),
|
||||
prompt_info=PromptInfo(prompt_type="db"),
|
||||
),
|
||||
|
|
@ -232,7 +241,7 @@ class TestPromptVersionsEndpoint:
|
|||
litellm_params=PromptLiteLLMParams(
|
||||
prompt_id="jack",
|
||||
prompt_integration="dotprompt",
|
||||
dotprompt_content="v3"
|
||||
dotprompt_content="v3",
|
||||
),
|
||||
prompt_info=PromptInfo(prompt_type="db"),
|
||||
),
|
||||
|
|
@ -241,20 +250,21 @@ class TestPromptVersionsEndpoint:
|
|||
litellm_params=PromptLiteLLMParams(
|
||||
prompt_id="jane",
|
||||
prompt_integration="dotprompt",
|
||||
dotprompt_content="jane"
|
||||
dotprompt_content="jane",
|
||||
),
|
||||
prompt_info=PromptInfo(prompt_type="db"),
|
||||
),
|
||||
}
|
||||
|
||||
# Mock the IN_MEMORY_PROMPT_REGISTRY at the import location
|
||||
with patch("litellm.proxy.prompts.prompt_registry.IN_MEMORY_PROMPT_REGISTRY") as mock_registry:
|
||||
mock_registry.IN_MEMORY_PROMPTS = mock_prompts
|
||||
# Patch IN_MEMORY_PROMPTS directly on the real singleton so the local
|
||||
# import inside get_prompt_versions picks up the patched data.
|
||||
from litellm.proxy.prompts.prompt_registry import IN_MEMORY_PROMPT_REGISTRY
|
||||
|
||||
with patch.object(IN_MEMORY_PROMPT_REGISTRY, "IN_MEMORY_PROMPTS", mock_prompts):
|
||||
|
||||
# Test with base prompt ID
|
||||
response = await get_prompt_versions(
|
||||
prompt_id="jack",
|
||||
user_api_key_dict=mock_user
|
||||
prompt_id="jack", user_api_key_dict=mock_user
|
||||
)
|
||||
|
||||
# Should return 3 versions of jack, sorted newest first
|
||||
|
|
@ -268,8 +278,7 @@ class TestPromptVersionsEndpoint:
|
|||
|
||||
# Test with versioned prompt ID (should strip version)
|
||||
response = await get_prompt_versions(
|
||||
prompt_id="jack.v1",
|
||||
user_api_key_dict=mock_user
|
||||
prompt_id="jack.v1", user_api_key_dict=mock_user
|
||||
)
|
||||
|
||||
assert len(response.prompts) == 3
|
||||
|
|
@ -289,19 +298,17 @@ class TestPromptVersionsEndpoint:
|
|||
from litellm.proxy.prompts.prompt_endpoints import get_prompt_versions
|
||||
|
||||
mock_user = UserAPIKeyAuth(
|
||||
api_key="test_key",
|
||||
user_role=LitellmUserRoles.PROXY_ADMIN
|
||||
api_key="test_key", user_role=LitellmUserRoles.PROXY_ADMIN
|
||||
)
|
||||
|
||||
with patch("litellm.proxy.prompts.prompt_registry.IN_MEMORY_PROMPT_REGISTRY") as mock_registry:
|
||||
mock_registry.IN_MEMORY_PROMPTS = {}
|
||||
from litellm.proxy.prompts.prompt_registry import IN_MEMORY_PROMPT_REGISTRY
|
||||
|
||||
with patch.object(IN_MEMORY_PROMPT_REGISTRY, "IN_MEMORY_PROMPTS", {}):
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await get_prompt_versions(
|
||||
prompt_id="nonexistent",
|
||||
user_api_key_dict=mock_user
|
||||
prompt_id="nonexistent", user_api_key_dict=mock_user
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 404
|
||||
assert "No versions found" in exc_info.value.detail
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue