From 02692cd64ab18b34bfbc15ba84d748ced204e537 Mon Sep 17 00:00:00 2001 From: b3nw Date: Mon, 6 Jul 2026 23:19:46 +0000 Subject: [PATCH] fix: surface agent's graceful summary on iteration-limit exhaustion (closes #5494) The WebUI already surfaces the model-generated summary when the agent's tool-iteration budget is exhausted. The remaining gap: when the agent hits the limit but its summary call yields no usable assistant content (common with reasoning-only responses), the user sees a bare `tool_limit_reached` error instead of any closure text. `hermes-agent.handle_max_iterations()` always returns a non-empty `final_response` (either the summary or a graceful fallback), and `AIAgent` exposes that string on the result dict. The WebUI, by contrast, only reads `result['messages']`, so an empty summary left no final assistant answer and routed to the error path. Inject `result['final_response']` as a final assistant turn when `max_iterations_reached` fires and no usable answer exists in `messages`. The existing `_mark_latest_assistant_tool_limit_status` flow then attaches the status card and the user sees the closure text instead of the bare error. Hermes-agent parity preserved (it's literally the same string); the bare-error path stays for the case where the agent died entirely (no fallback either). Tests cover: fallback-injection surfaces closure text + status card, no double-injection when a real summary already exists, the existing 'died entirely' apperror path is unchanged, and three unit cases for the helper's skip conditions. Co-authored-by: b3nw --- api/streaming.py | 48 ++++++++++ tests/test_tool_limit_terminal_state.py | 117 ++++++++++++++++++++++++ 2 files changed, 165 insertions(+) diff --git a/api/streaming.py b/api/streaming.py index 6eeb3892..7a6e6209 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -1389,6 +1389,40 @@ def _agent_result_tool_limit_reached(result) -> bool: return False +def _maybe_inject_max_iteration_summary_fallback(messages, result) -> list: + """Append the agent's graceful summary text as an assistant turn when one is missing. + + When ``AIAgent`` exhausts its iteration budget, ``agent.handle_max_iterations`` + always returns a non-empty ``final_response`` — either the model-generated + summary or a graceful fallback (e.g. ``"I reached the iteration limit and + couldn't generate a summary."``). Hermes Agent surfaces that string as the + final answer to the user; the WebUI, by contrast, reads only ``messages``, + so an empty summary (common with reasoning-only responses) left the user + with a bare ``tool_limit_reached`` error instead of any closure text. + + When ``_tool_limit_reached`` is true and ``messages`` ends without a final + assistant answer, inject ``result['final_response']`` as a new assistant + turn so ``_mark_latest_assistant_tool_limit_status`` can attach the status + card in the normal flow and the user sees the same closure text as + hermes-agent. Returns the (possibly new) messages list; does nothing when + a usable assistant answer already exists or when ``result`` carries no + graceful fallback text. + """ + if not isinstance(result, dict): + return list(messages or []) + fallback = result.get('final_response') + if not isinstance(fallback, str) or not fallback.strip(): + return list(messages or []) + out = list(messages or []) + if not _session_lacks_final_assistant_answer(out): + return out + # Append a synthetic summary turn. Tag it so downstream consumers can + # distinguish it from model-emitted assistant turns if needed; mirrors the + # synthetic-scaffolding flag convention already used elsewhere (#5334). + out.append({"role": "assistant", "content": fallback, "_max_iteration_summary_fallback": True}) + return out + + def _mark_latest_assistant_tool_limit_status(messages) -> bool: """Annotate the latest usable assistant final answer as limit-stopped.""" for msg in reversed(list(messages or [])): @@ -8239,6 +8273,20 @@ def _run_agent_streaming( _result_messages, enabled=_tool_limit_reached, ) + # #5494 — parity with hermes-agent's handle_max_iterations() return + # value. When the agent produced no usable summary assistant + # message but result['final_response'] carries a graceful fallback + # string, inject it as a final assistant turn so the user sees + # closure text instead of a bare tool_limit_reached error. Apply + # the synthesis to result['messages'] AND _result_messages so + # downstream _all_result_messages checks (silent-failure detection + # at #all_result_messages on the same turn) see the fallback too. + if _tool_limit_reached: + _result_messages = _maybe_inject_max_iteration_summary_fallback( + _result_messages, result + ) + if isinstance(result, dict) and result.get('messages') is not None: + result = {**result, 'messages': _result_messages} if cancel_event.is_set(): _finalize_cancelled_turn(s, ephemeral=False) try: diff --git a/tests/test_tool_limit_terminal_state.py b/tests/test_tool_limit_terminal_state.py index 80e4cd38..1a2becdc 100644 --- a/tests/test_tool_limit_terminal_state.py +++ b/tests/test_tool_limit_terminal_state.py @@ -280,6 +280,123 @@ def test_streaming_tool_limit_without_final_answer_emits_no_final_apperror(tmp_p ) +def test_streaming_tool_limit_with_fallback_final_response_surfaces_closure_text(tmp_path, monkeypatch): + """#5494 — when the agent produced no usable summary but returned a graceful + fallback string (handle_max_iterations() always returns one), inject that as a + final assistant turn so the user sees closure text instead of a bare error. + """ + graceful = "I reached the iteration limit and couldn't generate a summary." + result = { + "turn_exit_reason": "max_iterations_reached(30/30)", + "final_response": graceful, + "messages": [ + {"role": "user", "content": "Do the long task."}, + {"role": "assistant", "content": "", "tool_calls": [{"id": "call_1"}]}, + {"role": "tool", "tool_call_id": "call_1", "content": "result"}, + {"role": "user", "content": streaming._MAX_ITERATION_SUMMARY_REQUEST}, + ], + } + + events, payload = _run_streaming_with_fake_agent(tmp_path, monkeypatch, result) + + # The user sees the graceful fallback, not a bare tool_limit_reached error. + assert not [ + ap for ev, ap in events if ev == "apperror" + and ap.get("type") == "tool_limit_reached" + ], "expected no tool_limit_reached apperror when fallback was returned" + done_payloads = [payload for event, payload in events if event == "done"] + assert done_payloads, "expected done SSE payload" + assert done_payloads[-1]["terminal_state"] == "tool_limit_reached" + + # Fallback text is shown as a final assistant message and is annotated + # with the status card so the UI can render the 'limit reached' chip. + assistant = payload["messages"][-1] + assert assistant["role"] == "assistant" + assert assistant["content"] == graceful + assert assistant["_terminal_state"] == "tool_limit_reached" + assert assistant["_statusCard"]["title"] == "Tool iteration limit reached" + # Synthetic scaffolding turn was still dropped, even after fallback injection. + assert all( + message.get("content") != streaming._MAX_ITERATION_SUMMARY_REQUEST + for message in payload["messages"] + ) + + +def test_streaming_tool_limit_with_fallback_does_not_double_inject_when_assistant_exists(tmp_path, monkeypatch): + """#5494 — when the agent already appended a model-generated summary, the + fallback flow is a no-op; the existing summary is preserved as before. + """ + summary = "I reached the limit; here is the summary." + result = { + "turn_exit_reason": "max_iterations_reached(30/30)", + "final_response": summary, + "messages": [ + {"role": "user", "content": "Do the long task."}, + {"role": "assistant", "content": "", "tool_calls": [{"id": "call_1"}]}, + {"role": "tool", "tool_call_id": "call_1", "content": "result"}, + {"role": "user", "content": streaming._MAX_ITERATION_SUMMARY_REQUEST}, + {"role": "assistant", "content": summary}, + ], + } + + events, payload = _run_streaming_with_fake_agent(tmp_path, monkeypatch, result) + + done_payloads = [payload for event, payload in events if event == "done"] + assert done_payloads + assistant_msgs = [ + m for m in payload["messages"] + if m.get("role") == "assistant" and m.get("content") == summary + ] + assert len(assistant_msgs) == 1, "fallback must not duplicate the existing summary" + assert assistant_msgs[0]["_terminal_state"] == "tool_limit_reached" + + +def test_maybe_inject_max_iteration_summary_fallback_unit(): + """Unit-level coverage for the injection helper.""" + messages = [ + {"role": "user", "content": "Do the long task."}, + {"role": "assistant", "content": "", "tool_calls": [{"id": "call_1"}]}, + {"role": "tool", "tool_call_id": "call_1", "content": "result"}, + ] + graceful = "I reached the iteration limit and couldn't generate a summary." + result = {"final_response": graceful} + + injected = streaming._maybe_inject_max_iteration_summary_fallback(messages, result) + + assert injected[-1]["role"] == "assistant" + assert injected[-1]["content"] == graceful + assert injected[-1]["_max_iteration_summary_fallback"] is True + + +def test_maybe_inject_max_iteration_summary_fallback_skips_when_assistant_present(): + messages = [ + {"role": "user", "content": "Do the long task."}, + {"role": "assistant", "content": "real summary"}, + ] + result = {"final_response": "fallback text"} + + out = streaming._maybe_inject_max_iteration_summary_fallback(messages, result) + assert out == messages + + +def test_maybe_inject_max_iteration_summary_fallback_skips_when_no_fallback(): + messages = [ + {"role": "user", "content": "Do the long task."}, + {"role": "assistant", "content": "", "tool_calls": [{"id": "call_1"}]}, + ] + + out = streaming._maybe_inject_max_iteration_summary_fallback(messages, {}) + assert out == messages + + out = streaming._maybe_inject_max_iteration_summary_fallback( + messages, {"final_response": " "} + ) + assert out == messages + + out = streaming._maybe_inject_max_iteration_summary_fallback(messages, None) + assert out == messages + + def test_streaming_tool_limit_terminal_failure_does_not_mark_final_answer(tmp_path, monkeypatch): result = { "status": "partial",