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",