From f87fbd1d205a1d4e195b24dcbd00f03c89c00896 Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Thu, 6 Aug 2026 15:40:44 -0500 Subject: [PATCH 1/3] ci: gate complexity with xenon, track its trend with wily Complexity is the one structural property that degrades without anyone deciding to degrade it: every change adds a branch for a good local reason, and the aggregate is nobody's decision. That makes it worth a gate rather than a review habit -- especially where coding agents contribute, since an agent optimizing for a passing test suite has no reason to notice a function crossing into unmaintainable. Add xenon as a ratchet, in CI and as a pre-commit hook running the identical check, so a clean pre-commit run means CI agrees. The thresholds -- no block above rank C, no module below B, average at A -- are the tightest the package passes today. That is deliberate: a gate set where the code already is holds the line without demanding a refactor first, and tightening a letter later is a visible commit rather than a silent drift. It failed on an injected rank-D function and named the block, which is the property that makes the fix local. Scope is the package, not the tests, which legitimately contain long branchy fixtures. The path is in args rather than passed by pre-commit because --max-average and --max-modules are whole-package judgements; computed over only the changed files, the average could drift upward one commit at a time while every individual commit passed. wily is advisory and never blocks. On a pull request the job writes a complexity diff against the base into the run summary, so a reviewer sees direction rather than a verdict. Rising complexity in a file that gained a genuinely complex feature is information, not a failure -- which is exactly why it is separate from the gate. Co-Authored-By: Claude Opus 5 --- .github/workflows/python-package.yml | 40 ++++++++++++++++++++++++++++ .pre-commit-config.yaml | 25 +++++++++++++++++ CONTRIBUTING.md | 22 +++++++++++++++ pyproject.toml | 8 ++++++ 4 files changed, 95 insertions(+) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index c56bdf39..3bb48b03 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -27,6 +27,46 @@ jobs: ruff check . --output-format=github ruff format --check . + complexity: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + with: + # wily needs history to compare against the base; the gate itself + # only needs the working tree. + fetch-depth: 0 + - name: Set up Python 3.14 + uses: actions/setup-python@v6 + with: + python-version: "3.14" + cache: "pip" + - name: Install metrics tooling + # Versions pinned in the [metrics] extra so CI and the local + # pre-commit hook grade identically. + run: pip install -e .[metrics] + - name: Complexity gate + # Ratchet, not an aspiration: these are the tightest thresholds the + # package currently passes. A change that regresses complexity fails + # here with the offending block named. Mirrors the xenon pre-commit + # hook, so a contributor sees the same verdict before pushing. + run: xenon --max-absolute C --max-modules B --max-average A dataretrieval + - name: Complexity trend vs base + # Advisory: reports which files moved and by how much, so a reviewer + # can see direction rather than a pass/fail. Never fails the build -- + # the gate above is what blocks. + if: github.event_name == 'pull_request' + continue-on-error: true + run: | + base="${{ github.event.pull_request.base.sha }}" + git fetch --quiet origin "$base" || true + wily build dataretrieval --max-revisions 40 >/dev/null 2>&1 || true + echo '### Complexity trend' >> "$GITHUB_STEP_SUMMARY" + echo '```' >> "$GITHUB_STEP_SUMMARY" + wily diff dataretrieval --revision "$base" 2>&1 | tail -40 \ + >> "$GITHUB_STEP_SUMMARY" || echo 'no comparable revision' \ + >> "$GITHUB_STEP_SUMMARY" + echo '```' >> "$GITHUB_STEP_SUMMARY" + package-artifact: runs-on: ubuntu-latest steps: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8dd44167..9305a68d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -38,6 +38,31 @@ repos: pass_filenames: false additional_dependencies: [httpx, anyio] + # Complexity ratchet. Thresholds are the tightest the package currently + # passes, so this holds the line rather than demanding a refactor: no block + # above rank C (cyclomatic complexity 20), no module below B, average at A. + # Tightening a letter is a deliberate commit, and the failure names the + # offending block, so the fix is local. Applied to the package only -- + # tests legitimately contain long, branchy fixtures. + - repo: https://github.com/rubik/xenon + rev: v0.9.3 + hooks: + - id: xenon + # The package path is in args, not passed by pre-commit: --max-average + # and --max-modules are whole-package judgements, and computing them + # over only the changed files would let the average drift upward one + # commit at a time. ``files`` decides whether to run, not what to scan. + args: + - "--max-absolute" + - "C" + - "--max-modules" + - "B" + - "--max-average" + - "A" + - "dataretrieval" + files: ^dataretrieval/.*\.py$ + pass_filenames: false + # Strip cell outputs + execution_count from notebooks on commit so the # diff is the source, not the rendered run. Demos still execute fine # locally; clean commits keep PRs reviewable and avoid quota/timestamp diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 024d6192..573654c2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -103,8 +103,30 @@ ruff format --check . mypy coverage run -m pytest tests/ coverage report -m +xenon --max-absolute C --max-modules B --max-average A dataretrieval ``` +The last one is a complexity ratchet: those thresholds are the tightest the +package passes today, so it fails only when a change makes complexity worse. It +names the offending block, so the fix is local -- usually extracting a branch +rather than restructuring. Install it with `pip install -e .[metrics]`; the +`xenon` pre-commit hook runs the identical check, so a clean pre-commit run means +CI agrees. + +To see the *trend* rather than a pass/fail, that extra also installs +[`wily`](https://github.com/tonybaloney/wily), which indexes metrics across git +history: + +```bash +wily build dataretrieval --max-revisions 50 # index recent commits (slow, once) +wily report dataretrieval # how the package moved over time +wily diff dataretrieval --revision main # what your branch changed +wily rank dataretrieval maintainability.mi # worst-maintained files today +``` + +`wily` is advisory and is never a merge gate -- rising complexity in a file that +gained a genuinely complex feature is information, not a failure. + For documentation changes, install `.[doc,nldi]` and run `make html` from `docs/`. The broader `make docs` target also runs doctests and network-dependent link checking. diff --git a/pyproject.toml b/pyproject.toml index cec99697..efdbd6c1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,6 +48,14 @@ dataretrieval = ["py.typed"] type-check = [ "mypy", ] +# Complexity gates and history. ``xenon`` fails a build when complexity +# regresses; ``wily`` tracks the trend across commits so a review can say +# whether a change moved the codebase, not just whether it passed. Kept out of +# ``test`` so the test job stays lean -- the metrics job installs this instead. +metrics = [ + "xenon==0.9.3", + "wily==1.25.0", +] test = [ "pytest > 5.0.0", "pytest-cov[all]", From 6baac3a3a6b383d2d89ea158ee4a6c3c492c971f Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Thu, 6 Aug 2026 15:43:54 -0500 Subject: [PATCH 2/3] chore: ignore the wily metrics cache `wily build` writes a .wily/ index of per-revision metrics. It is a local cache, rebuildable from git history in one command, and large enough to be worth keeping out of diffs. Co-Authored-By: Claude Opus 5 --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 182189e4..0b08fada 100644 --- a/.gitignore +++ b/.gitignore @@ -112,3 +112,6 @@ ENV/ # macOS *.DS_Store + +# wily metrics cache (rebuildable: `wily build dataretrieval`) +.wily/ From 4dc101fe1e1792864b9433ce1562c5151ca298c9 Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Thu, 6 Aug 2026 14:16:15 -0500 Subject: [PATCH 3/3] ci: drop wheel-smoke assertions that cannot fail The two remaining find_spec checks were as redundant as the eight removed with the adapter-split cleanup: the script imports dataretrieval.ogc.engine directly, and waterdata/__init__.py imports .api eagerly and unconditionally, so both modules are already in sys.modules by the time the assertions run. A module missing from the wheel raises at the import statement several lines earlier -- the find_spec call is unreachable as a failure mode. Verified by extracting the script and running it both ways: it still passes when the package resolves outside GITHUB_WORKSPACE, and still fails when it resolves from inside the checkout, which is the regression it exists to catch (source-tree imports masking a broken wheel, #347). importlib.util goes with them; importlib.resources.files is a separate import and stays. Co-Authored-By: Claude Opus 5 --- .github/workflows/python-package.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 3bb48b03..6230850f 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -87,7 +87,6 @@ jobs: "$venv/bin/python" -m pip install "$GITHUB_WORKSPACE"/wheelhouse/*.whl cd "$RUNNER_TEMP" "$venv/bin/python" -I - <<'PY' - import importlib.util import os from importlib.resources import files from pathlib import Path @@ -100,8 +99,6 @@ jobs: checkout = Path(os.environ["GITHUB_WORKSPACE"]).resolve() installed = Path(dataretrieval.__file__).resolve() assert not installed.is_relative_to(checkout), (installed, checkout) - assert importlib.util.find_spec("dataretrieval.waterdata.api") is not None - assert importlib.util.find_spec("dataretrieval.ogc.engine") is not None assert files("dataretrieval").joinpath("py.typed").is_file() assert waterdata.get_daily assert ngwmn.get_sites