Skip to content

fix: surface exhausted empty model responses - #23505

Open
taivu1998 wants to merge 1 commit into
NousResearch:mainfrom
taivu1998:tdv/issue-17248-empty-response
Open

fix: surface exhausted empty model responses#23505
taivu1998 wants to merge 1 commit into
NousResearch:mainfrom
taivu1998:tdv/issue-17248-empty-response

Conversation

@taivu1998

Copy link
Copy Markdown

Summary

  • add explicit empty_response_exhausted metadata when the agent exhausts empty-response retries
  • keep the legacy final_response == "(empty)" sentinel for compatibility while preventing hooks, external memory sync, background reviews, CLI titles, and TTS from treating it as a real assistant answer
  • show a clear classic CLI message instead of rendering the raw (empty) sentinel
  • add regressions for replayed tool history, post-tool exhaustion cleanup, hook/memory skipping, successful nudge recovery, and CLI display normalization

Root Cause

Issue #17248 reports sessions that appear to complete with a literal (empty) answer after tool calls or empty model responses. The agent already had retry and cleanup scaffolding, but the terminal sentinel was indistinguishable from meaningful assistant content to downstream surfaces such as plugin hooks, memory sync, title generation, TTS, and CLI display.

Fix

This PR preserves the existing sentinel return value for compatibility, but marks the exhausted-empty path with explicit result metadata:

  • empty_response_exhausted: true
  • error_code: "empty_response_exhausted"
  • a short user-facing error string

The agent then gates post-response integrations on a meaningful-final-response check, while the CLI normalizes exhausted sentinel results into a helpful visible error message.

Validation

  • HERMES_HOME=/private/tmp/hermes-agent-17248-publish-home-focused ./scripts/run_tests.sh tests/run_agent/test_run_agent.py -k "empty_response or replayed_tool_history" - 8 passed
  • HERMES_HOME=/private/tmp/hermes-agent-17248-publish-home-adjacent ./scripts/run_tests.sh tests/run_agent/test_empty_response_recovery_persistence.py tests/run_agent/test_message_sequence_repair.py tests/cli/test_empty_response_display.py - 18 passed
  • HERMES_HOME=/private/tmp/hermes-agent-17248-publish-home-full ./scripts/run_tests.sh tests/run_agent/test_run_agent.py - 326 passed
  • uv run ruff check run_agent.py cli.py tests/run_agent/test_run_agent.py tests/cli/test_empty_response_display.py - passed
  • git diff --check - passed

Fixes #17248

@taivu1998
taivu1998 marked this pull request as ready for review May 11, 2026 03:46
@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 11, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the exhausted-empty response path. The user-visible raw-sentinel symptom has since been superseded on current main: agent/turn_finalizer.py:299-326 replaces (empty) with an actionable explanation, with end-to-end coverage in tests/run_agent/test_turn_completion_explainer.py:142-163.

Problems

  • The PR edits the old post-loop implementation in run_agent.py; current main runs these post-response effects from agent/turn_finalizer.py:337-480, so the patch needs a non-mechanical port.
  • Current main still passes the generated explanation to hooks and external-memory sync (agent/turn_finalizer.py:343-378, 462-478), and the CLI still permits auto-title/TTS (cli.py:12542-12560, 12705-12708). This means the side-effect-suppression objective remains relevant, but it must be keyed from the turn-exit reason/metadata rather than the old raw sentinel.

Suggested changes

  • Preserve current main's visible explanation, add exhausted-empty metadata from empty_response_exhausted, and use it to gate the affected integrations and CLI title/TTS paths.

Automated hermes-sweeper review.

Comment thread run_agent.py
@@ -15025,7 +15035,8 @@ def _stop_spinner():
# Fired once per turn after the tool-calling loop completes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main's live post-loop path is now agent/turn_finalizer.py, and it replaces (empty) with a user-facing explanation before hooks run. Please port this predicate there using turn_exit_reason == "empty_response_exhausted"; preserving the old raw-sentinel condition would not cover the current execution path.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
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 sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Empty final response after tool calls can hit unbound assistant_msg path

3 participants