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 0d5e9f31..b8302ec4 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 a1645407..a951d975 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