From f3af8b7488ba91d250c5f33bddaeabefdbfc59d2 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:58:10 +0400 Subject: [PATCH 01/15] Exempt tests from SLF001, INP001 and PLC0415 Tests deliberately reach into private members, pytest test directories aren't packages, and tests often import inside a test after patching the environment. Application code still gets every rule. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- docs/generated/rules.md | 8 ++++---- pyproject.toml | 12 +++++++----- rules/registry.toml | 4 ++-- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 81677f6d6..a4cce241b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -129,7 +129,7 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. - `api.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (ANN, BLE, D, DTZ, E4, E7, E9, EM, ERA, EXE, F, FIX, FLY, FURB, G, I, LOG, PIE, PYI, RET, RSE, S, T20, TC, TD, TRY, UP, W)) - `api.quality.runtime-evaluated-models-resolve`: Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. (blocking: pytest `tests/unit/test_runtime_evaluated_annotations.py`) - `api.quality.tests-pass`: The unit, contract and to-refactor test suites pass. (blocking: CI job `test`) -- `api.quality.tests-ruff-exemptions`: Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201). Every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, DTZ001, DTZ011, PLR2004, S101, S105, S106, S603, S607, S608, T201 for `**/tests/**`) +- `api.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). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201). Every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, DTZ001, DTZ011, INP001, PLC0415, PLR2004, S101, S105, S106, S603, S607, S608, SLF001, T201 for `**/tests/**`) ### Observability diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 0e98f9965..7184b03b2 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -278,7 +278,7 @@ flowchart LR | `api.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. | mechanical | enforced | yes | ruff check (ANN, BLE, D, DTZ, E4, E7, E9, EM, ERA, EXE, F, FIX, FLY, FURB, G, I, LOG, PIE, PYI, RET, RSE, S, T20, TC, TD, TRY, UP, W) | `pr.yml:lint` | — | | `api.quality.runtime-evaluated-models-resolve` | Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. | mechanical | enforced | yes | pytest `tests/unit/test_runtime_evaluated_annotations.py` | `pr.yml:test` | **unproven** | | `api.quality.tests-pass` | The unit, contract and to-refactor test suites pass. | mechanical | enforced | yes | CI job `test` | `pr.yml:test` | — | -| `api.quality.tests-ruff-exemptions` | Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201). Every other ruff rule applies to tests. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, DTZ001, DTZ011, PLR2004, S101, S105, S106, S603, S607, S608, T201 for `**/tests/**` | `pr.yml:lint` | — | +| `api.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). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201). Every other ruff rule applies to tests. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, DTZ001, DTZ011, INP001, PLC0415, PLR2004, S101, S105, S106, S603, S607, S608, SLF001, T201 for `**/tests/**` | `pr.yml:lint` | — | ```mermaid flowchart LR @@ -307,7 +307,7 @@ flowchart LR r_api_quality_tests_pass("api.quality.tests-pass (enforced)") e_CI_job_test["CI job test"] r_api_quality_tests_ruff_exemptions("api.quality.tests-ruff-exemptions (enforced)") - e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__PLR2004__S101__S105__S106__S603__S607__S608__T201_for____tests___["ruff per-file ignore of ANN, D1, DTZ001, DTZ011, PLR2004, S101, S105, S106, S603, S607, S608, T201 for **/tests/**"] + e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__INP001__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201_for____tests___["ruff per-file ignore of ANN, D1, DTZ001, DTZ011, INP001, PLC0415, PLR2004, S101, S105, S106, S603, S607, S608, SLF001, T201 for **/tests/**"] r_api_quality_alembic_revisions_unused_imports -.-> gap_unenforced r_api_quality_ci_cd_scripts_ruff_exemptions --> e_ruff_per_file_ignore_of_S310__T201_for__github___ e_ruff_per_file_ignore_of_S310__T201_for__github___ --> j_CI_pr_yml_lint @@ -327,8 +327,8 @@ flowchart LR e_pytest_tests_unit_test_runtime_evaluated_annotations_py --> j_CI_pr_yml_test r_api_quality_tests_pass --> e_CI_job_test e_CI_job_test --> j_CI_pr_yml_test - r_api_quality_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__PLR2004__S101__S105__S106__S603__S607__S608__T201_for____tests___ - e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__PLR2004__S101__S105__S106__S603__S607__S608__T201_for____tests___ --> j_CI_pr_yml_lint + r_api_quality_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__INP001__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201_for____tests___ + e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__INP001__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201_for____tests___ --> j_CI_pr_yml_lint ``` ## Observability diff --git a/pyproject.toml b/pyproject.toml index cfb6fccbe..e3522dda9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -113,12 +113,14 @@ exclude = ["migrations/*/versions/*.py"] # command-line scripts; the rules guard checks each pattern covers nothing else # (docs/engineering/skills/rules-registry.md#file-level-exceptions). # Tests may use pytest's bare assert, and need no docstrings, annotations or -# named constants for magic numbers. They may also use fake credentials, naive -# datetimes, subprocess calls to repository scripts, SQL built from strings and -# prints. Every other rule applies to tests. +# named constants for magic numbers. They may also reach into private members, +# live in directories that aren't packages, import inside a test (often after +# patching the environment), and use fake credentials, naive datetimes, +# subprocess calls to repository scripts, SQL built from strings and prints. +# Every other rule applies to tests. "**/tests/**" = [ - "ANN", "D1", "DTZ001", "DTZ011", "PLR2004", "S101", "S105", "S106", "S603", - "S607", "S608", "T201", + "ANN", "D1", "DTZ001", "DTZ011", "INP001", "PLC0415", "PLR2004", "S101", + "S105", "S106", "S603", "S607", "S608", "SLF001", "T201", ] # CI/CD scripts print their results and fetch fixed URLs. ".github/**" = ["S310", "T201"] diff --git a/rules/registry.toml b/rules/registry.toml index 68e960285..4c0d0a70a 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -744,7 +744,7 @@ kind = "none" [[rule]] id = "api.quality.tests-ruff-exemptions" category = "quality" -statement = "Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201). Every other ruff rule applies to tests." +statement = "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). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201). Every other ruff rule applies to tests." rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" scope = ["tests/**/*.py", "scripts/rules/tests/**/*.py"] tier = "mechanical" @@ -754,7 +754,7 @@ autofix = false kind = "ruff" mode = "per-file-ignore" pattern = "**/tests/**" -codes = ["ANN", "D1", "DTZ001", "DTZ011", "PLR2004", "S101", "S105", "S106", "S603", "S607", "S608", "T201"] +codes = ["ANN", "D1", "DTZ001", "DTZ011", "INP001", "PLC0415", "PLR2004", "S101", "S105", "S106", "S603", "S607", "S608", "SLF001", "T201"] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "lint" From ce2b440bfa40bb5109e100b8dbc9c772d7515de3 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 22:05:49 +0400 Subject: [PATCH 02/15] Declare FastAPI dependencies with Annotated (FAST002) The route modules use `from __future__ import annotations`, and FastAPI resolves string annotations against module globals only, so the query dependencies move from per-router closures to module-level Annotated aliases. The OpenAPI documents are byte-identical. Co-Authored-By: Claude Opus 5.5 --- .../fastapi_routes/v2/households/routes.py | 24 ++++++++----- .../fastapi_routes/v2/policies/routes.py | 24 ++++++++----- .../v2/user_households/routes.py | 35 +++++++++++++------ .../fastapi_routes/v2/user_policies/routes.py | 22 +++++++----- tests/unit/test_query_parameters.py | 20 +++++++---- 5 files changed, 84 insertions(+), 41 deletions(-) diff --git a/policyengine_api/fastapi_routes/v2/households/routes.py b/policyengine_api/fastapi_routes/v2/households/routes.py index e5964e47b..59c74b7fe 100644 --- a/policyengine_api/fastapi_routes/v2/households/routes.py +++ b/policyengine_api/fastapi_routes/v2/households/routes.py @@ -2,7 +2,7 @@ from __future__ import annotations -from typing import TYPE_CHECKING, TypeVar +from typing import TYPE_CHECKING, Annotated, TypeVar from uuid import UUID from fastapi import APIRouter, Depends, Request @@ -48,6 +48,17 @@ ) +HouseholdCreateQueryDependency = Annotated[ + HouseholdCreateQuery, Depends(query_dependency(HouseholdCreateQuery)) +] +HouseholdDetailQueryDependency = Annotated[ + HouseholdDetailQuery, Depends(query_dependency(HouseholdDetailQuery)) +] +HouseholdCollectionQueryDependency = Annotated[ + HouseholdCollectionQuery, Depends(query_dependency(HouseholdCollectionQuery)) +] + + async def enforce_household_request_size(request: Request) -> None: """Reject declared or actual request bodies larger than 1 MiB.""" content_length = request.headers.get("content-length") @@ -104,9 +115,6 @@ def build_v2_household_router( ) -> APIRouter: """Build native household routes without opening a database connection.""" router = APIRouter(prefix="/v2", responses=HOUSEHOLD_ERROR_RESPONSES) - create_query = query_dependency(HouseholdCreateQuery) - detail_query = query_dependency(HouseholdDetailQuery) - collection_query = query_dependency(HouseholdCollectionQuery) service_factory = _service_factory(dependencies) @router.post( @@ -123,8 +131,8 @@ def build_v2_household_router( ) def create_household( body: HouseholdCreateRequest, - query: HouseholdCreateQuery = Depends(create_query), - _size: None = Depends(enforce_household_request_size), + query: HouseholdCreateQueryDependency, + _size: Annotated[None, Depends(enforce_household_request_size)], ) -> HouseholdDetailResponse | JSONResponse: if body.country_id != query.country_id: return v2_error_response(400, "Body country_id must match query country_id") @@ -150,7 +158,7 @@ def create() -> HouseholdDetailResponse | JSONResponse: ) def get_household( household_id: UUID, - query: HouseholdDetailQuery = Depends(detail_query), + query: HouseholdDetailQueryDependency, ) -> HouseholdDetailResponse | JSONResponse: def read() -> HouseholdDetailResponse: item = service_factory().get_household( @@ -169,7 +177,7 @@ def read() -> HouseholdDetailResponse: summary="List immutable households", ) def get_households( - query: HouseholdCollectionQuery = Depends(collection_query), + query: HouseholdCollectionQueryDependency, ) -> HouseholdPageResponse | JSONResponse: def read() -> HouseholdPageResponse: page = service_factory().list_households( diff --git a/policyengine_api/fastapi_routes/v2/policies/routes.py b/policyengine_api/fastapi_routes/v2/policies/routes.py index c77000afa..063d87c66 100644 --- a/policyengine_api/fastapi_routes/v2/policies/routes.py +++ b/policyengine_api/fastapi_routes/v2/policies/routes.py @@ -2,7 +2,7 @@ from __future__ import annotations -from typing import TYPE_CHECKING, TypeVar +from typing import TYPE_CHECKING, Annotated, TypeVar from uuid import UUID from fastapi import APIRouter, Depends, Request @@ -53,6 +53,17 @@ ) +PolicyCreateQueryDependency = Annotated[ + PolicyCreateQuery, Depends(query_dependency(PolicyCreateQuery)) +] +PolicyDetailQueryDependency = Annotated[ + PolicyDetailQuery, Depends(query_dependency(PolicyDetailQuery)) +] +PolicyCollectionQueryDependency = Annotated[ + PolicyCollectionQuery, Depends(query_dependency(PolicyCollectionQuery)) +] + + async def enforce_policy_request_size(request: Request) -> None: """Bound both declared and actual request bytes before service creation.""" content_length = request.headers.get("content-length") @@ -109,9 +120,6 @@ def build_v2_policy_router( ) -> APIRouter: """Build native policy routes without opening a database connection.""" router = APIRouter(prefix="/v2", responses=POLICY_ERROR_RESPONSES) - create_query = query_dependency(PolicyCreateQuery) - detail_query = query_dependency(PolicyDetailQuery) - collection_query = query_dependency(PolicyCollectionQuery) service_factory = _service_factory(dependencies) @router.post( @@ -128,8 +136,8 @@ def build_v2_policy_router( ) def create_policy( body: PolicyCreateRequest, - query: PolicyCreateQuery = Depends(create_query), - _size: None = Depends(enforce_policy_request_size), + query: PolicyCreateQueryDependency, + _size: Annotated[None, Depends(enforce_policy_request_size)], ) -> PolicyDetailResponse | JSONResponse: if body.country_id != query.country_id: return v2_error_response( @@ -163,7 +171,7 @@ def create() -> PolicyDetailResponse | JSONResponse: ) def get_policy( policy_id: UUID, - query: PolicyDetailQuery = Depends(detail_query), + query: PolicyDetailQueryDependency, ) -> PolicyDetailResponse | JSONResponse: def read() -> PolicyDetailResponse: item = service_factory().get_policy( @@ -182,7 +190,7 @@ def read() -> PolicyDetailResponse: summary="List immutable policies", ) def get_policies( - query: PolicyCollectionQuery = Depends(collection_query), + query: PolicyCollectionQueryDependency, ) -> PolicyPageResponse | JSONResponse: def read() -> PolicyPageResponse: page = service_factory().list_policies( diff --git a/policyengine_api/fastapi_routes/v2/user_households/routes.py b/policyengine_api/fastapi_routes/v2/user_households/routes.py index c5239a1cf..42b9d6cb8 100644 --- a/policyengine_api/fastapi_routes/v2/user_households/routes.py +++ b/policyengine_api/fastapi_routes/v2/user_households/routes.py @@ -2,7 +2,7 @@ from __future__ import annotations -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Annotated from uuid import UUID from fastapi import APIRouter, Depends @@ -44,6 +44,24 @@ ) +UserHouseholdCreateQueryDependency = Annotated[ + UserHouseholdCreateQuery, Depends(query_dependency(UserHouseholdCreateQuery)) +] +UserHouseholdDetailQueryDependency = Annotated[ + UserHouseholdDetailQuery, Depends(query_dependency(UserHouseholdDetailQuery)) +] +UserHouseholdCollectionQueryDependency = Annotated[ + UserHouseholdCollectionQuery, + Depends(query_dependency(UserHouseholdCollectionQuery)), +] +UserHouseholdUpdateQueryDependency = Annotated[ + UserHouseholdUpdateQuery, Depends(query_dependency(UserHouseholdUpdateQuery)) +] +UserHouseholdDeleteQueryDependency = Annotated[ + UserHouseholdDeleteQuery, Depends(query_dependency(UserHouseholdDeleteQuery)) +] + + def _service_factory( dependencies: NativeRouteDependencies, ) -> Callable[[], V2UserHouseholdResourceService]: @@ -73,11 +91,6 @@ def build_v2_user_household_router( Depends(enforce_user_write_request_size), ], ) - create_query = query_dependency(UserHouseholdCreateQuery) - detail_query = query_dependency(UserHouseholdDetailQuery) - collection_query = query_dependency(UserHouseholdCollectionQuery) - update_query = query_dependency(UserHouseholdUpdateQuery) - delete_query = query_dependency(UserHouseholdDeleteQuery) service_factory = _service_factory(dependencies) @router.post( @@ -94,7 +107,7 @@ def build_v2_user_household_router( ) def create_user_household( body: UserHouseholdCreateRequest, - query: UserHouseholdCreateQuery = Depends(create_query), + query: UserHouseholdCreateQueryDependency, principal: AccountPrincipal = Depends(require_user), ) -> UserHouseholdDetailResponse | JSONResponse: if body.country_id != query.country_id: @@ -118,7 +131,7 @@ def create() -> UserHouseholdDetailResponse: ) def get_user_household( association_id: UUID, - query: UserHouseholdDetailQuery = Depends(detail_query), + query: UserHouseholdDetailQueryDependency, principal: AccountPrincipal = Depends(require_user), ) -> UserHouseholdDetailResponse | JSONResponse: def read() -> UserHouseholdDetailResponse: @@ -139,7 +152,7 @@ def read() -> UserHouseholdDetailResponse: summary="List the current user's saved households", ) def get_user_households( - query: UserHouseholdCollectionQuery = Depends(collection_query), + query: UserHouseholdCollectionQueryDependency, principal: AccountPrincipal = Depends(require_user), ) -> UserHouseholdPageResponse | JSONResponse: def read() -> UserHouseholdPageResponse: @@ -169,7 +182,7 @@ def read() -> UserHouseholdPageResponse: def patch_user_household_route( association_id: UUID, body: UserHouseholdPatchRequest, - query: UserHouseholdUpdateQuery = Depends(update_query), + query: UserHouseholdUpdateQueryDependency, principal: AccountPrincipal = Depends(require_user), ) -> UserHouseholdDetailResponse | JSONResponse: def patch() -> UserHouseholdDetailResponse: @@ -194,7 +207,7 @@ def patch() -> UserHouseholdDetailResponse: ) def delete_user_household_route( association_id: UUID, - query: UserHouseholdDeleteQuery = Depends(delete_query), + query: UserHouseholdDeleteQueryDependency, principal: AccountPrincipal = Depends(require_user), ) -> Response | JSONResponse: def delete() -> Response: diff --git a/policyengine_api/fastapi_routes/v2/user_policies/routes.py b/policyengine_api/fastapi_routes/v2/user_policies/routes.py index d9c360585..702ba0c45 100644 --- a/policyengine_api/fastapi_routes/v2/user_policies/routes.py +++ b/policyengine_api/fastapi_routes/v2/user_policies/routes.py @@ -2,7 +2,7 @@ from __future__ import annotations -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Annotated from uuid import UUID from fastapi import APIRouter, Depends @@ -41,6 +41,14 @@ ) +CountryQueryDependency = Annotated[ + CountryQuery, Depends(query_dependency(CountryQuery)) +] +UserPolicyCollectionQueryDependency = Annotated[ + UserPolicyCollectionQuery, Depends(query_dependency(UserPolicyCollectionQuery)) +] + + def _service_factory( dependencies: NativeRouteDependencies, ) -> Callable[[], V2UserPolicyResourceService]: @@ -70,8 +78,6 @@ def build_v2_user_policy_router( Depends(enforce_user_write_request_size), ], ) - country_query = query_dependency(CountryQuery) - collection_query = query_dependency(UserPolicyCollectionQuery) service_factory = _service_factory(dependencies) @router.post( @@ -89,7 +95,7 @@ def build_v2_user_policy_router( ) def create_user_policy( body: UserPolicyCreateRequest, - query: CountryQuery = Depends(country_query), + query: CountryQueryDependency, principal: AccountPrincipal = Depends(require_user), ) -> UserPolicyDetailResponse | JSONResponse: if body.country_id != query.country_id: @@ -116,7 +122,7 @@ def create() -> UserPolicyDetailResponse: ) def get_user_policy( association_id: UUID, - query: CountryQuery = Depends(country_query), + query: CountryQueryDependency, principal: AccountPrincipal = Depends(require_user), ) -> UserPolicyDetailResponse | JSONResponse: def read() -> UserPolicyDetailResponse: @@ -137,7 +143,7 @@ def read() -> UserPolicyDetailResponse: summary="List the current user's saved policies", ) def get_user_policies( - query: UserPolicyCollectionQuery = Depends(collection_query), + query: UserPolicyCollectionQueryDependency, principal: AccountPrincipal = Depends(require_user), ) -> UserPolicyPageResponse | JSONResponse: def read() -> UserPolicyPageResponse: @@ -166,7 +172,7 @@ def read() -> UserPolicyPageResponse: def patch_user_policy_route( association_id: UUID, body: UserPolicyPatchRequest, - query: CountryQuery = Depends(country_query), + query: CountryQueryDependency, principal: AccountPrincipal = Depends(require_user), ) -> UserPolicyDetailResponse | JSONResponse: def patch() -> UserPolicyDetailResponse: @@ -191,7 +197,7 @@ def patch() -> UserPolicyDetailResponse: ) def delete_user_policy_route( association_id: UUID, - query: CountryQuery = Depends(country_query), + query: CountryQueryDependency, principal: AccountPrincipal = Depends(require_user), ) -> Response | JSONResponse: def delete() -> Response: diff --git a/tests/unit/test_query_parameters.py b/tests/unit/test_query_parameters.py index 1bf80cce8..11a906758 100644 --- a/tests/unit/test_query_parameters.py +++ b/tests/unit/test_query_parameters.py @@ -41,6 +41,17 @@ class ExplicitListQuery(CountryQuery): ] +ExplicitListQueryDependency = Annotated[ + ExplicitListQuery, Depends(query_dependency(ExplicitListQuery)) +] +PolicyCollectionQueryDependency = Annotated[ + PolicyCollectionQuery, Depends(query_dependency(PolicyCollectionQuery)) +] +HouseholdCollectionQueryDependency = Annotated[ + HouseholdCollectionQuery, Depends(query_dependency(HouseholdCollectionQuery)) +] + + def test_canonical_required_defaults_and_normalization() -> None: parsed = parse_query_items(PolicyCollectionQuery, [("country_id", "US")]) @@ -139,11 +150,10 @@ def test_multidict_adapter_preserves_the_canonical_contract() -> None: def _test_client() -> TestClient: app = FastAPI() - dependency = query_dependency(ExplicitListQuery) @app.get("/resources") def resources( - query: ExplicitListQuery = Depends(dependency), + query: ExplicitListQueryDependency, ) -> dict[str, object]: return query.model_dump(mode="json") @@ -187,11 +197,10 @@ def test_openapi_matches_composed_runtime_query_contract() -> None: def test_policy_collection_openapi_defaults_and_bounds() -> None: app = FastAPI() - dependency = query_dependency(PolicyCollectionQuery) @app.get("/policies") def policies( - query: PolicyCollectionQuery = Depends(dependency), + query: PolicyCollectionQueryDependency, ) -> dict[str, object]: return query.model_dump(mode="json") @@ -339,11 +348,10 @@ def test_household_query_models_reject_unknown_and_duplicate_scalars( def test_household_collection_openapi_matches_runtime_contract() -> None: app = FastAPI() - dependency = query_dependency(HouseholdCollectionQuery) @app.get("/households") def households( - query: HouseholdCollectionQuery = Depends(dependency), + query: HouseholdCollectionQueryDependency, ) -> dict[str, object]: return query.model_dump(mode="json") From f651a80e9605110dd560f7ae9df1c5b65f7e32a4 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 22:08:31 +0400 Subject: [PATCH 03/15] Rename internal names and explain the kept ones (N, A004, SLF001) Rename the purely internal names: the outcome bounds and labels in the decile impact helpers, the rules tool's BaseUnavailableError and its tests' assert_violation helper, a test's list of uncalculated variables, the PostgreSQL UUID import, and the tests' SQLAlchemy TimeoutError imports (now PoolTimeoutError, so they no longer shadow the builtin). Keep, with reasons, the Reform subclass name that policyengine-core uses as the reform key, the gRPC stubs' Export methods, and the three private accesses that have no public accessor. Co-Authored-By: Claude Opus 5.5 --- policyengine_api/country.py | 2 +- policyengine_api/runtime_cache/fake.py | 2 +- .../services/economy_comparison.py | 16 +++-- .../services/report_output_service.py | 2 +- policyengine_api/worker_spm_release.py | 2 +- scripts/rules/baseline.py | 6 +- scripts/rules/guard.py | 4 +- scripts/rules/tests/test_guard.py | 70 ++++++++++--------- .../integration/test_alembic_v2_lifecycle.py | 6 +- tests/integration/test_simulations.py | 7 +- tests/unit/routes/test_canonical_spm.py | 4 +- tests/unit/services/test_policy_mirroring.py | 5 +- .../services/test_user_policy_mirroring.py | 5 +- .../test_observability_collector_verifier.py | 4 +- tests/unit/v2/test_household_routes.py | 5 +- tests/unit/v2/test_policy_routes.py | 5 +- 16 files changed, 78 insertions(+), 67 deletions(-) diff --git a/policyengine_api/country.py b/policyengine_api/country.py index da57c41a9..1c5228baa 100644 --- a/policyengine_api/country.py +++ b/policyengine_api/country.py @@ -692,7 +692,7 @@ def modify_parameters(parameters: ParameterNode) -> ParameterNode: return parameters - class reform(Reform): + class reform(Reform): # noqa: N801 # policyengine-core uses the class name as the reform key def apply(self) -> None: self.modify_parameters(modify_parameters) diff --git a/policyengine_api/runtime_cache/fake.py b/policyengine_api/runtime_cache/fake.py index bc5e2f195..af842d4b0 100644 --- a/policyengine_api/runtime_cache/fake.py +++ b/policyengine_api/runtime_cache/fake.py @@ -191,7 +191,7 @@ def execute(self) -> list[object]: With ``transaction`` the replay holds the backend lock, as MULTI/EXEC would; without it, other callers' commands may interleave. """ - lock = self.backend._lock if self.transaction else RLock() + lock = self.backend._lock if self.transaction else RLock() # noqa: SLF001 # a transactional replay holds the backend lock, as MULTI/EXEC would with lock: results = [ getattr(self.backend, name)(*args, **kwargs) diff --git a/policyengine_api/services/economy_comparison.py b/policyengine_api/services/economy_comparison.py index 4e8c5f573..03ce82fc7 100644 --- a/policyengine_api/services/economy_comparison.py +++ b/policyengine_api/services/economy_comparison.py @@ -345,15 +345,17 @@ def intra_decile_impact(baseline: dict, reform: dict) -> dict: outcome_groups = {} all_outcomes = {} - BOUNDS = [-np.inf, -0.05, -1e-3, 1e-3, 0.05, np.inf] - LABELS = [ + income_change_bounds = [-np.inf, -0.05, -1e-3, 1e-3, 0.05, np.inf] + outcome_labels = [ "Lose more than 5%", "Lose less than 5%", "No change", "Gain less than 5%", "Gain more than 5%", ] - for lower, upper, label in zip(BOUNDS[:-1], BOUNDS[1:], LABELS): + for lower, upper, label in zip( + income_change_bounds[:-1], income_change_bounds[1:], outcome_labels + ): outcome_groups[label] = [] for i in range(1, 11): in_decile: np.ndarray = decile == i @@ -402,15 +404,17 @@ def intra_wealth_decile_impact(baseline: dict, reform: dict) -> dict: outcome_groups = {} all_outcomes = {} - BOUNDS = [-np.inf, -0.05, -1e-3, 1e-3, 0.05, np.inf] - LABELS = [ + income_change_bounds = [-np.inf, -0.05, -1e-3, 1e-3, 0.05, np.inf] + outcome_labels = [ "Lose more than 5%", "Lose less than 5%", "No change", "Gain less than 5%", "Gain more than 5%", ] - for lower, upper, label in zip(BOUNDS[:-1], BOUNDS[1:], LABELS): + for lower, upper, label in zip( + income_change_bounds[:-1], income_change_bounds[1:], outcome_labels + ): outcome_groups[label] = [] for i in range(1, 11): in_decile: np.ndarray = decile == i diff --git a/policyengine_api/services/report_output_service.py b/policyengine_api/services/report_output_service.py index c69e60531..29bb28522 100644 --- a/policyengine_api/services/report_output_service.py +++ b/policyengine_api/services/report_output_service.py @@ -210,7 +210,7 @@ def _get_linked_simulations( def get_simulation(simulation_id: int) -> Simulation | None: if bootstrap_dual_write_state: try: - return self.simulation_service._ensure_simulation_dual_write_state( + return self.simulation_service._ensure_simulation_dual_write_state( # noqa: SLF001 # reuses the simulation service bootstrap inside this session; no public equivalent session, simulation_id, report_output.country_id, diff --git a/policyengine_api/worker_spm_release.py b/policyengine_api/worker_spm_release.py index f83c9ebf2..2efe488f9 100644 --- a/policyengine_api/worker_spm_release.py +++ b/policyengine_api/worker_spm_release.py @@ -44,7 +44,7 @@ def get_spm_capability( def _bundle_package_versions() -> dict[str, str]: - bundle = constants._policyengine_bundle + bundle = constants._policyengine_bundle # noqa: SLF001 # constants has no public accessor for the manifest; read at call time so tests can patch it packages = bundle.get("packages") if isinstance(bundle, dict) else None if not isinstance(packages, dict): msg = "The installed PolicyEngine bundle manifest has no packages" diff --git a/scripts/rules/baseline.py b/scripts/rules/baseline.py index 5bc3feb90..ab52ec124 100644 --- a/scripts/rules/baseline.py +++ b/scripts/rules/baseline.py @@ -13,7 +13,7 @@ from pathlib import Path -class BaseUnavailable(Exception): +class BaseUnavailableError(Exception): """The base ref could not be read.""" @@ -26,7 +26,7 @@ def default_base_ref() -> str | None: def load_base_registry(root: Path, ref: str) -> Registry | None: """Return the registry as it is on ``ref``, or None if the base has none yet. - Raise BaseUnavailable when ``ref`` does not name a commit. + Raise BaseUnavailableError when ``ref`` does not name a commit. """ verify = subprocess.run( ["git", "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"], @@ -35,7 +35,7 @@ def load_base_registry(root: Path, ref: str) -> Registry | None: text=True, ) if verify.returncode != 0: - raise BaseUnavailable(ref) + raise BaseUnavailableError(ref) shown = subprocess.run( ["git", "show", f"{ref}:{REGISTRY_PATH.as_posix()}"], cwd=root, diff --git a/scripts/rules/guard.py b/scripts/rules/guard.py index 3efa3d4f5..243102cc6 100644 --- a/scripts/rules/guard.py +++ b/scripts/rules/guard.py @@ -9,7 +9,7 @@ from scripts.rules.adapters import check_adapters from scripts.rules.baseline import ( - BaseUnavailable, + BaseUnavailableError, check_baselines, default_base_ref, load_base_registry, @@ -109,7 +109,7 @@ def run_guard( else: try: base = load_base_registry(root, ref) - except BaseUnavailable: + except BaseUnavailableError: if os.environ.get("GITHUB_ACTIONS"): result.violations.append( Violation( diff --git a/scripts/rules/tests/test_guard.py b/scripts/rules/tests/test_guard.py index 00151ed23..46738fc2d 100644 --- a/scripts/rules/tests/test_guard.py +++ b/scripts/rules/tests/test_guard.py @@ -91,7 +91,7 @@ def test_rules_md_lists_gaps(self) -> None: class SchemaTest(unittest.TestCase): - def assertViolation(self, repo: Repo, fragment: str) -> None: + def assert_violation(self, repo: Repo, fragment: str) -> None: messages = repo.messages() self.assertTrue(any(fragment in m for m in messages), messages) @@ -100,19 +100,19 @@ def test_missing_required_field(self) -> None: repo.edit( "rules/registry.toml", 'statement = "Routes never issue SQL."\n', "" ) - self.assertViolation(repo, "statement must be a non-empty string") + self.assert_violation(repo, "statement must be a non-empty string") def test_unknown_enforcer_kind(self) -> None: with Repo(render=False) as repo: repo.edit("rules/registry.toml", 'kind = "mypy"', 'kind = "flake8"') - self.assertViolation(repo, "enforcer.kind is 'flake8'") + self.assert_violation(repo, "enforcer.kind is 'flake8'") def test_bad_status(self) -> None: with Repo(render=False) as repo: repo.edit( "rules/registry.toml", 'status = "doc-only"', 'status = "optional"' ) - self.assertViolation(repo, "status is 'optional'") + self.assert_violation(repo, "status is 'optional'") def test_unknown_key(self) -> None: with Repo(render=False) as repo: @@ -121,7 +121,7 @@ def test_unknown_key(self) -> None: "autofix = true", 'autofix = true\nowner = "someone"', ) - self.assertViolation(repo, "unknown key 'owner'") + self.assert_violation(repo, "unknown key 'owner'") def test_duplicate_rule_id(self) -> None: with Repo(render=False) as repo: @@ -130,7 +130,7 @@ def test_duplicate_rule_id(self) -> None: 'id = "ex.quality.types"', 'id = "ex.quality.format"', ) - self.assertViolation(repo, "duplicate rule id") + self.assert_violation(repo, "duplicate rule id") def test_scope_glob_must_match(self) -> None: with Repo(render=False) as repo: @@ -139,7 +139,7 @@ def test_scope_glob_must_match(self) -> None: 'scope = ["rules/registry.toml"]', 'scope = ["nowhere/*.py"]', ) - self.assertViolation(repo, "scope 'nowhere/*.py' matches no files") + self.assert_violation(repo, "scope 'nowhere/*.py' matches no files") def test_doc_only_rule_cannot_claim_enforcer(self) -> None: with Repo(render=False) as repo: @@ -148,7 +148,7 @@ def test_doc_only_rule_cannot_claim_enforcer(self) -> None: 'status = "enforced"\nautofix = true', 'status = "doc-only"\nautofix = true', ) - self.assertViolation(repo, "doc-only rules have no enforcer") + self.assert_violation(repo, "doc-only rules have no enforcer") def test_enforced_rule_needs_ci(self) -> None: extra = """ @@ -166,7 +166,7 @@ def test_enforced_rule_needs_ci(self) -> None: nodes = ["tests/unit/test_boundaries.py::test_routes_have_no_sql"] """ with Repo(with_rule(extra), render=False) as repo: - self.assertViolation(repo, "enforced rules must run in CI") + self.assert_violation(repo, "enforced rules must run in CI") def test_ruff_is_never_ratcheted(self) -> None: with Repo(render=False) as repo: @@ -176,7 +176,7 @@ def test_ruff_is_never_ratcheted(self) -> None: 'kind = "ruff"\nmode = "format"', 'kind = "ruff"\nmode = "format"\n[rule.baseline]\nfile = "rules/baselines/ruff.txt"\ncount = 1', ) - self.assertViolation(repo, "ruff is never ratcheted") + self.assert_violation(repo, "ruff is never ratcheted") def test_ratcheted_without_baseline(self) -> None: with Repo(render=False) as repo: @@ -185,7 +185,7 @@ def test_ratcheted_without_baseline(self) -> None: 'status = "enforced"\nautofix = false\n[rule.enforcer]\nkind = "mypy"', 'status = "ratcheted"\nautofix = false\n[rule.enforcer]\nkind = "mypy"', ) - self.assertViolation(repo, "ratcheted rules need a baseline") + self.assert_violation(repo, "ratcheted rules need a baseline") def test_baseline_count_must_match_file(self) -> None: with Repo(with_rule(RATCHETED_RULE.format(count=3)), render=False) as repo: @@ -193,7 +193,7 @@ def test_baseline_count_must_match_file(self) -> None: "rules/baselines/ratcheted.txt", "# tolerated\npkg/routes.py\npkg/other.py\n", ) - self.assertViolation( + self.assert_violation( repo, "baseline count is 3 but rules/baselines/ratcheted.txt lists 2" ) @@ -204,11 +204,11 @@ def test_judgment_rules_need_review_enforcer(self) -> None: 'tier = "mechanical"\nstatus = "enforced"\nautofix = true', 'tier = "judgment"\nstatus = "enforced"\nautofix = true', ) - self.assertViolation(repo, "judgment rules are enforced by review") + self.assert_violation(repo, "judgment rules are enforced by review") class ResolutionTest(unittest.TestCase): - def assertViolation(self, repo: Repo, fragment: str) -> None: + def assert_violation(self, repo: Repo, fragment: str) -> None: messages = repo.messages() self.assertTrue(any(fragment in m for m in messages), messages) @@ -219,7 +219,7 @@ def test_unresolved_pytest_node(self) -> None: "def test_routes_have_no_sql", "def test_renamed", ) - self.assertViolation(repo, "has no test 'test_routes_have_no_sql'") + self.assert_violation(repo, "has no test 'test_routes_have_no_sql'") def test_parametrized_node_resolves(self) -> None: with Repo() as repo: @@ -234,7 +234,7 @@ def test_parametrized_node_resolves(self) -> None: def test_test_outside_make_target_is_not_collected(self) -> None: with Repo() as repo: repo.edit("Makefile", "tests/unit", "tests/contract") - self.assertViolation(repo, "is not collected by 'make test'") + self.assert_violation(repo, "is not collected by 'make test'") def test_project_must_be_in_matrix(self) -> None: workflow = """ @@ -275,7 +275,7 @@ def test_project_must_be_in_matrix(self) -> None: **with_rule(extra), } with Repo(files) as repo: - self.assertViolation( + self.assert_violation( repo, "project projects/beta is not in the matrix of job 'test'" ) @@ -283,12 +283,12 @@ def test_missing_job(self) -> None: with Repo() as repo: repo.edit("rules/registry.toml", 'job = "typecheck"', 'job = "types"') repo.render() - self.assertViolation(repo, "no job 'types'") + self.assert_violation(repo, "no job 'types'") def test_invocation_only_in_comment_does_not_count(self) -> None: with Repo() as repo: repo.edit(".github/workflows/pr.yml", "run: make test", "run: echo skipped") - self.assertViolation(repo, "does not run 'make test'") + self.assert_violation(repo, "does not run 'make test'") def test_unknown_quality_guard(self) -> None: with Repo() as repo: @@ -296,7 +296,7 @@ def test_unknown_quality_guard(self) -> None: "rules/registry.toml", 'name = "rules-registry"', 'name = "renamed"' ) repo.render() - self.assertViolation(repo, "no guard named 'renamed'") + self.assert_violation(repo, "no guard named 'renamed'") def test_mypy_path_must_be_configured(self) -> None: with Repo() as repo: @@ -304,7 +304,7 @@ def test_mypy_path_must_be_configured(self) -> None: "rules/registry.toml", 'paths = ["pkg"]', 'paths = ["pkg", "other"]' ) repo.render() - self.assertViolation(repo, "['other'] not in [tool.mypy].files") + self.assert_violation(repo, "['other'] not in [tool.mypy].files") def test_ruff_lint_codes_must_be_selected(self) -> None: extra = """ @@ -323,7 +323,7 @@ def test_ruff_lint_codes_must_be_selected(self) -> None: codes = ["B"] """ with Repo(with_rule(extra)) as repo: - self.assertViolation(repo, "ruff does not select ['B']") + self.assert_violation(repo, "ruff does not select ['B']") def test_proof_files_must_exist(self) -> None: with Repo(render=False) as repo: @@ -333,7 +333,7 @@ def test_proof_files_must_exist(self) -> None: 'kind = "mypy"\npaths = ["pkg"]\n[rule.proof]\nbad_fixture = "rules/fixtures/bad.py"\n' 'good_fixture = "rules/fixtures/good.py"\ntest = "tests/unit/test_boundaries.py::test_routes_have_no_sql"', ) - self.assertViolation( + self.assert_violation( repo, "proof bad_fixture rules/fixtures/bad.py does not exist" ) @@ -434,7 +434,7 @@ def test_proof_with_named_unittest_job(self) -> None: class HiddenRuleTest(unittest.TestCase): - def assertViolation(self, repo: Repo, fragment: str) -> None: + def assert_violation(self, repo: Repo, fragment: str) -> None: messages = repo.messages() self.assertTrue(any(fragment in m for m in messages), messages) @@ -444,7 +444,7 @@ def test_unregistered_quality_guard(self) -> None: "scripts/run_quality_guards.py", 'GUARDS = (("rules-registry", lambda: []), ("secret-guard", lambda: []))\n', ) - self.assertViolation(repo, "quality guard 'secret-guard'") + self.assert_violation(repo, "quality guard 'secret-guard'") def test_unregistered_guard_test(self) -> None: with Repo() as repo: @@ -453,14 +453,14 @@ def test_unregistered_guard_test(self) -> None: "tests/unit/test_boundaries.py", text + "\n\ndef test_new_boundary():\n pass\n", ) - self.assertViolation( + self.assert_violation( repo, "test_boundaries.py::test_new_boundary is not referenced" ) def test_unregistered_mypy_file(self) -> None: with Repo() as repo: repo.write("pyproject.toml", '[tool.mypy]\nfiles = ["pkg", "scripts"]\n') - self.assertViolation(repo, "mypy checks 'scripts'") + self.assert_violation(repo, "mypy checks 'scripts'") def test_unregistered_ruff_ignore(self) -> None: with Repo() as repo: @@ -468,7 +468,7 @@ def test_unregistered_ruff_ignore(self) -> None: "pyproject.toml", '[tool.mypy]\nfiles = ["pkg"]\n\n[tool.ruff.lint.per-file-ignores]\n"pkg/*.py" = ["E501"]\n', ) - self.assertViolation(repo, "ruff per-file-ignores 'pkg/*.py' has no rule") + self.assert_violation(repo, "ruff per-file-ignores 'pkg/*.py' has no rule") def test_file_level_exceptions_only_for_tests_ci_and_scripts(self) -> None: program = 'def main():\n pass\n\n\nif __name__ == "__main__":\n main()\n' @@ -495,7 +495,7 @@ def test_file_level_exceptions_only_for_tests_ci_and_scripts(self) -> None: def test_file_level_exception_outside_allowed_files_is_rejected(self) -> None: with Repo(with_rule(per_file_ignore_rule("routes", "pkg/*.py"))) as repo: repo.write("pyproject.toml", per_file_ignore_config("pkg/*.py")) - self.assertViolation( + self.assert_violation( repo, "covers files that may not take file-level exceptions: pkg/routes.py", ) @@ -510,25 +510,27 @@ def test_file_level_exception_needs_a_program_outside_scripts(self) -> None: } ) as repo: repo.write("pyproject.toml", per_file_ignore_config("pkg/cli.py")) - self.assertViolation(repo, "may not take file-level exceptions: pkg/cli.py") + self.assert_violation( + repo, "may not take file-level exceptions: pkg/cli.py" + ) def test_file_level_exception_star_crosses_directories(self) -> None: # Ruff's * also matches "/", so "*/routes.py" reaches pkg/routes.py. with Repo(with_rule(per_file_ignore_rule("star", "*/routes.py"))) as repo: repo.write("pyproject.toml", per_file_ignore_config("*/routes.py")) - self.assertViolation( + self.assert_violation( repo, "may not take file-level exceptions: pkg/routes.py" ) def test_negated_file_level_exception_is_rejected(self) -> None: with Repo(with_rule(per_file_ignore_rule("negated", "!tests/**"))) as repo: repo.write("pyproject.toml", per_file_ignore_config("!tests/**")) - self.assertViolation(repo, "pattern '!tests/**' is negated") + self.assert_violation(repo, "pattern '!tests/**' is negated") def test_file_level_exception_must_match_a_file(self) -> None: with Repo(with_rule(per_file_ignore_rule("stale", "nowhere/**"))) as repo: repo.write("pyproject.toml", per_file_ignore_config("nowhere/**")) - self.assertViolation(repo, "pattern 'nowhere/**' matches no files") + self.assert_violation(repo, "pattern 'nowhere/**' matches no files") def test_unregistered_ci_job(self) -> None: with Repo() as repo: @@ -537,7 +539,7 @@ def test_unregistered_ci_job(self) -> None: ".github/workflows/pr.yml", text + " deploy:\n steps:\n - run: ./deploy.sh\n", ) - self.assertViolation(repo, "CI job 'deploy'") + self.assert_violation(repo, "CI job 'deploy'") def test_ignored_ci_job_with_reason_passes(self) -> None: with Repo() as repo: diff --git a/tests/integration/test_alembic_v2_lifecycle.py b/tests/integration/test_alembic_v2_lifecycle.py index 12643f452..b62ef16c1 100644 --- a/tests/integration/test_alembic_v2_lifecycle.py +++ b/tests/integration/test_alembic_v2_lifecycle.py @@ -13,7 +13,7 @@ from alembic.config import Config from alembic.migration import MigrationContext from sqlalchemy import create_engine, inspect, text -from sqlalchemy.dialects.postgresql import UUID as PostgresUUID +from sqlalchemy.dialects import postgresql from policyengine_api.constants import REPO from policyengine_api.data.v2.migration_target import ( @@ -334,7 +334,7 @@ def test_empty_upgrade_check_base_downgrade_and_reupgrade() -> None: ) if column["name"] == "user_id" ) - assert isinstance(user_policy_id["type"], PostgresUUID) + assert isinstance(user_policy_id["type"], postgresql.UUID) user_columns = { column["name"]: column for column in inspect(engine).get_columns("users", schema="public") @@ -703,7 +703,7 @@ def report_run_checks() -> set[str]: try: command.upgrade(config, "head") - assert isinstance(idempotency_column_type(), PostgresUUID) + assert isinstance(idempotency_column_type(), postgresql.UUID) assert "ck_report_runs_idempotency_key_nonblank" not in report_run_checks() model_id = uuid4() diff --git a/tests/integration/test_simulations.py b/tests/integration/test_simulations.py index 64c53aa2e..fbaf856cb 100644 --- a/tests/integration/test_simulations.py +++ b/tests/integration/test_simulations.py @@ -23,8 +23,9 @@ def test__given_any_number_of_axes__sim_returns_valid_arrays( country = COUNTRIES.get(TEST_COUNTRY_ID) result = country.calculate(household_with_axes, {}).household - # This variable does not function like others; it is a list of member names and is not calculated - FORBIDDEN_VARIABLES = ["members"] + # This variable does not function like others; it is a list of member + # names and is not calculated. + uncalculated_variables = ["members"] # Verify every variable value is expanded across the configured axis. for entity_type in result: @@ -32,7 +33,7 @@ def test__given_any_number_of_axes__sim_returns_valid_arrays( continue for entity_id in result[entity_type]: for variable_name in result[entity_type][entity_id]: - if variable_name in FORBIDDEN_VARIABLES: + if variable_name in uncalculated_variables: continue for period in result[entity_type][entity_id][variable_name]: value = result[entity_type][entity_id][variable_name][period] diff --git a/tests/unit/routes/test_canonical_spm.py b/tests/unit/routes/test_canonical_spm.py index 3c7095625..38605f390 100644 --- a/tests/unit/routes/test_canonical_spm.py +++ b/tests/unit/routes/test_canonical_spm.py @@ -524,10 +524,10 @@ def test_valid_spm_database_timeout_keeps_stage11_safe_persistence_response( ): from unittest.mock import Mock - from sqlalchemy.exc import TimeoutError + from sqlalchemy.exc import TimeoutError as PoolTimeoutError sessions = Mock() - sessions.begin.side_effect = TimeoutError("database credential secret") + sessions.begin.side_effect = PoolTimeoutError("database credential secret") monkeypatch.setenv("DB_WRITE_HOUSEHOLD", write_source) monkeypatch.setattr( household_routes, "household_service", HouseholdService(sessions) diff --git a/tests/unit/services/test_policy_mirroring.py b/tests/unit/services/test_policy_mirroring.py index 8b9d81e11..ad2bdaf3f 100644 --- a/tests/unit/services/test_policy_mirroring.py +++ b/tests/unit/services/test_policy_mirroring.py @@ -6,7 +6,8 @@ from uuid import UUID import pytest -from sqlalchemy.exc import OperationalError, TimeoutError +from sqlalchemy.exc import OperationalError +from sqlalchemy.exc import TimeoutError as PoolTimeoutError from policyengine_api.services.policy_mirroring import ( PolicyMirrorUnavailableError, @@ -97,7 +98,7 @@ def test_logging_failure_does_not_change_successful_mirror_result() -> None: OperationalError("statement secret", {}, Exception("credential")), "database", ), - (TimeoutError("pool timeout"), "database"), + (PoolTimeoutError("pool timeout"), "database"), (LegacyPolicyMappingIntegrityError("changed source hash"), "integrity"), (RuntimeError("policy content secret"), "unexpected"), ], diff --git a/tests/unit/services/test_user_policy_mirroring.py b/tests/unit/services/test_user_policy_mirroring.py index ef9f70b89..681e2bb4b 100644 --- a/tests/unit/services/test_user_policy_mirroring.py +++ b/tests/unit/services/test_user_policy_mirroring.py @@ -8,7 +8,8 @@ import pytest from sqlalchemy import event as sqlalchemy_event from sqlalchemy import select -from sqlalchemy.exc import OperationalError, TimeoutError +from sqlalchemy.exc import OperationalError +from sqlalchemy.exc import TimeoutError as PoolTimeoutError from policyengine_api.data.v1_models import UserPolicyMirrorEvent from policyengine_api.services.user_policy_mirroring import ( @@ -135,7 +136,7 @@ def test_logging_failure_does_not_change_successful_mirror_result() -> None: OperationalError("statement secret", {}, Exception("credential")), "database", ), - (TimeoutError("pool timeout"), "database"), + (PoolTimeoutError("pool timeout"), "database"), (LegacyUserPolicyIntegrityError("mapping conflict"), "integrity"), (RuntimeError("caller data secret"), "unexpected"), ], diff --git a/tests/unit/test_observability_collector_verifier.py b/tests/unit/test_observability_collector_verifier.py index 06d7669d1..c3d07f8c2 100644 --- a/tests/unit/test_observability_collector_verifier.py +++ b/tests/unit/test_observability_collector_verifier.py @@ -54,7 +54,7 @@ class Stub: def __init__(self, _channel): pass - def Export(self, request, *, metadata, timeout): + def Export(self, request, *, metadata, timeout): # noqa: N802 # mirrors the gRPC stub method name exports[signal] = (request, metadata, timeout) if reject: raise _Unimplemented @@ -133,7 +133,7 @@ class AcceptingStub: def __init__(self, _channel): pass - def Export(self, *_args, **_kwargs): + def Export(self, *_args, **_kwargs): # noqa: N802 # mirrors the gRPC stub method name return _accepted_response() monkeypatch.setattr( diff --git a/tests/unit/v2/test_household_routes.py b/tests/unit/v2/test_household_routes.py index ec3b21bfb..30be00ec7 100644 --- a/tests/unit/v2/test_household_routes.py +++ b/tests/unit/v2/test_household_routes.py @@ -8,7 +8,8 @@ import pytest from fastapi.testclient import TestClient from flask import Flask, jsonify -from sqlalchemy.exc import OperationalError, TimeoutError +from sqlalchemy.exc import OperationalError +from sqlalchemy.exc import TimeoutError as PoolTimeoutError from policyengine_api.asgi_factory import create_asgi_app from policyengine_api.data.v2.settings import V2ConfigurationError @@ -316,7 +317,7 @@ def test_list_passes_exact_year_filter_and_canonical_pagination() -> None: 503, "unavailable", ), - (TimeoutError("pool timeout"), 503, "unavailable"), + (PoolTimeoutError("pool timeout"), 503, "unavailable"), (RuntimeError("credential and household value"), 500, "operation failed"), ], ) diff --git a/tests/unit/v2/test_policy_routes.py b/tests/unit/v2/test_policy_routes.py index 4d2c8db50..0eb153d19 100644 --- a/tests/unit/v2/test_policy_routes.py +++ b/tests/unit/v2/test_policy_routes.py @@ -8,7 +8,8 @@ import pytest from fastapi.testclient import TestClient from flask import Flask, jsonify -from sqlalchemy.exc import OperationalError, TimeoutError +from sqlalchemy.exc import OperationalError +from sqlalchemy.exc import TimeoutError as PoolTimeoutError from policyengine_api.asgi_factory import create_asgi_app from policyengine_api.data.v2.catalog.catalog_selection import ( @@ -327,7 +328,7 @@ def test_list_passes_exact_model_filter_and_canonical_pagination() -> None: 503, "unavailable", ), - (TimeoutError("pool timeout"), 503, "unavailable"), + (PoolTimeoutError("pool timeout"), 503, "unavailable"), (RuntimeError("credential and policy value"), 500, "operation failed"), ], ) From 9e9e707a29991431c7b3172f6ca439504a5871bd Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 22:11:58 +0400 Subject: [PATCH 04/15] Declare the directories that have no __init__.py on purpose (INP001) policyengine_api is a namespace package (its routes and libs directories included), and .github, dashboard/experiments, the two Alembic environments and rules/fixtures hold scripts and fixtures that are never imported as packages. Listing them in ruff's namespace-packages changes no other rule's result, except N999 on the hyphenated .github/find-api-model-versions.py, which keeps its name because update-policyengine-package.sh runs it by that path. Co-Authored-By: Claude Opus 5.5 --- .github/find-api-model-versions.py | 1 + pyproject.toml | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/.github/find-api-model-versions.py b/.github/find-api-model-versions.py index 9ae2ab7d4..619a5f73a 100644 --- a/.github/find-api-model-versions.py +++ b/.github/find-api-model-versions.py @@ -1,3 +1,4 @@ +# noqa: N999 # update-policyengine-package.sh runs this script by its hyphenated path """CI helper that reads the versions pinned by the installed PolicyEngine bundle. It checks the package and data versions against the API's version constants, diff --git a/pyproject.toml b/pyproject.toml index e3522dda9..edfb6c71d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -80,6 +80,24 @@ packages = ["policyengine_api"] [tool.hatch.metadata] allow-direct-references = true +[tool.ruff] +# Directories with no __init__.py on purpose (INP001). Ruff treats each one, and +# every directory below it, as a package when it resolves modules. +# - policyengine_api is a namespace package, which mypy reads with +# explicit_package_bases. This also covers its routes and libs directories, +# which have no __init__.py either. +# - .github (with .github/scripts), dashboard/experiments and migrations/v1 and +# v2 hold standalone scripts and Alembic environments, run but never imported. +# - rules/fixtures holds rule proof fixtures, which tests read as source. +namespace-packages = [ + ".github", + "dashboard/experiments", + "migrations/v1", + "migrations/v2", + "policyengine_api", + "rules/fixtures", +] + [tool.ruff.lint] # Rule families are enabled one themed group per PR, each fixing everything it # finds (no ratchet), until the select list is "ALL". See From 6318e3015245f59282d3e1900623920bd8e712ff Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 22:38:22 +0400 Subject: [PATCH 05/15] Move the safe imports to the top level and explain the rest (PLC0415) Move four imports out of functions: request_context now reads get_runtime through the observability package it already imports, the simulation entrypoint imports SPMValidationError at the top (spm was already loaded), the four v2 resource routers import their default service factories, and the route dependencies import is_ready. None creates a cycle or loads a third-party or standard-library module that was not already loaded; the ASGI and Flask entry points import exactly the same modules as before. The remaining deferred imports keep an inline reason: they load country models, PolicyEngine.py, spm_calculator, Google clients, the v2 persistence stack (whose database module registers an atexit hook) or the bundle manifest, break an import cycle, need configuration at import, or are looked up per call so tests can replace them. Co-Authored-By: Claude Opus 5.5 --- .../data/v2/catalog/extraction.py | 6 +- policyengine_api/data/v2/settings.py | 4 +- .../fastapi_routes/dependencies.py | 61 +++++++++++++------ .../fastapi_routes/v2/households/routes.py | 7 +-- .../fastapi_routes/v2/metadata/common.py | 2 +- .../fastapi_routes/v2/policies/routes.py | 7 +-- .../v2/user_households/routes.py | 7 +-- .../fastapi_routes/v2/user_policies/routes.py | 7 +-- policyengine_api/libs/gateway_auth.py | 4 +- .../libs/simulation_entrypoint.py | 7 +-- policyengine_api/readiness.py | 8 ++- policyengine_api/request_context.py | 5 +- .../services/household_calculation_service.py | 8 ++- .../services/household_mirroring.py | 10 ++- policyengine_api/services/policy_mirroring.py | 10 ++- .../services/user_policy_mirroring.py | 8 ++- .../services/v2/policies/validators.py | 4 +- policyengine_api/spm.py | 13 +++- policyengine_api/warmup.py | 4 +- 19 files changed, 114 insertions(+), 68 deletions(-) diff --git a/policyengine_api/data/v2/catalog/extraction.py b/policyengine_api/data/v2/catalog/extraction.py index 4111c966a..571c40c85 100644 --- a/policyengine_api/data/v2/catalog/extraction.py +++ b/policyengine_api/data/v2/catalog/extraction.py @@ -745,8 +745,10 @@ def extract_catalog( def extract_installed_catalog() -> NormalizedCatalog: """Load and normalize the installed PolicyEngine.py certified catalog.""" - import policyengine - from policyengine.bundle import get_current_bundle + import policyengine # noqa: PLC0415 # policyengine loads every country model; only catalog extraction needs it + from policyengine.bundle import ( # noqa: PLC0415 # policyengine loads every country model; only catalog extraction needs it + get_current_bundle, + ) policyengine_version = importlib_metadata.version("policyengine") return extract_catalog( diff --git a/policyengine_api/data/v2/settings.py b/policyengine_api/data/v2/settings.py index 33d8fdfea..8e154dba9 100644 --- a/policyengine_api/data/v2/settings.py +++ b/policyengine_api/data/v2/settings.py @@ -92,7 +92,9 @@ def _required(environ: Mapping[str, str], name: str) -> str: @cache def _load_secret_from_secret_manager(resource_name: str) -> str: """Resolve a v2 runtime URL only when a preview request selects it.""" - from google.cloud import secretmanager + from google.cloud import ( # noqa: PLC0415 # the Google client loads only when a secret resource is configured + secretmanager, + ) client = secretmanager.SecretManagerServiceClient() response = client.access_secret_version(request={"name": resource_name}) diff --git a/policyengine_api/fastapi_routes/dependencies.py b/policyengine_api/fastapi_routes/dependencies.py index a7d0712a3..bf4bab2ba 100644 --- a/policyengine_api/fastapi_routes/dependencies.py +++ b/policyengine_api/fastapi_routes/dependencies.py @@ -11,6 +11,7 @@ from fastapi.security import APIKeyHeader, HTTPAuthorizationCredentials, HTTPBearer from policyengine_api.libs.user_auth import UNAUTHENTICATED_USER_HEADER +from policyengine_api.readiness import is_ready if TYPE_CHECKING: from collections.abc import Callable @@ -695,25 +696,29 @@ def health_check(self) -> bool: def _default_readiness_probe() -> bool: - from policyengine_api.readiness import is_ready - return is_ready() def _default_gateway_client_factory() -> SimulationGatewayProbe: - from policyengine_api.libs.simulation_entrypoint import SimulationEntrypointClient + from policyengine_api.libs.simulation_entrypoint import ( # noqa: PLC0415 # builds its client at import, which needs the gateway URL configured + SimulationEntrypointClient, + ) return SimulationEntrypointClient() def _default_metadata_reader_factory() -> MetadataReader: - from policyengine_api.services.metadata_service import MetadataService + from policyengine_api.services.metadata_service import ( # noqa: PLC0415 # loads every country model; only the default reader needs it + MetadataService, + ) return MetadataService() def _default_specification_provider() -> JSONObject: - from policyengine_api.specification import OPENAPI_SPECIFICATION + from policyengine_api.specification import ( # noqa: PLC0415 # reads and builds the OpenAPI document at import + OPENAPI_SPECIFICATION, + ) return OPENAPI_SPECIFICATION @@ -724,11 +729,15 @@ def _running_policyengine_version() -> str: def _default_v2_metadata_reader_factory() -> V2MetadataResourceReader: - from policyengine_api.data.v2.database import get_v2_session_factory - from policyengine_api.services.v2.metadata.database_session import ( + from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + get_v2_session_factory, + ) + from policyengine_api.services.v2.metadata.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs MetadataDatabaseSession, ) - from policyengine_api.services.v2.metadata.services import V2MetadataService + from policyengine_api.services.v2.metadata.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs + V2MetadataService, + ) return V2MetadataService( MetadataDatabaseSession(get_v2_session_factory()()), @@ -737,11 +746,15 @@ def _default_v2_metadata_reader_factory() -> V2MetadataResourceReader: def _default_v2_policy_service_factory() -> V2PolicyResourceService: - from policyengine_api.data.v2.database import get_v2_session_factory - from policyengine_api.services.v2.policies.database_session import ( + from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + get_v2_session_factory, + ) + from policyengine_api.services.v2.policies.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs PolicyDatabaseSession, ) - from policyengine_api.services.v2.policies.services import V2PolicyService + from policyengine_api.services.v2.policies.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs + V2PolicyService, + ) return V2PolicyService( PolicyDatabaseSession(get_v2_session_factory()), @@ -750,11 +763,15 @@ def _default_v2_policy_service_factory() -> V2PolicyResourceService: def _default_v2_household_service_factory() -> V2HouseholdResourceService: - from policyengine_api.data.v2.database import get_v2_session_factory - from policyengine_api.services.v2.households.database_session import ( + from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + get_v2_session_factory, + ) + from policyengine_api.services.v2.households.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs HouseholdDatabaseSession, ) - from policyengine_api.services.v2.households.services import V2HouseholdService + from policyengine_api.services.v2.households.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs + V2HouseholdService, + ) return V2HouseholdService( HouseholdDatabaseSession(get_v2_session_factory()), @@ -817,11 +834,13 @@ def _default_legacy_reference_checker() -> LegacyReferenceChecker: def _default_v2_user_policy_service_factory() -> V2UserPolicyResourceService: - from policyengine_api.data.v2.database import get_v2_session_factory - from policyengine_api.services.v2.user_policies.database_session import ( + from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + get_v2_session_factory, + ) + from policyengine_api.services.v2.user_policies.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs UserPolicyDatabaseSession, ) - from policyengine_api.services.v2.user_policies.services import ( + from policyengine_api.services.v2.user_policies.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs V2UserPolicyService, ) @@ -832,11 +851,13 @@ def _default_v2_user_policy_service_factory() -> V2UserPolicyResourceService: def _default_v2_user_household_service_factory() -> V2UserHouseholdResourceService: - from policyengine_api.data.v2.database import get_v2_session_factory - from policyengine_api.services.v2.user_households.database_session import ( + from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + get_v2_session_factory, + ) + from policyengine_api.services.v2.user_households.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs UserHouseholdDatabaseSession, ) - from policyengine_api.services.v2.user_households.services import ( + from policyengine_api.services.v2.user_households.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs V2UserHouseholdService, ) diff --git a/policyengine_api/fastapi_routes/v2/households/routes.py b/policyengine_api/fastapi_routes/v2/households/routes.py index 59c74b7fe..630e5f708 100644 --- a/policyengine_api/fastapi_routes/v2/households/routes.py +++ b/policyengine_api/fastapi_routes/v2/households/routes.py @@ -10,6 +10,9 @@ from starlette.responses import JSONResponse from policyengine_api.data.v2.settings import V2ConfigurationError +from policyengine_api.fastapi_routes.dependencies import ( + _default_v2_household_service_factory, +) from policyengine_api.fastapi_routes.query_parameters import query_dependency from policyengine_api.fastapi_routes.v2.errors import ( V2RequestTooLargeError, @@ -81,10 +84,6 @@ def _service_factory( ) -> Callable[[], V2HouseholdResourceService]: if dependencies.v2_household_service_factory is not None: return dependencies.v2_household_service_factory - from policyengine_api.fastapi_routes.dependencies import ( - _default_v2_household_service_factory, - ) - return _default_v2_household_service_factory diff --git a/policyengine_api/fastapi_routes/v2/metadata/common.py b/policyengine_api/fastapi_routes/v2/metadata/common.py index 366860544..52eae3a80 100644 --- a/policyengine_api/fastapi_routes/v2/metadata/common.py +++ b/policyengine_api/fastapi_routes/v2/metadata/common.py @@ -78,7 +78,7 @@ def read_resource( try: factory = dependencies.v2_metadata_reader_factory if factory is None: - from policyengine_api.fastapi_routes.dependencies import ( + from policyengine_api.fastapi_routes.dependencies import ( # noqa: PLC0415 # looked up per request, so tests can replace the default factory _default_v2_metadata_reader_factory, ) diff --git a/policyengine_api/fastapi_routes/v2/policies/routes.py b/policyengine_api/fastapi_routes/v2/policies/routes.py index 063d87c66..f9b885d6e 100644 --- a/policyengine_api/fastapi_routes/v2/policies/routes.py +++ b/policyengine_api/fastapi_routes/v2/policies/routes.py @@ -14,6 +14,9 @@ MetadataCatalogVersionNotFoundError, ) from policyengine_api.data.v2.settings import V2ConfigurationError +from policyengine_api.fastapi_routes.dependencies import ( + _default_v2_policy_service_factory, +) from policyengine_api.fastapi_routes.query_parameters import query_dependency from policyengine_api.fastapi_routes.v2.errors import ( V2RequestTooLargeError, @@ -86,10 +89,6 @@ def _service_factory( ) -> Callable[[], V2PolicyResourceService]: if dependencies.v2_policy_service_factory is not None: return dependencies.v2_policy_service_factory - from policyengine_api.fastapi_routes.dependencies import ( - _default_v2_policy_service_factory, - ) - return _default_v2_policy_service_factory diff --git a/policyengine_api/fastapi_routes/v2/user_households/routes.py b/policyengine_api/fastapi_routes/v2/user_households/routes.py index 42b9d6cb8..836a3c76d 100644 --- a/policyengine_api/fastapi_routes/v2/user_households/routes.py +++ b/policyengine_api/fastapi_routes/v2/user_households/routes.py @@ -8,6 +8,9 @@ from fastapi import APIRouter, Depends from starlette.responses import JSONResponse, Response +from policyengine_api.fastapi_routes.dependencies import ( + _default_v2_user_household_service_factory, +) from policyengine_api.fastapi_routes.query_parameters import query_dependency from policyengine_api.fastapi_routes.v2.errors import v2_error_response from policyengine_api.fastapi_routes.v2.user_households.request_models import ( @@ -67,10 +70,6 @@ def _service_factory( ) -> Callable[[], V2UserHouseholdResourceService]: if dependencies.v2_user_household_service_factory is not None: return dependencies.v2_user_household_service_factory - from policyengine_api.fastapi_routes.dependencies import ( - _default_v2_user_household_service_factory, - ) - return _default_v2_user_household_service_factory diff --git a/policyengine_api/fastapi_routes/v2/user_policies/routes.py b/policyengine_api/fastapi_routes/v2/user_policies/routes.py index 702ba0c45..f7078b485 100644 --- a/policyengine_api/fastapi_routes/v2/user_policies/routes.py +++ b/policyengine_api/fastapi_routes/v2/user_policies/routes.py @@ -8,6 +8,9 @@ from fastapi import APIRouter, Depends from starlette.responses import JSONResponse, Response +from policyengine_api.fastapi_routes.dependencies import ( + _default_v2_user_policy_service_factory, +) from policyengine_api.fastapi_routes.query_parameters import query_dependency from policyengine_api.fastapi_routes.v2.errors import v2_error_response from policyengine_api.fastapi_routes.v2.user_policies.request_models import ( @@ -54,10 +57,6 @@ def _service_factory( ) -> Callable[[], V2UserPolicyResourceService]: if dependencies.v2_user_policy_service_factory is not None: return dependencies.v2_user_policy_service_factory - from policyengine_api.fastapi_routes.dependencies import ( - _default_v2_user_policy_service_factory, - ) - return _default_v2_user_policy_service_factory diff --git a/policyengine_api/libs/gateway_auth.py b/policyengine_api/libs/gateway_auth.py index c940f61db..110467d06 100644 --- a/policyengine_api/libs/gateway_auth.py +++ b/policyengine_api/libs/gateway_auth.py @@ -64,7 +64,9 @@ def gateway_auth_required() -> bool: @cache def _load_secret_from_secret_manager(resource_name: str) -> str: """Fetch one secret payload from Google Secret Manager.""" - from google.cloud import secretmanager + from google.cloud import ( # noqa: PLC0415 # the Google client loads only when gateway auth reads a secret + secretmanager, + ) client = secretmanager.SecretManagerServiceClient() response = client.access_secret_version(request={"name": resource_name}) diff --git a/policyengine_api/libs/simulation_entrypoint.py b/policyengine_api/libs/simulation_entrypoint.py index b1995d42a..0583221c8 100644 --- a/policyengine_api/libs/simulation_entrypoint.py +++ b/policyengine_api/libs/simulation_entrypoint.py @@ -25,6 +25,7 @@ current_observability_id, current_request_id, ) +from policyengine_api.spm import SPMValidationError from policyengine_api.worker_spm import raise_worker_spm_error, validate_worker_spm @@ -256,7 +257,7 @@ def _normalize_submission_payload(self, payload: dict) -> dict: msg = "Dataset overrides are not supported" raise ValueError(msg) - from policyengine_api.constants import ( + from policyengine_api.constants import ( # noqa: PLC0415 # constants reads the installed bundle manifest at import COUNTRY_PACKAGE_VERSIONS, POLICYENGINE_VERSION, ) @@ -273,8 +274,6 @@ def _normalize_submission_payload(self, payload: dict) -> dict: ) if resolved is not None: if bundle_version == "latest": - from policyengine_api.spm import SPMValidationError - error_code = "SPM_SETTINGS_INVALID" raise SPMValidationError( error_code, @@ -423,8 +422,6 @@ def get_spm_capability( bundle publishes no capability. Raise SPMValidationError if the routes are unreachable or ambiguous. """ - from policyengine_api.spm import SPMValidationError - try: response = self.client.get(f"{self.base_url}/versions") response.raise_for_status() diff --git a/policyengine_api/readiness.py b/policyengine_api/readiness.py index 7d7eb4bb3..69bae7f08 100644 --- a/policyengine_api/readiness.py +++ b/policyengine_api/readiness.py @@ -16,20 +16,22 @@ def validate_policy_runtime_configuration() -> None: """Validate v1 source selectors, v2 persistence, user auth and measurement.""" - from policyengine_api.data.v2.settings import ( + from policyengine_api.data.v2.settings import ( # noqa: PLC0415 # keeps importing readiness cheap; only validation needs configuration load_v2_runtime_database_settings, ) from policyengine_api.libs.auth0_management import ( load_auth0_management_settings, ) from policyengine_api.libs.user_auth import load_user_auth_settings - from policyengine_api.migration_flags import ( + from policyengine_api.migration_flags import ( # noqa: PLC0415 # keeps importing readiness cheap; only validation needs configuration get_v1_household_read_source, get_v1_household_write_source, get_v1_policy_read_source, get_v1_policy_write_source, ) - from policyengine_api.spm import normalize_spm_selection + from policyengine_api.spm import ( # noqa: PLC0415 # keeps importing readiness cheap; only validation needs configuration + normalize_spm_selection, + ) get_v1_policy_write_source() get_v1_policy_read_source() diff --git a/policyengine_api/request_context.py b/policyengine_api/request_context.py index e8eb7c270..e5e32f088 100644 --- a/policyengine_api/request_context.py +++ b/policyengine_api/request_context.py @@ -7,6 +7,7 @@ import flask +from policyengine_api import observability from policyengine_api.observability.identifiers import ( generate_observability_id, normalize_observability_id, @@ -60,9 +61,7 @@ def _bind_observability_id(observability_id: str) -> str: else: _asgi_observability_id.set(observability_id) try: - from policyengine_api.observability import get_runtime - - get_runtime().set_context(observability_id=observability_id) + observability.get_runtime().set_context(observability_id=observability_id) except Exception: # noqa: BLE001, S110 # telemetry must never fail the request pass return observability_id diff --git a/policyengine_api/services/household_calculation_service.py b/policyengine_api/services/household_calculation_service.py index 3f54eeabc..454b78190 100644 --- a/policyengine_api/services/household_calculation_service.py +++ b/policyengine_api/services/household_calculation_service.py @@ -131,7 +131,9 @@ def add_yearly_variables( changed in place and returned. """ if countries is None: - from policyengine_api.country import COUNTRIES + from policyengine_api.country import ( # noqa: PLC0415 # country imports this module and loads every country model + COUNTRIES, + ) countries = COUNTRIES metadata = countries.get(country_id).metadata @@ -179,7 +181,9 @@ def _primary_sessions(self) -> sessionmaker[Session]: def _countries(self) -> dict: if self._country_provider is not None: return self._country_provider() - from policyengine_api.country import COUNTRIES + from policyengine_api.country import ( # noqa: PLC0415 # country imports this module and loads every country model + COUNTRIES, + ) return COUNTRIES diff --git a/policyengine_api/services/household_mirroring.py b/policyengine_api/services/household_mirroring.py index 7ee534d87..26bd073ce 100644 --- a/policyengine_api/services/household_mirroring.py +++ b/policyengine_api/services/household_mirroring.py @@ -46,11 +46,15 @@ class HouseholdMirrorUnavailableError(RuntimeError): def _default_mirror_factory() -> LegacyHouseholdMirror: - from policyengine_api.data.v2.database import get_v2_session_factory - from policyengine_api.services.v2.households.database_session import ( + from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + get_v2_session_factory, + ) + from policyengine_api.services.v2.households.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs HouseholdDatabaseSession, ) - from policyengine_api.services.v2.households.services import V2HouseholdService + from policyengine_api.services.v2.households.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs + V2HouseholdService, + ) return V2HouseholdService(HouseholdDatabaseSession(get_v2_session_factory())) diff --git a/policyengine_api/services/policy_mirroring.py b/policyengine_api/services/policy_mirroring.py index 64209c9a6..7757baeb5 100644 --- a/policyengine_api/services/policy_mirroring.py +++ b/policyengine_api/services/policy_mirroring.py @@ -46,11 +46,15 @@ class PolicyMirrorUnavailableError(RuntimeError): def _default_mirror_factory() -> LegacyPolicyMirror: - from policyengine_api.data.v2.database import get_v2_session_factory - from policyengine_api.services.v2.policies.database_session import ( + from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + get_v2_session_factory, + ) + from policyengine_api.services.v2.policies.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs PolicyDatabaseSession, ) - from policyengine_api.services.v2.policies.services import V2PolicyService + from policyengine_api.services.v2.policies.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs + V2PolicyService, + ) return V2PolicyService(PolicyDatabaseSession(get_v2_session_factory())) diff --git a/policyengine_api/services/user_policy_mirroring.py b/policyengine_api/services/user_policy_mirroring.py index c14fe6c60..3f92f9152 100644 --- a/policyengine_api/services/user_policy_mirroring.py +++ b/policyengine_api/services/user_policy_mirroring.py @@ -59,11 +59,13 @@ class UserPolicyMirrorUnavailableError(RuntimeError): def _default_mirror_factory() -> LegacyUserPolicyMirror: - from policyengine_api.data.v2.database import get_v2_session_factory - from policyengine_api.services.v2.user_policies.database_session import ( + from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + get_v2_session_factory, + ) + from policyengine_api.services.v2.user_policies.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs UserPolicyDatabaseSession, ) - from policyengine_api.services.v2.user_policies.services import ( + from policyengine_api.services.v2.user_policies.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs V2UserPolicyService, ) diff --git a/policyengine_api/services/v2/policies/validators.py b/policyengine_api/services/v2/policies/validators.py index 84ba23cbe..974423f55 100644 --- a/policyengine_api/services/v2/policies/validators.py +++ b/policyengine_api/services/v2/policies/validators.py @@ -97,7 +97,9 @@ def validate_policy_catalog( resolved_parameter_ids: set[UUID], ) -> ResolvedPolicyCreationInput: """Validate preloaded catalog records and bind them to policy content.""" - from policyengine_api.services.v2.policies.types import ResolvedPolicyCreationInput + from policyengine_api.services.v2.policies.types import ( # noqa: PLC0415 # types imports this module + ResolvedPolicyCreationInput, + ) if policy_input.tax_benefit_model_id != selected.model.id: msg = "tax_benefit_model_id does not match the selected country catalog" diff --git a/policyengine_api/spm.py b/policyengine_api/spm.py index 1640f9c8e..e729c754b 100644 --- a/policyengine_api/spm.py +++ b/policyengine_api/spm.py @@ -454,7 +454,9 @@ def validate_spm_calculation_provenance( def _current_bundle() -> dict: - from policyengine_api.constants import _policyengine_bundle + from policyengine_api.constants import ( # noqa: PLC0415 # constants reads the installed bundle manifest at import + _policyengine_bundle, + ) return _policyengine_bundle @@ -469,7 +471,10 @@ def _installed_country_implements_spm(country_id: str) -> bool: it computes SPM exactly as it always has, and a model with it still needs a certified configuration before this API will run it. """ - from policyengine_api.constants import COUNTRIES, COUNTRY_PACKAGE_NAMES + from policyengine_api.constants import ( # noqa: PLC0415 # constants reads the installed bundle manifest at import + COUNTRIES, + COUNTRY_PACKAGE_NAMES, + ) package_name = dict(zip(COUNTRIES, COUNTRY_PACKAGE_NAMES)).get(country_id) if package_name is None: @@ -493,7 +498,9 @@ def _installed_country_implements_spm(country_id: str) -> bool: @lru_cache(maxsize=4) def _selected_forecast(expected_sha256: str) -> SPMForecast: - from spm_calculator.rolling_forecast import load_forecast + from spm_calculator.rolling_forecast import ( # noqa: PLC0415 # spm_calculator loads only when a forecast is selected + load_forecast, + ) return load_forecast(expected_sha256=expected_sha256) diff --git a/policyengine_api/warmup.py b/policyengine_api/warmup.py index 92200b95d..f947bebfc 100644 --- a/policyengine_api/warmup.py +++ b/policyengine_api/warmup.py @@ -46,7 +46,9 @@ def run_startup_warmup() -> None: from serving (the first real request would just be slow), and it must never leave the service permanently unready. """ - from policyengine_api.country import COUNTRIES + from policyengine_api.country import ( # noqa: PLC0415 # country models load during warmup, not at import + COUNTRIES, + ) for country_id in _requested_countries(): household = WARMUP_HOUSEHOLDS.get(country_id) From a2b1d2ddfaeefa4f274aa224e6d4c602ac432131 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 22:42:00 +0400 Subject: [PATCH 06/15] Wrap long lines in the application, scripts and fixtures (E501) Long strings are split by implicit concatenation, so every log message, error message and published Field description keeps its exact text; long comments are rewrapped or moved above the line they describe. Only four docstrings change, none of them published: a Flask route's, two internal functions' and a rule proof fixture's. Three lines keep a suppression: a dotted module path that cannot be wrapped, and the GPT-4 prompts and a script's --help summary, whose text would change if wrapped. Co-Authored-By: Claude Opus 5.5 --- dashboard/experiments/gpt4_api_user.py | 7 ++-- policyengine_api/constants.py | 12 ++++--- policyengine_api/country.py | 10 ++++-- .../libs/simulation_entrypoint.py | 28 ++++++++++++---- policyengine_api/query_parameters.py | 9 +++-- .../routes/report_output_routes.py | 3 +- .../services/economy_comparison.py | 17 +++++----- policyengine_api/services/economy_service.py | 33 +++++++++++++++---- .../services/report_spec_service.py | 5 ++- .../services/v2/comparison_runs/validators.py | 10 ++++-- .../services/v2/households/validators.py | 5 ++- .../services/v2/metadata/services.py | 2 +- policyengine_api/spm.py | 5 ++- .../validate_household_payload.py | 3 +- .../good_validators.py | 5 ++- scripts/measure_cloud_run_runtime.py | 2 +- scripts/process_v1_household_mirror_event.py | 5 ++- 17 files changed, 116 insertions(+), 45 deletions(-) diff --git a/dashboard/experiments/gpt4_api_user.py b/dashboard/experiments/gpt4_api_user.py index cf3dfbe61..154144fe4 100644 --- a/dashboard/experiments/gpt4_api_user.py +++ b/dashboard/experiments/gpt4_api_user.py @@ -51,7 +51,8 @@ def embed(prompt: str, engine: str = "text-embedding-ada-002") -> list[float]: # its values by period, for example hmrc.income_tax.rates.uk[0].rate set to 0.25 # for the period 2023-01-01.2024-01-01. -# We're going to do this with GPT4- we'll pass in the question, the related parameter metadata, and the task description. +# We're going to do this with GPT4- we'll pass in the question, the related parameter +# metadata, and the task description. # First, get related parameters via a search through GPT-embedded parameters. @@ -81,7 +82,7 @@ def write_prompt(question: str, relevant_parameters: object) -> str: Reform syntax you should write in: {{ "parameter_name": {{ "start_date.end_date": value }} }} e.g. "basic rate at 25%" -> {{ "hmrc.income_tax.rates.uk[0].rate": {{ "2023-01-01.2024-01-01": 0.25 }} }} Reply only with valid JSON. - """ + """ # noqa: E501 # the prompt is sent to GPT-4 verbatim; wrapping would change it reform_generation_prompt = write_prompt(question, top5) @@ -146,7 +147,7 @@ def get_policyengine_impact(reform: object) -> object: Task: Write a concise single-sentence response to the question, using only the impact data. If you can't from the data, say it. Give financial amounts in short-form (e.g. £1bn, 250m, etc.). -""" +""" # noqa: E501 # the prompt is sent to GPT-4 verbatim; wrapping would change it answer = get_gpt4_response(question_answer_prompt) diff --git a/policyengine_api/constants.py b/policyengine_api/constants.py index 4ff7c2ed1..492bbde03 100644 --- a/policyengine_api/constants.py +++ b/policyengine_api/constants.py @@ -246,13 +246,15 @@ def get_report_output_cache_version(country_id: str) -> str: "national", # National level (e.g., "us") "state", # US states (e.g., "state/ca", "state/ny") "place", # US Census places (e.g., "place/NJ-57000") - "congressional_district", # US congressional districts (e.g., "congressional_district/CA-37") + # US congressional districts (e.g., "congressional_district/CA-37") + "congressional_district", ) UK_REGION_TYPES = ( "national", # National level (e.g., "uk") "country", # UK countries (e.g., "country/england", "country/scotland") - "constituency", # UK parliamentary constituencies (e.g., "constituency/Aldershot") + # UK parliamentary constituencies (e.g., "constituency/Aldershot") + "constituency", "local_authority", # UK local authorities (e.g., "local_authority/Maidstone") ) @@ -262,11 +264,13 @@ def get_report_output_cache_version(country_id: str) -> str: "us": [ "state/", # US states (e.g., "state/ca", "state/ny") "place/", # US Census places (e.g., "place/NJ-57000") - "congressional_district/", # US congressional districts (e.g., "congressional_district/CA-37") + # US congressional districts (e.g., "congressional_district/CA-37") + "congressional_district/", ], "uk": [ "country/", # UK countries (e.g., "country/england", "country/scotland") - "constituency/", # UK parliamentary constituencies (e.g., "constituency/Aldershot") + # UK parliamentary constituencies (e.g., "constituency/Aldershot") + "constituency/", "local_authority/", # UK local authorities (e.g., "local_authority/Maidstone") ], } diff --git a/policyengine_api/country.py b/policyengine_api/country.py index 1c5228baa..94aa6ff6a 100644 --- a/policyengine_api/country.py +++ b/policyengine_api/country.py @@ -179,7 +179,8 @@ def _json_safe( def build_microsimulation_options(self) -> dict: """Build the regions, time periods and datasets offered for economy runs.""" - # { region: [{ name: "uk", label: "the UK" }], time_period: [{ name: 2022, label: "2022", ... }] } + # Shape: { region: [{ name: "uk", label: "the UK" }], + # time_period: [{ name: 2022, label: "2022", ... }] } options = dict() if self.country_id == "uk": constituency_names_path = ( @@ -396,7 +397,8 @@ def build_parameters(self) -> dict: } elif isinstance(parameter, ParameterScaleBracket): bracket_index = int(parameter.name[parameter.name.index("[") + 1 : -1]) - # Set the label to 'first bracket' for the first bracket, 'second bracket' for the second, etc. + # Set the label to 'first bracket' for the first bracket, 'second + # bracket' for the second, etc. bracket_label = f"bracket {bracket_index + 1}" parameter_data[parameter.name] = { "type": "parameterNode", @@ -681,7 +683,9 @@ def modify_parameters(parameters: ParameterNode) -> ParameterNode: start, end = period.split(".") node_type = type(node.values_list[-1].value) if node_type is int: - node_type = float # '0' is of type int by default, but usually we want to cast to float. + # '0' is of type int by default, but usually we want to cast to + # float. + node_type = float if node.values_list[-1].value is None: node_type = float node.update( diff --git a/policyengine_api/libs/simulation_entrypoint.py b/policyengine_api/libs/simulation_entrypoint.py index 0583221c8..c083615e2 100644 --- a/policyengine_api/libs/simulation_entrypoint.py +++ b/policyengine_api/libs/simulation_entrypoint.py @@ -343,7 +343,9 @@ def run(self, payload: dict) -> ModalSimulationExecution: except httpx.HTTPStatusError as e: logger.log_struct( { - "message": f"Simulation entrypoint HTTP error: {e.response.status_code}", + "message": ( + f"Simulation entrypoint HTTP error: {e.response.status_code}" + ), "observability_id": current_observability_id(), "response_text": e.response.text[:500], }, @@ -391,7 +393,9 @@ def run_budget_window_batch(self, payload: dict) -> ModalBudgetWindowBatchExecut except httpx.HTTPStatusError as e: logger.log_struct( { - "message": f"Simulation batch API HTTP error: {e.response.status_code}", + "message": ( + f"Simulation batch API HTTP error: {e.response.status_code}" + ), "response_text": e.response.text[:500], }, severity="ERROR", @@ -526,7 +530,10 @@ def get_execution_by_id(self, job_id: str) -> ModalSimulationExecution: except httpx.HTTPStatusError as e: logger.log_struct( { - "message": f"Simulation entrypoint HTTP error polling job {job_id}: {e.response.status_code}", + "message": ( + f"Simulation entrypoint HTTP error polling job {job_id}: " + f"{e.response.status_code}" + ), "response_text": e.response.text[:500], }, severity="ERROR", @@ -536,7 +543,10 @@ def get_execution_by_id(self, job_id: str) -> ModalSimulationExecution: except httpx.RequestError as e: logger.log_struct( { - "message": f"Simulation entrypoint request error polling job {job_id}: {str(e)}", + "message": ( + f"Simulation entrypoint request error polling job {job_id}: " + f"{str(e)}" + ), }, severity="ERROR", ) @@ -570,7 +580,10 @@ def get_budget_window_batch_by_id( except httpx.HTTPStatusError as e: logger.log_struct( { - "message": f"Simulation batch API HTTP error polling job {batch_job_id}: {e.response.status_code}", + "message": ( + "Simulation batch API HTTP error polling job " + f"{batch_job_id}: {e.response.status_code}" + ), "response_text": e.response.text[:500], }, severity="ERROR", @@ -580,7 +593,10 @@ def get_budget_window_batch_by_id( except httpx.RequestError as e: logger.log_struct( { - "message": f"Simulation batch API request error polling job {batch_job_id}: {str(e)}", + "message": ( + "Simulation batch API request error polling job " + f"{batch_job_id}: {str(e)}" + ), }, severity="ERROR", ) diff --git a/policyengine_api/query_parameters.py b/policyengine_api/query_parameters.py index e48a42ae8..06e38cf01 100644 --- a/policyengine_api/query_parameters.py +++ b/policyengine_api/query_parameters.py @@ -177,7 +177,9 @@ class BudgetWindowEconomyQuery(EconomyQuery): window_size: Annotated[int, BeforeValidator(parse_integer_query_value)] = Field( ge=1, le=BUDGET_WINDOW_MAX_YEARS, - description="Number of years; start_year + window_size - 1 must not exceed 2099", + description=( + "Number of years; start_year + window_size - 1 must not exceed 2099" + ), ) target: Literal["general"] = "general" @@ -185,7 +187,10 @@ class BudgetWindowEconomyQuery(EconomyQuery): def validate_end_year(self) -> BudgetWindowEconomyQuery: """Reject a window whose last year falls after BUDGET_WINDOW_MAX_END_YEAR.""" if int(self.start_year) + self.window_size - 1 > BUDGET_WINDOW_MAX_END_YEAR: - msg = f"budget-window end_year must be {BUDGET_WINDOW_MAX_END_YEAR} or earlier" + msg = ( + "budget-window end_year must be " + f"{BUDGET_WINDOW_MAX_END_YEAR} or earlier" + ) raise ValueError(msg) return self diff --git a/policyengine_api/routes/report_output_routes.py b/policyengine_api/routes/report_output_routes.py index 0975a853f..d1c588fed 100644 --- a/policyengine_api/routes/report_output_routes.py +++ b/policyengine_api/routes/report_output_routes.py @@ -61,7 +61,8 @@ def create_report_output(country_id: str) -> Response: Request body should contain: - simulation_1_id (int): The first simulation ID (required) - - simulation_2_id (int | None): The second simulation ID (optional, for comparisons) + - simulation_2_id (int | None): The second simulation ID (optional, for + comparisons) - year (str | None): The year for the report (optional) """ diff --git a/policyengine_api/services/economy_comparison.py b/policyengine_api/services/economy_comparison.py index 03ce82fc7..eccf99f22 100644 --- a/policyengine_api/services/economy_comparison.py +++ b/policyengine_api/services/economy_comparison.py @@ -630,19 +630,17 @@ def uk_constituency_breakdown( repo_filename="parliamentary_constituency_weights.h5", ) with h5py.File(constituency_weights_path, "r") as f: - weights = f[ - "2025" - ][ - ... - ] # {2025: array(650, 100180) where cell i, j is the weight of household record i in constituency j} + # {2025: array(650, 100180) where cell i, j is the weight of household + # record i in constituency j} + weights = f["2025"][...] constituency_names_path = download_huggingface_dataset( repo="policyengine/policyengine-uk-data-public", repo_filename="constituencies_2024.csv", ) - constituency_names = pd.read_csv( - constituency_names_path - ) # columns code (constituency code), name (constituency name), x, y (geographic position) + # Columns: code (constituency code), name (constituency name), x, y + # (geographic position). + constituency_names = pd.read_csv(constituency_names_path) for i in range(len(constituency_names)): name: str = constituency_names.iloc[i]["name"] @@ -849,7 +847,8 @@ def compare_economic_outputs( reform (dict): The reform economy. country_id (str): The country identifier (e.g., "uk", "us"). region (str | None): The region filter (e.g., "uk", "local_authority/Leicester", - "constituency/Aldershot", "country/scotland"). Used to filter breakdown results. + "constituency/Aldershot", "country/scotland"). Used to filter breakdown + results. Returns: dict: The comparison of the two economies. diff --git a/policyengine_api/services/economy_service.py b/policyengine_api/services/economy_service.py index 6b4cfa7fe..d14b0a263 100644 --- a/policyengine_api/services/economy_service.py +++ b/policyengine_api/services/economy_service.py @@ -952,7 +952,10 @@ def _get_or_create_economic_impact( ) -> EconomicImpactResult: logger.log_struct( { - "message": "Received request for economic impact; checking if already in reform_impacts table", + "message": ( + "Received request for economic impact; checking if already in " + "reform_impacts table" + ), **setup_options.model_dump(), }, severity="INFO", @@ -988,7 +991,9 @@ def _get_or_create_economic_impact( if impact_action == ImpactAction.COMPLETED: logger.log_struct( { - "message": "Found completed economic impact in db; returning result", + "message": ( + "Found completed economic impact in db; returning result" + ), **setup_options.model_dump(), }, severity="INFO", @@ -1004,7 +1009,10 @@ def _get_or_create_economic_impact( if impact_action == ImpactAction.COMPUTING: logger.log_struct( { - "message": "Found computing economic impact record in db; confirming this is still computing", + "message": ( + "Found computing economic impact record in db; confirming this " + "is still computing" + ), **setup_options.model_dump(), }, severity="INFO", @@ -1063,7 +1071,10 @@ def _get_or_create_economic_impact( return EconomicImpactResult.computing() logger.log_struct( { - "message": "No previous economic impact record found in db; creating new simulation run", + "message": ( + "No previous economic impact record found in db; creating new " + "simulation run" + ), **setup_options.model_dump(), }, severity="INFO", @@ -1184,10 +1195,16 @@ def _build_budget_window_progress_message( active_years = ", ".join(computing_years[:2]) if len(computing_years) > 2: active_years = f"{active_years} + {len(computing_years) - 2} more" - return f"Scoring {active_years} ({completed_count} of {total_years} complete)..." + return ( + f"Scoring {active_years} " + f"({completed_count} of {total_years} complete)..." + ) if queued_years: - return f"Queued {queued_years[0]} ({completed_count} of {total_years} complete)..." + return ( + f"Queued {queued_years[0]} " + f"({completed_count} of {total_years} complete)..." + ) return f"Scoring budget window ({completed_count} of {total_years} complete)..." @@ -1832,7 +1849,9 @@ def _set_reform_impact_complete( except Exception as e: logger.log_struct( { - "message": f"Error setting completed reform impact record: {str(e)}", + "message": ( + f"Error setting completed reform impact record: {str(e)}" + ), **setup_options.model_dump(), } ) diff --git a/policyengine_api/services/report_spec_service.py b/policyengine_api/services/report_spec_service.py index a48b630b7..adfa1e9cf 100644 --- a/policyengine_api/services/report_spec_service.py +++ b/policyengine_api/services/report_spec_service.py @@ -186,7 +186,10 @@ def build_report_spec( for simulation in (simulation_1, simulation_2) ] if household_ids[0] != household_ids[1]: - msg = "Household comparison report specs require matching household IDs" + msg = ( + "Household comparison report specs require matching " + "household IDs" + ) raise ValueError(msg) return HouseholdReportSpec( country_id=report_output.country_id, diff --git a/policyengine_api/services/v2/comparison_runs/validators.py b/policyengine_api/services/v2/comparison_runs/validators.py index 278cd061f..d0f3c3a64 100644 --- a/policyengine_api/services/v2/comparison_runs/validators.py +++ b/policyengine_api/services/v2/comparison_runs/validators.py @@ -126,7 +126,10 @@ def _require_equal_fields( ) -> None: for field_name in fields: if getattr(existing, field_name) != getattr(candidate, field_name): - msg = f"comparison-run field {field_name} is immutable for an existing identity" + msg = ( + f"comparison-run field {field_name} is immutable for an existing " + "identity" + ) raise ComparisonRunIdentityError(msg) @@ -336,7 +339,10 @@ def require_lifecycle_transition( incomplete runs may return to running. """ if candidate not in ALLOWED_TRANSITIONS[current]: - msg = f"comparison-run lifecycle cannot move from {current.value} to {candidate.value}" + msg = ( + "comparison-run lifecycle cannot move from " + f"{current.value} to {candidate.value}" + ) raise ComparisonRunStateTransitionError(msg) diff --git a/policyengine_api/services/v2/households/validators.py b/policyengine_api/services/v2/households/validators.py index e46520cd6..a25a7c165 100644 --- a/policyengine_api/services/v2/households/validators.py +++ b/policyengine_api/services/v2/households/validators.py @@ -152,7 +152,10 @@ def normalize_household_document( msg = f"{collection} must be a list" raise HouseholdValidationError(msg) if len(records) > MAXIMUM_HOUSEHOLD_ENTITIES: - msg = f"{collection} must contain at most {MAXIMUM_HOUSEHOLD_ENTITIES} records" + msg = ( + f"{collection} must contain at most " + f"{MAXIMUM_HOUSEHOLD_ENTITIES} records" + ) raise HouseholdValidationError(msg) record_ids: set[str] = set() normalized_records: list[dict[str, Any]] = [] diff --git a/policyengine_api/services/v2/metadata/services.py b/policyengine_api/services/v2/metadata/services.py index 8082c6fad..4e448f951 100644 --- a/policyengine_api/services/v2/metadata/services.py +++ b/policyengine_api/services/v2/metadata/services.py @@ -16,7 +16,7 @@ read_dataset, read_datasets, ) -from policyengine_api.services.v2.metadata.database_connectors.reads_parameter_tree import ( +from policyengine_api.services.v2.metadata.database_connectors.reads_parameter_tree import ( # noqa: E501 # a dotted module path cannot be wrapped read_parameter_children, ) from policyengine_api.services.v2.metadata.database_connectors.reads_parameters import ( diff --git a/policyengine_api/spm.py b/policyengine_api/spm.py index e729c754b..e6e4bedb1 100644 --- a/policyengine_api/spm.py +++ b/policyengine_api/spm.py @@ -71,7 +71,10 @@ def _selection_schema(schema: dict[str, Any]) -> None: county_vintage: str = Field(default="2020", pattern=r"^[0-9]{4}$") as_of: str | None = Field( default=None, - description="ISO calendar date (YYYY-MM-DD); must satisfy the selected artifact's information date", + description=( + "ISO calendar date (YYYY-MM-DD); must satisfy the selected artifact's " + "information date" + ), json_schema_extra={ "format": "date", "pattern": r"^[0-9]{4}-[0-9]{2}-[0-9]{2}$", diff --git a/policyengine_api/utils/payload_validators/validate_household_payload.py b/policyengine_api/utils/payload_validators/validate_household_payload.py index 750c9874a..5eb1cbca9 100644 --- a/policyengine_api/utils/payload_validators/validate_household_payload.py +++ b/policyengine_api/utils/payload_validators/validate_household_payload.py @@ -17,7 +17,8 @@ def validate_household_payload( payload (dict): The payload to validate. Returns: - tuple[bool, str]: A tuple containing a boolean indicating whether the payload is valid and a message. + tuple[bool, str]: A tuple containing a boolean indicating whether the + payload is valid and a message. """ # Check that all required keys are present diff --git a/rules/fixtures/api.v2.validators-transformations-no-db/good_validators.py b/rules/fixtures/api.v2.validators-transformations-no-db/good_validators.py index aae3fa903..f80095c8c 100644 --- a/rules/fixtures/api.v2.validators-transformations-no-db/good_validators.py +++ b/rules/fixtures/api.v2.validators-transformations-no-db/good_validators.py @@ -1,4 +1,7 @@ -"""Proof fixture: a validator that checks an already-loaded record. The rule must accept it.""" +"""Proof fixture: a validator that checks an already-loaded record. + +The rule must accept it. +""" from policyengine_api.data.v2.models import Policy diff --git a/scripts/measure_cloud_run_runtime.py b/scripts/measure_cloud_run_runtime.py index 9ca1ddd99..80c1eff61 100644 --- a/scripts/measure_cloud_run_runtime.py +++ b/scripts/measure_cloud_run_runtime.py @@ -9,7 +9,7 @@ full request body, while the handler only reads the household/policy keys, so a top-level nonce field busts the cache without changing the computation. Cells that measure engine capacity must run with --cache-bust always (the default). -""" +""" # noqa: E501 # the summary is the --help description; keep its text from __future__ import annotations diff --git a/scripts/process_v1_household_mirror_event.py b/scripts/process_v1_household_mirror_event.py index 5be2f988e..7785598e5 100755 --- a/scripts/process_v1_household_mirror_event.py +++ b/scripts/process_v1_household_mirror_event.py @@ -89,7 +89,10 @@ def validate_command_environment( "ALLOWED_HOUSEHOLD_MIRROR_OPERATOR_IDENTITY", ) if operator_identity != allowed_operator_identity: - msg = "declared operator identity is not authorized for household event processing" + msg = ( + "declared operator identity is not authorized for household event " + "processing" + ) raise HouseholdEventCommandConfigurationError(msg) return { "environment": selected_environment, From 1cdbb940eb52797a744ef07572194c8f2ac65742 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 22:44:41 +0400 Subject: [PATCH 07/15] Wrap long lines in the rules tool and its tests (E501) Every split string keeps its exact text (the parsed code is unchanged), so the rendered rules page and the AGENTS.md block are identical apart from the tool hash. Two test docstrings are reworded to fit. Co-Authored-By: Claude Opus 5.5 --- scripts/rules/adapters.py | 3 +- scripts/rules/baseline.py | 3 +- scripts/rules/ci_scripts.py | 6 ++- scripts/rules/cli.py | 5 ++- scripts/rules/discover.py | 6 ++- scripts/rules/guard.py | 3 +- scripts/rules/render.py | 16 +++++--- scripts/rules/resolve.py | 6 ++- scripts/rules/tests/test_ci_scripts.py | 10 +++-- scripts/rules/tests/test_guard.py | 53 +++++++++++++++++--------- scripts/rules/tests/test_workflows.py | 13 +++++-- scripts/rules/validate.py | 6 ++- 12 files changed, 89 insertions(+), 41 deletions(-) diff --git a/scripts/rules/adapters.py b/scripts/rules/adapters.py index 19a7ee979..a16c5b40c 100644 --- a/scripts/rules/adapters.py +++ b/scripts/rules/adapters.py @@ -48,7 +48,8 @@ def v(message: str, fix: str) -> None: if others: v( f"references other docs ({', '.join(others)})", - f"list skills and rules in {instructions} only; keep {adapter} a pointer", + f"list skills and rules in {instructions} only; " + f"keep {adapter} a pointer", ) if BEGIN in text: v( diff --git a/scripts/rules/baseline.py b/scripts/rules/baseline.py index ab52ec124..2af9b1ac5 100644 --- a/scripts/rules/baseline.py +++ b/scripts/rules/baseline.py @@ -81,7 +81,8 @@ def check_baselines( new.id, "baseline", f"status weakened from {old.status} to {new.status}", - 'add [rule.downgrade] reason = "..." so the downgrade is explicit and reviewable', + 'add [rule.downgrade] reason = "..." so the downgrade is ' + "explicit and reviewable", ) ) for rule_id in sorted(current_ids - {rule.id for rule in base.rules}): diff --git a/scripts/rules/ci_scripts.py b/scripts/rules/ci_scripts.py index 571b63ac2..77fe38324 100644 --- a/scripts/rules/ci_scripts.py +++ b/scripts/rules/ci_scripts.py @@ -166,7 +166,8 @@ def compare(registry: Registry, findings: Findings, root: Path) -> list[Violatio "untested workflow script", findings.untested, config.untested_baseline, - "add a test that runs the script (see docs/engineering/skills/ci-workflows.md)", + "add a test that runs the script " + "(see docs/engineering/skills/ci-workflows.md)", ), ( "shellcheck", @@ -190,7 +191,8 @@ def compare(registry: Registry, findings: Findings, root: Path) -> list[Violatio rule_id, "ci-scripts", f"baseline entry no longer applies: {entry}", - f"remove it from {baseline_path} and lower the rule's baseline count", + f"remove it from {baseline_path} and lower the rule's " + "baseline count", ) ) return violations diff --git a/scripts/rules/cli.py b/scripts/rules/cli.py index 78ebf6149..eb9edf9fb 100644 --- a/scripts/rules/cli.py +++ b/scripts/rules/cli.py @@ -47,7 +47,10 @@ def main(argv: list[str] | None = None) -> int: parser.add_argument( "--write-baselines", action="store_true", - help="ci-scripts only: record today's findings as the baselines (when adopting the rule)", + help=( + "ci-scripts only: record today's findings as the baselines " + "(when adopting the rule)" + ), ) args = parser.parse_args(argv) root = args.root.resolve() diff --git a/scripts/rules/discover.py b/scripts/rules/discover.py index a352c05ef..152ec9469 100644 --- a/scripts/rules/discover.py +++ b/scripts/rules/discover.py @@ -149,7 +149,8 @@ def discover_hidden(registry: Registry, root: Path) -> list[Violation]: violations.append( _hidden( f"guard test {node} is not referenced by any rule", - "add it to a pytest rule's nodes, or to discovery.ignore_tests with a reason", + "add it to a pytest rule's nodes, or to discovery.ignore_tests " + "with a reason", ) ) @@ -162,7 +163,8 @@ def discover_hidden(registry: Registry, root: Path) -> list[Violation]: violations.append( _hidden( f"CI job '{job_id}' in {workflow} enforces no registered rule", - "reference it from a rule's [[rule.ci]], or add it to discovery.ignore_jobs with a reason", + "reference it from a rule's [[rule.ci]], or add it to " + "discovery.ignore_jobs with a reason", ) ) return violations diff --git a/scripts/rules/guard.py b/scripts/rules/guard.py index 243102cc6..99ba5e357 100644 --- a/scripts/rules/guard.py +++ b/scripts/rules/guard.py @@ -104,7 +104,8 @@ def run_guard( return result if ref is None: result.notes.append( - "no base ref (set --base-ref or GITHUB_BASE_REF); baseline comparison skipped" + "no base ref (set --base-ref or GITHUB_BASE_REF); " + "baseline comparison skipped" ) else: try: diff --git a/scripts/rules/render.py b/scripts/rules/render.py index 2b70361e4..68002382e 100644 --- a/scripts/rules/render.py +++ b/scripts/rules/render.py @@ -129,9 +129,10 @@ def render_rules_md(registry: Registry) -> str: "Generated by `python -m scripts.rules render` from `rules/registry.toml`. " "Do not edit by hand.", "", - f"Every rule that governs `{registry.repo}` is listed here with how it is enforced. " - "Blocking rules fail CI. The Gaps section lists what is written down but not yet " - f"enforced or proven. To add or change a rule, see `{SKILL_DOC}`.", + f"Every rule that governs `{registry.repo}` is listed here with how it " + "is enforced. Blocking rules fail CI. The Gaps section lists what is " + "written down but not yet enforced or proven. To add or change a rule, " + f"see `{SKILL_DOC}`.", "", "## Summary", "", @@ -184,7 +185,8 @@ def section(title: str, items: list[str]) -> None: out += [ f"## {title}", "", - "| Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof |", + "| Rule | Statement | Tier | Status | Blocking " + "| Enforced by | CI | Proof |", "| --- | --- | --- | --- | --- | --- | --- | --- |", ] for r in group: @@ -220,9 +222,11 @@ def render_agents_block(registry: Registry) -> str: out = [ BEGIN, "", - "Every rule for this repository lives in `rules/registry.toml` and is rendered,", + "Every rule for this repository lives in " + "`rules/registry.toml` and is rendered,", "with its enforcement and gaps, in `docs/generated/rules.md`. Blocking rules", - "fail CI. Doc-only rules are not checked automatically, so follow them yourself.", + "fail CI. Doc-only rules are not checked automatically, " + "so follow them yourself.", f"To add or change a rule, read `{SKILL_DOC}`.", "", ] diff --git a/scripts/rules/resolve.py b/scripts/rules/resolve.py index d00d15e91..78a41a2d8 100644 --- a/scripts/rules/resolve.py +++ b/scripts/rules/resolve.py @@ -467,7 +467,8 @@ def resolve_proof_ci(rule: Rule, root: Path) -> list[Violation]: rule.id, "proof", "the proof test is not tied to a CI job", - "add proof.workflow, proof.job and proof.invocation naming the job that runs it", + "add proof.workflow, proof.job and proof.invocation naming the " + "job that runs it", ) ] violations = [] @@ -516,7 +517,8 @@ def _test_coverage( Violation( rule_id, check, - f"{path} is not collected by {described} (collects {collected})", + f"{path} is not collected by {described} " + f"(collects {collected})", "add it to what the job runs, or move the test", ) ) diff --git a/scripts/rules/tests/test_ci_scripts.py b/scripts/rules/tests/test_ci_scripts.py index c05a4bc3f..354ad5adc 100644 --- a/scripts/rules/tests/test_ci_scripts.py +++ b/scripts/rules/tests/test_ci_scripts.py @@ -38,7 +38,7 @@ def require_shellcheck(test: unittest.TestCase) -> None: class TestCiScriptsProof(unittest.TestCase): - """Proofs that each ci-scripts check rejects a bad example and accepts a good one.""" + """Proofs: each ci-scripts check rejects a bad example and accepts a good one.""" def test_rule_proof_multiline_run(self) -> None: self.assertEqual( @@ -80,7 +80,10 @@ def test_blank_lines_do_not_count(self) -> None: self.assertEqual(code_lines("\nmake build\n\n"), 1) def test_folded_block_counts_each_line(self) -> None: - text = "jobs:\n a:\n steps:\n - run: >-\n pytest a\n b\n" + text = ( + "jobs:\n a:\n steps:\n - run: >-\n pytest a\n" + " b\n" + ) jobs, _ = parse_jobs(text) block = jobs["a"].run_blocks[0] self.assertEqual(block.code_lines, 2) @@ -194,7 +197,8 @@ def test_enforced_rule_without_baseline_keeps_its_id(self) -> None: REGISTRY.replace("ex.ci.multiline", "ex.ci.workflow-scripts-tested") .replace('status = "ratcheted"', 'status = "enforced"') .replace( - '[rule.baseline]\nfile = "rules/baselines/ci-multiline-run.txt"\ncount = 1\n', + '[rule.baseline]\nfile = "rules/baselines/ci-multiline-run.txt"\n' + "count = 1\n", "", ) ) diff --git a/scripts/rules/tests/test_guard.py b/scripts/rules/tests/test_guard.py index 46738fc2d..0c5773ed9 100644 --- a/scripts/rules/tests/test_guard.py +++ b/scripts/rules/tests/test_guard.py @@ -174,7 +174,8 @@ def test_ruff_is_never_ratcheted(self) -> None: repo.edit( "rules/registry.toml", 'kind = "ruff"\nmode = "format"', - 'kind = "ruff"\nmode = "format"\n[rule.baseline]\nfile = "rules/baselines/ruff.txt"\ncount = 1', + 'kind = "ruff"\nmode = "format"\n[rule.baseline]\n' + 'file = "rules/baselines/ruff.txt"\ncount = 1', ) self.assert_violation(repo, "ruff is never ratcheted") @@ -330,8 +331,10 @@ def test_proof_files_must_exist(self) -> None: repo.edit( "rules/registry.toml", 'kind = "mypy"\npaths = ["pkg"]', - 'kind = "mypy"\npaths = ["pkg"]\n[rule.proof]\nbad_fixture = "rules/fixtures/bad.py"\n' - 'good_fixture = "rules/fixtures/good.py"\ntest = "tests/unit/test_boundaries.py::test_routes_have_no_sql"', + 'kind = "mypy"\npaths = ["pkg"]\n[rule.proof]\n' + 'bad_fixture = "rules/fixtures/bad.py"\n' + 'good_fixture = "rules/fixtures/good.py"\n' + 'test = "tests/unit/test_boundaries.py::test_routes_have_no_sql"', ) self.assert_violation( repo, "proof bad_fixture rules/fixtures/bad.py does not exist" @@ -359,7 +362,8 @@ def test_pytest_rule_proof_runs_in_its_ci_job(self) -> None: self.with_proof( repo, 'nodes = ["tests/unit/test_boundaries.py::test_routes_have_no_sql"]', - 'bad_fixture = "rules/fixtures/bad.py"\ngood_fixture = "rules/fixtures/good.py"\n' + 'bad_fixture = "rules/fixtures/bad.py"\n' + 'good_fixture = "rules/fixtures/good.py"\n' 'test = "tests/unit/test_proof.py::test_rule_proof_routes"', ) self.assertEqual(repo.messages(), []) @@ -373,7 +377,8 @@ def test_proof_outside_the_ci_command_is_rejected(self) -> None: self.with_proof( repo, 'nodes = ["tests/unit/test_boundaries.py::test_routes_have_no_sql"]', - 'bad_fixture = "rules/fixtures/bad.py"\ngood_fixture = "rules/fixtures/good.py"\n' + 'bad_fixture = "rules/fixtures/bad.py"\n' + 'good_fixture = "rules/fixtures/good.py"\n' 'test = "tests/contract/test_proof.py::test_rule_proof_routes"', ) messages = "\n".join(repo.messages()) @@ -391,7 +396,8 @@ def test_non_pytest_rule_proof_must_name_its_job(self) -> None: self.with_proof( repo, 'paths = ["pkg"]', - 'bad_fixture = "rules/fixtures/bad.py"\ngood_fixture = "rules/fixtures/good.py"\n' + 'bad_fixture = "rules/fixtures/bad.py"\n' + 'good_fixture = "rules/fixtures/good.py"\n' 'test = "tests/unit/test_proof.py::test_rule_proof_types"', ) self.assertIn( @@ -405,7 +411,9 @@ def test_proof_with_named_unittest_job(self) -> None: ) files = { **self.PROOF_FILES, - "tools/tests/test_proof.py": "class TestProof:\n def test_rule_proof_types(self):\n pass\n", + "tools/tests/test_proof.py": ( + "class TestProof:\n def test_rule_proof_types(self):\n pass\n" + ), } with Repo(files, render=False) as repo: repo.write( @@ -415,10 +423,12 @@ def test_proof_with_named_unittest_job(self) -> None: repo.edit( "rules/registry.toml", "[discovery]\n", - '[discovery]\nignore_jobs = [{ job = "tool-tests", reason = "runs tool tests" }]\n', + "[discovery]\n" + 'ignore_jobs = [{ job = "tool-tests", reason = "runs tool tests" }]\n', ) proof = ( - 'bad_fixture = "rules/fixtures/bad.py"\ngood_fixture = "rules/fixtures/good.py"\n' + 'bad_fixture = "rules/fixtures/bad.py"\n' + 'good_fixture = "rules/fixtures/good.py"\n' 'test = "tools/tests/test_proof.py::TestProof::test_rule_proof_types"\n' 'workflow = ".github/workflows/pr.yml"\njob = "tool-tests"\n' 'invocation = "unittest discover -s tools/tests"' @@ -442,7 +452,8 @@ def test_unregistered_quality_guard(self) -> None: with Repo() as repo: repo.write( "scripts/run_quality_guards.py", - 'GUARDS = (("rules-registry", lambda: []), ("secret-guard", lambda: []))\n', + 'GUARDS = (("rules-registry", lambda: []), ' + '("secret-guard", lambda: []))\n', ) self.assert_violation(repo, "quality guard 'secret-guard'") @@ -466,7 +477,8 @@ def test_unregistered_ruff_ignore(self) -> None: with Repo() as repo: repo.write( "pyproject.toml", - '[tool.mypy]\nfiles = ["pkg"]\n\n[tool.ruff.lint.per-file-ignores]\n"pkg/*.py" = ["E501"]\n', + '[tool.mypy]\nfiles = ["pkg"]\n\n' + '[tool.ruff.lint.per-file-ignores]\n"pkg/*.py" = ["E501"]\n', ) self.assert_violation(repo, "ruff per-file-ignores 'pkg/*.py' has no rule") @@ -551,7 +563,9 @@ def test_ignored_ci_job_with_reason_passes(self) -> None: repo.edit( "rules/registry.toml", "[discovery]\n", - '[discovery]\nignore_jobs = [{ job = "deploy", reason = "deploys, enforces nothing" }]\n', + "[discovery]\n" + 'ignore_jobs = [{ job = "deploy", ' + 'reason = "deploys, enforces nothing" }]\n', ) repo.render() self.assertEqual(repo.messages(), []) @@ -610,7 +624,8 @@ def test_expired_exception(self) -> None: repo.edit( "rules/registry.toml", 'kind = "mypy"\npaths = ["pkg"]', - 'kind = "mypy"\npaths = ["pkg"]\n[[rule.exception]]\npath = "pkg/routes.py"\n' + 'kind = "mypy"\npaths = ["pkg"]\n[[rule.exception]]\n' + 'path = "pkg/routes.py"\n' 'reason = "migrating"\nexpires = 2026-05-01', ) repo.render() @@ -625,7 +640,8 @@ def test_exception_is_listed_in_rules_md(self) -> None: repo.edit( "rules/registry.toml", 'kind = "mypy"\npaths = ["pkg"]', - 'kind = "mypy"\npaths = ["pkg"]\n[[rule.exception]]\npath = "pkg/routes.py"\n' + 'kind = "mypy"\npaths = ["pkg"]\n[[rule.exception]]\n' + 'path = "pkg/routes.py"\n' 'reason = "migrating"\nexpires = 2026-12-01', ) repo.render() @@ -655,9 +671,12 @@ def test_baseline_may_not_grow(self) -> None: def test_downgrade_needs_reason(self) -> None: base = self.registry("") weakened = REGISTRY.replace( - 'status = "enforced"\nautofix = true\n[rule.enforcer]\nkind = "ruff"\nmode = "format"\n[[rule.ci]]\n' - 'workflow = ".github/workflows/pr.yml"\njob = "lint"\ninvocation = "ruff format --check"', - 'status = "advisory"\nautofix = true\n[rule.enforcer]\nkind = "ruff"\nmode = "format"', + 'status = "enforced"\nautofix = true\n[rule.enforcer]\nkind = "ruff"\n' + 'mode = "format"\n[[rule.ci]]\n' + 'workflow = ".github/workflows/pr.yml"\njob = "lint"\n' + 'invocation = "ruff format --check"', + 'status = "advisory"\nautofix = true\n[rule.enforcer]\nkind = "ruff"\n' + 'mode = "format"', ) current, _ = load_text(weakened) assert current is not None diff --git a/scripts/rules/tests/test_workflows.py b/scripts/rules/tests/test_workflows.py index 137337c52..bf03f566e 100644 --- a/scripts/rules/tests/test_workflows.py +++ b/scripts/rules/tests/test_workflows.py @@ -1,4 +1,7 @@ -"""Tests for the GitHub Actions subset reader, pinned against both repos' real PR workflows.""" +"""Tests for the GitHub Actions subset reader. + +They are pinned against both repos' real PR workflows. +""" from __future__ import annotations @@ -55,7 +58,10 @@ def test_block_scalar_stops_at_sibling_key(self) -> None: self.assertTrue(jobs["build"].invokes("make build")) def test_comments_are_ignored(self) -> None: - text = "jobs:\n a:\n steps:\n # run: make test\n - run: echo hi # make test\n" + text = ( + "jobs:\n a:\n steps:\n # run: make test\n" + " - run: echo hi # make test\n" + ) jobs, _ = parse_jobs(text) self.assertFalse(jobs["a"].invokes("make test")) @@ -67,7 +73,8 @@ def test_quoted_hash_is_not_a_comment(self) -> None: def test_inline_matrix_list(self) -> None: jobs, _ = parse_jobs( - 'jobs:\n a:\n strategy:\n matrix:\n python: ["3.12", "3.13"]\n' + "jobs:\n a:\n strategy:\n matrix:\n" + ' python: ["3.12", "3.13"]\n' ) self.assertEqual(jobs["a"].matrix["python"], ("3.12", "3.13")) diff --git a/scripts/rules/validate.py b/scripts/rules/validate.py index 406b40ef6..94f6824c9 100644 --- a/scripts/rules/validate.py +++ b/scripts/rules/validate.py @@ -178,7 +178,8 @@ def v(message: str, fix: str) -> None: if kind is Kind.RUFF and rule.baseline is not None: v( "ruff is never ratcheted", - "remove the baseline and fix every violation in a standalone PR that also changes the ruff select list", + "remove the baseline and fix every violation in a standalone PR that " + "also changes the ruff select list", ) if tier is Tier.JUDGMENT and kind not in (Kind.LLM_REVIEW, Kind.NONE): v( @@ -330,7 +331,8 @@ def v(rule_id: str | None, message: str, fix: str, check: str = "schema") -> Non elif baseline_entries(path) != rule.baseline.count: v( rule.id, - f"baseline count is {rule.baseline.count} but {rule.baseline.file} lists {baseline_entries(path)}", + f"baseline count is {rule.baseline.count} but " + f"{rule.baseline.file} lists {baseline_entries(path)}", "make the count match the file", "baseline", ) From cf84a6389c57218d15d9af9b8d15c54f3c9ac28b Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 22:47:21 +0400 Subject: [PATCH 08/15] Wrap long lines in the tests (E501) Split long strings (same text), rewrap comments and reword over-long test docstrings. Thirteen test functions whose names alone overflowed the line get shorter names; nothing references them. The economy service tests reach the long-named reform-impact lookup mock through a small hash_prefix_lookup helper. Co-Authored-By: Claude Opus 5.5 --- tests/contract/registry.py | 4 +- tests/contract/test_v1_route_contracts.py | 4 +- .../test_environment_variables.py | 5 +- tests/to_refactor/api/test_api.py | 5 +- .../unit/data/test_remote_database_config.py | 4 +- .../test_household_dual_write_routes.py | 2 +- .../routes/test_metadata_routes_streaming.py | 5 +- .../unit/services/test_economy_comparison.py | 18 +-- tests/unit/services/test_economy_service.py | 105 ++++++++++-------- .../unit/services/test_household_mirroring.py | 4 +- tests/unit/services/test_metadata_service.py | 10 +- tests/unit/test_alembic_skill.py | 3 +- tests/unit/test_alembic_workflows.py | 2 +- tests/unit/test_country.py | 3 +- tests/unit/test_deploy_pipeline_scripts.py | 3 +- tests/unit/test_workflow_guard_scripts.py | 5 +- 16 files changed, 106 insertions(+), 76 deletions(-) diff --git a/tests/contract/registry.py b/tests/contract/registry.py index 60ccd9d19..778c3da26 100644 --- a/tests/contract/registry.py +++ b/tests/contract/registry.py @@ -557,7 +557,9 @@ class WorkflowContract: WorkflowContract( name="region_selection", current_contract="api_v1_compatible", - future_owner_pr="PR 9: v2 Metadata, Regions, Datasets, Parameters, and Variables", + future_owner_pr=( + "PR 9: v2 Metadata, Regions, Datasets, Parameters, and Variables" + ), requests=( ContractRequest( method="GET", diff --git a/tests/contract/test_v1_route_contracts.py b/tests/contract/test_v1_route_contracts.py index 34e487f1f..6d7455266 100644 --- a/tests/contract/test_v1_route_contracts.py +++ b/tests/contract/test_v1_route_contracts.py @@ -619,6 +619,8 @@ def test_invalid_country_contract(contract_client: ContractClient): response_json(response), { "status": "error", - "message": "Country zz not found. Available countries are: uk, us, ca, ng, il", + "message": ( + "Country zz not found. Available countries are: uk, us, ca, ng, il" + ), }, ) diff --git a/tests/env_variables/test_environment_variables.py b/tests/env_variables/test_environment_variables.py index de4175d7b..e64a81578 100644 --- a/tests/env_variables/test_environment_variables.py +++ b/tests/env_variables/test_environment_variables.py @@ -87,7 +87,7 @@ def test_hugging_face_token(self): reason="Skipping in debug mode", ) def test_github_microdata_auth_token(self): - """Test if POLICYENGINE_GITHUB_MICRODATA_AUTH_TOKEN is valid by querying GitHub user API.""" + """Check POLICYENGINE_GITHUB_MICRODATA_AUTH_TOKEN with the GitHub user API.""" token = os.getenv("POLICYENGINE_GITHUB_MICRODATA_AUTH_TOKEN") assert token is not None, "POLICYENGINE_GITHUB_MICRODATA_AUTH_TOKEN is not set" @@ -104,7 +104,8 @@ def test_github_microdata_auth_token(self): ) assert token_validation_response.status_code == 200, ( - f"Invalid POLICYENGINE_GITHUB_MICRODATA_AUTH_TOKEN: {token_validation_response.text}" + "Invalid POLICYENGINE_GITHUB_MICRODATA_AUTH_TOKEN: " + f"{token_validation_response.text}" ) token_user_details = token_validation_response.json() diff --git a/tests/to_refactor/api/test_api.py b/tests/to_refactor/api/test_api.py index 8625dc45a..af7f3c7d0 100644 --- a/tests/to_refactor/api/test_api.py +++ b/tests/to_refactor/api/test_api.py @@ -41,7 +41,10 @@ def assert_response_data_matches_expected(data: dict, expected: dict): if isinstance(value, dict): assert_response_data_matches_expected(data[key], value) elif data[key] != value: - msg = f"Value {data[key]} for key {key} does not match expected value {value}.\n {data}" + msg = ( + f"Value {data[key]} for key {key} does not match expected value " + f"{value}.\n {data}" + ) raise ValueError(msg) diff --git a/tests/unit/data/test_remote_database_config.py b/tests/unit/data/test_remote_database_config.py index 118594f27..a10b60c0c 100644 --- a/tests/unit/data/test_remote_database_config.py +++ b/tests/unit/data/test_remote_database_config.py @@ -23,7 +23,9 @@ def test_remote_database_config_defaults_non_target_settings(monkeypatch): monkeypatch.delenv("POLICYENGINE_DB_NAME", raising=False) assert get_remote_database_config() == { - "instance_connection_name": "policyengine-api:us-central1:policyengine-api-data", + "instance_connection_name": ( + "policyengine-api:us-central1:policyengine-api-data" + ), "db_user": "policyengine", "db_name": "policyengine", } diff --git a/tests/unit/routes/test_household_dual_write_routes.py b/tests/unit/routes/test_household_dual_write_routes.py index bc94a8487..a219dae06 100644 --- a/tests/unit/routes/test_household_dual_write_routes.py +++ b/tests/unit/routes/test_household_dual_write_routes.py @@ -147,7 +147,7 @@ def test_dual_write_keeps_other_v1_countries_entirely_on_cloud_sql( copy_event.assert_not_called() -def test_destination_failure_reports_committed_source_and_does_not_claim_retry_repairs_it( +def test_destination_failure_reports_committed_source_without_claiming_retry_repair( monkeypatch, ) -> None: monkeypatch.setenv("DB_WRITE_HOUSEHOLD", "dual_write") diff --git a/tests/unit/routes/test_metadata_routes_streaming.py b/tests/unit/routes/test_metadata_routes_streaming.py index ed2423c14..667d46f3c 100644 --- a/tests/unit/routes/test_metadata_routes_streaming.py +++ b/tests/unit/routes/test_metadata_routes_streaming.py @@ -1,4 +1,7 @@ -"""Flask fallback metadata route: large bodies stream, small bodies keep Content-Length.""" +"""Flask fallback metadata route tests. + +Large bodies stream, and small bodies keep Content-Length. +""" import importlib import json diff --git a/tests/unit/services/test_economy_comparison.py b/tests/unit/services/test_economy_comparison.py index 07eba32dc..ce9bde4b6 100644 --- a/tests/unit/services/test_economy_comparison.py +++ b/tests/unit/services/test_economy_comparison.py @@ -165,7 +165,7 @@ def test__given_uk_country__returns_breakdown( assert "Isle of Anglesey" in result.by_local_authority def test__region_categorization_by_code_prefix(self): - """Test that region categorization logic correctly identifies UK nations by code prefix.""" + """Test that region categorization identifies UK nations by code prefix.""" # This is a unit test for the region categorization logic # We test the logic directly rather than through the full function @@ -305,12 +305,12 @@ def test__downloads_from_correct_repos( assert calls[1][1]["repo_filename"] == "local_authorities_2021.csv" def test__given_constituency_region__returns_none(self): - """When simulating a constituency, local authority breakdown should not be computed.""" + """Skip the local authority breakdown when simulating a constituency.""" result = uk_local_authority_breakdown({}, {}, "uk", "constituency/Aldershot") assert result is None def test__given_constituency_region_with_code__returns_none(self): - """When simulating a constituency by code, local authority breakdown should not be computed.""" + """Skip the local authority breakdown for a constituency given by code.""" result = uk_local_authority_breakdown({}, {}, "uk", "constituency/E12345678") assert result is None @@ -320,7 +320,7 @@ def test__given_constituency_region_with_code__returns_none(self): def test__given_specific_la_region__returns_only_that_la( self, mock_read_csv, mock_h5py_file, mock_download ): - """When simulating a specific local authority, only that LA should be returned.""" + """Return only that LA when simulating a specific local authority.""" mock_download.side_effect = [ "/path/to/weights.h5", "/path/to/names.csv", @@ -361,7 +361,7 @@ def test__given_specific_la_region__returns_only_that_la( def test__given_country_scotland_region__returns_only_scottish_las( self, mock_read_csv, mock_h5py_file, mock_download ): - """When simulating country/scotland, only Scottish local authorities should be returned.""" + """Return only Scottish local authorities when simulating country/scotland.""" mock_download.side_effect = [ "/path/to/weights.h5", "/path/to/names.csv", @@ -512,12 +512,12 @@ def test__given_non_uk_country_nigeria__returns_none(self): assert result is None def test__given_local_authority_region__returns_none(self): - """When simulating a local authority, constituency breakdown should not be computed.""" + """Skip the constituency breakdown when simulating a local authority.""" result = uk_constituency_breakdown({}, {}, "uk", "local_authority/Leicester") assert result is None def test__given_local_authority_region_with_code__returns_none(self): - """When simulating a local authority by code, constituency breakdown should not be computed.""" + """Skip the constituency breakdown for a local authority given by code.""" result = uk_constituency_breakdown({}, {}, "uk", "local_authority/E06000016") assert result is None @@ -527,7 +527,7 @@ def test__given_local_authority_region_with_code__returns_none(self): def test__given_specific_constituency_region__returns_only_that_constituency( self, mock_read_csv, mock_h5py_file, mock_download ): - """When simulating a specific constituency, only that constituency should be returned.""" + """Return only that constituency when simulating a specific constituency.""" mock_download.side_effect = [ "/path/to/weights.h5", "/path/to/names.csv", @@ -570,7 +570,7 @@ def test__given_specific_constituency_region__returns_only_that_constituency( def test__given_country_scotland_region__returns_only_scottish_constituencies( self, mock_read_csv, mock_h5py_file, mock_download ): - """When simulating country/scotland, only Scottish constituencies should be returned.""" + """Return only Scottish constituencies when simulating country/scotland.""" mock_download.side_effect = [ "/path/to/weights.h5", "/path/to/names.csv", diff --git a/tests/unit/services/test_economy_service.py b/tests/unit/services/test_economy_service.py index 3110b3512..9a80cfeda 100644 --- a/tests/unit/services/test_economy_service.py +++ b/tests/unit/services/test_economy_service.py @@ -59,6 +59,14 @@ pytest_plugins = ("tests.fixtures.services.economy_service",) +def hash_prefix_lookup(reform_impacts_service: MagicMock) -> MagicMock: + """Return the mocked lookup of reform impacts by options-hash prefix.""" + lookup: MagicMock = ( + reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix + ) + return lookup + + @pytest.fixture(autouse=True) def stable_observability_lifecycle(): with ( @@ -172,7 +180,7 @@ def test__given_completed_impact__returns_completed_result( mock_submission_claim_id, ): completed_impact = create_mock_reform_impact(status="ok") - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [ + hash_prefix_lookup(mock_reform_impacts_service).return_value = [ completed_impact ] @@ -189,9 +197,7 @@ def test__given_completed_impact__returns_completed_result( "data_version": MOCK_DATA_VERSION, "dataset": MOCK_RESOLVED_DATASET, } - ( - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.assert_called_once() - ) + (hash_prefix_lookup(mock_reform_impacts_service).assert_called_once()) mock_simulation_entrypoint.run.assert_not_called() def test__given_orm_decoded_completed_impact__returns_completed_result( @@ -211,7 +217,7 @@ def test__given_orm_decoded_completed_impact__returns_completed_result( completed_impact.reform_impact_json = json.loads( completed_impact.reform_impact_json ) - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [ + hash_prefix_lookup(mock_reform_impacts_service).return_value = [ completed_impact ] @@ -253,7 +259,7 @@ def test__given_cached_error_impact__returns_error_message( status="error", message=cached_message, ) - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [ + hash_prefix_lookup(mock_reform_impacts_service).return_value = [ failed_impact ] @@ -283,7 +289,7 @@ def test__given_legacy_completed_impact__refreshes_cache( reform_impact_json=json.dumps(MOCK_REFORM_IMPACT_DATA), options_hash=MOCK_LOOKUP_OPTIONS_HASH, ) - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [ + hash_prefix_lookup(mock_reform_impacts_service).return_value = [ completed_impact ] @@ -297,7 +303,7 @@ def test__given_legacy_completed_impact__refreshes_cache( ) mock_simulation_entrypoint.run.assert_called_once() - def test__given_computing_impact_with_succeeded_execution__returns_completed_result( + def test__given_computing_impact_with_succeeded_execution__returns_completed( self, economy_service, base_params, @@ -311,7 +317,7 @@ def test__given_computing_impact_with_succeeded_execution__returns_completed_res mock_submission_claim_id, ): computing_impact = create_mock_reform_impact(status="computing") - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [ + hash_prefix_lookup(mock_reform_impacts_service).return_value = [ computing_impact ] mock_simulation_entrypoint.get_execution_status.return_value = "complete" @@ -350,7 +356,7 @@ def test__given_computing_impact_with_failed_execution__returns_error_result( mock_submission_claim_id, ): computing_impact = create_mock_reform_impact(status="computing") - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [ + hash_prefix_lookup(mock_reform_impacts_service).return_value = [ computing_impact ] mock_simulation_entrypoint.get_execution_status.return_value = "failed" @@ -368,7 +374,7 @@ def test__given_computing_impact_with_failed_execution__returns_error_result( ) mock_reform_impacts_service.set_error_reform_impact.assert_called_once() - def test__given_computing_impact_with_active_execution__returns_computing_result( + def test__given_computing_impact_with_active_execution__returns_computing( self, economy_service, base_params, @@ -382,7 +388,7 @@ def test__given_computing_impact_with_active_execution__returns_computing_result mock_submission_claim_id, ): computing_impact = create_mock_reform_impact(status="computing") - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [ + hash_prefix_lookup(mock_reform_impacts_service).return_value = [ computing_impact ] mock_simulation_entrypoint.get_execution_status.return_value = "running" @@ -405,7 +411,7 @@ def test__given_no_previous_impact__creates_new_simulation( mock_datetime, mock_submission_claim_id, ): - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [] + hash_prefix_lookup(mock_reform_impacts_service).return_value = [] result = economy_service.get_economic_impact(**base_params) @@ -443,7 +449,7 @@ def test__given_no_previous_impact__starts_observability_after_claim( mock_datetime, mock_submission_claim_id, ): - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [] + hash_prefix_lookup(mock_reform_impacts_service).return_value = [] lifecycle_events = [] mock_reform_impacts_service.claim_reform_impact_start.side_effect = ( lambda **_kwargs: lifecycle_events.append("claim") or True @@ -474,7 +480,7 @@ def test__selected_observability_id_is_applied_to_containing_spans( mock_datetime, mock_submission_claim_id, ): - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [] + hash_prefix_lookup(mock_reform_impacts_service).return_value = [] with patch( "policyengine_api.services.economy_service.set_runtime_context" @@ -711,7 +717,7 @@ def test__given_policies_created_through_orm__submits_decoded_json( ) reform_impacts = MagicMock() - reform_impacts.get_all_reform_impacts_by_options_hash_prefix.return_value = [] + hash_prefix_lookup(reform_impacts).return_value = [] simulation_gateway = MagicMock() simulation_gateway.resolve_app_name.return_value = ( "policyengine-simulation-test", @@ -761,7 +767,7 @@ def test__given_no_previous_impact__includes_metadata_in_simulation_params( mock_submission_claim_id, ): """Verify that _metadata with policy IDs is passed to simulation API.""" - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [] + hash_prefix_lookup(mock_reform_impacts_service).return_value = [] economy_service.get_economic_impact(**base_params) @@ -845,11 +851,11 @@ def test__given_runtime_cache_version__uses_versioned_economy_cache_key( "policyengine_api.services.economy_service.get_economy_impact_cache_version", lambda country_id, api_version=None: cache_version, ) - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [] + hash_prefix_lookup(mock_reform_impacts_service).return_value = [] economy_service.get_economic_impact(**base_params) - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.assert_called_once_with( + hash_prefix_lookup(mock_reform_impacts_service).assert_called_once_with( MOCK_COUNTRY_ID, MOCK_POLICY_ID, MOCK_BASELINE_POLICY_ID, @@ -876,11 +882,11 @@ def test__given_default_dataset__queries_previous_impacts_with_resolved_bundle( mock_datetime, mock_submission_claim_id, ): - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [] + hash_prefix_lookup(mock_reform_impacts_service).return_value = [] economy_service.get_economic_impact(**base_params) - call_args = mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.call_args.args + call_args = hash_prefix_lookup(mock_reform_impacts_service).call_args.args assert call_args[4] == MOCK_DATASET assert call_args[6] == MOCK_LOOKUP_OPTIONS_HASH assert call_args[7] == economy_service._build_options_hash_lookup_pattern( @@ -904,7 +910,7 @@ def test__given_completed_impact__uses_resolved_runtime_bundle_for_cache_lookup( mock_submission_claim_id, ): completed_impact = create_mock_reform_impact(status="ok") - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [ + hash_prefix_lookup(mock_reform_impacts_service).return_value = [ completed_impact ] @@ -931,7 +937,7 @@ def test__given_existing_impact__restores_stored_observability_id( mock_submission_claim_id, ): completed_impact = create_mock_reform_impact(status="ok") - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [ + hash_prefix_lookup(mock_reform_impacts_service).return_value = [ completed_impact ] @@ -950,7 +956,7 @@ def test__given_existing_impact__restores_stored_observability_id( restore_observability_id.assert_called_once_with(MOCK_OBSERVABILITY_ID) start_observability_id.assert_not_called() - def test__given_cached_impact_and_runtime_lookup_fails__then_returns_cached_result( + def test__given_cached_impact_and_runtime_lookup_fails__returns_cached_result( self, economy_service, base_params, @@ -964,7 +970,7 @@ def test__given_cached_impact_and_runtime_lookup_fails__then_returns_cached_resu mock_submission_claim_id, ): completed_impact = create_mock_reform_impact(status="ok") - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [ + hash_prefix_lookup(mock_reform_impacts_service).return_value = [ completed_impact ] mock_simulation_entrypoint.resolve_app_name.side_effect = RuntimeError( @@ -979,7 +985,7 @@ def test__given_cached_impact_and_runtime_lookup_fails__then_returns_cached_resu ) mock_simulation_entrypoint.run.assert_not_called() - def test__given_legacy_cached_impact_without_resolved_app_name__then_refreshes_cache( + def test__given_legacy_cached_impact_without_resolved_app_name__refreshes_cache( self, economy_service, base_params, @@ -997,7 +1003,7 @@ def test__given_legacy_cached_impact_without_resolved_app_name__then_refreshes_c reform_impact_json=json.dumps(MOCK_REFORM_IMPACT_DATA), options_hash=MOCK_LOOKUP_OPTIONS_HASH, ) - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [ + hash_prefix_lookup(mock_reform_impacts_service).return_value = [ completed_impact ] @@ -1011,7 +1017,7 @@ def test__given_legacy_cached_impact_without_resolved_app_name__then_refreshes_c ) mock_simulation_entrypoint.run.assert_called_once() - def test__given_legacy_and_refreshed_cached_impacts__then_reuses_refreshed_entry( + def test__given_legacy_and_refreshed_cached_impacts__reuses_refreshed_entry( self, economy_service, base_params, @@ -1030,7 +1036,7 @@ def test__given_legacy_and_refreshed_cached_impacts__then_reuses_refreshed_entry options_hash=MOCK_LOOKUP_OPTIONS_HASH, ) refreshed_impact = create_mock_reform_impact(status="ok") - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.side_effect = [ + hash_prefix_lookup(mock_reform_impacts_service).side_effect = [ [legacy_impact, refreshed_impact], [refreshed_impact], ] @@ -1044,13 +1050,10 @@ def test__given_legacy_and_refreshed_cached_impacts__then_reuses_refreshed_entry "data_version": MOCK_DATA_VERSION, "dataset": MOCK_RESOLVED_DATASET, } - assert ( - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.call_count - == 2 - ) + assert hash_prefix_lookup(mock_reform_impacts_service).call_count == 2 mock_simulation_entrypoint.run.assert_not_called() - def test__given_legacy_cached_impact_and_runtime_lookup_fails__then_returns_cached_result( + def test__given_legacy_cached_impact_and_lookup_fails__returns_cached_result( self, economy_service, base_params, @@ -1068,7 +1071,7 @@ def test__given_legacy_cached_impact_and_runtime_lookup_fails__then_returns_cach reform_impact_json=json.dumps(MOCK_REFORM_IMPACT_DATA), options_hash=MOCK_LOOKUP_OPTIONS_HASH, ) - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [ + hash_prefix_lookup(mock_reform_impacts_service).return_value = [ completed_impact ] mock_simulation_entrypoint.resolve_app_name.side_effect = RuntimeError( @@ -1081,7 +1084,7 @@ def test__given_legacy_cached_impact_and_runtime_lookup_fails__then_returns_cach assert result.data["policyengine_bundle"]["model_version"] is None mock_simulation_entrypoint.run.assert_not_called() - def test__given_legacy_computing_impact_without_resolved_app_name__then_reuses_execution( + def test__given_legacy_computing_impact_without_app_name__then_reuses_execution( self, economy_service, base_params, @@ -1098,7 +1101,7 @@ def test__given_legacy_computing_impact_without_resolved_app_name__then_reuses_e status="computing", reform_impact_json=json.dumps({}), ) - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [ + hash_prefix_lookup(mock_reform_impacts_service).return_value = [ computing_impact ] mock_simulation_entrypoint.get_execution_status.return_value = "running" @@ -1122,7 +1125,7 @@ def test__given_exception__raises_error( mock_datetime, mock_submission_claim_id, ): - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.side_effect = Exception( + hash_prefix_lookup(mock_reform_impacts_service).side_effect = Exception( "Database error" ) @@ -1143,7 +1146,7 @@ def test__given_uk_request__preserves_model_version_in_bundle( mock_submission_claim_id, ): mock_country_package_versions["uk"] = "2.7.8" - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [] + hash_prefix_lookup(mock_reform_impacts_service).return_value = [] economy_service.get_economic_impact( country_id="uk", @@ -1663,7 +1666,7 @@ def test__given_gateway_raises_before_returning_batch__clears_start_claim( ) @pytest.mark.parametrize("status_code", [400, 422]) - def test__given_modal_rejects_batch_submission_for_validation__returns_failed_result( + def test__given_modal_rejects_invalid_batch_submission__returns_failed_result( self, economy_service, base_params, @@ -1671,7 +1674,7 @@ def test__given_modal_rejects_batch_submission_for_validation__returns_failed_re mock_budget_window_cache, status_code, ): - mock_simulation_entrypoint.run_budget_window_batch.side_effect = make_http_status_error( + rejection = make_http_status_error( status_code, { "detail": ( @@ -1680,6 +1683,7 @@ def test__given_modal_rejects_batch_submission_for_validation__returns_failed_re ) }, ) + mock_simulation_entrypoint.run_budget_window_batch.side_effect = rejection result = economy_service.get_budget_window_economic_impact(**base_params) @@ -1790,12 +1794,13 @@ def test__given_end_year_after_2099__raises_value_error( with pytest.raises( ValueError, match=( - f"budget-window end_year must be {BUDGET_WINDOW_MAX_END_YEAR} or earlier" + f"budget-window end_year must be {BUDGET_WINDOW_MAX_END_YEAR} " + "or earlier" ), ): economy_service.get_budget_window_economic_impact(**base_params) - def test__given_runtime_cache_version__uses_versioned_cache_key_for_budget_window( + def test__given_runtime_cache_version__uses_versioned_budget_window_cache_key( self, economy_service, base_params, @@ -1937,7 +1942,9 @@ def test_given_valid_parameters_calls_service_correctly( self, economy_service, mock_reform_impacts_service ): expected_impacts = [create_mock_reform_impact()] - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = expected_impacts + hash_prefix_lookup( + mock_reform_impacts_service + ).return_value = expected_impacts result = economy_service._get_previous_impacts( MOCK_COUNTRY_ID, @@ -1951,7 +1958,7 @@ def test_given_valid_parameters_calls_service_correctly( ) assert result == expected_impacts - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.assert_called_once_with( + hash_prefix_lookup(mock_reform_impacts_service).assert_called_once_with( MOCK_COUNTRY_ID, MOCK_POLICY_ID, MOCK_BASELINE_POLICY_ID, @@ -1995,7 +2002,7 @@ def test__given_existing_impacts__returns_first_impact( create_mock_reform_impact(), create_mock_reform_impact(), ] - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = impacts + hash_prefix_lookup(mock_reform_impacts_service).return_value = impacts result = economy_service._get_most_recent_impact(setup_options) @@ -2008,7 +2015,7 @@ def test__given_exact_and_prefix_matches__prefers_exact_options_hash( create_mock_reform_impact(options_hash=MOCK_LOOKUP_OPTIONS_HASH), create_mock_reform_impact(options_hash=MOCK_OPTIONS_HASH), ] - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = impacts + hash_prefix_lookup(mock_reform_impacts_service).return_value = impacts result = economy_service._get_most_recent_impact(setup_options) @@ -2018,7 +2025,7 @@ def test__given_no_impacts__returns_none( self, economy_service, setup_options, mock_reform_impacts_service ): # Arrange - mock_reform_impacts_service.get_all_reform_impacts_by_options_hash_prefix.return_value = [] + hash_prefix_lookup(mock_reform_impacts_service).return_value = [] # Act result = economy_service._get_most_recent_impact(setup_options) @@ -2236,7 +2243,7 @@ def test__given_modal_failed_state__then_returns_error_result( assert result.data is None mock_reform_impacts_service.set_error_reform_impact.assert_called_once() - def test__given_modal_failed_state_with_error_message__then_includes_error_in_message( + def test__given_modal_failure_with_error_message__includes_error_in_message( self, economy_service, setup_options, diff --git a/tests/unit/services/test_household_mirroring.py b/tests/unit/services/test_household_mirroring.py index a33b904df..5afc046ca 100644 --- a/tests/unit/services/test_household_mirroring.py +++ b/tests/unit/services/test_household_mirroring.py @@ -175,9 +175,7 @@ def test_destination_failure_is_secret_safe_and_leaves_event_pending( assert "private value" not in repr(payload) -def test_source_completion_failure_records_committed_destination_and_pending_event() -> ( - None -): +def test_source_completion_failure_records_destination_and_pending_event() -> None: mirror = MagicMock() mirror.mirror_legacy_household.return_value = _result() diff --git a/tests/unit/services/test_metadata_service.py b/tests/unit/services/test_metadata_service.py index 25927e75f..86423faff 100644 --- a/tests/unit/services/test_metadata_service.py +++ b/tests/unit/services/test_metadata_service.py @@ -14,7 +14,10 @@ def test_get_metadata_nonexistent_country(self): # THEN it should raise an Exception with pytest.raises( Exception, - match=f"Attempted to get metadata for a nonexistant country: '{invalid_country_id}'", + match=( + "Attempted to get metadata for a nonexistant country: " + f"'{invalid_country_id}'" + ), ): service.get_metadata(invalid_country_id) @@ -28,7 +31,10 @@ def test_get_metadata_empty_country_id(self): # THEN it should raise an Exception with pytest.raises( Exception, - match=f"Attempted to get metadata for a nonexistant country: '{empty_country_id}'", + match=( + "Attempted to get metadata for a nonexistant country: " + f"'{empty_country_id}'" + ), ): service.get_metadata(empty_country_id) diff --git a/tests/unit/test_alembic_skill.py b/tests/unit/test_alembic_skill.py index c61312390..772a4ac66 100644 --- a/tests/unit/test_alembic_skill.py +++ b/tests/unit/test_alembic_skill.py @@ -10,7 +10,8 @@ def test_model_agnostic_alembic_skill_is_discoverable(): in (REPO / "docs" / "engineering" / "skills" / "README.md").read_text() ) # AGENTS.md is the one source of agent instructions; CLAUDE.md and the - # Copilot instructions only point to it (rule api.agents.adapters-point-to-agents-md). + # Copilot instructions only point to it + # (rule api.agents.adapters-point-to-agents-md). assert ( "docs/engineering/skills/alembic-migrations.md" in (REPO / "AGENTS.md").read_text() diff --git a/tests/unit/test_alembic_workflows.py b/tests/unit/test_alembic_workflows.py index 5e978e5a7..f95d0d594 100644 --- a/tests/unit/test_alembic_workflows.py +++ b/tests/unit/test_alembic_workflows.py @@ -286,7 +286,7 @@ def test_cloud_sql_workflow_uses_oidc_and_separate_database_credentials(): ) -def test_database_environment_validation_rejects_production_migration_credentials_in_staging(): +def test_database_environment_validation_rejects_production_credentials_in_staging(): result = subprocess.run( ["bash", ".github/scripts/validate_database_environment.sh", "cloud-sql"], cwd=REPO, diff --git a/tests/unit/test_country.py b/tests/unit/test_country.py index ea2c753fe..b9577320b 100644 --- a/tests/unit/test_country.py +++ b/tests/unit/test_country.py @@ -302,7 +302,8 @@ def test__file_has_360_local_authorities(self, local_authorities_df): def test__all_codes_are_valid_ons_codes(self, local_authorities_df): """Verify all codes follow ONS local authority code patterns.""" for code in local_authorities_df["code"]: - # ONS codes start with E (England), S (Scotland), W (Wales), or N (Northern Ireland) + # ONS codes start with E (England), S (Scotland), W (Wales), or N + # (Northern Ireland) assert code[0] in ["E", "S", "W", "N"] def test__all_names_are_non_empty(self, local_authorities_df): diff --git a/tests/unit/test_deploy_pipeline_scripts.py b/tests/unit/test_deploy_pipeline_scripts.py index d29b3807f..cef35fe3d 100644 --- a/tests/unit/test_deploy_pipeline_scripts.py +++ b/tests/unit/test_deploy_pipeline_scripts.py @@ -95,7 +95,8 @@ def _fake_make(tmp_path: Path, exit_code: int = 0) -> tuple[Path, Path]: env_dump = tmp_path / "env.txt" make = bin_dir / "make" make.write_text( - f'#!/usr/bin/env bash\necho "args=$*" > "{env_dump}"\nenv >> "{env_dump}"\nexit {exit_code}\n' + f'#!/usr/bin/env bash\necho "args=$*" > "{env_dump}"\n' + f'env >> "{env_dump}"\nexit {exit_code}\n' ) make.chmod(0o755) return bin_dir, env_dump diff --git a/tests/unit/test_workflow_guard_scripts.py b/tests/unit/test_workflow_guard_scripts.py index 0763e7f15..b43585f09 100644 --- a/tests/unit/test_workflow_guard_scripts.py +++ b/tests/unit/test_workflow_guard_scripts.py @@ -1,4 +1,7 @@ -"""Tests for small workflow scripts: require_git_ref.sh and cleanup_cloud_run_images.sh.""" +"""Tests for small workflow scripts. + +The scripts are require_git_ref.sh and cleanup_cloud_run_images.sh. +""" from __future__ import annotations From bf9ef759dee9f93dd88d3309f4f8f7bc98ef5c33 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 22:49:47 +0400 Subject: [PATCH 09/15] Enable ruff group 7 (structure and imports) Select A, E, FAST, INP, N, PLC and SLF. The earlier commits in this branch fix or explain every finding, so ruff reports none. Co-Authored-By: Claude Opus 5.5 --- pyproject.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index edfb6c71d..595e2aa13 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -116,6 +116,8 @@ select = [ "BLE", "EM", "TRY", # Group 6: security, logging and debugging leftovers. "DTZ", "ERA", "EXE", "FIX", "G", "LOG", "S", "T20", "TD", + # Group 7: structure and imports. + "A", "E", "FAST", "INP", "N", "PLC", "SLF", ] # Mutually exclusive pydocstyle pairs: keep D211 (no blank line before a class # docstring) and D212 (summary on the first line). From ce2ca7f87943ef041b5ef3160903f756496e227c Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 23:03:00 +0400 Subject: [PATCH 10/15] Register group 7 in the ruff-lint rule api.quality.ruff-lint now lists A, E, FAST, INP, N, PLC and SLF. The rendered rules page also picks up the rules tool's new hash. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- changelog.d/ruff-structure-imports.changed.md | 1 + docs/generated/rules.md | 8 ++++---- rules/registry.toml | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) create mode 100644 changelog.d/ruff-structure-imports.changed.md diff --git a/AGENTS.md b/AGENTS.md index a4cce241b..940c1e774 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -126,7 +126,7 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. - `api.quality.environment-variables-load`: Required environment variables and credentials load in a deployed-like environment. (blocking: CI job `test_env_vars`) - `api.quality.mypy-v2`: Shared query models, FastAPI dependencies, v2 routes and services, and the user sign-in and account-deletion libraries pass mypy with untyped definitions disallowed. (blocking: mypy) - `api.quality.ruff-format`: All Python files are formatted with ruff format. (blocking: ruff format) -- `api.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (ANN, BLE, D, DTZ, E4, E7, E9, EM, ERA, EXE, F, FIX, FLY, FURB, G, I, LOG, PIE, PYI, RET, RSE, S, T20, TC, TD, TRY, UP, W)) +- `api.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in pyproject.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)) - `api.quality.runtime-evaluated-models-resolve`: Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. (blocking: pytest `tests/unit/test_runtime_evaluated_annotations.py`) - `api.quality.tests-pass`: The unit, contract and to-refactor test suites pass. (blocking: CI job `test`) - `api.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). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201). Every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, DTZ001, DTZ011, INP001, PLC0415, PLR2004, S101, S105, S106, S603, S607, S608, SLF001, T201 for `**/tests/**`) diff --git a/changelog.d/ruff-structure-imports.changed.md b/changelog.d/ruff-structure-imports.changed.md new file mode 100644 index 000000000..930b209df --- /dev/null +++ b/changelog.d/ruff-structure-imports.changed.md @@ -0,0 +1 @@ +Enabled ruff's structure and import rules (INP, PLC, SLF, N, A, FAST, E): long lines are wrapped, FastAPI dependencies are declared with Annotated, deferred imports that avoid heavy loads, cycles or import-time I/O carry a reason, and directories without an __init__.py on purpose are declared to ruff as namespace packages; tests are also exempt from SLF001, INP001 and PLC0415. Published schemas and the HTTP API are unchanged. diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 7184b03b2..b6253bab4 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -275,7 +275,7 @@ flowchart LR | `api.quality.environment-variables-load` | Required environment variables and credentials load in a deployed-like environment. | mechanical | enforced | yes | CI job `test_env_vars` | `pr.yml:test_env_vars` | — | | `api.quality.mypy-v2` | Shared query models, FastAPI dependencies, v2 routes and services, and the user sign-in and account-deletion libraries pass mypy with untyped definitions disallowed. | mechanical | enforced | yes | mypy | `pr.yml:typecheck-v2` | — | | `api.quality.ruff-format` | All Python files are formatted with ruff format. | mechanical | enforced | yes | ruff format | `pr.yml:lint` | — | -| `api.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. | mechanical | enforced | yes | ruff check (ANN, BLE, D, DTZ, E4, E7, E9, EM, ERA, EXE, F, FIX, FLY, FURB, G, I, LOG, PIE, PYI, RET, RSE, S, T20, TC, TD, TRY, UP, W) | `pr.yml:lint` | — | +| `api.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in pyproject.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` | — | | `api.quality.runtime-evaluated-models-resolve` | Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. | mechanical | enforced | yes | pytest `tests/unit/test_runtime_evaluated_annotations.py` | `pr.yml:test` | **unproven** | | `api.quality.tests-pass` | The unit, contract and to-refactor test suites pass. | mechanical | enforced | yes | CI job `test` | `pr.yml:test` | — | | `api.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). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201). Every other ruff rule applies to tests. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, DTZ001, DTZ011, INP001, PLC0415, PLR2004, S101, S105, S106, S603, S607, S608, SLF001, T201 for `**/tests/**` | `pr.yml:lint` | — | @@ -300,7 +300,7 @@ flowchart LR r_api_quality_ruff_format("api.quality.ruff-format (enforced)") e_ruff_format["ruff format"] r_api_quality_ruff_lint("api.quality.ruff-lint (enforced)") - e_ruff_check__ANN__BLE__D__DTZ__E4__E7__E9__EM__ERA__EXE__F__FIX__FLY__FURB__G__I__LOG__PIE__PYI__RET__RSE__S__T20__TC__TD__TRY__UP__W_["ruff check (ANN, BLE, D, DTZ, E4, E7, E9, EM, ERA, EXE, F, FIX, FLY, FURB, G, I, LOG, PIE, PYI, RET, RSE, S, 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_["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)"] r_api_quality_runtime_evaluated_models_resolve("api.quality.runtime-evaluated-models-resolve (enforced)") e_pytest_tests_unit_test_runtime_evaluated_annotations_py["pytest tests/unit/test_runtime_evaluated_annotations.py"] j_CI_pr_yml_test["CI pr.yml:test"] @@ -321,8 +321,8 @@ flowchart LR e_mypy --> j_CI_pr_yml_typecheck_v2 r_api_quality_ruff_format --> e_ruff_format e_ruff_format --> j_CI_pr_yml_lint - r_api_quality_ruff_lint --> e_ruff_check__ANN__BLE__D__DTZ__E4__E7__E9__EM__ERA__EXE__F__FIX__FLY__FURB__G__I__LOG__PIE__PYI__RET__RSE__S__T20__TC__TD__TRY__UP__W_ - e_ruff_check__ANN__BLE__D__DTZ__E4__E7__E9__EM__ERA__EXE__F__FIX__FLY__FURB__G__I__LOG__PIE__PYI__RET__RSE__S__T20__TC__TD__TRY__UP__W_ --> j_CI_pr_yml_lint + r_api_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_api_quality_runtime_evaluated_models_resolve --> e_pytest_tests_unit_test_runtime_evaluated_annotations_py e_pytest_tests_unit_test_runtime_evaluated_annotations_py --> j_CI_pr_yml_test r_api_quality_tests_pass --> e_CI_job_test diff --git a/rules/registry.toml b/rules/registry.toml index 4c0d0a70a..4f32709cc 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -700,7 +700,7 @@ autofix = true [rule.enforcer] kind = "ruff" mode = "lint" -codes = ["ANN", "BLE", "D", "DTZ", "E4", "E7", "E9", "EM", "ERA", "EXE", "F", "FIX", "FLY", "FURB", "G", "I", "LOG", "PIE", "PYI", "RET", "RSE", "S", "T20", "TC", "TD", "TRY", "UP", "W"] +codes = ["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"] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "lint" From adc25c4f612916355762012ce663ba0de26fe642 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 01:19:36 +0400 Subject: [PATCH 11/15] Declare the user routes' FastAPI dependencies with Annotated (FAST002) The /v2/me, user-report, user-simulation, user-household and user-policy routes that #3891 added or reshaped took 44 dependencies as parameter defaults. Their query dependencies become module-level Annotated aliases, as the other v2 routes do, and the request-size check and the upgrade route's unauthenticated-user header become Annotated parameters. Each handler's principal comes from require_user, the authenticator that NativeRouteDependencies injects per router. FastAPI resolves string annotations against module globals only: with `from __future__ import annotations`, Annotated[AccountPrincipal, Depends(require_user)] would silently become a required "principal" query parameter. These five modules therefore no longer use the future import, so their annotations are evaluated where the handlers are defined, and import the route dependency types and their default service factory at run time. Moving the default factory import out of _service_factory also clears its PLC0415 finding in the three new modules, as the other v2 resource routers already do; the dependencies module is already loaded wherever these routers are built. The OpenAPI documents, with the injected test authenticator and with the default one, are byte-identical. Co-Authored-By: Claude Opus 5.5 --- .../fastapi_routes/v2/me/routes.py | 64 +++++++++---------- .../v2/user_households/routes.py | 26 +++----- .../fastapi_routes/v2/user_policies/routes.py | 26 +++----- .../fastapi_routes/v2/user_reports/routes.py | 55 ++++++++-------- .../v2/user_simulations/routes.py | 50 +++++++-------- 5 files changed, 99 insertions(+), 122 deletions(-) diff --git a/policyengine_api/fastapi_routes/v2/me/routes.py b/policyengine_api/fastapi_routes/v2/me/routes.py index 7bb04376b..3c64d0659 100644 --- a/policyengine_api/fastapi_routes/v2/me/routes.py +++ b/policyengine_api/fastapi_routes/v2/me/routes.py @@ -7,14 +7,18 @@ requires a credential; authentication is a router-level dependency. """ -from __future__ import annotations - -from typing import TYPE_CHECKING +from collections.abc import Callable +from typing import Annotated from uuid import UUID from fastapi import APIRouter, Depends, Header from starlette.responses import JSONResponse, Response +from policyengine_api.fastapi_routes.dependencies import ( + NativeRouteDependencies, + V2AccountResourceService, + _default_v2_account_service_factory, +) from policyengine_api.fastapi_routes.query_parameters import query_dependency from policyengine_api.fastapi_routes.v2.errors import V2ErrorResponse from policyengine_api.fastapi_routes.v2.me.request_models import ( @@ -45,13 +49,9 @@ UserPrincipal, ) -if TYPE_CHECKING: - from collections.abc import Callable - - from policyengine_api.fastapi_routes.dependencies import ( - NativeRouteDependencies, - V2AccountResourceService, - ) +CountryQueryDependency = Annotated[ + CountryQuery, Depends(query_dependency(CountryQuery)) +] def _service_factory( @@ -59,10 +59,6 @@ def _service_factory( ) -> Callable[[], V2AccountResourceService]: if dependencies.v2_account_service_factory is not None: return dependencies.v2_account_service_factory - from policyengine_api.fastapi_routes.dependencies import ( - _default_v2_account_service_factory, - ) - return _default_v2_account_service_factory @@ -74,11 +70,10 @@ def build_v2_me_router(dependencies: NativeRouteDependencies) -> APIRouter: responses=USER_RESOURCE_ERROR_RESPONSES, dependencies=[Depends(require_user)], ) - country_query = query_dependency(CountryQuery) service_factory = _service_factory(dependencies) def require_signed_in_user( - principal: AccountPrincipal = Depends(require_user), + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> UserPrincipal: return require_signed_in_principal(principal) @@ -95,8 +90,8 @@ def require_signed_in_user( ), ) def get_me( - query: CountryQuery = Depends(country_query), - principal: AccountPrincipal = Depends(require_user), + query: CountryQueryDependency, + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> MeDetailResponse | JSONResponse: def read() -> MeDetailResponse: item = service_factory().get_account( @@ -115,9 +110,9 @@ def read() -> MeDetailResponse: ) def patch_me( body: AccountPatchRequest, - query: CountryQuery = Depends(country_query), - principal: AccountPrincipal = Depends(require_user), - _size: None = Depends(enforce_user_write_request_size), + query: CountryQueryDependency, + principal: Annotated[AccountPrincipal, Depends(require_user)], + _size: Annotated[None, Depends(enforce_user_write_request_size)], ) -> MeDetailResponse | JSONResponse: def patch() -> MeDetailResponse: item = service_factory().update_account( @@ -161,8 +156,8 @@ def patch() -> MeDetailResponse: ), ) def delete_me( - query: CountryQuery = Depends(country_query), - principal: AccountPrincipal = Depends(require_user), + query: CountryQueryDependency, + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> Response | JSONResponse: def delete() -> Response: service_factory().delete_account(principal=principal) @@ -180,8 +175,8 @@ def delete() -> Response: ), ) def export_me( - query: CountryQuery = Depends(country_query), - principal: AccountPrincipal = Depends(require_user), + query: CountryQueryDependency, + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> MeExportResponse | JSONResponse: def export() -> MeExportResponse: result = service_factory().export_account( @@ -209,14 +204,17 @@ def export() -> MeExportResponse: ), ) def upgrade_me( - query: CountryQuery = Depends(country_query), - principal: UserPrincipal = Depends(require_signed_in_user), - unauthenticated_user: str = Header( - alias=UNAUTHENTICATED_USER_HEADER, - pattern=CANONICAL_UUID_PATTERN, - description="The browser's unauthenticated user ID (a canonical UUID).", - ), - _size: None = Depends(enforce_user_write_request_size), + query: CountryQueryDependency, + principal: Annotated[UserPrincipal, Depends(require_signed_in_user)], + unauthenticated_user: Annotated[ + str, + Header( + alias=UNAUTHENTICATED_USER_HEADER, + pattern=CANONICAL_UUID_PATTERN, + description="The browser's unauthenticated user ID (a canonical UUID).", + ), + ], + _size: Annotated[None, Depends(enforce_user_write_request_size)], ) -> MeUpgradeResponse | JSONResponse: def upgrade() -> MeUpgradeResponse: result = service_factory().upgrade_account( diff --git a/policyengine_api/fastapi_routes/v2/user_households/routes.py b/policyengine_api/fastapi_routes/v2/user_households/routes.py index 836a3c76d..04b2c63f7 100644 --- a/policyengine_api/fastapi_routes/v2/user_households/routes.py +++ b/policyengine_api/fastapi_routes/v2/user_households/routes.py @@ -1,14 +1,15 @@ """Native FastAPI routes for v2 user-household associations.""" -from __future__ import annotations - -from typing import TYPE_CHECKING, Annotated +from collections.abc import Callable +from typing import Annotated from uuid import UUID from fastapi import APIRouter, Depends from starlette.responses import JSONResponse, Response from policyengine_api.fastapi_routes.dependencies import ( + NativeRouteDependencies, + V2UserHouseholdResourceService, _default_v2_user_household_service_factory, ) from policyengine_api.fastapi_routes.query_parameters import query_dependency @@ -38,15 +39,6 @@ ) from policyengine_api.services.v2.identity.types import AccountPrincipal -if TYPE_CHECKING: - from collections.abc import Callable - - from policyengine_api.fastapi_routes.dependencies import ( - NativeRouteDependencies, - V2UserHouseholdResourceService, - ) - - UserHouseholdCreateQueryDependency = Annotated[ UserHouseholdCreateQuery, Depends(query_dependency(UserHouseholdCreateQuery)) ] @@ -107,7 +99,7 @@ def build_v2_user_household_router( def create_user_household( body: UserHouseholdCreateRequest, query: UserHouseholdCreateQueryDependency, - principal: AccountPrincipal = Depends(require_user), + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> UserHouseholdDetailResponse | JSONResponse: if body.country_id != query.country_id: return v2_error_response(400, "Body country_id must match query country_id") @@ -131,7 +123,7 @@ def create() -> UserHouseholdDetailResponse: def get_user_household( association_id: UUID, query: UserHouseholdDetailQueryDependency, - principal: AccountPrincipal = Depends(require_user), + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> UserHouseholdDetailResponse | JSONResponse: def read() -> UserHouseholdDetailResponse: item = service_factory().get_user_household( @@ -152,7 +144,7 @@ def read() -> UserHouseholdDetailResponse: ) def get_user_households( query: UserHouseholdCollectionQueryDependency, - principal: AccountPrincipal = Depends(require_user), + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> UserHouseholdPageResponse | JSONResponse: def read() -> UserHouseholdPageResponse: page = service_factory().list_user_households( @@ -182,7 +174,7 @@ def patch_user_household_route( association_id: UUID, body: UserHouseholdPatchRequest, query: UserHouseholdUpdateQueryDependency, - principal: AccountPrincipal = Depends(require_user), + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> UserHouseholdDetailResponse | JSONResponse: def patch() -> UserHouseholdDetailResponse: item = service_factory().patch_user_household( @@ -207,7 +199,7 @@ def patch() -> UserHouseholdDetailResponse: def delete_user_household_route( association_id: UUID, query: UserHouseholdDeleteQueryDependency, - principal: AccountPrincipal = Depends(require_user), + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> Response | JSONResponse: def delete() -> Response: service_factory().delete_user_household( diff --git a/policyengine_api/fastapi_routes/v2/user_policies/routes.py b/policyengine_api/fastapi_routes/v2/user_policies/routes.py index f7078b485..4a2f843dc 100644 --- a/policyengine_api/fastapi_routes/v2/user_policies/routes.py +++ b/policyengine_api/fastapi_routes/v2/user_policies/routes.py @@ -1,14 +1,15 @@ """Native FastAPI routes for v2 user-policy associations.""" -from __future__ import annotations - -from typing import TYPE_CHECKING, Annotated +from collections.abc import Callable +from typing import Annotated from uuid import UUID from fastapi import APIRouter, Depends from starlette.responses import JSONResponse, Response from policyengine_api.fastapi_routes.dependencies import ( + NativeRouteDependencies, + V2UserPolicyResourceService, _default_v2_user_policy_service_factory, ) from policyengine_api.fastapi_routes.query_parameters import query_dependency @@ -35,15 +36,6 @@ ) from policyengine_api.services.v2.identity.types import AccountPrincipal -if TYPE_CHECKING: - from collections.abc import Callable - - from policyengine_api.fastapi_routes.dependencies import ( - NativeRouteDependencies, - V2UserPolicyResourceService, - ) - - CountryQueryDependency = Annotated[ CountryQuery, Depends(query_dependency(CountryQuery)) ] @@ -95,7 +87,7 @@ def build_v2_user_policy_router( def create_user_policy( body: UserPolicyCreateRequest, query: CountryQueryDependency, - principal: AccountPrincipal = Depends(require_user), + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> UserPolicyDetailResponse | JSONResponse: if body.country_id != query.country_id: return v2_error_response( @@ -122,7 +114,7 @@ def create() -> UserPolicyDetailResponse: def get_user_policy( association_id: UUID, query: CountryQueryDependency, - principal: AccountPrincipal = Depends(require_user), + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> UserPolicyDetailResponse | JSONResponse: def read() -> UserPolicyDetailResponse: item = service_factory().get_user_policy( @@ -143,7 +135,7 @@ def read() -> UserPolicyDetailResponse: ) def get_user_policies( query: UserPolicyCollectionQueryDependency, - principal: AccountPrincipal = Depends(require_user), + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> UserPolicyPageResponse | JSONResponse: def read() -> UserPolicyPageResponse: page = service_factory().list_user_policies( @@ -172,7 +164,7 @@ def patch_user_policy_route( association_id: UUID, body: UserPolicyPatchRequest, query: CountryQueryDependency, - principal: AccountPrincipal = Depends(require_user), + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> UserPolicyDetailResponse | JSONResponse: def patch() -> UserPolicyDetailResponse: item = service_factory().patch_user_policy( @@ -197,7 +189,7 @@ def patch() -> UserPolicyDetailResponse: def delete_user_policy_route( association_id: UUID, query: CountryQueryDependency, - principal: AccountPrincipal = Depends(require_user), + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> Response | JSONResponse: def delete() -> Response: service_factory().delete_user_policy( diff --git a/policyengine_api/fastapi_routes/v2/user_reports/routes.py b/policyengine_api/fastapi_routes/v2/user_reports/routes.py index 502ca468b..d81170bd5 100644 --- a/policyengine_api/fastapi_routes/v2/user_reports/routes.py +++ b/policyengine_api/fastapi_routes/v2/user_reports/routes.py @@ -4,14 +4,18 @@ shareable link, read without one through ``/v2/shared-reports``. """ -from __future__ import annotations - -from typing import TYPE_CHECKING, Any +from collections.abc import Callable +from typing import Annotated, Any from uuid import UUID from fastapi import APIRouter, Depends from starlette.responses import JSONResponse, Response +from policyengine_api.fastapi_routes.dependencies import ( + NativeRouteDependencies, + V2UserReportResourceService, + _default_v2_user_report_service_factory, +) from policyengine_api.fastapi_routes.query_parameters import query_dependency from policyengine_api.fastapi_routes.v2.errors import ( V2ErrorResponse, @@ -42,14 +46,6 @@ ) from policyengine_api.services.v2.identity.types import AccountPrincipal -if TYPE_CHECKING: - from collections.abc import Callable - - from policyengine_api.fastapi_routes.dependencies import ( - NativeRouteDependencies, - V2UserReportResourceService, - ) - SHARED_REPORT_ERROR_RESPONSES: dict[int | str, dict[str, Any]] = { 404: { "model": V2ErrorResponse, @@ -64,15 +60,19 @@ } +CountryQueryDependency = Annotated[ + CountryQuery, Depends(query_dependency(CountryQuery)) +] +UserReportCollectionQueryDependency = Annotated[ + UserReportCollectionQuery, Depends(query_dependency(UserReportCollectionQuery)) +] + + def _service_factory( dependencies: NativeRouteDependencies, ) -> Callable[[], V2UserReportResourceService]: if dependencies.v2_user_report_service_factory is not None: return dependencies.v2_user_report_service_factory - from policyengine_api.fastapi_routes.dependencies import ( - _default_v2_user_report_service_factory, - ) - return _default_v2_user_report_service_factory @@ -93,8 +93,6 @@ def build_v2_user_report_router( Depends(enforce_user_write_request_size), ], ) - country_query = query_dependency(CountryQuery) - collection_query = query_dependency(UserReportCollectionQuery) service_factory = _service_factory(dependencies) @router.post( @@ -112,8 +110,8 @@ def build_v2_user_report_router( ) def create_user_report( body: UserReportCreateRequest, - query: CountryQuery = Depends(country_query), - principal: AccountPrincipal = Depends(require_user), + query: CountryQueryDependency, + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> UserReportDetailResponse | JSONResponse: if body.country_id != query.country_id: return v2_error_response(400, "Body country_id must match query country_id") @@ -136,8 +134,8 @@ def create() -> UserReportDetailResponse: ) def get_user_report( association_id: UUID, - query: CountryQuery = Depends(country_query), - principal: AccountPrincipal = Depends(require_user), + query: CountryQueryDependency, + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> UserReportDetailResponse | JSONResponse: def read() -> UserReportDetailResponse: item = service_factory().get_user_report( @@ -157,8 +155,8 @@ def read() -> UserReportDetailResponse: summary="List the current user's saved reports", ) def get_user_reports( - query: UserReportCollectionQuery = Depends(collection_query), - principal: AccountPrincipal = Depends(require_user), + query: UserReportCollectionQueryDependency, + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> UserReportPageResponse | JSONResponse: def read() -> UserReportPageResponse: page = service_factory().list_user_reports( @@ -180,8 +178,8 @@ def read() -> UserReportPageResponse: def patch_user_report_route( association_id: UUID, body: UserReportPatchRequest, - query: CountryQuery = Depends(country_query), - principal: AccountPrincipal = Depends(require_user), + query: CountryQueryDependency, + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> UserReportDetailResponse | JSONResponse: def patch() -> UserReportDetailResponse: item = service_factory().patch_user_report( @@ -205,8 +203,8 @@ def patch() -> UserReportDetailResponse: ) def delete_user_report_route( association_id: UUID, - query: CountryQuery = Depends(country_query), - principal: AccountPrincipal = Depends(require_user), + query: CountryQueryDependency, + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> Response | JSONResponse: def delete() -> Response: service_factory().delete_user_report( @@ -226,7 +224,6 @@ def build_v2_shared_report_router( ) -> APIRouter: """Build the credential-free read of a saved report by its link.""" router = APIRouter(prefix="/v2", responses=SHARED_REPORT_ERROR_RESPONSES) - country_query = query_dependency(CountryQuery) service_factory = _service_factory(dependencies) @router.get( @@ -242,7 +239,7 @@ def build_v2_shared_report_router( ) def get_shared_report( association_id: UUID, - query: CountryQuery = Depends(country_query), + query: CountryQueryDependency, ) -> SharedUserReportDetailResponse | JSONResponse: def read() -> SharedUserReportDetailResponse: item = service_factory().get_shared_user_report( diff --git a/policyengine_api/fastapi_routes/v2/user_simulations/routes.py b/policyengine_api/fastapi_routes/v2/user_simulations/routes.py index 0915ba779..10e6bb6a0 100644 --- a/policyengine_api/fastapi_routes/v2/user_simulations/routes.py +++ b/policyengine_api/fastapi_routes/v2/user_simulations/routes.py @@ -1,13 +1,17 @@ """Native FastAPI routes for v2 user-simulation associations.""" -from __future__ import annotations - -from typing import TYPE_CHECKING +from collections.abc import Callable +from typing import Annotated from uuid import UUID from fastapi import APIRouter, Depends from starlette.responses import JSONResponse, Response +from policyengine_api.fastapi_routes.dependencies import ( + NativeRouteDependencies, + V2UserSimulationResourceService, + _default_v2_user_simulation_service_factory, +) from policyengine_api.fastapi_routes.query_parameters import query_dependency from policyengine_api.fastapi_routes.v2.errors import v2_error_response from policyengine_api.fastapi_routes.v2.user_resources import ( @@ -32,13 +36,13 @@ ) from policyengine_api.services.v2.identity.types import AccountPrincipal -if TYPE_CHECKING: - from collections.abc import Callable - - from policyengine_api.fastapi_routes.dependencies import ( - NativeRouteDependencies, - V2UserSimulationResourceService, - ) +CountryQueryDependency = Annotated[ + CountryQuery, Depends(query_dependency(CountryQuery)) +] +UserSimulationCollectionQueryDependency = Annotated[ + UserSimulationCollectionQuery, + Depends(query_dependency(UserSimulationCollectionQuery)), +] def _service_factory( @@ -46,10 +50,6 @@ def _service_factory( ) -> Callable[[], V2UserSimulationResourceService]: if dependencies.v2_user_simulation_service_factory is not None: return dependencies.v2_user_simulation_service_factory - from policyengine_api.fastapi_routes.dependencies import ( - _default_v2_user_simulation_service_factory, - ) - return _default_v2_user_simulation_service_factory @@ -70,8 +70,6 @@ def build_v2_user_simulation_router( Depends(enforce_user_write_request_size), ], ) - country_query = query_dependency(CountryQuery) - collection_query = query_dependency(UserSimulationCollectionQuery) service_factory = _service_factory(dependencies) @router.post( @@ -88,8 +86,8 @@ def build_v2_user_simulation_router( ) def create_user_simulation( body: UserSimulationCreateRequest, - query: CountryQuery = Depends(country_query), - principal: AccountPrincipal = Depends(require_user), + query: CountryQueryDependency, + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> UserSimulationDetailResponse | JSONResponse: if body.country_id != query.country_id: return v2_error_response(400, "Body country_id must match query country_id") @@ -114,8 +112,8 @@ def create() -> UserSimulationDetailResponse: ) def get_user_simulation( association_id: UUID, - query: CountryQuery = Depends(country_query), - principal: AccountPrincipal = Depends(require_user), + query: CountryQueryDependency, + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> UserSimulationDetailResponse | JSONResponse: def read() -> UserSimulationDetailResponse: item = service_factory().get_user_simulation( @@ -137,8 +135,8 @@ def read() -> UserSimulationDetailResponse: summary="List the current user's saved simulations", ) def get_user_simulations( - query: UserSimulationCollectionQuery = Depends(collection_query), - principal: AccountPrincipal = Depends(require_user), + query: UserSimulationCollectionQueryDependency, + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> UserSimulationPageResponse | JSONResponse: def read() -> UserSimulationPageResponse: page = service_factory().list_user_simulations( @@ -162,8 +160,8 @@ def read() -> UserSimulationPageResponse: def patch_user_simulation_route( association_id: UUID, body: UserSimulationPatchRequest, - query: CountryQuery = Depends(country_query), - principal: AccountPrincipal = Depends(require_user), + query: CountryQueryDependency, + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> UserSimulationDetailResponse | JSONResponse: def patch() -> UserSimulationDetailResponse: item = service_factory().patch_user_simulation( @@ -189,8 +187,8 @@ def patch() -> UserSimulationDetailResponse: ) def delete_user_simulation_route( association_id: UUID, - query: CountryQuery = Depends(country_query), - principal: AccountPrincipal = Depends(require_user), + query: CountryQueryDependency, + principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> Response | JSONResponse: def delete() -> Response: service_factory().delete_user_simulation( From da5eb457eac35099e09a187857707c5c63c258fc Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 01:23:51 +0400 Subject: [PATCH 12/15] Move the safe import #3891 added to the top level and explain the rest Of the 18 PLC0415 findings left in the code #3891 added, the route dependencies' account-request authenticator import moves to the top level. Its module is already loaded wherever the dependencies module is, and it creates no cycle; the ASGI and Flask entry points import exactly the same modules as before. The other deferred imports keep an inline reason, worded as for the existing ones: the v1 session factory is looked up per call so tests can replace it; the v1 reference checks, v2 persistence, the user-account service and Auth0 Management load only when a default factory runs or a v1 reference is checked; readiness validation loads the user-auth and Auth0 settings only when it runs; and the Google Secret Manager client loads only when Auth0 Management reads a secret. Co-Authored-By: Claude Opus 5.5 --- .../fastapi_routes/dependencies.py | 43 +++++++++++-------- policyengine_api/libs/auth0_management.py | 4 +- policyengine_api/readiness.py | 6 ++- 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/policyengine_api/fastapi_routes/dependencies.py b/policyengine_api/fastapi_routes/dependencies.py index bf4bab2ba..81a1e6154 100644 --- a/policyengine_api/fastapi_routes/dependencies.py +++ b/policyengine_api/fastapi_routes/dependencies.py @@ -10,6 +10,7 @@ from fastapi import Depends from fastapi.security import APIKeyHeader, HTTPAuthorizationCredentials, HTTPBearer +from policyengine_api.fastapi_routes.v2.user_auth import authenticate_account_request from policyengine_api.libs.user_auth import UNAUTHENTICATED_USER_HEADER from policyengine_api.readiness import is_ready @@ -800,10 +801,6 @@ def _default_require_user( unauthenticated_user: str | None = Depends(_unauthenticated_user_scheme), ) -> AccountPrincipal: """Authenticate the request's account; fails closed without a credential.""" - from policyengine_api.fastapi_routes.v2.user_auth import ( - authenticate_account_request, - ) - return authenticate_account_request( None if credentials is None else f"Bearer {credentials.credentials}", unauthenticated_user, @@ -811,11 +808,13 @@ def _default_require_user( def _build_v1_legacy_reference_service() -> LegacyReferenceChecker: - from policyengine_api.data.orm import get_v1_session_factory - from policyengine_api.services.v2.legacy_refs.database_session import ( + from policyengine_api.data.orm import ( # noqa: PLC0415 # looked up per call, so tests can replace the v1 session factory + get_v1_session_factory, + ) + from policyengine_api.services.v2.legacy_refs.database_session import ( # noqa: PLC0415 # v1 reference checks load only when a request checks one LegacyReferenceDatabaseSession, ) - from policyengine_api.services.v2.legacy_refs.services import ( + from policyengine_api.services.v2.legacy_refs.services import ( # noqa: PLC0415 # v1 reference checks load only when a request checks one V1LegacyReferenceService, ) @@ -826,7 +825,7 @@ def _build_v1_legacy_reference_service() -> LegacyReferenceChecker: def _default_legacy_reference_checker() -> LegacyReferenceChecker: """Defer the v1 session factory until a request checks a v1 reference.""" - from policyengine_api.services.v2.legacy_refs.services import ( + from policyengine_api.services.v2.legacy_refs.services import ( # noqa: PLC0415 # v1 reference checks load only when a default factory runs DeferredLegacyReferenceService, ) @@ -868,11 +867,13 @@ def _default_v2_user_household_service_factory() -> V2UserHouseholdResourceServi def _default_v2_user_simulation_service_factory() -> V2UserSimulationResourceService: - from policyengine_api.data.v2.database import get_v2_session_factory - from policyengine_api.services.v2.user_simulations.database_session import ( + from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + get_v2_session_factory, + ) + from policyengine_api.services.v2.user_simulations.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs UserSimulationDatabaseSession, ) - from policyengine_api.services.v2.user_simulations.services import ( + from policyengine_api.services.v2.user_simulations.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs V2UserSimulationService, ) @@ -883,11 +884,13 @@ def _default_v2_user_simulation_service_factory() -> V2UserSimulationResourceSer def _default_v2_user_report_service_factory() -> V2UserReportResourceService: - from policyengine_api.data.v2.database import get_v2_session_factory - from policyengine_api.services.v2.user_reports.database_session import ( + from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + get_v2_session_factory, + ) + from policyengine_api.services.v2.user_reports.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs UserReportDatabaseSession, ) - from policyengine_api.services.v2.user_reports.services import ( + from policyengine_api.services.v2.user_reports.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs V2UserReportService, ) @@ -898,12 +901,16 @@ def _default_v2_user_report_service_factory() -> V2UserReportResourceService: def _default_v2_account_service_factory() -> V2AccountResourceService: - from policyengine_api.data.v2.database import get_v2_session_factory - from policyengine_api.libs.auth0_management import build_auth0_user_deleter - from policyengine_api.services.v2.user_accounts.database_session import ( + from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + get_v2_session_factory, + ) + from policyengine_api.libs.auth0_management import ( # noqa: PLC0415 # Auth0 Management loads only when a default factory runs + build_auth0_user_deleter, + ) + from policyengine_api.services.v2.user_accounts.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs AccountDatabaseSession, ) - from policyengine_api.services.v2.user_accounts.services import ( + from policyengine_api.services.v2.user_accounts.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs V2AccountService, ) diff --git a/policyengine_api/libs/auth0_management.py b/policyengine_api/libs/auth0_management.py index 1db5aea88..224915b97 100644 --- a/policyengine_api/libs/auth0_management.py +++ b/policyengine_api/libs/auth0_management.py @@ -112,7 +112,9 @@ def load_auth0_management_settings( @cache def _load_secret_from_secret_manager(resource_name: str) -> str: - from google.cloud import secretmanager + from google.cloud import ( # noqa: PLC0415 # the Google client loads only when Auth0 Management reads a secret + secretmanager, + ) client = secretmanager.SecretManagerServiceClient() response = client.access_secret_version(request={"name": resource_name}) diff --git a/policyengine_api/readiness.py b/policyengine_api/readiness.py index 69bae7f08..e0bab4bed 100644 --- a/policyengine_api/readiness.py +++ b/policyengine_api/readiness.py @@ -19,10 +19,12 @@ def validate_policy_runtime_configuration() -> None: from policyengine_api.data.v2.settings import ( # noqa: PLC0415 # keeps importing readiness cheap; only validation needs configuration load_v2_runtime_database_settings, ) - from policyengine_api.libs.auth0_management import ( + from policyengine_api.libs.auth0_management import ( # noqa: PLC0415 # keeps importing readiness cheap; only validation needs configuration load_auth0_management_settings, ) - from policyengine_api.libs.user_auth import load_user_auth_settings + from policyengine_api.libs.user_auth import ( # noqa: PLC0415 # keeps importing readiness cheap; only validation needs configuration + load_user_auth_settings, + ) from policyengine_api.migration_flags import ( # noqa: PLC0415 # keeps importing readiness cheap; only validation needs configuration get_v1_household_read_source, get_v1_household_write_source, From 767d68b30f6c89ecfe426b78ed2320436e1cad31 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 01:27:35 +0400 Subject: [PATCH 13/15] Wrap the long lines #3891 added (E501) The user-service proof fixture's docstring puts the rule's verdict on its own line, as the validator fixtures do. The import side-effect test's subprocess script asserts the association qualification module is unloaded across three lines, and the identity persistence test's qualification test names the qualification module once, as qualification, so its pending checker fits on one line. The checks are unchanged. Co-Authored-By: Claude Opus 5.5 --- .../api.v2.user-services-no-sql/good_services.py | 5 ++++- .../test_v2_user_identity_persistence.py | 15 ++++++--------- tests/unit/v2/test_import_side_effects.py | 5 ++++- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/rules/fixtures/api.v2.user-services-no-sql/good_services.py b/rules/fixtures/api.v2.user-services-no-sql/good_services.py index c09a0099e..ac47e42a9 100644 --- a/rules/fixtures/api.v2.user-services-no-sql/good_services.py +++ b/rules/fixtures/api.v2.user-services-no-sql/good_services.py @@ -1,4 +1,7 @@ -"""Proof fixture: a user service that leaves queries to its connector. The rule must accept it.""" +"""Proof fixture: a user service that leaves queries to its connector. + +The rule must accept it. +""" from sqlmodel import Session diff --git a/tests/integration/test_v2_user_identity_persistence.py b/tests/integration/test_v2_user_identity_persistence.py index e0f0f694d..b8658b9ae 100644 --- a/tests/integration/test_v2_user_identity_persistence.py +++ b/tests/integration/test_v2_user_identity_persistence.py @@ -710,20 +710,17 @@ def test_qualification_reads_counts_in_a_read_only_transaction(sessions) -> None V2_SUPABASE_PROJECT_REF: "abcdefghijklmnopqrst", V2_SUPABASE_ENVIRONMENT: "test", } + qualification = user_association_migration_qualification observed_read_only: list[object] = [] def pending_checker(connection) -> bool: observed_read_only.append(connection.connection.dbapi_connection.read_only) - return user_association_migration_qualification.user_association_migration_is_pending( - connection - ) + return qualification.user_association_migration_is_pending(connection) - evidence = ( - user_association_migration_qualification.qualify_user_association_migration( - environ, - engine_builder=lambda _settings: create_engine(disposable_url), - pending_checker=pending_checker, - ) + evidence = qualification.qualify_user_association_migration( + environ, + engine_builder=lambda _settings: create_engine(disposable_url), + pending_checker=pending_checker, ) assert observed_read_only == [True] diff --git a/tests/unit/v2/test_import_side_effects.py b/tests/unit/v2/test_import_side_effects.py index 056446085..7b753004d 100644 --- a/tests/unit/v2/test_import_side_effects.py +++ b/tests/unit/v2/test_import_side_effects.py @@ -73,7 +73,10 @@ def reject_ddl(*args, **kwargs): assert "policyengine_api.data.v2.catalog.initialization" not in sys.modules assert "policyengine_api.data.v2.policy_migration_qualification" not in sys.modules assert "policyengine_api.data.v2.household_migration_qualification" not in sys.modules -assert "policyengine_api.data.v2.user_association_migration_qualification" not in sys.modules +assert ( + "policyengine_api.data.v2.user_association_migration_qualification" + not in sys.modules +) """ result = subprocess.run( From b4f180331bbc45638d0a522838e1897bf1854f8b Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 08:35:13 +0400 Subject: [PATCH 14/15] Document why the user route modules evaluate annotations eagerly The typing-only-imports skill now says why the five signed-in user route modules don't use `from __future__ import annotations`: their handlers depend on the authenticator each router is given, which FastAPI cannot resolve from a string annotation against module globals. Co-Authored-By: Claude Opus 5.5 --- docs/engineering/skills/typing-only-imports.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/engineering/skills/typing-only-imports.md b/docs/engineering/skills/typing-only-imports.md index b442abf5b..89675adc7 100644 --- a/docs/engineering/skills/typing-only-imports.md +++ b/docs/engineering/skills/typing-only-imports.md @@ -36,6 +36,21 @@ where `package/__init__.py` re-exports it) does not match its listed definition path. Ruff does not follow inheritance across modules, so every package class that models in other modules inherit from is listed. +### Route modules with per-router dependencies + +FastAPI resolves an endpoint's string annotations against its module's globals +only. A route that declares a dependency with `Annotated` therefore needs every +name in that annotation at module level. Query dependencies are module-level +aliases, such as `CountryQueryDependency`. The signed-in user routes also depend +on `require_user`, the authenticator `NativeRouteDependencies` injects into each +router, which exists only inside the router builder. Their modules +(`fastapi_routes/v2/me`, `user_households`, `user_policies`, `user_reports` and +`user_simulations`) don't use `from __future__ import annotations`, so +`Annotated[AccountPrincipal, Depends(require_user)]` is evaluated where the +handler is defined, and the names their other annotations use are imported at +runtime. With the future import, FastAPI would silently treat `principal` as a +required query parameter. + ## Adding a model base When you add a model class that models in other modules inherit from, add the From a8c5058c5cc63a8eb24e1a72bc65bd5d833947f9 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 10:38:40 +0400 Subject: [PATCH 15/15] List the lazily loaded modules for ruff instead of suppressing each import Thirteen packages and modules are only ever imported inside the function that needs them: - policyengine and spm_calculator, which load country models; - the v2 database module, which registers an exit hook at import; - the nine v2 database_session modules, which open persistence only when a default factory runs; - Auth0 Management, which loads the Google client. No application module imports any of them at module level. pyproject.toml now lists them in banned-module-level-imports and selects TID253. Importing one at module level fails (TYPE_CHECKING imports are still fine), and PLC0415 accepts their in-function imports without a suppression. That removes 28 PLC0415 suppressions. Tests and CI/CD scripts may still import them at module level: TID253 joins their file-level exceptions, since find-api-model-versions.py imports policyengine. Modules that some application module does import at module level keep their inline reasons. These include google.cloud, the v2 services modules, constants and country. Comments and configuration only: no code changes. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 6 +-- changelog.d/ruff-structure-imports.changed.md | 2 +- docs/generated/rules.md | 24 ++++++------ .../data/v2/catalog/extraction.py | 4 +- .../fastapi_routes/dependencies.py | 36 ++++++++--------- policyengine_api/readiness.py | 2 +- .../services/household_mirroring.py | 4 +- policyengine_api/services/policy_mirroring.py | 4 +- .../services/user_policy_mirroring.py | 4 +- policyengine_api/spm.py | 2 +- pyproject.toml | 39 ++++++++++++++++--- rules/registry.toml | 10 ++--- 12 files changed, 82 insertions(+), 55 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 940c1e774..7cf1033eb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -120,16 +120,16 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. ### Code quality and types - `api.quality.alembic-revisions-unused-imports`: Ruff doesn't lint Alembic revision files (lint.exclude in pyproject.toml), because they must stay exactly as Alembic generates them. (advisory: nothing, not blocking) -- `api.quality.ci-cd-scripts-ruff-exemptions`: CI/CD scripts under .github may print their results and fetch fixed URLs (T201, S310); every other ruff rule applies to them. (blocking: ruff per-file ignore of S310, T201 for `.github/**`) +- `api.quality.ci-cd-scripts-ruff-exemptions`: CI/CD scripts under .github may print their results, fetch fixed URLs and import the lazily loaded packages at module level (T201, S310, TID253); every other ruff rule applies to them. (blocking: ruff per-file ignore of S310, T201, TID253 for `.github/**`) - `api.quality.command-line-modules-ruff-exemptions`: Application modules that also run as command-line programs (if __name__ == "__main__") may print their output (T201); every other ruff rule applies to them. (blocking: ruff per-file ignore of T201 for `policyengine_api/{data/v2/catalog/initialization,data/v2/household_migration_qualification,data/v2/policy_migration_qualification,data/v2/user_association_migration_qualification,worker_spm_release}.py`) - `api.quality.command-line-scripts-ruff-exemptions`: Command-line scripts under scripts/ may print their output, run other programs, fetch fixed URLs, stamp today's date, pick random samples and ignore a failed best-effort step (T201, S603, S607, S310, DTZ011, S311, S110); every other ruff rule applies to them. (blocking: ruff per-file ignore of DTZ011, S110, S310, S311, S603, S607, T201 for `scripts/**`) - `api.quality.environment-variables-load`: Required environment variables and credentials load in a deployed-like environment. (blocking: CI job `test_env_vars`) - `api.quality.mypy-v2`: Shared query models, FastAPI dependencies, v2 routes and services, and the user sign-in and account-deletion libraries pass mypy with untyped definitions disallowed. (blocking: mypy) - `api.quality.ruff-format`: All Python files are formatted with ruff format. (blocking: ruff format) -- `api.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in pyproject.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)) +- `api.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in pyproject.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, TID253, TRY, UP, W)) - `api.quality.runtime-evaluated-models-resolve`: Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. (blocking: pytest `tests/unit/test_runtime_evaluated_annotations.py`) - `api.quality.tests-pass`: The unit, contract and to-refactor test suites pass. (blocking: CI job `test`) -- `api.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). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201). Every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, DTZ001, DTZ011, INP001, PLC0415, PLR2004, S101, S105, S106, S603, S607, S608, SLF001, T201 for `**/tests/**`) +- `api.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). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201), and may import the lazily loaded packages at module level (TID253). Every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, DTZ001, DTZ011, INP001, PLC0415, PLR2004, S101, S105, S106, S603, S607, S608, SLF001, T201, TID253 for `**/tests/**`) ### Observability diff --git a/changelog.d/ruff-structure-imports.changed.md b/changelog.d/ruff-structure-imports.changed.md index 930b209df..dff021521 100644 --- a/changelog.d/ruff-structure-imports.changed.md +++ b/changelog.d/ruff-structure-imports.changed.md @@ -1 +1 @@ -Enabled ruff's structure and import rules (INP, PLC, SLF, N, A, FAST, E): long lines are wrapped, FastAPI dependencies are declared with Annotated, deferred imports that avoid heavy loads, cycles or import-time I/O carry a reason, and directories without an __init__.py on purpose are declared to ruff as namespace packages; tests are also exempt from SLF001, INP001 and PLC0415. Published schemas and the HTTP API are unchanged. +Enabled ruff's structure and import rules (INP, PLC, SLF, N, A, FAST, E): long lines are wrapped, FastAPI dependencies are declared with Annotated, deferred imports that avoid heavy loads, cycles or import-time I/O carry a reason unless the module is in ruff's banned-module-level-imports allowlist of lazily loaded modules (policyengine, spm_calculator, the v2 database and session modules and Auth0 Management, which TID253 keeps out of module-level imports), and directories without an __init__.py on purpose are declared to ruff as namespace packages; tests are also exempt from SLF001, INP001 and PLC0415. Published schemas and the HTTP API are unchanged. diff --git a/docs/generated/rules.md b/docs/generated/rules.md index b6253bab4..849cff093 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -269,23 +269,23 @@ flowchart LR | Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | | --- | --- | --- | --- | --- | --- | --- | --- | | `api.quality.alembic-revisions-unused-imports` | Ruff doesn't lint Alembic revision files (lint.exclude in pyproject.toml), because they must stay exactly as Alembic generates them. | mechanical | advisory | no | nothing | — | — | -| `api.quality.ci-cd-scripts-ruff-exemptions` | CI/CD scripts under .github may print their results and fetch fixed URLs (T201, S310); every other ruff rule applies to them. | mechanical | enforced | yes | ruff per-file ignore of S310, T201 for `.github/**` | `pr.yml:lint` | — | +| `api.quality.ci-cd-scripts-ruff-exemptions` | CI/CD scripts under .github may print their results, fetch fixed URLs and import the lazily loaded packages at module level (T201, S310, TID253); every other ruff rule applies to them. | mechanical | enforced | yes | ruff per-file ignore of S310, T201, TID253 for `.github/**` | `pr.yml:lint` | — | | `api.quality.command-line-modules-ruff-exemptions` | Application modules that also run as command-line programs (if __name__ == "__main__") may print their output (T201); every other ruff rule applies to them. | mechanical | enforced | yes | ruff per-file ignore of T201 for `policyengine_api/{data/v2/catalog/initialization,data/v2/household_migration_qualification,data/v2/policy_migration_qualification,data/v2/user_association_migration_qualification,worker_spm_release}.py` | `pr.yml:lint` | — | | `api.quality.command-line-scripts-ruff-exemptions` | Command-line scripts under scripts/ may print their output, run other programs, fetch fixed URLs, stamp today's date, pick random samples and ignore a failed best-effort step (T201, S603, S607, S310, DTZ011, S311, S110); every other ruff rule applies to them. | mechanical | enforced | yes | ruff per-file ignore of DTZ011, S110, S310, S311, S603, S607, T201 for `scripts/**` | `pr.yml:lint` | — | | `api.quality.environment-variables-load` | Required environment variables and credentials load in a deployed-like environment. | mechanical | enforced | yes | CI job `test_env_vars` | `pr.yml:test_env_vars` | — | | `api.quality.mypy-v2` | Shared query models, FastAPI dependencies, v2 routes and services, and the user sign-in and account-deletion libraries pass mypy with untyped definitions disallowed. | mechanical | enforced | yes | mypy | `pr.yml:typecheck-v2` | — | | `api.quality.ruff-format` | All Python files are formatted with ruff format. | mechanical | enforced | yes | ruff format | `pr.yml:lint` | — | -| `api.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in pyproject.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` | — | +| `api.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in pyproject.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, TID253, TRY, UP, W) | `pr.yml:lint` | — | | `api.quality.runtime-evaluated-models-resolve` | Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. | mechanical | enforced | yes | pytest `tests/unit/test_runtime_evaluated_annotations.py` | `pr.yml:test` | **unproven** | | `api.quality.tests-pass` | The unit, contract and to-refactor test suites pass. | mechanical | enforced | yes | CI job `test` | `pr.yml:test` | — | -| `api.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). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201). Every other ruff rule applies to tests. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, DTZ001, DTZ011, INP001, PLC0415, PLR2004, S101, S105, S106, S603, S607, S608, SLF001, T201 for `**/tests/**` | `pr.yml:lint` | — | +| `api.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). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201), and may import the lazily loaded packages at module level (TID253). Every other ruff rule applies to tests. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, DTZ001, DTZ011, INP001, PLC0415, PLR2004, S101, S105, S106, S603, S607, S608, SLF001, T201, TID253 for `**/tests/**` | `pr.yml:lint` | — | ```mermaid flowchart LR r_api_quality_alembic_revisions_unused_imports("api.quality.alembic-revisions-unused-imports (advisory)") gap_unenforced>"not enforced"] r_api_quality_ci_cd_scripts_ruff_exemptions("api.quality.ci-cd-scripts-ruff-exemptions (enforced)") - e_ruff_per_file_ignore_of_S310__T201_for__github___["ruff per-file ignore of S310, T201 for .github/**"] + e_ruff_per_file_ignore_of_S310__T201__TID253_for__github___["ruff per-file ignore of S310, T201, TID253 for .github/**"] j_CI_pr_yml_lint["CI pr.yml:lint"] r_api_quality_command_line_modules_ruff_exemptions("api.quality.command-line-modules-ruff-exemptions (enforced)") e_ruff_per_file_ignore_of_T201_for_policyengine_api__data_v2_catalog_initialization_data_v2_household_migration_qualification_data_v2_policy_migration_qualification_data_v2_user_association_migration_qualification_worker_spm_release__py["ruff per-file ignore of T201 for policyengine_api/{data/v2/catalog/initialization,data/v2/household_migration_qualification,data/v2/policy_migration_qualification,data/v2/user_association_migration_qualification,worker_spm_release}.py"] @@ -300,17 +300,17 @@ flowchart LR r_api_quality_ruff_format("api.quality.ruff-format (enforced)") e_ruff_format["ruff format"] r_api_quality_ruff_lint("api.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__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__TID253__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, TID253, TRY, UP, W)"] r_api_quality_runtime_evaluated_models_resolve("api.quality.runtime-evaluated-models-resolve (enforced)") e_pytest_tests_unit_test_runtime_evaluated_annotations_py["pytest tests/unit/test_runtime_evaluated_annotations.py"] j_CI_pr_yml_test["CI pr.yml:test"] r_api_quality_tests_pass("api.quality.tests-pass (enforced)") e_CI_job_test["CI job test"] r_api_quality_tests_ruff_exemptions("api.quality.tests-ruff-exemptions (enforced)") - e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__INP001__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201_for____tests___["ruff per-file ignore of ANN, D1, DTZ001, DTZ011, INP001, PLC0415, PLR2004, S101, S105, S106, S603, S607, S608, SLF001, T201 for **/tests/**"] + e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__INP001__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201__TID253_for____tests___["ruff per-file ignore of ANN, D1, DTZ001, DTZ011, INP001, PLC0415, PLR2004, S101, S105, S106, S603, S607, S608, SLF001, T201, TID253 for **/tests/**"] r_api_quality_alembic_revisions_unused_imports -.-> gap_unenforced - r_api_quality_ci_cd_scripts_ruff_exemptions --> e_ruff_per_file_ignore_of_S310__T201_for__github___ - e_ruff_per_file_ignore_of_S310__T201_for__github___ --> j_CI_pr_yml_lint + r_api_quality_ci_cd_scripts_ruff_exemptions --> e_ruff_per_file_ignore_of_S310__T201__TID253_for__github___ + e_ruff_per_file_ignore_of_S310__T201__TID253_for__github___ --> j_CI_pr_yml_lint r_api_quality_command_line_modules_ruff_exemptions --> e_ruff_per_file_ignore_of_T201_for_policyengine_api__data_v2_catalog_initialization_data_v2_household_migration_qualification_data_v2_policy_migration_qualification_data_v2_user_association_migration_qualification_worker_spm_release__py e_ruff_per_file_ignore_of_T201_for_policyengine_api__data_v2_catalog_initialization_data_v2_household_migration_qualification_data_v2_policy_migration_qualification_data_v2_user_association_migration_qualification_worker_spm_release__py --> j_CI_pr_yml_lint r_api_quality_command_line_scripts_ruff_exemptions --> e_ruff_per_file_ignore_of_DTZ011__S110__S310__S311__S603__S607__T201_for_scripts___ @@ -321,14 +321,14 @@ flowchart LR e_mypy --> j_CI_pr_yml_typecheck_v2 r_api_quality_ruff_format --> e_ruff_format e_ruff_format --> j_CI_pr_yml_lint - r_api_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_api_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__TID253__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__TID253__TRY__UP__W_ --> j_CI_pr_yml_lint r_api_quality_runtime_evaluated_models_resolve --> e_pytest_tests_unit_test_runtime_evaluated_annotations_py e_pytest_tests_unit_test_runtime_evaluated_annotations_py --> j_CI_pr_yml_test r_api_quality_tests_pass --> e_CI_job_test e_CI_job_test --> j_CI_pr_yml_test - r_api_quality_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__INP001__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201_for____tests___ - e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__INP001__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201_for____tests___ --> j_CI_pr_yml_lint + r_api_quality_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__INP001__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201__TID253_for____tests___ + e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__INP001__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201__TID253_for____tests___ --> j_CI_pr_yml_lint ``` ## Observability diff --git a/policyengine_api/data/v2/catalog/extraction.py b/policyengine_api/data/v2/catalog/extraction.py index 571c40c85..1a09f2abb 100644 --- a/policyengine_api/data/v2/catalog/extraction.py +++ b/policyengine_api/data/v2/catalog/extraction.py @@ -745,8 +745,8 @@ def extract_catalog( def extract_installed_catalog() -> NormalizedCatalog: """Load and normalize the installed PolicyEngine.py certified catalog.""" - import policyengine # noqa: PLC0415 # policyengine loads every country model; only catalog extraction needs it - from policyengine.bundle import ( # noqa: PLC0415 # policyengine loads every country model; only catalog extraction needs it + import policyengine + from policyengine.bundle import ( get_current_bundle, ) diff --git a/policyengine_api/fastapi_routes/dependencies.py b/policyengine_api/fastapi_routes/dependencies.py index 81a1e6154..99dc73cc5 100644 --- a/policyengine_api/fastapi_routes/dependencies.py +++ b/policyengine_api/fastapi_routes/dependencies.py @@ -730,10 +730,10 @@ def _running_policyengine_version() -> str: def _default_v2_metadata_reader_factory() -> V2MetadataResourceReader: - from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + from policyengine_api.data.v2.database import ( get_v2_session_factory, ) - from policyengine_api.services.v2.metadata.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs + from policyengine_api.services.v2.metadata.database_session import ( MetadataDatabaseSession, ) from policyengine_api.services.v2.metadata.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs @@ -747,10 +747,10 @@ def _default_v2_metadata_reader_factory() -> V2MetadataResourceReader: def _default_v2_policy_service_factory() -> V2PolicyResourceService: - from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + from policyengine_api.data.v2.database import ( get_v2_session_factory, ) - from policyengine_api.services.v2.policies.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs + from policyengine_api.services.v2.policies.database_session import ( PolicyDatabaseSession, ) from policyengine_api.services.v2.policies.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs @@ -764,10 +764,10 @@ def _default_v2_policy_service_factory() -> V2PolicyResourceService: def _default_v2_household_service_factory() -> V2HouseholdResourceService: - from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + from policyengine_api.data.v2.database import ( get_v2_session_factory, ) - from policyengine_api.services.v2.households.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs + from policyengine_api.services.v2.households.database_session import ( HouseholdDatabaseSession, ) from policyengine_api.services.v2.households.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs @@ -811,7 +811,7 @@ def _build_v1_legacy_reference_service() -> LegacyReferenceChecker: from policyengine_api.data.orm import ( # noqa: PLC0415 # looked up per call, so tests can replace the v1 session factory get_v1_session_factory, ) - from policyengine_api.services.v2.legacy_refs.database_session import ( # noqa: PLC0415 # v1 reference checks load only when a request checks one + from policyengine_api.services.v2.legacy_refs.database_session import ( LegacyReferenceDatabaseSession, ) from policyengine_api.services.v2.legacy_refs.services import ( # noqa: PLC0415 # v1 reference checks load only when a request checks one @@ -833,10 +833,10 @@ def _default_legacy_reference_checker() -> LegacyReferenceChecker: def _default_v2_user_policy_service_factory() -> V2UserPolicyResourceService: - from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + from policyengine_api.data.v2.database import ( get_v2_session_factory, ) - from policyengine_api.services.v2.user_policies.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs + from policyengine_api.services.v2.user_policies.database_session import ( UserPolicyDatabaseSession, ) from policyengine_api.services.v2.user_policies.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs @@ -850,10 +850,10 @@ def _default_v2_user_policy_service_factory() -> V2UserPolicyResourceService: def _default_v2_user_household_service_factory() -> V2UserHouseholdResourceService: - from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + from policyengine_api.data.v2.database import ( get_v2_session_factory, ) - from policyengine_api.services.v2.user_households.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs + from policyengine_api.services.v2.user_households.database_session import ( UserHouseholdDatabaseSession, ) from policyengine_api.services.v2.user_households.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs @@ -867,10 +867,10 @@ def _default_v2_user_household_service_factory() -> V2UserHouseholdResourceServi def _default_v2_user_simulation_service_factory() -> V2UserSimulationResourceService: - from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + from policyengine_api.data.v2.database import ( get_v2_session_factory, ) - from policyengine_api.services.v2.user_simulations.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs + from policyengine_api.services.v2.user_simulations.database_session import ( UserSimulationDatabaseSession, ) from policyengine_api.services.v2.user_simulations.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs @@ -884,10 +884,10 @@ def _default_v2_user_simulation_service_factory() -> V2UserSimulationResourceSer def _default_v2_user_report_service_factory() -> V2UserReportResourceService: - from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + from policyengine_api.data.v2.database import ( get_v2_session_factory, ) - from policyengine_api.services.v2.user_reports.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs + from policyengine_api.services.v2.user_reports.database_session import ( UserReportDatabaseSession, ) from policyengine_api.services.v2.user_reports.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs @@ -901,13 +901,13 @@ def _default_v2_user_report_service_factory() -> V2UserReportResourceService: def _default_v2_account_service_factory() -> V2AccountResourceService: - from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + from policyengine_api.data.v2.database import ( get_v2_session_factory, ) - from policyengine_api.libs.auth0_management import ( # noqa: PLC0415 # Auth0 Management loads only when a default factory runs + from policyengine_api.libs.auth0_management import ( build_auth0_user_deleter, ) - from policyengine_api.services.v2.user_accounts.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs + from policyengine_api.services.v2.user_accounts.database_session import ( AccountDatabaseSession, ) from policyengine_api.services.v2.user_accounts.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs diff --git a/policyengine_api/readiness.py b/policyengine_api/readiness.py index e0bab4bed..f94255c82 100644 --- a/policyengine_api/readiness.py +++ b/policyengine_api/readiness.py @@ -19,7 +19,7 @@ def validate_policy_runtime_configuration() -> None: from policyengine_api.data.v2.settings import ( # noqa: PLC0415 # keeps importing readiness cheap; only validation needs configuration load_v2_runtime_database_settings, ) - from policyengine_api.libs.auth0_management import ( # noqa: PLC0415 # keeps importing readiness cheap; only validation needs configuration + from policyengine_api.libs.auth0_management import ( load_auth0_management_settings, ) from policyengine_api.libs.user_auth import ( # noqa: PLC0415 # keeps importing readiness cheap; only validation needs configuration diff --git a/policyengine_api/services/household_mirroring.py b/policyengine_api/services/household_mirroring.py index 26bd073ce..189401cb5 100644 --- a/policyengine_api/services/household_mirroring.py +++ b/policyengine_api/services/household_mirroring.py @@ -46,10 +46,10 @@ class HouseholdMirrorUnavailableError(RuntimeError): def _default_mirror_factory() -> LegacyHouseholdMirror: - from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + from policyengine_api.data.v2.database import ( get_v2_session_factory, ) - from policyengine_api.services.v2.households.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs + from policyengine_api.services.v2.households.database_session import ( HouseholdDatabaseSession, ) from policyengine_api.services.v2.households.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs diff --git a/policyengine_api/services/policy_mirroring.py b/policyengine_api/services/policy_mirroring.py index 7757baeb5..674d9c75b 100644 --- a/policyengine_api/services/policy_mirroring.py +++ b/policyengine_api/services/policy_mirroring.py @@ -46,10 +46,10 @@ class PolicyMirrorUnavailableError(RuntimeError): def _default_mirror_factory() -> LegacyPolicyMirror: - from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + from policyengine_api.data.v2.database import ( get_v2_session_factory, ) - from policyengine_api.services.v2.policies.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs + from policyengine_api.services.v2.policies.database_session import ( PolicyDatabaseSession, ) from policyengine_api.services.v2.policies.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs diff --git a/policyengine_api/services/user_policy_mirroring.py b/policyengine_api/services/user_policy_mirroring.py index 3f92f9152..08f7a7c62 100644 --- a/policyengine_api/services/user_policy_mirroring.py +++ b/policyengine_api/services/user_policy_mirroring.py @@ -59,10 +59,10 @@ class UserPolicyMirrorUnavailableError(RuntimeError): def _default_mirror_factory() -> LegacyUserPolicyMirror: - from policyengine_api.data.v2.database import ( # noqa: PLC0415 # registers an atexit hook at import; v2 persistence loads only when needed + from policyengine_api.data.v2.database import ( get_v2_session_factory, ) - from policyengine_api.services.v2.user_policies.database_session import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs + from policyengine_api.services.v2.user_policies.database_session import ( UserPolicyDatabaseSession, ) from policyengine_api.services.v2.user_policies.services import ( # noqa: PLC0415 # v2 persistence loads only when a default factory runs diff --git a/policyengine_api/spm.py b/policyengine_api/spm.py index e6e4bedb1..932149b93 100644 --- a/policyengine_api/spm.py +++ b/policyengine_api/spm.py @@ -501,7 +501,7 @@ def _installed_country_implements_spm(country_id: str) -> bool: @lru_cache(maxsize=4) def _selected_forecast(expected_sha256: str) -> SPMForecast: - from spm_calculator.rolling_forecast import ( # noqa: PLC0415 # spm_calculator loads only when a forecast is selected + from spm_calculator.rolling_forecast import ( load_forecast, ) diff --git a/pyproject.toml b/pyproject.toml index 595e2aa13..7e014260b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -117,7 +117,7 @@ select = [ # Group 6: security, logging and debugging leftovers. "DTZ", "ERA", "EXE", "FIX", "G", "LOG", "S", "T20", "TD", # Group 7: structure and imports. - "A", "E", "FAST", "INP", "N", "PLC", "SLF", + "A", "E", "FAST", "INP", "N", "PLC", "SLF", "TID253", ] # Mutually exclusive pydocstyle pairs: keep D211 (no blank line before a class # docstring) and D212 (summary on the first line). @@ -136,14 +136,16 @@ exclude = ["migrations/*/versions/*.py"] # named constants for magic numbers. They may also reach into private members, # live in directories that aren't packages, import inside a test (often after # patching the environment), and use fake credentials, naive datetimes, -# subprocess calls to repository scripts, SQL built from strings and prints. -# Every other rule applies to tests. +# subprocess calls to repository scripts, SQL built from strings and prints, +# and may import the lazily loaded packages at module level. Every other rule +# applies to tests. "**/tests/**" = [ "ANN", "D1", "DTZ001", "DTZ011", "INP001", "PLC0415", "PLR2004", "S101", - "S105", "S106", "S603", "S607", "S608", "SLF001", "T201", + "S105", "S106", "S603", "S607", "S608", "SLF001", "T201", "TID253", ] -# CI/CD scripts print their results and fetch fixed URLs. -".github/**" = ["S310", "T201"] +# CI/CD scripts print their results, fetch fixed URLs and may import the +# lazily loaded packages at module level. +".github/**" = ["S310", "T201", "TID253"] # Command-line scripts print their output, run other programs, fetch fixed # URLs, stamp today's date and pick random samples. "scripts/**" = [ @@ -202,6 +204,31 @@ showcontent = true directory = "removed" name = "Removed" showcontent = true +[tool.ruff.lint.flake8-tidy-imports] +# Packages and modules that load only inside the function that needs them, +# never at module level (TID253 enforces it; TYPE_CHECKING imports are fine): +# policyengine and spm_calculator load country models, the v2 database module +# registers an exit hook at import, the v2 database sessions open persistence +# only when a default factory runs, and Auth0 Management loads the Google +# client. Ruff's PLC0415 accepts in-function imports of these without a +# suppression. A module belongs here only if no application module imports it +# at module level. +banned-module-level-imports = [ + "policyengine", + "policyengine_api.data.v2.database", + "policyengine_api.libs.auth0_management", + "policyengine_api.services.v2.households.database_session", + "policyengine_api.services.v2.legacy_refs.database_session", + "policyengine_api.services.v2.metadata.database_session", + "policyengine_api.services.v2.policies.database_session", + "policyengine_api.services.v2.user_accounts.database_session", + "policyengine_api.services.v2.user_households.database_session", + "policyengine_api.services.v2.user_policies.database_session", + "policyengine_api.services.v2.user_reports.database_session", + "policyengine_api.services.v2.user_simulations.database_session", + "spm_calculator", +] + [tool.ruff.lint.flake8-type-checking] # Classes whose annotations Pydantic, SQLModel or SQLAlchemy evaluate at runtime, so their # imports must stay at runtime. Ruff checks direct bases only, so every diff --git a/rules/registry.toml b/rules/registry.toml index 4f32709cc..33bf32d48 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -700,7 +700,7 @@ autofix = true [rule.enforcer] kind = "ruff" mode = "lint" -codes = ["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"] +codes = ["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", "TID253", "TRY", "UP", "W"] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "lint" @@ -744,7 +744,7 @@ kind = "none" [[rule]] id = "api.quality.tests-ruff-exemptions" category = "quality" -statement = "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). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201). Every other ruff rule applies to tests." +statement = "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). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201), and may import the lazily loaded packages at module level (TID253). Every other ruff rule applies to tests." rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" scope = ["tests/**/*.py", "scripts/rules/tests/**/*.py"] tier = "mechanical" @@ -754,7 +754,7 @@ autofix = false kind = "ruff" mode = "per-file-ignore" pattern = "**/tests/**" -codes = ["ANN", "D1", "DTZ001", "DTZ011", "INP001", "PLC0415", "PLR2004", "S101", "S105", "S106", "S603", "S607", "S608", "SLF001", "T201"] +codes = ["ANN", "D1", "DTZ001", "DTZ011", "INP001", "PLC0415", "PLR2004", "S101", "S105", "S106", "S603", "S607", "S608", "SLF001", "T201", "TID253"] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "lint" @@ -763,7 +763,7 @@ invocation = "ruff check ." [[rule]] id = "api.quality.ci-cd-scripts-ruff-exemptions" category = "quality" -statement = "CI/CD scripts under .github may print their results and fetch fixed URLs (T201, S310); every other ruff rule applies to them." +statement = "CI/CD scripts under .github may print their results, fetch fixed URLs and import the lazily loaded packages at module level (T201, S310, TID253); every other ruff rule applies to them." rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" scope = [".github/**/*.py"] tier = "mechanical" @@ -773,7 +773,7 @@ autofix = false kind = "ruff" mode = "per-file-ignore" pattern = ".github/**" -codes = ["S310", "T201"] +codes = ["S310", "T201", "TID253"] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "lint"