From 95dee173e7e0724addc274d665e16156aa9d2dd0 Mon Sep 17 00:00:00 2001
From: teknium1 <127238744+teknium1@users.noreply.github.com>
Date: Fri, 18 Sep 2026 03:17:02 -0700
Subject: [PATCH] fix(agent): drop non-invariant disable-rung test; document
 thinking-state 400 trade-off (#114460)

test_disable_drop_rung_sets_session_flag_once stays green with agent/turn_recovery.py from
main (that hunk is comment/message text only), so it is not an invariant of this fix; the
spent-path test already carries the flag-false control. Docstring on
is_reasoning_field_rejection records the accepted trade-off for thinking-state 400s
("Function calling is not supported when thinking is enabled"): the marker sits 6 chars from
the token, so no proximity gate separates it from the forward wordings; cost is one
dropped-disable retry before the spent path falls back.
---
 agent/error_classifier.py                     |  7 ++++++-
 .../agent/test_reasoning_disable_recovery.py  | 20 ++-----------------
 2 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/agent/error_classifier.py b/agent/error_classifier.py
index eb0c7863e6..771631a479 100644
--- a/agent/error_classifier.py
+++ b/agent/error_classifier.py
@@ -464,7 +464,12 @@ def is_reasoning_field_rejection(error_msg: str) -> bool:
     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."""
+    model, so both the main loop and the auxiliary ladder retry once without the disable.
+
+    Known trade-off: a 400 about a thinking *state* ("Function calling is not supported when
+    thinking is enabled") also matches — the marker sits right next to the token, so no proximity
+    rule separates it from the forward wordings. Cost is one dropped-disable retry before the
+    spent path takes the fallback chain; the auxiliary ladder already treated it this way."""
     msg = (error_msg or "").lower()
     token = _REASONING_FIELD_TOKEN.search(msg)
     if token is None:
diff --git a/tests/agent/test_reasoning_disable_recovery.py b/tests/agent/test_reasoning_disable_recovery.py
index 163fae876e..081a1c087f 100644
--- a/tests/agent/test_reasoning_disable_recovery.py
+++ b/tests/agent/test_reasoning_disable_recovery.py
@@ -83,24 +83,8 @@ def test_spent_disable_drop_falls_back_instead_of_replaying_the_request():
     assert verdict.action == "break"
     assert agent.activated == [True]
 
-    # Control: before the drop has run the verdict stays retryable (the rung gets its shot).
+    # Control: before the drop has run the verdict stays retryable (the rung gets its shot;
+    # the one-shot drop itself is pre-existing ``turn_recovery`` behaviour, not asserted here).
     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