diff --git a/src/google/adk/flows/llm_flows/contents.py b/src/google/adk/flows/llm_flows/contents.py index 9b95a3a813..399e4c70c0 100644 --- a/src/google/adk/flows/llm_flows/contents.py +++ b/src/google/adk/flows/llm_flows/contents.py @@ -146,9 +146,14 @@ def _rearrange_events_for_async_function_responses_in_history( events: list[Event], ) -> list[Event]: """Rearrange the async function_response events in the history.""" + # `get_function_responses()` and `get_function_calls()` each rebuild a list + # by rescanning every part, and upstream calls them repeatedly per event. + # Compute both once per event and reuse them below. + responses_per_event = [event.get_function_responses() for event in events] + calls_per_event = [event.get_function_calls() for event in events] + function_call_id_to_response_events_index: dict[str, int] = {} - for i, event in enumerate(events): - function_responses = event.get_function_responses() + for i, function_responses in enumerate(responses_per_event): if function_responses: for function_response in function_responses: function_call_id = function_response.id @@ -158,14 +163,14 @@ def _rearrange_events_for_async_function_responses_in_history( return events result_events: list[Event] = [] - for event in events: - if event.get_function_responses(): + for i, event in enumerate(events): + if responses_per_event[i]: # function_response should be handled together with function_call below. continue - elif event.get_function_calls(): + elif calls_per_event[i]: function_response_events_indices = set() - for function_call in event.get_function_calls(): + for function_call in calls_per_event[i]: function_call_id = function_call.id if function_call_id in function_call_id_to_response_events_index: function_response_events_indices.add( @@ -287,6 +292,13 @@ def _rearrange_events_for_latest_function_response( return result_events +_INTERNAL_FUNCTION_NAMES = frozenset({ + 'adk_framework', + REQUEST_EUC_FUNCTION_CALL_NAME, + REQUEST_CONFIRMATION_FUNCTION_CALL_NAME, +}) + + def _is_part_invisible( p: types.Part, *, include_thoughts: bool = False ) -> bool: @@ -439,12 +451,51 @@ def _should_include_event_in_context( ev_iso = getattr(event, 'isolation_scope', None) if ev_iso != isolation_scope: return False + + # Single pass over the parts. `_contains_empty_content` and the three + # internal-event predicates below each re-walk `event.content.parts`; this + # derives the same two facts once. The returned expression keeps upstream's + # operand order and short-circuit structure exactly. + content = event.content + all_parts_invisible = True + is_internal_event = False + if content and content.parts: + for part in content.parts: + function_call = part.function_call + if function_call is not None: + if function_call.name in _INTERNAL_FUNCTION_NAMES: + is_internal_event = True + break + all_parts_invisible = False + continue + function_response = part.function_response + if function_response is not None: + if function_response.name in _INTERNAL_FUNCTION_NAMES: + is_internal_event = True + break + all_parts_invisible = False + continue + if all_parts_invisible and not _is_part_invisible( + part, include_thoughts=include_thoughts + ): + all_parts_invisible = False + + if is_internal_event: + return False + + if event.actions and event.actions.compaction: + contains_empty_content = False + else: + contains_empty_content = ( + not content + or not content.role + or not content.parts + or all_parts_invisible + ) and (not event.output_transcription and not event.input_transcription) + return not ( - _contains_empty_content(event, include_thoughts=include_thoughts) + contains_empty_content or not _is_event_belongs_to_branch(current_branch, event) - or _is_adk_framework_event(event) - or _is_auth_event(event) - or _is_request_confirmation_event(event) ) @@ -671,16 +722,19 @@ def _copy_content_for_request( if not parts: return new_content + if not strip_client_function_call_ids: + new_content.parts = [part.model_copy() for part in parts] + return new_content + new_parts = [] for part in parts: new_part = part.model_copy() - if strip_client_function_call_ids: - fc = new_part.function_call - if fc and fc.id and fc.id.startswith(AF_FUNCTION_CALL_ID_PREFIX): - new_part.function_call = fc.model_copy(update={'id': None}) - fr = new_part.function_response - if fr and fr.id and fr.id.startswith(AF_FUNCTION_CALL_ID_PREFIX): - new_part.function_response = fr.model_copy(update={'id': None}) + fc = new_part.function_call + if fc and fc.id and fc.id.startswith(AF_FUNCTION_CALL_ID_PREFIX): + new_part.function_call = fc.model_copy(update={'id': None}) + fr = new_part.function_response + if fr and fr.id and fr.id.startswith(AF_FUNCTION_CALL_ID_PREFIX): + new_part.function_response = fr.model_copy(update={'id': None}) new_parts.append(new_part) new_content.parts = new_parts return new_content @@ -741,9 +795,11 @@ def _get_contents( ) ] - has_compaction_events = any( - e.actions and e.actions.compaction for e in raw_filtered_events - ) + has_compaction_events = False + for e in raw_filtered_events: + if e.actions and e.actions.compaction: + has_compaction_events = True + break if has_compaction_events: events_to_process = _process_compaction_events(raw_filtered_events) diff --git a/tests/unittests/flows/llm_flows/test_contents.py b/tests/unittests/flows/llm_flows/test_contents.py index 7243fbe7f0..e95ffdc3fa 100644 --- a/tests/unittests/flows/llm_flows/test_contents.py +++ b/tests/unittests/flows/llm_flows/test_contents.py @@ -2041,3 +2041,118 @@ def test_task_input_user_content_preserves_non_ascii(): assert "שלום עולם" in text assert "北京" in text assert "\\u" not in text + + +def _internal_call_event(name: str, **kwargs) -> Event: + """An event whose only part is an internal ADK function call.""" + return Event( + invocation_id="inv_internal", + author="agent", + content=types.Content( + role="model", + parts=[ + types.Part( + function_call=types.FunctionCall( + id="fc-1", name=name, args={} + ) + ) + ], + ), + **kwargs, + ) + + +@pytest.mark.parametrize( + "internal_name", + [ + "adk_framework", + REQUEST_EUC_FUNCTION_CALL_NAME, + REQUEST_CONFIRMATION_FUNCTION_CALL_NAME, + ], +) +def test_internal_events_excluded_even_with_transcription(internal_name): + """Internal ADK events stay excluded whatever else the event carries. + + The exclusion is independent of the emptiness check, so an internal event is + filtered out even when a transcription or a compaction action would otherwise + make it non-empty. + """ + compaction = EventCompaction( + start_timestamp=1.0, + end_timestamp=2.0, + compacted_content=types.Content( + role="model", parts=[types.Part(text="summary")] + ), + ) + + plain = _internal_call_event(internal_name) + with_transcription = _internal_call_event( + internal_name, + input_transcription=types.Transcription(text="hello"), + ) + with_compaction = _internal_call_event( + internal_name, actions=EventActions(compaction=compaction) + ) + + for event in (plain, with_transcription, with_compaction): + assert not contents._should_include_event_in_context(None, event) + + +def test_internal_event_with_empty_role_and_transcription_excluded(): + """The internal-event exclusion does not depend on the content role. + + A transcription keeps the event from being empty, and an empty role keeps the + parts from counting as visible content; the internal function call must still + exclude the event. + """ + event = Event( + invocation_id="inv_internal_empty_role", + author="agent", + content=types.Content( + role="", + parts=[ + types.Part( + function_call=types.FunctionCall( + id="fc-1", name="adk_framework", args={} + ) + ) + ], + ), + input_transcription=types.Transcription(text="hello"), + ) + + assert not contents._should_include_event_in_context(None, event) + + +def test_compaction_event_included_but_still_branch_filtered(): + """A compaction event is not empty, but the other filters still apply.""" + compaction = EventCompaction( + start_timestamp=1.0, + end_timestamp=2.0, + compacted_content=types.Content( + role="model", parts=[types.Part(text="summary")] + ), + ) + event = Event( + invocation_id="inv_compaction", + author="agent", + branch="root.other_agent", + content=types.Content(role="model", parts=[types.Part(text="hi")]), + actions=EventActions(compaction=compaction), + ) + + assert contents._should_include_event_in_context(None, event) + assert not contents._should_include_event_in_context( + "root.third_agent", event + ) + + +def test_content_with_parts_but_empty_role_is_empty(): + """Emptiness depends on the role as well as on the parts.""" + event = Event( + invocation_id="inv_empty_role", + author="agent", + content=types.Content(role="", parts=[types.Part(text="hi")]), + ) + + assert not contents._should_include_event_in_context(None, event)