From b17d17aaf8a9ae38890e4362d828a249f7934be5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 17:36:26 +0000 Subject: [PATCH 1/3] fix(stubs): drop a parameter lmfit 1.2 does not have, and catch the next one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `stubs/lmfit/model.pyi` declared `Model.fit(..., coerce_farray=...)`. That exists on lmfit 1.3 and not on 1.2.0, which is the floor `pyproject.toml` declares. A stub has to be true across the supported range, and nothing here passes the argument, so it is gone. The more useful half is why the stub test did not catch it. It skipped any function whose real signature ends in `*args`/`**kwargs`, on the reasoning that the stub is allowed to stop early there — which is right, and is the whole approach: declare the parameters we use and no more. But "may stop early" is not "may invent". `**kwargs` upstream makes an invented parameter *worse*, not better: the call is swallowed at runtime, so the mistake surfaces as a silently ignored argument rather than a TypeError. The check now separates the two claims — every declared name must exist, and the order of the ones that do must match — and applies to lmfit and ObsPy through one helper instead of two near-copies. Verified both ways rather than assumed: restoring `coerce_farray` passes against the installed lmfit 1.3.4 and fails against 1.2.0 with the message naming the parameter. The old check passed against both. Found while confirming the floors CI job, which is a separate problem and not addressed here: `uv run` re-syncs the project environment, so that job has been discarding the `--resolution lowest-direct` install and testing the newest of everything. What it would find is that `lmfit>=1.2` and `numpy>=2.0` cannot both be satisfied — lmfit below 1.3.0 calls `np.asfarray`, removed in NumPy 2.0. --- stubs/lmfit/model.pyi | 4 ++- tests/test_stubs.py | 63 ++++++++++++++++++++++++++----------------- 2 files changed, 42 insertions(+), 25 deletions(-) diff --git a/stubs/lmfit/model.pyi b/stubs/lmfit/model.pyi index 5eb7ffe..b6855e1 100644 --- a/stubs/lmfit/model.pyi +++ b/stubs/lmfit/model.pyi @@ -51,6 +51,8 @@ class Model: nan_policy: str | None = ..., calc_covar: bool = ..., max_nfev: int | None = ..., - coerce_farray: bool = ..., + # `coerce_farray` exists on lmfit 1.3 and NOT on 1.2.0, which is this + # project's declared floor. Left out rather than declared: a stub must + # be true across the supported range, and nothing here passes it. **kwargs: Any, ) -> ModelResult: ... diff --git a/tests/test_stubs.py b/tests/test_stubs.py index 1bd6e97..d1a588d 100644 --- a/tests/test_stubs.py +++ b/tests/test_stubs.py @@ -164,6 +164,41 @@ def test_every_declared_function_exists_with_the_same_parameters( ) +def _assert_signature_matches(label: str, node: ast.FunctionDef, function: Any) -> None: + """Every parameter the stub names must exist upstream, in the same order. + + Two separate claims, and the second is why this is a function rather than + one assertion. A stub is allowed to stop early where upstream ends in + ``**kwargs`` — declaring the parameters we use and no more is the whole + approach here. It is **not** allowed to name a parameter that does not + exist, and ``**kwargs`` upstream does not license that: it swallows the + call at runtime, so the mistake shows up as a silently ignored argument + rather than a `TypeError`. + + The first version skipped any function with ``*args`` or ``**kwargs`` + outright. That is exactly how ``Model.fit(coerce_farray=...)`` — which + exists on lmfit 1.3 and not on 1.2, this project's declared floor — sat in + the stubs unnoticed. + """ + try: + actual = [p for p in inspect.signature(function).parameters if p != "self"] + except (TypeError, ValueError): # pragma: no cover + return # a C-level or property object; nothing to compare + + declared = _declared_parameters(node) + #: Variadic names, which a stub may legitimately not mirror. + named = [p for p in actual if p not in ("args", "kwargs", "kws", "options")] + + invented = [p for p in declared if p not in actual] + assert not invented, ( + f"{label}: stub declares {invented}, which the installed library does " + f"not take. It has {actual}." + ) + assert declared[: len(named)] == named[: len(declared)], ( + f"{label}: stub declares {declared}, library has {actual}" + ) + + def test_the_class_methods_take_the_parameters_the_stub_claims() -> None: """The same check for methods, where the risk is identical.""" checks: list[tuple[str, Any]] = [ @@ -174,18 +209,8 @@ def test_the_class_methods_take_the_parameters_the_stub_claims() -> None: for name, methods in _stub_classes(STUBS / stub).items(): cls = getattr(real, name) for node in methods: - function = getattr(cls, node.name) - try: - actual = [ - p for p in inspect.signature(function).parameters if p != "self" - ] - except (TypeError, ValueError): # pragma: no cover - continue # a C-level or property object; nothing to compare - declared = _declared_parameters(node) - if "args" in actual or "kwargs" in actual: - continue # upstream forwards; the stub says so too - assert declared[: len(actual)] == actual[: len(declared)], ( - f"{name}.{node.name}: stub declares {declared}, ObsPy has {actual}" + _assert_signature_matches( + f"{name}.{node.name}", node, getattr(cls, node.name) ) @@ -220,18 +245,8 @@ def test_lmfit_methods_take_the_parameters_the_stub_claims() -> None: for name, methods in _stub_classes(STUBS / stub).items(): cls = getattr(real, name) for node in methods: - function = getattr(cls, node.name) - try: - actual = [ - p for p in inspect.signature(function).parameters if p != "self" - ] - except (TypeError, ValueError): # pragma: no cover - continue - declared = _declared_parameters(node) - if "args" in actual or "kwargs" in actual: - actual = [p for p in actual if p not in ("args", "kwargs")] - assert declared[: len(actual)] == actual[: len(declared)], ( - f"{name}.{node.name}: stub declares {declared}, lmfit has {actual}" + _assert_signature_matches( + f"{name}.{node.name}", node, getattr(cls, node.name) ) From 41cd88d13b547ef9a3b1a6076507307cdbec589c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 18:27:00 +0000 Subject: [PATCH 2/3] fix(deps): raise two floors that were never installable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `floors` job has never tested the floors. It runs uv pip install --resolution lowest-direct -e ".[dev]" uv run pytest -m "not dataset" -q and `uv run` re-resolves the project first. With no committed lock file it installs the newest of everything and discards the line above it, so the job is a duplicate of the ubuntu/3.11 matrix entry — green for that reason, for as long as it has existed. Its own comment says "CI otherwise installs the newest of everything, so the floors are only ever tested by a user", describing exactly what continued to happen. Reproduced rather than inferred: `uv run` prints `Uninstalled 2 packages / Installed 2 packages` and comes back with lmfit 1.3.4 and numpy 2.4.6 where the previous step had put 1.2.0 and 2.0.0. Two declared floors were wrong, and both broke real behaviour: - `lmfit>=1.2` could never be satisfied alongside `numpy>=2.0`. lmfit below 1.3.0 calls `np.asfarray`, removed in NumPy 2.0, so every fit raised AttributeError — 15 tests fail on the declared floor. Bisected: 1.2.0, 1.2.1 and 1.2.2 all fail; 1.3.0 works. - `scipy>=1.13` silently broke the quadratic multitaper. 1.15 reimplemented `scipy.optimize.nnls`; below it the vendored `qiinv` inversion does not converge for every input scale, and peak recovery moves between 0.53 and 1.02 for the same signal at different amplitudes. `test_quadratic.py`'s own scale-invariance test catches it — given a job that installs the floor. Bisected: 1.13 and 1.14 fail, 1.15 passes. Both raised to what the suite was measured to pass on: 523 pass at the corrected floors, through the fixed job end to end. `tools/check_floors.py` asserts the installed versions *are* the declared minimums. That guard matters more than the floors themselves, because the failure mode is invisible by construction — a floors job testing the newest versions looks exactly like one that works. Verified both ways: it passes against a floor environment and fails against the dev environment. Its first version called `2.0` and `2.0.0` different versions and failed against everything. Floors are written at whatever precision reads well, so both sides are zero-padded to equal length rather than truncated to the shorter, which would accept 2.0.5 for a floor of 2.0. The workflow half — `--no-sync` on both `uv run` lines — is not in this commit: pushing `.github/workflows/test.yml` needs a permission this account does not have, so it was applied directly to main and this branch is rebased on top of it. That ordering leaves main's `floors` job red until this merges, because it now calls a script main does not yet have. `uv.lock` is gitignored. Not committing one is deliberate: a lock file is what would make `--resolution lowest-direct` meaningless again. --- .gitignore | 4 ++ docs/REFACTOR_PLAN.md | 28 +++++++++ pyproject.toml | 17 +++++- tools/check_floors.py | 139 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 186 insertions(+), 2 deletions(-) create mode 100644 tools/check_floors.py diff --git a/.gitignore b/.gitignore index 0450f1e..5180ad2 100644 --- a/.gitignore +++ b/.gitignore @@ -42,3 +42,7 @@ docs/_build/ # changes. The inputs (Tutorial/Data, Tutorial/MetaData) *are* committed. Tutorial/Spectra/*.h5 Tutorial/Spectra/FlatFiles/ + +# Created by `uv run` without --no-sync; this project resolves fresh on +# purpose so the floors job can test the declared minimums. +uv.lock diff --git a/docs/REFACTOR_PLAN.md b/docs/REFACTOR_PLAN.md index b8e537c..46f1f3a 100644 --- a/docs/REFACTOR_PLAN.md +++ b/docs/REFACTOR_PLAN.md @@ -1901,6 +1901,34 @@ plan: libraries and checks the declarations against them. It caught an error in the stubs on its first run. +**Dependency floors are measured, not guessed.** The `floors` CI job installs +`--resolution lowest-direct` so the declared minimums are exercised rather than +left for a user to discover. It did not work: `uv run` re-resolves the project +before running, and with no committed lock file it installed the newest of +everything and discarded the floor install. The job was a duplicate of the +ubuntu/3.11 matrix entry, and green for that reason — for as long as it had +existed. + +Two declared floors were wrong, and both broke real behaviour: + +| declared | actual | what failed below it | +|---|---|---| +| `lmfit>=1.2` | `>=1.3` | 1.2.x calls `np.asfarray`, removed in NumPy 2.0. `lmfit>=1.2` and `numpy>=2.0` were never both satisfiable; 15 tests fail. | +| `scipy>=1.13` | `>=1.15` | 1.15 reimplemented `scipy.optimize.nnls`. Below it the vendored `qiinv` does not converge for every input scale and the quadratic estimator's peak recovery moves between 0.53 and 1.02 for the same signal. | + +`tools/check_floors.py` asserts the installed versions *are* the declared +minimums, and the job runs it before pytest. That guard is the point rather +than the floors themselves: a floors job quietly testing the newest versions +looks exactly like one that works, so the mismatch has to be an error rather +than something a reader might notice. + +This is the third instance in this document of one shape — an intention stated +here, restated as fact in a comment, with no mechanism behind it. The other two +were "CI can assert it never grows" for the mypy backlog, and the +`ignore_missing_imports` recommendation that would have made every ObsPy and +lmfit object `Any`. Worth reading the remaining "CI can ..." sentences in this +plan as open questions rather than as descriptions. + **pre-commit** runs ruff (lint + format), mypy, `nbstripout` on the tutorial notebook, and `check-added-large-files` — the last one specifically to stop another 70 waveform files landing in git. diff --git a/pyproject.toml b/pyproject.toml index 2021997..028d1a4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,11 +21,24 @@ classifiers = [ "Topic :: Scientific/Engineering :: Physics", ] +# Every floor here is the version the suite was measured to pass on, not the +# oldest that imports — see the `floors` job in .github/workflows/test.yml and +# tools/check_floors.py. Two of them were wrong until measured: `lmfit>=1.2` +# was unsatisfiable against `numpy>=2.0`, and `scipy>=1.13` silently broke the +# quadratic multitaper. dependencies = [ "numpy>=2.0", # np.trapezoid; np.trapz was the pre-2.0 spelling - "scipy>=1.13", # first release supporting numpy 2 + # 1.15 reimplemented `scipy.optimize.nnls`. On 1.13/1.14 the vendored + # `qiinv` inversion does not converge for every input scale, and the + # quadratic estimator's peak recovery moves between 0.53 and 1.02 for the + # same signal at different amplitudes — which its own scale-invariance + # test catches, given a CI job that actually installs the floor. + "scipy>=1.15", "obspy>=1.5", # 1.4.x imports pkg_resources, gone from setuptools 81+ - "lmfit>=1.2", + # 1.2.x calls `np.asfarray`, removed in NumPy 2.0. `lmfit>=1.2` and + # `numpy>=2.0` could never both be satisfied; every fit raised + # AttributeError on that combination. + "lmfit>=1.3", "pandas>=2.2.2", # first release with numpy 2 ABI wheels "matplotlib>=3.9", # numpy 2 support "pooch>=1.8", diff --git a/tools/check_floors.py b/tools/check_floors.py new file mode 100644 index 0000000..2778f04 --- /dev/null +++ b/tools/check_floors.py @@ -0,0 +1,139 @@ +"""Assert that the installed versions really are the declared minimums. + +The ``floors`` CI job exists to exercise the oldest dependency set the project +claims to support. It did not. The job ran:: + + uv pip install --resolution lowest-direct -e ".[dev]" + uv run pytest -m "not dataset" -q + +and ``uv run`` re-syncs the project environment before running — there is no +committed lock file, so it resolved fresh to the newest of everything and +uninstalled what the previous step had just put in place. The job was a +duplicate of the ubuntu/3.11 test matrix entry, and green for that reason. + +The comment above it read *"Exercise the declared minimums. CI otherwise +installs the newest of everything, so the floors are only ever tested by a +user."* — describing precisely what continued to happen. + +What it would have caught, once fixed: + +* ``lmfit>=1.2`` and ``numpy>=2.0`` could never both be satisfied. lmfit below + 1.3.0 calls ``np.asfarray``, removed in NumPy 2.0, so every fit raised + ``AttributeError``. 15 tests fail on the declared floor. +* ``scipy>=1.13`` silently broke the quadratic multitaper. 1.15 reimplemented + ``scipy.optimize.nnls``; before that the vendored ``qiinv`` inversion does + not converge for every input scale, and peak recovery moves between 0.53 and + 1.02 for the same signal at different amplitudes. + +Neither was theoretical and neither was visible from a green CI. + +So this script is the guard: a job that stops installing floors now *fails* +rather than passing quietly, because that failure mode is invisible by +construction — a floors job testing the newest versions looks exactly like a +floors job that works. + +Run with no arguments; exits non-zero and prints every mismatch. + +**``--no-sync`` in the workflow is the other half, and it is load-bearing.** +Both ``uv run`` lines in the ``floors`` job carry it:: + + - run: uv pip install --resolution lowest-direct -e ".[dev]" + - run: uv run --no-sync python tools/check_floors.py + - run: uv run --no-sync pytest -m "not dataset" -q + +Dropping it from either line puts the job straight back to testing the newest +of everything. The difference is that this script now turns that into a +failure rather than a silent pass — which is the whole reason it exists, since +the two states are otherwise indistinguishable from a green run. +""" + +from __future__ import annotations + +import re +import sys +import tomllib +from importlib.metadata import PackageNotFoundError, version +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent + +#: ``name>=X.Y`` — the only form used, and the only one this understands. A +#: requirement written any other way is reported rather than skipped, so a new +#: spelling cannot quietly drop out of the check. +FLOOR = re.compile(r"^([A-Za-z0-9_.\-]+)\s*>=\s*([0-9][0-9A-Za-z.\-]*)$") + + +def _requirements() -> list[str]: + project = tomllib.loads((ROOT / "pyproject.toml").read_text())["project"] + required: list[str] = list(project["dependencies"]) + # `dev` pulls in the test tooling and repeats the io extra; its versions + # are installed by the same command, so they are checked the same way. + for name, extra in project.get("optional-dependencies", {}).items(): + if name == "dev": + required += extra + return required + + +def _normalise(name: str) -> str: + return re.sub(r"[-_.]+", "-", name).lower() + + +def _same_version(declared: str, installed: str) -> bool: + """``2.0`` and ``2.0.0`` are the same version; ``2.0`` and ``2.4.6`` are not. + + Floors are written at whatever precision reads well — ``numpy>=2.0``, + ``pandas>=2.2.2`` — while the resolver installs a full release number. So + both are padded to equal length before comparison rather than truncated to + the shorter, which would accept 2.0.5 for a floor of 2.0. + """ + a = [int(p) for p in re.findall(r"\d+", declared)] + b = [int(p) for p in re.findall(r"\d+", installed)] + width = max(len(a), len(b)) + return a + [0] * (width - len(a)) == b + [0] * (width - len(b)) + + +def main() -> int: + problems: list[str] = [] + checked = 0 + + for requirement in _requirements(): + # Strip any environment marker; nothing here uses one, but a marker + # would otherwise be parsed as part of the version. + text = requirement.split(";")[0].strip() + match = FLOOR.match(text) + if match is None: + problems.append( + f"{text!r} is not of the form 'name>=X.Y', so its floor is not " + "being checked. Extend tools/check_floors.py rather than " + "leaving it unchecked." + ) + continue + + name, declared = match.groups() + try: + installed = version(_normalise(name)) + except PackageNotFoundError: + problems.append(f"{name} is declared but not installed") + continue + + checked += 1 + if not _same_version(declared, installed): + problems.append( + f"{name}: declared floor {declared}, but {installed} is " + "installed. This job is meant to run the floors — if the " + "install step resolved something newer, it is not testing " + "what it claims to." + ) + + if problems: + print("floors are not what is installed:\n", file=sys.stderr) + for problem in problems: + print(f" - {problem}", file=sys.stderr) + return 1 + + print(f"all {checked} declared floors are the installed versions") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From 8ba4a172af56e926f662eb89a7717de75b72a6f8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 19:08:04 +0000 Subject: [PATCH 3/3] docs(plan): audit the enforcement claims, and correct four that were false MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three claims in this plan have now turned out to describe mechanisms that did not exist — "CI can assert it never grows" for the mypy backlog, the `ignore_missing_imports` recommendation that would have made every ObsPy and lmfit object `Any`, and the `floors` job that discarded its own lowest-direct install. One shape three times: an intention stated in the plan, restated as fact in a comment, with nothing behind it. So the rest were checked rather than read. New §6.6 records the result; the sections it corrects are fixed in place rather than annotated, so the document does not have to be read alongside its own errata. False, and corrected: - "pre-commit runs ruff, **mypy**, nbstripout, check-added-large-files" — mypy is not a hook and never was. Defensible (it needs the project environment and the `typecheck` job covers it), but the plan should not claim a hook that is not installed. The real set is listed now, including the local `no-session-links` commit-msg hook — which appeared nowhere in this document and is the only thing keeping private session URLs out of a public repository's history. - "Conventional Commits, **`commitlint`-enforced**" — no such hook. The convention is followed by hand. - "`sphinx-build -W` … an **undocumented public symbol** fails the build" — wrong even once the docs build exists. `-W` promotes warnings Sphinx already emits, and autodoc emits none for a symbol it was never asked to document. That needs `sphinx.ext.coverage` or `nitpicky`. - The `test.yml` row described one matrix step doing ruff, mypy and pytest. It is five jobs — `lint`, `typecheck`, `test`, `floors`, plus `build.yml`. One claim whose mechanism is absent but whose property holds, which is worth separating rather than filing under either heading: "regression tests pin an explicit config file, never the defaults". Nothing pins a config; the golden tests read the shipped defaults. But the property was tested directly — bump `smoothing.n_bins` 151 -> 158 and 9 of 25 golden tests fail, loudly — because the golden values are frozen in a committed JSON file. The residual gap is real but narrower: a *regenerated* reference silently adopts whatever defaults were current, since the file records no config. Three claims checked and holding: the Parseval contract parametrised over `ESTIMATORS`, `PLOT_COLUMNS` having exactly one definition, and the registry-wide noise-rotation property test. One passage is simply stale — §5.2 still describes `BW_METHOD`/`ROT_METHOD` as globals in `spectral`, "a shell over `core`", with `sp.ROT_METHOD = 1` as a live escape hatch. Both `spectral.py` and `_config_legacy.py` were deleted in phase 2. The audit needed §6.6, so branch layout moved to §6.7. All 21 `§6.x` cross-references were re-resolved against the headings; four pointed at the wrong section afterwards and are fixed. --- docs/REFACTOR_PLAN.md | 119 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 106 insertions(+), 13 deletions(-) diff --git a/docs/REFACTOR_PLAN.md b/docs/REFACTOR_PLAN.md index 46f1f3a..d782c02 100644 --- a/docs/REFACTOR_PLAN.md +++ b/docs/REFACTOR_PLAN.md @@ -1035,7 +1035,7 @@ and there are presumably more in your working directories. Two ways out, and the first is much better: 1. **Convert in the old environment.** Phase 0 is already building a Docker image - where the 0.1.0 code runs (§6.5). Add a small + where the 0.1.0 code runs (§6.7). Add a small `scripts/convert_legacy_spec.py` to it that loads `.spec` files with the old classes present and writes the new HDF5 format. This is a one-shot migration with no lasting cost to the codebase. @@ -1586,7 +1586,7 @@ Two further problems with reproducing the published run from this repository: workflow the paper describes. Only `09f57b9` (removed cython) changes `specmod/` after that, so the realistic candidates are `ba3f7ec` and `453c77c`. Pick one, record the reasoning, and move on — this is the - strongest possible argument for the `v0.1.0` tag (§6.6) and for `hatch-vcs` + strongest possible argument for the `v0.1.0` tag (§6.7) and for `hatch-vcs` (§6.4): it must never be this hard again. 2. **The full published pipeline is not in this repository.** The two-stage inversion (fit Ω/`f_c`/`t*` free, then fix event `f_c` to the @@ -1929,10 +1929,23 @@ were "CI can assert it never grows" for the mypy backlog, and the lmfit object `Any`. Worth reading the remaining "CI can ..." sentences in this plan as open questions rather than as descriptions. -**pre-commit** runs ruff (lint + format), mypy, `nbstripout` on the tutorial +**pre-commit** runs ruff (lint + format), `nbstripout` on the tutorial notebook, and `check-added-large-files` — the last one specifically to stop another 70 waveform files landing in git. +Two corrections to what this used to say, both found by the audit in §6.6. +**mypy is not a pre-commit hook.** It never was; the sentence listed it and +nothing installed it. That is a defensible position — mypy needs the project +environment and is slow to run on every commit, and the `typecheck` CI job +covers it — but the plan should not claim a hook that is not there. The +actually-installed set is `ruff-check`, `ruff-format`, +`check-added-large-files`, `check-toml`, `check-yaml`, `end-of-file-fixer`, +`trailing-whitespace`, `mixed-line-ending`, `nbstripout` and a local +`no-session-links` commit-msg hook. That last one is not mentioned anywhere +else in this document and is the only mechanism keeping private session URLs +out of a public repository's history — it belongs in the plan rather than +existing only in the config file. + ### 6.3 Documentation (Sphinx) - **Sphinx** with `pydata-sphinx-theme` (the NumPy/SciPy/ObsPy house style — @@ -1945,8 +1958,14 @@ another 70 waveform files landing in git. - `intersphinx` to numpy, scipy, obspy, lmfit, matplotlib. - `sphinx.ext.doctest` — the units/normalisation examples in §4.2 and §4.4 are exactly the kind of thing that should be executable in the docs. -- `sphinx-build -W` (warnings as errors) in CI: a broken cross-reference or an - undocumented public symbol fails the build. +- `sphinx-build -W` (warnings as errors) in CI: a broken cross-reference fails + the build. **Not** an undocumented public symbol, which is what this line + used to claim — `-W` promotes warnings that Sphinx already emits, and + autodoc emits none for a symbol it was never asked to document. Catching + that needs `sphinx.ext.coverage` with `coverage_show_missing_items`, or + `nitpicky` for unresolved references. Worth having; it is a different + setting, and writing it as a property of `-W` would have meant discovering + the gap only after trusting it. - Structure: Getting started → User guide (preprocessing, transforms, SNR, fitting) → **Theory** (the normalisation conventions, one page, with the Parseval contract stated explicitly) → Tutorial → API reference → Migration @@ -1986,7 +2005,7 @@ hook. It is a small discipline and it is what makes the changelog automatic. | Workflow | Trigger | Does | |---|---|---| -| `test.yml` | PR, push | matrix 3.11/3.12/3.13 × ubuntu/macos → ruff check, ruff format --check, mypy, pytest + coverage → Codecov | +| `test.yml` | PR, push | `lint` (ruff check + format), `typecheck` (mypy), `test` matrix 3.11/3.12/3.13 × ubuntu/macos (pytest + coverage → Codecov), `floors` (`--resolution lowest-direct`, see §6.6). Five job names, not one matrix — the row used to describe them as a single matrix step | | `docs.yml` | PR, push | `sphinx-build -W`; on `main`, deploy to GitHub Pages. Builds on PRs too, so doc breakage is caught before merge | | `build.yml` | PR, push | sdist + wheel, `twine check`, install-from-wheel smoke test in a clean env (catches missing package data) | | `release-please.yml` | push to `main` | maintains the release PR; creates tag + GitHub Release on merge | @@ -2003,7 +2022,78 @@ is needed (§3.1). Tag `v0.2.0` at the end of Phase 2 to prove the pipeline; Zenodo DOI give the project a citable artefact, which it currently lacks entirely. -### 6.6 Branch layout and preserving the pre-refactor state +### 6.6 Audit: which enforcement claims in this document are real + +Three separate claims in this plan turned out to describe mechanisms that did +not exist, each discovered only when something downstream went wrong: + +1. *"The override list is the migration backlog, and CI can assert it never + grows."* Became the comment `# CI asserts it never grows` above the mypy + override. Nothing did. Adding a fourth module would have been green. +2. *`ignore_missing_imports` for ObsPy and lmfit.* Would have made every object + from both libraries `Any` — in the two modules where nearly every value + comes from one of them. Silences the import; checks nothing. +3. *The `floors` job.* `uv run` re-resolved the project and discarded the + `--resolution lowest-direct` install, so the job tested the newest of + everything while its comment said the opposite. Two declared floors were + unsatisfiable as a result. + +One shape, three times: an intention stated here, restated as fact in a +comment, with no mechanism behind it. So the rest were checked rather than +read. What follows is the result, and it is recorded because a plan that has +been audited once is worth more than one that reads confidently throughout. + +**Claims that were false, now corrected in place:** + +| Claim | Reality | +|---|---| +| "pre-commit runs ruff, **mypy**, nbstripout, check-added-large-files" | mypy is not a hook and never was. CI's `typecheck` job covers it. §6.2 corrected. | +| "Conventional Commits, **`commitlint`-enforced**" (§7) | No `commitlint` hook. The convention is followed by hand. §7 corrected. | +| "`sphinx-build -W` … an **undocumented public symbol** fails the build" | `-W` promotes warnings Sphinx emits; autodoc emits none for a symbol it was never asked to document. Needs `sphinx.ext.coverage` or `nitpicky`. §6.3 corrected. | +| `test.yml` "matrix … → ruff, mypy, pytest" | Five jobs, not one matrix: `lint`, `typecheck`, `test`, `floors`, plus `build.yml`. §6.5 corrected. | + +**A claim whose mechanism is absent but whose property holds — for a different +reason, which matters:** + +> "Regression tests pin an explicit config file, never the defaults, so +> changing a default cannot silently move a golden test." + +Nothing pins a config. `tests/test_golden_reference.py` reads the shipped +defaults through `load_config()`. But the property was tested directly — +bumping `smoothing.n_bins` from 151 to 158 and running the suite — and **9 of +25 golden tests fail**, loudly. The golden values live in a committed JSON +file, so any behaviour change breaks them whatever moved it. + +The residual gap is narrower than the claim suggests but real: the golden file +records no config, so a *regenerated* reference silently adopts whatever +defaults were current. Pinning a study file, as this plan proposes, is what +would close it. Until then the protection is "the numbers are frozen", not +"the settings are frozen". + +**Claims that were checked and hold:** + +- "One normalisation contract, enforced by one test, for all backends" — the + Parseval contract is asserted across the registry in `tests/test_transforms.py`. +- "`PLOT_COLUMNS` … semantic grouping makes [two disagreeing copies] + structurally impossible" — one definition, `viz.plot_columns`, read through + the config by both `fitting` and `plotting`. +- "the registry-wide test asserts the property" (noise rotation returning a + factor ≥ 1) — `tests/test_collection.py` asserts finiteness, shape and + positivity for every registered model. + +**One passage that is simply stale**, in §5.2 on configuration: it describes +`BW_METHOD` and `ROT_METHOD` surviving as module-level globals in `spectral` +"a shell over `core`", and `sp.ROT_METHOD = 1` as a live escape hatch. +`spectral.py` and `_config_legacy.py` were both deleted in phase 2. The test it +names still exists and still asserts a derivation, but of `_compare_settings` +in `pipeline`, not of legacy globals. + +**Not yet built, and correctly forward-looking** — no correction needed, but +worth listing so nothing here reads as a description of the present: Sphinx docs and +`docs.yml`, `release-please`, `datasets/magna_2020.toml`, the acquisition layer +of §5.2, and the two-stage fit API of §5.2.5. + +### 6.7 Branch layout and preserving the pre-refactor state **`master` is frozen; `main` is the trunk.** `main` was branched from `master` at `453c77c` and is where all refactor work lands. `master` is never committed to @@ -2096,7 +2186,9 @@ year. `docs:`, `test:`, `chore:`; `feat!:` or a `BREAKING CHANGE:` trailer for breaks), enforced by a `commitlint` pre-commit hook. This is what makes the changelog and version bumps automatic. The same `commit-msg` stage should reject -Claude session URLs (§6.6) — public repository, private links. +Claude session URLs (§6.7) — public repository, private links. That hook +exists and is installed; the `commitlint` half of this sentence does not +(§6.6). **PyPI name.** `specmod` is unregistered (checked: 404 on `specmod`, `spec-mod` and `pyspecmod`). Worth claiming with the `v0.2.0` release at the end of Phase 2 @@ -2111,7 +2203,7 @@ Each phase ends green on CI and is independently mergeable. | Phase | Work | Depends on | Rough size | |---|---|---|---| -| **0. Safety net** | Freeze `master`, default branch → `main`, optional `v0.1.0` tag (§6.6); reproducible legacy env (`Dockerfile`: gfortran + ObsPy 1.2.0 / SciPy 1.4.1 / NumPy 1.18 / pandas 1.0.0 (§5.2.6)); write `datasets/magna_2020.toml` and a first cut of `specmod.acquire`, publish the artifact as a `data-v1` release asset (§5.2); capture golden outputs for PNR **and** Magna; reproduce Table S2 / Figure 2 with 0.1.1 (§5.2.6 step 2); convert any `.spec` files (§4.6) | — | 1.5–2 days | +| **0. Safety net** | Freeze `master`, default branch → `main`, optional `v0.1.0` tag (§6.7); reproducible legacy env (`Dockerfile`: gfortran + ObsPy 1.2.0 / SciPy 1.4.1 / NumPy 1.18 / pandas 1.0.0 (§5.2.6)); write `datasets/magna_2020.toml` and a first cut of `specmod.acquire`, publish the artifact as a `data-v1` release asset (§5.2); capture golden outputs for PNR **and** Magna; reproduce Table S2 / Figure 2 with 0.1.1 (§5.2.6 step 2); convert any `.spec` files (§4.6) | — | 1.5–2 days | | **1. Make it installable** | `pyproject.toml` + hatch-vcs, `src/` layout, `__init__.py`; ruff config, one-shot `ruff format` + `.git-blame-ignore-revs`, module renames to snake_case; mypy skeleton; pre-commit; `test`/`build` CI; `.gitignore`, `CITATION.cff`; fix the three hard breakages (§1) and the four `F821` bugs ruff finds (§2.5); delete `Tests/Tutorial/`, strip notebook outputs, subset the inventory (§5.1) | 0 | 3–4 days | | **2. De-globalise** | `config/` package per §4.7 — semantic groups, layer resolution, `config show`/`freeze`, provenance stamping; remove all module-level config reads (tracked by `PLW0603`); `Motion`/`AmplitudeKind` enums; `Spectrum` as a frozen dataclass with `duration`; mutable class attrs (`RUF012`); `isinstance` checks; `logging`. **Tag `v0.2.0`** | 1 | 3–4 days | | **2b. Release plumbing** | Sphinx skeleton + `pydata-sphinx-theme` + autodoc/napoleon/intersphinx; `docs.yml` → GH Pages; release-please + `publish.yml` (PyPI Trusted Publishing); Zenodo webhook. Parallel with 2 | 1 | 1–2 days | @@ -2149,8 +2241,9 @@ end-to-end proves the pipeline while the stakes are zero. - **Backwards compatibility** — clean break. No downstream users, so the 0.x API is removed outright: no `legacy.py`, no deprecation cycle (§3.1). Breaking changes expected throughout `0.x`; `1.0` when the API settles. -- **Commit convention** — Conventional Commits, `commitlint`-enforced, driving - release-please (§6.4). +- **Commit convention** — Conventional Commits, driving release-please (§6.4). + Followed by hand today; the `commitlint` hook of §6.4 is **not installed**, + so nothing enforces it (§6.6). - **Validation anchor** — Magna 2020, with the published workflow transcribed into `datasets/magna_2020.toml` and Figure 2 / Tables S1–S2 as regression targets (§5.2.4–§5.2.6). @@ -2165,8 +2258,8 @@ end-to-end proves the pipeline while the stakes are zero. (§4.7). Current behaviour stays the default. - **Tooling** — ruff for lint and format, mypy staged to strict, Sphinx for docs, automated versioning and publishing for both docs and package (§6). -- **Branch layout** — `master` frozen as the pre-refactor record, `main` as the new trunk (§6.6). One of the three - preservation layers in §6.5. +- **Branch layout** — `master` frozen as the pre-refactor record, `main` as the new trunk (§6.7). One of the three + preservation layers in §6.7. ### Still open