fix: set failed=True for repeated_outer_errors exit + drop append_message
Follow-up to @BrunoBza's #93062: 1. Set failed=True only for the new repeated_outer_errors exit reason. Previously the error exit left failed=False, so finalize_turn reported completed=True for a turn that actually failed — incorrect. 2. Don't append_message the assistant response at the break. A thinking- prefill or interim assistant may already be the tail, and appending would create assistant→assistant role-alternation violation. finalize_turn (lines 341-353) handles this safely by checking _tail_role != 'assistant' before appending. 3. Update test to assert failed=True and completed=False for the repeated_outer_errors exit.
This commit is contained in:
@@ -8512,14 +8512,18 @@ def run_conversation(
|
||||
_turn_exit_reason = f"local_processing_error({error_msg[:80]})"
|
||||
final_response = f"I apologize, but I encountered an error while processing the model response: {error_msg}"
|
||||
elif _outer_error_count >= _outer_error_cap:
|
||||
failed = True
|
||||
_turn_exit_reason = f"repeated_outer_errors({error_msg[:80]})"
|
||||
final_response = f"I apologize, but I encountered repeated errors: {error_msg}"
|
||||
else:
|
||||
_turn_exit_reason = f"error_near_max_iterations({error_msg[:80]})"
|
||||
final_response = f"I apologize, but I encountered repeated errors: {error_msg}"
|
||||
# Append as assistant so the history stays valid for
|
||||
# session resume (avoids consecutive user messages).
|
||||
append_message(messages, {"role": "assistant", "content": final_response})
|
||||
# Don't append the assistant message here — a thinking-prefill
|
||||
# or interim assistant may already be the tail, and appending
|
||||
# would create assistant→assistant. finalize_turn handles
|
||||
# this case safely (lines 341-353: appends only when
|
||||
# _tail_role != "assistant"), matching the pattern at other
|
||||
# break sites that set final_response without appending.
|
||||
break
|
||||
|
||||
# Post-loop turn finalization extracted to agent/turn_finalizer.finalize_turn
|
||||
|
||||
@@ -103,9 +103,9 @@ class TestOuterErrorRetryBound:
|
||||
result = loop_agent.run_conversation("hello")
|
||||
|
||||
# The bound path returns an apology as final_response with
|
||||
# failed=False (same contract as the legacy near-limit exit), so the
|
||||
# failed=True (the turn did not complete successfully), so the
|
||||
# meaningful assertions are: bounded call count + dedicated exit
|
||||
# reason + apology text.
|
||||
# reason + apology text + failed flag.
|
||||
assert result["api_calls"] <= 8, (
|
||||
"The loop must give up within the per-turn error bound instead "
|
||||
"of retrying an unlimited-budget failure forever."
|
||||
@@ -114,6 +114,8 @@ class TestOuterErrorRetryBound:
|
||||
f"unexpected exit reason: {result['turn_exit_reason']}"
|
||||
)
|
||||
assert "repeated errors" in (result["final_response"] or "")
|
||||
assert result["failed"] is True
|
||||
assert result["completed"] is False
|
||||
|
||||
def test_recovery_after_failures_completes_normally(self, loop_agent):
|
||||
"""Early escaping failures followed by success must NOT trip the new
|
||||
|
||||
Reference in New Issue
Block a user