feat(tools): add propagate_conversation_history option to AgentTool#6020
feat(tools): add propagate_conversation_history option to AgentTool#6020GiulioCMSanto wants to merge 3 commits into
Conversation
|
Response from ADK Triaging Agent Hello @GiulioCMSanto, thank you for creating this PR! It looks like a great addition to While you have provided an excellent description of your manual E2E test, to fully align with our contribution guidelines, could you please provide:
This information will help reviewers verify and understand the changes more efficiently. Thank you! |
|
I will start by reading the instructions in the The complete and detailed pull request analysis is available in the created artifact: Executive Summary of the PR
|
|
Hi @GiulioCMSanto , Thank you for your contribution! We appreciate you taking the time to submit this pull request. Can you please fix the failing mypy-diff tests before we can proceed with the review. |
rohityan
added
the
request clarification
Done @rohityan ! Thank you for considering the PR 😄 |
There was a problem hiding this comment.
Hi @GiulioCMSanto, thanks for puting together this PR to add conversational history propagation! It’s a great addition for improving multi-turn awareness in tool agents.
To prevent potential token overflow or context-window issues in longer sessions, I've left a few suggestions to introduce an optional "max_history_messages" configuration parameter. This allows users to cap the injected history to the most recent N eligible turns.
I've also appended a unit test to verify the capping logic and parameter forwarding.
| if self.propagate_conversation_history: | ||
| await _inject_conversation_history( | ||
| tool_context, runner.session_service, session | ||
| ) |
There was a problem hiding this comment.
Pass the configured "max_history_messages" limit down into the helper function when injecting the history.
| if self.propagate_conversation_history: | |
| await _inject_conversation_history( | |
| tool_context, runner.session_service, session | |
| ) | |
| if self.propagate_conversation_history: | |
| await _inject_conversation_history( | |
| tool_context, | |
| runner.session_service, | |
| session, | |
| max_history_messages=self.max_history_messages, | |
| ) |
| *, | ||
| include_plugins: bool = True, | ||
| propagate_grounding_metadata: bool = False, | ||
| propagate_conversation_history: bool = False, |
There was a problem hiding this comment.
Accept max_history_messages as an optional configuration keyword argument in the AgentTool constructor.
| propagate_conversation_history: bool = False, | |
| propagate_conversation_history: bool = False, | |
| max_history_messages: Optional[int] = None, |
| self.skip_summarization: bool = skip_summarization | ||
| self.include_plugins = include_plugins | ||
| self.propagate_grounding_metadata = propagate_grounding_metadata | ||
| self.propagate_conversation_history = propagate_conversation_history |
There was a problem hiding this comment.
Store max_history_messages as an instance variable on the AgentTool.
| self.propagate_conversation_history = propagate_conversation_history | |
| self.propagate_conversation_history = propagate_conversation_history | |
| self.max_history_messages = max_history_messages |
| *, | ||
| include_plugins: bool = True, | ||
| propagate_grounding_metadata: bool = False, | ||
| propagate_conversation_history: bool = False, |
There was a problem hiding this comment.
Expose the max_history_messages parameter on the _TaskAgentTool subclass constructor.
| propagate_conversation_history: bool = False, | |
| propagate_conversation_history: bool = False, | |
| max_history_messages: Optional[int] = None, |
| skip_summarization, | ||
| include_plugins=include_plugins, | ||
| propagate_grounding_metadata=propagate_grounding_metadata, | ||
| propagate_conversation_history=propagate_conversation_history, |
There was a problem hiding this comment.
Forward the max_history_messages argument to the base AgentTool constructor via super().init.
| propagate_conversation_history=propagate_conversation_history, | |
| propagate_conversation_history=propagate_conversation_history, | |
| max_history_messages=max_history_messages, |
| async def _inject_conversation_history( | ||
| tool_context: ToolContext, | ||
| session_service: BaseSessionService, | ||
| session: Session, | ||
| ) -> None: |
There was a problem hiding this comment.
Add the optional max_history_messages parameter to the history injection helper function.
| async def _inject_conversation_history( | |
| tool_context: ToolContext, | |
| session_service: BaseSessionService, | |
| session: Session, | |
| ) -> None: | |
| async def _inject_conversation_history( | |
| tool_context: ToolContext, | |
| session_service: BaseSessionService, | |
| session: Session, | |
| max_history_messages: Optional[int] = None, | |
| ) -> None: |
| Args: | ||
| tool_context: The parent tool context containing the session to read | ||
| from. | ||
| session_service: The child runner's session service for appending | ||
| events. | ||
| session: The child session to inject events into. |
There was a problem hiding this comment.
Update max_history_messages argument in the helper function's docstring.
| Args: | |
| tool_context: The parent tool context containing the session to read | |
| from. | |
| session_service: The child runner's session service for appending | |
| events. | |
| session: The child session to inject events into. | |
| Args: | |
| tool_context: The parent tool context containing the session to read from. | |
| session_service: The child runner's session service for appending events. | |
| session: The child session to inject events into. | |
| max_history_messages: The maximum number of recent conversation turns to inject. |
| for parent_event in tool_context.session.events: | ||
| if not parent_event.content or not parent_event.content.parts: | ||
| continue | ||
| if parent_event.content.role not in ('user', 'model'): | ||
| continue | ||
| text_parts = [ | ||
| part | ||
| for part in parent_event.content.parts | ||
| if part.text and not part.thought | ||
| ] | ||
| if not text_parts: | ||
| continue | ||
| await session_service.append_event( | ||
| session, | ||
| Event( | ||
| author=parent_event.author, | ||
| content=types.Content( | ||
| role=parent_event.content.role, | ||
| parts=text_parts, | ||
| ), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Gather and filter the eligible history events first, then slice them to match the configured limit before injecting.
| for parent_event in tool_context.session.events: | |
| if not parent_event.content or not parent_event.content.parts: | |
| continue | |
| if parent_event.content.role not in ('user', 'model'): | |
| continue | |
| text_parts = [ | |
| part | |
| for part in parent_event.content.parts | |
| if part.text and not part.thought | |
| ] | |
| if not text_parts: | |
| continue | |
| await session_service.append_event( | |
| session, | |
| Event( | |
| author=parent_event.author, | |
| content=types.Content( | |
| role=parent_event.content.role, | |
| parts=text_parts, | |
| ), | |
| ), | |
| ) | |
| eligible_events = [] | |
| for parent_event in tool_context.session.events: | |
| if not parent_event.content or not parent_event.content.parts: | |
| continue | |
| if parent_event.content.role not in ('user', 'model'): | |
| continue | |
| text_parts = [ | |
| part | |
| for part in parent_event.content.parts | |
| if part.text and not part.thought | |
| ] | |
| if not text_parts: | |
| continue | |
| eligible_events.append((parent_event, text_parts)) | |
| if max_history_messages is not None: | |
| eligible_events = eligible_events[-max_history_messages:] | |
| for parent_event, text_parts in eligible_events: | |
| await session_service.append_event( | |
| session, | |
| Event( | |
| author=parent_event.author, | |
| content=types.Content( | |
| role=parent_event.content.role, | |
| parts=text_parts, | |
| ), | |
| ), | |
| ) |
| for part in content.parts | ||
| if part.text | ||
| ] | ||
| assert 'I need 2 bedrooms' in user_texts |
There was a problem hiding this comment.
| assert 'I need 2 bedrooms' in user_texts | |
| assert 'I need 2 bedrooms' in user_texts | |
| @mark.asyncio | |
| async def test_propagate_history_with_max_messages_caps_injection(monkeypatch): | |
| """Setting max_history_messages limits injected history to the most recent turns.""" | |
| child_session_events: list[Event] = [] | |
| class RecordingSessionService(InMemorySessionService): | |
| async def append_event(self, session, event): | |
| child_session_events.append(event) | |
| return await super().append_event(session, event) | |
| monkeypatch.setattr( | |
| 'google.adk.sessions.in_memory_session_service.InMemorySessionService', | |
| RecordingSessionService, | |
| ) | |
| tool_agent = Agent(name='tool_agent', model='test-model') | |
| # Limit history to the 2 most recent messages | |
| agent_tool = AgentTool( | |
| agent=tool_agent, | |
| propagate_conversation_history=True, | |
| max_history_messages=2, | |
| ) | |
| root_agent = Agent(name='root_agent', model='test-model', tools=[agent_tool]) | |
| parent_session_service = InMemorySessionService() | |
| parent_session = await parent_session_service.create_session( | |
| app_name='test_app', user_id='user' | |
| ) | |
| # Add 3 historical text events | |
| messages = ['turn 1', 'turn 2', 'turn 3'] | |
| for i, msg in enumerate(messages): | |
| await parent_session_service.append_event( | |
| parent_session, | |
| Event( | |
| author='user' if i % 2 == 0 else 'root_agent', | |
| content=types.Content( | |
| role='user' if i % 2 == 0 else 'model', | |
| parts=[types.Part.from_text(text=msg)], | |
| ), | |
| ), | |
| ) | |
| async def _empty_async_generator(): | |
| if False: | |
| yield None | |
| class StubRunner: | |
| def __init__(self, **kwargs): | |
| self.session_service = kwargs.get('session_service') | |
| from ..plugins.plugin_manager import PluginManager | |
| self.plugin_manager = PluginManager(plugins=kwargs.get('plugins')) | |
| def run_async(self, *args, **kwargs): | |
| return _empty_async_generator() | |
| async def close(self): | |
| pass | |
| monkeypatch.setattr('google.adk.runners.Runner', StubRunner) | |
| invocation_context = InvocationContext( | |
| invocation_id='inv-id', | |
| agent=root_agent, | |
| session=parent_session, | |
| session_service=parent_session_service, | |
| memory_service=InMemoryMemoryService(), | |
| plugin_manager=PluginManager(), | |
| run_config=RunConfig(), | |
| ) | |
| tool_context = ToolContext(invocation_context) | |
| await agent_tool.run_async( | |
| args={'request': 'do something'}, tool_context=tool_context | |
| ) | |
| # Only the last 2 turns should have been injected | |
| assert len(child_session_events) == 2 | |
| assert child_session_events[0].content.parts[0].text == 'turn 2' | |
| assert child_session_events[1].content.parts[0].text == 'turn 3' |
4 participants
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
Testing Plan
Unit Tests:
New tests added to
tests/unittests/tools/test_agent_tool.py:test_propagate_history_injects_user_and_model_eventstest_propagate_history_excludes_function_call_eventstest_propagate_history_creates_events_without_state_deltastate_delta(prevents child state corruption)test_propagate_history_disabled_by_defaultFalsebehavior is unchanged — zero history in childtest_propagate_history_child_sees_context_end_to_endManual End-to-End (E2E) Tests:
Tested with a multi-turn conversational agent where a parent routes to a tool agent for filter extraction. Without
propagate_conversation_history=True, the child misclassifies user constraints (treats mandatory requirements as soft preferences). With it enabled, the child correctly identifies non-negotiable requirements from the full conversation context.Checklist
Additional context
Implementation follows the
propagate_grounding_metadatapattern exactly:propagate_conversation_history: bool = FalseonAgentTool.__init___inject_conversation_historymodule-level helper (~25 lines)run_asyncaftercreate_session_TaskAgentTool.__init__Not modified (intentionally):
_SingleTurnAgentTool— usesrun_node(parent already shares context)AgentToolConfig/from_config— matching precedent (propagate_grounding_metadatadidn't wire through config either)Design decisions:
False— existing behavior unchanged for all current users.user/modeltext events are injected —function_call/function_responseevents are excluded because they reference tools that only exist in the parent runner.Eventobjects are created — preventsstate_deltafrom original events being replayed in the child session.thoughtparts are filtered out — internal reasoning shouldn't leak to the child.