evalbench-review

EvalBench code review

Review a change on four axes, in this order. Verification comes first — an architecturally beautiful change that doesn't run is still broken, and running the code surfaces real defects that reading alone will not.

1. Does it work? Prove it by execution, not by reading.
2. Does it follow EvalBench architecture? Registration wiring, base-class contracts, import style, isolation, concurrency, config-driven behavior, docs.
3. Does it still ship? The image builds, and GKE / Cloud Run / Cloud Build still work. A green local suite says nothing about any of these.
4. Is it a good PR? Reviewable size, single purpose, a description that answers what and why, and code that is clear, simple, concise, maintainable and idiomatic.

Do not skip phase 1 because a diff "looks obviously fine". Do not stop after phase 1 because the tests pass — a change can pass every test and still be wired into the wrong layer, break the container build, or be unreviewable.

Phase 0 — Scope the diff

Establish exactly what changed before reading anything else.

BASE=$(git merge-base HEAD origin/main)
git diff --stat $BASE...HEAD
git diff -M $BASE...HEAD                 # -M so renames don't read as rewrites
git status --short                       # uncommitted work counts as part of the change

For a PR under review: gh pr diff <n> and gh pr view <n>.

Size the change first — it sets how you review everything else

Defect detection degrades as diffs grow, so measure before you read:

git diff --shortstat $BASE...HEAD -- . \
  ':(exclude)uv.lock' ':(exclude)CHANGELOG.md' \
  ':(exclude)evalbench/evalproto/*_pb2*' ':(exclude)datasets/**/*.json'

Generated and vendored files are volume, not review surface — uv.lock alone is 5,000 lines. Judge against the hand-written count:

• Under ~400 lines — review normally.
• Over ~400 lines — report it as a pr-hygiene finding with a concrete split proposal, naming the seams.
• Any size — pace at no more than ~500 lines per hour. If the change is larger than you can review at that rate, say which parts you read carefully and which you skimmed. Never imply uniform depth you didn't apply.