Skip to content

Commit be8fcf4

Browse files
DeanChensjcopybara-github
authored andcommitted
docs: correct the documented contract of the invocation-subtree event filter
`_get_events(current_branch=True)` documented itself as returning events on "the current branch or any descendant sub-branch", but that is only the user arm, and not even all of it. The docstring now states the whole rule: * A user event matches this branch, a descendant sub-branch, or no branch. * When `self.branch` is `None`, every user event matches whatever branch it is on. An empty-string branch is the opposite rather than a synonym: it is a real value in the workflow code and matches no branched event. * A user event carrying function responses must additionally answer a call issued on this branch or below it, and is dropped otherwise even when it sits on exactly this branch. * Every other event must sit on exactly this branch. So a confirmation answered by the user on a sub-branch is visible while the agent event that requested it is not. The asymmetry needs a per-caller review before it is widened -- not because sibling trees would become reachable (`_BranchPath.is_descendant_of` already excludes those) but because every caller would start seeing a descendant's internal events. No behavior change. The docstring also notes that `contents._is_event_belongs_to_branch` deliberately matches the *opposite* direction, because it answers a different question -- "what history may this agent see?" rather than "what happened inside this invocation?". The descendant test also now uses `_BranchPath.is_descendant_of` instead of a hand-rolled `startswith(f"{branch}.")`. That helper was already imported and used elsewhere in the same file. The empty-branch guard is kept: an empty branch is a real value in the workflow code, and a bare descendant test would treat every branched event as its descendant. Co-authored-by: Shangjie Chen <deanchen@google.com> PiperOrigin-RevId: 970789610
1 parent ef2d680 commit be8fcf4

2 files changed

Lines changed: 170 additions & 4 deletions

File tree

src/google/adk/agents/invocation_context.py

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,36 @@ def _get_events(
449449
if current_branch:
450450

451451
def _is_branch_match(event: Event) -> bool:
452-
"""Determines if an event belongs to the current branch or any descendant sub-branch."""
452+
"""Determines whether an event is part of this invocation's subtree.
453+
454+
The rule differs by author, deliberately but asymmetrically.
455+
456+
A user event matches when it sits on this branch, on a descendant
457+
sub-branch (e.g. a child NodeTool/WorkflowTool execution tree), or on
458+
no branch at all; and when ``self.branch`` is ``None``, every user
459+
event matches whatever branch it is on. An empty-string branch is the
460+
opposite rather than a synonym for that: it is a real branch value in
461+
the workflow code and matches no branched event. A user event carrying
462+
function responses must additionally answer a function call issued on
463+
this branch or below it -- one answering a call from anywhere else is
464+
dropped even when it sits on exactly this branch, which is what keeps
465+
a reply from leaking across parallel trees.
466+
467+
Any other event must sit on exactly this branch; a descendant's own
468+
events are not returned.
469+
470+
So a confirmation answered by the user on a sub-branch is visible here
471+
while the agent event that requested it is not. Widening the non-user
472+
rule to descendants would not expose sibling trees --
473+
``_BranchPath.is_descendant_of`` excludes those -- but it would hand
474+
every caller a descendant's internal events, so it needs a per-caller
475+
review rather than a blanket change.
476+
477+
Note this is the opposite direction from
478+
``contents._is_event_belongs_to_branch``, which asks a different
479+
question -- "what history may this agent see?" -- and so matches
480+
*ancestor* branches instead. Both are intended.
481+
"""
453482
if getattr(event, "author", None) == "user":
454483
frs = event.get_function_responses()
455484
if frs and self.branch and self.session:
@@ -477,16 +506,24 @@ def _is_branch_match(event: Event) -> bool:
477506
if not (fr_ids & branch_fc_ids):
478507
return False
479508

480-
# Match events yielded directly on this branch or on descendant sub-branches
481-
# (e.g. child NodeTool/WorkflowTool execution trees).
509+
# Match events yielded directly on this branch or on descendant
510+
# sub-branches (e.g. child NodeTool/WorkflowTool execution trees).
511+
# The `self.branch` guard keeps an empty branch from matching every
512+
# branched event, which a bare descendant test would do.
482513
if (
483514
event.branch is None
484515
or self.branch is None
485516
or event.branch == self.branch
486-
or (self.branch and event.branch.startswith(f"{self.branch}."))
517+
or (
518+
self.branch
519+
and _BranchPath.from_string(event.branch).is_descendant_of(
520+
_BranchPath.from_string(self.branch)
521+
)
522+
)
487523
):
488524
return True
489525
return False
526+
# Non-user events: exactly this branch, per the docstring above.
490527
return event.branch == self.branch
491528

492529
results = [e for e in results if _is_branch_match(e)]

tests/unittests/agents/test_invocation_context.py

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
from google.adk.sessions.session import Session
2727
from google.genai.types import Content
2828
from google.genai.types import FunctionCall
29+
from google.genai.types import FunctionResponse
2930
from google.genai.types import Part
3031
import pytest
3132

@@ -794,3 +795,131 @@ def test_count_is_per_invocation_context(self):
794795

795796
with pytest.raises(LlmCallsLimitExceededError):
796797
second.increment_llm_call_count()
798+
799+
800+
def _ctx_on_branch(branch, events):
801+
"""An InvocationContext on `branch` over a session holding `events`."""
802+
return InvocationContext(
803+
session_service=Mock(spec=BaseSessionService),
804+
agent=Mock(spec=BaseAgent),
805+
invocation_id='inv_1',
806+
branch=branch,
807+
session=Mock(spec=Session, events=events),
808+
)
809+
810+
811+
def test_get_events_current_branch_includes_user_event_on_sub_branch():
812+
"""A user event from a descendant sub-branch belongs to this subtree."""
813+
user_on_child = Event(
814+
invocation_id='inv_1', author='user', branch='agent_1.child'
815+
)
816+
ctx = _ctx_on_branch('agent_1', [user_on_child])
817+
818+
assert ctx._get_events(current_branch=True) == [user_on_child]
819+
820+
821+
def test_get_events_current_branch_excludes_agent_event_on_sub_branch():
822+
"""A non-user event from a descendant sub-branch is not returned.
823+
824+
This is asymmetric with the user case above on purpose: widening it would
825+
hand every caller a descendant's internal events. Pinned here so the
826+
asymmetry is a stated contract rather than an accident.
827+
"""
828+
agent_on_child = Event(
829+
invocation_id='inv_1', author='some_agent', branch='agent_1.child'
830+
)
831+
ctx = _ctx_on_branch('agent_1', [agent_on_child])
832+
833+
assert ctx._get_events(current_branch=True) == []
834+
835+
836+
def test_get_events_current_branch_excludes_sibling_branch():
837+
"""A sibling branch is never part of this subtree."""
838+
user_on_sibling = Event(
839+
invocation_id='inv_1', author='user', branch='agent_2'
840+
)
841+
ctx = _ctx_on_branch('agent_1', [user_on_sibling])
842+
843+
assert ctx._get_events(current_branch=True) == []
844+
845+
846+
def test_get_events_empty_branch_does_not_match_every_branched_event():
847+
"""An empty branch must not behave like "match everything".
848+
849+
An empty string is a real branch value in the workflow code, and a bare
850+
descendant test would treat every branched event as its descendant.
851+
"""
852+
user_on_branch = Event(invocation_id='inv_1', author='user', branch='agent_1')
853+
ctx = _ctx_on_branch('', [user_on_branch])
854+
855+
assert ctx._get_events(current_branch=True) == []
856+
857+
858+
def _call_event(branch, call_id):
859+
"""A non-user event issuing function call `call_id` on `branch`."""
860+
return Event(
861+
invocation_id='inv_1',
862+
author='some_agent',
863+
branch=branch,
864+
content=Content(
865+
parts=[Part(function_call=FunctionCall(id=call_id, name='t'))]
866+
),
867+
)
868+
869+
870+
def _user_response_event(branch, call_id):
871+
"""A user event answering function call `call_id` on `branch`."""
872+
return Event(
873+
invocation_id='inv_1',
874+
author='user',
875+
branch=branch,
876+
content=Content(
877+
parts=[
878+
Part(
879+
function_response=FunctionResponse(
880+
id=call_id, name='t', response={}
881+
)
882+
)
883+
]
884+
),
885+
)
886+
887+
888+
def test_get_events_current_branch_keeps_user_response_to_a_call_here():
889+
"""A reply answering a call issued in this subtree is returned."""
890+
call_here = _call_event('agent_1.child', 'fc_1')
891+
reply = _user_response_event('agent_1', 'fc_1')
892+
ctx = _ctx_on_branch('agent_1', [call_here, reply])
893+
894+
assert ctx._get_events(current_branch=True) == [reply]
895+
896+
897+
def test_get_events_current_branch_drops_user_response_to_a_call_elsewhere():
898+
"""Sitting on this branch is not enough for a reply to a foreign call.
899+
900+
The function-response gate is the only difference from the test above, so a
901+
reply that answers a parallel tree's call is dropped even though its own
902+
branch matches exactly.
903+
"""
904+
call_elsewhere = _call_event('agent_2', 'fc_1')
905+
reply = _user_response_event('agent_1', 'fc_1')
906+
ctx = _ctx_on_branch('agent_1', [call_elsewhere, reply])
907+
908+
assert ctx._get_events(current_branch=True) == []
909+
910+
911+
def test_get_events_without_a_branch_matches_every_user_event():
912+
"""A context with no branch sees user events wherever they sit.
913+
914+
Non-user events stay on the strict rule, so this also pins the asymmetry:
915+
the agent event beside it is not returned.
916+
"""
917+
user_elsewhere = Event(
918+
invocation_id='inv_1', author='user', branch='agent_2.child'
919+
)
920+
agent_elsewhere = Event(
921+
invocation_id='inv_1', author='some_agent', branch='agent_2.child'
922+
)
923+
ctx = _ctx_on_branch(None, [user_elsewhere, agent_elsewhere])
924+
925+
assert ctx._get_events(current_branch=True) == [user_elsewhere]

0 commit comments

Comments
 (0)