research-code-review

Research code-review gate

A correctness-and-judgment gate that runs after any code is written or changed in Phases 2–3, and when reviewing a PR. It has two halves: the 4-step design judgment (this skill's core) and test discipline. Mechanical bug-hunting is delegated to the built-in /code-review skill so this one stays focused on judgment.

The 4-step review (apply in order, every time)

1. What is the change trying to solve? State the goal in one sentence. If you can't, the change is unclear — stop and clarify before reviewing further. Tie it back to the research question or the experiment it serves.
2. Conflict & pattern-fit. Does it conflict with the existing design/decisions? Does it follow the surrounding code's patterns, naming, and conventions (match the repo's house style, not a new personal one)? Flag silent contradictions with earlier choices.
3. Tradeoffs — benefit vs shortcoming. What does it buy, and at what cost (complexity, speed, memory, coupling, statistical validity)? Is the tradeoff worth it for a research artifact, or is it premature optimization / over-engineering? Name the shortcoming explicitly even when recommending the change.
4. Legibility & validity. Is it readable and correct? For research code specifically: does it preserve experimental validity — no data leakage, correct train/val/test temporal split, fixed/recorded seeds, metrics computed as claimed, results reproducible from the committed code?

Write the review as those four points, not a vibe. It's legitimate to conclude "no change needed" — don't manufacture findings (see research-reflection on avoiding tilted, change-for-its-own-sake behavior).

Extra checks for statistical machinery

• Guarantee ≠ power. Code implementing a statistical guarantee (error control, calibration, coverage) can be theoretically valid yet practically broken by a tuning choice — the guarantee still "holds" while the procedure alarms constantly or never. Verify empirical behavior at realistic problem scale (false-alarm rate, power/delay, on inputs matching real SNR), not just the math↔code map. A validity test suite should include an empirical check of the guarantee's usefulness, not only its correctness.
• Guarantee ≠ usability for multi-stream / rank-threshold rules. A multiplicity procedure (BH-style rank thresholds, family-wise combinations) can carry a correct bound and still be unusable when the per-stream statistic has a null plateau that sits above the lower-rank thresholds — an all-healthy library then gets declared en bloc while the (loose) bound is satisfied. Before adopting any combined rule, simulate it on an all-null population at realistic scale and pin the failure mode as a regression test if you reject it; the per-stream rule with a union bound is often the honest default.
• Every null cell against its nominal bound. A spike or validity verdict must tabulate each null-hypothesis condition (every dependence / noise / regime variant) against the guarantee's nominal level — not summarise "holds empirically." Any cell above the bound is a validity violation that becomes a prioritized open ADR before any downstream claim builds on the machinery; stored results routinely contain a violation the verdict prose never surfaced.