You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
create_empty_state() only walked sub_agents, so graph-based Workflow roots crashed when the dev server tried to add the current session to an eval set. Workflow children live in workflow.graph.nodes, not sub_agents.
This updates the state traversal to:
keep the existing sub_agents walk for normal agents
also walk graph.nodes for Workflow-style roots and nested graph nodes
track visited objects so shared graph/agent nodes are not processed repeatedly
I have successfully performed a comprehensive, read-only analysis of Pull Request #6001 and its related issue #5995.
I have created a premium, structured review report for you. You can read the full, detailed analysis in the newly generated artifact:
👉 pr_analysis_report.md
Key Highlights & Summary of Findings
Google CLA Compliance Gate (Passed):
Verified that the contributor @he-yufeng has successfully signed and passed the standard Google CLA check (re-verified via standard GitHub checks status).
Root Cause Resolved:
The state initialization utility create_empty_state() in state.py previously assumed the root agent is a standard BaseAgent and traversed child elements via .sub_agents.
In graph-based Workflows (introduced in ADK 2.x), child nodes live in .graph.nodes, which produced an AttributeError and crashed the ADK Dev Server with a 500 Internal Server Error when developers attempted to save active sessions to evaluation sets.
The PR resolves this systematically by dynamically querying and traversing both sub_agents and graph.nodes. It also elegantly handles cyclic or shared node graphs using an object identity cycle-tracker (visited set).
Principles & Style Compliance (Pass with Nits):
Quality: The solution is robust and does not introduce any breaking changes to the public API namespace.
Testing: Excellent behavior-focused test coverage added in the new unit test file, utilizing 100% real components (Workflow, LlmAgent, and edge bounds) without mocking.
Styling/Typing Nits:
Nit 1: The new test file tests/unittests/cli/utils/test_state.py is missing the mandatory from __future__ import annotations header import.
Nit 2: Parameter typing in create_empty_state(...) was relaxed to Any instead of leveraging ADK's BaseNode interface (the common ancestor for both BaseAgent and Workflow), which could be annotated more strongly.
Next Steps and Recommendations
Recommendation: Approve with Nits. The patch is high-impact, low-risk, and handles DAG structures extremely cleanly. We can request
Hi @he-yufeng , Thank you for your contribution! We appreciate you taking the time to submit this pull request.
Can you address these 2 suggestions.
Nit 1: The new test file tests/unittests/cli/utils/test_state.py is missing the mandatory from __future__ import annotations header import.
Nit 2: Parameter typing in create_empty_state(...) was relaxed to Any instead of leveraging ADK's BaseNode interface (the common ancestor for both BaseAgent and Workflow), which could be annotated more strongly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eval[Component] This issue is related to evaluationrequest clarification[Status] The maintainer need clarification or more information from the author
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #5995.
create_empty_state()only walkedsub_agents, so graph-basedWorkflowroots crashed when the dev server tried to add the current session to an eval set. Workflow children live inworkflow.graph.nodes, notsub_agents.This updates the state traversal to:
sub_agentswalk for normal agentsgraph.nodesfor Workflow-style roots and nested graph nodesTesting