Skip to content

Commit a30851e

Browse files
Jacksunweicopybara-github
authored andcommitted
fix: Fixes thought handling in contents.py and refactors its unit tests
Before this change, other agent's reply with thought will still be inserted in the outgoing LlmRequest due to the wrong `else` statement for calling all other type of part. This commit also refactors test_contents.py to be behavior-oriented tests, instead of implementation-oriented, and add more test cases to cover expected scenarios. The tests are divided into the following files with different focus: - test_contents.py: covers the basic logic of event filter; - test_contents_branch.py: covers the behavior related to branch, which takes effect when ParallelAgent is used. - test_contents_other_agent.py: covers the retelling behavior to include other agents' reply as context for the current agent. - test_contents_function.py: covers the function_call/function_response rearrangement logic mainly for `LongRunningFunctionTool`. PiperOrigin-RevId: 802759821
1 parent fe8b37b commit a30851e

File tree

8 files changed

+1604
-528
lines changed

8 files changed

+1604
-528
lines changed

src/google/adk/agents/remote_a2a_agent.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@
6363
from ..a2a.logs.log_utils import build_a2a_response_log
6464
from ..agents.invocation_context import InvocationContext
6565
from ..events.event import Event
66-
from ..flows.llm_flows.contents import _convert_foreign_event
6766
from ..flows.llm_flows.contents import _is_other_agent_reply
67+
from ..flows.llm_flows.contents import _present_other_agent_message
6868
from ..flows.llm_flows.functions import find_matching_function_call
6969
from .base_agent import BaseAgent
7070

@@ -338,7 +338,7 @@ def _construct_message_parts_from_session(
338338
context_id = None
339339
for event in reversed(ctx.session.events):
340340
if _is_other_agent_reply(self.name, event):
341-
event = _convert_foreign_event(event)
341+
event = _present_other_agent_message(event)
342342
elif event.author == self.name:
343343
# stop on content generated by current a2a agent given it should already
344344
# be in remote session

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

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import copy
1818
from typing import AsyncGenerator
19-
from typing import Generator
2019
from typing import Optional
2120

2221
from google.genai import types
@@ -203,6 +202,25 @@ def _rearrange_events_for_latest_function_response(
203202
return result_events
204203

205204

205+
def _contains_empty_content(event: Event) -> bool:
206+
"""Check if an event should be skipped due to missing or empty content.
207+
208+
This can happen to the evnets that only changed session state.
209+
210+
Args:
211+
event: The event to check.
212+
213+
Returns:
214+
True if the event should be skipped, False otherwise.
215+
"""
216+
return (
217+
not event.content
218+
or not event.content.role
219+
or not event.content.parts
220+
or event.content.parts[0].text == ''
221+
)
222+
223+
206224
def _get_contents(
207225
current_branch: Optional[str], events: list[Event], agent_name: str = ''
208226
) -> list[types.Content]:
@@ -222,16 +240,7 @@ def _get_contents(
222240
# Parse the events, leaving the contents and the function calls and
223241
# responses from the current agent.
224242
for event in events:
225-
if (
226-
not event.content
227-
or not event.content.role
228-
or not event.content.parts
229-
or event.content.parts[0].text == ''
230-
):
231-
# Skip events without content, or generated neither by user nor by model
232-
# or has empty text.
233-
# E.g. events purely for mutating session states.
234-
243+
if _contains_empty_content(event):
235244
continue
236245
if not _is_event_belongs_to_branch(current_branch, event):
237246
# Skip events not belong to current branch.
@@ -242,11 +251,12 @@ def _get_contents(
242251
if _is_request_confirmation_event(event):
243252
# Skip request confirmation events.
244253
continue
245-
filtered_events.append(
246-
_convert_foreign_event(event)
247-
if _is_other_agent_reply(agent_name, event)
248-
else event
249-
)
254+
255+
if _is_other_agent_reply(agent_name, event):
256+
if converted_event := _present_other_agent_message(event):
257+
filtered_events.append(converted_event)
258+
else:
259+
filtered_events.append(event)
250260

251261
# Rearrange events for proper function call/response pairing
252262
result_events = _rearrange_events_for_latest_function_response(
@@ -305,19 +315,18 @@ def _is_other_agent_reply(current_agent_name: str, event: Event) -> bool:
305315
)
306316

307317

308-
def _convert_foreign_event(event: Event) -> Event:
309-
"""Converts an event authored by another agent as a user-content event.
318+
def _present_other_agent_message(event: Event) -> Optional[Event]:
319+
"""Presents another agent's message as user context for the current agent.
310320
311-
This is to provide another agent's output as context to the current agent, so
312-
that current agent can continue to respond, such as summarizing previous
313-
agent's reply, etc.
321+
Reformats the event with role='user' and adds '[agent_name] said:' prefix
322+
to provide context without confusion about authorship.
314323
315324
Args:
316-
event: The event to convert.
325+
event: The event from another agent to present as context.
317326
318327
Returns:
319-
The converted event.
320-
328+
Event reformatted as user-role context with agent attribution, or None
329+
if no meaningful content remains after filtering.
321330
"""
322331
if not event.content or not event.content.parts:
323332
return event
@@ -326,8 +335,10 @@ def _convert_foreign_event(event: Event) -> Event:
326335
content.role = 'user'
327336
content.parts = [types.Part(text='For context:')]
328337
for part in event.content.parts:
329-
# Exclude thoughts from the context.
330-
if part.text and not part.thought:
338+
if part.thought:
339+
# Exclude thoughts from the context.
340+
continue
341+
elif part.text:
331342
content.parts.append(
332343
types.Part(text=f'[{event.author}] said: {part.text}')
333344
)
@@ -354,6 +365,10 @@ def _convert_foreign_event(event: Event) -> Event:
354365
else:
355366
content.parts.append(part)
356367

368+
# If no meaningful parts were added (only "For context:" remains), return None
369+
if len(content.parts) == 1:
370+
return None
371+
357372
return Event(
358373
timestamp=event.timestamp,
359374
author='user',
@@ -429,7 +444,11 @@ def _merge_function_response_events(
429444
def _is_event_belongs_to_branch(
430445
invocation_branch: Optional[str], event: Event
431446
) -> bool:
432-
"""Event belongs to a branch, when event.branch is prefix of the invocation branch."""
447+
"""Check if an event belongs to the current branch.
448+
449+
This is for event context segration between agents. E.g. agent A shouldn't
450+
see output of agent B.
451+
"""
433452
if not invocation_branch or not event.branch:
434453
return True
435454
return invocation_branch.startswith(event.branch)

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,15 @@ def populate_client_function_call_id(model_response_event: Event) -> None:
6565
function_call.id = generate_client_function_call_id()
6666

6767

68-
def remove_client_function_call_id(content: types.Content) -> None:
68+
def remove_client_function_call_id(content: Optional[types.Content]) -> None:
69+
"""Removes ADK-generated function call IDs from content before sending to LLM.
70+
71+
Strips client-side function call/response IDs that start with 'adk-' prefix
72+
to avoid sending internal tracking IDs to the model.
73+
74+
Args:
75+
content: Content containing function calls/responses to clean.
76+
"""
6977
if content and content.parts:
7078
for part in content.parts:
7179
if (

tests/unittests/agents/test_remote_a2a_agent.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,7 @@ def test_construct_message_parts_from_session_success(self):
515515
self.mock_session.events = [mock_event]
516516

517517
with patch(
518-
"google.adk.agents.remote_a2a_agent._convert_foreign_event"
518+
"google.adk.agents.remote_a2a_agent._present_other_agent_message"
519519
) as mock_convert:
520520
mock_convert.return_value = mock_event
521521

@@ -937,7 +937,7 @@ async def test_full_workflow_with_direct_agent_card(self):
937937

938938
# Mock dependencies
939939
with patch(
940-
"google.adk.agents.remote_a2a_agent._convert_foreign_event"
940+
"google.adk.agents.remote_a2a_agent._present_other_agent_message"
941941
) as mock_convert:
942942
mock_convert.return_value = mock_event
943943

0 commit comments

Comments
 (0)