From 8aad997af7cb8e448cbdac6e37818c53a0b36b39 Mon Sep 17 00:00:00 2001 From: JAYARAM RADHAKRISHNAN Date: Thu, 16 Jul 2026 16:38:23 -0400 Subject: [PATCH 1/2] docs: add adversarial code review guide A reproduce-by-execution method for reviewing high-risk PRs: isolate in a worktree and baseline, fan out independent skeptic sub-agents scoped one per risk surface, verify headline findings by execution, then rank and credit what's correct. Includes the reusable sub-agent prompt template. --- docs/adversarial-review.md | 87 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 docs/adversarial-review.md diff --git a/docs/adversarial-review.md b/docs/adversarial-review.md new file mode 100644 index 00000000..b3f74281 --- /dev/null +++ b/docs/adversarial-review.md @@ -0,0 +1,87 @@ +# Adversarial Code Review + +A method for reviewing pull requests where **every claim carries a reproduction or it doesn't ship**. Optimized for finding real correctness bugs (not style nits) in high-risk changes: security/compliance seams, algorithms, vendored code, async/concurrency, and anything that fails silently. + +## The method + +Five moves, in order. + +### 1. Isolate & baseline +Review in a throwaway git worktree checked out at the PR head, so the working tree is never touched: + +```bash +git fetch pull//head:pr- +git worktree add /tmp/review- pr- +``` + +*Before* judging anything, run the project's lint / type / test commands and record the result. That's your baseline — it lets you separate PR-caused breakage from pre-existing environmental noise (a missing optional dep, deselected markers, a flaky unrelated test). + +### 2. Fan out independent skeptics +Spawn several sub-agents, each scoped to **one** risk surface (core algorithm / integration / tests-and-packaging). They must not see each other's conclusions — independent agreement is signal, not echo. Tell each to **hunt for bugs and reproduce them**, not to summarize the code. Two or three agents is usually right; more just produces overlap. + +### 3. Reproduce by execution, not eyeballing +A finding isn't real until it's been run. "Crashes on `tool_calls: None`" is worth nothing until you've watched it throw. Standalone repros (`uv run python -c "..."`, a scratch test, a one-off script) are the currency of the review. + +### 4. Verify before you report +Re-run the agents' **headline** claims yourself. Agents are confidently wrong sometimes — catching a claim that's false *in the author's favor* matters as much as catching a bug. Only surface what survives your own repro. + +### 5. Rank, separate, and credit +Blockers first, then high / medium / low. Distinguish verified from hypothesized. Say plainly which claimed fixes are genuinely correct so the author doesn't churn on the parts they nailed. A review that only lists faults is a worse review. + +**Through-line: evidence over opinion.** + +## The reusable sub-agent prompt + +Fill in the bracketed parts, one instance per risk surface. + +``` +Adversarial code review. Code is checked out at: +Base branch is `main` (or `upstream/main`); `git diff main...HEAD -- ` +shows only this PR's changes. + +Focus ONLY on: . +(Another reviewer owns — do not duplicate.) + +Context: <1–3 sentences on what the code does and any claims the author makes>. + +If this is a RE-review, here are the previously-reported issues that were +supposedly fixed — verify each is ACTUALLY fixed AND that the fix introduced +no new bug: + + +Your job: find REAL bugs by reasoning AND by executing. Be adversarial. +Specifically probe: + 1. Empty / degenerate / malformed inputs — write and RUN standalone repros + (`cd && uv run python -c "..."`). Try: empty collections, + single elements, None/""/negative/huge values, missing keys, wrong types. + 2. Correctness of the core logic — off-by-one, shape/index bugs, wrong math, + division-by-zero, NaN propagation, silently-swallowed exceptions, + mutable defaults, incorrect normalization. + 3. Fail-open vs fail-closed — if this guards something (auth, PII, money, + deletes), does an unexpected error let bad data THROUGH? State it plainly. + 4. Integration seams — every call site that reaches the risky path; anything + that bypasses the intended choke point; breaking changes to public APIs. + 5. Test quality — are tests real assertions or over-mocked to pass trivially? + Do they use the REAL dependency or a stub that hides the bug? What is the + single highest-value MISSING test? + +Rules: + - Report ONLY findings you verified by reading or executing. No speculation. + - For each finding: exact file:line, what's wrong, a concrete triggering + input, and observed-vs-expected. + - Rank by severity. Distinguish real bugs from cosmetic nits. + - If a claimed fix is genuinely correct, say so plainly. + - If the code is solid, say that — do NOT manufacture findings. + - Be concise. +``` + +Two knobs to turn per PR: +- **Number of agents** = number of genuinely independent risk surfaces (usually 2–3). Overlapping scopes produce echo, not coverage. +- **"Be adversarial" + "don't manufacture findings"** always appear together. The first pushes them to dig; the second stops them inventing severity to look useful. + +## Posting the review + +- Draft first; show the human before posting. +- Structure: a summary body (a **status table crediting what's fixed** is great on re-reviews) plus inline comments anchored to `file:line` at the PR head SHA. +- Inline comments only attach to lines present in the diff. Files added wholesale are fully anchorable; for a change to an *unchanged* line, anchor to the nearest related diff line or put it in the body. +- Choose the verdict deliberately: `REQUEST_CHANGES` for a real correctness drop or several mediums; `COMMENT`/`APPROVE` when only nits remain. From df0728667244fa139d0ea23753e953526958aa82 Mon Sep 17 00:00:00 2001 From: JAYARAM RADHAKRISHNAN Date: Thu, 16 Jul 2026 16:46:46 -0400 Subject: [PATCH 2/2] chore: share /adversarial-review slash command with the team Un-ignore .claude/commands/ so project slash commands are checked into the repo and available to anyone who clones it (personal .claude/settings*.json stay ignored). Adds the /adversarial-review command, which drives the flow in docs/adversarial-review.md. --- .claude/commands/adversarial-review.md | 30 ++++++++++++++++++++++++++ .gitignore | 4 +++- 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 .claude/commands/adversarial-review.md diff --git a/.claude/commands/adversarial-review.md b/.claude/commands/adversarial-review.md new file mode 100644 index 00000000..af2e5dcc --- /dev/null +++ b/.claude/commands/adversarial-review.md @@ -0,0 +1,30 @@ +--- +description: Adversarial, reproduce-by-execution review of a GitHub PR (fan-out sub-agents, verify, draft review) +argument-hint: [repo] [--post] +allowed-tools: Bash, Read, Grep, Glob, Agent, Write +--- + +Run an adversarial code review of pull request **#$1**, following `docs/adversarial-review.md`. + +Method — evidence over opinion; every finding carries a reproduction or it doesn't ship. Do NOT report style nits. + +## Steps + +1. **Fetch & size the PR.** + - `gh pr view $1 --repo --json title,author,state,mergeable,mergeStateStatus,body,additions,deletions,changedFiles,headRefOid,commits` (default REPO to this repo's upstream/origin; a second arg to this command overrides it). + - `gh pr view $1 --repo --json files --jq '.files[]|"\(.additions)+ \(.deletions)- \(.path)"' | sort -rn` to see the shape. + - If the PR body claims specific bugs/fixes, note them — they become verification targets. If this is a **re-review**, fetch your prior review (`gh api repos//pulls/$1/reviews`) so each old finding gets a fixed / still-open / regressed verdict. + +2. **Isolate in a worktree.** Fetch the head into a temp branch and add a worktree under the scratchpad dir; never touch the user's working tree. Clean it up at the end (`git worktree remove --force`, delete the temp branch). + +3. **Baseline BEFORE judging.** Install extras if the PR needs them, then run the project's lint / type / test commands and record results, so PR-caused breakage is distinguishable from environmental noise. Note the base SHA (`upstream/main`) for `git diff main...HEAD`. + +4. **Fan out independent skeptics.** Launch 2–3 sub-agents **in parallel** (one message, multiple Agent calls), each scoped to ONE risk surface (e.g. core algorithm / integration & breaking changes / plugins-config-tests-packaging). Give each the sub-agent prompt template from `docs/adversarial-review.md`, filled in for its surface. They must reproduce findings by execution and not duplicate each other. + +5. **Verify headline claims yourself.** Re-run each agent's most severe finding with your own repro. Only keep what survives. Watch for claims that are wrong *in the author's favor* too (mis-stated breaking changes, "deleted" tests that were merely moved). + +6. **Synthesize & show the user.** Rank blocker → high → medium → low; separate verified from hypothesized; **credit what's genuinely correct** (a status table is ideal on re-reviews). Present the results and STOP — do not post unless `--post` was passed or the user asks. + +7. **Post (only when asked).** Build a review with a summary body + inline comments anchored to `file:line` at the head SHA (inline comments must land on diff lines; otherwise put them in the body). Pick the verdict deliberately (`REQUEST_CHANGES` for a real correctness drop or several mediums; else `COMMENT`/`APPROVE`). Submit as the user's own GitHub account with **no AI attribution**. Verify every inline comment anchored, then report the review URL. + +Extra arguments: `$ARGUMENTS` (e.g. a repo override, or `--post` to post without a second confirmation). diff --git a/.gitignore b/.gitignore index bd26a3ad..1d9ae033 100644 --- a/.gitignore +++ b/.gitignore @@ -10,7 +10,9 @@ evolve_data demo/workdir/.claude/ .bob .bob-sandbox-home/ -.claude +# Ignore local Claude Code state, but share team commands (checked in for everyone) +.claude/* +!.claude/commands/ dist .coverage .evolve