From 73edb7aeb3ef115a69f1f3e9d8bc4e7cbbe5c262 Mon Sep 17 00:00:00 2001 From: taivu1998 <46636857+taivu1998@users.noreply.github.com> Date: Sun, 10 May 2026 15:59:04 -0700 Subject: [PATCH] fix: surface exhausted empty responses --- cli.py | 47 ++++++- run_agent.py | 23 +++- tests/cli/test_empty_response_display.py | 54 ++++++++ tests/run_agent/test_run_agent.py | 163 +++++++++++++++++++++++ 4 files changed, 280 insertions(+), 7 deletions(-) create mode 100644 tests/cli/test_empty_response_display.py diff --git a/cli.py b/cli.py index b63cde590b7a..9a9e3c9aab91 100644 --- a/cli.py +++ b/cli.py @@ -1349,6 +1349,28 @@ def _protect(match: re.Match[str]) -> str: return _WINDOWS_PATH_WITH_DOT_SEGMENT_RE.sub(_protect, text) +EMPTY_RESPONSE_EXHAUSTED_MESSAGE = ( + "The model returned no visible response after retries. If this happened " + "after tool calls, the tools completed but the model did not produce a " + "final answer. Try again, switch models, or configure a fallback provider." +) + + +def _normalize_final_response_for_cli(result, response): + """Convert internal empty-response sentinels into user-facing CLI text.""" + if not isinstance(result, dict): + return response or "", False + + empty_exhausted = bool(result.get("empty_response_exhausted")) + empty_exhausted = ( + empty_exhausted or result.get("error_code") == "empty_response_exhausted" + ) + if empty_exhausted and (response or "") == "(empty)": + return EMPTY_RESPONSE_EXHAUSTED_MESSAGE, True + + return response or "", False + + def _render_final_assistant_content(text: str, mode: str = "render"): """Render final assistant content as markdown, stripped text, or raw text.""" from rich.markdown import Markdown @@ -7423,6 +7445,9 @@ def _bg_thinking(text: str) -> None: ) response = result.get("final_response", "") if result else "" + response, _empty_response_error = _normalize_final_response_for_cli( + result, response + ) if not response and result and result.get("error"): response = f"Error: {result['error']}" @@ -10254,9 +10279,18 @@ def run_agent(): # Get the final response response = result.get("final_response", "") if result else "" + response, empty_response_error = _normalize_final_response_for_cli( + result, response + ) # Auto-generate session title after first exchange (non-blocking) - if response and result and not result.get("failed") and not result.get("partial"): + if ( + response + and result + and not empty_response_error + and not result.get("failed") + and not result.get("partial") + ): try: from agent.title_generator import maybe_auto_title # Route title-generation failures through the agent's @@ -10347,7 +10381,9 @@ def run_agent(): _resp_color = "#CD7F32" _resp_text = "#FFF8DC" - is_error_response = result and (result.get("failed") or result.get("partial")) + is_error_response = empty_response_error or ( + result and (result.get("failed") or result.get("partial")) + ) already_streamed = self._stream_started and self._stream_box_opened and not is_error_response if use_streaming_tts and _streaming_box_opened and not is_error_response: # Text was already printed sentence-by-sentence; just close the box @@ -10389,7 +10425,12 @@ def run_agent(): # Speak response aloud if voice TTS is enabled # Skip batch TTS when streaming TTS already handled it - if self._voice_tts and response and not use_streaming_tts: + if ( + self._voice_tts + and response + and not use_streaming_tts + and not empty_response_error + ): self._voice_speak_response_async(response) diff --git a/run_agent.py b/run_agent.py index 96d4d8517fe6..997fc5d081c7 100644 --- a/run_agent.py +++ b/run_agent.py @@ -11506,6 +11506,9 @@ def run_conversation( self._last_content_with_tools = None self._last_content_tools_all_housekeeping = False self._mute_post_response = False + empty_response_exhausted = False + empty_response_error = None + empty_response_code = None self._unicode_sanitization_passes = 0 self._tool_guardrails.reset_for_turn() self._tool_guardrail_halt_decision = None @@ -14685,6 +14688,7 @@ def _stop_spinner(): "results above and continue with the task." ), "_empty_recovery_synthetic": True, + "_empty_recovery_user_nudge": True, }) continue @@ -14786,6 +14790,12 @@ def _stop_spinner(): # fallback configured). Fall through to the # "(empty)" terminal. _turn_exit_reason = "empty_response_exhausted" + empty_response_exhausted = True + empty_response_code = "empty_response_exhausted" + empty_response_error = ( + "Model returned no visible response after " + "empty-response retries were exhausted." + ) reasoning_text = self._extract_reasoning(assistant_message) self._drop_trailing_empty_response_scaffolding(messages) assistant_msg = self._build_assistant_message(assistant_message, finish_reason) @@ -15025,7 +15035,8 @@ def _stop_spinner(): # Fired once per turn after the tool-calling loop completes. # Plugins can transform the LLM's output text before it's returned. # First hook to return a string wins; None/empty return leaves text unchanged. - if final_response and not interrupted: + _has_meaningful_final_response = bool(final_response) and not empty_response_exhausted + if _has_meaningful_final_response and not interrupted: try: from hermes_cli.plugins import invoke_hook as _invoke_hook _transform_results = _invoke_hook( @@ -15046,7 +15057,7 @@ def _stop_spinner(): # Fired once per turn after the tool-calling loop completes. # Plugins can use this to persist conversation data (e.g. sync # to an external memory system). - if final_response and not interrupted: + if _has_meaningful_final_response and not interrupted: try: from hermes_cli.plugins import invoke_hook as _invoke_hook _invoke_hook( @@ -15105,6 +15116,10 @@ def _stop_spinner(): "cost_status": self.session_cost_status, "cost_source": self.session_cost_source, } + if empty_response_exhausted: + result["empty_response_exhausted"] = True + result["error_code"] = empty_response_code + result["error"] = empty_response_error if self._tool_guardrail_halt_decision is not None: result["guardrail"] = self._tool_guardrail_halt_decision.to_metadata() # If a /steer landed after the final assistant turn (no more tool @@ -15136,13 +15151,13 @@ def _stop_spinner(): # External memory provider: sync the completed turn + queue next prefetch. self._sync_external_memory_for_turn( original_user_message=original_user_message, - final_response=final_response, + final_response=final_response if _has_meaningful_final_response else None, interrupted=interrupted, ) # Background memory/skill review — runs AFTER the response is delivered # so it never competes with the user's task for model attention. - if final_response and not interrupted and (_should_review_memory or _should_review_skills): + if _has_meaningful_final_response and not interrupted and (_should_review_memory or _should_review_skills): try: self._spawn_background_review( messages_snapshot=list(messages), diff --git a/tests/cli/test_empty_response_display.py b/tests/cli/test_empty_response_display.py new file mode 100644 index 000000000000..6ed83df50358 --- /dev/null +++ b/tests/cli/test_empty_response_display.py @@ -0,0 +1,54 @@ +"""CLI display helpers for exhausted empty model responses.""" + +from __future__ import annotations + +from cli import ( + EMPTY_RESPONSE_EXHAUSTED_MESSAGE, + _normalize_final_response_for_cli, +) + + +def test_empty_response_exhaustion_replaces_literal_empty_sentinel(): + response, is_error = _normalize_final_response_for_cli( + { + "final_response": "(empty)", + "empty_response_exhausted": True, + "error_code": "empty_response_exhausted", + }, + "(empty)", + ) + + assert is_error is True + assert response == EMPTY_RESPONSE_EXHAUSTED_MESSAGE + assert "(empty)" not in response + assert "no visible response" in response + + +def test_legacy_empty_response_exhaustion_code_replaces_sentinel(): + response, is_error = _normalize_final_response_for_cli( + { + "final_response": "(empty)", + "error_code": "empty_response_exhausted", + }, + "(empty)", + ) + + assert is_error is True + assert response == EMPTY_RESPONSE_EXHAUSTED_MESSAGE + + +def test_non_exhausted_empty_sentinel_is_left_unchanged(): + response, is_error = _normalize_final_response_for_cli({}, "(empty)") + + assert is_error is False + assert response == "(empty)" + + +def test_normal_response_is_left_unchanged(): + response, is_error = _normalize_final_response_for_cli( + {"final_response": "Done."}, + "Done.", + ) + + assert is_error is False + assert response == "Done." diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index 5bc485e0711c..2d2c84542849 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -2707,6 +2707,169 @@ def test_truly_empty_response_succeeds_on_nudge(self, agent): assert result["final_response"] == "Here is the actual answer." assert result["api_calls"] == 2 # 1 original + 1 nudge retry + def test_replayed_tool_history_empty_nudge_keeps_messages_protocol_valid(self, agent): + """Empty recovery after replayed tool history must use real message dicts.""" + self._setup_agent(agent) + conversation_history = [ + { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": "call_replayed", + "type": "function", + "function": { + "name": "web_search", + "arguments": "{}", + }, + } + ], + }, + { + "role": "tool", + "tool_call_id": "call_replayed", + "content": '{"ok": true}', + }, + ] + empty_resp = _mock_response(content=None, finish_reason="stop") + success_resp = _mock_response(content="Recovered after replay", finish_reason="stop") + agent.client.chat.completions.create.side_effect = [empty_resp, success_resp] + persisted_messages = [] + + def fake_persist(messages, conversation_history=None): + persisted_messages.append([m.copy() for m in messages]) + + with ( + patch.object(agent, "_persist_session", side_effect=fake_persist), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + result = agent.run_conversation( + "continue after the tool", + conversation_history=conversation_history, + ) + + assert result["completed"] is True + assert result["final_response"] == "Recovered after replay" + assert agent.client.chat.completions.create.call_count == 2 + + retry_messages = agent.client.chat.completions.create.call_args_list[1].kwargs[ + "messages" + ] + assert retry_messages[-2]["role"] == "assistant" + assert retry_messages[-2]["content"] == "(empty)" + assert retry_messages[-2]["_empty_recovery_synthetic"] is True + assert retry_messages[-1]["role"] == "user" + assert retry_messages[-1]["_empty_recovery_user_nudge"] is True + + assert all("_empty_recovery_synthetic" not in m for m in result["messages"]) + assert all("_empty_recovery_user_nudge" not in m for m in result["messages"]) + assert persisted_messages + assert all( + "_empty_recovery_synthetic" not in m and "_empty_recovery_user_nudge" not in m + for m in persisted_messages[-1] + ) + + def test_tool_empty_response_exhaustion_reports_metadata_and_cleans_scaffolding( + self, agent + ): + """Exhausted post-tool empty recovery should be explicit but not persisted.""" + self._setup_agent(agent) + tool_resp = _mock_response( + content="", + finish_reason="tool_calls", + tool_calls=[ + _mock_tool_call(name="web_search", arguments="{}", call_id="call_empty") + ], + ) + empty_resp = _mock_response(content=None, finish_reason="stop") + agent.client.chat.completions.create.side_effect = [ + tool_resp, + empty_resp, + empty_resp, + empty_resp, + empty_resp, + empty_resp, + ] + persisted_messages = [] + hook_calls = [] + + def fake_persist(messages, conversation_history=None): + persisted_messages.append([m.copy() for m in messages]) + + def record_hook(name, **kwargs): + hook_calls.append((name, kwargs)) + return [] + + with ( + patch("run_agent.handle_function_call", return_value='{"ok": true}'), + patch("hermes_cli.plugins.invoke_hook", side_effect=record_hook), + patch.object(agent, "_persist_session", side_effect=fake_persist), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + patch.object(agent, "_sync_external_memory_for_turn") as sync_memory, + ): + result = agent.run_conversation("call the tool") + + assert result["completed"] is True + assert result["final_response"] == "(empty)" + assert result["turn_exit_reason"] == "empty_response_exhausted" + assert result["empty_response_exhausted"] is True + assert result["error_code"] == "empty_response_exhausted" + assert "no visible response" in result["error"] + assert agent.client.chat.completions.create.call_count == 6 + assert all("_empty_recovery_synthetic" not in m for m in result["messages"]) + assert all("_empty_recovery_user_nudge" not in m for m in result["messages"]) + assert all("_empty_terminal_sentinel" not in m for m in result["messages"]) + assert persisted_messages + assert all( + "_empty_recovery_synthetic" not in m + and "_empty_recovery_user_nudge" not in m + and "_empty_terminal_sentinel" not in m + for m in persisted_messages[-1] + ) + sync_memory.assert_called_once() + assert sync_memory.call_args.kwargs["final_response"] is None + hook_names = [name for name, _kwargs in hook_calls] + assert "transform_llm_output" not in hook_names + assert "post_llm_call" not in hook_names + assert "pre_api_request" in hook_names + assert "post_api_request" in hook_names + + def test_tool_empty_response_nudge_success_cleans_transient_messages(self, agent): + """Successful post-tool empty recovery should not leak synthetic messages.""" + self._setup_agent(agent) + tool_resp = _mock_response( + content="", + finish_reason="tool_calls", + tool_calls=[ + _mock_tool_call(name="web_search", arguments="{}", call_id="call_recover") + ], + ) + empty_resp = _mock_response(content=None, finish_reason="stop") + success_resp = _mock_response(content="Recovered after tools", finish_reason="stop") + agent.client.chat.completions.create.side_effect = [ + tool_resp, + empty_resp, + success_resp, + ] + + with ( + patch("run_agent.handle_function_call", return_value='{"ok": true}'), + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + result = agent.run_conversation("call the tool") + + assert result["completed"] is True + assert result["final_response"] == "Recovered after tools" + assert "empty_response_exhausted" not in result + assert agent.client.chat.completions.create.call_count == 3 + assert all("_empty_recovery_synthetic" not in m for m in result["messages"]) + assert all("_empty_recovery_user_nudge" not in m for m in result["messages"]) + assert all("_empty_terminal_sentinel" not in m for m in result["messages"]) + def test_empty_response_triggers_fallback_provider(self, agent): """After 3 empty retries, fallback provider is activated and produces content.""" self._setup_agent(agent)