From b20fc8b4e4609e5efc6c0a84f324e3d750238db1 Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Fri, 28 Aug 2026 13:57:34 +0200 Subject: [PATCH 1/2] Document code principles in AGENTS.md [ci skip] --- AGENTS.md | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 1719e182545..32e7ea127a4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,6 +15,67 @@ This project has an explicit policy on AI-generated contributions: @CONTRIBUTING.md +Two rules for agents on top of that policy: + +- **Do not add AI co-authorship trailers to commits.** No `Co-Authored-By:` line for Claude / + Copilot / Cursor / any other tool, and no bot as the commit author. The human opening the PR is + the sole author, takes full responsibility for the contents, and must be able to explain and + defend every line on request. Disclose the tools used and the scope of their assistance in the + PR description instead, as CONTRIBUTING.md requires. +- **Optimize for the reviewer and for long-term maintainability, not for output volume.** Every + line you add is read by a volunteer reviewer and then maintained by humans in perpetuity. See + "Keep changes small when possible" below — oversized diffs are the single most common problem + with agent-assisted PRs here. + +## Keep changes small when possible + +A big diff is a cost, not an accomplishment. Aim for the smallest change that delivers the +user-visible behavior; anything else can be added later, when something actually needs it. + +- **Sketch before you generate.** For anything beyond a local fix, summarize the design in a few + sentences for the human you are working with, and confirm with them that it is the smallest one + that works before writing code. Getting agreement on a design for larger work is worthwhile, but + the human decides whether and how to raise it publicly, and writes it in their own words — as + CONTRIBUTING.md says, do not paste agent-written text into issues, PRs, or comments. Prefer + landing a minimal version and iterating in follow-ups over one large PR. +- **Stop and re-plan at roughly 150 new lines of implementation.** Growth past that — or reaching + for a new module, a new test file, or a new dependency — is a signal that the design is too + elaborate, not that progress is being made. Stop, tell the human, and offer the smaller version. +- **YAGNI: do not build what the feature does not yet need.** No manifests, format versioning or + generations, garbage collection, custom locking, retry logic, JSON side-car metadata, + platform-specific fallbacks, or extension points unless a test in the same PR fails without + them. Unrequested "robustness" is the main way an agent PR reaches +1500. This goes double for + public API: a new parameter, keyword value, or config knob needs a demonstrated end-user need, + not a hypothetical one, because every one of them is documented, tested, and supported forever. + Hard-code the single behavior that is actually wanted and add the option later if someone asks. +- **DRY: reuse or refactor before writing something new.** Grep for machinery that already + exists — `__hash__` and other dunders on the object, helpers in `mne/utils/`, `docdict` entries + for parameter text, existing generic test harnesses, joblib / NumPy / SciPy idioms — and build + on it rather than writing a bespoke implementation. When the code you need is almost right but + not reusable as it stands, factor the shared part out into a private helper (or generalize an + existing private function) and call it from both places, rather than copying it and editing the + copy. Near-duplicate blocks that drift apart are a long-term maintenance cost here. +- **Implement at the highest layer that already exists.** Adding behavior once to a base class + (e.g. `BaseRaw`) is usually both smaller and broader than adding it to three subclasses, and it + covers formats added later for free. +- **Never promote an optional dependency to a required one** as a side effect of a feature, and do + not add a dependency at all without asking first. +- **Extend the existing test harness instead of adding a test module.** A new + `mne/**/tests/test_.py` is a signal to stop and look: most behavior belongs in an + existing test file, and new I/O behavior usually belongs in the generic + `mne/io/tests/test_raw.py::_test_raw_reader`, which runs for every format. A few compact + assertions that run everywhere beat hundreds of lines that run once. +- **Check that a new API spelling does not already mean something else.** A new sentinel or + keyword value (`preload="auto"`, `memmap="auto"`, ...) must not collide with an existing meaning + of the same string on a related argument. + +Worked example of what to avoid: mne-tools/mne-python#14216 opened at +1523/-27, with a 628-line +private module, a 679-line dedicated test file, and an optional dependency promoted to required. +After review it delivered the same feature in ~114 implementation lines and ~150 test lines, with +no manifests, generations, scavenger, or lock file, implemented on `BaseRaw` so it worked for +every reader instead of three, and smoke-tested from `_test_raw_reader`. That second version is +what should have been written first. + ## Common commands From c902d8b90d6f6ec2278a0ae6911c247de16b744e Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Sat, 29 Aug 2026 11:21:05 +0200 Subject: [PATCH 2/2] FIX: Tweak --- AGENTS.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 32e7ea127a4..49fc2920c7a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,11 +60,17 @@ user-visible behavior; anything else can be added later, when something actually covers formats added later for free. - **Never promote an optional dependency to a required one** as a side effect of a feature, and do not add a dependency at all without asking first. -- **Extend the existing test harness instead of adding a test module.** A new - `mne/**/tests/test_.py` is a signal to stop and look: most behavior belongs in an - existing test file, and new I/O behavior usually belongs in the generic +- **Add to an existing test before writing a new one.** In order of preference: extend a test + function that already builds the objects you need — a parametrized one especially, since the new + assertion then runs across every case for free — then add to the existing test module for that + code, and only then write a new test function or file. A new `mne/**/tests/test_.py` is a + signal to stop and look, and new I/O behavior usually belongs in the generic `mne/io/tests/test_raw.py::_test_raw_reader`, which runs for every format. A few compact - assertions that run everywhere beat hundreds of lines that run once. + assertions that run everywhere beat hundreds of lines that run once, and re-created setup is one + of the most common things reviewers ask to have deleted. In mne-tools/mne-python#14248 a 30-line + standalone test became fewer than 10 lines added to the existing parametrized + `test_anonymize_with_io`, which already had the fixture, the save/load round trip, and the + `daysback` parametrization that exposed the bug. - **Check that a new API spelling does not already mean something else.** A new sentinel or keyword value (`preload="auto"`, `memmap="auto"`, ...) must not collide with an existing meaning of the same string on a related argument.