From e4a575f9700e30f978910d385daa8c9678525b72 Mon Sep 17 00:00:00 2001 From: isayev Date: Mon, 3 Aug 2026 19:44:03 -0400 Subject: [PATCH] docs: close M53 -- the last two entries are wrong, not unresolved No code change. Both remaining dead-code claims were checked against source and neither holds. ASE/thermo's `model_name` parameter is read: Calculator stores it and passes it to to_model_species at thermo.py:536. It was genuinely unused when the audit was written; the C3/C4 species-conversion work made it live. exceptions.py's "4 classes never raised" is wrong, and the shape of the claim is why. Only ModelError is never raised directly, and that is deliberate: ModelLoadError and NumericalError subclass it, and cli/errors.py maps ModelError to exit code 5 precisely so both subclasses inherit that code. Deleting it would break the exit-code scheme this release documents. "Never raised" is not "unused" for a base class, and an audit that greps for `raise X` cannot tell the two apart. Final tally for M53: of 13 entries, 4 were real. Three had already been done, six were wrong on re-check, and the four real ones were removed in #135 and #136. Nothing further to delete. The lesson worth keeping is that a dead-code finding decays faster than any other kind. The same work that fixes defects revives symbols the audit recorded as dead -- mol2atoms, STANDARD_PRESSURE, FailedMolecule, print_failures and now model_name were all dead when written up and are all live now. A mechanical sweep of this list would have deleted five live symbols, two of which vib_hessian depends on. Both corrections are also written inline in the audit manifest, so neither can be re-derived from its finding ID. --- .../review-2026-07-30-package-audit.md | 4 +-- .../follow-ups-after-4.0.0-remediation.md | 28 +++++++++++++++++-- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/.claude/review-manifests/review-2026-07-30-package-audit.md b/.claude/review-manifests/review-2026-07-30-package-audit.md index 0d5e9f3..b8302ec 100644 --- a/.claude/review-manifests/review-2026-07-30-package-audit.md +++ b/.claude/review-manifests/review-2026-07-30-package-audit.md @@ -536,10 +536,10 @@ All verified unreferenced in `src/`: | `cli/progress.py:25-39`, `70-92`, `142-163` | `create_progress`, `update`, `IsomerProgressCallback` | test-only; `best_energy` never assigned, so `make_panel:136-137` is unreachable | | `utils/file_ops.py:352-366` | `housekeeping_helper` | test-only | | `ASE/thermo.py:197-209` | `mol2atoms` | dead, while its body is inlined at `:225-228` and `:443-446` | -| `exceptions.py:41,72,81,89` | 4 classes | never raised; see M29 | +| `exceptions.py:41,72,81,89` | 4 classes | ~~never raised~~ **WRONG as of 2026-08-03**: only `ModelError` is never raised directly, and deliberately -- `ModelLoadError`/`NumericalError` subclass it and `cli/errors.py` maps it to exit code 5 so both inherit that code. "Never raised" != "unused" for a base class | | `constants.py:10,11,38,46,79` | 5 constants | `SUPPORTED_MODELS` is the very whitelist M50 shows written four times; `STANDARD_PRESSURE` unused while `thermo.py:308-309` hardcodes `101325`; `check_connectivity` hardcodes `1.25`/`1.1` at `chemistry.py:331,336` instead of the two constants named for them | | `model_wrapper.py:60-85`, `120-174` | legacy `name` API + `_legacy_forward` (55 lines) | says "removed in Auto3D v2.0"; package is **3.5.0** | -| `ASE/thermo.py:159` | `model_name` param | accepted, never read, yet `calc_thermo:459` passes it explicitly | +| `ASE/thermo.py:159` | `model_name` param | ~~accepted, never read~~ **WRONG as of 2026-08-03**: `Calculator` reads `self.model_name` and passes it to `to_model_species`; the C3/C4 species-conversion work made it live | `encode_smiles`/`decode_smiles`/`housekeeping_helper`/`filter_unique` are in `utils/__init__.py` `__all__`, and the four exceptions are in `docs/source/api.rst:86-92` — removals are API-breaking and need a deprecation cycle. diff --git a/docs/superpowers/follow-ups-after-4.0.0-remediation.md b/docs/superpowers/follow-ups-after-4.0.0-remediation.md index a164540..a951d97 100644 --- a/docs/superpowers/follow-ups-after-4.0.0-remediation.md +++ b/docs/superpowers/follow-ups-after-4.0.0-remediation.md @@ -121,13 +121,35 @@ present and dead: | **already deleted** by earlier phases | `pad_molecular_batch`, `cli/progress` `create_progress`, `IsomerProgressCallback` | | **not dead — M53 wrong** | `utils/stereo_check` (6 live uses), `cli/results` `FailedMolecule` + `print_failures` (live from `run.py` since the C6/C7 reconciliation work — the finding's "run.py admits failures is always []" no longer holds), `ASE/thermo` `mol2atoms`, `STANDARD_PRESSURE`, `isomers/parallel_embed` | | **genuinely dead — deleted** | `utils_file.py` (whole module), `count_from_output`, `encode_smiles`, `decode_smiles`, `housekeeping_helper`, and 3 constants (`BOND_STRETCH_TOLERANCE`, `COLLISION_THRESHOLD`, `SUPPORTED_MODELS`) | -| **unresolved — line numbers stale** | `exceptions.py` "4 classes never raised" (line 41 is `OptimizationError`, raised 3x, so the cited lines no longer point at what the finding describes); `ASE/thermo` `model_name` param (the `model_wrapper` legacy `name` API is **DONE 2026-08-03**) | +| **resolved 2026-08-03 — also wrong** | `exceptions.py` "4 classes never raised"; `ASE/thermo`'s `model_name` param | +| **resolved 2026-08-03 — was right, removed** | `model_wrapper`'s legacy `name` API | Net: **256 lines removed from `src/`, 167 from `tests/`** — not the ~450 of `src/` the finding claimed, because a third of it was gone and half of the rest is alive. -The three unresolved entries need their own pass; the audit's line numbers cannot -be used to find them. +**M53 is now closed.** The last three entries were resolved by checking source +rather than line numbers: + +- **`model_wrapper`'s legacy `name` API** — right, and removed. The deprecation + said "removed in Auto3D v2.0"; the package reached 3.0.0 with it in place. +- **`ASE/thermo`'s `model_name` parameter** — **wrong**. `Calculator` reads + `self.model_name` at `thermo.py:536`, passing it to `to_model_species`. The + C3/C4 species-conversion work made it live after the audit was written. +- **`exceptions.py`'s "4 classes never raised"** — **wrong**, and the shape of the + claim is the problem. Only `ModelError` is never raised directly, and that is + deliberate: `ModelLoadError` and `NumericalError` subclass it, and + `cli/errors.py:29` maps it to exit code 5 precisely so both subclasses inherit + that code. Deleting it would break the exit-code scheme the 3.0.0 release + documents. "Never raised" is not the same as "unused" for a base class — an + audit that greps for `raise X` cannot tell them apart. + +**Final tally for M53: of 13 entries, 4 were real.** Three had already been done, +six were wrong when re-checked, and the remaining four were deleted in #135/#136. +Nothing further to remove. The single most useful lesson is that a dead-code +finding decays faster than any other kind: the same work that fixes defects revives +symbols the audit saw as dead (`mol2atoms`, `STANDARD_PRESSURE`, `FailedMolecule`, +`print_failures`, `model_name`), and a mechanical sweep would have deleted five +live ones. ## What the remediation closed