Blog

Code review checklist with a copyable PR template

Thirty checks grouped by correctness, tests, security, performance, readability and AI-written code, plus a Markdown pull request template you can copy.

Alex Mercer

A code review checklist is a short list of questions a reviewer asks of every pull request: does the change do what it claims, is it tested, is it safe, will it hold up in production, and can the next person maintain it. Ours has 30 checks in six groups, including one for code a coding agent wrote, plus a Markdown template you can paste into your pull request template so authors run the same checks before they ask for review.

Treat the list as a floor. Anything a formatter, linter, type checker or CI job can enforce belongs in those tools, so the checklist only holds what needs a person's judgment. For where review fits in the wider workflow, see our guide to what code review is and how the process works.

Before you start

Do these before you read the first line of the diff:

  • Read the description and the linked issue. You can't judge whether code is correct until you know what it's supposed to do.

  • Check CI. If tests or lint fail, ask for a fix first and review after.

  • Check the size. A pull request that mixes a refactor with a feature is hard to review well, so ask for two. More on this in Does PR size actually matter?

If you review on GitHub, our step-by-step guide to reviewing a pull request on GitHub covers the Files changed tab, suggestions and approvals.

Correctness

  1. Does the change do what the description and issue say? Compare the behavior with the acceptance criteria. Watch for partial implementations, hard-coded values that only make the demo work, and edits unrelated to the stated goal.

  2. Is the logic right at the edges? Empty collections, null or missing values, zero, negative numbers, the first and last item, time zones, daylight saving changes, very large inputs.

  3. Do errors fail safely? No empty catch blocks. Errors are logged with enough context to debug, resources get closed, retries have a limit, and users see a message they can act on.

  4. Is concurrent or repeated work safe? Two requests updating the same row, a job that runs twice, a webhook delivered again. Look for missing transactions, locks or idempotency keys.

  5. Does it break existing callers or data? Removed or renamed API fields, new required parameters, changed types, and schema changes without a migration, a backfill or a way to roll back.

Edge-case bugs are easy to miss because the code reads fine:

def last_n(items, n):
    return items[-n:]
def last_n(items, n):
    return items[-n:]
def last_n(items, n):
    return items[-n:]

A review comment might say: "last_n(items, 0) returns the whole list, because items[-0:] is the same as items[0:]. Return [] when n <= 0 and add a test for it."

Tests

  1. Is the new behavior tested? A bug fix should come with a test that fails without the fix.

  2. Would the tests fail if the code were wrong? Assertions should check results. A test that only proves nothing threw, or that repeats the implementation line by line, proves little.

  3. Are error paths and edge cases covered? Timeouts, invalid input, empty results and permission failures need tests too, along with the happy path.

  4. Do the tests run the real code? Heavy mocking can end up testing the mocks. Mock the network and the clock, and leave the function under test alone.

  5. Are the tests deterministic? No sleeps, real network calls, shared state, dependence on test order, or dates that change the result next month.

Security

  1. Is every input validated on the server? Client-side checks help users; they don't protect the server. OWASP's Secure Code Review Cheat Sheet recommends allowlists over blocklists.

  2. Are queries and commands parameterized? User input should never be concatenated into SQL, shell commands or HTML.

  3. Is authorization checked for every resource? Being logged in isn't enough. If changing an ID in a URL returns someone else's record, that's an insecure direct object reference. Enforce access on the server and deny by default.

  4. Are secrets out of code, logs and error messages? No keys committed to the repository, no tokens in log lines, no stack traces or internal details in responses.

  5. Is every new dependency needed and safe? Check what it pulls in, whether it's maintained, its license and its known vulnerabilities. On GitHub, the rich diff of a manifest or lock file shows dependency review: what changed, and whether it adds known vulnerabilities.

In code, the parameterized-query check looks like this:

# Unsafe: the email becomes part of the SQL text
cur.execute(f"SELECT id FROM users WHERE email = '{email}'")

# Safe: the driver sends the email as a separate parameter
cur.execute("SELECT id FROM users WHERE email = %s", (email,))
# Unsafe: the email becomes part of the SQL text
cur.execute(f"SELECT id FROM users WHERE email = '{email}'")

# Safe: the driver sends the email as a separate parameter
cur.execute("SELECT id FROM users WHERE email = %s", (email,))
# Unsafe: the email becomes part of the SQL text
cur.execute(f"SELECT id FROM users WHERE email = '{email}'")

# Safe: the driver sends the email as a separate parameter
cur.execute("SELECT id FROM users WHERE email = %s", (email,))

The placeholder syntax (%s, ? or :name) depends on your database driver.

For pull requests that touch login, sessions or tokens, go further with our security review checklist for authentication changes.

Performance

  1. Are there queries or network calls inside a loop? The classic N+1: one query for a list, then one more per item. Batch or join instead.

  2. Is the work bounded? Queries have limits or pagination, inputs have size caps, external calls have timeouts, and retries back off and stop.

  3. Does it load more than it needs? Whole tables or files read into memory, SELECT * where two columns would do, the same data fetched twice in one request.

  4. Is anything expensive on a hot path? Work repeated on every request or loop iteration that could run once or be cached. If the change adds a cache, check how entries expire.

  5. Will it hold up on production data? A migration, backfill or new index that finishes instantly on a dev database can run for a long time on a large table. Ask how it behaves at production size and whether it can run in steps.

Readability and maintainability

  1. Can someone new to this code follow it? Clear names, functions that do one thing, shallow nesting. If you had to read a function three times, say so.

  2. Does it follow existing patterns? Business logic where the rest of it lives, the same error handling and logging style, existing helpers instead of new ones.

  3. Is there duplication to remove, or abstraction nobody needs yet? Copy-pasted blocks drift apart over time. A plugin system with one caller is cost with no payoff yet.

  4. Do comments explain why? The code already says what it does. Comments should give the reason, the constraint or the incident behind it. Delete commented-out code.

  5. Are docs and config updated? README, API docs, environment variables, runbooks and changelogs that this change makes wrong. New feature flags get an owner and a removal plan.

AI-written code

Code from a coding agent gets the same review as anyone else's, plus five extra checks. Review keeps finding real bugs in it. On cubic's Bug Bench, which counts cubic review findings on AI-written code that a developer upvoted or a coding agent addressed, the seven models in its current ranking have relative bug rates from 0.65 to 1.50 (1.00 is average, lower is better). Every ranked model has counted bugs, and the highest rate is more than twice the lowest. The data is observational: teams choose their models, so each rate reflects the code a model was used for as well as the model.

  1. Does every package, import and API exist? Models can invent package names and call functions that aren't in the version you use. A study accepted at USENIX Security 2025 generated 576,000 code samples with 16 code-generating models and found that, on average, at least 5.2% of the packages commercial models referenced were hallucinated, and 21.7% for open-source models. The authors treat this as a new form of package confusion attack, since anyone can publish a real package under a name a model invents. Check each new dependency against its registry.

  2. Does the diff stay in scope? Agents sometimes reformat files, rename things or refactor code the task didn't need. Ask for unrelated changes in their own pull request, and check that a generated description matches the actual changes.

  3. Were any tests deleted, skipped or loosened? Read the test diff as closely as the code. Removed assertions, new skip markers, wider tolerances and regenerated snapshots can turn a red build green without fixing anything.

  4. Does it reuse existing code and follow your rules? An agent that didn't read the rest of the repository may add a second date helper, a different validation library or its own logging style.

  5. Do errors surface? Watch for broad exception handlers that swallow failures, fallback values that hide bugs, and placeholder logic left in.

The person who opens the pull request owns it, whoever wrote the code. If they can't explain a change, request changes.

An AI reviewer can take the first pass on these checks; cubic, for example, checks every pull request against your whole codebase and your team's rules, which you write in plain English. Our AI code review guide covers how that work splits between AI and human reviewers.

Copy this code review template

Save the template below as .github/pull_request_template.md. GitHub also reads pull_request_template.md from the repository root or from docs/. Once the file is on your default branch, GitHub fills it into the body of every new pull request. Each - [ ] line renders as a task list checkbox, and HTML comments are hidden in the rendered description, so they work as prompts for the author.

## What this changes
<!-- One or two sentences. Link the issue, for example #123. -->

## Why
<!-- The problem this solves, or the decision behind it. -->

## How I tested it
<!-- Commands you ran, screenshots, or the new tests that cover it. -->

## Risk and rollout
<!-- Migrations, feature flags, config changes, anything to watch after deploy. -->

## AI assistance
<!-- Which parts did a coding agent write, with which tool and model? What did you check by hand? -->

## What this changes
<!-- One or two sentences. Link the issue, for example #123. -->

## Why
<!-- The problem this solves, or the decision behind it. -->

## How I tested it
<!-- Commands you ran, screenshots, or the new tests that cover it. -->

## Risk and rollout
<!-- Migrations, feature flags, config changes, anything to watch after deploy. -->

## AI assistance
<!-- Which parts did a coding agent write, with which tool and model? What did you check by hand? -->

## What this changes
<!-- One or two sentences. Link the issue, for example #123. -->

## Why
<!-- The problem this solves, or the decision behind it. -->

## How I tested it
<!-- Commands you ran, screenshots, or the new tests that cover it. -->

## Risk and rollout
<!-- Migrations, feature flags, config changes, anything to watch after deploy. -->

## AI assistance
<!-- Which parts did a coding agent write, with which tool and model? What did you check by hand? -->

To offer different templates, say one for features and one for hotfixes, put them in .github/PULL_REQUEST_TEMPLATE/ and pick one with the template query parameter when you open the pull request. Our pull request template guide covers templates in more depth.

Keep the checklist useful

  • Automate what you can. Formatting, lint rules, type errors, known-vulnerable dependencies and committed secrets belong in CI, where a tool checks them on every push.

  • Prune and add. Drop items that never catch anything. After an incident that review could have caught, add the check that would have caught it.

  • Label your comments. Prefix optional feedback with Nit: or Optional: so the author knows what blocks the merge. Our code review glossary explains nit, PTAL and the rest of the shorthand.

  • Tailor it per repository. A payments service and a marketing site need different security and performance items. Keep a short shared core and add sections where a codebase needs them.

FAQ

What should be included in a code review?

Check that the change does what it says, that tests would catch it breaking, that it's safe (input handling, authorization, secrets, dependencies), that it performs at production scale, and that the next person can maintain it. Read the description and linked issue first so you know what "correct" means. Leave formatting and style to formatters and linters.

What is the process of code review?

The author opens a pull request with a description and a link to the issue, and CI runs. A reviewer reads the description, then the diff, leaves comments and submits a decision; on GitHub that's Comment, Approve or Request changes. The author pushes fixes, the reviewer checks them, and the change merges once the required approvals and checks pass.

Which is the best tool for code review?

No single tool covers a whole review. A practical setup combines your platform's pull request reviews (on GitHub or GitLab), linters and type checkers in CI, a SAST scanner such as CodeQL or Semgrep, Dependabot alerts for vulnerable dependencies, and an AI reviewer such as GitHub's Copilot code review or cubic (our product). People still own design and intent.

Table of contents