Review an AI-generated pull request by tracing the requested behavior through the actual diff, testing the important failure boundaries, and checking that the reported evidence applies to the current revision. Green CI is useful evidence about the checks that ran. It does not establish that the checks express the right requirements or that the patch preserves behavior outside their coverage.
The authoring tool does not change your application’s contract. It does change how easily a polished explanation can arrive alongside a large, plausible patch. Treat the explanation as a guide to inspect, then form a judgment from the code and observable results.
Establish the change before judging the implementation
Read the originating requirement and restate its intended effect. For example:
Add an option to include archived orders in a tenant’s order list. Tenant isolation must remain unchanged.
That sentence identifies both new behavior and an invariant. A review that checks only “archived orders now appear” misses half the contract.
Inspect the complete changed-file list, including tests, build scripts, dependency files, generated artifacts, and CI configuration. If the patch changes how checks run, determine what the new green result actually covers. A deleted assertion or a narrowed test selection can change the meaning of success.
GitHub’s guide to agent pull requests similarly emphasizes reviewing test and CI changes and requiring evidence for nontrivial behavior. The exercise below demonstrates one specific gap rather than estimating how often agents make it.
A plausible filter with a missing boundary
Here is the proposed implementation in a synthetic order-listing function:
def list_orders(rows, tenant, include_archived=False):
return [
row for row in rows
if include_archived
or (row["tenant"] == tenant and not row["archived"])
]
It is concise and appears to extend the existing predicate. The problem is the location of include_archived. When it is true, the entire condition becomes true for every row, including rows belonging to another tenant.
Two supplied tests passed in the local fixture:
- The default call returns only active orders for tenant
a. - A call with archived orders enabled, using only tenant
adata, returns both of that tenant’s rows.
Both tests are meaningful. Neither challenges the interaction between the new option and tenant isolation.
A reviewer added a dataset with active and archived orders from tenants a and b. For tenant a, enabling archived orders should return IDs [1, 2]. The proposed function returned [1, 2, 3, 4]. The new boundary test failed.
The repaired predicate keeps tenant membership outside the optional archive condition:
def list_orders(rows, tenant, include_archived=False):
return [
row for row in rows
if row["tenant"] == tenant
and (include_archived or not row["archived"])
]
The two supplied tests and the new boundary test then passed. These runs used Python 3.14.2 and its unittest framework. This is a synthetic list filter, not a test of a live service or a complete authorization design.
Turn the finding into review feedback someone can act on
A useful review comment identifies the trigger, incorrect outcome, expected boundary, and smallest evidence that establishes the problem:
When
include_archived=True, the first branch of the OR accepts every row before tenant membership is checked. A request for tenantatherefore returns tenantbrows. Keep the tenant predicate mandatory and add a mixed-tenant test with the option enabled.
That is stronger than “this looks insecure” or “add more tests.” It gives the author a concrete behavior to reproduce and preserves the invariant that the fix must satisfy.
For a real service, continue following the boundary. Is the tenant value derived from authenticated identity or supplied by the caller? Does the database query enforce the same restriction? Can another endpoint bypass it? The list fixture proves a predicate error, but it cannot answer questions about an application that was not inspected.
Check tests for what they can disprove
Read a new test’s setup before its assertions. If every fixture row belongs to the same tenant, the test cannot reveal cross-tenant leakage. If a mock always returns the new response shape, it cannot prove the deployed provider returns that shape.
For a bug fix, establish that the new regression test fails on the relevant old behavior and passes after the fix. For a feature, identify both a positive case and a boundary that must remain unchanged. Avoid demanding redundant tests simply to increase a count.
Watch for assertions copied from the implementation’s output without an independent requirement. A snapshot can accurately preserve the wrong behavior. Check whether the expected value comes from the contract, a known example, or a verified prior behavior.
When the patch affects concurrency, persistence, authorization, or external calls, a small unit test may need support from an integration check or a targeted manual exercise. State the missing evidence if the required environment is unavailable; do not imply that passing unit tests cover it.
Reconcile the review with the exact revision
A PR can change after a comment, a local test run, or a CI result. Record the reviewed commit and inspect later changes. If the author repairs the tenant predicate but also alters the response schema, the old review does not cover the newly introduced behavior.
Keep these forms of evidence separate:
| Evidence | What it establishes | What it leaves open |
|---|---|---|
| Diff inspection | The reviewer examined particular changes. | Runtime behavior not exercised. |
| Local test command and result | Specific checks ran in a recorded environment. | Other runtimes, services, and revision changes. |
| Remote CI at a commit | The configured jobs reported their results for that commit. | Requirements those jobs do not test. |
| Staging or production readback | An observed deployed behavior at a time and target. | Behavior outside the observation. |
A generated PR summary should not collapse those rows into “fully verified.” Ask for the commands, results, relevant omissions, and revision needed to assess the claim.
Make the merge decision proportional to the change
A small presentation fix may be supported by a focused diff and a visual check. A data migration needs evidence about existing data, execution order, and recovery. A new dependency needs a reason, compatibility evidence, and a review of the resulting dependency changes. Apply the scrutiny the effect requires.
Before approval, the reviewer should be able to explain what changed, why the chosen implementation satisfies the request, which important boundaries were checked, and what uncertainty remains. An unresolved finding should stay explicit rather than being buried beneath a passing check count.
In the fixture, that decision was straightforward: the proposed patch was not ready despite its two passing tests; the repaired predicate passed the added boundary check. The useful review contribution was identifying the missing invariant and producing evidence that challenged it.
Documentation and the synthetic review exercise were checked October 28, 2026. No real pull request was merged and no agent accuracy benchmark was performed.
