Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 67 additions & 13 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,10 @@ rules. Read the complete governance document before making or reviewing changes:
- INPUT parameter behavior changes must update `docs/parameters.yaml` and
`docs/advanced/input_files/input-main.md`, or the PR must state why no update
is required.
- Report the exact verification performed. Do not claim completion without
fresh test or check output.
- For multi-step refactors (e.g., splitting a large `.cpp` into several
files), build and commit after each step rather than batching all changes
before verification. This keeps the blast radius small when a step
surfaces a missing include or instantiation error.
- Report the exact verification performed, and match the depth of verification
to the risk of the change. `Verification Tiers` below states which evidence
each kind of change owes. Never present an unrun command as if it had passed,
and never silently claim a lower tier than the change belongs to.
- Prefer `std::vector` over raw `new`/`delete` for dynamic arrays; before
converting class members, confirm no external code consumes them as raw
pointers (e.g., `std::vector<bool>` has no `.data()`), and use
Expand All @@ -71,15 +69,70 @@ rules. Read the complete governance document before making or reviewing changes:
- Prefer the repository CMake/CTest flow already used by CI. For focused local
checks, use commands such as `ctest --test-dir build -V -R MODULE_MD` after a
usable build exists.
- The executable is not named `abacus`. CMake names it
`abacus_<level>_<para-or-device>` (`ABACUS_BIN_NAME` in the top-level
`CMakeLists.txt`), where level is `pw`, `basic`, `ml`, `std`, or `max`, and
`install` keeps that name. Locate it the way `.github/workflows/test.yml`
does, and use the resolved path in recorded commands:

```bash
ABACUS_BIN=$(find build -name "abacus_*" -type f -executable | head -1)
```

- For INPUT-related changes, verify both documentation and CLI behavior when an
executable is available: `./build/abacus -h <parameter>` and
`./build/abacus --check-input` from a valid case directory.
- For executable identity, record `./build/abacus --version` or the equivalent
installed `abacus --version` command used during verification.
executable is available: `"$ABACUS_BIN" -h <parameter>` and
`"$ABACUS_BIN" --check-input` from a valid case directory.
- For executable identity, record `"$ABACUS_BIN" --version` together with the
binary name it resolved to.
- Reuse existing Docker and toolchain assets. Do not add a new container,
compiler setup, or calculation-task skill unless the PR explicitly requires
and justifies it.

## Verification Tiers

The verification a change owes depends on what the change can break, not on how
large the diff is. Pick the highest tier that applies, produce its evidence, and
record the exact commands in the PR.

- **Tier 1, static checks only.** Text no compiler reads: documentation,
comments, PR templates, governance files, whitespace or line-ending
normalization. Evidence: the governance check from `Local Commands` below,
plus regenerated `docs/parameters.yaml` and
`docs/advanced/input_files/input-main.md` when those artifacts are in scope.
A local build is not expected, and configuring one to satisfy this tier is
wasted effort.
- **Tier 2, must compile.** Changes that can break the build without changing
any number: include and header-dependency edits, moving code between
translation units, splitting or merging files, renames, mechanical call-site
updates, interface signature changes, `new`/`delete` to `std::vector`
conversions, and `CMakeLists.txt` linkage. Evidence: a build of the affected
targets plus the unit tests of the touched modules, such as
`ctest --test-dir build -R MODULE_MD`. Header-dependency trimming belongs
here even when it only deletes lines, because a removed include can break an
unrelated translation unit that relied on it transitively.
- **Tier 3, must reproduce behavior.** Changes that can move numbers or alter
control flow: algorithms, operators, heterogeneous kernels, eigensolvers,
mixing, occupations, symmetry, INPUT semantics or defaults. Evidence: Tier 2,
plus at least one relevant case from `tests/`, plus the CLI checks above for
INPUT and command-line changes. Say which reference values were compared, not
only that the case ran.

For a multi-step Tier 2 or Tier 3 refactor, such as splitting a large `.cpp`
into several files, build and commit after each step rather than batching every
change before the first build. This keeps the blast radius small when a step
surfaces a missing include or instantiation error.

When the evidence a tier requires cannot be produced locally, say so
specifically: name the tier, name the command that could not be run, and give
the reason, such as no Linux toolchain in the checkout, no configured build
tree, a sandbox restriction, or a missing optional dependency. ABACUS builds on
Linux in practice, so a Windows or otherwise unsupported checkout is a
legitimate reason to stop at static checks and leave compile and runtime
evidence to CI. A disclosed gap is acceptable and reviewable. An undisclosed gap
is not, and neither is provisioning a new toolchain or container to reach a tier
the change never required; reuse the existing Docker and toolchain assets or
defer to CI.

## Local Runtime Testing

- Set `OMP_NUM_THREADS=1` for ABACUS runtime, integration, and MPI tests unless
Expand Down Expand Up @@ -111,7 +164,7 @@ rules. Read the complete governance document before making or reviewing changes:

- Member -> free function: inventory `this->` reads; pass as params (const
for config, ref for mutable state); move only when body is `this`-free;
keep thin wrapper; compile each step.
keep thin wrapper; compile each step when a usable build exists (Tier 2).
- Extract a base-class nested-vector member in three steps (hold + forward,
switch writers, delete legacy) so no commit mixes old-storage writes with
new-storage reads.
Expand All @@ -132,8 +185,9 @@ full mixed-line-ending hook only for intentional repository-wide normalization.

## PR Self-Check

- Confirm the PR body states exact commands run, whether they passed or failed,
and why any expected check could not be run.
- Confirm the PR body names the verification tier reached, states the exact
commands run and whether they passed or failed, and explains why any check
expected at that tier could not be run.
- Keep warning rationales concrete. For example, a header include warning can be
acceptable when the header owns a value member that requires the complete type.
- Keep historical-debt notes separate from new deterministic errors introduced
Expand Down
21 changes: 15 additions & 6 deletions docs/developers_guide/agent_governance.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ AI agents have additional workflow obligations:

- Inspect existing interfaces before using them.
- State uncertainty instead of inventing business rules or APIs.
- Report exact verification results and any checks that could not be run.
- Report exact verification results and any checks that could not be run. Match
verification depth to the risk of the change using the tiers in `AGENTS.md`,
and name the tier reached rather than implying an unrun command passed.

## Rule Grading Matrix

Expand Down Expand Up @@ -165,16 +167,23 @@ changed and the PR explains why the new reference is correct.
## CLI Verification

When a usable ABACUS executable is present, INPUT and command-line changes
should include the relevant CLI checks in the PR verification record:
should include the relevant CLI checks in the PR verification record.

The executable is not named `abacus`. CMake names it
`abacus_<level>_<para-or-device>` through `ABACUS_BIN_NAME`, and `install`
preserves that name, so resolve the path first the way
`.github/workflows/test.yml` does:

```bash
./build/abacus --version
./build/abacus -h <parameter>
./build/abacus --check-input
ABACUS_BIN=$(find build -name "abacus_*" -type f -executable | head -1)
"$ABACUS_BIN" --version
"$ABACUS_BIN" -h <parameter>
"$ABACUS_BIN" --check-input
```

Run `--check-input` from a directory containing a valid `INPUT` case. If no
local executable or valid case is available, state that explicitly in the PR.
local executable or valid case is available, state that explicitly in the PR
rather than configuring a new build solely to produce the output.

## AI PR Review Integration

Expand Down
Loading