From d641e3f9849f82801ea9d7ea1be33cb1c82cfb18 Mon Sep 17 00:00:00 2001 From: Jeremiah Lowin <153965+jlowin@users.noreply.github.com> Date: Fri, 27 Mar 2026 10:10:58 -0400 Subject: [PATCH] add "fix causes, not symptoms" to review and contributor guidelines --- CLAUDE.md | 8 ++++++++ CONTRIBUTING.md | 1 + 2 files changed, 9 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index a85214784..0fd0608ef 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -101,6 +101,14 @@ The handwritten notes are prepended above the auto-generated changelog and are t - Minor fixes: keep body short and concise - No "test plan" sections or testing summaries +### Code Review Guidelines + +- **Fix causes, not symptoms.** When a PR works around a problem instead of addressing why it occurs, that's a red flag. A side-channel that compensates for a missing step adds permanent complexity. If the fix doesn't change the code path where the bug actually happens, ask why not. +- Focus on API design and naming clarity +- Identify confusing patterns (e.g., parameter values that contradict defaults) or non-idiomatic code (mutable defaults, etc.). Contributed code will need to be maintained indefinitely, and by someone other than the author (unless the author is a maintainer). +- Suggest specific improvements, not generic "add more tests" comments +- Think about API ergonomics from a user perspective + ### Code Standards - Python ≥ 3.10 with full type annotations diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e948f0c0d..dcbd6a60c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -36,6 +36,7 @@ If you do open a PR: - **Keep it focused.** One logical change per PR. Don't bundle unrelated fixes or refactors. - **Match existing patterns.** Follow the code style, type annotation conventions, and test patterns you see in the codebase. Run `uv run prek run --all-files` before submitting. - **Write tests.** Bug fixes should include a test that fails without the fix. Enhancements should include tests for the new behavior. +- **Fix the cause, not the symptom.** If the bug is that a code path skips a step, the fix should make it stop skipping that step — not add compensation elsewhere. Workaround-style fixes will be sent back for revision. - **Don't submit generated boilerplate.** We review every line. PRs that read like unedited LLM output — verbose descriptions, speculative changes, shotgun-style fixes — will be closed. ## What we'll close without review