# Code Review

Reads a change against what it was meant to do, and reports what breaks, where, and under which input — before it reaches anyone who did not ask for it.

## Deliverable

One Markdown document, `code-review-report.md`, in the structure set out under **Output** below. It takes a change produced under **Implementation Planning** and returns findings the author can act on directly.

## Required inputs

- **The change itself** — a diff, a set of changed files, or a comparison between two revisions in the version control system.
- **The intent of the change** — what it is meant to do, in the author's words or from the specification it implements.

If either is absent, stop and report it. A review without stated intent finds style; it cannot find a change that correctly does the wrong thing.

## Optional inputs

- The surrounding code the change calls into and is called from
- The test suite, and whether it passes on the change
- The inputs the system accepts and the states it can be in
- The error handling and logging conventions the project follows
- Security, privacy or compliance requirements that apply to this area
- Prior reviews of the same area and what they asked for

Absent optional inputs limit the review's reach, and every finding is scoped accordingly: a finding about behaviour that could not be read is a question, not a defect.

## Execution

**1 — Establish what the change is meant to do.** Restate the intent in one sentence and list the behaviours it implies. Every behaviour goes on the list, including the error paths and the empty case. This list is what the diff is measured against.

**2 — Read the change in execution order, not file order.** Follow the path a request or a call actually takes through the changed code. Reading alphabetically hides the interaction between two files that only fails when both run.

**3 — Look for correctness before anything else.** Null and empty values, boundaries, ordering, concurrent access, partial failure, resource release, and every input the code does not validate. For each one, name the input or the state that triggers it.

**4 — Test every finding against the code.** A finding must name the file, the line and the input or state under which it fails. If the failure cannot be demonstrated from the code that was read, the finding is recorded as a question in the review, not asserted as a defect.

**5 — Check the change against the behaviour list.** Anything on the list from step 1 that the diff does not implement is a gap, and is reported as such rather than as an opinion about what would be nice.

**6 — Check what is not there.** Missing tests for the changed paths, unhandled error branches, states the change introduces but does not clean up, and callers that were not updated.

**7 — Sort by severity and write the fix.** Each finding gets a severity and the change that resolves it. A finding with no proposed fix is incomplete, unless the fix is a decision the author must make — in which case name the decision.

## Output

`code-review-report.md`, in this order:

- **1. Input and scope** — what was reviewed, which revisions, what was not read, and when
- **2. Stated intent** — the intent and the behaviours it implies
- **3. Findings** — ordered by severity; per finding: severity, file, line, what breaks, the input or state that triggers it, and the fix
- **4. Questions** — suspected problems that could not be demonstrated, each with what would settle it
- **5. Gaps against intent** — behaviours the intent implies that the change does not implement
- **6. Missing coverage** — changed paths with no test, and unhandled error branches
- **7. What was not reviewed** — files, paths or behaviour outside the material supplied

## Validation

The report is ready when all of these hold:

- Every finding in section 3 names a file, a line and the input or state that triggers the failure
- No finding in section 3 lacks a demonstrated failure; the undemonstrated ones sit in section 4
- Every behaviour listed in section 2 appears as implemented, as a finding, or in section 5
- Every finding carries a severity and either a fix or the decision it needs
- Section 6 lists the changed paths with no test, or states that all of them are covered
- Section 7 names what the supplied material did not allow to be checked

Fail the run if a finding is asserted with no file and line, or if style comments are ordered above correctness findings.

## Failure handling

- **No intent supplied** — stop. Report that the change cannot be reviewed against its purpose, and that a review of code against itself catches only internal inconsistency.
- **A description instead of a diff** — stop. Report that a description of a change is not a change, and name what is needed.
- **No access to the surrounding code** — review the diff alone, move every finding that depends on a caller into section 4 as a question, and say plainly what could not be read.
- **Intent and code disagree** — report it as a finding at the highest severity the disagreement can cause, and name both sources. Do not decide which one is correct.
- **A very large change** — review it in the order of step 2, deliver what was covered, and name the rest in section 7. A partial review that states its boundary is usable; one that implies completeness is not.