From b23f31c2bd4977fa49f493e38816b7e70c666a55 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sat, 19 Sep 2026 01:15:24 -0700 Subject: [PATCH] fix: cover the attempt-line verdict through handle_api_error The retryable kwarg only reached log_api_error_attempt from turn_api_error.handle_api_error, yet both tests called the helper directly, so dropping the forwarding line kept the suite green. Replace the retryable=True direct-call test with one that drives the production entry (classifier patched to a non-retryable 401) and asserts 'attempt 1/3, not retryable' on the log and status buffer: red with the kwarg removed, green with it restored. Also note why _touch_activity keeps the plain counter: it is a watchdog liveness label, not chat output. --- agent/turn_api_error.py | 2 ++ .../agent/test_turn_recovery_attempt_line.py | 34 +++++++++++++++---- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/agent/turn_api_error.py b/agent/turn_api_error.py index aeba470219..b19dd9ecf0 100644 --- a/agent/turn_api_error.py +++ b/agent/turn_api_error.py @@ -136,6 +136,8 @@ def handle_api_error( retry_count += 1 elapsed_time = time.time() - api_start_time + # Liveness/watchdog label only (never shown in chat), so the classifier's + # "not retryable" verdict is named on the logged attempt line below instead. agent._touch_activity(f"API error recovery (attempt {retry_count}/{max_retries})") error_type, error_msg, _provider, _base, _model = log_api_error_attempt( diff --git a/tests/agent/test_turn_recovery_attempt_line.py b/tests/agent/test_turn_recovery_attempt_line.py index f2e08daf1f..0c841ef065 100644 --- a/tests/agent/test_turn_recovery_attempt_line.py +++ b/tests/agent/test_turn_recovery_attempt_line.py @@ -7,7 +7,8 @@ counter that failed to advance (#73237). The classifier's verdict now rides the same line on both surfaces (logger + buffered status trace).""" import logging -from unittest.mock import MagicMock +import time +from unittest.mock import MagicMock, patch from agent.turn_recovery import log_api_error_attempt @@ -37,9 +38,30 @@ def test_non_retryable_failure_is_named_on_log_and_status_line(caplog): assert "not retryable" in agent._buffer_vprint.call_args_list[0].args[0] -def test_retryable_failure_keeps_the_plain_attempt_counter(caplog): +def test_production_entry_forwards_the_classifier_verdict_to_the_attempt_line(caplog): + """``handle_api_error`` must hand ``classified.retryable`` to the log line; a bare + ``attempt 1/3`` there is the exact regression #73237 reported.""" + from types import SimpleNamespace + + from agent.error_classifier import ClassifiedError, FailoverReason + from agent.turn_api_error import handle_api_error + agent = _agent() - with caplog.at_level(logging.WARNING, logger="agent.conversation_loop"): - _call(agent, retryable=True) - assert "(attempt 1/3)" in caplog.text - assert "not retryable" not in caplog.text + agent._interrupt_requested = True # leave the loop right after the attempt line + agent.clear_interrupt.return_value = True + verdict = ClassifiedError(reason=FailoverReason.auth, status_code=401, retryable=False) + with patch("agent.turn_api_error.classify_api_error", return_value=verdict), patch( + "agent.turn_api_error.recover_before_classification", return_value=(False, "sys") + ), patch( + "agent.turn_api_error.recover_after_classification", return_value=(False, False) + ), caplog.at_level(logging.WARNING, logger="agent.conversation_loop"): + out = handle_api_error( + agent, api_error=RuntimeError("401"), _retry=SimpleNamespace(), thinking_spinner=None, + messages=[], api_messages=[], api_kwargs={}, system_message=None, + active_system_prompt="sys", conversation_history=[], approx_tokens=10, retry_count=0, + max_retries=3, compression_attempts=0, max_compression_attempts=1, api_call_count=1, + api_request_id="r", api_start_time=time.time(), effective_task_id=None, turn_id="t", + ) + assert out.action == "break" + assert "attempt 1/3, not retryable" in caplog.text + assert "not retryable" in agent._buffer_vprint.call_args_list[0].args[0]