-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat(agents): add validation for unique sub-agent names (#3557) #3576
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
feat(agents): add validation for unique sub-agent names (#3557) #3576
Conversation
Summary of ChangesHello @sarojrout, 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 addresses a potential ambiguity issue where Highlights
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 a valuable validation check to ensure that sub-agents within a BaseAgent have unique names. The implementation is sound and the inclusion of unit tests is great. I've provided a couple of suggestions. One is to refine the error message to avoid redundancy when a name is duplicated multiple times. The other is to strengthen the unit tests to be more specific in their assertions, which will make them more robust against future changes. Overall, this is a solid contribution that improves the reliability of agent creation.
Change duplicates from list to set for better efficiency and cleaner code. Remove unnecessary set() conversion when sorting duplicates.
|
Hi @sarojrout , can you fix the lint errors. |
done. can you pls verify? thanks! |
|
Hi @slilichenko, can you please review this now. |
|
@ryanaiagent LGTM, but I am not an official reviewer/committer on this project. |
|
Hi @DeanChensj , can you please review this. |
Merge #3576 **Please ensure you have read the [contribution guide](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) before creating a pull request.** ### Link to Issue or Description of Change **1. Link to an existing issue (if applicable):** - Closes: #3557 - Related: #_issue_number_ **2. Or, if no issue exists, describe the change:** _If applicable, please follow the issue templates to provide as much detail as possible._ **Problem:** When creating a BaseAgent with multiple sub-agents, there was no validation to ensure that all sub-agents have unique names. This could lead to confusion when trying to find or reference specific sub-agents by name, as duplicate names would make it ambiguous which agent is being referenced. **Solution:** Added a @field_validator for the sub_agents field in BaseAgent that validates all sub-agents have unique names. The validator: Checks for duplicate names in the sub-agents list Raises a ValueError with a clear error message listing all duplicate names found Returns the validated list if all names are unique Handles edge cases like empty lists gracefully ### Testing Plan _Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes._ **Unit Tests:** - [x] I have added or updated unit tests for my change. - [x] All unit tests pass locally. _Please include a summary of passed `pytest` results._ Added 4 new test cases in tests/unittests/agents/test_base_agent.py: test_validate_sub_agents_unique_names_single_duplicate: Verifies that a single duplicate name raises ValueError test_validate_sub_agents_unique_names_multiple_duplicates: Verifies that multiple duplicate names are all reported in the error message test_validate_sub_agents_unique_names_no_duplicates: Verifies that unique names pass validation successfully test_validate_sub_agents_unique_names_empty_list: Verifies that empty sub-agents list passes validation All tests pass locally. You can run with: pytest tests/unittests/agents/test_base_agent.py::test_validate_sub_agents_unique_names_single_duplicate tests/unittests/agents/test_base_agent.py::test_validate_sub_agents_unique_names_multiple_duplicates tests/unittests/agents/test_base_agent.py::test_validate_sub_agents_unique_names_no_duplicates tests/unittests/agents/test_base_agent.py::test_validate_sub_agents_unique_names_empty_list -v **Manual End-to-End (E2E) Tests:** _Please provide instructions on how to manually test your changes, including any necessary setup or configuration. Please provide logs or screenshots to help reviewers better understand the fix._ Test Case 1: Duplicate names should raise error from google.adk.agents import Agent agent1 = Agent(name="sub_agent", model="gemini-2.5-flash") agent2 = Agent(name="sub_agent", model="gemini-2.5-flash") # Same name # This should raise ValueError try: parent = Agent( name="parent", model="gemini-2.5-flash", sub_agents=[agent1, agent2] ) except ValueError as e: print(f"Expected error: {e}") # Output: Found duplicate sub-agent names: `sub_agent`. All sub-agents must have unique names. Test Case 2: Unique names should work from google.adk.agents import Agent agent1 = Agent(name="agent1", model="gemini-2.5-flash") agent2 = Agent(name="agent2", model="gemini-2.5-flash") # This should work without error parent = Agent( name="parent", model="gemini-2.5-flash", sub_agents=[agent1, agent2] ) print("Success: Unique names validated correctly") ### Checklist - [x] I have read the [CONTRIBUTING.md](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) document. - [x] I have performed a self-review of my own code. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] New and existing unit tests pass locally with my changes. - [x] I have manually tested my changes end-to-end. - [x] Any dependent changes have been merged and published in downstream modules. ### Additional context This change adds validation at the BaseAgent level, so it automatically applies to all agent types that inherit from BaseAgent (e.g., LlmAgent, LoopAgent, etc.). The validation uses Pydantic's field validator system, which runs during object initialization, ensuring the constraint is enforced early and consistently. The error message clearly identifies which names are duplicated, making it easy for developers to fix the issue: COPYBARA_INTEGRATE_REVIEW=#3576 from sarojrout:feat/validate-unique-sub-agent-names 07adf1f PiperOrigin-RevId: 835358118
|
Thank you @sarojrout for your contribution! 🎉 Your changes have been successfully imported and merged via Copybara in commit 2247a45. Closing this PR as the changes are now in the main branch. |
Merge google#3576 **Please ensure you have read the [contribution guide](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) before creating a pull request.** ### Link to Issue or Description of Change **1. Link to an existing issue (if applicable):** - Closes: google#3557 - Related: #_issue_number_ **2. Or, if no issue exists, describe the change:** _If applicable, please follow the issue templates to provide as much detail as possible._ **Problem:** When creating a BaseAgent with multiple sub-agents, there was no validation to ensure that all sub-agents have unique names. This could lead to confusion when trying to find or reference specific sub-agents by name, as duplicate names would make it ambiguous which agent is being referenced. **Solution:** Added a @field_validator for the sub_agents field in BaseAgent that validates all sub-agents have unique names. The validator: Checks for duplicate names in the sub-agents list Raises a ValueError with a clear error message listing all duplicate names found Returns the validated list if all names are unique Handles edge cases like empty lists gracefully ### Testing Plan _Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes._ **Unit Tests:** - [x] I have added or updated unit tests for my change. - [x] All unit tests pass locally. _Please include a summary of passed `pytest` results._ Added 4 new test cases in tests/unittests/agents/test_base_agent.py: test_validate_sub_agents_unique_names_single_duplicate: Verifies that a single duplicate name raises ValueError test_validate_sub_agents_unique_names_multiple_duplicates: Verifies that multiple duplicate names are all reported in the error message test_validate_sub_agents_unique_names_no_duplicates: Verifies that unique names pass validation successfully test_validate_sub_agents_unique_names_empty_list: Verifies that empty sub-agents list passes validation All tests pass locally. You can run with: pytest tests/unittests/agents/test_base_agent.py::test_validate_sub_agents_unique_names_single_duplicate tests/unittests/agents/test_base_agent.py::test_validate_sub_agents_unique_names_multiple_duplicates tests/unittests/agents/test_base_agent.py::test_validate_sub_agents_unique_names_no_duplicates tests/unittests/agents/test_base_agent.py::test_validate_sub_agents_unique_names_empty_list -v **Manual End-to-End (E2E) Tests:** _Please provide instructions on how to manually test your changes, including any necessary setup or configuration. Please provide logs or screenshots to help reviewers better understand the fix._ Test Case 1: Duplicate names should raise error from google.adk.agents import Agent agent1 = Agent(name="sub_agent", model="gemini-2.5-flash") agent2 = Agent(name="sub_agent", model="gemini-2.5-flash") # Same name # This should raise ValueError try: parent = Agent( name="parent", model="gemini-2.5-flash", sub_agents=[agent1, agent2] ) except ValueError as e: print(f"Expected error: {e}") # Output: Found duplicate sub-agent names: `sub_agent`. All sub-agents must have unique names. Test Case 2: Unique names should work from google.adk.agents import Agent agent1 = Agent(name="agent1", model="gemini-2.5-flash") agent2 = Agent(name="agent2", model="gemini-2.5-flash") # This should work without error parent = Agent( name="parent", model="gemini-2.5-flash", sub_agents=[agent1, agent2] ) print("Success: Unique names validated correctly") ### Checklist - [x] I have read the [CONTRIBUTING.md](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) document. - [x] I have performed a self-review of my own code. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] New and existing unit tests pass locally with my changes. - [x] I have manually tested my changes end-to-end. - [x] Any dependent changes have been merged and published in downstream modules. ### Additional context This change adds validation at the BaseAgent level, so it automatically applies to all agent types that inherit from BaseAgent (e.g., LlmAgent, LoopAgent, etc.). The validation uses Pydantic's field validator system, which runs during object initialization, ensuring the constraint is enforced early and consistently. The error message clearly identifies which names are duplicated, making it easy for developers to fix the issue: COPYBARA_INTEGRATE_REVIEW=google#3576 from sarojrout:feat/validate-unique-sub-agent-names 07adf1f PiperOrigin-RevId: 835358118
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):
2. Or, if no issue exists, describe the change:
If applicable, please follow the issue templates to provide as much detail as
possible.
Problem:
When creating a BaseAgent with multiple sub-agents, there was no validation to ensure that all sub-agents have unique names. This could lead to confusion when trying to find or reference specific sub-agents by name, as duplicate names would make it ambiguous which agent is being referenced.
Solution:
Added a @field_validator for the sub_agents field in BaseAgent that validates all sub-agents have unique names. The validator:
Checks for duplicate names in the sub-agents list
Raises a ValueError with a clear error message listing all duplicate names found
Returns the validated list if all names are unique
Handles edge cases like empty lists gracefully
Testing Plan
Please describe the tests that you ran to verify your changes. This is required
for all PRs that are not small documentation or typo fixes.
Unit Tests:
Please include a summary of passed
pytestresults.Added 4 new test cases in tests/unittests/agents/test_base_agent.py:
test_validate_sub_agents_unique_names_single_duplicate: Verifies that a single duplicate name raises ValueError
test_validate_sub_agents_unique_names_multiple_duplicates: Verifies that multiple duplicate names are all reported in the error message
test_validate_sub_agents_unique_names_no_duplicates: Verifies that unique names pass validation successfully
test_validate_sub_agents_unique_names_empty_list: Verifies that empty sub-agents list passes validation
All tests pass locally. You can run with:
pytest tests/unittests/agents/test_base_agent.py::test_validate_sub_agents_unique_names_single_duplicate tests/unittests/agents/test_base_agent.py::test_validate_sub_agents_unique_names_multiple_duplicates tests/unittests/agents/test_base_agent.py::test_validate_sub_agents_unique_names_no_duplicates tests/unittests/agents/test_base_agent.py::test_validate_sub_agents_unique_names_empty_list -v
Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any
necessary setup or configuration. Please provide logs or screenshots to help
reviewers better understand the fix.
Test Case 1: Duplicate names should raise error
from google.adk.agents import Agent
agent1 = Agent(name="sub_agent", model="gemini-2.5-flash")
agent2 = Agent(name="sub_agent", model="gemini-2.5-flash") # Same name
This should raise ValueError
try:
parent = Agent(
name="parent",
model="gemini-2.5-flash",
sub_agents=[agent1, agent2]
)
except ValueError as e:
print(f"Expected error: {e}")
# Output: Found duplicate sub-agent names:
sub_agent. All sub-agents must have unique names.Test Case 2: Unique names should work
from google.adk.agents import Agent
agent1 = Agent(name="agent1", model="gemini-2.5-flash")
agent2 = Agent(name="agent2", model="gemini-2.5-flash")
This should work without error
parent = Agent(
name="parent",
model="gemini-2.5-flash",
sub_agents=[agent1, agent2]
)
print("Success: Unique names validated correctly")
Checklist
Additional context
This change adds validation at the BaseAgent level, so it automatically applies to all agent types that inherit from BaseAgent (e.g., LlmAgent, LoopAgent, etc.). The validation uses Pydantic's field validator system, which runs during object initialization, ensuring the constraint is enforced early and consistently.
The error message clearly identifies which names are duplicated, making it easy for developers to fix the issue: