docs: relocate the common-pitfalls material into the user docs

The removed .claude/rules/common-pitfalls.md held traps whose failure modes are silent. Its content is redistributed to the docs page that owns each subject rather than to a new page: indexing/ghost-cell and parameter-plumbing traps into contributing.md's Common Pitfalls section, compiler and backend traps into a new Silent-Failure Traps section in gpuParallelization.md, and test-selection traps into testing.md. Material already covered in those pages (Riemann j/j+1 indexing, the add-a-parameter procedure and its still-manual list, the AMD case-opt bound pattern) was dropped rather than duplicated. The testing.md --only bullet described substring matching and is corrected to whole-element matching.

Written with assistance from Claude Code.

Author: sbryngelson

docs/documentation/contributing.md

@@ -175,6 +175,9 @@ Both human reviewers and AI code reviewers reference this section.
 
 - MFC uses **non-unity lower bounds** (e.g., `idwbuff(1)%%beg:idwbuff(1)%%end` with negative ghost-cell indices). Always verify loop bounds match array declarations.
 - **Riemann solver indexing:** Left states at `j`, right states at `j+1`. Off-by-one here corrupts fluxes.
+- **Grid extents:** `m`, `n`, `p` are cell counts in x, y, z (1D sets `n = p = 0`, 2D sets `p = 0`). The interior is `0:m`, the ghost region `-buff_size:m+buff_size`, and cell boundaries run `x_cb(-1-buff_size:m+buff_size)`. Bounds are carried in `idwint(1:3)` (interior) and `idwbuff(1:3)` (with ghosts).
+- **`buff_size` is not a single formula.** It is set per reconstruction scheme in `s_configure_coordinate_bounds` (`src/common/m_helper_basic.fpp`) and floored higher for Lagrange bubbles and immersed boundaries. Read that routine rather than assuming a value.
+- **Never hard-code an equation index.** They live in the `eqn_idx` struct (`eqn_idx_info` in `src/common/m_derived_types.fpp`, populated by `s_initialize_eqn_idx` in `src/common/m_global_parameters_common.fpp`): `%%cont`, `%%mom`, `%%E`, `%%adv`, plus the optional ranges `%%bub`, `%%stress`, `%%species`, and `%%B`. Index positions depend on `model_eqns` and on which features are enabled, so changing either moves every index.
 
 ### Precision and Type Safety
 
@@ -212,6 +215,14 @@ Both human reviewers and AI code reviewers reference this section.
 - CLI schema in `toolchain/mfc/cli/commands.py` must match argument parsing.
 - Check subprocess calls for shell injection risks and missing error handling.
 
+### Parameter Plumbing
+
+- **Derived-type parameters are not auto-broadcast.** `generated_bcast.fpp` covers namelist *scalars* only. Each derived type (`chem_params`, `lag_params`, `rburn`) needs a hand-written `_emit_<name>` in `toolchain/mfc/params/generators/fortran_gen.py` plus its call site in that generator's simulation branch, and, if it is read on device, an explicit `$:GPU_UPDATE(device='[name]')` in both the target's `m_global_parameters.fpp` and `src/simulation/m_start_up.fpp` — `GPU_DECLARE` alone does not make it device-resident. Regrouping existing scalars into a derived type silently drops their broadcast, leaving every non-root rank holding the `dflt_real` sentinel. Single-rank golden files cannot catch this, so pair such a change with a `ppn=2` test and confirm it fails without the emitter.
+- **A `patch_ib` member that immersed-boundary ghost-point code reads must also be set in `s_add_cloud_particle`** (`src/simulation/m_particle_cloud.fpp`). `particle_cloud_ibs` is allocated without default initialization, and `s_reduce_ib_patch_array` copies the whole struct into `patch_ib`, overwriting the defaults assigned in `s_assign_default_values_to_user_inputs`. Anything left unset reaches the solver as uninitialized memory, and only where the allocation is not already zero-filled. A platform-only NaN is the signature of this class: a garbage `v_blow` once failed an AMD lane with `ICFL is NaN` while every NVIDIA lane and all local runs passed.
+- **Runtime checks go where they run.** Shared constraints belong in `src/common/m_checker_common.fpp`, simulation-only ones in `src/simulation/m_checker.fpp`, and pre- and post-process ones in their own `m_checker.fpp`. Those two `s_check_inputs` are currently empty; that is still the correct home for their checks, not `m_checker_common`.
+- **Analytic initial conditions are compiled into the binary** and their expressions are AST-validated at case load, so syntax errors and unknown variables surface immediately and by name. Each IC variable maps to an `eqn_idx` expression in `QPVF_IDX_VARS` (`toolchain/mfc/case.py`); adding a patch-settable conserved variable means updating that map and the Fortran `eqn_idx` builder together, because a mismatch is a silent wrong index.
+- **Under `--case-optimization` the baked-in constants are dropped from the namelist**, so changing one requires a rebuild rather than a case-file edit.
+
 ### Compiler Portability
 
 - Any compiler-specific code (`#ifdef __INTEL_COMPILER` etc.) must have fallbacks for all four supported compilers.

docs/documentation/gpuParallelization.md

@@ -864,6 +864,55 @@ while the host still registers it. The first launch aborts with
 followed by a segmentation fault. Never place a GPU kernel inside a `block` construct;
 hoist it into its own (module) subroutine with the locals passed as arguments.
 
+## Silent-Failure Traps
+
+Every entry here was measured. They share a failure mode: the build stays green and the
+answer is wrong, or one backend diverges from all the others.
+
+- **Do not wrap `GPU_LOOP` in `GPU_PARALLEL` for spatial loops.** `GPU_LOOP` emits empty
+  directives on Cray and AMD, so the loop runs serially with no diagnostic. Spatial loops
+  always use `GPU_PARALLEL_LOOP` / `END_GPU_PARALLEL_LOOP`.
+- **An array whose bound is a device global** (`dimension(num_fluids)`,
+  `dimension(num_species)`) may be passed to a device routine from a parallel-loop body,
+  but **not from inside another `GPU_ROUTINE(parallelism='[seq]')`**. Cray OpenACC rejects
+  the second form with `ftn-7066 ... Global in accelerator routine without declare`, and
+  reports it at whatever line it gave up on: remove one trigger and the message walks
+  forward to the next call, so the reported line is not the cause. Only the plain lanes
+  fail, since `--case-optimization` turns those bounds into `parameter`s — a green case-opt
+  lane beside a failing plain one is the signature. Form such a call in the loop body and
+  pass scalars deeper. Neither `cray_inline`, nor a `num_fluids_max` bound, nor dropping
+  optional dummies avoids it; all three were tried.
+- **A device routine containing any `GPU_LOOP` must be called with scalars, never with an
+  array element.** On Cray OpenACC 19.0.0 through 21.0.2 at `-O2` (`-O0` and `-O1` are
+  correct, OpenMP offload is unaffected) the element is misaddressed: an `intent(in)`
+  element reads as garbage and an `intent(out)` element is never written. Every `routine`
+  level is affected, including a conforming `loop vector` inside `routine vector`. Either
+  ingredient alone is fine, which is why a call like `s_compute_pressure(q%%sf(j,k,l), ...)`
+  into a loop-free helper works. Copy elements into locals before the call and receive into
+  a local. Do not instead delete the `seq` directives: they are the idiom every device
+  routine here uses. See [#1815](https://github.com/MFlowCode/MFC/issues/1815).
+- **Call `m_thermochem` species routines from the kernel, not from inside a
+  `GPU_ROUTINE`.** Calling `get_species_*` from within a device routine gives Cray OpenMP a
+  runtime `Memory access fault by GPU node-N` on the first step while every other backend
+  runs. The build is clean and only a case that reaches the path shows it. Evaluate them at
+  the call site and pass the arrays in.
+- **nvfortran 23.11 and 24.1 segfault** (`fort2 TERMINATED by signal 11`) on a caller that
+  passes a `parameter` array from `m_thermochem`, such as `molecular_weights`, into a
+  declare-target routine. Read such arrays directly in the kernel, or pass a plain local
+  computed from them.
+- **The `USING_AMD` fypp guards are load-bearing, not a stale workaround.** They swap a
+  device-global array bound for a literal in `src/common/include/shared_parallel_macros.fpp`
+  and its 86 use sites. Setting `USING_AMD = False` and rebuilding amdflang `--gpu mp`
+  without case optimization compiles completely clean, then produces NaNs in CBC, the
+  `wave_speeds=2` Riemann path, immersed boundaries, surface tension, QBMM and viscous
+  cases, and MHD HLLD, while both Lagrange bubble cases complete with out-of-tolerance
+  answers. A compile-only check returns green, so any attempt to remove these must run the
+  tests rather than just build.
+- `@:ACC_SETUP_VFs` and `@:ACC_SETUP_SFs` compile only under Cray. Around MPI, use
+  `GPU_UPDATE(host=...)` before a send and `GPU_UPDATE(device=...)` after a receive.
+
+------------------------------------------------------------------------------------------
+
 ## Compiler Documentation
 
 - [Cray & OpenMP Docs](https://cpe.ext.hpe.com/docs/24.11/cce/man7/intro_openmp.7.html#environment-variables)

docs/documentation/testing.md

@@ -13,7 +13,7 @@ A test is considered passing when our error tolerances are met in order to maint
 `./mfc.sh test` has the following unique options:
 - `-l` outputs the full list of tests
 - `--from` (`-f)` and `--to` (`t`) restrict testing to a range of contiguous slugs
-- `--only` (`-o`) restricts testing to a non-contiguous range of tests based on if their trace contains a certain feature
+- `--only` (`-o`) restricts testing to a non-contiguous range of tests whose trace contains a given whole trace element (see [Selection and Execution Pitfalls](#selection-and-execution-pitfalls) for the exact matching rules)
 - `--test-all` (`a`) test post process and ensure the Silo database files are correct
 - `--percent` (`%`) to specify a percentage of the test suite to select at random and test
 - `--max-attempts` (`-m`) the maximum number of attempts to make on a test before considering it failed
@@ -92,6 +92,38 @@ If a trace is empty (that is, the empty string `""`), it will not appear in the
 
 Finally, the case is appended to the `cases` list, which will be returned by the `list_cases` function.
 
+### Selection and Execution Pitfalls
+
+Each of these fails quietly rather than loudly.
+
+- **`--only` matches whole trace elements, not substrings**, and it ANDs labels while ORing
+  UUIDs (`_filter_only` in `toolchain/mfc/test/test.py`). `--only bubbles` matches nothing,
+  because the trace element is `Bubbles`; `--only low_Mach=1 low_Mach=2` asks for cases
+  carrying both labels at once and also matches nothing. An empty selection then exits
+  **143**, which reads like an external kill rather than an empty filter. Pass UUIDs when
+  you want the union of several groups.
+- **Sibling `define_case_d` calls at the same stack level are never combined.** Two switches
+  that only matter together therefore get no effective coverage unless one is pushed onto
+  the stack and the other defined beneath it — `avg_state=1`, for instance, is only read
+  when `wave_speeds=2`. Check reachability before trusting that a flag is tested.
+- **`--no-build` silently runs whatever binary is already on disk**, including one built for
+  a different configuration. Chemistry has its own configuration that a plain `./mfc.sh
+  build` never produces, so a `--no-build` run can report failures from stale binaries and
+  hide real compile breaks. Run chemistry-touching sets without it.
+- **Identify the newest binary by the binary's own mtime**, not by its install directory's:
+  a stale configuration's directory can be newer than a fresh build's.
+- **The pre-commit hook lives in the main repository's `.git/hooks/`**, and git exports
+  `GIT_DIR` there during a commit, so from a worktree the toolchain lint enumerates the
+  other checkout and fails. Run `./mfc.sh precheck` by hand and commit with `--no-verify`.
+- **`/tmp` is node-local.** Scratch does not survive a compute-node change, and its absence
+  is silence rather than an error. Keep patches and resource baselines on a shared
+  filesystem.
+- **An unexplained golden-file difference is a bug report, not noise to be regenerated
+  away.** Regenerate only the affected tests.
+
+Tests are generated programmatically in `toolchain/mfc/test/cases.py`; a test's UUID is the
+CRC32 of its trace string, and `./mfc.sh test -l` lists every one.
+
 ### Testing Post Process
 
 To test the post-processing code, append the `-a` or `--test-all` option: