Guard the rapidfuzz-dependent assertion in test_validate

[sbryngelson]

test_family_attr_typo_gives_targeted_error fails rather than skips when rapidfuzz is not importable.

Cause

The test feeds patch_ib(1)%geometri to check_unknown_params and asserts that geometry appears in the resulting "Valid attributes: ..." list. _family_attr_error (toolchain/mfc/params/validate.py:78-90) orders candidates by similarity and then truncates at 8 entries:

similar = suggest_similar(attr, valid, max_suggestions=len(valid))
ordered = similar + [v for v in valid if v not in similar] if similar else valid
if len(ordered) > 8:
    shown = ", ".join(ordered[:8]) + f", ... ({len(ordered)} total)"

suggest.py degrades gracefully when the import fails — RAPIDFUZZ_AVAILABLE = False and suggest_similar returns [] — so the ordering falls back to plain alphabetical. patch_ib now has 26 attributes and geometry sits past the 8-entry cut behind airfoil_id, angles(1..3), angular_vel(1..3), burn_rate_exp, so the assertion fails.

rapidfuzz is a declared dependency in toolchain/pyproject.toml, so a properly provisioned environment always has it and CI is unaffected. But suggest.py supports running without it, and the two neighbouring tests in the same class that depend on fuzzy matching already carry @unittest.skipUnless(RAPIDFUZZ_AVAILABLE, "rapidfuzz not installed"). This one was missed, so a partial environment gets a failure instead of a skip.

Change

Split the similarity-ordering assertion into its own guarded test. The rapidfuzz-independent assertions — one error, "Valid attributes" present, no "Did you mean" — stay unguarded so the targeted-error path keeps its coverage either way.

Verification

without rapidfuzz:  22 passed, 3 skipped
with rapidfuzz:     25 passed
ruff check / ruff format --check:  clean

---

Handover notes

Branch and commit

repo    MFlowCode/MFC
branch  fix/test-validate-rapidfuzz-guard   (base: master)
  579b4a2  Guard the rapidfuzz-dependent assertion in test_family_attr_typo_gives_targeted_error

Single commit, one file: toolchain/mfc/params_tests/test_validate.py. No Fortran, no goldens.

Environment

cd toolchain
python -m pytest mfc/params_tests/test_validate.py -q

No MFC build needed. To reproduce the original failure, run in an environment without rapidfuzz installed:

without rapidfuzz:  22 passed, 3 skipped
with rapidfuzz:     25 passed

ruff check / ruff format --check clean (ruff==0.6.5, config in .ruff.toml, line-length 200).

Scope note

rapidfuzz is a declared dependency in toolchain/pyproject.toml, so a properly provisioned environment always has it and CI was never affected. What is real is narrower: toolchain/mfc/params/suggest.py deliberately supports running without it (RAPIDFUZZ_AVAILABLE = False, suggest_similar returns []), and two neighbouring tests in the same class already carry @unittest.skipUnless(RAPIDFUZZ_AVAILABLE, ...). This one did not, so a partial environment got a failure instead of a skip.

An earlier draft of this description overstated it as an "optional extra"; corrected.

Why the assertion depends on rapidfuzz

_family_attr_error in toolchain/mfc/params/validate.py orders candidates by similarity and then truncates at 8:

similar = suggest_similar(attr, valid, max_suggestions=len(valid))
ordered = similar + [v for v in valid if v not in similar] if similar else valid
if len(ordered) > 8:
    shown = ", ".join(ordered[:8]) + f", ... ({len(ordered)} total)"

Without rapidfuzz the ordering falls back to plain alphabetical. patch_ib now has 26 attributes and geometry sits past the cut behind airfoil_id, angles(1..3), angular_vel(1..3), burn_rate_exp.

The fix splits the similarity-ordering assertion into its own guarded test and leaves the rapidfuzz-independent assertions (one error, "Valid attributes" present, no "Did you mean") running unconditionally, so the targeted-error path keeps coverage either way.

CI note

This PR saw two rounds of Frontier failures that were not caused by it — a one-file Python test edit cannot affect Fortran GPU builds. Both rounds cleared on rerun. The first round died at a uniform ~15 minutes across heterogeneous jobs (AMD/CCE x cpu/gpu-omp/gpu-acc) while configured timeouts are 120/480 min, and other PRs' Frontier jobs succeeded an hour later taking 40-77 min on the same machine. If it recurs, rerun before investigating.

Note that gh run rerun <id> --failed refuses while the workflow is still running; the three residual failures on the second round needed a later retry.

---

Purpose

Housekeeping, unrelated to the EOS series. toolchain/mfc/params/suggest.py deliberately supports running without rapidfuzz; one test in test_validate.py did not honour that contract while its two siblings did. Restores the file's own convention so a partial environment gets a skip rather than a failure.

Smallest and lowest-risk PR of the current set — one test file, no Fortran, no goldens.

---

Working conventions and hazards (shared across this series)

Collected from the work that produced #1705, #1709, #1712, #1713, #1714, #1716. Every one of these cost real time or produced a wrong result before being caught.

Testing

A regression test that cannot fail is worse than no test. Always verify the negative: revert the fix, rebuild, confirm the case fails, restore. Two ways this silently broke here:

• git stash push -- <file> has nothing to stash once the fix is committed, so the "reverted" run tests the fixed binary and reports a pass identical to a real one. Use git checkout master -- <file>, rebuild, test, then git checkout HEAD -- <file>.
• Picking the wrong case configuration. A probe case at model_eqns = 3 cannot detect a sound-speed defect, because the six-equation branch of s_compute_speed_of_sound touches neither H nor qv.

The golden packer discards data. toolchain/mfc/packer/pack.py treated every .dat under D/ as <x> [<y> <z>] <value> and kept only the last column of each row. Probe and integral output are multi-column time series, so most columns were never compared (#1711, fixed in #1712). Before asserting that a golden covers something, check it is actually in golden.txt.

Case labels are load-bearing. The golden UUID is crc32(sha1(str(trace))) — the label chain determines the directory name. Renaming a label renames the golden. Avoid ! in labels (history expansion in interactive bash).

Local suite runs are flaky at high -j. Non-reproducible failures appeared on several unrelated branches at -j 12–16 (chemistry cases, probe cases) that passed individually and in clean reruns. Re-run before investigating.

Removing parameters or features

Deregistering a parameter breaks things that are not the source tree. Removing pref/rhoref from the registry broke the entire suite because BASE_CFG in toolchain/mfc/test/case.py set them for every case. Also check fp_stability.py, params_tests/mutation_tests.py, and lint fixtures that use real parameter names as examples.

Grep the generated artifacts, not just the sources. A stale TYPED_DECLS entry naming a deleted type survived removal and did not break the build only because the parameter had also left every target's namelist vars, so it was never emitted. Check generated_decls.fpp, generated_constants.fpp, SIM_GPU_DECL_VARS, and the MPI broadcast generators.

Dead-local tell: after removing a block, a local with exactly one remaining occurrence in its file is almost certainly its own declaration. Two occurrences often means declaration plus a private() entry.

Fortran is case-insensitive. A local pRef shadowed the module global pref in the hardcoded-IC files; the read site was spelled pref and looked like a reference to the global. It is not. Confirm scope before concluding a global is live.

GPU

A CPU test run cannot catch a missing private(). It is a silent device race. Audit by hand or by script when adding per-cell state.

Do not match GPU_PARALLEL_LOOP naively — END_GPU_PARALLEL_LOOP contains the same substring and will register as a loop start, producing false positives. Exclude it explicitly.

Derived-type components cannot have runtime extents. dimension(num_fluids) in a type fails to compile outside case-optimized builds, where num_fluids is a parameter.

Benchmarking

Run ./mfc.sh bench in the foreground on an idle machine. Running it in the background while a pre-commit precheck ran at -j 12 produced a bogus +50% regression on a case that executes none of the changed code. Baseline noise here is ±3.5%; run the baseline twice before trusting any delta, and sanity-check that the regressing cases actually execute the modified code.

GitHub mechanics

• --force-with-lease needs an explicit SHA (--force-with-lease=<branch>:<sha>) when the ref has not been fetched in the current clone; the bare form fails with "stale info".
• gh run view --log-failed can miss the real output entirely. On the Frontier jobs the failing step carried only a non-zero exit while the actual test output lived in a separate Print Logs step that succeeded. Fetch the full log.
• gh run rerun <id> --failed refuses while the workflow is still running; retry later.
• Frontier jobs fail spuriously with some regularity — uniform ~15-minute deaths across heterogeneous jobs, and shared-Lustre install races (file INSTALL cannot set modification time ... No such file or directory, exit 143). Check for a real error before assuming a code fault.

[Copilot]

Pull request overview

This PR fixes a unit-test robustness issue in the Python parameter-validation test suite by ensuring an assertion that depends on the optional rapidfuzz dependency is only evaluated when rapidfuzz is available, while keeping the rapidfuzz-independent coverage in place.

Changes:

• Removes the unguarded assertion that "geometry" appears in the truncated “Valid attributes …” list for a family-attribute typo.
• Adds a new @unittest.skipUnless(RAPIDFUZZ_AVAILABLE, ...) test that asserts "geometry" is present when similarity ordering (via rapidfuzz) is available.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.77%. Comparing base (8dfe8c7) to head (579b4a2).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1705   +/-   ##
=======================================
  Coverage   60.77%   60.77%           
=======================================
  Files          83       83           
  Lines       20872    20872           
  Branches     3101     3101           
=======================================
  Hits        12685    12685           
  Misses       6121     6121           
  Partials     2066     2066           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.