Skip to content

fix(agent): explain abnormal turn endings instead of blank/partial reply - #34470

Closed
Bartok9 wants to merge 2 commits into
NousResearch:mainfrom
Bartok9:fix/turn-completion-explainer-34452
Closed

fix(agent): explain abnormal turn endings instead of blank/partial reply#34470
Bartok9 wants to merge 2 commits into
NousResearch:mainfrom
Bartok9:fix/turn-completion-explainer-34452

Conversation

@Bartok9

@Bartok9 Bartok9 commented May 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • When a turn ends abnormally after tools (empty content after retries, a partial/truncated stream, exhausted retries, or an iteration/budget limit), surface a single user-visible explanation of why the reply stopped instead of leaving a blank or fragmentary response box.
  • Normal successful short replies (e.g. Done.) stay completely quiet.

Motivation

Closes #34452.

After substantive tool calls (read_file, patch, …) the turn could end with no usable assistant reply or only a fragment (e.g. The). The internal turn_exit_reason values (empty_response_exhausted, partial_stream_recovery, all_retries_exhausted_no_response, max_iterations_reached, …) already exist but were never explained to the user at end of turn — the failure was silent from the UI's perspective, and the user couldn't tell whether to send continue, switch model/provider, or inspect tool output.

The issue explicitly asks for behavior "similar in spirit to the file-mutation verifier footer, but for turn-completion failures." This PR follows that existing pattern exactly:

  • New _format_turn_completion_explanation(turn_exit_reason) maps an abnormal reason to a short, actionable message. It returns "" for normal text_response(...) exits and for diagnostic-only reasons (unknown, guardrail_halt — which already surfaces its own message), so healthy turns are never annotated.
  • At turn end in agent/conversation_loop.py, right after the file-mutation verifier footer, when there is no genuinely usable reply this turn — an empty response, the (empty) terminal sentinel, or a suspiciously short partial fragment (≤24 chars without sentence-ending punctuation, and not a text_response exit) — the explanation either replaces the bare (empty) sentinel or is appended after a partial fragment so the user sees both what arrived and why it stopped.
  • Gated by display.turn_completion_explainer (default True) with HERMES_TURN_COMPLETION_EXPLAINER env override, mirroring _file_mutation_verifier_enabled so gateway and CLI share one setting.

Verification

  • python3 -m pytest tests/run_agent/test_turn_completion_explainer.py — 11 passed (formatter map, enable/disable seam, end-to-end empty-exhausted surfaces the explanation, normal Done. stays quiet).
  • python3 -m pytest tests/run_agent/test_run_agent.py tests/run_agent/test_tool_call_guardrail_runtime.py — 362 passed. Five pre-existing tests that asserted the bare final_response == "(empty)" sentinel were updated to assert the new user-visible explanation (the (empty) sentinel was precisely the silent-failure symptom this issue reports); all other assertions in those tests (api-call counts, status emissions, completed) are unchanged.
  • Did NOT change: streaming/recovery paths, the empty-response retry/fallback machinery, or the turn_exit_reason values themselves — only their end-of-turn presentation when no usable reply exists.

Differential value (related, not duplicate)

Existing open PRs address adjacent but different failure modes:

None of them produce the consolidated end-of-turn explanation for the empty/exhausted/iteration-limit cases that #34452 asks for. This PR is complementary: it explains the genuinely-no-reply outcome rather than recovering or delivering content.

When a turn ends abnormally after substantive tool calls (empty content
after retries, a partial/truncated stream, exhausted retries, or an
iteration/budget limit), the CLI/TUI response area was left blank or
showed only a fragment (e.g. "The") with no consolidated reason. The
internal turn_exit_reason values (empty_response_exhausted,
partial_stream_recovery, etc.) were never surfaced to the user.

Add a turn-completion explainer that mirrors the existing file-mutation
verifier footer: at turn end, map an abnormal turn_exit_reason to a
short, actionable message and either replace the bare "(empty)"
sentinel or append the reason after a partial fragment. Normal
text_response exits (e.g. a terse "Done.") stay quiet.

Gated by display.turn_completion_explainer (default on) with
HERMES_TURN_COMPLETION_EXPLAINER env override, matching the
file-mutation verifier seam.

Closes NousResearch#34452
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard labels May 29, 2026
PR NousResearch#34470 adds an explainer suffix to abnormal turn endings (e.g.
max_iterations_reached) so users see why the response is short instead
of receiving a bare/blank reply. test_tool_call_validation_accepts_dict_arguments
runs the agent at max_iterations=3 which hits the explainer path; the
existing strict-equality assertion (== "done") no longer matches once
the suffix is appended.

Switch the assertion to .startswith("done") so the test continues to
verify that the models actual text survives intact while leaving the
explainer suffix wording owned by conversation_loop (where it belongs).

Test now passes (1 passed in 0.88s).
teknium1 pushed a commit that referenced this pull request May 30, 2026
PR #34470 adds an explainer suffix to abnormal turn endings (e.g.
max_iterations_reached) so users see why the response is short instead
of receiving a bare/blank reply. test_tool_call_validation_accepts_dict_arguments
runs the agent at max_iterations=3 which hits the explainer path; the
existing strict-equality assertion (== "done") no longer matches once
the suffix is appended.

Switch the assertion to .startswith("done") so the test continues to
verify that the models actual text survives intact while leaving the
explainer suffix wording owned by conversation_loop (where it belongs).

Test now passes (1 passed in 0.88s).
teknium1 pushed a commit that referenced this pull request May 30, 2026
PR #34470 adds an explainer suffix to abnormal turn endings (e.g.
max_iterations_reached) so users see why the response is short instead
of receiving a bare/blank reply. test_tool_call_validation_accepts_dict_arguments
runs the agent at max_iterations=3 which hits the explainer path; the
existing strict-equality assertion (== "done") no longer matches once
the suffix is appended.

Switch the assertion to .startswith("done") so the test continues to
verify that the models actual text survives intact while leaving the
explainer suffix wording owned by conversation_loop (where it belongs).

Test now passes (1 passed in 0.88s).
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #34848 — your commits were cherry-picked onto current main with your authorship preserved in git log (59b0ea9, de6d602). Thanks for the clean implementation following the file-mutation-verifier pattern; we added the DEFAULT_CONFIG key registration and shortened the footer prefix on top. #34848

@teknium1 teknium1 closed this May 30, 2026
KKT-OPT pushed a commit to KKT-OPT/hermes-agent that referenced this pull request May 31, 2026
PR NousResearch#34470 adds an explainer suffix to abnormal turn endings (e.g.
max_iterations_reached) so users see why the response is short instead
of receiving a bare/blank reply. test_tool_call_validation_accepts_dict_arguments
runs the agent at max_iterations=3 which hits the explainer path; the
existing strict-equality assertion (== "done") no longer matches once
the suffix is appended.

Switch the assertion to .startswith("done") so the test continues to
verify that the models actual text survives intact while leaving the
explainer suffix wording owned by conversation_loop (where it belongs).

Test now passes (1 passed in 0.88s).
alt-glitch pushed a commit that referenced this pull request Jun 14, 2026
PR #34470 adds an explainer suffix to abnormal turn endings (e.g.
max_iterations_reached) so users see why the response is short instead
of receiving a bare/blank reply. test_tool_call_validation_accepts_dict_arguments
runs the agent at max_iterations=3 which hits the explainer path; the
existing strict-equality assertion (== "done") no longer matches once
the suffix is appended.

Switch the assertion to .startswith("done") so the test continues to
verify that the models actual text survives intact while leaving the
explainer suffix wording owned by conversation_loop (where it belongs).

Test now passes (1 passed in 0.88s).
T02200059 pushed a commit to T02200059/hermes-agent that referenced this pull request Jun 18, 2026
PR NousResearch#34470 adds an explainer suffix to abnormal turn endings (e.g.
max_iterations_reached) so users see why the response is short instead
of receiving a bare/blank reply. test_tool_call_validation_accepts_dict_arguments
runs the agent at max_iterations=3 which hits the explainer path; the
existing strict-equality assertion (== "done") no longer matches once
the suffix is appended.

Switch the assertion to .startswith("done") so the test continues to
verify that the models actual text survives intact while leaving the
explainer suffix wording owned by conversation_loop (where it belongs).

Test now passes (1 passed in 0.88s).
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
PR NousResearch#34470 adds an explainer suffix to abnormal turn endings (e.g.
max_iterations_reached) so users see why the response is short instead
of receiving a bare/blank reply. test_tool_call_validation_accepts_dict_arguments
runs the agent at max_iterations=3 which hits the explainer path; the
existing strict-equality assertion (== "done") no longer matches once
the suffix is appended.

Switch the assertion to .startswith("done") so the test continues to
verify that the models actual text survives intact while leaving the
explainer suffix wording owned by conversation_loop (where it belongs).

Test now passes (1 passed in 0.88s).
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
PR NousResearch#34470 adds an explainer suffix to abnormal turn endings (e.g.
max_iterations_reached) so users see why the response is short instead
of receiving a bare/blank reply. test_tool_call_validation_accepts_dict_arguments
runs the agent at max_iterations=3 which hits the explainer path; the
existing strict-equality assertion (== "done") no longer matches once
the suffix is appended.

Switch the assertion to .startswith("done") so the test continues to
verify that the models actual text survives intact while leaving the
explainer suffix wording owned by conversation_loop (where it belongs).

Test now passes (1 passed in 0.88s).
donbowman pushed a commit to donbowman/hermes-agent that referenced this pull request Jul 13, 2026
PR NousResearch#34470 adds an explainer suffix to abnormal turn endings (e.g.
max_iterations_reached) so users see why the response is short instead
of receiving a bare/blank reply. test_tool_call_validation_accepts_dict_arguments
runs the agent at max_iterations=3 which hits the explainer path; the
existing strict-equality assertion (== "done") no longer matches once
the suffix is appended.

Switch the assertion to .startswith("done") so the test continues to
verify that the models actual text survives intact while leaving the
explainer suffix wording owned by conversation_loop (where it belongs).

Test now passes (1 passed in 0.88s).
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
PR NousResearch#34470 adds an explainer suffix to abnormal turn endings (e.g.
max_iterations_reached) so users see why the response is short instead
of receiving a bare/blank reply. test_tool_call_validation_accepts_dict_arguments
runs the agent at max_iterations=3 which hits the explainer path; the
existing strict-equality assertion (== "done") no longer matches once
the suffix is appended.

Switch the assertion to .startswith("done") so the test continues to
verify that the models actual text survives intact while leaving the
explainer suffix wording owned by conversation_loop (where it belongs).

Test now passes (1 passed in 0.88s).
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
PR NousResearch#34470 adds an explainer suffix to abnormal turn endings (e.g.
max_iterations_reached) so users see why the response is short instead
of receiving a bare/blank reply. test_tool_call_validation_accepts_dict_arguments
runs the agent at max_iterations=3 which hits the explainer path; the
existing strict-equality assertion (== "done") no longer matches once
the suffix is appended.

Switch the assertion to .startswith("done") so the test continues to
verify that the models actual text survives intact while leaving the
explainer suffix wording owned by conversation_loop (where it belongs).

Test now passes (1 passed in 0.88s).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Agent turn ends with empty/partial reply after tools — user gets no explanation

3 participants