Learn

The bug in this diff is in a caller the diff never touched

22 October 2026 · Code review: find it, then defend it

The interviewer shares a pull request with one changed line and asks whether you would approve it. You read the line, it is correct, and you say so. The interviewer then asks who calls this function, and you realise the review was never about the line.

The situation

A review round often hides the defect outside the diff. The changed code is fine on its own; what changed is a promise the function makes to its callers. The skill being tested is whether you go and read those callers before you approve, and whether you can show the failure with an input rather than a hunch.

Here is an original example. The pull request title is "Avoid building a list of overdue invoices". The change replaces square brackets with round ones:

# before
def overdue_invoices(invoices, today):
    return [i for i in invoices if i.due < today]

# after
def overdue_invoices(invoices, today):
    return (i for i in invoices if i.due < today)

The unit test for overdue_invoices wraps the result in list(...) and checks the ids, so it still passes.

What goes wrong, with the input that shows it

Search for callers. One of them, in a file the pull request does not touch, reads the result twice:

def run_reminders(invoices, today):
    overdue = overdue_invoices(invoices, today)
    total = sum(i.amount for i in overdue)
    log.info("overdue total %s", total)
    sent = 0
    for inv in overdue:
        send_reminder(inv)
        sent += 1
    return sent

Take three invoices: one due on 20 October for 40, one due on 21 October for 60, one due on 1 November for 10, and run it on 22 October. Before the change, the log says 100 and two reminders go out. After it, the log still says 100, and run_reminders returns 0. The sum used up the generator, so the loop has nothing left to read. No exception, no warning, and the one number anyone looks at is still right.

Compare Java. A Stream used a second time throws IllegalStateException, so the same mistake there fails loudly on the first run. Python's generator fails silently, which is exactly why this is the finding to lead with.

Now rank it the way you would rank any finding:

• Who gets hurt? Customers who should have been reminded and were not, and the team that finds out from the revenue figures weeks later.
• When does it show? Not in CI, unless some test counts the reminders sent. Not in the logs, because the total is correct.
• Can it be undone? Late reminders can be sent, but the missed days cannot be recovered.

What to say out loud

"The changed line is correct, but it changes what the function returns from a list to a one-shot iterator, so I checked the callers. run_reminders reads the result twice. With two overdue invoices it logs the right total and then sends zero reminders, silently. I would block this as it stands. The cheapest fix is to keep the list. If memory really is the concern, I would ask how many overdue invoices we have at most; if it is thousands rather than millions, a list costs nothing worth saving. If we do want the generator, every caller has to be checked in the same change, and I would add a test that counts reminders sent."

That answer names the contract change, gives a concrete input, states the effect, offers a fix, and asks the one question that decides whether the change was worth making.

The exercise

1. Pick a function in your own code that returns a collection and has at least two callers.
2. Write down, in one line, what each caller assumes about the result: can it be read twice, can it be empty, can it be modified, is it sorted.
3. Imagine the return type changed to something lazier or narrower. For each caller, write the smallest input that would behave differently.
4. For the worst one, say aloud in under thirty seconds: the change, the input, the effect, the fix.
5. Then say which question to the author would settle whether the change is worth keeping.

Where this fits in Gadfly

Gadfly has four rooms: algorithms, system design, behavioral and code review. The code review room takes a class you wrote yourself, a connected GitHub repository, or a project from its catalog, and reviews it together with the transcript of how the code was produced. It does not execute the code, so a trace like the three invoices above is something you reason through aloud. Code review is part of a session pack. You code in Java or Python, and the tutor speaks English.