Ship it or send it back

Level 5: Level 5: Judging Code You Did Not Writehard26 mincode reviewverificationcommunicationjudgment

The whole level as one repeatable pass: six questions in order, a verdict, and a comment specific enough to act on.

A pass you can run when you are tired

Everything in this level is a technique. This lesson is the order you apply them in, so that reviewing a hundred-line diff at the end of a long day still produces the same result as reviewing it fresh.

Six questions. In this order, because each one tells you what to look for in the next.

1. What is the contract?

Say it in one sentence, including what happens for the awkward inputs, before you read the body. If you cannot, stop here: that is the review comment, and it is the most valuable one you will write today. Code that does not have a statable contract cannot be verified by anybody, including its author in six months.

2. What are the boundaries?

The body tells you. Any division, any index, any comparison against a threshold, any accumulator seeded with a literal, any assumption that a collection is non-empty. Write down the three inputs you would need in order to be sure, and check whether the tests contain them. Usually they do not.

3. What happens on the error paths?

Every try, every except, every external call, every parse. Is the exception narrowed to the one that is expected? Is a failure visible in the value that comes back, or does it vanish? What does the caller see when the dependency is down?

4. What does it mutate?

Arguments, module-level state, files, rows, anything outside the function. A function that sorts its argument in place is a different contract from one that returns a sorted copy, and the difference is invisible in the return value.

5. What will it cost at real size?

Membership tests against lists, front-of-list removals, string concatenation in loops, sorting inside loops. Correct output at the wrong cost is the one defect no test result will show you.

6. What does it depend on, and does it exist?

Imports, methods, keyword arguments, version-specific behavior. Loud failures are cheap. The dangerous case is a real API used with the wrong meaning.

Check yourself

Why does the review pass start with the contract instead of reading the body?

Because the body is usually too long to read carefullyBecause without a contract there is no definition of wrong, so nothing you notice is a findingBecause the contract is where most bugs areBecause reading the body first biases you towards approving it

Turning findings into a verdict

Not everything you notice should block a merge. Sorting your findings is part of the job, and doing it badly is how reviews become slow without becoming better.

Block when the code is wrong for an input that will occur, when a failure is silent, when data can be corrupted, or when the cost is wrong at the size the system actually runs at.

Comment without blocking when the finding is a readability preference, a smaller improvement, or a question you would like answered but whose answer will not change the merge.

Say nothing about style a formatter already owns.

One rule makes the difference between a review that gets acted on and one that gets a thumbs up and no change: every blocking comment names an input. Not "this might break on edge cases". Instead: "page_slice(rows, 1, 3) returns rows 4 to 6, and page 1 should be rows 1 to 3."

Check yourself

Sort each review finding by whether it should block the merge.

except Exception: pass around the parse loop, so malformed rows vanish

The helper could be a comprehension instead of a loop

The function sorts its argument in place, and two callers reuse that list

A membership test against a list inside a loop over 200k rows

The variable named data could have a more specific name

No test covers the empty input the contract explicitly mentions

Writing the comment

A comment that gets acted on has three parts: the line, the input, and the consequence. Anything shorter starts a discussion instead of a fix.

Line 6: seen is a list, so n in seen scans it. On the 200k-row nightly import that is about twenty billion comparisons and the job will not finish in its window. Making seen a set is a one-word change.

Line 12: this returns 0 when there are no ratings, and the caller renders it as the average. A listing with no reviews will display "0.0 stars" rather than "no reviews yet". Should this return None instead?

Notice the second one is a question. Plenty of findings are genuinely questions about intent, and asking is faster and more accurate than guessing at what the author meant.

Check yourself

Which review comment is most likely to result in a change?

This does not handle edge casesI would write this differentlyLine 6 returns 2 for page_count(10, 10), and 10 items at 10 per page is 1 pagePlease add more tests

What this level was actually about

The market changed the question. Writing a small function that passes hidden tests is the task that is now automated end to end, and the interviews that used to test it have moved on to something else: here is code you did not write, tell me what is wrong with it and prove it.

Every technique in this level is one answer to that question.

Recover the contract, trace the boundary before you run anything.
Assume the example passes, because whatever produced the code was fitted to it.
Know the failure shapes on sight, so a suspicion becomes a specific input.
Write the assertion that fails now, because that is the only durable artifact.
Use properties and a slow oracle when examples run out.
Shrink the failing input until it explains itself.
Repair to the class the contract names, and do not rewrite.
Read for cost, which no test result will show you.
Wrap unreliable dependencies, validate what they return, verify the APIs are real.

None of this is anti-AI. Generated code is a draft from a fast collaborator who does not know your codebase, has not read your contract, and will not be on call when it fails. Judging that draft accurately, and proving your judgement with a test, is the part of the job that got more valuable, not less.

Interview nuance: if an interviewer hands you a function and asks what you think, the pass in this lesson is your answer, spoken out loud. Contract, boundaries, error paths, mutation, cost, dependencies. You will find something almost every time, because the code was chosen to have something in it, and you will find it in a visibly repeatable way rather than by luck.

Check yourself

You have thirty minutes to review a 200 line generated pull request. What produces the most value?

Read every line carefully from top to bottomRun the six questions against the two or three functions that touch data or moneyRun the test suite and approve if it is greenRewrite the parts you would have written differently

Worked example (Python)

def weekly_summary(entries):
    total = 0
    for day, minutes in entries:
        total += minutes
    return {"total": total, "average": round(total / len(entries), 1)}


# Question 1: what is the contract when a day was never logged?
# Question 2: what happens on an empty list?
print(weekly_summary([["mon", 30], ["tue", 45]]))

Apply

Your turn

The task this lesson builds to.

Write a function check(name) that returns True only when the candidate stored under name is a correct apply_refund, and False otherwise.

The contract: apply_refund(order, amount) refunds amount cents against an order dictionary holding "total" and "refunded". The amount is capped so "refunded" never exceeds "total". An amount of zero or less refunds nothing. The function returns a new dictionary and does not modify the one it was given.

The starter holds five candidates. Two are correct, and the three that are not each fail a different question from the review pass: one mutates its argument, one skips the cap, and one lets a negative amount run backwards. Keep check as the last function in the file.

3 hints and 5 automated checks are waiting in the workspace.

Practice

Make it stick

A second problem on the same idea, so it survives past today.

Your habit-tracking product emails every user a weekly summary on Monday morning, and an assistant wrote the weekly_summary function in the starter. It was tested on one full week of data, which is the only shape it handles. Users who missed a day, users who logged nothing all week, and users who signed up on Sunday are all currently getting either a crash or a wrong number.

Repair weekly_summary so it satisfies its contract.

The contract: entries is a list of [day, minutes] pairs, where minutes is a whole number or None for a day that was never logged. Return a dictionary with "days_logged" (how many entries have a real number), "total" (the sum of those minutes), "best_day" (the day with the most minutes, with ties going to the one earliest in the list, and None when nothing was logged), and "average" (the total divided by "days_logged", rounded to one decimal place, or 0.0 when nothing was logged).

A logged day of zero minutes still counts as logged.

3 hints and 5 automated checks are waiting in the workspace.