A code review checklist for production changes

The pull request has thirty comments about naming, a suggestion to extract a helper, and two approvals. It also lets two customers reserve the last item. Everyone read the code. Nobody followed the state transition far enough to ask what happens when both requests arrive together.

A useful code review checklist directs attention toward failures that matter. It is not a catalog of everything a developer should know. If every item has equal weight, formatting gets the same attention as authorization, and the easy observations crowd out the expensive ones.

Start with the behavior the change promises. Then follow the data, the effects, and the deployment path until you can explain why that promise survives the conditions the system actually faces.

Before reading line by line, ask the author for a short description of the user-visible change, its constraints, and the validation performed. For a reservation endpoint, that could be: "A signed-in customer can reserve available stock. A retry must return the existing reservation. Concurrent requests must never reserve more units than exist."

That paragraph is a better starting point than a file list. It gives you claims to challenge. Google's guide to navigating a review likewise recommends understanding the overall change and examining its most important part first. The review order below applies that principle to production risk.

Review the failure that could hurt a customer before the improvement that could please a reader.

This order is a prioritization tool, not permission to skip maintainability. A confusing design can hide a correctness problem. The point is to connect the design comment to the behavior it protects.

Assume request authentication and basic input binding happen outside this abbreviated C# handler. The proposed implementation uses an EF Core entity and one database save:

var stock = await db.Stock.SingleAsync(
    x => x.ProductId == request.ProductId, cancellationToken);

if (stock.Available < request.Quantity)
    return Results.Conflict();

stock.Available -= request.Quantity;
db.Reservations.Add(new Reservation
{
    CustomerId = currentCustomer.Id,
    ProductId = request.ProductId,
    Quantity = request.Quantity,
    RequestKey = request.RequestKey
});

await db.SaveChangesAsync(cancellationToken);
return Results.Ok();

The first review question is about the allowed quantity. Zero should not create a meaningless reservation. A negative quantity increases available stock. The comparison looks like a guard, but without positive input validation it protects the wrong domain. Ask for a concrete invalid-input test before discussing whether the condition belongs in a helper.

Next, trace two requests. Both load Available = 1. Both pass the check for a quantity of one. Each writes the entity's available value as zero and inserts a reservation. If no concurrency token or equivalent protection is configured, the stock value may look fine while two reservations exist. Looking only for a negative inventory count misses the defect.

Read the entity mapping before asserting that this race exists. A concurrency token might already protect the update, in which case the question becomes how the handler handles the conflict. Optimistic concurrency is a complete topic of its own. The review habit is simpler: inspect the mechanism that makes the claimed invariant true, rather than assuming a save method provides it.

One possible repair uses a conditional database update. The following PostgreSQL statement is a focused illustration of the stock operation, not a complete reservation implementation:

UPDATE stock
SET available = available - @quantity
WHERE product_id = @product_id
  AND @quantity > 0
  AND available >= @quantity
RETURNING product_id, available;

The application binds the parameters through its database client. A returned row means this operation claimed the stock; no returned row means it did not. The reservation insert must share the same transaction, so an insert failure rolls back the stock change. PostgreSQL documents how an updating command waits for a concurrent updater and re-evaluates its condition in Read Committed isolation.

The review is still unfinished. What if the transaction commits but the response is lost? A retry can claim stock again unless the request key is enforced durably. A uniqueness rule scoped to the customer and request key can be part of the design, but the handler must also return the previous result and reject reuse with a different payload. A uniqueness exception alone is not the promised retry behavior.

Translate the findings into observable tests. With one available unit, release two independent requests from a barrier and assert exactly one successful reservation. Assert both the final stock and the reservation count. Then repeat a committed request with the same key and verify that no additional units are consumed. Finally, force the reservation insert to fail and verify rollback.

These tests distinguish the repaired mechanism from the original implementation. A test that only asserts SaveChangesAsync was called cannot establish any of those outcomes. Use the real database for the concurrency and transaction behavior; an in-memory substitute may implement different rules.

Suppose the repair also introduces a required request-key column. The final application may compile perfectly while the deployment fails. During a rolling release, an older application instance can still insert reservations without that field. A migration that immediately requires the new value can break those in-flight requests.

Ask what exists at each stage. Can the old writer operate after the first migration? What will the new reader do with old rows? How are duplicates handled before adding a uniqueness rule? Does rollback mean reverting code only, or would it require reconstructing deleted data?

For this hypothetical change, a staged design might first add nullable storage, deploy compatible readers and writers, backfill where the product semantics permit, then tighten constraints after old writers have drained. Historical reservations may need a separate treatment because inventing request keys does not recreate the original client's retry intent. The plan must answer that question rather than hiding it inside a migration filename.

The same reasoning applies to events and APIs. A new consumer may encounter messages produced before the new field existed. A browser opened yesterday may call an endpoint after today's deployment. Review the combinations that can coexist, not only the versions in the author's branch.

After following the worked example, the reusable checklist can be compact:

1. State the behavior and the valid input domain. Identify who is authorized to perform it.
2. Trace the read, decision, and write. Find the enforcement point for each important invariant.
3. Walk through overlap and retry. Identify what survives process failure and what can happen twice.
4. Inspect old/new coexistence. Explain migrations, fallback behavior, and how the release can stop.
5. Read tests for distinguishing observations. Require evidence against the specific plausible bug.
6. Check operational visibility. A failed reservation should be diagnosable without logging secrets.
7. Review clarity and scope. Remove accidental complexity and keep unrelated changes separate.

Adapt it to the change. A documentation correction does not need a transaction analysis. An authorization change needs deeper attention to identity and ownership than an internal formatting refactor. Google's review guidance covers design, functionality, tests, complexity, and surrounding context; none of those categories requires the same amount of time for every diff.

"This feels unsafe" leaves the author guessing. A useful comment names the trigger, the consequence, and the missing evidence:

Blocking: two requests can both read Available = 1 before either saves.
Without a concurrency guard, both can insert a reservation while the final
stock value remains zero. Please enforce the stock claim atomically and
add a concurrent test that checks reservation count as well as stock.

That comment leaves room for different correct implementations. It explains why the issue matters and what would demonstrate a fix. A separate nonblocking suggestion can propose a clearer name without pretending the naming preference has the same consequence.

Avoid turning review into a demand for perfection. Google's review standard favors changes that improve overall code health and distinguishes optional polish from required corrections. Record unresolved production risk explicitly; let harmless preferences remain preferences.

A checklist reminds you where to look. Practice teaches you what a convincing fix looks like. Katabench's practice tracks expose different failures behind apparently working code: behavioral regressions, structural problems, inefficient queries, and exploitable inputs. The grading model makes the applicable constraints explicit instead of relying on a passing happy-path example.

That deliberate practice complements review work. A reviewer still needs the real system's requirements, deployment context, and operating conditions. But repeatedly fixing code under an explicit constraint builds a useful reflex: ask which observation would prove that this particular promise survives this particular failure. That is the question a good checklist should keep alive.