Enable ruff argument, idiom and pytest-style rules (ARG, B, C4, FBT, PERF, PLW, PT, PTH, RUF, SIM and others)

.github/scripts/update_policyengine_package.py

@@ -83,8 +83,10 @@ def update_requirements(
         ),
     }
 
-    for location, dependencies in requirements.items():
-        dependencies = dependencies if isinstance(dependencies, list) else []
+    for location, declared_dependencies in requirements.items():
+        dependencies = (
+            declared_dependencies if isinstance(declared_dependencies, list) else []
+        )
         matches = [
             dependency
             for dependency in dependencies
@@ -140,7 +142,7 @@ def update_requirements(
     ) as temporary:
         temporary.write(text.replace(old_pin, new_pin))
         temporary_path = temporary.name
-    os.replace(temporary_path, pyproject)
+    Path(temporary_path).replace(pyproject)
 
 
 def find_issue(title: str) -> None:

AGENTS.md

@@ -139,9 +139,10 @@ 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, BLE, D, DTZ, E, E4, E7, E9, EM, ERA, EXE, F, FAST, FIX, FLY, FURB, G, I, INP, LOG, N, PIE, PLC, PYI, RET, RSE, S, SLF, T20, TC, TD, TRY, UP, W))
+- `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, TRY, UP, W))
 - `sim.quality.tests-pass`: Each project's unit tests pass in its own locked environment. (blocking: CI job `test`)
 - `sim.quality.tests-ruff-exemptions`: Test files may use pytest's bare assert, need no docstrings, annotations or named constants for magic numbers, and may reach into private members, live outside packages and import inside a test (S101, D1, ANN, PLR2004, SLF001, INP001, PLC0415); every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, INP001, PLC0415, PLR2004, S101, SLF001 for `**/tests/**`)
+- `sim.quality.unittest-ruff-exemptions`: The shared rules tool's tests use unittest, because the tool needs only the standard library, so ruff's pytest-style rules (PT) don't apply to them. (blocking: ruff per-file ignore of PT for `scripts/rules/tests/**`)
 
 ### CI workflows and scripts
 

changelog_entry.yaml

@@ -4,6 +4,7 @@
       - Made CLAUDE.md and the Copilot instructions thin pointers to AGENTS.md, required workflow logic longer than one line to live in tested and shellchecked scripts, and registered the tracking-issue rule
       - 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
     changed:
+      - 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
       - 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
       - 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
       - Enabled ruff's exception rules (TRY, EM, BLE); exception messages are built before the raise, raises inside try blocks moved into helpers, and every deliberate catch-all or traceback-free error log carries a reasoned suppression; message text is unchanged, and error types change only in an operator script's two type checks

docs/generated/rules.md

@@ -8,11 +8,11 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf
 
 | Tier | enforced | ratcheted | advisory | doc-only | Total |
 | --- | ---: | ---: | ---: | ---: | ---: |
-| mechanical | 34 | 1 | 0 | 14 | 49 |
+| mechanical | 35 | 1 | 0 | 14 | 50 |
 | metric | 0 | 0 | 0 | 0 | 0 |
 | judgment | 0 | 0 | 0 | 0 | 0 |
 
-**Blocking:** 35 of 49 rules.
+**Blocking:** 36 of 50 rules.
 
 ## Gaps
 
@@ -246,9 +246,10 @@ flowchart LR
 | `sim.quality.every-project-tested` | Every project and library with unit tests runs them in the PR test matrix. | mechanical | doc-only | no | nothing | — | — |
 | `sim.quality.ruff-format-src` | Source code under projects/*/src and libs/*/src is formatted with ruff format. | mechanical | enforced | yes | ruff format | `pr.yml:lint` | — |
 | `sim.quality.ruff-format-tests` | Test code is formatted with ruff format too; CI currently checks only src directories. | mechanical | doc-only | no | nothing | — | — |
-| `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. | mechanical | enforced | yes | ruff check (A, ANN, BLE, D, DTZ, E, E4, E7, E9, EM, ERA, EXE, F, FAST, FIX, FLY, FURB, G, I, INP, LOG, N, PIE, PLC, PYI, RET, RSE, S, SLF, T20, TC, TD, TRY, UP, W) | `pr.yml:lint` | — |
+| `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. | mechanical | enforced | yes | 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, TRY, UP, W) | `pr.yml:lint` | — |
 | `sim.quality.tests-pass` | Each project's unit tests pass in its own locked environment. | mechanical | enforced | yes | CI job `test` | `pr.yml:test` | — |
 | `sim.quality.tests-ruff-exemptions` | Test files may use pytest's bare assert, need no docstrings, annotations or named constants for magic numbers, and may reach into private members, live outside packages and import inside a test (S101, D1, ANN, PLR2004, SLF001, INP001, PLC0415); every other ruff rule applies to tests. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, INP001, PLC0415, PLR2004, S101, SLF001 for `**/tests/**` | `pr.yml:lint` | — |
+| `sim.quality.unittest-ruff-exemptions` | The shared rules tool's tests use unittest, because the tool needs only the standard library, so ruff's pytest-style rules (PT) don't apply to them. | mechanical | enforced | yes | ruff per-file ignore of PT for `scripts/rules/tests/**` | `pr.yml:lint` | — |
 
 ```mermaid
 flowchart LR
@@ -261,24 +262,28 @@ flowchart LR
   e_ruff_format["ruff format"]
   r_sim_quality_ruff_format_tests("sim.quality.ruff-format-tests (doc-only)")
   r_sim_quality_ruff_lint("sim.quality.ruff-lint (enforced)")
-  e_ruff_check__A__ANN__BLE__D__DTZ__E__E4__E7__E9__EM__ERA__EXE__F__FAST__FIX__FLY__FURB__G__I__INP__LOG__N__PIE__PLC__PYI__RET__RSE__S__SLF__T20__TC__TD__TRY__UP__W_["ruff check (A, ANN, BLE, D, DTZ, E, E4, E7, E9, EM, ERA, EXE, F, FAST, FIX, FLY, FURB, G, I, INP, LOG, N, PIE, PLC, PYI, RET, RSE, S, SLF, T20, TC, TD, TRY, UP, W)"]
+  e_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__TRY__UP__W_["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, TRY, UP, W)"]
   r_sim_quality_tests_pass("sim.quality.tests-pass (enforced)")
   e_CI_job_test["CI job test"]
   j_CI_pr_yml_test["CI pr.yml:test"]
   r_sim_quality_tests_ruff_exemptions("sim.quality.tests-ruff-exemptions (enforced)")
   e_ruff_per_file_ignore_of_ANN__D1__INP001__PLC0415__PLR2004__S101__SLF001_for____tests___["ruff per-file ignore of ANN, D1, INP001, PLC0415, PLR2004, S101, SLF001 for **/tests/**"]
+  r_sim_quality_unittest_ruff_exemptions("sim.quality.unittest-ruff-exemptions (enforced)")
+  e_ruff_per_file_ignore_of_PT_for_scripts_rules_tests___["ruff per-file ignore of PT for scripts/rules/tests/**"]
   r_sim_quality_deployed_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__INP001__PLC0415__PLR2004__S101__SLF001_for____authenticated_tests___
   e_ruff_per_file_ignore_of_ANN__D1__INP001__PLC0415__PLR2004__S101__SLF001_for____authenticated_tests___ --> j_CI_pr_yml_lint
   r_sim_quality_every_project_tested -.-> gap_unenforced
   r_sim_quality_ruff_format_src --> e_ruff_format
   e_ruff_format --> j_CI_pr_yml_lint
   r_sim_quality_ruff_format_tests -.-> gap_unenforced
-  r_sim_quality_ruff_lint --> e_ruff_check__A__ANN__BLE__D__DTZ__E__E4__E7__E9__EM__ERA__EXE__F__FAST__FIX__FLY__FURB__G__I__INP__LOG__N__PIE__PLC__PYI__RET__RSE__S__SLF__T20__TC__TD__TRY__UP__W_
-  e_ruff_check__A__ANN__BLE__D__DTZ__E__E4__E7__E9__EM__ERA__EXE__F__FAST__FIX__FLY__FURB__G__I__INP__LOG__N__PIE__PLC__PYI__RET__RSE__S__SLF__T20__TC__TD__TRY__UP__W_ --> j_CI_pr_yml_lint
+  r_sim_quality_ruff_lint --> e_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__TRY__UP__W_
+  e_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__TRY__UP__W_ --> j_CI_pr_yml_lint
   r_sim_quality_tests_pass --> e_CI_job_test
   e_CI_job_test --> j_CI_pr_yml_test
   r_sim_quality_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__INP001__PLC0415__PLR2004__S101__SLF001_for____tests___
   e_ruff_per_file_ignore_of_ANN__D1__INP001__PLC0415__PLR2004__S101__SLF001_for____tests___ --> j_CI_pr_yml_lint
+  r_sim_quality_unittest_ruff_exemptions --> e_ruff_per_file_ignore_of_PT_for_scripts_rules_tests___
+  e_ruff_per_file_ignore_of_PT_for_scripts_rules_tests___ --> j_CI_pr_yml_lint
 ```
 
 ## CI workflows and scripts

libs/policyengine-fastapi/src/policyengine_fastapi/auth/jwt_decoder.py

@@ -1,6 +1,7 @@
 """FastAPI dependency that verifies RS256 bearer JWTs against an issuer's JWKS."""
 
 import logging
+from typing import Annotated
 
 import jwt
 from fastapi import Depends, HTTPException, status
@@ -20,7 +21,7 @@ class JWTDecoder:
     or yields None when ``auto_error`` is False.
     """
 
-    def __init__(self, issuer: str, audience: str, auto_error: bool = True) -> None:
+    def __init__(self, issuer: str, audience: str, *, auto_error: bool = True) -> None:
         """Store the expected claims and create a JWKS client for the issuer.
 
         Keys are fetched from ``{issuer}.well-known/jwks.json``, so ``issuer``
@@ -34,9 +35,9 @@ def __init__(self, issuer: str, audience: str, auto_error: bool = True) -> None:
 
     def __call__(
         self,
-        token: HTTPAuthorizationCredentials | None = Depends(
-            HTTPBearer(auto_error=False)
-        ),
+        token: Annotated[
+            HTTPAuthorizationCredentials | None, Depends(HTTPBearer(auto_error=False))
+        ],
     ) -> dict[str, str] | None:
         """Return the verified token's claims.
 
@@ -64,10 +65,10 @@ def _validate(self, token: HTTPAuthorizationCredentials | None) -> dict[str, str
             ).key
         except jwt.exceptions.PyJWKClientError as error:
             LOG.error("Unable to get public key for JWT validation: %s", error)  # noqa: TRY400  # callers control the token, so a traceback per bad token would let them flood the logs
-            raise HTTPException(status_code=status.HTTP_403_FORBIDDEN)
+            raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) from error
         except jwt.exceptions.DecodeError as error:
             LOG.error("Unable to get public key for JWT validation: %s", error)  # noqa: TRY400  # callers control the token, so a traceback per bad token would let them flood the logs
-            raise HTTPException(status_code=status.HTTP_403_FORBIDDEN)
+            raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) from error
 
         try:
             payload = jwt.decode(
@@ -77,7 +78,8 @@ def _validate(self, token: HTTPAuthorizationCredentials | None) -> dict[str, str
                 audience=self.audience,
                 issuer=self.issuer,
             )
-        except Exception as error:  # noqa: BLE001  # any decode or claim-validation failure must become a 403, never a 500
+        # Any decode or claim-validation failure must become a 403, never a 500.
+        except Exception as error:
             LOG.info("Invalid bearer token: %s", error)
-            raise HTTPException(status_code=status.HTTP_403_FORBIDDEN)
+            raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) from error
         return payload

libs/policyengine-fastapi/src/policyengine_fastapi/health/__init__.py

@@ -41,7 +41,7 @@ def __init__(self, name: str, probes: dict[str, HealthProbe]) -> None:
 
     def report(self) -> SystemStatus:
         """Run every probe; the system is healthy only if all of its probes are."""
-        detail = [self.probes[key]() for key in self.probes.keys()]
+        detail = [self.probes[key]() for key in self.probes]
         first_unhealthy = next((d for d in detail if d.healthy is not True), None)
         return SystemStatus(
             name=self.name, healthy=first_unhealthy is None, detail=detail
@@ -67,7 +67,7 @@ def register(self, reporter: HealthSystemReporter) -> None:
 
     def report(self) -> HealthStatus:
         """Report every system; the service is healthy only if all systems are."""
-        system_status = [self.systems[key].report() for key in self.systems.keys()]
+        system_status = [self.systems[key].report() for key in self.systems]
         first_unhealthy = next(
             (s for s in system_status if s.healthy is not True), None
         )

libs/policyengine-fastapi/src/policyengine_fastapi/observability/emitters.py

@@ -105,7 +105,7 @@ def emit_histogram(
     def record_artifact_manifest(self, manifest: TracerArtifactManifest) -> None:
         """Discard the manifest."""
 
-    def span(self, name: str, attributes: Mapping[str, Any] | None = None) -> NoOpSpan:
+    def span(self, name: str, attributes: Mapping[str, Any] | None = None) -> NoOpSpan:  # noqa: ARG002  # implements the Observability protocol's span()
         """Return a span that records nothing."""
         return NoOpSpan()
 

libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py

@@ -36,7 +36,7 @@ def filter(self, record: logging.LogRecord) -> bool | logging.LogRecord:
         """Qualify the record's ``otelTraceID``, if set, then apply the base filter."""
         if not hasattr(record, "otelTraceID"):
             return super().filter(record)
-        record.otelTraceID = f"projects/{self.project}/traces/{record.otelTraceID}"  # type: ignore
+        record.otelTraceID = f"projects/{self.project}/traces/{record.otelTraceID}"  # type: ignore[attr-defined]
         return super().filter(record)
 
 

libs/policyengine-fastapi/tests/test_observability.py

@@ -1,5 +1,6 @@
 from datetime import UTC, datetime
 
+import pytest
 from policyengine_fastapi.observability import (
     NoOpObservability,
     ObservabilityConfig,
@@ -30,13 +31,8 @@ def test_parse_header_value_pairs__supports_commas_and_newlines():
 
 
 def test_parse_header_value_pairs__raises_for_invalid_pair():
-    try:
+    with pytest.raises(ValueError, match="key=value"):
         parse_header_value_pairs("Authorization")
-    except ValueError as error:
-        assert "key=value" in str(error)
-    else:
-        msg = "Expected invalid OTLP header parsing to fail"
-        raise AssertionError(msg)
 
 
 def test_observability_config_disabled__returns_disabled_defaults():
@@ -108,7 +104,7 @@ def test_contract_models__serialize_expected_shapes():
 
 
 def test_contract_models__reject_extra_fields():
-    try:
+    with pytest.raises(ValidationError, match="unexpected"):
         SimulationLifecycleEvent(
             event_name="simulation.stage.completed",
             stage=SimulationStage.WORKER_COMPLETED,
@@ -118,11 +114,6 @@ def test_contract_models__reject_extra_fields():
             observability_id="run-789",
             unexpected=True,
         )
-    except ValidationError as error:
-        assert "unexpected" in str(error)
-    else:
-        msg = "Expected extra field validation to fail"
-        raise AssertionError(msg)
 
 
 def test_noop_observability__accepts_calls_without_side_effects():

libs/policyengine-simulation-contract/src/policyengine_simulation_contract/dataset_uri.py

@@ -95,9 +95,7 @@ def _policyengine_data_runtime_uri(
         package_version is not None
         and artifact_revision is not None
         and reference.revision == artifact_revision
-    ):
-        selected_revision = package_version
-    elif selected_revision is None:
+    ) or selected_revision is None:
         selected_revision = package_version
 
     if selected_revision is not None:

libs/policyengine-simulation-contract/src/policyengine_simulation_contract/spm.py

@@ -79,10 +79,9 @@ def serialize_selection(  # noqa: ANN201  # a return annotation would become the
     @classmethod
     def validate_as_of(cls, value: str | None) -> str | None:
         """Require ``as_of`` to be a canonical ``YYYY-MM-DD`` date when given."""
-        if value is not None:
-            if date.fromisoformat(value).isoformat() != value:
-                msg = "as_of must be an ISO calendar date (YYYY-MM-DD)"
-                raise ValueError(msg)
+        if value is not None and date.fromisoformat(value).isoformat() != value:
+            msg = "as_of must be an ISO calendar date (YYYY-MM-DD)"
+            raise ValueError(msg)
         return value
 
     @model_validator(mode="after")

libs/policyengine-simulation-contract/tests/conftest.py

@@ -33,7 +33,7 @@ def mock_modal(monkeypatch):
 
     class MockModalDict:
         @staticmethod
-        def from_name(name: str, create_if_missing: bool = False):
+        def from_name(name: str, *, create_if_missing: bool = False):
             if create_if_missing and name not in mock_dicts:
                 mock_dicts[name] = {}
             if name not in mock_dicts:

libs/policyengine-simulation-contract/tests/test_budget_window_state.py

@@ -1,5 +1,6 @@
 """Tests for budget-window batch state helpers."""
 
+import pytest
 from policyengine_simulation_contract.budget_window_state import (
     build_batch_status_response,
     create_initial_batch_state,
@@ -84,7 +85,8 @@ def test_build_batch_status_response_computes_progress_from_completed_years():
     assert response.queued_years == ["2029"]
 
 
-def test_batch_state_round_trips_through_modal_dict(mock_modal):
+@pytest.mark.usefixtures("mock_modal")
+def test_batch_state_round_trips_through_modal_dict():
     request = BudgetWindowBatchRequest(
         country="us",
         region="state/ca",