Merge pull request #115850 from NousResearch/fix/boa-vision-image-openai-auxtemp
fix(auxiliary): vision and other auxiliary calls to gpt-5.x / o-series no longer pay a temperature 400 round-trip (#51083, salvage #51157)
This commit is contained in:
@@ -631,13 +631,41 @@ def _is_codex_spark(model: Optional[str], provider: Optional[str] = None) -> boo
|
||||
return _codex_route_bare_model(model, provider) == "gpt-5.3-codex-spark"
|
||||
|
||||
|
||||
def _is_openai_default_temperature_only(model: Optional[str]) -> bool:
|
||||
"""True for OpenAI reasoning families that 400 (``unsupported_value``) on any non-default
|
||||
``temperature``: gpt-5.x (incl. dated snapshots, ``-pro``, ``-codex``), o1/o3/o4. The
|
||||
``gpt-5-chat`` non-reasoning line still accepts it (#51083)."""
|
||||
bare = _bare_model(model)
|
||||
return bare.startswith(("gpt-5", "o1", "o3", "o4")) and not bare.startswith("gpt-5-chat")
|
||||
|
||||
|
||||
# Routes (host + model) that rejected ``temperature`` at runtime; the next call omits it up front
|
||||
# instead of paying the 400 round-trip again (the retry alone left #51083's first call to time out).
|
||||
_TEMPERATURE_REJECTED_ROUTES: set = set()
|
||||
|
||||
|
||||
def remember_temperature_rejection(
|
||||
provider: Optional[str], base_url: Optional[str], rejected_kwargs: Dict[str, Any], error: BaseException,
|
||||
) -> None:
|
||||
from agent.auxiliary_structured_output import _route_key
|
||||
_TEMPERATURE_REJECTED_ROUTES.add((_route_key(provider, base_url), _bare_model(rejected_kwargs.get("model"))))
|
||||
|
||||
|
||||
def _fixed_temperature_for_model(
|
||||
model: Optional[str], base_url: Optional[str] = None
|
||||
model: Optional[str], base_url: Optional[str] = None, provider: Optional[str] = None,
|
||||
) -> "Optional[float] | object":
|
||||
"""``OMIT_TEMPERATURE`` (drop the key; Kimi/Moonshot), a fixed ``float``, or ``None``."""
|
||||
"""``OMIT_TEMPERATURE`` (drop the key; Kimi/Moonshot, OpenAI reasoning families, routes that
|
||||
already rejected it), a fixed ``float``, or ``None``."""
|
||||
if _is_kimi_model(model):
|
||||
logger.debug("Omitting temperature for Kimi model %r (server-managed)", model)
|
||||
return OMIT_TEMPERATURE
|
||||
if _is_openai_default_temperature_only(model):
|
||||
logger.debug("Omitting temperature for %r (accepts only the default)", model)
|
||||
return OMIT_TEMPERATURE
|
||||
from agent.auxiliary_structured_output import _route_key
|
||||
if (_route_key(provider, base_url), _bare_model(model)) in _TEMPERATURE_REJECTED_ROUTES:
|
||||
logger.debug("Omitting temperature for %r (route rejected it earlier)", model)
|
||||
return OMIT_TEMPERATURE
|
||||
return 0.5 if _is_arcee_trinity_thinking(model) else None
|
||||
|
||||
|
||||
@@ -6531,9 +6559,10 @@ def _build_call_kwargs(
|
||||
kwargs: Dict[str, Any] = {"model": model, "messages": messages, "timeout": timeout}
|
||||
if no_progress_timeout is not None:
|
||||
kwargs["no_progress_timeout"] = no_progress_timeout
|
||||
effective_base = base_url or (_current_custom_base_url() if provider == "custom" else "")
|
||||
# Per-model fixed/omitted temperature, then Opus 4.7+ sampling bans: it rejects any
|
||||
# non-default temperature/top_p/top_k, so drop silently rather than 400 when the aux model flips.
|
||||
fixed_temperature = _fixed_temperature_for_model(model, base_url)
|
||||
fixed_temperature = _fixed_temperature_for_model(model, effective_base, provider)
|
||||
if fixed_temperature is OMIT_TEMPERATURE:
|
||||
temperature = None # strip — let server choose
|
||||
elif fixed_temperature is not None:
|
||||
@@ -6542,7 +6571,6 @@ def _build_call_kwargs(
|
||||
from agent.anthropic_adapter import _forbids_sampling_params
|
||||
if not _forbids_sampling_params(model):
|
||||
kwargs["temperature"] = temperature
|
||||
effective_base = base_url or (_current_custom_base_url() if provider == "custom" else "")
|
||||
provider_norm = str(provider or "").strip().lower()
|
||||
if max_tokens is not None and _forwards_max_tokens(provider, provider_norm, model, effective_base, task):
|
||||
kwargs.update(auxiliary_max_tokens_param(max_tokens, model=model)) # picks max_completion_tokens where needed
|
||||
@@ -7337,7 +7365,7 @@ def _parameter_rungs(client: Any, max_tokens: Optional[int]) -> tuple:
|
||||
(optional) records the rejection per route so the next call omits the field up front."""
|
||||
return (
|
||||
(lambda exc: _is_unsupported_parameter_error(exc, "temperature"), _without_temperature,
|
||||
"provider rejected temperature; retrying without it", None),
|
||||
"provider rejected temperature; retrying without it", remember_temperature_rejection),
|
||||
(_is_structured_output_rejection, _without_structured_output_format,
|
||||
"provider rejected the structured-output format field; retrying without it "
|
||||
"(schema enforcement degrades to prompt compliance)", remember_structured_output_rejection),
|
||||
@@ -7382,7 +7410,10 @@ def _ladder_parameter_rungs(
|
||||
_LadderStep("call", (client, retry_kwargs)), _param_rung_accepts)
|
||||
if first_err is None:
|
||||
if remember is not None:
|
||||
remember(route.resolved_provider, route.base_info, kwargs, rejection)
|
||||
# Same key _build_call_kwargs looks up (base_info or resolved_base_url), so the
|
||||
# memory hits when the client exposes no base_url but the task resolved one.
|
||||
remember(route.resolved_provider, route.base_info or route.resolved_base_url,
|
||||
kwargs, rejection)
|
||||
return resp, None, retry_kwargs
|
||||
kwargs = retry_kwargs
|
||||
return None, first_err, kwargs
|
||||
|
||||
@@ -2455,7 +2455,7 @@ class TestKimiTemperatureOmitted:
|
||||
"model",
|
||||
[
|
||||
"anthropic/claude-sonnet-4-6",
|
||||
"gpt-5.4",
|
||||
"gpt-4.1",
|
||||
"deepseek-chat",
|
||||
],
|
||||
)
|
||||
|
||||
@@ -112,7 +112,7 @@ def test_structured_param_rejection_strips_reasoning_effort_on_retry():
|
||||
def test_fallback_candidate_recovers_from_rejected_temperature():
|
||||
client = _rejecting_client("temperature")
|
||||
resp = _call_fallback_candidate_sync(
|
||||
client, "gpt-5-mini", "fallback_chain[0](openai)", task="title_generation",
|
||||
client, "relay-model-x", "fallback_chain[0](openai)", task="title_generation",
|
||||
messages=[{"role": "user", "content": "hi"}], temperature=0.3, max_tokens=16, tools=None,
|
||||
effective_timeout=30.0, effective_extra_body={}, reasoning_config=None,
|
||||
)
|
||||
|
||||
@@ -206,11 +206,13 @@ class _FlakyClient:
|
||||
|
||||
|
||||
def _aux_patches(client):
|
||||
# gpt-4.1: a model the aux path still SENDS temperature to. gpt-5.x omits it up front
|
||||
# (#51083), which would leave the reactive strip rung nothing to strip.
|
||||
return (
|
||||
patch("agent.auxiliary_client._resolve_task_provider_model",
|
||||
return_value=("openai-codex", "gpt-5.5", None, None, None)),
|
||||
return_value=("openai-codex", "gpt-4.1", None, None, None)),
|
||||
patch("agent.auxiliary_client._get_cached_client",
|
||||
return_value=(client, "gpt-5.5")),
|
||||
return_value=(client, "gpt-4.1")),
|
||||
patch("agent.auxiliary_client._validate_llm_response",
|
||||
side_effect=lambda resp, _task, **_kw: resp),
|
||||
)
|
||||
|
||||
@@ -28,12 +28,76 @@ from unittest.mock import patch, MagicMock, AsyncMock
|
||||
import pytest
|
||||
|
||||
from agent.auxiliary_client import (
|
||||
OMIT_TEMPERATURE,
|
||||
_TEMPERATURE_REJECTED_ROUTES,
|
||||
_build_call_kwargs,
|
||||
_fixed_temperature_for_model,
|
||||
call_llm,
|
||||
async_call_llm,
|
||||
_is_unsupported_parameter_error,
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _forget_rejected_routes():
|
||||
_TEMPERATURE_REJECTED_ROUTES.clear()
|
||||
yield
|
||||
_TEMPERATURE_REJECTED_ROUTES.clear()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("model", ["gpt-5.5", "openai/gpt-5.5-pro", "gpt-5.1-2026-01-01", "gpt-5-codex", "o3-mini", "o4-mini"])
|
||||
def test_openai_default_only_families_omit_temperature_up_front(model):
|
||||
"""#51083: OpenAI reasoning families 400 on temperature != 1, so the first request already omits
|
||||
it instead of paying a rejected round-trip; gpt-5-chat and gpt-4.1 still get the caller's value."""
|
||||
assert _fixed_temperature_for_model(model) is OMIT_TEMPERATURE
|
||||
kwargs = _build_call_kwargs("openai-api", model, [{"role": "user", "content": "hi"}], temperature=0.1)
|
||||
assert "temperature" not in kwargs
|
||||
for accepts in ("gpt-5-chat-latest", "gpt-4.1"):
|
||||
assert _build_call_kwargs("openai-api", accepts, [], temperature=0.1)["temperature"] == 0.1
|
||||
|
||||
|
||||
def test_route_that_rejected_temperature_omits_it_next_call():
|
||||
"""#51083: after one ``unsupported_value`` on temperature the route+model is remembered and the
|
||||
next call sends a single request without it; a different model on the route is unaffected."""
|
||||
client = MagicMock()
|
||||
client.base_url = "https://relay.example/v1"
|
||||
client.chat.completions.create.side_effect = [
|
||||
RuntimeError("Error code: 400 - {'error': {'message': \"Unsupported value: 'temperature' does not support 0.1 with this model. Only the default (1) value is supported.\", 'param': 'temperature', 'code': 'unsupported_value'}}"),
|
||||
_dummy_response(), _dummy_response(), _dummy_response()]
|
||||
with (
|
||||
patch("agent.auxiliary_client._resolve_task_provider_model",
|
||||
return_value=("custom", "relay-model-x", None, None, None)),
|
||||
patch("agent.auxiliary_client._get_cached_client", return_value=(client, "relay-model-x")),
|
||||
patch("agent.auxiliary_client._validate_llm_response", side_effect=lambda resp, _task, **_kw: resp),
|
||||
):
|
||||
call_llm(task="vision", messages=[{"role": "user", "content": "a"}], temperature=0.1)
|
||||
call_llm(task="vision", messages=[{"role": "user", "content": "b"}], temperature=0.1)
|
||||
calls = client.chat.completions.create.call_args_list
|
||||
assert [c.kwargs.get("temperature") for c in calls] == [0.1, None, None]
|
||||
assert _fixed_temperature_for_model("relay-model-y", "https://relay.example/v1", "custom") is None
|
||||
|
||||
|
||||
def test_route_memory_keyed_on_effective_base_url_when_client_has_none():
|
||||
"""The rejection is recorded under the same key the kwargs builder looks up: when the client
|
||||
exposes no ``base_url`` but the task resolved one, the resolved URL is the effective key, so the
|
||||
second call still omits temperature instead of paying the 400 again."""
|
||||
client = MagicMock()
|
||||
client.base_url = None
|
||||
client.chat.completions.create.side_effect = [
|
||||
RuntimeError("Error code: 400 - {'error': {'message': \"Unsupported value: 'temperature' does not support 0.1 with this model. Only the default (1) value is supported.\", 'param': 'temperature', 'code': 'unsupported_value'}}"),
|
||||
_dummy_response(), _dummy_response(), _dummy_response()]
|
||||
with (
|
||||
patch("agent.auxiliary_client._resolve_task_provider_model",
|
||||
return_value=("custom", "relay-model-x", "https://relay.example/v1", "k", None)),
|
||||
patch("agent.auxiliary_client._get_cached_client", return_value=(client, "relay-model-x")),
|
||||
patch("agent.auxiliary_client._validate_llm_response", side_effect=lambda resp, _task, **_kw: resp),
|
||||
):
|
||||
call_llm(task="compression", messages=[{"role": "user", "content": "a"}], temperature=0.1)
|
||||
call_llm(task="compression", messages=[{"role": "user", "content": "b"}], temperature=0.1)
|
||||
calls = client.chat.completions.create.call_args_list
|
||||
assert [c.kwargs.get("temperature") for c in calls] == [0.1, None, None]
|
||||
|
||||
|
||||
class TestIsUnsupportedTemperatureError:
|
||||
"""The detector must match the phrasings providers actually return."""
|
||||
|
||||
@@ -93,9 +157,9 @@ class TestCallLlmUnsupportedTemperatureRetry:
|
||||
|
||||
with (
|
||||
patch("agent.auxiliary_client._resolve_task_provider_model",
|
||||
return_value=("openai-codex", "gpt-5.5", None, None, None)),
|
||||
return_value=("openai-codex", "relay-model-x", None, None, None)),
|
||||
patch("agent.auxiliary_client._get_cached_client",
|
||||
return_value=(client, "gpt-5.5")),
|
||||
return_value=(client, "relay-model-x")),
|
||||
patch("agent.auxiliary_client._validate_llm_response",
|
||||
side_effect=lambda resp, _task, **_kw: resp),
|
||||
):
|
||||
@@ -131,9 +195,9 @@ class TestCallLlmUnsupportedTemperatureRetry:
|
||||
|
||||
with (
|
||||
patch("agent.auxiliary_client._resolve_task_provider_model",
|
||||
return_value=("openai-codex", "gpt-5.5", None, None, None)),
|
||||
return_value=("openai-codex", "relay-model-x", None, None, None)),
|
||||
patch("agent.auxiliary_client._get_cached_client",
|
||||
return_value=(client, "gpt-5.5")),
|
||||
return_value=(client, "relay-model-x")),
|
||||
patch("agent.auxiliary_client._validate_llm_response",
|
||||
side_effect=lambda resp, _task, **_kw: resp),
|
||||
patch("agent.auxiliary_client._try_payment_fallback",
|
||||
@@ -161,9 +225,9 @@ class TestCallLlmUnsupportedTemperatureRetry:
|
||||
|
||||
with (
|
||||
patch("agent.auxiliary_client._resolve_task_provider_model",
|
||||
return_value=("openai-codex", "gpt-5.5", None, None, None)),
|
||||
return_value=("openai-codex", "relay-model-x", None, None, None)),
|
||||
patch("agent.auxiliary_client._get_cached_client",
|
||||
return_value=(client, "gpt-5.5")),
|
||||
return_value=(client, "relay-model-x")),
|
||||
patch("agent.auxiliary_client._validate_llm_response",
|
||||
side_effect=lambda resp, _task, **_kw: resp),
|
||||
patch("agent.auxiliary_client._try_payment_fallback",
|
||||
@@ -193,9 +257,9 @@ class TestAsyncCallLlmUnsupportedTemperatureRetry:
|
||||
|
||||
with (
|
||||
patch("agent.auxiliary_client._resolve_task_provider_model",
|
||||
return_value=("openai-codex", "gpt-5.5", None, None, None)),
|
||||
return_value=("openai-codex", "relay-model-x", None, None, None)),
|
||||
patch("agent.auxiliary_client._get_cached_client",
|
||||
return_value=(client, "gpt-5.5")),
|
||||
return_value=(client, "relay-model-x")),
|
||||
patch("agent.auxiliary_client._validate_llm_response",
|
||||
side_effect=lambda resp, _task, **_kw: resp),
|
||||
):
|
||||
@@ -228,9 +292,9 @@ class TestAsyncCallLlmUnsupportedTemperatureRetry:
|
||||
|
||||
with (
|
||||
patch("agent.auxiliary_client._resolve_task_provider_model",
|
||||
return_value=("openai-codex", "gpt-5.5", None, None, None)),
|
||||
return_value=("openai-codex", "relay-model-x", None, None, None)),
|
||||
patch("agent.auxiliary_client._get_cached_client",
|
||||
return_value=(client, "gpt-5.5")),
|
||||
return_value=(client, "relay-model-x")),
|
||||
patch("agent.auxiliary_client._validate_llm_response",
|
||||
side_effect=lambda resp, _task, **_kw: resp),
|
||||
patch("agent.auxiliary_client._try_payment_fallback",
|
||||
|
||||
Reference in New Issue
Block a user