-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Feature: MapAgent (#1828) #4001
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @ananyablonko, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this 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 introduces the MapAgent, a new agent that applies a single sub-agent to multiple inputs in parallel. This is a great addition to the ADK, addressing a need for parallel processing of dynamic data sets. The implementation is clean and leverages existing components like _merge_agent_run from ParallelAgent. The unit tests are comprehensive, covering various agent compositions and edge cases.
I've added a couple of comments to improve code style and maintainability. One is about using proper comments in the agent implementation, and the other suggests refactoring duplicated code in the tests by using an existing helper function. Overall, this is a solid contribution.
Fix comments and test (Gemini code assist)
|
/gemini review |
There was a problem hiding this 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 introduces the MapAgent, a new agent that applies a sub-agent to a list of inputs in parallel. The implementation is solid and the accompanying unit tests provide good coverage for various agent compositions. I've identified a critical bug in how sub-agents are cloned, which could lead to issues with parent_agent references. I've also suggested an improvement to make input parsing more robust against malformed data. Overall, this is a great addition to the ADK.
| # The event is removed only for this invocation. | ||
| ctx.session.events.pop(i) | ||
|
|
||
| agent_input = RootModel[list[str]].model_validate_json(input_message).root |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current implementation assumes the input message is always a valid JSON-serialized list of strings. If input_message is an empty string or not a valid JSON, model_validate_json will raise a ValidationError, causing the agent to crash. It's better to handle this potential error gracefully.
I suggest wrapping this call in a try...except block to catch ValidationError. If parsing fails, you could log a warning and default to an empty list of prompts, making the agent more robust.
You'll also need to add import logging, from pydantic import ValidationError, and initialize a logger: logger = logging.getLogger("google_adk." + __name__) at the top of the file.
| agent_input = RootModel[list[str]].model_validate_json(input_message).root | |
| try: | |
| agent_input = RootModel[list[str]].model_validate_json(input_message).root | |
| except ValidationError: | |
| logger.warning("Invalid input for MapAgent, expected a JSON list of strings, got: %s", input_message) | |
| agent_input = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Respectfully, I believe invoking this agent with a non-list input is a developer error. In that case, wrong input should cause a crash.
For now, I added a test to ensure this behavior.
|
Hi @ananyablonko , Thank you for your contribution! We appreciate you taking the time to submit this pull request. |
Adds basic functionality of MapAgent.
ADK misses a map functionality, where a single agent operates in parallel on multiple data.
For example, if I want to create N images in parallel, where N is only known at runtime, this cannot be done using ADK.
To circumvent this, one might use a Parallel Agent with N agents that are clones of one another, and create the agent dynamically. This causes problems as explained in the linked feature request.
A MapAgent comes to the rescue!
Sample Usage of MapAgent:
The PR includes 100% coverage unittests that use all kinds of agents as sub agents or parents of the MapAgent.