Skip to content

Commit 2f489f4

Browse files
committed
fix: avoid false-positive retries on sub-agent and no-tool-call responses
The empty response retry was too aggressive — it triggered on: 1. Sub-agents (AgentTool, ParallelAgent) that legitimately return no content 2. First LLM calls with no prior tool execution Fixes: - Add null guard for last_event in is_final_response check (NoneType crash) - Only retry after at least one tool call in the invocation, since the bug only manifests when models return empty after processing tool results - Remove append_event for resume message (caused session state corruption in pause/resume flows and leaked to UI) - Silent retry instead (proven 100% recovery rate in production tests) - Update scenario tests to include tool call before empty response
1 parent 1ce76f7 commit 2f489f4

File tree

3 files changed

+146
-185
lines changed

3 files changed

+146
-185
lines changed

src/google/adk/flows/llm_flows/base_llm_flow.py

Lines changed: 17 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,6 @@
6969
# tool execution. Prevents infinite loops if the model keeps returning empty.
7070
_MAX_EMPTY_RESPONSE_RETRIES = 2
7171

72-
# Message injected into the conversation when the model returns an empty
73-
# response, nudging it to resume execution on the next attempt.
74-
_EMPTY_RESPONSE_RESUME_MESSAGE = (
75-
'Your previous response was empty. '
76-
'Please resume execution from where you left off.'
77-
)
78-
7972

8073
def _has_meaningful_content(event: Event) -> bool:
8174
"""Returns whether the event has content worth showing to the user.
@@ -782,27 +775,35 @@ async def run_async(
782775
) -> AsyncGenerator[Event, None]:
783776
"""Runs the flow."""
784777
empty_response_count = 0
778+
has_prior_tool_call = False
785779
while True:
786780
last_event = None
787781
async with Aclosing(self._run_one_step_async(invocation_context)) as agen:
788782
async for event in agen:
789783
last_event = event
790784
yield event
785+
# Track if any tool calls have been executed in this invocation.
786+
# Empty responses are only retried after tool execution because
787+
# that is where models intermittently return 0 output tokens.
788+
if event.get_function_calls():
789+
has_prior_tool_call = True
791790

792791
# Determine if the model returned an empty / useless response that
793792
# should be retried. Three cases:
794-
# 1. No event at all (model/adapter yielded nothing)
795-
# 2. Last event is partial with no meaningful content (streaming +
793+
# 1. Last event is partial with no meaningful content (streaming +
796794
# thinking: only thought chunks arrived, no final response)
797-
# 3. Last event is a final response with no meaningful content
795+
# 2. Last event is a final response with no meaningful content
798796
# (non-streaming empty response, or streaming empty aggregated)
799797
is_empty_response = False
800-
if not last_event:
801-
is_empty_response = True
802-
elif last_event.partial and not _has_meaningful_content(last_event):
798+
if (
799+
last_event
800+
and last_event.partial
801+
and not _has_meaningful_content(last_event)
802+
):
803803
is_empty_response = True
804804
elif (
805-
last_event.is_final_response()
805+
last_event
806+
and last_event.is_final_response()
806807
and not _has_meaningful_content(last_event)
807808
and last_event.author == invocation_context.agent.name
808809
):
@@ -815,35 +816,15 @@ async def run_async(
815816

816817
if (
817818
is_empty_response
819+
and has_prior_tool_call
818820
and empty_response_count < _MAX_EMPTY_RESPONSE_RETRIES
819821
):
820822
empty_response_count += 1
821823
logger.warning(
822-
'Model returned an empty response (attempt %d/%d),'
823-
' injecting resume message and re-prompting.',
824+
'Model returned an empty response (attempt %d/%d), re-prompting.',
824825
empty_response_count,
825826
_MAX_EMPTY_RESPONSE_RETRIES,
826827
)
827-
# Inject a resume nudge into the session so the next LLM call
828-
# sees it in its context and is more likely to continue.
829-
# We append directly to the session (not yield) so that the
830-
# message reaches the model but is NOT sent to the UI/SSE stream.
831-
resume_event = Event(
832-
id=Event.new_id(),
833-
invocation_id=invocation_context.invocation_id,
834-
author='user',
835-
branch=invocation_context.branch,
836-
content=types.Content(
837-
role='user',
838-
parts=[
839-
types.Part.from_text(text=_EMPTY_RESPONSE_RESUME_MESSAGE)
840-
],
841-
),
842-
)
843-
await invocation_context.session_service.append_event(
844-
session=invocation_context.session,
845-
event=resume_event,
846-
)
847828
continue
848829

849830
if (

tests/unittests/flows/llm_flows/test_base_llm_flow_partial_handling.py

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -91,18 +91,14 @@ async def test_run_async_breaks_on_final_response():
9191

9292

9393
@pytest.mark.asyncio
94-
async def test_run_async_retries_then_breaks_on_no_last_event():
95-
"""Test that run_async retries when there is no last event, then breaks."""
96-
# Create a mock model that returns empty responses (no content).
97-
# Need enough responses to cover initial call + max retries.
98-
from google.adk.flows.llm_flows.base_llm_flow import _MAX_EMPTY_RESPONSE_RETRIES
94+
async def test_run_async_breaks_on_no_last_event():
95+
"""Test that run_async breaks when there is no last event."""
96+
# content=None is filtered by _postprocess_async, so no events are
97+
# yielded and last_event stays None. The loop should break immediately
98+
# (no retry) because this is legitimate for sub-agents.
99+
empty_response = LlmResponse(content=None, partial=False)
99100

100-
empty_responses = [
101-
LlmResponse(content=None, partial=False)
102-
for _ in range(_MAX_EMPTY_RESPONSE_RETRIES + 1)
103-
]
104-
105-
mock_model = testing_utils.MockModel.create(responses=empty_responses)
101+
mock_model = testing_utils.MockModel.create(responses=[empty_response])
106102

107103
agent = Agent(name='test_agent', model=mock_model)
108104
invocation_context = await testing_utils.create_invocation_context(
@@ -116,12 +112,8 @@ async def test_run_async_retries_then_breaks_on_no_last_event():
116112
async for event in flow.run_async(invocation_context):
117113
events.append(event)
118114

119-
# Resume events are appended to session (not yielded), so no user
120-
# events should appear in the output stream. Verify retries happened
121-
# by checking how many responses were consumed.
122-
assert mock_model.response_index == _MAX_EMPTY_RESPONSE_RETRIES
123-
leaked = [e for e in events if e.author == 'user']
124-
assert len(leaked) == 0, 'Resume messages must not leak to output'
115+
# Should have no events because empty responses are filtered out
116+
assert len(events) == 0
125117

126118

127119
@pytest.mark.asyncio

0 commit comments

Comments
 (0)