ci: add cognitive-complexity and dependency-direction gates, plus a weekly structural sweep

.github/workflows/code-health.yml

@@ -0,0 +1,118 @@
+# Periodic structural sweep: duplication, coupling, cohesion, dead code, and the
+# maintainability trend.
+#
+# Advisory by design, and nothing here fails the build. These measures move over
+# months rather than commits, and they report findings a reviewer has to judge
+# rather than obey -- a metric can call good design a violation. The merge gates
+# live in python-package.yml. See CONTRIBUTING.md for how to read the output.
+
+name: Code Health
+
+on:
+  schedule:
+    # 08:00 UTC Mondays: a week's worth of merges, read before the week starts.
+    - cron: "0 8 * * 1"
+  # On demand, for checking whether a refactor actually moved the numbers.
+  workflow_dispatch:
+
+concurrency:
+  group: ${{ github.workflow }}
+  cancel-in-progress: false
+
+permissions:
+  contents: read
+
+jobs:
+  health:
+    runs-on: ubuntu-latest
+    timeout-minutes: 20
+    steps:
+      - uses: actions/checkout@v6
+        with:
+          # wily indexes git history; a shallow clone gives it one revision to
+          # trend against, which is no trend at all.
+          fetch-depth: 0
+      - name: Set up Python 3.13
+        uses: actions/setup-python@v6
+        with:
+          python-version: "3.13"
+          cache: "pip"
+      - name: Install analysis tooling
+        run: |
+          python -m pip install --upgrade pip
+          pip install -e .[metrics,health]
+
+      - name: Structural analysis (pyscn)
+        # One output format per invocation, so run it twice: JSON to read the
+        # numbers, HTML to read the report. Never fails the job -- a non-zero
+        # exit here means "findings", which are the point.
+        continue-on-error: true
+        run: |
+          pyscn analyze --json --no-open dataretrieval | tee pyscn-summary.txt
+          pyscn analyze --html --no-open dataretrieval >/dev/null
+
+      - name: Check the analysis actually resolved the package
+        # pyscn infers a project root, and when it guesses wrong it silently
+        # resolves only a fraction of the imports -- which *raises* the score,
+        # because most of what it grades is dependency-derived. A degraded run
+        # therefore looks like an improved one. Record the resolved edge count
+        # next to the score so that is visible rather than flattering.
+        if: always()
+        continue-on-error: true
+        run: |
+          python - <<'PY' > pyscn-sanity.txt
+          import glob, json, os
+          reports = sorted(glob.glob(".pyscn/reports/*.json"))
+          if not reports:
+              print("no pyscn JSON report found"); raise SystemExit
+          s = json.load(open(reports[-1]))["system"]["Summary"]
+          root, deps = s["ProjectRoot"], s["TotalDependencies"]
+          print(f"modules={s['TotalModules']} resolved_dependencies={deps} root={root}")
+          if os.path.realpath(root) != os.path.realpath(os.getcwd()):
+              print(f"WARNING: project root {root!r} is not the checkout; "
+                    "import resolution is probably degraded and the scores "
+                    "above are not comparable to previous runs.")
+          PY
+          cat pyscn-sanity.txt
+
+      - name: Maintainability ranking (wily)
+        # Worst-maintained files today, and how the package has moved recently.
+        run: |
+          wily build dataretrieval --max-revisions 50 >/dev/null 2>&1 || true
+          {
+            echo '## Maintainability ranking'
+            echo '```'
+            wily rank dataretrieval maintainability.mi 2>&1 | head -25 \
+              || echo 'wily index unavailable'
+            echo '```'
+          } > wily-summary.txt
+
+      - name: Publish report
+        # ``always()`` so a partial report still lands rather than a bare red X.
+        if: always()
+        run: |
+          {
+            echo '## Structural analysis'
+            echo '```'
+            cat pyscn-summary.txt 2>/dev/null || echo 'pyscn produced no output'
+            cat pyscn-sanity.txt 2>/dev/null || true
+            echo '```'
+            cat wily-summary.txt 2>/dev/null || true
+            echo
+            echo 'Full reports are attached to this run as the'
+            echo '`code-health-report` artifact. Findings are advisory --'
+            echo 'read them as leads, not verdicts (see CONTRIBUTING.md).'
+            echo 'The merge gates live in the Python package workflow.'
+          } >> "$GITHUB_STEP_SUMMARY"
+
+      - name: Upload reports
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: code-health-report
+          path: |
+            .pyscn/reports/
+            pyscn-summary.txt
+            wily-summary.txt
+          retention-days: 90
+          if-no-files-found: warn

.github/workflows/python-package.yml

@@ -41,15 +41,18 @@ jobs:
           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.
+        # Versions pinned in the extra so CI and pre-commit grade identically.
         run: pip install -e .[metrics]
+      # Ratchets, not aspirations: thresholds sit where the package already is,
+      # so they fail on regression rather than demanding a refactor. Each
+      # mirrors a pre-commit hook, so a clean local run means CI agrees.
       - 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: Cognitive complexity gate
+        run: complexipy --max-complexity-allowed 27 --failed dataretrieval
+      - name: Dependency-direction contracts
+        # Rules and rationale live in .importlinter and the ADRs it cites.
+        run: lint-imports
       - 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 --

.gitignore

@@ -115,3 +115,6 @@ ENV/
 
 # wily metrics cache (rebuildable: `wily build dataretrieval`)
 .wily/
+
+# pyscn analysis reports (rebuildable: `pyscn analyze dataretrieval`)
+.pyscn/

.importlinter

@@ -0,0 +1,132 @@
+; Declarative dependency-direction contracts for ``dataretrieval``.
+;
+; This file owns every rule that is purely a statement about the import graph.
+; ``tests/architecture_test.py`` owns the rules that are not: symbol-level
+; claims, ``__all__`` surfaces, AST shape, and the one boundary that must be
+; asserted positively. The split is deliberate and the two do not overlap --
+; a rule enforced twice is a rule that gets updated once.
+;
+; Run with ``lint-imports`` (installed by the ``[metrics]`` extra).
+
+[importlinter]
+root_package = dataretrieval
+; Contracts describe what runs, matching the AST suite. ``ogc.interruptions``
+; and ``ogc.chunking`` reference each other's types under ``TYPE_CHECKING``;
+; that is a documentation edge, not a runtime one, and no cycle exists at
+; import time.
+exclude_type_checking_imports = True
+
+[importlinter:contract:layers]
+name = Dependencies point toward stable policy (ADR 0003, ADR 0006)
+type = layers
+containers =
+    dataretrieval
+layers =
+    ngwmn | nldi | nwis | streamstats | waterdata | wateruse | wqp
+    ogc
+    utils
+    transport
+    progress
+    codes | combining | rdb | response_metadata
+    credentials
+    exceptions
+; Every top-level module must be placed in the stack deliberately. A new
+; top-level module fails this contract until someone decides where it sits.
+exhaustive = True
+exhaustive_ignores =
+    _version
+
+[importlinter:contract:acyclic]
+name = The runtime import graph is acyclic (ADR 0001)
+type = acyclic_siblings
+; Applied to the whole package, not just the two subsystems whose acyclicity
+; was previously asserted by hand. The claim held package-wide already; stating
+; it once at the root covers ``ogc`` and ``transport`` and every future
+; subpackage without a new rule per package.
+ancestors =
+    dataretrieval
+
+[importlinter:contract:ogc-consumers]
+name = Only NGWMN and Water Data consume the OGC subsystem (ADR 0003)
+type = protected
+; Read as: nothing outside this list may reach into OGC, directly or through a
+; submodule. An allowlist rather than a denylist, so a new service module is
+; refused by default instead of being silently permitted until someone
+; remembers to add it.
+;
+; The root ``dataretrieval`` package is deliberately NOT an allowed importer.
+; ``allowed_importers`` is matched with ``as_packages``, so naming the root here
+; would make every module in the distribution an allowed importer and the
+; contract could never fail. Its two real imports are listed as explicit
+; exceptions below instead -- narrow, visible, and they fail if they go stale.
+protected_modules =
+    dataretrieval.ogc
+allowed_importers =
+    dataretrieval.ngwmn
+    dataretrieval.waterdata
+ignore_imports =
+; The package __init__ re-exports the resumable-call and interruption types;
+; they are part of the documented public surface, not a service reaching in.
+    dataretrieval -> dataretrieval.ogc.chunking
+    dataretrieval -> dataretrieval.ogc.interruptions
+
+[importlinter:contract:ogc-facade]
+name = NGWMN consumes the OGC facade only, never its internals (ADR 0007)
+type = forbidden
+source_modules =
+    dataretrieval.ngwmn
+; The wildcard is what makes this durable: a new ``ogc`` submodule is covered
+; the day it is added, without editing this contract.
+forbidden_modules =
+    dataretrieval.ogc.**
+; Direct imports only. Reaching an internal *through* the facade is the design,
+; not a violation -- ``ngwmn -> ogc -> ogc.engine`` is how the seam is supposed
+; to work, and the default (indirect included) forbids exactly that.
+allow_indirect_imports = True
+
+[importlinter:contract:ogc-internal-seams]
+name = OGC internal seams (ADR 0003)
+type = forbidden
+source_modules =
+; Feature shaping is downstream of execution. If shaping imports engine the
+; subsystem gains a cycle and the schema fetch on an empty frame becomes
+; reachable from request construction.
+    dataretrieval.ogc.shaping
+forbidden_modules =
+    dataretrieval.ogc.engine
+
+[importlinter:contract:nwis-quarantine]
+name = Deprecated NWIS has no dependents (ADR 0005)
+type = forbidden
+source_modules =
+    dataretrieval.codes
+    dataretrieval.combining
+    dataretrieval.credentials
+    dataretrieval.exceptions
+    dataretrieval.ngwmn
+    dataretrieval.nldi
+    dataretrieval.ogc
+    dataretrieval.progress
+    dataretrieval.rdb
+    dataretrieval.streamstats
+    dataretrieval.transport
+    dataretrieval.utils
+    dataretrieval.waterdata
+    dataretrieval.wateruse
+    dataretrieval.wqp
+forbidden_modules =
+    dataretrieval.nwis
+
+[importlinter:contract:waterdata-families]
+name = Water Data collection families do not reach through each other (ADR 0007)
+type = independence
+; Kept in step with ``_WATERDATA_FAMILIES`` in tests/architecture_test.py, which
+; derives the facade's expected export union from the same six modules. A
+; seventh family fails that test until it is listed there; add it here too.
+modules =
+    dataretrieval.waterdata.cql
+    dataretrieval.waterdata.measurements
+    dataretrieval.waterdata.metadata
+    dataretrieval.waterdata.reference
+    dataretrieval.waterdata.samples
+    dataretrieval.waterdata.time_series

.pre-commit-config.yaml

@@ -63,6 +63,40 @@ repos:
         files: ^dataretrieval/.*\.py$
         pass_filenames: false
 
+  # Cognitive-complexity ratchet, the counterpart to xenon's cyclomatic one.
+  # The two measure different things: xenon counts branches, complexipy counts
+  # how hard the control flow is to hold in your head, so nesting and early
+  # exits weigh differently. 27 is the package's current maximum
+  # (``nldi.get_features``), so this holds the line rather than demanding a
+  # refactor. Unlike xenon this one is per-function, so scanning only the
+  # changed files gives the same verdict CI gives for the whole package.
+  - repo: https://github.com/rohaquinlop/complexipy-pre-commit
+    rev: v6.2.0
+    hooks:
+      - id: complexipy
+        args: ["--max-complexity-allowed", "27", "--failed"]
+        files: ^dataretrieval/.*\.py$
+
+  # Dependency-direction contracts (ADR 0003, 0005, 0006, 0007). Complements
+  # tests/architecture_test.py, which enforces named pairwise claims by parsing
+  # the AST; this checks the whole layer stack over the *transitive* graph, so
+  # a service reaching transport through OGC fails here. Runs in about a
+  # second. Declared as a local hook rather than using the upstream one, which
+  # is ``language: system`` and so needs lint-imports already on PATH.
+  - repo: local
+    hooks:
+      - id: import-linter
+        name: import-linter
+        description: Enforce the dependency layers declared in .importlinter
+        entry: lint-imports
+        language: python
+        additional_dependencies: ["import-linter==2.13"]
+        # Reads the package tree from the repo root, not a file list; grimp
+        # resolves ``dataretrieval`` relative to the working directory, so the
+        # hook needs neither the package nor its runtime deps installed.
+        pass_filenames: false
+        files: ^(dataretrieval/.*\.py|\.importlinter)$
+
   # 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