Skip to content

Commit 0b31359

Browse files
author
leo.lc.chien
committed
fix(llm): recover empty tool-loop responses instead of returning null
When the model returns an empty response with no tool calls, the tool loop's empty-response retry only fires when iterations remain (`iteration < max_tool_iterations - 1`). At the dialectic `minimal` tier (MAX_TOOL_ITERATIONS=1) that guard is `0 < 0 == False`, so the retry can never fire and the tool-less branch returns empty content without reaching the post-loop synthesis call. The empty string then surfaces as a null dialectic response (the chat endpoint maps a falsy answer to None), so broad `minimal` queries returned null even though the same query answered correctly at `low`/`medium`. Decouple empty-response recovery from the iteration budget: keep the nudge-and-loop when iterations remain, otherwise break to the forced synthesis call so a final answer is always produced. This is a general tool-loop correctness fix; `minimal` is where it is guaranteed to surface. The synthesis prompt and log now distinguish empty-recovery from genuine max-iteration exhaustion. Adds a deterministic regression test covering the single-iteration recovery path and the multi-iteration control.
1 parent 4f9a413 commit 0b31359

2 files changed

Lines changed: 191 additions & 26 deletions

File tree

src/llm/tool_loop.py

Lines changed: 48 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,10 @@ async def execute_tool_loop(
404404
# DialecticCompletedEvent.hit_input_token_cap reflect the cap hit
405405
# (the toolless path tracks this in src/llm/api.py:325-340).
406406
hit_input_token_cap = False
407+
# Set when an empty tool-less response can't be retried in-loop (no
408+
# iterations left) and we break out to the forced synthesis call to
409+
# recover a final answer instead of returning empty.
410+
empty_recovery_synthesis = False
407411
# Track effective tool_choice — switches from "required"/"any" to "auto" after iter 1.
408412
effective_tool_choice = tool_choice
409413

@@ -492,24 +496,34 @@ async def _call_with_messages(
492496
if not response.tool_calls_made:
493497
logger.debug("No tool calls in response, finishing")
494498

495-
if (
496-
isinstance(response.content, str)
497-
and not response.content.strip()
498-
and empty_response_retries < 1
499-
and iteration < max_tool_iterations - 1
500-
):
499+
is_empty_response = (
500+
isinstance(response.content, str) and not response.content.strip()
501+
)
502+
if is_empty_response and empty_response_retries < 1:
501503
empty_response_retries += 1
502-
conversation_messages.append(
503-
{
504-
"role": "user",
505-
"content": (
506-
"Your last response was empty. Provide a concise answer "
507-
"to the original query using the available context."
508-
),
509-
}
510-
)
511-
iteration += 1
512-
continue
504+
if iteration < max_tool_iterations - 1:
505+
# Budget remains: nudge the model and loop again.
506+
conversation_messages.append(
507+
{
508+
"role": "user",
509+
"content": (
510+
"Your last response was empty. Provide a concise answer "
511+
"to the original query using the available context."
512+
),
513+
}
514+
)
515+
iteration += 1
516+
continue
517+
if not stream_final:
518+
# No iterations left to loop — notably the minimal tier
519+
# (max_tool_iterations=1), where `iteration < max_tool_iterations - 1`
520+
# is `0 < 0 == False` so the nudge above can never fire.
521+
# Break out to the forced synthesis call after the loop
522+
# rather than returning an empty/null answer. Without
523+
# this, the empty-response safety net silently no-ops for
524+
# single-iteration levels.
525+
empty_recovery_synthesis = True
526+
break
513527

514528
if stream_final:
515529
# Snapshot the plan that just succeeded — streaming retries
@@ -657,19 +671,27 @@ async def _call_with_messages(
657671

658672
iteration += 1
659673

660-
logger.warning(
661-
f"Tool execution loop reached max iterations ({max_tool_iterations})"
662-
)
674+
if empty_recovery_synthesis:
675+
logger.debug("Empty tool-less response, no retries left; forcing synthesis")
676+
synthesis_prompt = (
677+
"Your previous response was empty. Using the context already available, "
678+
"provide your final answer to the original query now. "
679+
"Do not attempt to call any tools."
680+
)
681+
else:
682+
logger.warning(
683+
f"Tool execution loop reached max iterations ({max_tool_iterations})"
684+
)
685+
synthesis_prompt = (
686+
"You have reached the maximum number of tool calls. "
687+
"Based on all the information you have gathered, provide your final response now. "
688+
"Do not attempt to call any more tools."
689+
)
663690

664-
# The max-iteration synthesis call gets iteration N+1 in telemetry so 's
691+
# The synthesis call gets iteration N+1 in telemetry so 's
665692
# AgentIterationEvent and this LLMCallCompletedEvent line up sequentially.
666693
synthesis_iteration = iteration + 1
667694

668-
synthesis_prompt = (
669-
"You have reached the maximum number of tool calls. "
670-
"Based on all the information you have gathered, provide your final response now. "
671-
"Do not attempt to call any more tools."
672-
)
673695
conversation_messages.append({"role": "user", "content": synthesis_prompt})
674696

675697
# Truncate again — the per-iteration truncate ran before the last tool
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
# pyright: reportPrivateUsage=false, reportUnknownLambdaType=false, reportUnknownArgumentType=false, reportArgumentType=false
2+
"""Isolation tests for empty-final-response recovery in `execute_tool_loop`.
3+
4+
Proves — independent of any deployment, stored data, or LLM nondeterminism —
5+
that when the model returns an *empty* final answer with no tool calls, the
6+
loop's recovery depends on the iteration budget in a way that silently breaks
7+
the `minimal` dialectic tier (`MAX_TOOL_ITERATIONS=1`).
8+
9+
The empty-response retry in `tool_loop.py` is gated on
10+
`iteration < max_tool_iterations - 1`. At `max_tool_iterations=1` that is
11+
`0 < 0` = False, so the ONLY empty-response safety net never fires and there
12+
is no synthesis fallback on the no-tool path → the loop returns an empty
13+
(null) answer. With `max_tool_iterations>=2` the retry fires and recovers.
14+
15+
`test_empty_final_response_recovered_at_single_iteration` FAILS on unfixed
16+
code (returns "") and passes once the recovery is decoupled from the
17+
iteration budget. The `..._with_budget` control passes on both, isolating the
18+
defect to the single-iteration case.
19+
"""
20+
21+
from __future__ import annotations
22+
23+
from typing import Any
24+
25+
import pytest
26+
27+
from src.llm import tool_loop
28+
from src.llm.runtime import AttemptPlan
29+
from src.llm.tool_loop import execute_tool_loop
30+
from src.llm.types import HonchoLLMCallResponse
31+
32+
33+
def _make_plan() -> AttemptPlan:
34+
"""Return a minimal AttemptPlan the mocked inner call passes straight through."""
35+
return AttemptPlan(
36+
provider="anthropic",
37+
model="claude-sonnet-4-5",
38+
client=object(),
39+
thinking_budget_tokens=None,
40+
reasoning_effort=None,
41+
selected_config=None,
42+
attempt=1,
43+
retry_attempts=1,
44+
is_fallback=False,
45+
)
46+
47+
48+
def _resp(content: str) -> HonchoLLMCallResponse[Any]:
49+
"""Build a no-tool-call LLM response carrying the given content."""
50+
return HonchoLLMCallResponse(
51+
content=content,
52+
input_tokens=10,
53+
output_tokens=5,
54+
cache_creation_input_tokens=0,
55+
cache_read_input_tokens=0,
56+
finish_reasons=["stop"],
57+
tool_calls_made=[],
58+
)
59+
60+
61+
class _EmptyThenRecover:
62+
"""Mock inner-LLM: the first tool-enabled call returns empty content with
63+
no tool calls (the failure the model actually produces at `minimal` on
64+
broad recall queries). Recovery paths return distinct sentinels so the
65+
test can tell HOW recovery happened:
66+
- `tools=None` → forced synthesis call → "recovered via synthesis"
67+
- 2nd tool call → nudge-and-loop retry → "recovered via retry"
68+
"""
69+
70+
def __init__(self) -> None:
71+
"""Initialize per-call counters for tool-enabled and synthesis calls."""
72+
self.tool_enabled_calls = 0
73+
self.synthesis_calls = 0
74+
75+
async def __call__(self, *_args: Any, **kwargs: Any) -> HonchoLLMCallResponse[Any]:
76+
"""Return empty then recovery content, keyed off whether tools are passed."""
77+
if kwargs.get("tools") is None: # forced final synthesis call
78+
self.synthesis_calls += 1
79+
return _resp("recovered via synthesis")
80+
self.tool_enabled_calls += 1
81+
if self.tool_enabled_calls == 1:
82+
return _resp("") # empty final answer, no tool calls
83+
return _resp("recovered via retry")
84+
85+
86+
_TOOLS = [{"name": "noop", "description": "no-op", "input_schema": {"type": "object"}}]
87+
88+
89+
async def _run(max_tool_iterations: int, mock: _EmptyThenRecover) -> Any:
90+
"""Drive execute_tool_loop with the mock at the given iteration budget."""
91+
return await execute_tool_loop(
92+
prompt="hi",
93+
max_tokens=250,
94+
messages=[{"role": "user", "content": "What do you know about me?"}],
95+
tools=_TOOLS,
96+
tool_choice="auto",
97+
tool_executor=lambda _name, _input: "",
98+
max_tool_iterations=max_tool_iterations,
99+
response_model=None,
100+
json_mode=False,
101+
temperature=None,
102+
stop_seqs=None,
103+
verbosity=None,
104+
enable_retry=False,
105+
retry_attempts=1,
106+
max_input_tokens=None,
107+
get_attempt_plan=_make_plan,
108+
before_retry_callback=lambda _r: None,
109+
stream_final=False,
110+
telemetry=None,
111+
)
112+
113+
114+
@pytest.mark.asyncio
115+
async def test_empty_final_response_recovered_at_single_iteration():
116+
"""minimal tier (`max_tool_iterations=1`): an empty no-tool answer must be
117+
recovered, not returned as null. FAILS on unfixed code (returns "")."""
118+
mock = _EmptyThenRecover()
119+
with pytest.MonkeyPatch.context() as mp:
120+
mp.setattr(tool_loop, "honcho_llm_call_inner", mock)
121+
result = await _run(1, mock)
122+
123+
assert isinstance(result, HonchoLLMCallResponse)
124+
assert result.content and result.content.strip(), (
125+
f"minimal tier returned an empty/null answer: {result.content!r}"
126+
)
127+
assert result.content == "recovered via synthesis"
128+
assert mock.synthesis_calls == 1
129+
130+
131+
@pytest.mark.asyncio
132+
async def test_empty_final_response_recovered_with_budget():
133+
"""Control: with `max_tool_iterations=2` the existing nudge-and-loop retry
134+
already recovers. Passes on both unfixed and fixed code, isolating the
135+
defect to the single-iteration case."""
136+
mock = _EmptyThenRecover()
137+
with pytest.MonkeyPatch.context() as mp:
138+
mp.setattr(tool_loop, "honcho_llm_call_inner", mock)
139+
result = await _run(2, mock)
140+
141+
assert isinstance(result, HonchoLLMCallResponse)
142+
assert result.content == "recovered via retry"
143+
assert mock.tool_enabled_calls == 2

0 commit comments

Comments
 (0)