Skip to content

feat: filter duplicate A2A event parts per conversation turn in agent executor#1092

Open
nan-yu wants to merge 2 commits intogoogle:mainfrom
nan-yu:orchestrator
Open

feat: filter duplicate A2A event parts per conversation turn in agent executor#1092
nan-yu wants to merge 2 commits intogoogle:mainfrom
nan-yu:orchestrator

Conversation

@nan-yu
Copy link
Copy Markdown
Collaborator

@nan-yu nan-yu commented Apr 8, 2026

Description

Before

Duplicate.results.webm

After

Duplicate.results.-.fix.webm

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds a filtering mechanism for duplicate A2A event parts in agent_executor.py. Feedback highlights a memory leak and thread-safety risk from using a class-level dictionary, a logic inconsistency in the reset conditions, and performance overhead from JSON serialization. Furthermore, the PR lacks required unit tests and a completed pre-review checklist as specified in the style guide.

Comment thread samples/agent/adk/orchestrator/agent_executor.py Outdated
Comment thread samples/agent/adk/orchestrator/agent_executor.py Outdated
Comment thread samples/agent/adk/orchestrator/agent_executor.py
Comment thread samples/agent/adk/orchestrator/agent_executor.py
Comment thread samples/agent/adk/orchestrator/agent_executor.py Outdated
@nan-yu nan-yu changed the title feat: filter duplicate A2A event parts per session in agent executor feat: filter duplicate A2A event parts per conversation turn in agent executor Apr 8, 2026
Copy link
Copy Markdown
Collaborator

@jacobsimionato jacobsimionato left a comment

Choose a reason for hiding this comment

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

Do you understand the source of the duplicates here? I worry that this change might mask real issues elsewhere in the system. Shouldn't ADK etc manage this correctly under the hood?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

2 participants