AI-Assisted Development

How I Review AI-Generated Code: A Practical Checklist

How I Review AI-Generated Code: A Practical Checklist

AI coding assistants are now a normal part of how many of us write software. I use them for scaffolding, boilerplate, tests and exploring unfamiliar APIs, and they make me faster. They also produce code that looks right, compiles, and is wrong in ways that are easy to miss. The speed is only worth having if the review is good. This is the checklist I use before AI-written code gets merged, with concrete C# examples of what each step catches.

Treat AI output like a pull request from a fast, new teammate

The mental model that works best for me: AI output is a pull request from a teammate who types very fast, has read a lot of code, and has never seen this codebase, its conventions, its production incidents or its business rules. You wouldn’t merge that person’s PR without reading it, so don’t merge the AI’s either.

Three principles follow from that:

  • You own it. Once it’s merged it’s your code. “The AI wrote it” isn’t an explanation anyone will accept during an incident review.
  • Smaller is safer. Ask for focused changes: one function, one endpoint, one test class. A 600-line generated diff is very hard to review properly.
  • Context improves output but doesn’t replace review. Giving the assistant your conventions, existing types and constraints produces better code. It doesn’t make the code correct.

Correctness first: build, run and read every line

The minimum bar: it compiles without new warnings, the existing tests pass, and you’ve run the new code path yourself. Then read every line and make sure you can explain what it does. If you can’t explain a line, don’t merge it.

The bugs I watch for most are the ones that compile and look plausible:

// AI-generated: looks fine, has two bugs
public async Task<List<Order>> GetRecentOrdersAsync(int customerId)
{
    var since = DateTime.Now.AddDays(-30);           // local time vs UTC in the DB
    return await _db.Orders
        .Where(o => o.CustomerId == customerId && o.CreatedAt > since)
        .ToListAsync();                              // unbounded result set
}

// Reviewed version
public async Task<List<Order>> GetRecentOrdersAsync(
    int customerId, int take = 50, CancellationToken ct = default)
{
    var since = DateTime.UtcNow.AddDays(-30);
    return await _db.Orders
        .AsNoTracking()
        .Where(o => o.CustomerId == customerId && o.CreatedAt > since)
        .OrderByDescending(o => o.CreatedAt)
        .Take(take)
        .ToListAsync(ct);
}

Neither problem shows up in a quick test against a small local database. Both show up in production. The questions I ask on every generated method:

  • Are there off-by-one errors or wrong boundaries (> vs >=)?
  • Are time zones handled consistently?
  • Is the result bounded and paged?
  • Is async done properly, with no .Result or .Wait() and with CancellationToken passed through?
  • Are null and empty inputs handled?

Hallucination checks: do the APIs, packages and config options exist?

AI assistants sometimes produce method names, overloads, configuration keys or NuGet package names that look completely plausible but don’t exist, or that exist in a different version from the one you use. The compiler catches non-existent methods. It doesn’t catch:

  • Configuration keys in appsettings.json that nothing reads, so a “timeout” setting silently has no effect.
  • Package names that don’t exist or, worse, that someone has registered to impersonate a real package. Check every new package on nuget.org: owner, download history, repository link, last update.
  • Behaviour claims in comments, such as “this method is thread-safe” or “this retries automatically”. Verify against the official docs.
  • Version mismatches, where code written for a newer or older framework version compiles but behaves differently.

My rule: any new dependency, configuration key or framework behaviour the code depends on gets checked against the official documentation before merge. It takes a few minutes and has saved me hours.

The .NET CLI can also flag known-vulnerable and outdated packages:

dotnet list package --vulnerable --include-transitive
dotnet list package --outdated

Security review: input validation, injection, secrets and auth

Generated code tends to follow the happy path, so security needs deliberate attention. The classic example is string-built SQL:

// AI-generated: SQL injection
var sql = $"SELECT * FROM Users WHERE Email = '{email}'";
var users = await connection.QueryAsync<User>(sql);

// Reviewed: parameterized (Dapper)
const string sql = "SELECT Id, Email, DisplayName FROM Users WHERE Email = @Email";
var users = await connection.QueryAsync<User>(sql, new { Email = email });

The same applies to EF Core. FromSqlInterpolated parameterizes interpolated values, but passing a pre-built string to FromSqlRaw does not. My security checklist for generated code:

  • Input validation: is every external input validated for type, length, range and format?
  • Injection: no string-concatenated SQL, shell commands, file paths, LDAP or XPath queries built from input.
  • Secrets: no keys, passwords or connection strings in code, tests or sample config. Generated examples often include realistic-looking placeholder secrets that end up committed.
  • Authorization: does the new endpoint have the right policy? Does it check that the caller owns the resource, not just that they’re logged in?
  • Data exposure: are entities returned directly from the API, leaking internal fields? Use DTOs.
  • Logging: are tokens, passwords or personal data being logged?
  • Crypto: no hand-rolled hashing or encryption. Use the framework’s vetted APIs.

Tests: let the AI write them, then check they test something

AI assistants are good at producing a lot of tests quickly. The risk is tests that pass whatever the code does. Watch for:

// Looks like a test, verifies nothing about behaviour
[Fact]
public async Task CreateOrder_Works()
{
    var result = await _service.CreateOrderAsync(new CreateOrder(1, 2));
    Assert.NotNull(result);
}

// Tests the actual rule
[Fact]
public async Task CreateOrder_rejects_quantity_above_stock()
{
    _inventory.SetStock(productId: 1, quantity: 5);

    var act = () => _service.CreateOrderAsync(new CreateOrder(ProductId: 1, Quantity: 6));

    await Assert.ThrowsAsync<InsufficientStockException>(act);
    Assert.Equal(5, _inventory.GetStock(1));   // and nothing was reserved
}

Questions I ask about generated tests:

  • Would this test fail if I broke the code? A quick way to find out is to break the code on purpose and run it.
  • Does it cover edge cases (empty, null, boundary values, concurrency) and not just the happy path?
  • Is it testing behaviour, or does it mock so much that it only tests the mocks?
  • Did the AI change an existing test’s expected value just to make it pass? That’s a red flag in any diff.

Maintainability: naming, duplication and fitting existing patterns

Code that’s correct and secure can still be a burden if it doesn’t fit the codebase. Generated code often:

  • Duplicates helpers that already exist, because the assistant didn’t know about them.
  • Introduces a new pattern: a different error-handling style, logging approach or mapping library from the rest of the project.
  • Over-engineers: interfaces with one implementation, factories for simple objects, abstractions nobody asked for.
  • Adds noisy comments that restate the code rather than explain why.

The fix is usually to ask for a revision (“use our existing Result<T> type and IClock abstraction”) or to edit by hand. The test I apply: would a teammate reading this in six months recognise it as our code?

The copy-paste checklist

Here’s the whole thing in a form you can paste into a pull request template:

## AI-assisted change review

### Correctness
- [ ] Builds with no new warnings; all tests pass
- [ ] I ran the new code path myself
- [ ] I can explain every line
- [ ] Boundaries, time zones, nulls and empty inputs handled
- [ ] Async is correct (no .Result/.Wait, CancellationToken passed)
- [ ] Queries are bounded/paged

### Hallucination check
- [ ] Every new API/overload verified in official docs
- [ ] Every new NuGet package verified (owner, downloads, repo)
- [ ] Config keys are actually read by the code
- [ ] `dotnet list package --vulnerable` is clean

### Security
- [ ] All external input validated
- [ ] No string-built SQL/commands/paths
- [ ] No secrets in code, tests or config
- [ ] Authorization and resource ownership checked
- [ ] DTOs used; no sensitive data in logs

### Tests
- [ ] Tests fail when the code is broken
- [ ] Edge cases covered, not only the happy path
- [ ] No existing assertions weakened to make tests pass

### Maintainability
- [ ] Reuses existing helpers and patterns
- [ ] No unnecessary abstractions
- [ ] Naming and comments match project conventions

None of this is specific to AI. It’s a good code review checklist for any change. The difference is volume: AI makes it easy to produce far more code than before, so the discipline has to scale with it. I use AI to go faster, and this checklist is how I make sure faster doesn’t mean worse.

Leave a Reply

Your email address will not be published. Required fields are marked *

Keep reading

Related articles