andre/crewAI
1
0
Fork 0
mirror of https://github.com/crewAIInc/crewAI.git synced 2026-07-27 09:45:16 +00:00
Code Issues Actions 6 Packages Projects Releases Wiki Activity

refactor(openai): drop the hardcoded model list entirely

Browse Source 
`_TOOLS_REASONING_EFFORT_INCOMPATIBLE` was kept as a "fast path" to skip a doomed
request for gpt-5.4/5.5/5.6. That was the wrong call: it still meant a model table
in this file, and I got that table wrong twice already -- first missing gpt-5.4 and
gpt-5.5 entirely, then missing the -mini and -nano variants.

Nothing is assumed up front now. The first call for a model goes out exactly as the
caller asked; if it comes back 400, `_call_completions` retries with
reasoning_effort="none" and records the model, so subsequent calls skip straight to
"none". Same shape as the responses-only handling.

Cost is one wasted round trip per model per process instead of a release every time
OpenAI extends the restriction.

`_supports_reasoning_effort_with_tools` is now purely a cache lookup, so both
recovery paths in this file are driven by what the API actually returns rather than
by what was measured on a particular night.

Verified live with no list present:

    learned before      -> set()
    1st call gpt-5.6-sol -> tool call returned (400 caught and recovered)
    learned after       -> {('https://api.openai.com/v1', 'gpt-5.6-sol')}
    2nd call            -> tool call returned, no 400
    real Agent + tools  -> 391

Tests: the fast-path cases are rewritten around learned state -- an unknown model
sends what was asked, a known one is rewritten to "none", learning doesn't leak to
other models, and the vacuous-pass guard is kept.
tests/llms/openai -> 159 passed, 1 skipped. Ruff + mypy clean.
This commit is contained in:
alex-clawd
2026-07-26 03:28:42 -07:00
parent 6c64ffb03f
commit 155ba11c4a
2 changed files with 50 additions and 90 deletions
Download Patch File Download Diff File Expand all files Collapse all files

50
lib/crewai/src/crewai/llms/providers/openai/completion.py
View File

@@ -67,25 +67,10 @@ if TYPE_CHECKING:
# conflict learned against one endpoint must not leak to another.
_LEARNED_RESPONSES_ONLY_MODELS: set[tuple[str, str]] = set()
# Known model families that reject `reasoning_effort` alongside function tools on
# /v1/chat/completions. This is only a fast path to avoid a wasted round trip --
# the authoritative check is `_is_tools_reasoning_effort_error`, which catches
# families we haven't measured yet.
#
# Measured against /v1/chat/completions: gpt-5.4, gpt-5.5 and gpt-5.6 (including
# -mini, -nano, dated snapshots and the 5.6 sol/terra/luna variants) all reject
# the combination. gpt-5.2 and earlier, o1, o3 and o4-mini accept it.
# Matched as substrings against the lowercased model name so provider prefixes
# ("openai/gpt-5.5") and suffixed variants ("gpt-5.6-sol") are covered too.
_TOOLS_REASONING_EFFORT_INCOMPATIBLE: tuple[str, ...] = (
"gpt-5.4",
"gpt-5.5",
"gpt-5.6",
)
# Models learned at runtime from a 400, keyed by the model string as configured.
# Populated by `_remember_reasoning_effort_conflict` so the retry is paid once
# per model per process rather than on every call.
# Models learned at runtime to reject `reasoning_effort` alongside function tools,
# keyed by (endpoint, model). Populated from the 400 by
# `_remember_reasoning_effort_conflict` so the wasted round trip is paid once per
# model per process rather than on every call.
_LEARNED_TOOLS_REASONING_EFFORT_CONFLICTS: set[tuple[str, str]] = set()
@@ -1749,27 +1734,23 @@ class OpenAICompletion(BaseLLM):
return f"Model {self.model} not found: {error}"
def _supports_reasoning_effort_with_tools(self, model: str) -> bool:
"""Whether a model accepts `reasoning_effort` together with function tools.
"""Whether this model is already known to reject the combination.
On /v1/chat/completions, newer reasoning models reject the combination:
Nothing is assumed up front -- no model list. A model is only known once a
400 has been seen for it, which `_call_completions` recovers from:
"Function tools with reasoning_effort are not supported for gpt-5.5
in /v1/chat/completions. To use function tools, use /v1/responses or
set reasoning_effort to 'none'."
Known families are listed in `_TOOLS_REASONING_EFFORT_INCOMPATIBLE` purely
to skip a doomed request. Models discovered through a 400 at runtime are
remembered in `_LEARNED_TOOLS_REASONING_EFFORT_CONFLICTS`, so OpenAI can
extend the restriction to new families without a release here.
So OpenAI can extend the restriction to new families without a release here.
The Responses API has no such restriction (it takes
`reasoning: {"effort": ...}`), so this only constrains the completions path.
"""
if self._model_cache_key(model) in _LEARNED_TOOLS_REASONING_EFFORT_CONFLICTS:
return False
model_lower = model.lower()
return not any(
family in model_lower for family in _TOOLS_REASONING_EFFORT_INCOMPATIBLE
return (
self._model_cache_key(model)
not in _LEARNED_TOOLS_REASONING_EFFORT_CONFLICTS
)
@staticmethod
@@ -1942,11 +1923,10 @@ class OpenAICompletion(BaseLLM):
params["tools"] = self._convert_tools_for_interference(tools)
params["tool_choice"] = "auto"
# gpt-5.4+ reject `reasoning_effort` on the completions endpoint as soon
# as function tools are present. Anything other than "none" is a hard 400,
# so drop it up front for models we already know about rather than burn a
# round trip. Families we haven't measured are caught by the 400 handler
# in `_call_completions` instead.
# Some models reject `reasoning_effort` on the completions endpoint as
# soon as function tools are present. Once a 400 has taught us that about
# this model, skip straight to "none" instead of repeating the doomed
# request; the first occurrence is recovered in `_call_completions`.
# Checked after the tools block on purpose: `reasoning_effort` can also
# arrive through `additional_params`, which bypasses the typed field.
if (

90
lib/crewai/tests/llms/openai/test_gpt56_tools_reasoning_effort.py
View File

@@ -77,37 +77,41 @@ def _clear_learned_conflicts():
completion_module._LEARNED_TOOLS_REASONING_EFFORT_CONFLICTS.clear()
class TestKnownFamiliesFastPath:
"""Models we've measured are stripped before the request goes out."""
class TestLearnedFastPath:
"""Nothing is assumed up front; a 400 is what teaches us about a model."""
@pytest.mark.parametrize(
"model",
[
"gpt-5.6",
"openai/gpt-5.6",
"gpt-5.6-sol",
"GPT-5.6-Terra",
"gpt-5.6-luna",
"gpt-5.5",
"gpt-5.5-2026-04-23",
"gpt-5.4",
"gpt-5.4-mini",
"gpt-5.4-nano",
],
)
def test_drops_reasoning_effort_for_incompatible_models(self, model: str):
llm = build(model, additional_params={"reasoning_effort": "high"})
def test_sends_reasoning_effort_for_an_unknown_model(self):
"""No model list, so the first request goes out as the caller asked.
The 400 it may come back with is recovered in `_call_completions`.
"""
llm = build("gpt-5.6", additional_params={"reasoning_effort": "high"})
params = llm._prepare_completion_params(MESSAGES, tools=TOOLS)
assert params["reasoning_effort"] == "high"
def test_forces_none_once_the_model_is_known(self):
"""After a 400, skip the doomed request and go straight to "none".
Explicit "none" rather than removing the key: gpt-5.6-* apply a
server-side default, so omitting it is rejected the same way.
"""
llm = build("gpt-5.6", additional_params={"reasoning_effort": "high"})
llm._remember_reasoning_effort_conflict()
params = llm._prepare_completion_params(MESSAGES, tools=TOOLS)
assert "reasoning_effort" in params, "the key must be sent, not dropped"
assert params["reasoning_effort"] == "none"
# Tools must survive — they are the reason the call exists.
assert params["tools"]
assert params["tool_choice"] == "auto"
def test_drops_reasoning_effort_supplied_via_additional_params(self):
def test_learning_applies_to_the_additional_params_leak(self):
"""additional_params bypasses the typed field, so it must be checked too."""
llm = build("gpt-5.5", additional_params={"reasoning_effort": "medium"})
llm._remember_reasoning_effort_conflict()
params = llm._prepare_completion_params(MESSAGES, tools=TOOLS)
@@ -116,51 +120,27 @@ class TestKnownFamiliesFastPath:
def test_keeps_reasoning_effort_without_tools(self):
"""Without tools the combination is legal, so nothing should change."""
llm = build("gpt-5.5", additional_params={"reasoning_effort": "high"})
llm._remember_reasoning_effort_conflict()
params = llm._prepare_completion_params(MESSAGES, tools=None)
assert params["reasoning_effort"] == "high"
def test_forces_none_rather_than_removing_the_parameter(self):
"""Omitting `reasoning_effort` is not sufficient.
def test_learning_does_not_touch_other_models(self):
"""A conflict is remembered per model, not globally."""
build("gpt-5.5")._remember_reasoning_effort_conflict()
other = build("gpt-5.2", additional_params={"reasoning_effort": "high"})
Measured against the live endpoint: gpt-5.6-* apply a server-side default,
so a request carrying no `reasoning_effort` at all is rejected exactly like
one carrying "high". Only an explicit "none" is accepted with tools.
gpt-5.6-sol tools, no reasoning_effort -> 400
gpt-5.6-sol tools, reasoning_effort=none -> OK
"""
llm = build("gpt-5.6-sol", additional_params={"reasoning_effort": "high"})
params = llm._prepare_completion_params(MESSAGES, tools=TOOLS)
assert "reasoning_effort" in params, "the key must be sent, not dropped"
assert params["reasoning_effort"] == "none"
def test_keeps_explicit_none_effort_with_tools(self):
"""OpenAI explicitly allows reasoning_effort='none' with tools here."""
llm = build("gpt-5.6", additional_params={"reasoning_effort": "none"})
params = llm._prepare_completion_params(MESSAGES, tools=TOOLS)
assert params["reasoning_effort"] == "none"
@pytest.mark.parametrize("model", ["o1", "o3-mini", "gpt-5.2", "gpt-5"])
def test_unaffected_models_keep_reasoning_effort_with_tools(self, model: str):
"""Measured as accepting the combination — must not be stripped."""
llm = build(model, additional_params={"reasoning_effort": "high"})
params = llm._prepare_completion_params(MESSAGES, tools=TOOLS)
params = other._prepare_completion_params(MESSAGES, tools=TOOLS)
assert params["reasoning_effort"] == "high"
def test_fast_path_actually_has_something_to_drop(self):
"""Guard against the drop tests passing vacuously.
"""Guard against the assertions above passing vacuously.
The typed `reasoning_effort` field is gated behind `is_o1_model`, so it
never reaches the payload for gpt-5.x. If a test set it that way there
would be nothing to strip and the assertion would pass for the wrong
would be nothing to rewrite and the assertion would pass for the wrong
reason — `additional_params` is the path that actually leaks.
"""
leaked = build(
@@ -168,9 +148,9 @@ class TestKnownFamiliesFastPath:
)._prepare_completion_params(MESSAGES, tools=None)
assert leaked["reasoning_effort"] == "high"
typed = build(
"gpt-5.5", reasoning_effort="high"
)._prepare_completion_params(MESSAGES, tools=None)
typed = build("gpt-5.5", reasoning_effort="high")._prepare_completion_params(
MESSAGES, tools=None
)
assert "reasoning_effort" not in typed