Skip to content

docs: close M53 — the last two entries are wrong, not unresolved #137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
isayev merged 1 commit into from
Aug 3, 2026
Merged

docs: close M53 — the last two entries are wrong, not unresolved #137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions .claude/review-manifests/review-2026-07-30-package-audit.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
28 changes: 25 additions & 3 deletions docs/superpowers/follow-ups-after-4.0.0-remediation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading