How I Review an AI-Generated Patch Before I Merge It

I would approve an AI-generated patch when I can connect the changed code to the intended behavior, explain why the relevant tests would catch a mistake, and identify what remains unverified. A confident explanation and a green test summary are useful starting points, but I still need to inspect what changed and what those tests exercised.

My review ends with one of three decisions: approve a sufficiently supported change, request changes for a demonstrated defect, or hold the decision because essential requirements or evidence are missing. Below, I work through a small hypothetical fix to show how I would reach that decision. It is an invented teaching example, not a customer incident or a benchmark of an AI coding tool.

Disclosure: This article is part of a TiboReset promotional campaign.

I write down the behavior before judging the patch

Suppose a small web application has a helper called pageSize. The product rule is simple: accept a numeric integer from 1 through 100, inclusive. Return the default size of 20 for anything else. For this example, values outside the range must use the default; they must not be clipped to the nearest boundary. Strings are not valid inputs to this helper.

The reported defect is that a request ending up with the numeric value 101 produces a page size of 101. The original implementation is:

function pageSize(value) {
  return value || 20;
}

An AI assistant proposes this replacement:

function pageSize(value) {
  return Math.min(value || 20, 100);
}

That change caps the result at 100, which sounds like progress if I only read a summary saying “fix the maximum page size.” But the expected result for 101 is 20. The proposed result is 100. The patch therefore implements a different policy from the one I was asked to review.

I would put that distinction into a review comment before discussing style: “For input 101, the requirement calls for the default 20; this implementation returns 100. Please reject out-of-range values rather than clamping them.” It names an input, an expected result, and an actual consequence of the code. The author can act on it without guessing what “handle limits better” means.

If nobody has decided whether an excessive value should be rejected, clipped, or replaced with a default, I would hold this part of the review and ask the owner of the behavior. I would not let the assistant’s implementation silently become the product requirement.

I read the diff together with the code that uses it

I start with the changed files and the exact revision under review. Then I follow this helper to its callers. Where is the query parameter converted into a number? Where is the returned value used? Does the API response or interface rely on the previous default? A correct helper can still be wired into the wrong place.

For the hypothetical application, I would expect a small helper change and relevant tests. A new package, a rewritten request handler, or a renamed response field would each need a separate reason. I would ask for an unrelated cleanup to be separated if it prevents me from tracing the fix. A slightly longer diff can be justified; an unexplained behavioral change cannot be justified by calling it cleanup.

I use Google’s guidance on what to look for in a code review as a useful reference for considering design, behavior, unnecessary complexity, and test validity. In particular, it asks reviewers to consider whether tests would fail on broken code. The worked example and decision record here are my own application of that idea, not a Google-approved procedure.

I also inspect changes to test configuration. If the patch fixes one helper but excludes its directory from the test runner, a green result has become less informative. If a generated file changes, I look for the source or generation step that explains it. I do not assume that every unfamiliar file is suspicious; I need enough context to explain why it belongs in this change.

I ask whether the new test could reject the wrong fix

These examples use Node’s built-in node:assert; assert.strictEqual checks both value and type. Imagine the patch arrives with this weaker assertion:

assert.ok(pageSize(101) <= 100);

It accepts the proposed result of 100, so it cannot distinguish the intended default behavior from the incorrect clamping behavior. It would also accept 0 or a negative number. The test has encoded a weaker rule than the requirement.

I would replace that check with an assertion about the actual contract:

assert.strictEqual(pageSize(101), 20);

That is a useful regression check for the reported defect. It is still only one check. A replacement function that always returns 20 would pass it while breaking every valid input except 20. I therefore add a normal case with a different value, such as assert.strictEqual(pageSize(37), 37), rather than treating the first green assertion as complete evidence.

I would also verify what the test imports. A test that copies the corrected implementation into its own file is testing that copy. It says little about whether the application calls the real helper. A mock that replaces the helper with a fixed answer creates a similar problem. I want the assertion to reach the production function through the project’s actual test setup.

Where practical, I would check the new regression test against the old implementation in an isolated checkout or disposable test harness. The useful observation is a failure at the relevant assertion, followed by a pass with the proposed correction. A compilation failure, an unavailable package, or a test that was never discovered does not establish that the assertion detects the bug. I would not alter a shared branch merely to demonstrate this.

I test the boundary and the default as separate promises

For this example, an implementation that directly expresses the stated rule would look like this:

function pageSize(value) {
  return Number.isInteger(value) && value >= 1 && value <= 100
    ? value
    : 20;
}

I would still review it against a small matrix. These are expected results for the hypothetical helper and the results implied by the earlier clamping expression, not measurements from a deployed application.

The valid endpoint checks matter even after the invalid-input test passes. If someone accidentally changes value <= 100 to value < 100, the 101 test still passes. The 100 test exposes the new error. This is the sort of deliberate, small counterexample I would use to assess whether the assertions protect the behavior I care about.

I would consider zero, non-finite numbers, and other invalid types too, according to the helper’s actual callers. However, I would not expand this into an invented promise to sanitize every possible API input. In the example, parsing a query string belongs upstream. A helper-level check cannot establish that a request containing limit=37 arrives as the number 37 rather than the string “37”.

I distinguish a test result from a report about a test

When an assistant says “all tests passed,” I look for the test command or job, its output, and the revision it exercised. I check whether the relevant tests ran, whether any were skipped, and whether the process finished successfully. A summary copied from a previous revision cannot validate changes made afterward.

I would read an unexpected failure before asking the assistant to change the code again. If the test runner cannot load a dependency, the application behavior remains untested. If an assertion reports 100 where the requirement says 20, that is directly relevant evidence of a defect. Those situations call for different next steps.

GitHub’s responsible-use guidance for Copilot agents explicitly warns that generated code can look valid while being incorrect, and calls for users to review and test it. I apply that boundary to the assistant’s explanation as well: an explanation can help me locate the intended fix, but it is not independent confirmation that the fix works.

For this small helper, I would seek focused tests first, then the application checks that the change actually affects. An integration check should exercise request parsing and the value passed to the pagination layer. If a visible pagination control changes, I would inspect that interaction too. I would not claim that a local helper assertion verifies a database query, a browser, or a deployment.

The amount of evidence should fit the consequence of a mistake. If I cannot inspect a material part of the path, I record that limitation and identify the person or check needed to resolve it. Asking a second AI to approve the first AI’s answer may produce useful questions, but agreement between two explanations does not supply the missing execution evidence.

I separate requested changes from missing evidence

My review comment should make the next action obvious. I would request changes to the clamping implementation because its result contradicts the written rule. I would hold approval of an otherwise plausible correction if the request parser has changed but nobody has checked how it supplies the helper’s argument. I would avoid describing both situations vaguely as “needs more testing.”

These are review decisions, not three promises about delivery. Approval of this patch would not establish that a deployment succeeded. A team may have separate merge permissions, release checks, and operational approvals. I keep my review statement precise enough that someone cannot mistake a helper review for approval of an entire release.

I use waiting time to finish the human review

If I am waiting before I can ask an AI assistant for another revision, I can still check the caller, tighten the expected outputs, and write the unresolved question. That is useful work even if another generated patch never arrives. I would leave the current decision as request changes or hold rather than merge simply to finish the task.

For optional scheduling context during that wait, TiboReset is an independent, public Codex reset tracker, not an official OpenAI service. I would keep that public context separate from the review evidence: it does not prove my account’s available quota, the quality of the code, or whether a patch is ready to merge. I would not plan a required correction around a promised reset.

Before asking for the next revision, I would specify the failed requirement and the smallest useful change. For this example: preserve valid integers, return 20 for invalid values, and ensure tests reject both the old implementation and an always-default replacement. That is a clearer revision request than asking the assistant to make the patch “production ready.”

I leave a decision record another reviewer can use

I would copy the following into a review note and replace every bracketed field with actual evidence. The fields intentionally distinguish what I inspected from what I still need. An unfilled field is not a successful check.

Change and reviewed revision: [identifier]
Intended behavior: [input, expected outcome, preserved behavior]
Files and callers inspected: [scope]
Reported defect: [reproducer and old result]
Relevant assertions: [tests and exact expected results]
Defect-detection check: [old/broken variant, result, or not run]
Execution evidence: [command/job, revision, time, outcome]
Integration or UI evidence: [result, or explicitly unverified]
Unresolved questions and owner: [specific gap and next action]
Decision: [approve / request changes / hold]
Reason: [evidence supporting that decision]
Re-review trigger: [what changed or what evidence must arrive]

For the hypothetical clamping patch, I would record request changes because 101 yields 100 instead of 20 and the supplied upper-bound assertion accepts that error. For the corrected helper, I would assess the actual test and integration evidence before filling in a decision. Seeing better-looking code alone would not let me mark those fields complete.

If a later revision changes the helper, its callers, or its tests, I would revisit the affected conclusions and check that the evidence still corresponds to the code being merged. The result I want is a review someone else can follow from requirement to change to evidence, without having to trust the assistant’s confidence or mine.