Merge pull request #5724 from nesquena/release/stage-5717
Release: iteration-limit graceful summary (#5717)
This commit is contained in:
nesquena-hermes
GitHub
commit
8fee7da3ee
3 changed files with 180 additions and 0 deletions
|
|
@ -5,6 +5,8 @@
|
|||
|
||||
### Fixed
|
||||
|
||||
- **The agent's closing summary now shows when it hits the iteration limit.** When a turn exhausted its tool/iteration budget, the agent produces a graceful closing message (a summary of what it got done, or a fallback like "I reached the iteration limit…"), but the WebUI showed a bare `tool_limit_reached` error card instead — most visible with reasoning-only models (extended-thinking Claude, DeepSeek, Codex Responses) whose summary arrives empty. The closing text is now surfaced as a proper assistant message. Purely additive and guarded — it never fires on a turn that already produced a final answer, never double-injects, and degrades to the old behavior when no fallback text is available. Thanks @claw-io (co-authored by @b3nw). (#5717, #5494)
|
||||
|
||||
- **The workspace file tree no longer jumps to the top when you expand a folder.** Every folder expand/collapse, breadcrumb navigation, refresh, and hidden-files toggle re-rendered the tree by wiping its container, which collapsed its height and made the browser reset the scroll position to the top — teleporting you away from where you were in a long tree. The scroll position is now captured before the re-render and restored after. Reported by @claw-io. (#5664, #5657)
|
||||
|
||||
- **Self-update recovers when a `.git/index.lock` is blocking it.** If "Update Now" failed because a stale `.git/index.lock` was present (a previous git process left it behind), the WebUI gave no in-UI way out — you were stuck on an un-updatable install. The update banner now detects that specific failure, shows the exact `rm -f …/.git/index.lock` command to run on the host, and offers a "Retry update" button that re-runs the normal update once the lock is gone. The server never deletes anything under `.git` itself (fail-closed — git's lock can't be safely auto-cleared without racing), so this is purely a detect-and-guide recovery. Thanks @b3nw. (#5688, #5687)
|
||||
|
|
|
|||
|
|
@ -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,24 @@ 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 the
|
||||
# downstream _all_result_messages checks (silent-failure detection
|
||||
# at api/streaming.py:_assistant_reply_added_after_current_turn)
|
||||
# see the fallback too. `finalize_turn` in the agent always returns
|
||||
# messages as a list, but we write back unconditionally so the
|
||||
# contract is "if we built a result-messages list, the silent-failure
|
||||
# classifier reads the augmented version."
|
||||
if _tool_limit_reached:
|
||||
_result_messages = _maybe_inject_max_iteration_summary_fallback(
|
||||
_result_messages, result
|
||||
)
|
||||
if isinstance(result, dict):
|
||||
result = {**result, 'messages': _result_messages}
|
||||
if cancel_event.is_set():
|
||||
_finalize_cancelled_turn(s, ephemeral=False)
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -280,6 +280,132 @@ 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 — handle_max_iterations() guarantees a non-empty ``final_response``
|
||||
on iteration-limit exhaustion. This test pins the WebUI contract that,
|
||||
when ``messages`` ends without a final assistant turn and ``final_response``
|
||||
is set, the user sees that closure text instead of a bare
|
||||
``tool_limit_reached`` error. Serves both as a live-bug fix pin and as a
|
||||
regression guard for the agent's "delivered final_response ⇒ assistant
|
||||
row" invariant: if a future agent regression drops that invariant, this
|
||||
test still passes because the WebUI honors the contract locally.
|
||||
"""
|
||||
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.
|
||||
# Either (a) we synthesized the fallback here, or (b) the agent guarantee
|
||||
# already added an assistant row and `_mark_latest_assistant_tool_limit_status`
|
||||
# attached the status card. Both routes satisfy the contract.
|
||||
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
|
||||
AND ``final_response`` carries the same text, the WebUI must not duplicate
|
||||
the assistant turn. Pins the no-op contract on the synthesis path.
|
||||
"""
|
||||
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",
|
||||
|
|
|
|||
Loading…
Reference in a new issue