CtrlK
BlogDocsLog inGet started
Tessl Logo

github-pr-review

Review a GitHub pull request and leave inline comments in a natural human reviewing voice, indistinguishable from comments typed by hand on the GitHub web UI. Deliberately bounded to defects a reader can settle by looking at the anchored line, rather than a deep audit, so every comment is cheap for the author to check. Scales to 2-20 comments by PR size, parallelises analysis across sub-agents, drafts for approval, then posts individually with human pacing. Use when the user says "review this PR", "review PR 123", pastes a github.com/.../pull/N link, or asks for comments on a pull request. Don't use for reviewing local uncommitted changes or a diff against a branch (use the `review` skill for that).

SKILL.md
Quality
Evals
Security

GitHub PR review

Reviews a pull request on GitHub and posts inline comments that read as though a person typed them by hand. The style is defined in reference/voice.md; replace that file to shift it.

Install

Drop this folder anywhere your agent discovers skills — ~/.agents/skills/, ~/.config/cloudcode/skills/, or any absolute path registered under skills.paths in ~/.config/cloudcode/cloudcode.json.

Set SKILL_DIR before running any command below, to wherever you put it. Every script invocation in this file is relative to it:

SKILL_DIR=~/.agents/skills/github-pr-review     # adjust to your install

Requires gh (authenticated) and python3. No other dependencies; pyyaml is used if present and degrades gracefully with a reported warning if not.

Living inside the google/adk-samples checkout

This copy is tracked in google/adk-samples at .agents/skills/github-pr-review/, which has two consequences worth knowing before you edit it.

Its tests run in that repo's CI. The root pyproject.toml puts .agents/skills in testpaths, and tools-tests.yml fires on any .agents/** change, so tests/ here is collected by uv run pytest alongside the repo's own tooling suite. A broken test in this skill turns a PR red. Run it before pushing.

python-format.yml scopes ruff to core/, contrib/ and plugins/, so nothing here is linted — the skill's own style is its own business.

It is deliberately absent from docs/recipe-handbook/skills-catalog.md. That catalog is for skills a recipe contributor should reach for; this one is a maintainer's reviewing tool, and advertising it invites PR authors to run a reviewer against their own PR. Keep it out when you edit the catalog.

Five things make this work, and all five are easy to get wrong:

  1. A comment must point at a fact, not ask the author to derive one. This is the strongest predictor of what gets posted: on PR #2373 it separated all 16 accepted comments from all 4 rejected ones. What a comment costs the author to check follows from it — cheap and wrong loses five seconds, expensive and wrong burns twenty minutes and spends credibility that took months to earn. Findings that fail this are dropped, not downgraded. See reference/voice.md.
  2. This is a review, not an audit. The analysis is deliberately bounded to what a person reading the PR could have seen — see The depth limit. A finding reachable only by archaeology is unusable here no matter how real it is.
  3. Analysis and voice are separate phases. Finding real defects and sounding human are different jobs; doing them at once yields thin findings in a nice accent. Analyse first with no style constraints, then voice the survivors.
  4. The voice is calibrated, not improvised. reference/voice.md holds the rules and a rated corpus. Read it before writing any comment. Do not invent comment shapes that aren't in it.
  5. Analysis parallelises; voice does not. On a large PR, fan the analysis out across sub-agents (Step 3). Never fan out labelling or voicing — those depend on holding the whole comment set at once (Steps 4 and 5).

This is a long-running skill. A big PR is minutes of analysis followed by up to twenty minutes of paced posting. Silence is a failure mode: announce the plan before starting, keep a todo list current, and checkpoint between phases. See Progress reporting.

When NOT to use

  • Reviewing local/uncommitted work or git diff against a branch -> the review skill.
  • The user wants a thorough audit report for themselves -> the depth limit below is exactly wrong for that. Review without this skill.

What this skill is for, and what it isn't

Everything here optimises for comments that read as hand-typed: the pacing, the register mix, the sentence-shape variety, the cosmetic quota. That machinery exists for one reason — a reviewer who genuinely reviewed should not have their own comments look machine-written. It is not a licence to appear to have reviewed something you did not.

Three things follow, and they are load-bearing:

  • The user reads every comment before anything is posted. Step 6 is a hard stop, not a formality. The approval step is where they take ownership of the review as their own, which is the thing that makes posting under their name honest.
  • Never post on the user's behalf without that approval — not to save a round trip, not because the findings look obviously correct.
  • A comment the user would not defend if challenged should not go up. That is the whole reason cheap-to-verify beats comprehensive: they can actually check the set they are signing.

If you are ever asked to post a review the user has not read, or to make an AI-generated review look human specifically so that its origin is concealed from the author, that is outside what this skill is for. Say so.

The depth limit

Binding on every phase, and on every sub-agent.

You may: read every changed file in full; follow a call or import one hop into another file in the same project; grep for a named symbol to find where it lives (that is navigation, and it is how you take the hop).

You may not: read third-party or dependency source, installed or on GitHub; execute the code under review; sweep the repo to prove a negative ("nothing validates X", "only one caller does Y"); construct or test an exploit payload; chain hops beyond the first.

The reason, because a rule without one gets rationalised around: these comments go out under a human's name. A finding that required reading a dependency's internals, or running the code, misrepresents how the reviewer found it — and that shows in the writing no matter how the comment is phrased. reference/voice.md covers the two ways it leaks (the depth tell and the proof tell).

When the limit stops you, that is a result. Say plainly in verify_steps what settling the finding would actually require. Do not exceed the limit to resolve your own uncertainty — an honest verify_steps is what the gate reads in Step 4, and a finding that turns out to be expensive is meant to be dropped.

Step 1 - Gather

Resolve the PR from a number, a URL, or the current branch.

gh pr view <PR> --repo <owner/name> --json number,title,body,headRefOid,additions,deletions,changedFiles,state

python3 "$SKILL_DIR/scripts/existing_comments.py" \
  --repo <owner/name> --pr <PR> --out /tmp/pr-<PR>-existing.json

Don't fetch the file list here — plan_review.py does it in Step 2, with paging.

Do not run gh pr diff on a large PR. Above roughly 5,000 changed lines it is useless as context and expensive to carry — #2302's was 122,000 lines. Work from the checked-out tree instead; for an all-additions PR the file content is the diff. Below that threshold it's fine and often the quickest way to see the shape.

Read the existing comments, and keep that file — Step 4 feeds it to the verifier, which drops anything already raised. This is what makes a PR re-reviewable: run it again after your own pass, or after a bot or a colleague, and you get only new material.

Suppression rules, applied automatically:

  • same line, or within 2 lines → dropped
  • similar wording to any existing comment, including top-level ones with no line → dropped
  • resolved threads still block — already discussed
  • outdated threads do not block the line (the code moved, so it deserves a fresh look) but their text still counts
  • bots block exactly like humans
  • comments the user cut on a previous review are blocked too, from the ledger written at Step 6. A rejection never expires as outdated — a decision the user made does not lapse because the code moved.

Also paste the existing comments into each lane prompt. The verifier is the backstop; a worker that never generates the duplicate is cheaper than one that gets filtered.

Then get the PR head into a local tree, and note its absolute path:

gh repo clone <owner/name> /tmp/pr-<PR> -- --depth 1 --no-tags && \
  cd /tmp/pr-<PR> && git fetch --depth 1 origin pull/<PR>/head && git checkout FETCH_HEAD
# or, against a clone you already have:
git -C <local-clone> fetch origin pull/<PR>/head && git -C <local-clone> checkout FETCH_HEAD

Do this once, here. Analysis workers read this tree concurrently and must never mutate it — five sub-agents each running gh pr checkout in the same directory is a race that leaves the tree on an arbitrary ref mid-review.

Step 2 - Plan, ask, announce

python3 "$SKILL_DIR/scripts/plan_review.py" \
  --repo <owner/name> --pr <PR> --json

The plan gives you, deterministically: which files to skip and why, the comment budget, whether to fan out, the lane assignments, and the post-run ETA.

It skips lockfiles, generated and vendored code, dist/, snapshots, binaries, large data fixtures, deleted files, and pure renames — a file moved with zero content change has nothing to review, which on a migration PR is most of the diff (65 of 123 files on #2373).

Lanes are packed so a source file and its tests land together (tools/x.py with tools/tests/test_x.py), because a reviewer holding only one of the pair cannot tell whether the test still covers the code. Lane sizes stay balanced regardless.

Ask before analysing — scope

Ask here rather than at the start, because the answer depends on the PR's size and shape, which you only know now:

PR #2302 "add the Horizon long-horizon agent recipe"
787 files, 122,547 lines, all additions. Skipping 16 (lockfiles, binaries).
Budget: 12-20 comments.

  Include tests/ (316 files) and web/ (238)?   [yes / no]

Argue for including tests/: test files are the richest source of instantly-checkable defects — a duplicated assertion, a test named for one thing that exercises another — and they are usually the least-reviewed part of a PR.

Act on the answer — re-run the planner with the flags. The default includes everything; the question is worthless if you don't pass it through:

python3 "$SKILL_DIR/scripts/plan_review.py" \
  --repo <owner/name> --pr <PR> --json [--no-tests] [--no-web]

Use the re-planned lane assignments, not the first run's.

House rules — google/adk-samples only. If the PR is in that repo and touches core/, contrib/ or plugins/, .github/review-rules.md is in force. Say so in the checkpoint. For every other repo, ignore it — the rules are repo-specific and applying them elsewhere produces confident nonsense.

Then:

  1. Seed the todo list — one item per phase, one per lane (see Progress reporting).
  2. Print the checkpoint, so the user knows the shape of the work and that nothing will be posted without them:
Tests included. 771 files across 10 lanes + consistency lane.
Analysis takes a few minutes. Nothing gets posted without your approval.

The budget comes from reviewable churn, not total — a PR that is 1,400 lines of regenerated lockfile plus 80 lines of hand-written code earns a small-PR budget.

Reviewable linesComments
< 502–3
50–2003–5
200–6005–8
600–15008–12
> 150012–20

Hard cap 20, whatever the size.

Step 3 - Analysis phase (no voice constraints)

Hunt for defects and write them down verbosely and technically. This output is internal — never shown to the user, never posted.

Work inside the depth limit, and within it read the full changed files, not just the diff — a broken invariant or an error path that no longer returns is invisible in a diff window. Where the diff touches a function, read the whole function; where it calls a helper you cannot judge blind, spend the hop.

Use the What to hunt block from reference/lane-prompt.md verbatim. It is one list, and the filter at the top of it is the load-bearing part: a finding must point at something observable at the line. If seeing that it is a defect requires the reader to do arithmetic, infer a pattern, or reason about consequences, it is out of scope however serious it looks.

Group at 3+. The same defect class in three or more places is ONE finding, on the clearest instance, with the count in evidence. Five "unused import" findings is one comment; the other four slots buy distinct defects. Two instances stay two.

The grouped comment's claim is about the anchored instance; the count is context, never something the reader must open files to confirm. a few unused imports in here is checkable at the anchor; unused imports here, also in X and Y is not. Group only instances that are individually real — sweeping a deliberate __init__.py re-export into an "unused imports" group makes the whole comment wrong.

In google/adk-samples, run the house-rules checker:

python3 "$SKILL_DIR/scripts/check_house_rules.py" \
  --repo-root <repo_path> --recipe <recipe> --json

24 of the 27 rules are decided deterministically there — take its findings verbatim. Two of them, H26 (env-read defaults, AST-based) and H27 (licence header consistency), report one grouped finding with a count rather than one per hit. Dispatch the house-rules lane (Template C) for the remaining three (H11, H16, H25), which need a judgement call. One — H48, a junk ownership.team — is reported whether or not the PR touched manifest.yaml, and is never trimmed; see Must-post findings.

Prefer the script over a lane wherever a rule is decidable — a script cannot hallucinate a violation, and a false "this will fail CI" is the most expensive comment this skill can produce.

The lane is defined by file identity, not churn: it receives the recipe's config surface (pyproject.toml, manifest.yaml, README.md, .env.example, the package __init__.py, Makefile, Dockerfile) and reads them in full even when they are pure renames or wholly outside the diff — see reference/rationale.md.

Workers assign one label:

  • severity: critical (security hole, data loss, corruption, auth bypass, injection, a crash on a reachable path) or no_critical (everything else worth saying). It orders the output; it never changes how a comment is worded.

Every finding also carries verify_steps (the literal procedure the author follows to settle it) and window (the real source lines at the anchor). verify_steps is the gate — verify_findings.py computes cheapness from it in Step 4, so a worker that writes it honestly is doing the right thing even when the finding then gets dropped. window is what gets shown to the user beside the comment.

Pure style, naming and formatting are not findings at any severity — discard them.

A finding claiming something is absent must say where you looked, or where it does appear. That is the one claim the anchored line cannot support, and it is the defect that produced the single wrong comment this skill has posted.

3a - Small PR (fan_out: false)

Do it inline, yourself. Below ~400 reviewable lines and ~8 files, spawning workers costs more than it saves.

3b - Large PR (fan_out: true)

One batch_task call. Lanes are the file shards from the plan:

batch_task({
  description: "PR 2302 analysis",
  concurrency: 10,
  verify: false,
  subtasks: [ ...one per file lane..., consistency lane ]
})

Build each prompt from reference/lane-prompt.md — Template A for file lanes, Template B for the consistency lane. subagent_type: general for all of them.

Fill in every placeholder: workers share no context with you or each other, so <PR>, <owner/name>, <repo_path> (the tree from Step 1), the lane's file list, and the existing-comment list all have to be spelled out in each prompt.

The consistency lane is not optional. It gets no file shard. It exists for the one class a per-file split structurally cannot see: the same element differing between files — three licence headers, two docstring styles, a constant written with different values in two places. Shard workers cannot see these; one worker comparing across files can.

It is bounded to structural comparison, never semantic tracing. Comparing four licence headers is a glance; following a value between modules is not, and that is what the previous version of this lane spent its time on, producing findings that were all discarded.

verify: false on purpose — the batch verifier would re-audit findings you are about to re-rank yourself in Step 4, adding a full agent pass of latency to the one phase this whole mechanism exists to make faster.

Merging the findings. Read each lane's findings.json. Then:

  • Dedupe. Two lanes reaching the same defect from different directions is common; it is one comment.
  • On any conflict about a cross-file inconsistency, the consistency lane wins — it saw every side.
  • Spot-check severity, don't re-derive it. Sanity-check that each critical really is one. Re-reading the cited code is inside the limit; going further to rescue a finding the gate will drop is not.
  • A lane that failed is not a blocker: note it, and either re-run that one lane with task or cover its files yourself.

Then checkpoint (see Progress reporting).

Step 4 - Gate or tag, then group

No budget cut happens here.

First, run the verifier — this is not optional

python3 "$SKILL_DIR/scripts/verify_findings.py" \
  --findings /tmp/pr-<N>-raw.json --repo-root <repo_path> \
  --repo <owner/name> --pr <N> --head-sha <headRefOid from Step 1> \
  --existing /tmp/pr-<N>-existing.json \
  --out /tmp/pr-<N>-verified.json

It does five things that were previously prose, done by hand, and sometimes skipped:

  1. Window vs file. Each finding's quoted source is diffed against the real file. A mismatch means the lane fabricated it — rejected, not downgraded.
  2. Addressability. Marks anchorable: false for any line outside every diff hunk.
  3. Cheap gate. Computes cheapness from verify_steps; a procedure that names a second file or says trace / assuming is not cheaply verifiable.
  4. Duplicate suppression. Drops anything already raised on the PR (see Step 1) and anything the user cut on a previous review — the rejection ledger loads automatically from --repo/--pr. --no-ledger disables it.
  5. Already-red suppression. With --head-sha, drops a CI-FAIL finding whose enforcing workflow is already failing on that commit, matched from the workflow file the finding cites in evidence. The author is looking at that red check already, and its message is more precise than ours. Advisory findings are never suppressed this way — nothing is failing for them, so the comment is the only way the author hears it. An unreadable status suppresses nothing. --no-ci-status disables it.

It also emits a fact-anchoring lint and a clustering hint for grouping. Both are advisory; read them, don't obey them blindly.

Take its output as the input to everything below. Findings it rejected do not come back — a fabricated window is not a labelling problem.

Un-anchorable findings are set aside, not discarded. A finding outside every hunk cannot be an inline comment, but it can still be true and important — the three hard CI failures on PR #2373 were all un-anchorable. They stay in the unanchorable bucket, carried to Step 6 and offered as one top-level issue comment. Never silently drop them, and never anchor them to an unrelated nearby line to force them through: a comment about requires-python pinned to a dependency line twenty rows away is worse than no comment.

The gate drops every not_cheap finding. Report a bare count at the checkpoint (23 findings dropped as not cheaply verifiable) — not a list. Handing over the list would pull the user into evaluating exactly the material the gate exists to spare them.

Then apply the fact-anchoring filter yourself, on the survivors. verify_steps catches findings that are expensive to check; it does not catch findings that are cheap to check but ask the reader to derive the defect. Delete the question from the comment: if what remains is not a statement of something visible at the line, drop it. This is the filter that separated all 16 accepted comments on #2373 from all 4 rejected ones — see the rated outcomes in reference/voice.md.

Cheapness is a property of the final comment text, not the raw finding. `ty` is on 3.10 while `requires-python` says 3.11 is cheap because it names both sides; "this contradicts the setting above" is the same defect and expensive. Re-check at the end of Step 5 once the wording exists.

Survivors are ordered critical first. There is no other label — confidence was removed because after the verifier runs, everything surviving is high-confidence by construction, so the column never discriminated.

Severity orders the output and nothing else. It never changes how a comment is worded (reference/voice.md).

Rules that still apply at this stage:

  • If there are not enough real issues, produce fewer. Padding produces exactly the vacuous filler that reads as machine-generated. If a PR is genuinely clean, say so and post nothing.
  • ≤ 2 cosmetic comments (stale years, a constant that disagrees, a naming inconsistency) — all no_critical by definition.
  • 1–2 positive remarks when genuinely warranted, suppressed entirely if anything critical was found. Nobody compliments the styling on a PR with a security hole.

Step 5 - Voice phase

Never delegate this step. Not to a sub-agent, not split across two passes. Everything below is a property of the comment set as a whole — the register mix, the clustering, the sentence-shape variety, the cosmetic quota. An agent holding one shard cannot honour any of them, and the failure is invisible per-comment and glaring in aggregate.

Read reference/voice.md now. Summary of the binding rules:

  • Every full-sentence comment ends in a question mark. Declarative multi-clause statements are banned.
  • Or use a lowercase fragment of 2–6 words with no terminal period.
  • Tone never escalates with severity. Command injection is raised as politely as a missing try/catch.
  • One thought per comment. No causal chains, no prescriptions stacked on diagnoses, no line numbers in prose.
  • Banned: bold labels, severity prefixes, emoji, headers, bullets, suggestion blocks, "Consider …", "It would be better to …".
  • Backticks on identifiers and paths.
  • No crafted payloads, and no claim resting on code you didn't read — the proof tell and the depth tell. Write the comment you would have written before doing the work, not after.
  • Name the referent, and carry the verification path. A comment must say what it points at, and if it depends on anything off the anchored line it must name that and say where. old version still holds the value is unusable; Does the previous version get destroyed anywhere? is not.

Aim for ~60% Register A / 40% Register B.

Findings arrive from workers as blunt technical prose (user-controlled filename is interpolated into os.system()). That is raw material, not a draft. Rewrite every one from scratch against the corpus.

Severity must not leak into the wording. A critical comment is not sharpened and a no_critical one is not softened. The user reads the ordering; the PR author reads only the comment, and it has to look the same either way.

Distribution matters as much as wording

  • Cluster on the substantive files. Uniform coverage — one comment per file, evenly spread — is a machine signature. Real reviewers dwell where the risk is and skim the rest. Fanning out by file makes this worse by default, since every lane returns findings: when you select, deliberately let some lanes contribute nothing.
  • Do not let two comments share a sentence shape. Three comments opening with It seems is a tell.
  • Vary length. A review where every comment is one line, or every comment is two sentences, looks generated.
  • Cross-reference in Register B when the same issue recurs (same issue as above) rather than restating it.

Before leaving this step, re-check the cheap label against the wording you actually wrote (Step 4), and run the cold-read test from reference/voice.md on every comment: anchored line ±10 plus the comment text, nothing else — can you tell what it refers to and how you'd check it? If not, rewrite or drop it.

Step 6 - Present, take a decision

Generate the report, print the summary, then stop and wait.

The report goes to ~/Documents/agents/pr-reviews/. Don't ask — that folder is the destination unless the user names another path in the same breath.

python3 "$SKILL_DIR/scripts/build_report.py" \
  --candidates /tmp/pr-<N>-candidates.json --repo <owner/name> --pr <N> \
  --out-md ~/Documents/agents/pr-reviews/pr-<N>-review.md

That writes one markdown report — summary table first, detail sections below — carrying the code window beside every comment, which is the whole point: the user must be able to run the check without opening a file.

--out-csv still exists for spreadsheet triage but is off by default; the CSV held the same rows as the markdown tables, so writing both left two copies of one report on disk.

Present the set

One table, ordered critical first, then by path. Every row carries the source line it is anchored to, so the user can settle it without opening a file.

16 comments ready (2 critical), ~14 min to post.
3 more findings are un-anchorable — see below.
47 findings dropped as not cheaply verifiable.

Report: ~/Documents/agents/pr-reviews/pr-2373-review.md

Recommend, don't decide.

  • Don't propose "criticals only" by reflex. A review that is entirely critical security findings is itself implausible; real ones mix a sandbox escape with a stale Python version.
  • Point out anything you are unsure of rather than burying it. The user cutting a comment costs nothing; a bad comment posted costs their credibility.

Then apply the size budget from Step 2. If the set exceeds the cap, trim and say exactly what was dropped. Never trim silently.

Must-post findings

H48 — an ownership.team naming a company, a filler word, or the author's own GitHub handle — is exempt from the budget and from the "is it worth a slot?" cut. It survives trimming even on a PR already at its cap.

It is the only field in the repo where a wrong value passes every deterministic check: the schema asks for a non-empty string and stops there. Nothing downstream notices, and the recipe ships with no reachable owner. So the comment goes out every time the checker fires, and when manifest.yaml is outside the diff hunks it goes in the un-anchorable top-level comment rather than being dropped.

Ask who maintains the recipe. Never propose a team name — the same reason as H17.

Never post without explicit approval, and let the user cut or reword anything. A wrong comment posted in their name costs their credibility with the PR author, so this step is where they take ownership of the review as their own.

Record what they cut, right after they choose and before posting:

python3 "$SKILL_DIR/scripts/rejections.py" --record \
  --repo <owner/name> --pr <PR> \
  --candidates /tmp/pr-<PR>-candidates.json \
  --approved /tmp/pr-<PR>-comments.json

--candidates is everything drafted, --approved is what they said yes to; the difference is the rejection. Pass the matching pair from this run — mismatched files record the wrong set and poison the ledger.

Step 1's suppression only sees comments that were posted. One the user read and cut left no trace on GitHub, so without this it returns on the next review and has to be rejected again — which happened three times on the #2373 re-review. The verifier loads the ledger automatically; nobody has to remember to pass it.

Un-anchorable findings

If the unanchorable bucket is non-empty, present it as its own short table and offer one top-level issue comment covering all of them:

Un-anchorable (3) — real, but the lines aren't in any diff hunk:
  pyproject.toml:69   [tool.ruff] tables        CI-FAIL
  pyproject.toml:38   requires-python >=3.10    CI-FAIL
  pyproject.toml:2    name != folder basename   CI-FAIL

Offer as one top-level comment?

This is the one permitted exception to "no summary comment" (Step 7). Keep it to a bulleted list of the findings, with no preamble about the review as a whole and no restatement of the inline comments.

Order them CI-failures first — an author will act on "this blocks the build" and may not act on a convention nit. Mark advisory items as such, and never claim CI will fail when .github/review-rules.md says the check is a ::notice.

Step 7 - Post

Write the approved set to JSON and dry-run it first — blocking, it's fast, and it is the gate:

python3 "$SKILL_DIR/scripts/post_comments.py" \
  --repo <owner/name> --pr <PR> --file /tmp/pr-comments.json --dry-run

It validates every line against the diff hunks and fails loudly if a line isn't addressable. Then run the real thing as a background job and hand control back:

python3 "$SKILL_DIR/scripts/post_comments.py" \
  --repo <owner/name> --pr <PR> --file /tmp/pr-comments.json
Posting 9 comments, ~8 min. Running in the background - you'll be notified when
it's done, so carry on with whatever you like.

Do not sleep-and-poll it. You get a completion notification; tail the job only if the user asks how far along it is.

The script posts each comment individually, in file order, sleeping a random 10–20s between them. Do not shorten the gaps — the pacing is the point (reference/rationale.md). If it fails partway, the .posted state file records what landed; re-run the identical command to resume.

A pending review blocks everything — GitHub allows one per user and this endpoint implicitly opens one, so every comment 422s. The script preflights and stops with instructions. Do not submit or discard their pending review for them: submitting posts publicly under their name and may carry a verdict; discarding destroys drafts they wrote. Surface it and let them choose.

No verdict and no summary comment. Do not approve, do not request changes, do not leave a top-level recap. The user handles the verdict themselves.

The single exception is the un-anchorable list from Step 6, when the user approves it — a bulleted list of findings whose lines are in no diff hunk. Not a recap: it says nothing about the review as a whole and repeats no inline comment.

comments.json format:

[
  {"path": "src/api.py", "line": 42, "body": "missing await here"},
  {"path": "src/api.py", "line": 88, "body": "Can we use `subprocess.run()` with a list here?"}
]

line must be a line present in the diff. Add "side": "LEFT" to comment on a deleted line.

Progress reporting

The user cannot see analysis happening. On a large PR the gap between "review this" and the draft is minutes, and the post run is minutes more. Fill it.

Todo list, seeded in Step 2 and kept current — this is the live progress bar:

- Gather PR + existing comments
- Plan lanes and budget
- Lane 1 analysis (4 files, 320 lines)
- Lane 2 analysis (6 files, 295 lines)
- ...
- Consistency lane
- Merge, dedupe and label findings
- Draft comments in voice
- Present table and await approval
- Post (background)

Mark each done as it lands, not in batches. On a small PR collapse the lane items into a single "Analyse changed files".

Checkpoints. Four short messages, no more. Prose, not tables.

  1. After the plan (Step 2) — the mode/scope question, then size, skips, budget, lane count, and the promise that nothing posts without approval.
  2. After analysis (Step 3/4) — the funnel, so the shape of the cut is visible:
    Analysis done. 74 findings across 12 lanes. 3 rejected by the verifier,
    25 un-anchorable, 26 dropped as not cheaply verifiable or style.
    20 candidates, drafting now.
  3. At the decision (Step 6) — the table, the un-anchorable list, the report path, the ETA.
  4. At posting (Step 7) — backgrounded, ETA, notification promise.

batch_task does not stream partial results, so nothing arrives between checkpoints 1 and 2. That makes checkpoint 1 load-bearing: it must state that analysis takes a few minutes, or the silence reads as a hang.

If anything degrades — a lane fails, the diff is unfetchable, the PR is closed, every file is skipped — say so immediately rather than quietly working around it.

Repository
google/adk-samples
Last updated
First committed

Is this your skill?

If you maintain this skill, you can claim it as your own. Once claimed, you can manage eval scenarios, bundle related skills, attach documentation or rules, and ensure cross-agent compatibility.