Refactor within ruff's complexity limits, then select every ruff rule (ALL)

.github/scripts/update_policyengine_package.py

@@ -10,7 +10,7 @@
 import tempfile
 from collections.abc import Mapping
 from pathlib import Path
-from typing import Any
+from typing import Any, NamedTuple
 
 import tomllib
 
@@ -201,6 +201,56 @@ def latest_version(package: str) -> None:
     _write_result(version)
 
 
+def _package_version(packages: Mapping[str, Any], name: str) -> str:
+    version = _mapping(packages.get(name)).get("version")
+    if not isinstance(version, str) or not version:
+        msg = f"Bundle has no version for {name}"
+        raise SystemExit(msg)
+    return version
+
+
+def _data_release_version(release: Mapping[str, Any], country: str) -> str:
+    version = release.get("version") or _mapping(release.get("data_package")).get(
+        "version"
+    )
+    if not isinstance(version, str) or not version:
+        msg = f"Bundle has no data release version for {country}"
+        raise SystemExit(msg)
+    return version
+
+
+def _data_package_version(release: Mapping[str, Any]) -> str:
+    version = _mapping(release.get("data_package")).get("version")
+    return version if isinstance(version, str) else ""
+
+
+def _dataset_sha256(release: Mapping[str, Any], country: str) -> str:
+    """Return the default dataset's digest, preferring the certified artifact's."""
+    default = release.get("default_dataset")
+    certified = _mapping(release.get("certified_data_artifact"))
+    if certified.get("dataset") == default and certified.get("sha256"):
+        value = certified["sha256"]
+    else:
+        reference = _mapping(_mapping(release.get("datasets")).get(default))
+        value = reference.get("sha256") or ""
+    digest = str(value).strip().lower()
+    if digest and not re.fullmatch(r"[0-9a-f]{64}", digest):
+        msg = f"Bundle records a malformed {country} dataset SHA-256"
+        raise SystemExit(msg)
+    return digest
+
+
+def _certificate_sha256(release: object) -> str:
+    # Receipt-v1 certificate digest; older wrappers lack the helper.
+    try:
+        from policyengine.provenance.receipt import (
+            certificate_sha256 as digest,
+        )
+    except ImportError:
+        return ""
+    return digest(release) if isinstance(release, Mapping) else ""
+
+
 def bundle_versions() -> None:
     """Print the package and data versions selected by PolicyEngine.py."""
     from policyengine.bundle import (
@@ -210,67 +260,20 @@ def bundle_versions() -> None:
     bundle = _mapping(get_current_bundle())
     packages = _mapping(bundle.get("packages"))
     data_releases = _mapping(bundle.get("data_releases"))
-
-    def package_version(name: str) -> str:
-        version = _mapping(packages.get(name)).get("version")
-        if not isinstance(version, str) or not version:
-            msg = f"Bundle has no version for {name}"
-            raise SystemExit(msg)
-        return version
-
-    def data_release_version(country: str) -> str:
-        release = _mapping(data_releases.get(country))
-        version = release.get("version") or _mapping(release.get("data_package")).get(
-            "version"
-        )
-        if not isinstance(version, str) or not version:
-            msg = f"Bundle has no data release version for {country}"
-            raise SystemExit(msg)
-        return version
-
-    def data_package_version(country: str) -> str:
-        release = _mapping(data_releases.get(country))
-        version = _mapping(release.get("data_package")).get("version")
-        return version if isinstance(version, str) else ""
-
-    def dataset_sha256(country: str) -> str:
-        release = _mapping(data_releases.get(country))
-        default = release.get("default_dataset")
-        certified = _mapping(release.get("certified_data_artifact"))
-        if certified.get("dataset") == default and certified.get("sha256"):
-            value = certified["sha256"]
-        else:
-            reference = _mapping(_mapping(release.get("datasets")).get(default))
-            value = reference.get("sha256") or ""
-        digest = str(value).strip().lower()
-        if digest and not re.fullmatch(r"[0-9a-f]{64}", digest):
-            msg = f"Bundle records a malformed {country} dataset SHA-256"
-            raise SystemExit(msg)
-        return digest
-
-    def certificate_sha256(country: str) -> str:
-        # Receipt-v1 certificate digest; older wrappers lack the helper.
-        try:
-            from policyengine.provenance.receipt import (
-                certificate_sha256 as digest,
-            )
-        except ImportError:
-            return ""
-        release = data_releases.get(country)
-        return digest(release) if isinstance(release, Mapping) else ""
-
+    us_release = _mapping(data_releases.get("us"))
+    uk_release = _mapping(data_releases.get("uk"))
     outputs = {
-        "policyengine_version": package_version("policyengine"),
-        "policyengine_core_version": package_version("policyengine-core"),
-        "spm_calculator_version": package_version("spm-calculator"),
-        "us_version": package_version("policyengine-us"),
-        "us_data_version": data_release_version("us"),
-        "uk_version": package_version("policyengine-uk"),
-        "uk_data_version": data_release_version("uk"),
-        "uk_data_package_version": data_package_version("uk"),
-        "us_dataset_sha256": dataset_sha256("us"),
-        "uk_dataset_sha256": dataset_sha256("uk"),
-        "uk_certificate_sha256": certificate_sha256("uk"),
+        "policyengine_version": _package_version(packages, "policyengine"),
+        "policyengine_core_version": _package_version(packages, "policyengine-core"),
+        "spm_calculator_version": _package_version(packages, "spm-calculator"),
+        "us_version": _package_version(packages, "policyengine-us"),
+        "us_data_version": _data_release_version(us_release, "us"),
+        "uk_version": _package_version(packages, "policyengine-uk"),
+        "uk_data_version": _data_release_version(uk_release, "uk"),
+        "uk_data_package_version": _data_package_version(uk_release),
+        "us_dataset_sha256": _dataset_sha256(us_release, "us"),
+        "uk_dataset_sha256": _dataset_sha256(uk_release, "uk"),
+        "uk_certificate_sha256": _certificate_sha256(data_releases.get("uk")),
     }
     for name, version in outputs.items():
         _write_result(f"{name}={version}")
@@ -293,25 +296,30 @@ def certificate_sha256(country: str) -> str:
 PROMOTABLE_VERDICTS = frozenset({"qualified", "needs_review"})
 
 
-def verify_dispatch() -> None:
-    """Check a policyengine-release payload against the installed bundle.
+class _DispatchComparison(NamedTuple):
+    """What comparing a dispatch with the installed bundle found.
 
-    Reads the dispatch ``client_payload`` JSON from ``RELEASE_PAYLOAD`` and
-    ``bundle-versions`` output from stdin. A payload without ``bundle.uk``
-    (older releases, manual runs) is accepted. A field the payload names
-    must equal the installed bundle's, and an installed bundle that cannot
-    report it is a mismatch, except for the certificate digest, which older
-    wrappers cannot compute. A qualification that says the pair was not
-    promotable is refused. ``dispatch_verified`` is true only when at least
-    one field was compared. Prints digests for the PR body.
+    ``mismatches`` lists the fields that disagree; ``compared`` counts the
+    fields both the dispatch and the installed bundle stated.
     """
-    raw = os.environ.get("RELEASE_PAYLOAD", "").strip()
-    payload = json.loads(raw) if raw and raw != "null" else {}
+
+    mismatches: list[str]
+    compared: int
+
+
+def _installed_fields(bundle_output: str) -> dict[str, str]:
+    """Read ``bundle-versions`` output: one ``key=value`` line per field."""
     installed = {}
-    for line in sys.stdin.read().splitlines():
+    for line in bundle_output.splitlines():
         key, _, value = line.partition("=")
         if key:
             installed[key.strip()] = value.strip()
+    return installed
+
+
+def _require_dispatched_wrapper(
+    payload: Mapping[str, Any], installed: Mapping[str, str]
+) -> None:
     if payload.get("version") and payload["version"] != installed.get(
         "policyengine_version"
     ):
@@ -320,7 +328,12 @@ def verify_dispatch() -> None:
             f"{installed.get('policyengine_version')} is installed."
         )
         raise SystemExit(msg)
-    expected = _mapping(_mapping(payload.get("bundle")).get("uk"))
+
+
+def _compare_dispatch_bundle(
+    expected: Mapping[str, Any], installed: Mapping[str, str]
+) -> _DispatchComparison:
+    """Compare each field the dispatch names with the installed bundle's value."""
     mismatches = []
     compared = 0
     for payload_key, installed_key in DISPATCH_CHECKS:
@@ -341,11 +354,11 @@ def verify_dispatch() -> None:
             claimed_text = claimed_text.lower()
         if claimed_text != actual:
             mismatches.append(f"{payload_key}: dispatch {claimed}, installed {actual}")
-    if mismatches:
-        raise SystemExit(
-            "The release dispatch names a different UK bundle from the installed "
-            "wheel: " + "; ".join(mismatches)
-        )
+    return _DispatchComparison(mismatches, compared)
+
+
+def _promotable_qualification(payload: Mapping[str, Any]) -> tuple[str, str]:
+    """Return the UK verdict and receipt digest, refusing malformed or rejected ones."""
     qualification = _mapping(_mapping(payload.get("qualification")).get("uk"))
     verdict = qualification.get("verdict") or ""
     receipt_sha256 = qualification.get("receipt_sha256") or ""
@@ -362,7 +375,35 @@ def verify_dispatch() -> None:
     ):
         msg = f"The release dispatch says the UK pair was not promotable ({verdict})"
         raise SystemExit(msg)
-    _write_result(f"dispatch_verified={'true' if compared else 'false'}")
+    return verdict, receipt_sha256
+
+
+def verify_dispatch() -> None:
+    """Check a policyengine-release payload against the installed bundle.
+
+    Reads the dispatch ``client_payload`` JSON from ``RELEASE_PAYLOAD`` and
+    ``bundle-versions`` output from stdin. A payload without ``bundle.uk``
+    (older releases, manual runs) is accepted. A field the payload names
+    must equal the installed bundle's, and an installed bundle that cannot
+    report it is a mismatch, except for the certificate digest, which older
+    wrappers cannot compute. A qualification that says the pair was not
+    promotable is refused. ``dispatch_verified`` is true only when at least
+    one field was compared. Prints digests for the PR body.
+    """
+    raw = os.environ.get("RELEASE_PAYLOAD", "").strip()
+    payload = json.loads(raw) if raw and raw != "null" else {}
+    installed = _installed_fields(sys.stdin.read())
+    _require_dispatched_wrapper(payload, installed)
+    comparison = _compare_dispatch_bundle(
+        _mapping(_mapping(payload.get("bundle")).get("uk")), installed
+    )
+    if comparison.mismatches:
+        raise SystemExit(
+            "The release dispatch names a different UK bundle from the installed "
+            "wheel: " + "; ".join(comparison.mismatches)
+        )
+    verdict, receipt_sha256 = _promotable_qualification(payload)
+    _write_result(f"dispatch_verified={'true' if comparison.compared else 'false'}")
     _write_result(f"uk_qualification_verdict={verdict}")
     _write_result(f"uk_qualification_receipt_sha256={receipt_sha256}")
 

AGENTS.md

@@ -146,7 +146,7 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`.
 - `sim.quality.every-project-tested`: Every project and library with unit tests runs them in the PR test matrix. (doc-only, not checked: follow it yourself)
 - `sim.quality.ruff-format-src`: Source code under projects/*/src and libs/*/src is formatted with ruff format. (blocking: ruff format)
 - `sim.quality.ruff-format-tests`: Test code is formatted with ruff format too; CI currently checks only src directories. (doc-only, not checked: follow it yourself)
-- `sim.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in ruff.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (A, ANN, ARG, ASYNC, B, BLE, C4, D, DTZ, E, E4, E7, E9, EM, ERA, EXE, F, FAST, FBT, FIX, FLY, FURB, G, I, INP, LOG, N, PD, PERF, PGH, PIE, PLC, PLW, PT, PTH, PYI, RET, RSE, RUF, S, SIM, SLF, T20, TC, TD, TID253, TRY, UP, W))
+- `sim.quality.ruff-lint`: All Python code passes ruff check with every ruff rule selected (ALL), with no ratchet, except the ignores in ruff.toml: COM812 and ISC001, which conflict with ruff format, and D203 and D213, which conflict with the kept D211 and D212. (blocking: ruff check (ALL))
 - `sim.quality.runtime-evaluated-models-resolve`: Every Pydantic model is runtime-evaluated for ruff: one of its bases, read through the imports of the model's own module, is listed in ruff.toml's runtime-evaluated-base-classes or is a runtime-evaluated class in that same module; and every Pydantic model resolves its annotations once its package is imported. (blocking: pytest `projects/policyengine-simulation-entry/tests/test_runtime_evaluated_annotations.py`, `projects/policyengine-simulation-executor/tests/test_runtime_evaluated_annotations.py`)
 - `sim.quality.src-programs-ruff-exemptions`: Programs under src (the Modal deployment utilities, and the OpenAPI and Stage 12 infrastructure commands) may print their output (T201), open URLs from their arguments (S310) and, for the Modal utilities run with python -m, live in a directory that isn't a package (INP001); every other ruff rule applies to them. (blocking: ruff per-file ignore of INP001, S310, T201 for `projects/*/src/{modal/utils/*,policyengine_simulation_entry/generate_openapi,policyengine_simulation_entry/stage12_infrastructure,policyengine_simulation_gateway/generate_openapi}.py`)
 - `sim.quality.tests-pass`: Each project's unit tests pass in its own locked environment. (blocking: CI job `test`)

changelog_entry.yaml

@@ -5,6 +5,7 @@
       - Added a rules registry (rules/registry.toml) with a standard-library tool that validates it, checks every rule is enforced where it claims, and renders docs/generated/rules.md and the Rules block in AGENTS.md
       - Ran Stage 12 US national simulations as 20 region-group calls each, and built an independent, immutable Stage 12 cache of three annual US datasets and 60 current-law baselines before each deployment, publishing it only after a recorded determinism check and recording its release in the v2 version manifest under a new key, while the existing key keeps a copy that earlier readers accept
     changed:
+      - Enabled every ruff rule (select ALL), ignoring only COM812 and ISC001, which conflict with ruff format, and the D203 and D213 halves of the mutually exclusive docstring pairs; complex functions, long parameter lists and magic numbers were split into smaller helpers and grouped parameters, each behind new characterization tests, across the contract and persistence validation, entry settings, app factory and Stage 12 adapter, gateway route resolution, Stage 12 coordination, workers, parity and qualification, the precompute and national reduce, the routing publisher and its command line, the release sentinel, the package update helper and the shared rules tool; Modal function, FastAPI route, Pydantic validator and command-line signatures and every OpenAPI document are unchanged; the contract library's batch-state builder and SPM selection resolver take their route values as one grouped argument, the simulation observability initializer drops a service_version parameter no caller passed, and no PLR0913 suppression is needed
       - Enabled ruff's argument, idiom and pytest-style rules (ARG, ASYNC, B, C4, FBT, PD, PERF, PGH, PLW, PT, PTH, RUF, SIM); unused arguments are removed, underscored or explained, FastAPI auth dependencies use Annotated, re-raised errors keep their cause, and boolean options of in-repo helpers are keyword-only; OpenAPI documents are unchanged, and the shared rules tool's unittest tests are exempt from pytest-style rules; tests take the unused-argument rules (ARG001, ARG002, ARG005) as file-level exceptions, since their fakes and stubs mirror real signatures; best-effort telemetry and cleanup calls that swallow any error use contextlib.suppress, with the reason as a comment
       - Enabled ruff's structure and import rules (INP, PLC, SLF, N, A, FAST, E); long lines are wrapped, deferred imports that Modal images, test seams, import cycles or heavy dependencies need carry a reason, script directories are declared to ruff as namespace packages, four internal exceptions gained an Error suffix, and published field names, OpenAPI documents and logged error types are unchanged; tests are also exempt from SLF001, INP001 and PLC0415; heavy or optional packages that load only inside functions are listed in ruff's banned-module-level-imports, which keeps them out of module-level imports (TID253) and lets their deferred imports go without a suppression, and the Modal utility programs take INP001 file-wide
       - Enabled ruff's security, logging and debugging rules (S, T20, LOG, G, DTZ, ERA, TD, FIX, EXE); asserts outside tests became explicit checks, command-line scripts keep their printed output, test subprocess calls go through helpers, and every deliberate exception carries a reasoned suppression; the gateway's auth misconfiguration log no longer repeats its message, and the deployed-service tests share the unit-test ruff exemptions; tests, CI/CD scripts and command-line scripts take their deliberate exceptions as registered file-level ruff ignores instead of inline suppressions, and the rules guard rejects file-level ignores anywhere else

docs/engineering/skills/rules-registry.md

@@ -115,10 +115,13 @@ fail the guard yet.
 
 ## Baselines
 
-- **Ruff is never ratcheted.** Adopting a ruff rule family is one standalone PR
-  that changes the select list and fixes every resulting violation. Don't use
-  `--add-noqa`. A finding that is kept gets an inline `# noqa: CODE  # reason`,
-  except in the files below.
+- **Ruff is never ratcheted.** `ruff.toml` selects `ALL`, so every ruff rule
+  applies except the four listed in `ignore` (two formatter conflicts and one
+  of each mutually exclusive docstring pair) and the registered
+  [file-level exceptions](#file-level-exceptions). When a ruff upgrade adds
+  rules, the upgrade PR fixes every new violation. Don't use `--add-noqa`.
+  Outside the files that allow file-level exceptions, a justified exception is
+  an inline `# noqa: CODE  # reason`.
 - Other enforcers (mypy, Pyright, Import Linter, custom guards) may start
   `ratcheted` with a baseline file of tolerated violations. Baselines may only
   shrink: the guard compares counts against the base branch.