ADK Change Reviewer
Review the working-tree diff against the seven dimensions below, report what is
wrong, and stop. Fix only what the user then asks you to fix.
Workflow
git status and git diff (add --staged for staged work) to get the exact
set of added, modified, and deleted files.- Review the diff file by file against the checklist.
- Emit the report in the format below.
- Stop. Do not edit any file, and do not offer to. Wait for the user to ask.
- If the user asks for fixes, apply them, then re-run the affected tests
(
pytest tests/unittests/{path}) and pre-commit run --files {paths}.
Step 4 is the part that is easy to get wrong: an unrequested fix buries the
findings the user asked for and mixes review output with new, unreviewed edits.
Checklist
1. Correctness
- Types: no new
mypy errors. CI diffs mypy output against the base
branch and fails only on newly introduced errors, so a pre-existing error in
a file you touched is not a blocker but a new one is.
- Imports: no circular imports; absolute imports where the module already
uses them.
- Exceptions: no bare
except:; catch a specific type and log with enough
context to identify the caller.
- Type discrimination: check an object's type before reading a
type-specific attribute (for example confirm a node is an
LlmAgent before
inspecting mode), so an unexpected node type raises nothing.
- Boundaries:
None, empty collections, zero, and empty strings are
handled by validation or a fallback default.
- Preconditions: state invariants are checked before the core logic runs.
2. Design
- Complexity: functions or classes that would be clearer split up.
- Coupling: high cohesion, low coupling; no anti-patterns introduced.
- Performance: redundant computation, repeated I/O in a loop, or
allocations that scale with input where they need not.
- Security: inputs validated, sensitive data not logged, no injection,
resource exhaustion, or exposure of internal state.
3. Style
Cross-reference the diff against the adk-style skill rather than restating
its rules here: visibility and _ prefixes, typing, Pydantic v2 patterns, lazy
logging, imports, async, and file organization.
Confirm the changed files pass pre-commit run --files {paths}.
4. Architecture and unintended outcomes
- Public API stability: does the change modify, remove, or narrow a
public interface, class, method, argument list, or CLI surface under
src/google/adk/? A breaking change needs a deprecation cycle first, because
the package is released under Semantic Versioning and users pin minor
versions.
- Execution and resumption: changes to workflows, nodes, or state must stay
compatible with the event execution lifecycle and with session resumption
(human-in-the-loop steps and checkpoints). See the
adk-architecture skill.
- Concurrency and lifetime: no race conditions; plugins, exporters, and
connections are closed on every path, including the error path.
5. Documentation impact
- User-facing documentation lives in the separate
adk-docs repository, so a
user-visible change needs a PR there as well; note it in the report.
- Guides under
docs/guides/ may need an update when a public API or workflow
pattern changes.
- If the change alters a code unit's design contract, the
adk-unit-design
skill owns the design document for that unit.
6. Samples
- Do existing samples under
contributing/samples/ still run against the
change?
- Does a new capability warrant a new sample? Follow the
adk-sample-creator
conventions if so.
7. Tests
- Every new or modified code path has a test under
tests/unittests/.
- A new test belongs in the existing
test_{module}*.py file for its unit, not
in a file named after the change, which fragments that unit's coverage.
- Tests follow the rules in the
adk-style testing reference: one behavior per
test, behavior-named tests, no assertions on private attributes, minimal
fixtures, arrange/act/assert structure.
Report format
Group findings by priority and give a file path and line for each. Do not
report a dimension with nothing to say.
- 🔴 Critical: bugs, type errors, race conditions, resource leaks, security
issues.
- 🟠 Design: complexity, readability, performance, architectural
misalignment.
- 🟡 Style: lint, formatting, non-lazy logging, typing mismatches.
- 🔵 Docs, tests, samples: missing or stale coverage, guides, samples.
1---2name: adk-review3description: Reviews the uncommitted changes in an adk-python working tree and reports correctness, design, public-API stability, test, sample and documentation gaps as a prioritized findings report, fixing them only when asked. Use when the user asks to review local changes, wants a self-review before opening a pull request, asks whether a change breaks the public API or needs tests, samples or docs, or asks what is wrong with the current diff. Required for changes to public APIs, core architecture (Runner, Workflow, BaseNode), new features and major refactors. Don't use for a single style nit (use adk-style), for diagnosing a failing test or a misbehaving agent at runtime (use adk-debug), or for wording a commit message or PR description (use adk-git).4---5
6# ADK Change Reviewer
7
8Review the working-tree diff against the seven dimensions below, report what is
9wrong, and stop. Fix only what the user then asks you to fix.
10
11## Workflow
12
131. `git status` and `git diff` (add `--staged` for staged work) to get the exact
14 set of added, modified, and deleted files.
152. Review the diff file by file against the checklist.
163. Emit the report in the format below.
174. Stop. Do not edit any file, and do not offer to. Wait for the user to ask.
185. If the user asks for fixes, apply them, then re-run the affected tests
19 (`pytest tests/unittests/{path}`) and `pre-commit run --files {paths}`.
20
21Step 4 is the part that is easy to get wrong: an unrequested fix buries the
22findings the user asked for and mixes review output with new, unreviewed edits.
23
24## Checklist
25
26### 1. Correctness
27
28- **Types**: no new `mypy` errors. CI diffs `mypy` output against the base
29 branch and fails only on *newly introduced* errors, so a pre-existing error in
30 a file you touched is not a blocker but a new one is.
31- **Imports**: no circular imports; absolute imports where the module already
32 uses them.
33- **Exceptions**: no bare `except:`; catch a specific type and log with enough
34 context to identify the caller.
35- **Type discrimination**: check an object's type before reading a
36 type-specific attribute (for example confirm a node is an `LlmAgent` before
37 inspecting `mode`), so an unexpected node type raises nothing.
38- **Boundaries**: `None`, empty collections, zero, and empty strings are
39 handled by validation or a fallback default.
40- **Preconditions**: state invariants are checked before the core logic runs.
41
42### 2. Design
43
44- **Complexity**: functions or classes that would be clearer split up.
45- **Coupling**: high cohesion, low coupling; no anti-patterns introduced.
46- **Performance**: redundant computation, repeated I/O in a loop, or
47 allocations that scale with input where they need not.
48- **Security**: inputs validated, sensitive data not logged, no injection,
49 resource exhaustion, or exposure of internal state.
50
51### 3. Style
52
53Cross-reference the diff against the `adk-style` skill rather than restating
54its rules here: visibility and `_` prefixes, typing, Pydantic v2 patterns, lazy
55logging, imports, async, and file organization.
56
57Confirm the changed files pass `pre-commit run --files {paths}`.
58
59### 4. Architecture and unintended outcomes
60
61- **Public API stability**: does the change modify, remove, or narrow a
62 public interface, class, method, argument list, or CLI surface under
63 `src/google/adk/`? A breaking change needs a deprecation cycle first, because
64 the package is released under Semantic Versioning and users pin minor
65 versions.
66- **Execution and resumption**: changes to workflows, nodes, or state must stay
67 compatible with the event execution lifecycle and with session resumption
68 (human-in-the-loop steps and checkpoints). See the `adk-architecture` skill.
69- **Concurrency and lifetime**: no race conditions; plugins, exporters, and
70 connections are closed on every path, including the error path.
71
72### 5. Documentation impact
73
74- User-facing documentation lives in the separate `adk-docs` repository, so a
75 user-visible change needs a PR there as well; note it in the report.
76- Guides under `docs/guides/` may need an update when a public API or workflow
77 pattern changes.
78- If the change alters a code unit's design contract, the `adk-unit-design`
79 skill owns the design document for that unit.
80
81### 6. Samples
82
83- Do existing samples under `contributing/samples/` still run against the
84 change?
85- Does a new capability warrant a new sample? Follow the `adk-sample-creator`
86 conventions if so.
87
88### 7. Tests
89
90- Every new or modified code path has a test under `tests/unittests/`.
91- A new test belongs in the existing `test_{module}*.py` file for its unit, not
92 in a file named after the change, which fragments that unit's coverage.
93- Tests follow the rules in the `adk-style` testing reference: one behavior per
94 test, behavior-named tests, no assertions on private attributes, minimal
95 fixtures, arrange/act/assert structure.
96
97## Report format
98
99Group findings by priority and give a file path and line for each. Do not
100report a dimension with nothing to say.
101
102- 🔴 **Critical**: bugs, type errors, race conditions, resource leaks, security
103 issues.
104- 🟠 **Design**: complexity, readability, performance, architectural
105 misalignment.
106- 🟡 **Style**: lint, formatting, non-lazy logging, typing mismatches.
107- 🔵 **Docs, tests, samples**: missing or stale coverage, guides, samples.
Google