diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 5dd1e3595f..aee6e7d922 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -27,7 +27,8 @@ from urllib.parse import urlparse, parse_qs, urlunparse from agent.error_classifier import ( _BILLING_PATTERNS, _OVERLOADED_PATTERNS, - is_reasoning_disable_rejected, + UNSUPPORTED_PARAM_MARKERS, + is_reasoning_field_rejection, ) from agent.auxiliary_structured_output import remember_structured_output_rejection from agent.codex_headers import ( @@ -3244,16 +3245,7 @@ def _is_unsupported_parameter_error(exc: Exception, param: str) -> bool: if not param_lower: return False err_lower = str(exc).lower() - # Bedrock Converse rejects sampling params for reasoning-first models with the contraction - # ("This model doesn't support the temperature field", xAI Grok) and inference-profile Claude - # with "`temperature` is deprecated for this model" (#111043). - return param_lower in err_lower and _contains_any(err_lower, ( - "unsupported parameter", "unsupported_parameter", "not supported", "does not support", - "doesn't support", "is deprecated for this model", - "unknown parameter", "unrecognized request argument", "unrecognized parameter", "invalid parameter", - # Strict pydantic-validated gateways (Fireworks) name the unknown field this way (#109774). - "extra inputs are not permitted", - )) + return param_lower in err_lower and _contains_any(err_lower, UNSUPPORTED_PARAM_MARKERS) def _is_structured_output_rejection(exc: Exception) -> bool: @@ -3307,10 +3299,7 @@ def _is_reasoning_field_rejection(exc: Exception) -> bool: status = getattr(exc, "status_code", None) if status is not None and status not in {400, 422}: return False - # Shared wording-class matcher (agent.error_classifier): the strict - # standalone-token gate inside keeps model-id segments and the "reasoning - # models" adjective on the provider-fallback rung (#114460). - return is_reasoning_disable_rejected(str(exc)) + return is_reasoning_field_rejection(str(exc)) def _without_reasoning_fields(kwargs: dict) -> Optional[dict]: diff --git a/agent/error_classifier.py b/agent/error_classifier.py index 501456714a..eb0c7863e6 100644 --- a/agent/error_classifier.py +++ b/agent/error_classifier.py @@ -435,43 +435,42 @@ _V_MALFORMED_TOOL_ARGS = _v(_R.format_error, retryable=False, should_fallback=Fa # A reasoning-mandatory route answering ``reasoning: {enabled: false}`` (Nous Portal + OpenRouter wording). _REASONING_MANDATORY_PATTERN = "reasoning is mandatory" -# Reasoning wire-field token shared with the auxiliary retry rung -# (agent/auxiliary_client._is_reasoning_field_rejection, which cannot be imported -# here — it imports from this module). Standalone field name only: never a model-id -# segment ("kimi-k2-thinking") nor the adjective in "... with reasoning models" (#114460). -_REASONING_DISABLE_TOKEN = re.compile( - r"(? bool: - """True when a provider error rejects a reasoning wire control by name (#114460). - Covers the forward markers ("Unrecognized request argument supplied: - reasoning_effort") and the reversed word order some providers use - ("reasoning_effort 'none' unsupported": field token plus standalone - "unsupported" within 32 chars). The main loop maps this to - reasoning_mandatory (drop the disable, retry); the auxiliary ladder maps it - to its strip-and-retry rung. Same wording class, same remedy. - """ +def is_reasoning_field_rejection(error_msg: str) -> bool: + """Provider 400 rejecting a reasoning wire control by name (``reasoning_effort``, ``reasoning``, + ``thinking``/``think``): the field token plus either a generic unsupported marker ("Unrecognized + request argument supplied: reasoning_effort", #112781) or a standalone "unsupported" next to the + field in either word order ("unsupported reasoning_effort"; "reasoning_effort 'none' unsupported; + use minimal|low|medium|high|xhigh", #114460). The route default is the right answer for such a + model, so both the main loop and the auxiliary ladder retry once without the disable.""" msg = (error_msg or "").lower() - if "reasoning" not in msg and "think" not in msg: + token = _REASONING_FIELD_TOKEN.search(msg) + if token is None: return False - if not any(name in msg and any(m in msg for m in _UNSUPPORTED_PARAM_MARKERS) - for name in ("reasoning", "think")): - token = _REASONING_DISABLE_TOKEN.search(msg) - if token is None or "unsupported" not in msg[token.end():token.end() + 32]: - return False - return _REASONING_DISABLE_TOKEN.search(msg) is not None + near = msg[max(0, token.start() - 32):token.end() + 32] + return "unsupported" in near or any(m in msg for m in UNSUPPORTED_PARAM_MARKERS) def _billing_hints(error_msg: str) -> Verdict: @@ -932,10 +931,11 @@ def _classify_400(c: _Ctx) -> Verdict: "conflicting authenticated continuation identities" in msg ): return _V_INVALID_ENCRYPTED - # Reasoning-mandatory route rejecting a disable (GLM-5.3 on Nous Portal / OpenRouter). Deterministic - # for the request shape, but the only bad field is ``reasoning: {enabled: false}`` — the loop drops - # the disable and retries once. Must precede request-validation, which would abort as format_error. - if _REASONING_MANDATORY_PATTERN in msg or is_reasoning_disable_rejected(msg): + # Route rejecting a reasoning disable: a reasoning-mandatory route (GLM-5.3 on Nous Portal / + # OpenRouter) or a chat-only relay that does not accept ``reasoning_effort: none`` at all + # (#114460). Deterministic for the request shape, but the only bad field is the disable — the + # loop drops it and retries once. Must precede request-validation, which would abort as format_error. + if _REASONING_MANDATORY_PATTERN in msg or is_reasoning_field_rejection(msg): return _V_REASONING_MANDATORY # 400 blaming a field this route never sent (Codex OAuth injects then rejects # prompt_cache_retention ~20% of the time): transient, retry identical request. diff --git a/agent/turn_api_error.py b/agent/turn_api_error.py index 1f056d9730..7ff73b728b 100644 --- a/agent/turn_api_error.py +++ b/agent/turn_api_error.py @@ -283,9 +283,17 @@ def settle_unrecovered_error( shrink_spent = classified.reason == FailoverReason.image_too_large and bool( getattr(_retry, "image_shrink_retry_attempted", False) ) + # Same shape for the reasoning-disable rung: the retry already went out without the disable, + # so a second reasoning-field rejection means the route refuses the configured reasoning + # controls themselves — nothing left to drop, so take the fallback chain now instead of + # replaying the identical request ``max_retries`` times (#114460). + reasoning_spent = classified.reason == FailoverReason.reasoning_mandatory and bool( + getattr(_retry, "reasoning_mandatory_retry_attempted", False) + ) is_client_error = ( is_local_validation_error or shrink_spent + or reasoning_spent or ( not classified.retryable and not classified.should_compress @@ -321,7 +329,7 @@ def settle_unrecovered_error( # the cascade. An UNCLASSIFIED local ValueError/TypeError keeps its historical fallback; # a recognised verdict that opts out wins even when the exception is a ValueError subclass. _unclassified_local = is_local_validation_error and classified.reason == FailoverReason.unknown - if classified.should_fallback or _unclassified_local or shrink_spent: + if classified.should_fallback or _unclassified_local or shrink_spent or reasoning_spent: # Announce the fallback only when a chain exists, else "trying fallback..." lies # before a silent abort. if agent._has_pending_fallback(): diff --git a/agent/turn_recovery.py b/agent/turn_recovery.py index 7d4a4b949e..008b3b3649 100644 --- a/agent/turn_recovery.py +++ b/agent/turn_recovery.py @@ -596,11 +596,13 @@ def recover_after_classification( "messages with image parts found; surfacing original error." ) - # Reasoning-mandatory route (Nous Portal / OpenRouter, e.g. GLM-5.3) 400s on - # ``reasoning: {enabled: false}``. The catalog guard in the provider profile normally swallows - # the disable, but a process that warmed its caps cache before the route flipped keeps sending - # it. One-shot: never send a disable again this session (the wire builder omits it → upstream - # default thinking), queue a catalog refresh so the guard is right next time, retry. + # Route rejecting a reasoning disable: a reasoning-mandatory route (Nous Portal / OpenRouter, + # e.g. GLM-5.3) 400s on ``reasoning: {enabled: false}``; a chat-only OpenAI-compatible relay + # 400s on the ``reasoning_effort: none`` the title/continuation disable projects (#114460). + # The catalog guard in the provider profile normally swallows the first, but a process that + # warmed its caps cache before the route flipped keeps sending it. One-shot: never send a + # disable again this session (the wire builder omits it → route default), queue a catalog + # refresh so the guard is right next time (no-op for providers without a catalog), retry. if ( classified.reason == FailoverReason.reasoning_mandatory and not _retry.reasoning_mandatory_retry_attempted @@ -612,8 +614,8 @@ def recover_after_classification( refresh_reasoning_caps_async(agent.provider) except Exception: pass - _vlines(agent, f"⚠️ {agent.model} requires reasoning — thinking stays on for this session, retrying...") - logger.warning("%sReasoning-mandatory recovery: dropping reasoning disable for %s", agent.log_prefix, agent.model) + _vlines(agent, f"⚠️ {agent.model} rejects disabling reasoning — using the route's default for this session, retrying...") + logger.warning("%sReasoning-disable recovery: dropping reasoning disable for %s", agent.log_prefix, agent.model) return True, recovered_with_pool # Provider rejected the image bytes; shrinking can't help, so strip image parts. diff --git a/tests/agent/test_auxiliary_parameter_rung_chaining.py b/tests/agent/test_auxiliary_parameter_rung_chaining.py index 0f613b18b6..e588ccdc07 100644 --- a/tests/agent/test_auxiliary_parameter_rung_chaining.py +++ b/tests/agent/test_auxiliary_parameter_rung_chaining.py @@ -69,22 +69,16 @@ def test_primary_rungs_chain_in_provider_order_and_strip_each_field_once(): def test_reasoning_effort_none_unsupported_reversed_wording(): - """#114460: providers that put the adjective last - (``reasoning_effort 'none' unsupported; ...``) must still fire the - strip-and-retry rung.""" - msg = ( - "Error code: 400 - " - "reasoning_effort 'none' unsupported; use minimal|low|medium|high|xhigh" + """Relays that put the adjective last (``reasoning_effort 'none' unsupported; use ...``) fire the + strip-and-retry rung like the forward wordings do; route gating that merely names a thinking + model, or an adjective-only "unsupported" far from any reasoning field, does not.""" + assert _is_reasoning_field_rejection( + _Bad400("Error code: 400 - reasoning_effort 'none' unsupported; use minimal|low|medium|high|xhigh") ) - assert _is_reasoning_field_rejection(_Bad400(msg)) - - -def test_reasoning_model_route_gating_still_not_a_field_rejection(): - """Route gating that merely names a thinking model is not a wire-field - rejection and must not strip.""" assert not _is_reasoning_field_rejection( _Bad400("The model kimi-k2-thinking is not supported when using this account") ) + assert not _is_reasoning_field_rejection(_Bad400("reasoning models: tool_choice 'required' is unsupported")) def test_fallback_candidate_recovers_from_rejected_temperature(): diff --git a/tests/agent/test_error_classifier.py b/tests/agent/test_error_classifier.py index 44c7db3076..963fab64c0 100644 --- a/tests/agent/test_error_classifier.py +++ b/tests/agent/test_error_classifier.py @@ -891,27 +891,23 @@ class TestClassifyApiError: assert result.should_fallback is False assert result.should_compress is False - def test_reasoning_effort_none_unsupported_wording_is_reasoning_mandatory(self): - """Reversed wording rejecting a reasoning disable (#114460): the route mandates - reasoning, so the loop must drop the disable and retry, not abort as format_error.""" - e = MockAPIError( - "Error code: 400 - reasoning_effort 'none' unsupported; " - "use minimal|low|medium|high|xhigh", - status_code=400, + def test_reasoning_field_rejection_is_reasoning_mandatory(self): + """A 400 rejecting a reasoning wire control by name — reversed ("reasoning_effort 'none' + unsupported; use ...", #114460) or forward ("Unrecognized request argument supplied: + reasoning_effort") — takes the drop-the-disable rung, not the format_error abort; a + model-id segment (kimi-k2-thinking) stays route gating.""" + for msg in ( + "Error code: 400 - reasoning_effort 'none' unsupported; use minimal|low|medium|high|xhigh", + "Unrecognized request argument supplied: reasoning_effort", + ): + result = classify_api_error(MockAPIError(msg, status_code=400), provider="custom", model="m") + assert result.reason == FailoverReason.reasoning_mandatory, msg + assert result.retryable is True and result.should_fallback is False + gated = classify_api_error( + MockAPIError("The model kimi-k2-thinking is not supported when using this account", status_code=400), + provider="custom", model="kimi-k2-thinking", ) - result = classify_api_error(e, provider="custom", model="halogen-qwen3.8-flash-next") - assert result.reason == FailoverReason.reasoning_mandatory - assert result.retryable is True - assert result.should_fallback is False - - def test_reasoning_model_route_gating_is_not_reasoning_mandatory(self): - """Model-id segments (kimi-k2-thinking) stay route gating, never disable rejection.""" - e = MockAPIError( - "The model kimi-k2-thinking is not supported when using this account", - status_code=400, - ) - result = classify_api_error(e, provider="custom", model="kimi-k2-thinking") - assert result.reason != FailoverReason.reasoning_mandatory + assert gated.reason != FailoverReason.reasoning_mandatory # ── Provider-specific: llama.cpp grammar-parse ── diff --git a/tests/agent/test_reasoning_disable_recovery.py b/tests/agent/test_reasoning_disable_recovery.py new file mode 100644 index 0000000000..163fae876e --- /dev/null +++ b/tests/agent/test_reasoning_disable_recovery.py @@ -0,0 +1,106 @@ +"""Main-loop recovery for a route that rejects a reasoning disable by field name (#114460). + +The classifier maps the rejection to ``reasoning_mandatory``; ``recover_after_classification`` +drops the disable once; a SECOND rejection in the same turn means the configured reasoning +controls themselves are refused, so the settle path takes the fallback chain instead of +replaying the identical request until ``max_retries``. +""" +from types import SimpleNamespace +from unittest.mock import patch + +from agent.error_classifier import FailoverReason, classify_api_error + +_REVERSED_400 = "reasoning_effort 'none' unsupported; use minimal|low|medium|high|xhigh" + + +class _FakeApiError(Exception): + def __init__(self, status_code, message): + super().__init__(f"Error code: {status_code} - {{'error': {{'message': {message!r}}}}}") + self.status_code = status_code + self.message = message + self.body = {"error": {"message": message, "type": "invalid_request_error"}} + + +class _Agent: + log_prefix = "" + verbose = False + provider = "custom" + model = "chat-only" + _fallback_chain = [object()] + _fallback_index = 0 + _credential_pool = None + + def __init__(self): + self.activated = [] + self._reasoning_disable_rejected = False + + def _recover_with_credential_pool(self, **kwargs): + return False, False + + def _has_pending_fallback(self): + return True + + def _try_activate_fallback(self, **kwargs): + self.activated.append(True) + return True + + def _summarize_api_error(self, error): + return str(error) + + def __getattr__(self, name): + return lambda *args, **kwargs: None + + +def _settle(disable_drop_attempted): + from agent.turn_api_error import settle_unrecovered_error + + agent = _Agent() + err = _FakeApiError(400, _REVERSED_400) + classified = classify_api_error(err, provider="custom", model=agent.model) + assert classified.reason == FailoverReason.reasoning_mandatory + retry = SimpleNamespace( + reasoning_mandatory_retry_attempted=disable_drop_attempted, image_shrink_retry_attempted=False, + copilot_stale_cred_retry_attempted=False, primary_recovery_attempted=False, + restart_with_redirected_messages=False, + ) + with patch("agent.conversation_loop._is_copilot_provider", lambda a: False), patch( + "agent.conversation_loop._arm_fallback_restart", lambda agent, msgs, prompt, retry: prompt + ), patch("agent.turn_api_error.compute_error_backoff", lambda *a, **k: 0), patch( + "agent.turn_api_error.interruptible_backoff_sleep", lambda *a, **k: None + ): + verdict = settle_unrecovered_error( + agent, api_error=err, classified=classified, _retry=retry, status_code=400, error_msg=str(err), + is_context_length_error=False, is_rate_limited=False, _is_zai_coding_overload=False, + _provider="custom", _base="http://relay.example/v1", _model=agent.model, messages=[], + api_messages=[], api_kwargs={}, active_system_prompt="", conversation_history=None, + approx_tokens=10, retry_count=0, max_retries=3, compression_attempts=0, api_call_count=1, + ) + return agent, verdict + + +def test_spent_disable_drop_falls_back_instead_of_replaying_the_request(): + agent, verdict = _settle(disable_drop_attempted=True) + assert verdict.action == "break" + assert agent.activated == [True] + + # Control: before the drop has run the verdict stays retryable (the rung gets its shot). + agent, verdict = _settle(disable_drop_attempted=False) + assert verdict.action == "fallthrough" + assert agent.activated == [] + + +def test_disable_drop_rung_sets_session_flag_once(): + from agent.turn_recovery import recover_after_classification + from agent.turn_retry_state import TurnRetryState + + agent = _Agent() + err = _FakeApiError(400, _REVERSED_400) + classified = classify_api_error(err, provider="custom", model=agent.model) + retry = TurnRetryState() + with patch("agent.conversation_loop._is_nous_inference_route", lambda *a, **k: False): + first, _ = recover_after_classification( + agent, err, classified, retry, status_code=400, error_context=None, messages=[], api_messages=[]) + second, _ = recover_after_classification( + agent, err, classified, retry, status_code=400, error_context=None, messages=[], api_messages=[]) + assert first is True and agent._reasoning_disable_rejected is True + assert second is False diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index d721ce8542..3270156395 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -1413,7 +1413,7 @@ Auxiliary task blocks additionally accept a `reasoning_effort` knob: This is the per-task counterpart of the global `agent.reasoning_effort`: run compression at `low` or vision at `none` to cut side-task latency and cost when your main model is an expensive reasoning model, without touching your main chat behavior. It applies to auxiliary-client tasks such as `vision`, `compression`, `title_generation`, and `curator`, across all three auxiliary wire formats (chat completions, Codex Responses, Anthropic Messages). An explicit `extra_body.reasoning` on the same task wins over the shorthand. A caller that turns thinking off for its own call (title generation does — a 64-token title has no room for reasoning) wins over both: the task-level effort is dropped for that request instead of being sent beside the provider's thinking-off field. -If the endpoint rejects the reasoning field outright (a chat-only model behind an OpenAI-compatible relay answering `400 Unrecognized request argument supplied: reasoning_effort`), the auxiliary call is retried once with every reasoning field omitted, so the task (for example the session title) still completes with the endpoint's default behaviour. +If the endpoint rejects the reasoning field outright (a chat-only model behind an OpenAI-compatible relay answering `400 Unrecognized request argument supplied: reasoning_effort`, or the reversed wording `400 reasoning_effort 'none' unsupported; use minimal|low|medium|high|xhigh`), the auxiliary call is retried once with every reasoning field omitted, so the task (for example the session title) still completes with the endpoint's default behaviour. The main conversation applies the same recovery: when a route rejects the reasoning-off request Hermes sends for a thinking-only truncated continuation, the disable is dropped for the rest of the session and the request is retried with the route's default. **Background review is different:** a same-model review fork always inherits the parent's reasoning effort. `auxiliary.background_review.reasoning_effort` is ignored on that path, including when the parent provider/model is explicitly selected. This preserves byte-identical reasoning settings, system prompt, full conversation snapshot, and tool definitions for prompt-cache parity; there is no independent-effort switch for same-model reviews. See [background review reasoning](/user-guide/features/memory#same-model-review-reasoning). When the review is routed to a different provider/model, `reasoning_effort` applies to that routed fork (unset = the routed provider's default). Hermes prints a one-time warning when the key is set but the review runs on the main model.