跳到正文
DEV Community· AI Spec Kit·· 2 小時前AI 評分68

AI 編碼智能體撰寫的 Pull Request 審查清單

A Checklist for Reviewing Pull Requests Written by AI Coding Agents

AI 導讀

AI Spec Kit 提出一份審查 AI 編碼智能體所寫 Pull Request 的清單,涵蓋範圍、測試、隱藏行為、依賴項、安全與資料、效能成本、文件及後續處置。清單建議確認至少一項新增測試在未套用變更時會失敗,並提供再次提示範本及批准、再次提示、拒絕並重做或接手四種處置;作者亦提供免費 .docx、.pdf 和 .md 版本。

正文

Coding agents made writing a pull request cheap. They didn't make reviewing one cheaper. If anything, review is now the bottleneck. The queue that grows fastest isn't unstarted work any more; it's finished work that nobody has reviewed yet.

The hard part isn't the obviously broken PR. CI catches those. The hard part is the PR that compiles, passes every check, has a confident description, and is still wrong in a way you'll only notice in production.

Agent-written code fails in fairly predictable ways, so you can review it with a predictable pass. Here's one, with examples of what each step catches.

0. Read the ticket before you read the diff

This sounds obvious, but it's the step that gets skipped most. If you open the diff first, the agent's framing becomes your framing. You'll check whether the code does what the PR description says, not whether it does what was asked.

Before opening the diff, make sure you can answer:

  • What was the ask, in one sentence?
  • Where are the acceptance criteria written down?
  • How risky is this change? Three tiers are enough:
    • Low: docs, tests, internal scripts.
    • Medium: user-facing behavior.
    • High: auth, payments, migrations, security, infrastructure.

High-risk changes get a full review by someone who didn't write the prompt. The person who prompted the agent has the same blind spots as the prompt.

1. Scope: does every changed file map to the ask?

Agents are generous. Ask for a bug fix and you may get the fix, plus a renamed helper, a reformatted module and a "small cleanup" in an unrelated file. Each of those is a place for a regression to hide, and they make the real change harder to see.

A quick way to spot drift:

git diff --stat main...HEAD

If a file in that list doesn't map to an acceptance criterion, ask why it's there. Also look for the opposite problem, where the work is quietly unfinished:

git diff main...HEAD | grep -nE '^\+.*(TODO|FIXME|XXX|NotImplemented)'

Stubs, hard-coded return values and commented-out blocks sometimes get presented as done. Check each acceptance criterion against the diff, not against the PR summary.

2. Tests: does at least one test fail without the change?

This is the single most useful check. A test that passes with or without the fix proves nothing about the fix.

# Assumes tests live outside src/. Keep the new tests,
# put the old implementation back, and run them.
git checkout main -- src/
npm test   # or pytest, go test, cargo test...
# At least one new test should FAIL here
git checkout HEAD -- src/   # restore the PR's version

If everything still passes, the tests aren't exercising the change.

Then read the test diff with suspicion. The pattern to watch for is a test that was loosened until it passed:

- expect(calculateTotal(cart)).toBe(107.99);
+ expect(calculateTotal(cart)).toBeTruthy();
- def test_rejects_expired_token():
+ @pytest.mark.skip(reason="flaky")
+ def test_rejects_expired_token():

Both of these make CI green. Neither should get through review without a stated reason. Other things to look for:

  • Snapshots regenerated wholesale.
  • Mocks that return exactly what the assertion expects.
  • Assertions that only check that no exception was thrown.

3. Hidden behavior changes

This is where "compiles but subtly wrong" lives. The agent changed something you didn't ask about, and nothing flags it.

Swallowed or broadened errors:

  try {
    await chargeCustomer(order);
- } catch (err) {
-   if (err instanceof CardDeclinedError) return { status: "declined" };
-   throw err;
- }
+ } catch (err) {
+   return { status: "declined" };
+ }

Every failure, including network timeouts and bugs, is now reported to the user as a declined card. The tests for the declined path still pass.

Other hotspots worth a slow read:

  • Changed defaults, config values or environment variable names.
  • Changed response shapes or error codes that clients depend on.
  • Edits to shared helpers. Search for the helper's other callers, because the agent probably only checked the one it was working on.
  • Ordering, rounding, off-by-one errors, time zones, retries, timeouts and cache keys.

4. Dependencies: does that package actually exist?

Agents sometimes import packages that don't exist, or that exist under a slightly different name. That's an annoyance when the install fails. It's a supply-chain risk when someone has registered the misspelled name.

For every new dependency, confirm it exists under that exact name and looks maintained:

npm view some-package name version time.modified repository.url
pip index versions some-package

Also check:

  • Lockfile changes match manifest changes, with no unrelated version bumps along for the ride.
  • The license is one you can ship.

While you're in this section, check the security basics that agents get wrong under pressure to "make it work":

  • Queries are parameterized, not built from strings.
  • No shell commands are assembled from user input.
  • Permission checks weren't removed or moved after the action they protect.
  • There are no new outbound calls to places you don't recognize.

5. Secrets and data

Agents copy whatever is in their context. If a real key, connection string or customer record was in a file or log they read, it can end up in a fixture, a test or the PR description.

git diff main...HEAD | grep -niE '(api[_-]?key|secret|token|password|passwd|BEGIN [A-Z ]*PRIVATE KEY|postgres://|mongodb(\+srv)?://)'

That grep is a crude first pass. Run your real secret scanner on the branch too. Also check:

  • New log lines don't capture personal data or whole request bodies.
  • Env files weren't committed.
  • Ignore files weren't loosened to make something work.

6. Performance and cost

Agent code tends to be correct at small scale and expensive at large scale. The classic case is a query inside a loop:

# Fine with 10 orders in the test fixture, painful with 50,000 in prod
for order in orders:
    order.customer = db.get_customer(order.customer_id)

Look for:

  • Unbounded fetches and missing pagination.
  • Polling, and retry loops without backoff.
  • Calls to paid APIs with no limits, timeouts or caching. That includes model APIs, where a retry loop can turn into a surprise bill.
  • Migrations or new indexes that will lock a production-sized table.

7. Docs and comments

Agents often leave narration behind:

// Now we update the function to handle the new case

That comment describes the agent's process, not the code. Comments should explain why, and they should match what the code does. Then check two more things:

  • If behavior or config changed, the README, API docs or runbook changed too.
  • Medium- and high-risk changes have a rollback plan, such as a flag or a reversible migration.

8. Decide: approve, re-prompt, reject or take over

Reviewing is only half the job. The other half is deciding what happens next, quickly. Four outcomes cover it:

Decision When
Approve Every check passes or is deliberately waived (and noted in the PR)
Re-prompt Scope drift, weak tests, a small hidden change, an unexplained choice
Reject and restart The approach is wrong, the ask was misunderstood, tests were weakened to pass, there's a security problem, or the diff is too big to review
Take over The same issue survives two re-prompts

When you re-prompt, be specific. "Please fix the tests" gets you another round of loosened assertions. This template works much better:

Review feedback on <PR title>:
1. <file>:<line>: <what's wrong>. Expected: <behavior>.
   Add a test that fails on the current code.
2. Revert the changes to <file>. They're out of scope.
3. Explain why <choice> was made, or change it to <option>.
Don't modify, skip or loosen existing tests.
Reply with a summary of what changed and the test output.

The "don't loosen existing tests" line matters more than it looks.

Reject is underused. If the approach is wrong, three rounds of re-prompting usually produce a wrong approach with better tests. Close the PR. Fix the ticket, because a fuzzy ticket is usually the root cause. Then restart with a smaller task.

Fix the pattern, not just the PR

If the same miss shows up in three PRs, stop catching it in review and fix it upstream. Weakened tests, surprise dependencies and unrelated refactors are the usual repeat offenders. Upstream can mean any of these:

  • The ticket template, for example "list the tests that must fail before the change".
  • The acceptance criteria.
  • The agent's standing instructions.

Review then gets faster because there's less to catch, not because you're checking less.

A printable version

I turned this pass into a free two-page checklist with tick boxes, the decision table and the re-prompt template. It comes as .docx, .pdf and .md, so you can paste it into your PR template or team wiki: Agent PR Review Checklist (free).

Disclosure: I made that checklist. It's free, and it's part of a small set of templates I sell for specifying and testing AI features.

What does your team check that's missing here? I'd like to hear which agent failure modes you keep running into.


This article was written with the help of AI tools, then reviewed and edited before publishing.

來源:DEV Community · dev.to