Track G — The Diff Bank

Thirty agent-produced diffs. Accept, reject, or revise — in ninety seconds each.

The agentic round gives you an oversized task and an AI agent. The agent will produce more code than you can read carefully, and the round is scored on which of it you let through. That is a trainable skill with a specific technique, and this is the drill.

Companion to WARMUP.md (the method, the worked hour, the takeover boundary). Every Python behaviour asserted here was measured on CPython 3.13.


Table of Contents


How to Drill This

Ninety seconds per diff, timer visible. That is roughly the real budget: an hour-long round with an agent producing five to ten diffs leaves you a couple of minutes each, and the ones you linger on are the ones you should have rejected in ten seconds.

For each: say the verdict first, then the reason. In the round you will be thinking out loud, and "Reject — time.sleep in an async function blocks the whole loop" is a complete answer. "Hmm, let me look at this…" followed by forty seconds of silence is not, even if you arrive at the same place.

The three verdicts, and they are not equally weighted:

VerdictMeansThe trap
ACCEPTShip itAccepting because it looks like the code around it
REVISEThe approach is right, one thing is wrongRewriting the whole thing when one line was wrong
REJECTThe approach is wrong; re-prompt or take overRejecting everything — see section H

Section H exists because reflexive rejection is a real failure mode, and interviewers test for it. An engineer who rejects all thirty scores worse than one who accepts the three that are correct, because the round is measuring judgement, not suspicion.


The Six-Pass Review, in Ninety Seconds

Read the diff six times, each pass looking for one thing. In this order — the passes are sorted by how cheaply they find a fatal problem, so you stop early on most diffs.

#PassTimeLooking for
1Does it do what I asked?10 sScope drift, a different problem solved, silent extra changes
2The error path15 sWhat happens when this fails? Is the exception caught, swallowed, or wrong?
3The boundary15 sEmpty, one, exactly-at-the-limit, negative, None
4The resource15 sWhat is opened, locked, or allocated — and is it released on every path?
5The concurrency15 sIs this async? Shared? Is there a check-then-act?
6The test20 sDoes it assert the behaviour, and could it fail?

Pass 6 is where most defects are found, because an agent's test almost always passes and often asserts nothing that would break if the code were wrong. Ask of every test: what single-line change to the implementation would make this fail? If you cannot name one, the test is decorative.

And one meta-rule that outranks all six: if a diff touches something you did not ask about, that alone is grounds to reject and re-prompt. Scope drift in an agent's output is the leading indicator that it misunderstood the task, and everything downstream inherits the misunderstanding.


A. Concurrency and Async (D1–D5)

D1

  async def fetch_all(self, urls):
      results = []
      for url in urls:
-         results.append(await self._get(url))
+     tasks = [self._get(u) for u in urls]
+     results = await asyncio.gather(*tasks)
      return results

REJECT. The intent — parallelize — is right; the implementation is unbounded. Ten thousand URLs means ten thousand coroutines scheduled at once: file-descriptor exhaustion locally, and a DoS against the target.

And a second defect that is easy to miss: gather without return_exceptions=True propagates the first exception while the other tasks keep running, orphaned. Their eventual failures surface as Task exception was never retrieved or vanish.

Re-prompt with the constraint: "bound concurrency with a semaphore of N, and use asyncio.TaskGroup so a failure cancels the siblings."


D2

  async def _retry(self, request):
      for attempt in range(self.max_attempts):
          try:
              return await self._send(request)
          except TransportError:
+             time.sleep(2 ** attempt)
              continue

REJECT — the highest-severity class in this section. time.sleep in a coroutine blocks the entire event loop, not just this task. Every other request in the process stalls for the backoff.

At max_attempts=5 that is 1+2+4+8 = 15 seconds of total service freeze, from one slow dependency. And it will not show up in a unit test, because a test with one request in flight cannot observe the other requests that were not there.

Fix: await asyncio.sleep(...). One word, and the diff is otherwise fine — which makes it REVISE if you are being generous, but say the severity out loud either way.


D3

  class RateLimiter:
      def allow(self, key):
          bucket = self._buckets[key]
          bucket.refill()
-         with self._lock:
-             if bucket.tokens >= 1:
-                 bucket.tokens -= 1
-                 return True
-         return False
+         if bucket.tokens >= 1:
+             bucket.tokens -= 1
+             return True
+         return False

REJECT. The agent removed a lock, presumably as an optimization, and reintroduced a check-then-act race: two threads read tokens == 1, both decrement, both return True, and the bucket goes negative. The limit is exceeded, silently, only under load.

Do not accept "the GIL makes it safe." Measured on 3.13, counter += 1 across four threads loses zero updates — but that is the eval breaker's current scheduling, not a guarantee, and it disappears under free-threading (PEP 703). A correctness argument that depends on an interpreter implementation detail is not a correctness argument.

The critical section is three arithmetic operations; the lock costs nothing. There was no optimization here to make.


D4

+ class Cache:
+     def __init__(self, entries={}):
+         self._entries = entries

REJECT. A mutable default is evaluated once, at function definition, so every Cache() created without an explicit argument shares the same dict. Two caches, one backing store.

Measured, the canonical demonstration:

def f(x, acc=[]): acc.append(x); return acc
f(1)  # [1]
f(2)  # [1, 2]   <-- the same list

Fix: def __init__(self, entries=None): self._entries = entries if entries is not None else {}.

This is a well-known Python trap and agents still produce it, usually when refactoring a signature. Ten seconds to spot; a shared-state bug that takes hours to find in production.


D5

  async def process(self, items):
      async with self._lock:
          for item in items:
-             await self._handle(item)
+             asyncio.create_task(self._handle(item))

REJECT, and it is worse than it looks. Two defects compound:

  1. The lock is released immediatelycreate_task returns instantly, so the loop finishes and the async with exits while every handler is still running. The lock now protects nothing.
  2. The tasks are never awaited and no reference is kept. Python's event loop holds only a weak reference to a running task, so a task with no strong reference can be garbage-collected mid-execution. The work silently does not happen, non-deterministically.

The second is the subtle one and it is documented behaviour: the asyncio docs explicitly say to keep a reference to the returned task.

Re-prompt: "gather the tasks inside the lock, or use a TaskGroup and keep the lock only around the state it protects."


B. Error Handling (D6–D9)

D6

  try:
      return self._transport.send(request)
- except (ConnectionError, TimeoutError):
+ except Exception:
      if attempt < self.max_attempts:
          continue
      raise

REJECT. except Exception catches programming errors — a TypeError from a bug in request construction, an AttributeError from a typo — and retries them three times before surfacing. An instant, clear failure becomes a slow, confusing one, and the retry metric now counts bugs as transient failures.

The narrower point worth making: it does not catch asyncio.CancelledError, which is a BaseException — verified: issubclass(asyncio.CancelledError, Exception) is False. But that is luck, not intent, and the diff would be equally wrong if it said except BaseException.

Retry only what is retryable, and be able to name the list.


D7

  def load_config(path):
      try:
          return json.loads(Path(path).read_text())
-     except json.JSONDecodeError as e:
-         raise ConfigError(f"invalid config at {path}") from e
+     except json.JSONDecodeError:
+         return {}

REJECT. A malformed config now starts the service with every setting at its default — silently. The failure mode is a production incident where the service is running, healthy by every health check, and behaving as if nobody configured it.

The general rule and it is worth stating as one: a config error must be loud and must happen at startup. Configuration is the one place where crashing is unambiguously the right behaviour, because the alternative is running with the wrong behaviour.

Note also what the agent deleted: the from e chain, which is what makes the traceback point at the actual JSON error. Even in an accepted version, raise ... from e should stay.


D8

  def transfer(self, src, dst, amount):
      self._debit(src, amount)
+     try:
+         self._credit(dst, amount)
+     except Exception:
+         logger.exception("credit failed")

REJECT — this one loses money. If _credit fails, the debit has already happened and is not rolled back. The log line is the only record, and the account is short by amount.

"Logged" is not "handled." A logger.exception in an except block with no re-raise, no compensation, and no return-value change is a swallowed error wearing a costume.

The correct shapes, in order of preference: one transaction covering both; or debit → outbox record → async credit with retry (the outbox pattern); or an explicit compensating credit back to src plus an alert, because compensation can itself fail.


D9

  def get_user(self, uid):
      row = self._db.fetchone("SELECT * FROM users WHERE id = ?", (uid,))
-     if row is None:
-         raise UserNotFound(uid)
-     return User.from_row(row)
+     return User.from_row(row) if row else None

REVISE — the change is defensible, but it is incomplete. Returning None versus raising is a real API design choice: None is fine for a lookup that is expected to miss, an exception is right when a miss is an error.

What makes it a REVISE rather than an ACCEPT: the diff changes a public contract and updates no callers. Every existing get_user(x).name now raises AttributeError: 'NoneType' at a place far from the cause.

The question to ask the agent — and asking it out loud is the scored behaviour — "how many callers assume this raises?" If the answer is "I didn't check", that is the finding.


C. Correctness and Boundaries (D10–D13)

D10

  def value_at(self, key, version):
      versions = self._versions[key]
-     i = bisect_right(versions, version) - 1
-     if i < 0:
-         return None
-     return self._values[key][i]
+     i = bisect_left(versions, version)
+     return self._values[key][i - 1]

REJECT — two bugs in two lines.

  1. bisect_left is the wrong predecessor. bisect_left(v, 57) - 1 gives the last element strictly less than 57; the correct predecessor query is bisect_right(v, 57) - 1, the last element 57. A read at exactly the version a write happened now returns the previous value.
  2. The i < 0 guard is gone. For a version before the key existed, i - 1 == -1, and Python's negative indexing returns the newest value instead of raising. A query for the distant past returns data from the future, with no error.

The second is the more dangerous because it is silent, and it is the single most common bug in this problem.


D11

  def expire(self, now):
-     for key in list(self._entries):
+     for key in self._entries:
          if self._entries[key].expiry <= now:
              del self._entries[key]

REJECT. Mutating a dict while iterating it raises RuntimeError: dictionary changed size during iteration — verified. The list(...) the agent removed was taking a snapshot of the keys, and it was there on purpose.

Why the agent did it: it looks like a wasteful allocation. It is a correctness requirement.

Watch for the same removal on sets and on dict.items(), and note that the failure is at least loud — unlike the equivalent on a list, where deleting during iteration silently skips elements because the indices shift under you. That silent version is the worse one, and this diff is the shape it takes.


D12

  def is_ready(self, progress):
-     return abs(progress - 1.0) < 1e-9
+     return progress == 1.0

REJECT. progress is accumulated in floating point, and floating-point sums do not land on exact values: 0.1 + 0.2 == 0.3 is False (it is 0.30000000000000004). A progress counter summed from tenths reaches 0.9999999999999999 and is_ready is never true — a hang with no error and no log line.

The one case where the agent's version would be right: if progress is assigned 1.0 rather than accumulated. So the review question is where the value comes from, which the diff does not show — and "I need to see the caller" is the correct thing to say rather than guessing.


D13

  def window(self, items, size):
-     for i in range(len(items) - size + 1):
+     for i in range(len(items) - size):
          yield items[i:i + size]

REJECT. Classic off-by-one: the last full window starts at len - size, and range is exclusive, so the bound must be len - size + 1. The agent's version silently drops the final window.

The tell that this class of bug is present: the diff changes a range bound with no accompanying test change. Any diff that adjusts an index expression and does not touch a test is suspicious by construction — either the tests do not cover the boundary, or the change is wrong.

Both are findings, and saying that is better than working the arithmetic in your head.


D. Security (D14–D17)

D14

- rows = db.execute("SELECT * FROM events WHERE tenant = ? AND type = ?", (tenant, typ))
+ rows = db.execute(f"SELECT * FROM events WHERE tenant = '{tenant}' AND type = '{typ}'")

REJECT, immediately, and stop reading the rest of the diff. SQL injection. The agent replaced a parameterized query with string interpolation, probably while adding a clause it found awkward to parameterize.

Any diff that turns a parameterized query into an f-string is an automatic reject regardless of how well-validated the inputs look, because the validation is somewhere else and can change.

And say the second-order thing: if the agent did this once it may have done it elsewhere in the same change. Grep the whole diff for f"SELECT, f"INSERT, .format( near SQL before reviewing anything else.


D15

  def serve_file(self, name):
-     path = (self.root / name).resolve()
-     if not path.is_relative_to(self.root):
-         raise Forbidden(name)
+     path = self.root / name
      return path.read_bytes()

REJECT. Path traversal. name = "../../etc/passwd" now escapes the root, and pathlib's / operator does no containment checking at all.

The removed check was doing two necessary things, and both matter: resolve() collapses .. and follows symlinks, and is_relative_to confirms containment. Checking containment without resolving is also broken — a symlink inside the root pointing outside it passes the string check.

Order matters: resolve, then check. Getting that order right is the actual knowledge being tested here.


D16

  def verify_token(self, provided, expected):
-     return hmac.compare_digest(provided, expected)
+     return provided == expected

REJECT. == on bytes short-circuits at the first differing byte, so comparison time leaks how many leading bytes are correct. An attacker measures and recovers the token byte by byte — 256 guesses per byte instead of 256^n total.

The counter-argument you will hear and how to answer it: "network jitter drowns the signal." It does not — statistical averaging over many samples recovers timing differences well below the jitter, and this has been demonstrated repeatedly against real services.

hmac.compare_digest is constant-time and is a drop-in. There is no cost to being right here, which is what makes accepting this diff indefensible.


D17

  def load_manifest(self, text):
-     return yaml.safe_load(text)
+     return yaml.load(text, Loader=yaml.Loader)

REJECT. yaml.Loader constructs arbitrary Python objects from tags like !!python/object/apply:os.system. Loading an untrusted manifest is remote code execution.

The agent's likely motivation is realsafe_load cannot construct custom types, so a manifest using them fails. The fix is a narrow custom loader, not the full one:

class ManifestLoader(yaml.SafeLoader): pass
ManifestLoader.add_constructor("!duration", _parse_duration)

REVISE with that direction, rather than a flat reject — the agent identified a genuine limitation and reached for the wrong tool, and pointing at the right tool is a better use of the turn.


E. Resource Management (D18–D20)

D18

  def process(self, path):
-     with open(path) as f:
-         data = json.load(f)
+     f = open(path)
+     data = json.load(f)
+     f.close()
      return transform(data)

REJECT. If json.load raises — a malformed file, which is the expected failure — close() never runs and the descriptor leaks. A service processing many malformed files exhausts its descriptor limit and then fails on everything, including healthy requests.

The failure is remote from the cause, which is what makes leaks expensive: the error surfaces as "cannot open socket" in an unrelated code path, hours later.

CPython's refcounting usually closes it at collection, which is what makes this survive testing and fail in production — under load, or on PyPy, or when an exception traceback holds a reference to the frame, the object outlives its scope.


D19

  class Client:
      def __init__(self):
-         self._cache = LRUCache(maxsize=10_000)
+         self._cache = {}

REJECT. An unbounded cache is a memory leak with a delay. It grows until the process is OOM- killed, and the time-to-failure depends on traffic, so it passes every test and every staging soak.

The question to ask, and it is the one that decides the fix: what is the key space? If keys are a bounded enum, a dict is fine and the agent may be right. If keys are user-supplied — request IDs, URLs, tenant strings — it is unbounded and an attacker can fill it deliberately.

Any dict keyed on user input and never deleted from is a memory leak, and phrasing the review that way finds these faster than looking for the word "cache".


D20

  async def query(self, sql):
      conn = await self._pool.acquire()
-     try:
-         return await conn.execute(sql)
-     finally:
-         await self._pool.release(conn)
+     result = await conn.execute(sql)
+     await self._pool.release(conn)
+     return result

REJECT. Connection leak on the exception path. A failing query never releases, the pool drains one connection per failure, and eventually every request blocks forever waiting for acquire.

This is a total outage triggered by a partial failure — the worst shape, because the trigger (some queries failing) is survivable and the consequence (all queries hanging) is not.

Prefer the construct that cannot be got wrong:

async with self._pool.acquire() as conn:
    return await conn.execute(sql)

A try/finally an agent can delete is worse than a context manager it cannot, and saying that — preferring the un-deletable construct — is the more valuable observation than spotting the leak.


F. Performance (D21–D23)

D21

  def drain(self):
      out = []
-     while self._queue:
-         out.append(self._queue.popleft())
+     while self._items:
+         out.append(self._items.pop(0))
      return out

REJECT. list.pop(0) shifts every remaining element, so draining n items is O(n²).

Measured at n = 100,000: deque.popleft 40.7 ns, list.pop(0) 15,895 ns390×, and the gap grows linearly.

This is the most common accidental O(n²) in Python. The agent probably changed the type from deque to list for an unrelated reason and this followed.

And note the cheaper fix if a list is required: out = self._items[:]; self._items.clear() — one copy, O(n) total, no per-item shifting.


D22

  def enrich(self, orders):
      for o in orders:
-         o.user = self._users_by_id[o.user_id]
+         o.user = self._db.fetchone("SELECT * FROM users WHERE id = ?", (o.user_id,))

REJECT. The N+1 query. One thousand orders is one thousand round trips; at 1 ms each that is a one-second request that was microseconds.

The fix is a batch fetch, and the version to ask for is explicit about the failure mode:

ids = {o.user_id for o in orders}
users = self._db.fetch_all("SELECT * FROM users WHERE id = ANY(?)", (list(ids),))
by_id = {u.id: u for u in users}
for o in orders:
    o.user = by_id.get(o.user_id)     # .get, not [] -- a missing user must not KeyError here

N+1 is invisible in a unit test with three rows and obvious in production. Say the scaling, not the principle: "this is one query per order — at our order volume that's a thousand round trips per request."


D23

  def parse_lines(self, lines):
+     pattern = re.compile(r"^(\w+)\s+(\d+)$")
      for line in lines:
-         m = re.match(r"^(\w+)\s+(\d+)$", line)
+         m = pattern.match(line)

ACCEPT — and understand why, because the common explanation is wrong.

The usual claim is that re.match(pattern, s) recompiles each time. It does notre caches compiled patterns. Measured, per call:

ns
re.compile(pat).match(s)248
re.match(pat, s) (module cache)420
precompiled c.match(s)104

So the real gain is 4× over re.match, and it comes from skipping the cache lookup, not a recompile. Note also that re.compile inside the loop would be faster than re.match — which is the opposite of the folk explanation.

Better still: hoist it to a module-level constant so it compiles once per process rather than once per call to parse_lines. Accept the diff and suggest that.


G. Test Quality (D24–D27)

D24

+ def test_retry_on_503(self):
+     client = Client(retry=Retry(max_attempts=3))
+     with mock.patch.object(client._transport, "send") as m:
+         m.return_value = Response(503)
+         client.get("https://example.com")
+     assert m.call_count == 3

REVISE. The assertion is real — it pins the retry count, which is the behaviour under test.

What is missing is what the user experiences: the test never asserts what client.get finally returns or raises after exhausting the attempts. As written it passes even if the client swallows the failure and returns None.

Add with pytest.raises(RetriesExhausted): or assert the returned response is the final 503.

The general question that finds this class: what single-line change to the implementation would make this test fail? Here, quite a few would not.


D25

+ def test_process_handles_empty(self):
+     result = process([])
+     assert result is not None

REJECT. assert result is not None is satisfied by [], {}, 0, "", False, and a MagicMock. It asserts almost nothing, and it will keep passing through any refactor.

A test that cannot fail is worse than no test: it occupies a line in the coverage report and creates false confidence that the empty case is handled.

Ask for the specific expectation: assert process([]) == []. And if the correct behaviour for empty input is genuinely unclear, that is the finding — the agent has written a test to cover a case whose semantics nobody has decided, and deciding it is the work.


D26

+ async def test_worker_processes_queue(self):
+     q = asyncio.Queue()
+     worker = asyncio.create_task(run_worker(q))
+     await q.put(Item(1))
+     await asyncio.sleep(0.1)
+     assert processed == [Item(1)]

REJECT. sleep(0.1) is a race, not a synchronization. It is slow (0.1 s × hundreds of tests), flaky on a loaded CI machine, and it does not actually establish that the work finished — it establishes that 100 ms passed.

Synchronize on the event, not the clock:

await q.put(Item(1))
await q.join()                     # returns when task_done() has been called for every item
assert processed == [Item(1)]
worker.cancel()                    # <-- and clean up, or the task leaks into the next test

The cancel matters too: an un-cancelled worker task survives the test and can interfere with later ones, which is a leading cause of "passes alone, fails in the suite".

A sleep in a test is always a bug, and it is worth stating as an absolute — there is always a condition to wait on instead.


D27

+ def test_report_output(self):
+     out = render_report(SAMPLE)
+     assert out == Path("tests/golden/report.txt").read_text()

REVISE — it depends entirely on how the golden file is maintained, and you must ask.

Legitimate when the output is complex structured text that would be impractical to assert by hand, the golden file is reviewed in the diff when it changes, and regeneration is deliberate.

Worthless when the workflow is "test fails → run with --update → commit". Then the test asserts only that the code ran without raising, and a genuine regression is committed as an expected change.

What to ask the agent: "is report.txt reviewed when it changes, and is regeneration a separate deliberate step?" Plus: the golden file must be in the diff. A snapshot test whose snapshot is generated rather than written is a test nobody has read.


H. Looks Wrong, Is Right (D28–D30)

These three exist because reflexive rejection is a scored failure. An engineer who rejects everything is not exercising judgement, and a round with an agent is about judgement. Each of these violates a rule you have been taught; each is correct here, and being able to say why the rule does not apply is worth more than either accepting or rejecting.

D28

  def build_response(self, chunks):
-     return "".join(chunks)
+     out = ""
+     for c in chunks:
+         out += c
+     return out

ACCEPT — with a caveat you must be able to state precisely.

The rule says += on strings in a loop is O(n²). On CPython it is not, because unicode_concatenate resizes the string in place when its refcount is 1. Measured:

njoin+=ratio
10,0000.28 ms0.40 ms1.47×
100,0002.89 ms4.19 ms1.45×
500,00014.68 ms22.06 ms1.50×

Constant ratio — linear, not quadratic.

But hold one extra reference and the optimization is defeated entirely:

njoin+= with an aliasratio
10,0000.28 ms32.3 ms114×
50,0001.39 ms415.8 ms300×

So the honest verdict is: accept, note that join is still better, and note that the diff is one prev = out away from being 300× slower. That is a much stronger answer than either "reject, that's O(n²)" (wrong on CPython) or "accept, it's fine" (fragile, and wrong on PyPy).

The transferable point: a performance rule that depends on an interpreter optimization should be stated with its precondition.


D29

  def handle(self, event):
+     # NOTE: deliberately not deduplicating here -- the sink is idempotent
+     # on (event_id, version) and dedup would need unbounded state.
      self._sink.write(event)

ACCEPT. This looks like an agent adding a comment instead of doing the work, and it is the opposite: it is a correctly reasoned decision not to build something.

Deduplicating at this layer would require either unbounded state or a window with a stated reordering bound, and the sink already provides the guarantee. Adding a dedupe here would be redundant work and a new memory leak.

Accepting this is the harder call and it is the right one. Reflexively rejecting a diff for "not doing enough" pushes the agent toward building things that should not exist, and an interviewer watching you demand redundant dedup has learned something about your judgement.

One legitimate follow-up: "is the sink's idempotency tested?" If the comment's premise is unverified, that is the work — not the dedup.


D30

  def get_config(self):
-     return self._config.copy()
+     return self._config

ACCEPT — conditionally, and the condition is the whole answer.

Returning an internal mutable directly looks like an encapsulation violation, and normally is. It is correct here if self._config is immutable — a frozen dataclass, a MappingProxyType, or a namedtuple. Then the copy() was pure overhead on a hot path.

So the verdict depends on a line the diff does not show, and the right move is to say so: "Accept if _config is immutable — let me check the declaration. If it's a plain dict, reject, because a caller can now mutate our state."

Naming what you need to see, rather than guessing, is the scored behaviour. In an agentic round you can actually go and look, and doing so out loud — "let me check how _config is declared" — is exactly the verification discipline being measured.


What Agents Get Wrong, Ranked

Across these thirty and the material in WARMUP.md, the defects cluster. Knowing the ranking is what lets you review fast — check the top of this list first and you find most problems in the first two passes.

RankFailureWhy agents do itDiffs
1Removing a guard that looks redundantThe guard's reason is not in the local contextD3, D10, D11, D15, D16, D18, D20
2Tests that cannot failOptimizing for "the test passes"D24, D25, D27
3Unbounded anythingThe bound is a non-functional requirement, rarely statedD1, D19
4Sync primitives in async codeTrained on far more sync Python than asyncD2, D5
5Broadening an exception clauseIt makes the immediate failure go awayD6, D7, D8
6String-building a query or a pathIt is the most natural way to express itD14, D15
7Changing a contract without updating callersThe callers are not in the context windowD9
8Boundary arithmeticGenuinely easy to get wrongD12, D13

Rank 1 is the dominant class and it has a single root cause: a guard's justification lives in the incident that caused it, not in the code. So the review question that finds these fastest is not "is this correct?" but "why was the deleted line there?" — and if you cannot answer, that is sufficient grounds to reject.

Say that rule out loud in the round. It is the most transferable thing in this document.


The Things You Cannot See in a Diff

And this is the section that separates a good agentic round from an excellent one. A diff shows you what changed. It does not show you:

InvisibleHow to check it in the round
Callers of a changed signaturegrep -rn "def get_user|get_user(" — 5 seconds, and D9 depended on it
Whether the deleted guard had a testgit log -S "is_relative_to" --oneline — find the commit that added it
What else the agent touchedgit diff --stat before reading any file. Scope drift is the leading indicator
Whether the new test actually runsRun it. Then break the implementation and confirm it fails
Whether it works on the real dataThe agent's fixture is not your production shape
Whether the change is in the hot pathThe diff has no profile attached

The single highest-value habit: run git diff --stat first, every time. A diff touching four files when you asked for one is a misunderstanding, and you learn that in two seconds instead of after reading three hundred lines.

And the strongest single move in the whole round: after the agent's tests pass, break the implementation deliberately and confirm the test fails.

# revert one line of the fix, run the test
if it still passes -> the test asserts nothing. This is the finding.

That takes thirty seconds, it directly tests the thing agents are worst at, and almost nobody does it. Doing it out loud, once, is worth more than reviewing five more diffs.


References

  • WARMUP.md — the eight-step method, the worked 60 minutes, the takeover boundary, prompting
  • README.md — Track G drills, the scoring rubric, the repo setup
  • ../coding/QUIZBANK.md — the underlying mechanisms: D10 is Q18, D21 is Q7, D3 is Q57, D26 is Q134
  • ../python-internals/QUIZBANK.md — the runtime behaviour behind D4, D5, D28
  • ../take-home/WARMUP.md — the deep-dive round, where you defend code you wrote
  • ../../CHEATSHEET.md#8-agentic-coding — the dense version for the morning of a round
  • CPython Objects/unicodeobject.c, unicode_concatenate — the in-place resize behind D28
  • Python docs, asyncio — Task object: "save a reference to the result of this function" — the D5 GC hazard
  • OWASP Top 10 — the categories behind D14–D17
  • Google. Testing on the Toilet: Change-Detector Tests Considered Harmful — the D27 argument