Review an AI generated pull request for plausible but wrong code

A pull request adds a beautifully named helper, clear comments, and several green tests. It also returns another account's records because the helper removed a scope filter. Plausible code is dangerous precisely because it looks consistent enough to escape casual review.

AI-generated changes deserve the same behavioral scrutiny as human-written changes. The authoring method does not determine correctness. What matters is whether the diff satisfies the contract under realistic inputs, failures, and permissions. This article uses an illustrative “recent invoices” endpoint to show a review method that finds errors beyond syntax.

Read the contract before the diff

Start with the requested behavior and existing consumers. The endpoint should return recent invoices for the authenticated account, in a stable order, with only public fields. A frontend may expect an array while a new helper returns a nested object. Both can be valid JavaScript and incompatible application behavior.

Write a short list of invariants. Account scope must remain server-derived. Ordering must be deterministic. Unpaid invoices must remain included if that is the existing rule. Private processor metadata must not enter the public response. These statements guide the review more effectively than reading every added line with equal attention.

Inspect the acceptance examples and compare them with the actual change. If the PR description promises only a sorting fix but changes serialization and authorization, the review scope must expand. A polished description cannot substitute for understanding the full behavioral delta.

Trace one request end to end

Follow an illustrative request from route entry through authentication, query construction, serialization, and response. Identify where each invariant is enforced. Do not assume an imported helper preserves ownership merely because its name includes “secure.”

Read the old implementation alongside the new one. A removed condition can matter more than hundreds of added lines. If the old query included accountId and the new query accepts only a date range, determine whether ownership moved elsewhere or disappeared. Follow the actual argument values, not the comments.

Then trace an error. A failed database query should not become a successful empty response unless that behavior is explicitly intended. Catching all exceptions and returning an empty list can hide outages and confuse users. Check which errors reach monitoring and which become client-visible outcomes.

Challenge plausible assumptions

Ask where identifiers, dates, and permissions originate. A generated helper may accept an account identifier from query parameters because the type looks convenient. Type checking confirms that a string is a string; it does not confirm that the user owns the resource it names.

Look for fabricated library methods, version mismatches, and optimistic claims in comments. A comment saying “atomic update” does not make two separate operations atomic. A function named sanitize does not establish safe HTML handling. Verify the implementation property directly and consult the installed library's documentation where needed.

Pay attention to defaults. A missing date range might query all history. An omitted page size might remove a protective bound. A null role might be treated as an administrator through an overly broad condition. Defaults are often where an apparently tidy abstraction changes the product contract.

Inspect tests for their ability to fail

A test that mocks the helper and asserts that the route calls it proves little about the helper's query. A test that expects whatever a fixture builder returns may reproduce the same wrong field names as the implementation. Ask what incorrect implementation each test would reject.

For account isolation, construct two synthetic accounts with distinct invoices. Request the first account's endpoint and assert that no invoice from the second appears. The fixture should make leakage visible, even when both accounts have invoices within the same date range. This is stronger than testing only a single account.

Check failure paths independently. Make the storage operation reject and assert the intended status, body, and logging behavior. Verify the test does not swallow the rejection or pass before asynchronous assertions run. The related article on AI-generated tests explores these false-confidence patterns in detail.

Use AI as a counterexample generator

Provide the diff, the contract, and a small set of known invariants. Ask an assistant to identify a concrete input or execution order that violates each invariant. Require file and symbol references and distinguish observed issues from hypotheses.

An effective request is: “Find a request that could return an invoice outside the authenticated account. Trace the arguments and query conditions. If scope is preserved, show where. Do not infer security from names.” This produces a more checkable review than asking whether the PR “looks good.”

Verify every proposed issue yourself. The assistant may miss a policy enforced in middleware or invent a problematic call path. A counterexample becomes a finding only when it matches executable behavior or a demonstrable contract violation. Keep speculative concerns out of the final review unless clearly labeled as unanswered questions.

Choose a correction proportional to the defect

If one scope predicate was removed, the smallest correction may restore it and add an isolation regression test. Replacing the entire data layer would introduce more uncertainty than the defect requires. If the new abstraction makes scope optional everywhere, a narrower interface may be justified.

Keep fixes tied to findings. A renamed variable can improve readability, but it should not distract from an authorization flaw. Separate blocking defects from optional improvements so the author can act without guessing priority. Explain the trigger, consequence, and required property in each substantive comment.

After correction, rerun the original counterexample. Tests added after a fix should also fail against the prior implementation when practical. This establishes that they protect the discovered behavior rather than merely exercising the new code.

Record evidence and remaining boundaries

The review conclusion should identify what was inspected and tested. For the invoice example, that might include route-to-query tracing, a two-account isolation test, serializer inspection, and error-response verification. Avoid saying “fully secure” based on a limited review.

Integration gaps deserve explicit wording. A mocked storage layer cannot establish the production query plan or permissions of the database account. A local test cannot prove deployment middleware is configured identically. These limits do not invalidate the review; they define its practical coverage.

A passing build is useful evidence for syntax and type compatibility. It is one part of a larger argument. The strongest review combines a clear contract, a traced implementation, meaningful counterexamples, and verification that the correction changes the observed failure.

Sources

GitHub Copilot best practices emphasizes reviewing and validating suggested code.

Anthropic: Building effective agents discusses checks in defined workflows and clear interfaces.