Code Review
Find defects that matter to the requested change. Review requests produce findings; apply fixes only when the conversation authorizes them. Use the relevant checks below according to the changed behavior and risk, not as a mandatory checklist for every diff.
Establish what is being reviewed
Identify the requested files or diff and the applicable project conventions. For a remote change, establish its base and exact head when accessible; for a local change, distinguish committed changes from working-tree edits. State any unavailable revision or context that limits the conclusion. If the change is retargeted or updated, refresh the affected scope before presenting the review as current.
Read enough surrounding code and callers to reconstruct the intended behavior. Existing design records explain tradeoffs; they are evidence to assess, not grounds to reject a change automatically. Neither a preferred module style nor a particular testing architecture is a universal correctness rule.
Trace the affected behavior
- Interfaces and compatibility: follow producers and consumers of changed arguments, results, errors, configuration, and persisted or wire data. Check old data and external callers when the project promises compatibility; absence of an internal caller does not prove a public API is unused.
- Lifecycle and concurrency: for asynchronous setup or teardown, locate resource ownership, publication, cancellation, and terminal outcomes. Check cancellation during waits, callbacks that throw or reenter, partially completed setup, repeated cleanup, and work still running after disposal where those scenarios can occur.
- Enforcement: follow an authorization or validation decision to the operation it governs. Check whether direct or alternate entry points bypass it and whether state can change between checking and acting.
- Owned and derived state: establish who may mutate or retain a value. Trace caches, notifications, and views to the authoritative state and actual success point; do not assume a type annotation prevents runtime mutation.
- Limits: check the complete emitted or retained result, including wrappers and metadata. For byte limits, distinguish bytes from characters; for batching, include oversized single items and exact-boundary cases.
- Real entry points: inspect the shipped loader, command, process, route, or other relevant entry path. Tests that construct an internal object directly may miss packaging, configuration, exports, initialization, or teardown failures.
- Documented and visible behavior: compare affected user instructions, diagnostics, prompts, and interface descriptions with the implementation. Request updates where a real contract would become false or incomplete; do not require a new document for every internal change.
For a suspected defect, identify a reachable trigger and consequence. An abstraction or defensive path being complex is a prompt to investigate its consumers and purpose, not itself a blocker.
Assess verification
Use existing tests, logs, and checks as evidence for the reviewed revision. Distinguish checks actually observed from an author's report, and identify material scenarios they do not cover. Run focused checks when useful and available; review can still report substantiated findings when execution is unavailable.
Inspect whether an assertion would fail on the suspected regression and whether it observes the required result. A test that repeats the implementation or mocks away the failing entry path provides weak evidence. Review snapshot changes semantically. Choose additional cases based on the failure mechanism rather than demanding an exhaustive matrix or a specific local/CI split.
Report findings
Lead with actionable defects, ordered by impact. Give each finding a precise location, triggering conditions, observed or reasoned failure, and supporting evidence. Separate confirmed defects from unresolved questions and optional design suggestions. Avoid duplicating tool output or reporting stylistic preferences as correctness issues.
State the reviewed scope and material verification limits. If no actionable issue was found, say so without claiming the code is defect-free. Posting comments, changing review status, and applying fixes remain separate actions governed by the user's request and existing authorization.