Your PR Is Green. Should You Still Be Nervous?

What you’ll learn

• What CI can prove and what it cannot
• The four questions a useful code review should answer
• Why tiny changes can have large consequences
• How codebase context changes a review
• A practical checklist for your next pull request

The PR that behaved perfectly

Imagine this pull request lands in your queue on a Friday afternoon. It is short enough to scan before your coffee goes cold, and it looks like the author has added a sensible safety check.

01export async function chargeOrder(orderId: string) {02  const order = await getOrder(orderId);0304  if (order.status !== "paid") {05    await paymentProvider.charge(order.total);06    await markOrderPaid(order.id);07  }08}

The guard stops an order that is already marked as paid from being charged again. Sensible. The code compiles, the unit tests pass, and the linter has no complaints. CI produces the reassuring green tick developers have been trained to love. Would you merge it? Take a moment. This is not a trick question. It is a trap question, which is a close relative with worse incident reports.

Two requests can read the same unpaid order at almost the same time. Both pass the guard. Both charge the customer. Only afterward do they mark the order as paid.

That space is where code review begins. The syntax is fine, but the behaviour breaks as soon as the system does two perfectly normal things at once.

Green means “the checks passed”

Teams often treat a green build as a verdict. It is closer to a receipt. CI can tell you that a known set of commands completed successfully: tests passed, types checked, dependencies scanned, formatting matched, and perhaps an image built. That is valuable. It is also bounded by what those checks know how to ask.

If nobody wrote a concurrent-payment test, the test suite cannot feel a disturbance in the force and invent one. It will pass the test it was given and stay completely silent about the question nobody asked.

That is why a green result has a much narrower meaning than we often give it. It does not automatically prove any of the following:

• the requirement was understood correctly;
• the code behaves safely under concurrency;
• every downstream caller still works;
• authorization is enforced at the right boundary;
• an existing helper was not reimplemented inconsistently;
• logs avoid sensitive data;
• the change follows architectural decisions made elsewhere;
• the feature will behave well in production.

This is not a criticism of CI. A smoke detector is useful even though it does not detect burglary. Problems begin when we ask one control to answer every question.

What code review is actually for

A useful review answers four questions. They move the conversation away from whether the diff looks tidy and toward whether the change can survive the real system around it.

1. Does the change do what we intended?

Start with behaviour, not formatting. What user or system outcome is supposed to change? Which outcome must remain unchanged? What assumptions did the author make about data, ordering, permissions, and failure?

In our payment example, the intent is “charge an unpaid order once.” The implementation currently means “charge an order if this particular request observed it as unpaid.” Those sentences sound similar until two requests arrive together.

An order may be charged at most once.

Once the invariant is explicit, the missing atomicity becomes much easier to see. You now have something concrete to review instead of a vague sense that the code looks reasonable.

2. What can this change affect?

A diff shows what changed. It does not show everything that depends on what changed. PerhapschargeOrder is called by the checkout endpoint, a webhook retry handler, an admin recovery tool, and a nightly reconciliation job. Perhaps one caller already retries automatically. Perhaps another assumes the function is idempotent. That is the difference between reviewing a line and reviewing its blast radius.

• Who calls this?
• What does this call?
• Which routes, jobs, or services can reach it?
• Which tests cover those paths?
• What changed since the surrounding design was introduced?

Without those answers, reviewers guess. Experienced reviewers often guess well. They also take holidays.

3. How can it fail in the real system?

Production adds ingredients that unit tests politely leave outside: concurrency, partial failure, retries, stale data, network timeouts, malformed inputs, unexpected permissions, and users with astonishing creativity. For the payment change, that means asking about the whole journey around the function, not only whether the condition reads correctly:

• What if the provider succeeds but markOrderPaid fails?
• What retries this function?
• Does the provider support an idempotency key?
• Can two workers process the same order?
• Is the state transition atomic?
• What evidence would let us reconcile an uncertain result?

Notice how little of this is visible in the syntax. The dangerous part is not a malformedif. It is the lifecycle around it.

4. Will the next developer understand and maintain it?

Correctness today is not the only concern. Review should also protect tomorrow’s changes.

Is the invariant represented clearly? Is the state transition owned by the right layer? Is failure visible? Is the behavior tested at the boundary where it matters? Does this duplicate an existing mechanism?

Maintainability comments are useful when they reduce future risk. “Rename this because I personally prefer another noun” belongs in the Nitpick Bin unless the name creates genuine ambiguity.

The three review layers

It helps to think of review as three layers rather than one giant activity. Each layer is good at a different kind of question, and the review gets weaker when one tries to do all the work.

The goal is not to replace one layer with another. It is to stop making senior engineers spend attention on work a machine can repeat, while machines surface the context humans need for judgment.

Together, those layers produce a better review conversation. The machine carries the repeatable work and brings the relevant context to the person making the decision.

One comment rearranges letters. The other may prevent a very awkward customer email. That is a useful test for whether a review note is protecting the system or only polishing the diff.

The Autter lens: from changed lines to affected systems

Autter builds an application graph from a repository: files, symbols, imports, call relationships, API routes, data models, tests, and reverse dependencies. When a PR changes a function, the useful question is not merely “is this function valid?” It is “where can this function’s behavior travel?”

For the simplified payment example, a contextual review could surface the five facts below. None of them is obvious from the changed lines alone.

1. chargeOrder is reachable from two independently retried entry points.
2. Both paths can operate on the same order.
3. The payment provider call occurs before the local state transition.
4. No concurrency test covers the invariant.
5. A related refund flow already uses provider idempotency keys.

Now the reviewer is not searching the repository from scratch. The relevant map is already on the table. This does not make the merge decision automatically. It makes the decision informed.

A safer shape for the example

The exact fix depends on the database and payment provider, but the design usually needs two protections that work together. One stops the provider from processing the same payment twice, and the other stops two workers from claiming the same order:

• a stable idempotency key understood by the provider;
• an atomic local transition that prevents two workers from claiming the same charge.

01export async function chargeOrder(orderId: string) {02  const claimed = await claimOrderForPayment(orderId);03  if (!claimed) return;0405  await paymentProvider.charge({06    amount: claimed.total,07    idempotencyKey: `order:${claimed.id}`,08  });0910  await markOrderPaid(claimed.id);11}

This snippet still does not solve every failure mode. For example, the system needs a recovery plan if the provider succeeds and the final database update fails. That is the point: serious review follows the behavior through the whole lifecycle rather than declaring victory at the closing brace.

Merge or block?

You are reviewing a PR that changes a shared authorization helper. Its unit tests pass. The diff updates six lines. The helper is used by 47 endpoints, but only two endpoint tests ran because the test selector is based on changed files. The diff looks small, but the reach is not. What do you do?

1. Merge. The helper’s own tests passed.
2. Request a variable rename, then merge. Clean names prevent incidents.
3. Block until the affected authorization paths and test selection are understood.
4. Close the laptop and develop an interest in pottery.

The best answer is C, although D remains emotionally available. The six changed lines are not the thing that should decide how deeply you review this PR.

The risk is not proportional to the number of changed lines. A six-line edit to a high-reachability helper can matter more than a 600-line isolated feature. Review depth should follow impact, not diff size.

What to take into your next PR

1. Intent: What invariant or user outcome must this code preserve?
2. Reach: Which callers, routes, jobs, services, and data flows can this affect?
3. Failure: What happens under retries, concurrency, partial failure, and malformed input?
4. Evidence: Which checks prove the important behavior, not merely the happy path?
5. Clarity: Will the next developer understand why this design exists?

If the answer to one of these is “I assume…,” you have found the next review question. That small pause is often the difference between approving the code you can see and reviewing the system that will actually run it.