diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 8848b64df2cb2..e3bbc1e9acd25 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -6586,18 +6586,54 @@ def _perform_api_call(next_api_kwargs): agent._response_was_previewed = False break + # Nudge availability is computed up front because it gates + # the housekeeping fallback shortcut below (#65600): a + # single empty completion after a housekeeping-only turn is + # NOT a reliable "the model is done" signal — weak/quantized + # local models intermittently choke and return empty right + # after a housekeeping call. Spend the one nudge retry + # first; the shortcut then handles the SECOND consecutive + # empty, which is a much stronger done signal. + _prior_was_tool = any( + m.get("role") == "tool" + for m in messages[-5:] # check recent messages + ) + # Detect Qwen3/Ollama-style in-content thinking blocks. + # Ollama puts in the content field (not in + # reasoning_content), so _has_structured below would + # miss it. We check here so thinking-only responses + # after tool calls route to prefill instead of nudge. + _has_inline_thinking = bool( + re.search( + r'||', + final_response or "", + re.IGNORECASE, + ) + ) + _nudge_available = ( + _prior_was_tool + and not getattr(agent, "_post_tool_empty_retried", False) + and not _has_inline_thinking # thinking model still working — let prefill handle + ) + # If the previous turn already delivered real content alongside # HOUSEKEEPING tool calls (e.g. "You're welcome!" + memory save), # the model has nothing more to say. Use the earlier content - # immediately instead of wasting API calls on retries. + # instead of wasting API calls on retries. # NOTE: Only use this shortcut when ALL tools in that turn were # housekeeping (memory, todo, etc.). When substantive tools # were called (terminal, search_files, etc.), the content was # likely mid-task narration ("I'll scan the directory...") and # the empty follow-up means the model choked — let the # post-tool nudge below handle that instead of exiting early. + # The `not _nudge_available` gate defers the shortcut past + # one nudge attempt (#65600) — see the comment above. fallback = getattr(agent, '_last_content_with_tools', None) - if fallback and getattr(agent, '_last_content_tools_all_housekeeping', False): + if ( + fallback + and getattr(agent, '_last_content_tools_all_housekeeping', False) + and not _nudge_available + ): _turn_exit_reason = "fallback_prior_turn_content" logger.info("Empty follow-up after tool calls — using prior turn content as final response") agent._emit_status("↻ Empty response after tool calls — using earlier content as final answer") @@ -6614,43 +6650,31 @@ def _perform_api_call(next_api_kwargs): # ── Post-tool-call empty response nudge ─────────── # The model returned empty after executing tool calls. - # This covers two cases: + # This covers three cases: # (a) No prior-turn content at all — model went silent # (b) Prior turn had content + SUBSTANTIVE tools (the # fallback above was skipped because the content # was mid-task narration, not a final answer) + # (c) Prior turn was housekeeping-only — the fallback + # above is deferred until this one nudge is spent, + # so a choked weak model gets a chance to finish + # instead of silently ending the turn (#65600) # Instead of giving up, nudge the model to continue by # appending a user-level hint. This is the #9400 case: # weaker models (mimo-v2-pro, GLM-5, etc.) sometimes # return empty after tool results instead of continuing # to the next step. One retry with a nudge usually # fixes it. - _prior_was_tool = any( - m.get("role") == "tool" - for m in messages[-5:] # check recent messages - ) - # Detect Qwen3/Ollama-style in-content thinking blocks. - # Ollama puts in the content field (not in - # reasoning_content), so _has_structured below would - # miss it. We check here so thinking-only responses - # after tool calls route to prefill instead of nudge. - _has_inline_thinking = bool( - re.search( - r'||', - final_response or "", - re.IGNORECASE, - ) - ) - if ( - _prior_was_tool - and not getattr(agent, "_post_tool_empty_retried", False) - and not _has_inline_thinking # thinking model still working — let prefill handle - ): + if _nudge_available: agent._post_tool_empty_retried = True - # Clear stale narration so it doesn't resurface - # on a later empty response after the nudge. - agent._last_content_with_tools = None - agent._last_content_tools_all_housekeeping = False + # Clear stale substantive narration so it doesn't + # resurface on a later empty response after the nudge. + # A HOUSEKEEPING fallback is deliberately preserved: + # if the nudge also comes back empty, the shortcut + # above still delivers the prior content instead of + # dead-ending in empty-response retries (#65600). + if not getattr(agent, '_last_content_tools_all_housekeeping', False): + agent._last_content_with_tools = None logger.info( "Empty response after tool calls — nudging model " "to continue processing" diff --git a/tests/run_agent/test_conversation_fallback_state.py b/tests/run_agent/test_conversation_fallback_state.py index a0d07c18abe8a..44ad9ee494e0c 100644 --- a/tests/run_agent/test_conversation_fallback_state.py +++ b/tests/run_agent/test_conversation_fallback_state.py @@ -205,3 +205,140 @@ def test_bare_tool_marker_is_not_reused_as_final_response(): f"Expected 3 API calls (including nudge), got: {result['api_calls']}." ) + +def test_housekeeping_only_turn_still_sets_fallback(): + """Regression: pure housekeeping turns (content + only housekeeping tools) + must still set the fallback so the post-response mute path works. This + verifies the fix doesn't break the original use case the fallback was + designed for. + + Since #65600 the shortcut is deferred past one nudge retry: the first + empty completion gets a nudge, and only a SECOND consecutive empty is + treated as "the model is done" and served the fallback content. + """ + with ( + patch("run_agent.get_tool_definitions", return_value=_tool_defs("memory")), + patch("run_agent.check_toolset_requirements", return_value={}), + patch("run_agent.OpenAI"), + ): + agent = AIAgent( + api_key="test-key", + base_url="https://openrouter.ai/api/v1/", + quiet_mode=True, + skip_context_files=True, + skip_memory=True, + ) + + agent._cached_system_prompt = "You are helpful." + agent._use_prompt_caching = False + agent.tool_delay = 0 + agent.compression_enabled = False + agent.save_trajectories = False + agent.valid_tool_names = {"memory"} + agent.client = MagicMock() + agent.client.chat.completions.create.side_effect = [ + # Turn 1: Content + housekeeping tool (should set fallback) + _response( + content="You're welcome!", + finish_reason="tool_calls", + tool_calls=[_tool_call("memory", "mem1")], + ), + # Turn 2: Empty response (should spend the one nudge retry) + _response(content="", finish_reason="stop"), + # Turn 3: Empty again (nudge spent — NOW use the housekeeping fallback) + _response(content="", finish_reason="stop"), + ] + + with ( + patch("run_agent.handle_function_call", return_value="ok"), + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + result = agent.run_conversation("save this") + + assert result["final_response"] == "You're welcome!", ( + f"Expected housekeeping fallback content, got: {result['final_response']}. " + f"Pure housekeeping turns should still set the fallback." + ) + assert "fallback_prior_turn_content" in result.get("turn_exit_reason", ""), ( + f"Expected fallback_prior_turn_content exit, got: {result['turn_exit_reason']}." + ) + assert result["api_calls"] == 3, ( + f"Expected 3 API calls (housekeeping turn, empty, nudged empty), " + f"got: {result['api_calls']}." + ) + + +def test_housekeeping_empty_follow_up_gets_one_nudge_before_fallback(): + """ + Regression test for #65600. + + An empty completion right after a housekeeping-only tool turn is not + always "the model has nothing more to say" — weak/quantized local models + intermittently choke and return empty mid-task. The shortcut used to + fire immediately, recycling the prior turn's narration as the final + response and silently ending the turn. + + Test sequence: + 1. Content + skill_manage (housekeeping) → sets fallback + 2. Empty completion → must get the standard post-tool nudge, NOT the + immediate fallback shortcut + 3. Model recovers with real content → that content is the final response + + Before the fix: step 2 took the shortcut, the final response was the + recycled step-1 narration, and the model never got a chance to finish + (2 API calls, reason=fallback_prior_turn_content). + """ + with ( + patch("run_agent.get_tool_definitions", return_value=_tool_defs("skill_manage")), + patch("run_agent.check_toolset_requirements", return_value={}), + patch("run_agent.OpenAI"), + ): + agent = AIAgent( + api_key="test-key", + base_url="https://openrouter.ai/api/v1/", + quiet_mode=True, + skip_context_files=True, + skip_memory=True, + ) + + agent._cached_system_prompt = "You are helpful." + agent._use_prompt_caching = False + agent.tool_delay = 0 + agent.compression_enabled = False + agent.save_trajectories = False + agent.valid_tool_names = {"skill_manage"} + agent.client = MagicMock() + agent.client.chat.completions.create.side_effect = [ + # Turn 1: Mid-task narration + housekeeping tool (sets fallback) + _response( + content="Reviewing the skill library now.", + finish_reason="tool_calls", + tool_calls=[_tool_call("skill_manage", "skill1")], + ), + # Turn 2: Model chokes — genuinely empty, but NOT done + _response(content="", finish_reason="stop"), + # Turn 3: Nudge lands, model finishes the task for real + _response(content="Skill library updated with two new entries.", finish_reason="stop"), + ] + + with ( + patch("run_agent.handle_function_call", return_value="ok"), + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + result = agent.run_conversation("review the skill library") + + assert result["final_response"] == "Skill library updated with two new entries.", ( + f"Expected the post-nudge recovery response, got: {result['final_response']}. " + f"The housekeeping fallback shortcut fired before the nudge retry." + ) + assert result["api_calls"] == 3, ( + f"Expected 3 API calls (housekeeping turn, choke, nudge recovery), " + f"got: {result['api_calls']}. 2 means the shortcut bypassed the nudge." + ) + assert result["turn_exit_reason"].startswith("text_response"), ( + f"Expected text_response exit, got: {result['turn_exit_reason']}." + )