Start with an observable contract
A useful code review begins with a claim you can check. Read the request, the caller, and the existing tests before judging the proposed implementation. Write down what must remain true: which inputs are valid, what the output means, and which side effects are allowed.
For this independent example, a profile service caches lookup results. A profile can be an object or null; null means the account was checked and does not exist. Both are valid cached results. A missing cache key means the origin has not been consulted. The review must preserve that distinction.
Review a plausible patch with a counterexample
Suppose an assistant proposes this helper. It is short, readable, and returns the right answer when the profile exists. However, a cached null enters the fallback branch and calls the origin again. The returned value can look correct while the service performs the wrong work.
async function lookup(id: string) {
const cached = profiles.get(id);
if (cached) return cached;
const value = await origin.lookup(id);
profiles.set(id, value);
return value;
}- Seed the cache with account-17 → null.
- Call lookup("account-17") twice with an origin spy.
- Expect null both times and zero origin calls.
- Also check an absent key and an existing profile so the repair preserves both paths.
Verify the repair at the service boundary
For this in-memory Map example, checking profiles.has(id) distinguishes a stored null from absence. That is a hypothesis to verify, not a reason to stop reviewing. A remote or expiring cache may instead need a single lookup result that includes a found flag; separate contains and get operations can disagree if state changes between them.
Run the new regression against the original code and confirm that it fails for the intended reason. Then apply the patch and rerun it, followed by the existing tests and build. Inspect the final diff for unrelated changes, relaxed assertions, or swallowed errors. Record which behaviors were checked and which assumptions remain.
if (profiles.has(id)) {
return profiles.get(id);
}Give the assistant a bounded review task
A useful request names the observed failure and the constraint: “The cache stores null as a valid result. Trace lookup and propose a regression proving a cache hit does not read the origin. Keep the public return type unchanged.” Review the resulting test as carefully as the implementation; a test that changes the requirement can make a broken patch appear correct.
If the proposal is incomplete, explain the missing case and request a smaller revision. Accepting, editing, or rejecting a patch should follow from the behavior you inspected. The assistant’s confidence and the patch’s length do not establish correctness.
Make your conclusion reproducible
A concise review might say: “Cached null previously triggered an origin request. The regression fails on the original helper and passes after checking membership. Existing hit and miss cases still pass. Concurrent remote-cache behavior is outside this example.” That gives another engineer enough context to repeat the check and understand its limits.
nrml exercises apply this workflow to existing repositories. The assignment arrives in the workspace, where you can inspect callers, ask the assistant for help, run commands, and review changes before finishing. The report explains the recorded evidence; it cannot observe reasoning you never expressed or behavior you never tested.