Skip to content

fix(agent): emit recovered/fallback text to stream clients before close (#31449) - #31539

Closed
haran2001 wants to merge 2 commits into
NousResearch:mainfrom
haran2001:fix/stream-recovery-emit-final-response
Closed

haran2001 wants to merge 2 commits into
NousResearch:mainfrom
haran2001:fix/stream-recovery-emit-final-response

Conversation

@haran2001

Copy link
Copy Markdown
Contributor

Summary

Follow-up from PR #31448 (issue #30770), which fixed the guardrail_halt exit branch in agent/conversation_loop.py to emit the synthesized halt message via stream_delta_callback before closing. Two structurally-identical sibling sites had the same gap and were explicitly flagged by #31449:

  1. partial_stream_recovery (~L3543) — assigns final_response = _recovered (text reconstructed from a partial stream after a <think> block) and breaks. The recovered text wasn't necessarily streamed in full this turn, so SSE/TUI consumers drained an empty delta queue and rendered a blank bubble.
  2. fallback_prior_turn_content (~L3582) — assigns final_response = agent._strip_think_blocks(fallback).strip() where fallback is the previous turn's content. The previous turn's content was streamed on the prior SSE response, so the current SSE writer also drained an empty queue.

This PR mirrors the guardrail-halt emit pattern (callback(text) + callback(None) inside a guarded try/except) at both sites. The blank-bubble symptom is now indistinguishable from a normal response close.

Test Plan

  • python -m pytest tests/run_agent/test_run_agent.py -q -o 'addopts=' -k 'partial_stream_recovery or fallback_prior_turn' — 5 passed (2 new + 3 existing).
    • new: test_partial_stream_recovery_emits_to_stream_callback — source-inspection regression that fails if the emit block is removed from the partial_stream_recovery branch.
    • new: test_fallback_prior_turn_content_emits_to_stream_callback — same shape for the fallback_prior_turn_content branch.
    • The existing live-loop tests (test_partial_stream_recovery_uses_streamed_content, _on_empty_stub, _preempts_prior_turn_fallback) still pass — they cover the recovery happy paths without depending on the new callback assertion.

Source-inspection rather than a live behavioral assertion was chosen for the new tests because setting stream_delta_callback on the existing fixture changes how the streaming pipeline accumulates _current_streamed_assistant_text (the field the recovery branch reads), so a live-loop assertion races with the streaming finalizer. The source check is durable: it catches anyone removing or relocating the emit block in either branch.

Closes #31449

@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 labels May 24, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

Competing with #31477 and #31478 — all three fix #31449 (stream_delta_callback gap at partial_stream_recovery and fallback_prior_turn_content sites). This PR mirrors the guardrail-halt emit pattern from merged #31448.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused recovery-path analysis. This is an automated hermes-sweeper review; current main now provides the requested delivery guarantee through a newer, stronger path.

  • e860a40e14b1e936ab2e5ad646546380ad0a4e57 (shipped in v2026.7.1) leaves partial_stream_recovery unpreviewed at agent/conversation_loop.py:4846-4851, so gateway fallback sends the recovered finalized response and its completion explanation.
  • That behavior is covered at tests/run_agent/test_turn_completion_explainer.py:165-193.
  • The fallback branch is not content from a prior SSE request: it is captured from the same tool-call turn at agent/conversation_loop.py:4634-4655 and emitted as an interim assistant message at agent/conversation_loop.py:4687-4688.
  • Gateway now suppresses a final send only after confirming the exact final text was delivered (gateway/run.py:19444-19462, gateway/run.py:20060-20073), so a missing preview cannot produce the blank-bubble failure described here.

The PR's intended guarantee is therefore already implemented on current main.

@teknium1 teknium1 closed this Jul 13, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label 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 P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

guardrail-halt-style silent stream close at partial_stream_recovery + fallback_prior_turn_content sites

3 participants