Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/google/adk/a2a/converters/event_converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ def _get_context_metadata(
("custom_metadata", event.custom_metadata),
("usage_metadata", event.usage_metadata),
("error_code", event.error_code),
("actions", event.actions),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The actions field on an Event object is initialized with default_factory=EventActions, so event.actions will never be None. As a result, it will always be serialized and included in the metadata, even if it's an empty object representing no actions. This adds unnecessary data to every A2A event.

To make this more efficient, the actions field should only be included when it contains meaningful data. The ideal solution would be to change the definition in src/google/adk/events/event.py to actions: Optional[EventActions] = None. This would allow the existing if field_value is not None: check to correctly filter out cases where no actions are present. Since that change is outside this PR's scope, this is a good candidate for a follow-up task.

]

for field_name, field_value in optional_fields:
Expand Down
9 changes: 7 additions & 2 deletions tests/unittests/a2a/converters/test_event_converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ def setup_method(self):
self.mock_event.error_message = None
self.mock_event.content = None
self.mock_event.long_running_tool_ids = None
self.mock_event.actions = Mock(spec=EventActions)
self.mock_event.actions.artifact_delta = None
self.mock_event.actions = None

def test_get_adk_event_metadata_key_success(self):
"""Test successful metadata key generation."""
Expand Down Expand Up @@ -161,6 +160,8 @@ def test_get_context_metadata_with_optional_fields(self):
mock_metadata = Mock()
mock_metadata.model_dump.return_value = {"test": "value"}
self.mock_event.grounding_metadata = mock_metadata
self.mock_event.actions = Mock()
self.mock_event.actions.model_dump.return_value = {"test_actions": "value"}

result = _get_context_metadata(
self.mock_event, self.mock_invocation_context
Expand All @@ -169,7 +170,11 @@ def test_get_context_metadata_with_optional_fields(self):
assert result is not None
assert f"{ADK_METADATA_KEY_PREFIX}branch" in result
assert f"{ADK_METADATA_KEY_PREFIX}grounding_metadata" in result
assert f"{ADK_METADATA_KEY_PREFIX}actions" in result
assert result[f"{ADK_METADATA_KEY_PREFIX}branch"] == "test-branch"
assert result[f"{ADK_METADATA_KEY_PREFIX}actions"] == {
"test_actions": "value"
}
Comment on lines +175 to +177
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This test assertion highlights a pre-existing inconsistency in event_converter.py. The _get_context_metadata function is type-hinted to return Dict[str, str], but this test correctly asserts that the value for the actions key is a dictionary, not a string.

While the A2A server expects Dict[str, Any] for metadata and this will work at runtime, the incorrect type hints are misleading and harm code maintainability.

To resolve this, the type hints in event_converter.py should be updated in a follow-up:

  • _get_context_metadata's return type should be Dict[str, Any].
  • _serialize_metadata_value's name, docstring, and return type hint should be changed to reflect that it can return non-string values (e.g., return type Any).


# Check if error_code is in the result - it should be there since we set it
if f"{ADK_METADATA_KEY_PREFIX}error_code" in result:
Expand Down