Published / updated29 Aug 2026

Reviewing AI-Generated Code

Before you start

You need: Articles 01–04. This is the checklist article — keep it open while you review.

Time: about 50 minutes.

Learning objective

Review generated code against a checklist that catches the bugs which produce no error message.

Topics

• Why generated code needs a different review
• The security checklist
• The correctness checklist
• The multi-tenant checklist
• Data-type traps
• Over-engineering
• Invented APIs
• A review script

Why generated code needs a different review

Human code and generated code fail differently.

Human code	Generated code
Bugs cluster around what the author found hard	Bugs cluster around what you did not specify
Style varies with the author	Style is confidently plausible
A gap usually looks like a gap	A gap looks like finished code
You can ask why	There is no why to ask

Generated code has no "this bit is rough" signal. Human code often signposts its own weak points — a comment, an awkward name, a // TODO. Generated code is uniformly polished, so the weak parts look exactly like the strong parts.

Review it as you would code from a capable stranger who has never seen your requirements. That is accurate, and it sets the right level of scepticism.

The security checklist

Highest priority. These are the failures with consequences beyond a bug.

SQL injection

// Reject
string sql = $"SELECT * FROM Student WHERE RollNumber = '{rollNumber}'";
string sql = "SELECT * FROM Student WHERE Name LIKE '%" + search + "%'";

// Accept
const string sql = "SELECT * FROM Student WHERE SchoolId = @SchoolId AND RollNumber = @RollNumber";

Any user input inside a SQL string is a rejection, no exceptions. Generated code does this more often when the query is dynamic — a search with optional filters is where it appears.

Authorisation

// Reject — no check
[HttpDelete("api/students/{id}")]
public async Task<IActionResult> Delete(int id)

// Accept
[Authorize(Roles = "Admin,Principal")]
[HttpDelete("api/students/{id}")]
public async Task<IActionResult> Delete(int id)

Check that the authorisation is on the server. Hiding a button in the frontend is interface, not security — the endpoint is still reachable from Postman.

Secrets

// Reject
private const string ConnectionString = "Server=prod;User Id=sa;Password=P@ssw0rd;";
private const string JwtKey = "my-super-secret-key-12345";

// Accept
private readonly string _connectionString = configuration.GetConnectionString("SchoolDb");

Generated code frequently hardcodes an example secret because that is what examples do. It looks like a placeholder and gets committed.

Password handling

// Reject
user.PasswordHash = password;
user.PasswordHash = Convert.ToBase64String(Encoding.UTF8.GetBytes(password));
user.PasswordHash = ComputeMd5(password);

// Accept
user.PasswordHash = BCrypt.Net.BCrypt.HashPassword(password);

Base64 is encoding, not hashing. MD5 and SHA-1 are unfit for passwords.

Data exposure

// Reject — returns PasswordHash, and every column
return Ok(await _context.Users.ToListAsync());

// Accept
return Ok(users.Select(u => new UserDto { PublicId = u.PublicId, Name = u.Name, Email = u.Email }));

Returning the entity directly leaks whatever the entity holds — password hashes, internal ids, other tenants' foreign keys.

Error messages

// Reject
catch (Exception ex) { return StatusCode(500, ex.ToString()); }

// Accept
catch (Exception ex)
{
    _logger.LogError(ex, "Failed to create student in school {SchoolId}", schoolId);
    return Problem("An error occurred processing your request.", statusCode: 500);
}

The correctness checklist

Absent versus zero

// Reject
if (result.MarksObtained >= subject.PassingMarks) { return "Pass"; }
if (result.IsAbsent) { return "Absent"; }
return "Fail";

// Accept
if (result.IsAbsent) { return "Absent"; }
if (result.MarksObtained >= subject.PassingMarks) { return "Pass"; }
return "Fail";

The absent check comes first, in every grading chain. This is the project's most-violated rule in generated code, and it produces wrong report cards with no error.

Nulls

// Reject — throws on an empty set
decimal total = payments.Sum(p => p.Amount);          // fine
FeeAccount account = accounts.First(a => ...);         // throws if none

// Accept
FeeAccount account = accounts.FirstOrDefault(a => ...);
if (account == null) { return NotFound(); }

-- Reject
SELECT SUM(Amount) FROM FeePayment WHERE FeeAccountId = @Id

-- Accept
SELECT ISNULL(SUM(Amount), 0) FROM FeePayment WHERE FeeAccountId = @Id

Ordering assumptions

// Reject — arbitrary result when there are several
FeeAccount account = await _context.FeeAccounts
    .FirstOrDefaultAsync(a => a.StudentId == studentId);

// Accept
FeeAccount account = await _context.FeeAccounts
    .Where(a => a.StudentId == studentId
             && a.SchoolId == schoolId
             && a.AcademicYear == currentYear)
    .FirstOrDefaultAsync();

First with no OrderBy and no uniqueness constraint is a bug waiting for the second row.

Joins

-- Reject if students without accounts must appear
FROM Student s JOIN FeeAccount f ON f.StudentId = s.Id

-- Accept
FROM Student s LEFT JOIN FeeAccount f ON f.StudentId = s.Id

Check the row count. An inner join that drops rows, or a join that multiplies them, produces a wrong total silently.

Async

// Reject
_auditRepository.LogAsync(entry);            // not awaited
var result = _service.GetAsync().Result;     // deadlock risk
async void Handler()                          // exceptions cannot be caught

// Accept
await _auditRepository.LogAsync(entry);

Disposal

// Reject
SqlConnection connection = new SqlConnection(cs);

// Accept
using (SqlConnection connection = new SqlConnection(cs)) { }

Transactions

// Reject — the second call is outside the transaction
using IDbTransaction transaction = connection.BeginTransaction();
await connection.ExecuteAsync(sql1, p1, transaction);
await connection.ExecuteAsync(sql2, p2);            // no transaction

// Accept — every call inside receives it
await connection.ExecuteAsync(sql2, p2, transaction);

Every command inside a transaction must be passed the transaction. Miss one and it commits independently — so a rollback leaves half the operation applied.

The multi-tenant checklist

Every one of these is silent. No exception, no log line, a 200 response.

// Reject
public async Task<IActionResult> GetStudents([FromQuery] int schoolId)

// Accept
int schoolId = int.Parse(User.FindFirst("schoolId").Value);

-- Reject
SELECT * FROM Student WHERE ClassName = @ClassName

-- Accept
SELECT * FROM Student WHERE SchoolId = @SchoolId AND ClassName = @ClassName AND IsDeleted = 0

// Reject — School B may be served School A's list
string key = $"students:{className}";

// Accept
string key = $"students:{schoolId}:{className}";

-- Reject
UNIQUE (RollNumber)

-- Accept
UNIQUE (SchoolId, RollNumber)

Four checks, every time:

1. Does SchoolId come from the claim?
2. Is it in every WHERE?
3. Is it in every cache key?
4. Is every uniqueness constraint composite?

Data-type traps

// Reject
public double Amount { get; set; }
public float TotalFees { get; set; }

// Accept
public decimal Amount { get; set; }

Amount FLOAT           -- reject
Amount DECIMAL(18,2)   -- accept

Trap	Correct
double / float for money	decimal / DECIMAL(18,2)
DateTime.Now on a server	DateTime.UtcNow, converted for display
string for an enum value	The enum
int for a public identifier in a URL	PublicId (Guid)
varchar for names	nvarchar, for non-ASCII names

DateTime.Now on a UTC server produces attendance dated one day off in IST, which looks like a logic bug and is not.

Over-engineering

Generated code frequently includes more than you asked for:

Added without request	Question
An interface with one implementation	Needed for testing, or ceremony?
A caching layer	Is there a measured performance problem?
A retry policy	Is the call actually flaky?
A generic repository	Does anything reuse it?
Try/catch around everything	Can you handle these, or are you hiding them?
A configuration option	Will it ever be configured?
An abstract base class	Is there a second implementation?

Delete what you did not ask for. Every unrequested abstraction is code someone must maintain, and speculative flexibility is the most common form of it.

A caching layer added without a measured problem is worse than none, because it introduces staleness bugs to solve a problem you did not have.

Invented APIs

students.WhereNotNull()                   // does not exist
services.AddSchoolPortalDefaults()        // does not exist
connection.QueryWithRetryAsync(...)       // does not exist

{ "Logging": { "EnableDetailedDatabaseErrors": true } }

A compile error catches most of these. The ones to worry about are configuration keys and JSON settings, which fail silently — the key is ignored and the setting simply never takes effect.

Verify any configuration key against the official documentation. A wrong key produces no error at any point.

A review script

Run these before opening a pull request containing generated code:

grep -rn "SELECT.*\" *+\|\$\"SELECT\|\$\"INSERT\|\$\"UPDATE" src/       # string-built SQL
grep -rn "double \|float " src/ --include=*.cs                          # money types
grep -rn "DateTime.Now" src/                                            # local time
grep -rn "catch (Exception)" src/                                       # swallowed exceptions
grep -rn "\.Result\|\.Wait()" src/                                      # sync-over-async
grep -rn "Password\s*=\|ApiKey\s*=\|ConnectionString\s*=\s*\"" src/      # hardcoded secrets
grep -rn "FromQuery.*schoolId" src/                                     # SchoolId from the request
grep -rLn "SchoolId" src/Data/                                          # queries with no tenant filter

The last one is worth explaining: -L lists files not containing SchoolId. In a multi-tenant data layer, any repository file it names needs justifying.

dotnet build -warnaserror
dotnet list package --vulnerable
dotnet test

-warnaserror catches CS4014 — the missing await — which is otherwise a warning nobody reads.

Errors you will hit

What to check	Failure that produces no error
SchoolId source	Taken from the request — another school's data, 200 response
Soft-delete filter	Deleted records reappear in reports
Money type	double — totals wrong by paise
Absent handling	Compared before the absent check — wrong report cards
Transaction propagation	Half the operation survives a rollback
Cache key	Missing tenant — intermittent cross-school leak
Returned entity	Salary and ParentPhone exposed

Seven checks, none of which throw. That is why reading beats running.

Common mistakes

• Reviewing generated code less carefully than a colleague's
• Reviewing style rather than correctness
• Trusting it because it compiles
• Missing the tenant filter
• Missing the absent-check order
• Accepting double for money
• Not checking transaction propagation
• Keeping abstractions you did not ask for
• Not verifying configuration keys
• No automated checks before the pull request

Practice

The course exercise is review generated code against the checklist.

1. Generate a search endpoint with optional filters. Check specifically for string-built SQL.
2. Generate a delete endpoint. Check whether it has an [Authorize] attribute.
3. Generate a data-access class and look for a hardcoded connection string.
4. Generate user registration and check how the password is stored.
5. Generate a "get all users" endpoint and check what fields the response exposes.
6. Generate grading logic five times. Count how often the absent check comes first.
7. Generate a fee total query and check for ISNULL around the SUM.
8. Generate a lookup with First and construct data where two rows match.
9. Generate a student-with-fees report and compare row counts before and after the join.
10. Generate a money field and check the type in both C# and SQL.
11. Generate a multi-statement transaction and check that every call receives it.
12. Generate a cached list and check the key for a tenant segment.
13. Generate a controller and check where SchoolId comes from.
14. Run every command in the review script against a real project and read the results.
15. Build with -warnaserror and see whether any generated code produces CS4014.
16. Ask for a repository method, then count how many things it added that you did not request.

Exercise 6 is the calibration exercise. Whatever rate you observe is the rate at which you must catch it in review.

You can now

• Review generated code against security and correctness checklists
• Catch the failures that produce no error message
• Verify a configuration key or package actually exists
• Remove abstractions you did not ask for
• Run an automated pass before opening a pull request

Review questions

1. Why does generated code lack the signals human code gives about its weak points?
2. Which four multi-tenant checks produce no error when they fail?
3. Why is First without ordering a bug in waiting?
4. Why is an unrequested caching layer worse than none?

Next: Responsible vibe coding