diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 8f7327a99b..778f6ef79b 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -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 diff --git a/tests/agent/test_auxiliary_client.py b/tests/agent/test_auxiliary_client.py index 68aae55ad1..8520fe4227 100644 --- a/tests/agent/test_auxiliary_client.py +++ b/tests/agent/test_auxiliary_client.py @@ -2455,7 +2455,7 @@ class TestKimiTemperatureOmitted: "model", [ "anthropic/claude-sonnet-4-6", - "gpt-5.4", + "gpt-4.1", "deepseek-chat", ], ) diff --git a/tests/agent/test_auxiliary_parameter_rung_chaining.py b/tests/agent/test_auxiliary_parameter_rung_chaining.py index 4df7b3ac43..0e336245a8 100644 --- a/tests/agent/test_auxiliary_parameter_rung_chaining.py +++ b/tests/agent/test_auxiliary_parameter_rung_chaining.py @@ -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, ) diff --git a/tests/agent/test_injected_param_strip_retry_registry.py b/tests/agent/test_injected_param_strip_retry_registry.py index c3c719d2fc..867e8dd831 100644 --- a/tests/agent/test_injected_param_strip_retry_registry.py +++ b/tests/agent/test_injected_param_strip_retry_registry.py @@ -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), ) diff --git a/tests/agent/test_unsupported_temperature_retry.py b/tests/agent/test_unsupported_temperature_retry.py index e60b835d71..28babcdcc4 100644 --- a/tests/agent/test_unsupported_temperature_retry.py +++ b/tests/agent/test_unsupported_temperature_retry.py @@ -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",