.agents/skills/adk-pr-triage/SKILL.md
This skill guides AI assistants in conducting a highly professional, rigorous, and constructive triage of GitHub pull requests (PRs) submitted to the google/adk-python repository. It parses the PR, retrieves its context, evaluates it against ADK's design, style, and testing principles, presents a premium analysis report, and authors tailored response feedback (such as structured push-back or approval comments) under direct user guidance.
[!IMPORTANT]
CRITICAL EXECUTION RULES: STOP AND ASK DECISION GATES
- MANDATORY PR ASSIGNMENT BLOCK GATE:
- BEFORE doing any metadata reading, diff-fetching, issue-viewing, or code analysis (Phase 1, Step 1.3 onwards), you MUST verify if the pull request is assigned to you (via the verification script).
- If the PR is NOT assigned to you:
- STOP calling tools and ask immediately: You must present the PR Assignment Block gate in your chat response:
"Pull Request #<pr_number> is NOT assigned to you. (Current assignees: <assignee_list>). Would you like to take over this Pull Request?"
- Wait for Instructions: Do NOT perform any code analysis or diff-fetching in this turn.
- Action Paths:
- Yes (Take Over): In your next turn, run the assignment command:
bashCRITICAL: Immediately after assigning, you MUST re-run the verification script to refresh the PR metadata with the updated assignees:gh pr edit <pr_number> --add-assignee "@me" --repo google/adk-pythonbashThen parse the updated PR details from the script's stdout and proceed with the remaining triage steps..venv/bin/python .agents/skills/adk-pr-triage/scripts/triage_pr.py <pr_number> --skip-update- No (Decline): Stop executing immediately and do not run any further tools or operations. State that triage has terminated.
- PR Analysis is strictly read-only: Do NOT create branches, modify workspace files, or post any comments in your first response (unless performing PR assignment under the takeover gate above).
- Triage Decision Gate: You must present your full in-depth PR review report first, and explicitly ask the user:
"Would you like me to push back on this pull request? (If yes, select one of the push-back reasons or write custom feedback, and I will author a professional and precise review message for you to review. If no, I will draft an approval response highlighting the positive aspects of the implementation.)" Wait for instructions before performing any branch creation or Gerrit push.
[!IMPORTANT] Strict Tooling Constraint: Do NOT use
curl,wget, or any HTTP requests to fetch PR/issue content. You MUST parse/extract the numbers and use strictly the customfetch_github_issue/fetch_github_prpython tools, theghcommand, or the helper scripts provided.
https://github.com/google/adk-python/pull/5885 -> 5885)..venv/bin/python .agents/skills/adk-pr-triage/scripts/triage_pr.py <pr_number> --skip-update
PR IS NOT ASSIGNED TO YOU: You MUST stop calling tools immediately, present the following assignment block decision gate in your chat response, and wait for the user's input:
"⚠️ Pull Request Assignment Block Pull Request #<pr_number> is NOT assigned to you. (Current assignees: <assignee_list>).
Would you like to take over this Pull Request?
- [Option 1]: Yes, take over Pull Request #<pr_number> (Assign the PR to myself and proceed with the triage analysis).
- [Option 2]: No, do not take over (Stop executing)."
If the user chooses Option 1: Run the assignment command:
gh pr edit <pr_number> --add-assignee "@me" --repo google/adk-python
CRITICAL: Immediately after assigning, you MUST re-run the verification helper script to fetch the updated metadata from GitHub and refresh the cached details:
.venv/bin/python .agents/skills/adk-pr-triage/scripts/triage_pr.py <pr_number> --skip-update
Then proceed to parse the updated PR details from the script's stdout in Step 1.3 and continue standard triage.
If the user chooses Option 2 (or declines): Stop executing immediately and do not run any further tools or operations. State that triage has terminated.
PR IS ALREADY ASSIGNED TO YOU: Proceed directly with Step 1.3.
[PR_METADATA_JSON] and [/PR_METADATA_JSON] tags. Do NOT write to or read from local cache files, and do NOT make separate network commands to fetch PR details. Parse the JSON metadata directly from the command's stdout:
number, title, body, state, url, author, additions, deletions, changedFiles, labels, assignees, closingIssuesReferences (used to locate linked issues).closingIssuesReferences array in the parsed JSON metadata from the script's stdout. If any closing issues are linked, fetch their details using the custom python tool fetch_github_issue(issue_number=<number>). This is preferred as it avoids command execution policy issues.
If the custom python tool is not available, run the gh command:
gh issue view <issue_number> --repo google/adk-python --json number,title,body,state
gh pr diff command to view the actual line-by-line diff of the PR:
gh pr diff <pr_number> --repo google/adk-python
Conduct an extremely thorough review of the changes by examining the diff and analyzing the local codebase. You must address the following three critical dimensions and organize your findings in a premium PR Review Report:
grep_search and inspecting target files with view_file.Evaluate the implementation against the established architectural, style, and testing guidelines. Use direct file links to code reference examples.
google.adk namespace? (Breaking changes are unacceptable under Semantic Versioning without an official deprecation cycle)..py module files under src/google/adk/ private by default (prefixed with a leading underscore, e.g., _my_module.py)?__init__.py using the __all__ list? Are internal helper classes and on-wire objects kept internal by omitting them from __all__?__init__.py? (Within ADK, framework-level imports from __init__.py are strictly prohibited to avoid circular dependencies and maintain clean encapsulation).Runner.run), while private/internal methods are descriptive (e.g., _validate_chat_agent_wiring)?from __future__ import annotations immediately after the license header?Any avoided in favor of precise types, abstract interfaces, or generics?X | None preferred for new code over the legacy Optional[X]?* for constructors with multiple attributes?list, dict, set) avoided? (Use None and instantiate within the method body).isinstance(obj, Type) instead of type(obj) is Type to support subclasses, and is a fallback else raise handled?Field() constraints for simple boundary checks?@field_validator (with mode='after') and @model_validator?use_attribute_docstrings=True configured in the model ConfigDict so that docstrings are utilized as field descriptions?PrivateAttr() and constructor logic mapped in model_post_init()?%-based templates rather than eager f-strings? (e.g., logging.info("Completed in %s ms", duration) is correct; logging.info(f"Completed in {duration} ms") is a violation).except: constructs?tests/ target public boundaries rather than internal execution states?BaseNode, Event, Context) used, restricting mocking exclusively to external web or network dependencies?Present the completed analysis report in your response. Follow the PR Review Report Template below for a highly premium, readable presentation.
At the end of your report, stop calling tools and output this explicit message:
"### 🛑 Review Decision Gate I have completed my in-depth analysis of Pull Request #<pr_number>. Please review the findings above.
How would you like to proceed with this Pull Request?
- [Option 1]: Push Back (Draft a professional, constructive feedback response with recommendations for the author).
- [Option 2]: Local Review (Checkout the PR locally, rebase onto the latest main, and run the
/adk-reviewskill to thoroughly verify and polish before pushing to Gerrit)."
Once the user provides their decision, perform the tailored operations in your subsequent turns:
If the user selects Local Review, run the following structured sequence:
main)..venv/bin/python .agents/skills/adk-pr-triage/scripts/triage_pr.py <pr_number>
main, and if rebase-update is blocked, falls back to updating via a merge commit. It handles all outputs and fallbacks gracefully.pr-triage-<pr_number>-[short_desc] (e.g. pr-triage-5875-parallelize-tool-union).git fetch origin main
git fetch https://github.com/google/adk-python.git pull/<pr_number>/head:pr-triage-<pr_number>-[short_desc]
git checkout pr-triage-<pr_number>-[short_desc]
git log -1 --pretty=%B
# 1. Capture the original author
ORIG_AUTHOR=$(git log -1 --format='%an <%ae>')
# 2. Reset to base and commit with the original author
git reset --soft $(git merge-base HEAD origin/main)
git commit --author="$ORIG_AUTHOR" -m "<PR message>"
"Merge <PR link>" to the very end of the commit message (separated by a blank line). If the PR metadata contains linked issues in closingIssuesReferences, you MUST also append "closes https://github.com/google/adk-python/issues/<issue_number>" for each linked issue on new lines. Use this shell command structure to do it in one-shot:
git commit --amend -m "$(git log -1 --pretty=%B)
Merge https://github.com/google/adk-python/pull/<pr_number>
closes https://github.com/google/adk-python/issues/<issue_number>"
commit-msg hook will automatically execute and append the Change-Id: footprint if not already present.main branch:
git rebase origin/main
/adk-review skill!Merge <PR link> footer, any closes <Issue link> footers, and the original Change-Id: footer. Do NOT change it.git commit -a --amend --no-edit
git push origin HEAD:refs/for/main
Present the initial analysis using the following structured format:
# 🔍 ADK Pull Request Review: PR #<pr_number>
**Title**: <PR Title>
**Author**: @<author_username>
**Status**: `<state>`
**Impact**: `<additions> additions`, `<deletions> deletions` across `<changedFiles> files`
## Detailed Findings & Analysis
### 1. Objectives & Impact ("What does it do?")
- **Context & Background**: [Briefly explain the background and the problem it targets. Reference linked Issue #<number> using markdown links if available]
- **Implementation Mechanism**: [Detail precisely which modules are modified and how the execution flow is altered]
- **Affected Surface**: [Highlight any changes to public classes, CLI interfaces, state models, or setup pipelines]
### 2. Legitimacy & Value ("Is it a valid and useful change?")
- **Workspace Verification**:
- Investigated current workspace files: [file_name.py](file:///absolute/path/to/src/google/adk/...#L123-L145) (using `view_file` / `grep_search`).
- Found that: [Describe the baseline condition that proves the bug exists or the feature is missing]
- **Value Assessment**: [Explain why this is a good addition. Does it solve a genuine real-world developer problem, improve performance, or prevent resources leaks?]
- **Alternative Approaches**: [Evaluate if there is an alternative implementation path. Did the author choose the cleanest design?]
### 3. Principle & Style Alignment Checklist ("Does it follow rules?")
* **Public API & Visibility Boundaries**:
* *Status*: [Pass / Fail / N/A]
* *Analysis*: [Check for breaking changes, private module conventions `_`, and explicit exports in `__init__.py` using `__all__`]
* **Code Quality, Typing & Conventions**:
* *Status*: [Pass / Fail / Nits]
* *Analysis*: [Check for `from __future__ import annotations`, absence of `Any`, modern unions `X | None`, lazy logging `%`, specific exception catching, and Pydantic v2 structures]
* **Robustness & Edge Cases**:
* *Status*: [Pass / Fail]
* *Analysis*: [Check for type discrimination (`isinstance`), boundaries, null checks, fallback else routes, and thread/async safety]
* **Test Integrity & Quality**:
* *Status*: [Pass / Fail / N/A]
* *Analysis*: [Check coverage, testing through public interfaces, minimal inline fixtures, and Arrange-Act-Assert formatting]
---
## Executive Summary
1. **Core Objective**: [Briefly summarize what issue is fixed or feature is introduced]
2. **Legitimacy & Value**: [Legitimate Fix / Valuable Feature / Duplicate / Redundant] - [1-sentence explanation]
3. **Alignment with Principles**: [Pass / Pass with Nits / Major Changes Required] - [1-sentence architecture alignment summary]
4. **Recommendation**: [Approve / Approve with Nits / Push Back (Request Changes)]
---
### 🛑 Review Decision Gate
I have completed my in-depth analysis of Pull Request #<pr_number>. Please review the findings above.
**How would you like to proceed with this Pull Request?**
- **[Option 1]**: **Push Back** (Draft a professional, constructive feedback response with recommendations for the author).
- **[Option 2]**: **Local Review** (Checkout the PR locally under `pr-triage-[pr_number]-[short_desc]`, rebase onto the latest main, and run the `/adk-review` skill to thoroughly verify and polish before pushing to Gerrit)."
Format the authored review response as a premium markdown snippet block:
# 💬 GitHub PR Review Draft Message
*Copy and paste this response directly into the GitHub review interface:*
---
### PR Review: <State (Request Changes / Comment / Approve)>
Hello @<author_username>! Thank you very much for contributing this pull request to improve ADK. I've conducted a thorough architectural and style review of your implementation against our design guidelines and standards.
Here is the feedback and a few suggested changes to align your patch with ADK's principles:
#### 🔴 Major Concerns / Blocks
1. **[Concern 1 Title, e.g., Import from init.py is not allowed]**
- **Target Code**: [filename.py:L100-L105](file:///absolute/path/to/src/google/adk/file_name.py#L100-L105)
- **Issue**: [Detailed explanation of why this violates design/architectural rules, referencing the relevant ADK skill like `adk-architecture` or `adk-style`]
- **Suggested Correction**:
```python
# Provide full, drop-in replacement code block
```
2. **[Concern 2 Title, e.g., Missing Unit Tests for Edge Cases]**
- **Target Code**: [test_filename.py](file:///absolute/path/to/tests/unittests/test_filename.py)
- **Issue**: [Detail what is missing, e.g., "We need verification coverage of boundaries like empty string and negative values."]
#### 🟡 Style & Quality Nits
1. **[Style Nit, e.g., Eager Logging formatting]**
- **Target Code**: [filename.py:L42](file:///absolute/path/to/src/google/adk/file_name.py#L42)
- **Suggestion**: Use lazy-evaluated `%` template syntax:
```python
# Corrected:
logging.info("User registered: %s", user_id)
```
2. **[Typing Nit, e.g., Optional[X] instead of X | None]**
- **Target Code**: [filename.py:L15](file:///absolute/path/to/src/google/adk/file_name.py#L15)
- **Suggestion**: Prefer more concise union type hint `X | None`.
#### 🟢 Positive Aspects
- [Highlight stellar work, e.g., "Excellent Pydantic v2 validation logic!" or "Highly readable and clean docstrings!"]
Please let me know if you have any questions on these suggestions, and let's work together to get this PR merged!
[!IMPORTANT] Command Sandbox Policy: When running commands via
run_command, you MUST ONLY useghorgitcommands. Commands likecurl,wget, or direct HTTP network requests are strictly forbidden and will be automatically denied. Furthermore, you MUST ONLY use simple commands without special characters (such as;,&,|,$,`,<,>,\n,\r,(,),{,},\). The runner environment runs a security policy that automatically denies any commands containing these characters. Always run cleanghorgitcommands directly with arguments, without redirections, command chaining, or shell expansions.
[!TIP] Always verify the baseline behavior in your active workspace before claiming something is a bug or invalid. Reading the current source files using
view_filegives you full context. [!IMPORTANT] When referencing files and line numbers in your reports and draft reviews, always use clickable markdown file links of format[filename.py](file:///absolute/path/to/file#L100-L120)without surrounding backticks around the brackets. Ensure the links represent valid absolute file paths in the local workspace.