---
title: Your PR Is Green. Should You Still Be Nervous? - Autter Blog
description: Tests are green, the code compiles, and the PR looks harmless. Learn what code review must catch beyond CI and why reviewing a diff is only the beginning.
url: https://autter.dev/blog/merge-room-episode-1-your-pr-is-green
date: 2026-08-21
author: Autter Team
tags: The Merge Room, Code Review, Engineering
reading_time: 9 min
site: Autter - Autter is your AI teammate for code review and testing: it reviews code, tests product impact, checks security, governs releases, and closes the loop from production failure to verified fix.
---

[← All posts](https://autter.dev/blog)

Aug 21, 2026

The Merge Room · Episode 01 / What code review is actually for

# Your PR Is Green. Should You Still Be Nervous?

Tests are green, the code compiles, and the PR looks harmless. Learn what code review must catch beyond CI and why reviewing a diff is only the beginning.

Autter Team 9 min read

- The Merge Room
- Code Review
- Engineering

Key takeaway

A green pull request tells you that the checks you wrote have passed. It does not tell you that the change is correct for every caller, safe under real traffic, consistent with the system’s rules, or harmless downstream. Good code review tests the assumptions around the code, not just the lines in the diff.

## 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.

```
export async function chargeOrder(orderId: string) {  const order = await getOrder(orderId);  if (order.status !== "paid") {    await paymentProvider.charge(order.total);    await markOrderPaid(order.id);  }}
```

*TypeScript*

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.

![Captain Patch tracing two concurrent requests that both pass a green gate before colliding at the same payment card](https://autter.dev/blog/merge-room-race-condition-v1.jpg)

*Two requests. Two green checks. One customer charged twice.*

> “Every individual line looks reasonable. The failure lives in the space between two executions.”

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. Perhaps`chargeOrder` 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.

![Captain Patch mapping one changed function to routes, retries, jobs, tools, tests, and a database](https://autter.dev/blog/merge-room-blast-radius-v1.jpg)

*The diff is the starting point. The affected system is the review surface.*

### 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 malformed`if`. 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.

| Layer | Best at finding | Typical examples |
| --- | --- | --- |
| Automated checks | Known, repeatable violations | Type errors, formatting, vulnerable dependencies, known security patterns |
| Contextual analysis | Relationships and affected paths | Downstream callers, duplicated logic, missing coverage, architectural drift |
| Human judgment | Intent, tradeoffs, and organizational context | Product behavior, acceptable risk, design clarity, rollout decisions |

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.

Low-value comment

Could you rename `result` to `paymentResult`?

Useful comment

This path can be entered by both the checkout request and the webhook retry. Can two workers charge the same order before either writes the new status? We may need a provider idempotency key plus an atomic state transition.

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.

```
export async function chargeOrder(orderId: string) {  const claimed = await claimOrderForPayment(orderId);  if (!claimed) return;  await paymentProvider.charge({    amount: claimed.total,    idempotencyKey: `order:${claimed.id}`,  });  await markOrderPaid(claimed.id);}
```

*TypeScript*

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.

The Merge Room · knowledge check

## Would you approve the merge?

Answer all three. Once you commit your answers, we’ll explain each call and unlock the Autter merch draw. Your score does not affect your chance of being selected.

**What does a green CI result prove?**

The change is safe in production Every downstream caller still works The configured checks completed successfully A senior engineer no longer needs to review the PR

**Why can a tiny diff deserve a deep review?**

Short code is usually suspicious Diff size does not reveal downstream reach Linters are less reliable on small files Review tools charge by the line

**Which comment creates the most review value?**

Please alphabetize these imports. I would have written this with a ternary. Both retrying entry points can call this before the state update. What prevents a duplicate charge? Can we rename data?

0 of 3 answered

Commit answers

Next in The Merge Room

Episode 02

## The diff is not the crime scene

We’ll follow one harmless-looking change through callers, dependencies, and tests to see why code review needs a map of the application. The diff shows us where to start, but it does not show us where the behaviour can travel.

[See what a pull request touches beyond its changed lines →](https://autter.dev/contact)

### Inside The Merge Room

Answer the knowledge check to unlock this episode’s Autter merch draw. Your score is for learning, not eligibility.

Take the knowledge check

## Put an assurance layer on every release.

Autter reviews every pull request before it merges, backed by humans. Connect a repo and see your first findings today.

[Get started](https://app.autter.dev/login)

## Keep reading

[Jul 28, 2026 · 15 min read Autter vs Claude Code Review: Should the AI That Writes Your Code Also Review It? Autter is the stronger first choice for teams that need risk-based verification, enforceable merge gates and predictable code assurance. See how it compares with Claude Code Review.](https://autter.dev/blog/autter-vs-claude-code-review)