How to Review AI Generated Code: A Practical Guide

Review AI-generated code in this order: does it solve the stated problem, does it behave correctly when run, does it fit your system and risk surface, and only then style and naming. A common failure looks like this: a 400-line diff arrives, CI is green, the code reads cleanly, and it is approved in a few minutes. Model output is fluent by default, so line-by-line reading rewards polish and hides the places where the code is confidently wrong about your system.

The short answer: review intent first, behavior second, lines last

I treat an AI-generated diff like a pull request from a very fast contributor who has never seen my production environment. It compiles and it reads well. It can still be wrong about my system, because the model only knows what was in the prompt and its training data.

The order I recommend:

1. Intent. Does the change solve the stated problem, within the stated constraints?
2. Behavior. When you run it, including on bad input, does it do what it claims?
3. Fit and risk. Does it match your architecture, your release process, your identity setup and your data-handling rules?
4. Lines. Naming, style and small refactors, last.

Most people do this backwards and spend their attention on formatting, because formatting is what is easiest to see.

The reason for the order is that plausible code makes line-by-line reading tiring and shallow. Every line looks reasonable, so nothing prompts you to slow down. Starting from intent and behavior points your attention at the places where a model is most likely to be wrong: assumptions about your system, edge cases, failure paths and dependencies.

When not to use the full process

A rename, a typo fix, a copy change or a boilerplate test needs a quick skim and a green CI run, not a ritual. The full process below is for changes that touch behavior, data, identity, infrastructure or dependencies. If you apply the heavy version to everything, reviewers learn to skim it, and that is worse than having no process.

Set up the review so it can fail before you read a line

A review that cannot fail is theater. Set up the conditions so that bad changes get stopped by something other than a tired reviewer.

Require the intent in the PR description

Ask for three things: the problem, the constraints, and what the prompt or instruction asked for. If the author cannot state what the change should do, you cannot review it. You can only check that it looks like code. This also exposes a common pattern, where the author accepted whatever the model produced and never wrote down what they wanted.

Keep the diff small enough to hold in your head

I prefer a judgment cap to a hard line count. The test is whether one person can explain the whole change after reading it once. I also ask the author to split anything that mixes a refactor with a behavior change. Models like to tidy things while they are in the file, and a behavior change buried in a rename-heavy diff is exactly what slips through.

Let machines take the mechanical layer

Before a human reads anything, these should have run:

• formatter, linter and type checker
• unit tests
• a static analyzer such as Semgrep
• secret scanning such as gitleaks
• an image and dependency scan with Trivy if the change touches a Dockerfile, a base image or a lockfile

These tools overlap a little and differ in what they are good at. This is how I think about them:

I use Trivy in CI for every image I ship, and I would pick Semgrep first for custom rules because writing a rule takes minutes. If your team already lives in GitHub and wants deeper analysis, CodeQL is a reasonable addition rather than a replacement. All of these produce false positives, so budget time to tune them.

Make CI the gate, not the reviewer's memory

If a check matters, it should block the merge. "Remember to run the scanner" does not survive a busy week. Branch protection with required checks is a few minutes of setup and removes a whole category of "I forgot".

Watch: Watch the video

Check what the model cannot know about your system

This is the section where human review earns its keep. The model saw your prompt and maybe a few files. It did not see your production environment.

Duplicated logic

Models often write a new helper instead of using the one already in your codebase. Before accepting any new utility function, search the repo for its purpose, not its name. A second date-parsing or permission-check helper is a future bug: the two will drift, and someone will fix one of them.

Operational constraints

For zero-downtime releases, ask these questions about every change:

• Is the schema change backward compatible with the previous version, which is still running during the rollout?
• Do the readiness and liveness probes still make sense after this change, for example a new startup dependency that makes the pod slow to become ready?
• Do config or secret changes need a rollout order?
• Does the new code assume a single instance, such as an in-memory cache or a local file?

Hypothetical example: a diff renames the column phone to contact_phone in one migration and updates the application code to match. It passes tests, because tests run against the new schema only. In a rolling update, old pods keep serving traffic for several minutes against the renamed column and start failing. The safe version is expand and contract across releases: add the new column, write to both, backfill, switch reads, and drop the old column in a later release. A model will rarely propose this unless you ask, because the prompt did not mention that old and new versions overlap.

Identity and authorization assumptions

Check which roles or scopes the code expects, whether it trusts a client-supplied claim, and whether it matches how your identity provider is actually configured. A model will happily write if user.role == "admin" against a role name that does not exist in your realm, or decode a token without verifying the signature because the example it learned from did so. If your roles and clients are defined as code, compare the diff against that definition rather than against your memory of it.

Data-handling rules

Look for new logs, analytics events and API responses that might carry personal data. A model will happily write logger.info("request", extra={"body": request.json}), which puts emails, phone numbers and free-text messages into your log storage. From a GDPR standpoint that is a new processing activity with its own retention problem. The review question is simple: what personal data can reach this line, and is it needed?

Verify that every dependency, API and version actually exists

Models can recommend packages that do not exist. That is not only an annoyance: if an attacker registers the invented name, the next developer who copies the suggestion installs the attacker's code. There is published research on this, including the USENIX Security 2025 paper "We Have a Package for You!" by Spracklen et al. I am not quoting its figures here, so read the paper for the measured rates and methodology. The practical point does not depend on a number: read the lockfile diff, not just the source diff.

For each new dependency

• Confirm it exists on the registry and is the package you think it is, not a near-miss of a popular name.
• Check maintainers, release history and license.
• Check whether it has had any activity in the past year, and how many open security issues it has.
• Ask whether the standard library or an existing dependency already covers the need. Deleting a dependency from a diff is often the best review comment.

Check API usage against the docs for the version you pin

Models often mix signatures from several major versions or use deprecated calls. The code looks right and may even run, until it hits a path that the old signature handled differently. Open the docs for your pinned version and verify the two or three calls that matter most.

Scan the branch

Run the dependency and image scanners on the branch so a vulnerable transitive dependency is caught before merge rather than during the next CVE sweep. Catching it at review costs a one-line version bump. Catching it later costs an incident response.

Read the tests first, then try to break the code

Read the tests before the implementation

If an AI wrote both, the tests frequently mirror the implementation. They prove nothing beyond "the code does what the code does". Reading the tests first tells you what behavior is actually pinned down, and it gives you a specification to compare against the implementation.

Do a manual mutation check

Flip a condition, remove a guard clause or return early, and confirm at least one test fails. If nothing fails, the tests are decoration.

Hypothetical example: a function gives a discount when quantity >= 10. Change it to > 10 and run the suite of 12 tests. If all 12 still pass, no test pins the boundary. Add cases for 9, 10 and 11, and the suite can now detect a real regression. This takes about five minutes per function and is the single fastest way I know to tell real tests from generated ones. Automated mutation testing tools exist for several languages and do this at scale, but the manual version is enough for a single PR.

Ask for the cases a model skips

• empty input and null values
• duplicates
• timeouts and partial failures
• retries that repeat a side effect
• concurrent requests
• very large payloads

Run it once for real

Run the change locally or in a preview environment and exercise the real path once. A green test suite is evidence, not proof. Tests run against mocks of your dependencies, and the mock was probably written by the same model that wrote the code.

Review the failure paths and security surface

Error handling is where generated code is weakest, because the happy path is what the prompt described and the happy path is what the examples in training data show.

Failure paths

Look for:

• swallowed exceptions and empty except or catch blocks
• broad catches that turn a bug into a silent no-op
• retries without backoff or jitter
• missing timeouts on network calls
• fallbacks that hide failures, such as returning an empty list when the database call fails

Hypothetical example: a payment-capture call is wrapped in a retry of three attempts on timeout, with no idempotency key. If the first request succeeded but the response was lost, each retry can charge the customer again. The review comment is not "add a comment", it is "this side effect is not safe to repeat; add an idempotency key or remove the retry".

The classic security list, checked deliberately

• input validation at the boundary, with an allowlist where possible
• authorization on every new endpoint, not just authentication
• SQL or shell commands built from user input instead of parameters
• secrets in code, config samples or log output
• permissive CORS, disabled TLS verification and chmod 777-style shortcuts that models copy from tutorials
• new containers running as root or with broad capabilities

Run the scanners, but do not stop there. A scanner will not tell you that a new endpoint returns another user's data because the ownership check is missing. That is a logic bug, and it takes a human who knows the data model.

Decide: approve, request changes, or re-prompt

One judgment call is specific to AI-generated code. If a diff needs more than one or two rounds of substantive fixes, I would rather discard it and re-prompt with better constraints than keep patching. Patching a design you did not choose tends to leave the original wrong assumptions in place. State the constraints you discovered during review, such as "must be backward compatible with the previous release" or "use the existing PermissionService", and generate again.

A short checklist for the PR template:

• Is the intent stated, and does the diff match it?
• Did CI pass, including the scanners?
• Is there duplicated logic, and did I search for it?
• Is the change safe while old and new versions run side by side?
• Are identity and data-handling assumptions correct for our setup?
• Does every new dependency exist, and is it needed?
• Did I break the code on purpose and see a test fail?
• Are failure paths handled, with timeouts and safe retries?

One honest limitation: this process costs time, and I have not measured how many defects it catches against a lighter one. It is judgment from running production systems, not a controlled result. Scanners produce noise, reviewers get tired, and none of it replaces tests, staged rollouts and monitoring. Treat review as one layer that gets the obvious and system-specific mistakes out before they reach production.