feat: add plan reflection planner #2848
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @devin2255, 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 a new PlanReflectionPlanner aimed at improving the robustness and accuracy of agent responses. By explicitly integrating reflection and replanning phases into the existing ReAct framework, the agent can now dynamically evaluate its progress, identify issues, and adjust its strategy, leading to more refined and insightful outcomes.
Highlights
- New Planner Introduction: Introduced a new
PlanReflectionPlannerdesigned to enhance the agent's reasoning by incorporating explicit reflection and replanning steps into the ReAct cycle. - Enhanced Planning Process: The new planner defines a structured, multi-phase process including Initial Planning, Execution, Reflection, Replanning, Iterative Execution, and Final Answer, with specific tags for each phase.
- Module Export: The
PlanReflectionPlannerhas been added to the__all__list insrc/google/adk/planners/__init__.pyto make it discoverable and usable within theplannersmodule.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
|
Response from ADK Triaging Agent Hello @devin2255, thank you for creating this PR! To help reviewers evaluate your contribution, could you please address the following points from our contribution guidelines?
You can find more details in our contribution guidelines. This information will help reviewers to review your PR more efficiently. Thanks! |
There was a problem hiding this comment.
Code Review
This pull request introduces a PlanReflectionPlanner to enforce a reflection step in the ReAct cycle. The implementation is mostly solid, but I've found a critical bug in the response processing logic that could lead to dropped function calls. I've also included several suggestions to improve code consistency, readability, and adherence to Python best practices, such as using consistent comment language, optimizing tag checking, and cleaning up string formatting. Please review the detailed comments.
|
|
||
| self._handle_non_function_call_parts(response_parts[i], preserved_parts) | ||
|
|
||
| if first_fc_part_index > 0: |
There was a problem hiding this comment.
There's a logic error in how function call parts are processed. If the first part in response_parts is a function call, first_fc_part_index will be 0, and the condition if first_fc_part_index > 0: will be false. This causes the subsequent loop that processes consecutive function calls to be skipped, potentially leading to dropped function calls from the response.
| if first_fc_part_index > 0: | |
| if first_fc_part_index != -1: |
| from ..models.llm_request import LlmRequest | ||
| from .base_planner import BasePlanner | ||
|
|
||
| # 使用与 PlanReActPlanner 兼容的标签 |
There was a problem hiding this comment.
This comment is in Chinese. For consistency with the rest of the codebase, which is primarily in English, please translate it.
| # 使用与 PlanReActPlanner 兼容的标签 | |
| # Use tags compatible with PlanReActPlanner |
| def _handle_non_function_call_parts( | ||
| self, response_part: types.Part, preserved_parts: list[types.Part] | ||
| ): |
There was a problem hiding this comment.
The type hint list[types.Part] is used here, but other parts of the file (e.g., line 36) use List[types.Part] from the typing module. For consistency and compatibility with Python versions before 3.9, it's better to use List from typing.
| def _handle_non_function_call_parts( | |
| self, response_part: types.Part, preserved_parts: list[types.Part] | |
| ): | |
| def _handle_non_function_call_parts( | |
| self, response_part: types.Part, preserved_parts: List[types.Part] | |
| ): |
| preserved_parts.append(types.Part(text=final_answer_text)) | ||
| else: | ||
| response_text = response_part.text or '' | ||
| # 包含所有标签，包括 REFLECTION_TAG |
There was a problem hiding this comment.
This comment is in Chinese. For consistency with the rest of the codebase, which is in English, please translate it.
| # 包含所有标签，包括 REFLECTION_TAG | |
| # Contains all tags, including REFLECTION_TAG |
| if response_text and ( | ||
| any( | ||
| response_text.startswith(tag) | ||
| for tag in [ | ||
| PLANNING_TAG, | ||
| REASONING_TAG, | ||
| ACTION_TAG, | ||
| REFLECTION_TAG, | ||
| REPLANNING_TAG, | ||
| ] | ||
| ) | ||
| ): |
There was a problem hiding this comment.
The current implementation for checking if response_text starts with a specific tag creates a new list of tags and uses any() with a generator expression on every call. This can be made more efficient and readable. The startswith() string method can accept a tuple of prefixes, which is more performant than iterating with any(). Consider defining the tags as a constant tuple at the class or module level for further optimization.
if response_text and response_text.startswith((
PLANNING_TAG,
REASONING_TAG,
ACTION_TAG,
REFLECTION_TAG,
REPLANNING_TAG,
)):| """Marks the response part as thought.""" | ||
| if response_part.text: | ||
| response_part.thought = True | ||
| return |
There was a problem hiding this comment.
This return statement is redundant in a function that does not return a value. It can be safely removed to make the code cleaner.
| def _build_reflection_planner_instruction(self) -> str: | ||
| """Builds the reflection planner instruction with stronger enforcement.""" | ||
|
|
||
| high_level_preamble = f""" |
There was a problem hiding this comment.
The multiline strings for the prompts (like high_level_preamble, planning_preamble, etc.) contain significant leading whitespace in the source code, which can affect readability. Consider using textwrap.dedent to remove common leading whitespace from every line in these strings. This will allow you to indent the string content in your code to match the surrounding code, improving readability, without the indentation appearing in the final string.
Example:
import textwrap
my_string = textwrap.dedent("""
This is an example.
The leading whitespace will be removed.
""")|
|
||
| planning_preamble = f""" | ||
| {PLANNING_TAG} Requirements: | ||
| Create a numbered plan that breaks down the user query into actionable steps. Each step should specify which tools to use. |
There was a problem hiding this comment.
There is trailing whitespace at the end of this line. It should be removed for code cleanliness.
Create a numbered plan that breaks down the user query into actionable steps. Each step should specify which tools to use.|
|
||
| replanning_preamble = f""" | ||
| {REPLANNING_TAG} Requirements (conditional): | ||
| Only if reflection reveals issues, create a revised plan and execute it with new {ACTION_TAG} and {REASONING_TAG} sections. |
There was a problem hiding this comment.
There is trailing whitespace at the end of this line. It should be removed for code cleanliness.
Only if reflection reveals issues, create a revised plan and execute it with new {ACTION_TAG} and {REASONING_TAG} sections.
No description provided.