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 <b3nw@users.noreply.github.com>
This commit is contained in:
1 parent
c4a3d4f816
commit
02692cd64a
2 files changed
+165
No files matched your search
@@ -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:
|
||||
|
||||
@@ -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",
|
||||
|
||||
Reference in new issue
Block a user