From 2dad66f1cd9de664b6907143bdbebbf45e375886 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:30:24 +0400 Subject: [PATCH 01/16] Fix ruff LOG and G findings Pass logging arguments instead of f-strings (G004); each message renders to the same text. Name the economy-comparison router's logger after its module rather than its file path (LOG002). Log the gateway's auth misconfiguration with logger.exception and a fixed message (G201): the traceback already ends with the exception and its message, which the old call also repeated in the message. Co-Authored-By: Claude Opus 5.5 --- .../src/policyengine_fastapi/auth/jwt_decoder.py | 6 +++--- .../src/policyengine_fastapi/opentelemetry/gcp.py | 3 ++- .../src/policyengine_simulation_executor/simulation.py | 2 +- .../policyengine_simulation_executor/simulation_runtime.py | 4 ++-- .../src/policyengine_simulation_gateway/auth.py | 4 ++-- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/libs/policyengine-fastapi/src/policyengine_fastapi/auth/jwt_decoder.py b/libs/policyengine-fastapi/src/policyengine_fastapi/auth/jwt_decoder.py index cdc6ce37c..a1356412c 100644 --- a/libs/policyengine-fastapi/src/policyengine_fastapi/auth/jwt_decoder.py +++ b/libs/policyengine-fastapi/src/policyengine_fastapi/auth/jwt_decoder.py @@ -63,10 +63,10 @@ def _validate(self, token: HTTPAuthorizationCredentials | None) -> dict[str, str token.credentials ).key except jwt.exceptions.PyJWKClientError as error: - LOG.error(f"Unable to get public key for JWT validation: {error}") # noqa: TRY400 # callers control the token, so a traceback per bad token would let them flood the logs + LOG.error("Unable to get public key for JWT validation: %s", error) # noqa: TRY400 # callers control the token, so a traceback per bad token would let them flood the logs raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) except jwt.exceptions.DecodeError as error: - LOG.error(f"Unable to get public key for JWT validation: {error}") # noqa: TRY400 # callers control the token, so a traceback per bad token would let them flood the logs + LOG.error("Unable to get public key for JWT validation: %s", error) # noqa: TRY400 # callers control the token, so a traceback per bad token would let them flood the logs raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) try: @@ -78,6 +78,6 @@ def _validate(self, token: HTTPAuthorizationCredentials | None) -> dict[str, str issuer=self.issuer, ) except Exception as error: # noqa: BLE001 # any decode or claim-validation failure must become a 403, never a 500 - LOG.info(f"Invalid bearer token: {error}") + LOG.info("Invalid bearer token: %s", error) raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) return payload diff --git a/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py b/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py index d2312e5a3..b96ad4f4d 100644 --- a/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py +++ b/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py @@ -58,7 +58,8 @@ def _get_project_id_from_metadata() -> str | None: return request.urlopen(req).read().decode() except Exception as err: # noqa: BLE001 # best-effort probe: any failure falls back to GOOGLE_CLOUD_PROJECT log.info( - f"Unable to get the google project id from a local metadata service: {err}" + "Unable to get the google project id from a local metadata service: %s", + err, ) return None diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation.py index 57203d8fb..d42e5da1c 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation.py @@ -13,7 +13,7 @@ ) from policyengine_simulation_executor.simulation_runtime import run_simulation_impl -logger = logging.getLogger(__file__) +logger = logging.getLogger(__name__) def create_router(runtime: ObservabilityRuntime) -> APIRouter: diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_runtime.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_runtime.py index 001bfd0ba..8bec8c60f 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_runtime.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_runtime.py @@ -130,7 +130,7 @@ def setup_gcp_credentials( for k, v in os.environ.items() if "GOOGLE" in k or "GCP" in k or "CREDENTIAL" in k } - logger.info(f"GCP-related env vars: {list(gcp_vars.keys())}") + logger.info("GCP-related env vars: %s", list(gcp_vars.keys())) if previous: logger.info("GOOGLE_APPLICATION_CREDENTIALS already set") @@ -158,7 +158,7 @@ def setup_gcp_credentials( creds_file.write(normalized) creds_file.flush() os.environ["GOOGLE_APPLICATION_CREDENTIALS"] = creds_file.name - logger.info(f"GCP credentials written to {creds_file.name}") + logger.info("GCP credentials written to %s", creds_file.name) yield finally: diff --git a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/auth.py b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/auth.py index d7f942bf1..f0ed3fb03 100644 --- a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/auth.py +++ b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/auth.py @@ -264,8 +264,8 @@ def require_auth( try: decoder = _get_decoder() - except RuntimeError as exc: - logger.error("Gateway auth misconfigured: %s", exc, exc_info=True) + except RuntimeError: + logger.exception("Gateway auth misconfigured") raise HTTPException( status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail="Gateway authentication is not configured.", From cc0f4d2439afa67b02244a9c857bf802fa72f1be Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:34:05 +0400 Subject: [PATCH 02/16] Keep naive policy dates explicit for ruff DTZ policyengine.py parses policy period keys with strptime and no zone, so its ParameterValue dates are naive, and the executor's period parsing and the tests that compare against those dates must stay naive too. Mark each kept naive datetime with a reasoned DTZ suppression, and route the policy tests' expected dates through one helper. Co-Authored-By: Claude Opus 5.5 --- .../simulation_runtime.py | 2 +- .../tests/test_simulation_runtime_policy.py | 33 +++++++++++++------ .../tests/test_stage12_runtime.py | 2 +- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_runtime.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_runtime.py index 8bec8c60f..4d2ddd118 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_runtime.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_runtime.py @@ -222,7 +222,7 @@ def _parse_policy_period_dates(text: str) -> tuple[datetime, datetime | None]: raise ValueError # The wrapper has always parsed dates with strptime, which also takes # unpadded months and days ("2027-4-6"); accept what it accepted. - dates = [datetime.strptime(part, "%Y-%m-%d") for part in parts] + dates = [datetime.strptime(part, "%Y-%m-%d") for part in parts] # noqa: DTZ007 # naive like policyengine.py's own period parsing; its ParameterValue dates are naive start = dates[0] stop = dates[1] if len(dates) == 2 else None if stop is not None and stop < start: diff --git a/projects/policyengine-simulation-executor/tests/test_simulation_runtime_policy.py b/projects/policyengine-simulation-executor/tests/test_simulation_runtime_policy.py index 1c2ee7fb7..6845e540d 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_runtime_policy.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_runtime_policy.py @@ -21,6 +21,15 @@ US_SALT_CAP = "gov.irs.deductions.itemized.salt_and_real_estate.cap.JOINT" +def _naive_policy_date(year: int, month: int, day: int) -> datetime: + """Return midnight on a date as a naive datetime. + + policyengine.py parses policy period keys with ``strptime`` and no zone, so + its ``ParameterValue`` dates are naive; an aware value never compares equal. + """ + return datetime(year, month, day) # noqa: DTZ001 # naive like the dates under test + + @pytest.fixture(scope="module") def uk(): country = _country_module("uk") @@ -59,8 +68,8 @@ def test_uk_interval_policy_preserves_inclusive_stop_date(uk, stop): assert value.parameter.name == UK_RATE assert value.parameter.tax_benefit_model_version is uk.model assert value.parameter.data_type is float - assert value.start_date == datetime(2027, 4, 6) - assert value.end_date == datetime.strptime(stop, "%Y-%m-%d") + assert value.start_date == _naive_policy_date(2027, 4, 6) + assert value.end_date == datetime.strptime(stop, "%Y-%m-%d") # noqa: DTZ007 # policyengine.py's ParameterValue dates are naive assert value.value == 0.21 assert reform_dict_from_parameter_values(policy.parameter_values) == raw @@ -83,9 +92,9 @@ def test_mixed_effective_dates_and_intervals_are_chronological(uk, reverse): (value.start_date, value.end_date, value.value) for value in policy.parameter_values ] == [ - (datetime(2026, 4, 6), None, 0.20), - (datetime(2027, 4, 6), datetime(2028, 4, 5), 0.21), - (datetime(2029, 4, 6), None, 0.22), + (_naive_policy_date(2026, 4, 6), None, 0.20), + (_naive_policy_date(2027, 4, 6), _naive_policy_date(2028, 4, 5), 0.21), + (_naive_policy_date(2029, 4, 6), None, 0.22), ] assert reform_dict_from_parameter_values(policy.parameter_values) == { UK_RATE: { @@ -165,7 +174,7 @@ def test_scalar_in_interval_policy_uses_request_year(uk, period, year): for value in simulation.policy.parameter_values if value.parameter.name == UK_ALLOWANCE ) - assert scalar.start_date == datetime(year, 1, 1) + assert scalar.start_date == _naive_policy_date(year, 1, 1) assert scalar.end_date is None assert scalar.value == 13000 assert scalar.parameter.data_type is int @@ -188,8 +197,8 @@ def test_us_interval_policy_uses_same_public_parameter_values(): assert value.parameter.name == US_SALT_CAP assert value.parameter.tax_benefit_model_version is us.model assert value.parameter.data_type is int - assert value.start_date == datetime(2027, 1, 1) - assert value.end_date == datetime(2027, 12, 31) + assert value.start_date == _naive_policy_date(2027, 1, 1) + assert value.end_date == _naive_policy_date(2027, 12, 31) assert value.value == 50000 assert reform_dict_from_parameter_values(policy.parameter_values) == raw @@ -237,8 +246,12 @@ def recording_compiler(reform, *, year, model_version): @pytest.mark.parametrize( "key, start, stop", [ - ("2027-4-6", datetime(2027, 4, 6), None), - ("2027-4-6.2028-4-5", datetime(2027, 4, 6), datetime(2028, 4, 5)), + ("2027-4-6", _naive_policy_date(2027, 4, 6), None), + ( + "2027-4-6.2028-4-5", + _naive_policy_date(2027, 4, 6), + _naive_policy_date(2028, 4, 5), + ), ], ) def test_unpadded_dates_are_accepted_as_the_wrapper_always_accepted_them( diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py b/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py index f76dbeda3..e6f930aad 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py @@ -1071,7 +1071,7 @@ def serialize(self): for role, stop in (("baseline", "2026-12-31"), ("reform", "2027-12-31")): (value,) = observed[role].policy.parameter_values - assert value.end_date == datetime.strptime(stop, "%Y-%m-%d"), role + assert value.end_date == datetime.strptime(stop, "%Y-%m-%d"), role # noqa: DTZ007 # policyengine.py's ParameterValue dates are naive def _uk_report() -> ReportExecutionInput: From c13dd025109c3bfb9b8f67062cc99e04a4e597e6 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:35:01 +0400 Subject: [PATCH 03/16] Replace asserts outside tests for ruff S101 The deployed-service test suite in authenticated_tests is a pytest suite that the tests glob misses, so give it the same test exemptions. Elsewhere, drop the asserts that only narrowed types: the SPM selection check now returns the hash and scenario it has already proved present, and the qualification runner closes over a non-optional runtime. The gateway image smoke raises RuntimeError when its report is incomplete, as the executor's static-file smoke does, and the fake model output raises AssertionError explicitly when the adapter asks for a mode other than JSON. Co-Authored-By: Claude Opus 5.5 --- .../policyengine_simulation_contract/spm.py | 20 +++++++++---------- .../test_simulation_output_builder.py | 4 +++- .../stage12_qualification.py | 6 +++--- .../smoke_app.py | 8 ++++++-- ruff.toml | 3 +++ 5 files changed, 24 insertions(+), 17 deletions(-) diff --git a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/spm.py b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/spm.py index 450950d69..bf47aadc3 100644 --- a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/spm.py +++ b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/spm.py @@ -229,7 +229,8 @@ class SPMRuntimeVersions(BaseModel): spm_calculator: str = Field(min_length=1, alias="spm-calculator") -def _require_complete_selection(selection: SPMSelection) -> None: +def _require_complete_selection(selection: SPMSelection) -> tuple[str, str]: + """Return a complete selection's artifact hash and scenario, else raise ValueError.""" required = set(SPMSelection.model_fields) missing = required.difference(selection.model_fields_set) if missing: @@ -242,18 +243,17 @@ def _require_complete_selection(selection: SPMSelection) -> None: if selection.scenario is None: msg = "Resolved SPM selection has no scenario" raise ValueError(msg) + return selection.forecast_content_sha256, selection.scenario def _resolved_spm_configuration( selection: SPMSelection, ) -> SPMResolvedConfiguration: """Convert a resolved request selection to the completed-result contract.""" - _require_complete_selection(selection) - assert selection.forecast_content_sha256 is not None - assert selection.scenario is not None + forecast_content_sha256, scenario = _require_complete_selection(selection) return SPMResolvedConfiguration( - forecast_content_sha256=selection.forecast_content_sha256, - scenario=selection.scenario, + forecast_content_sha256=forecast_content_sha256, + scenario=scenario, geography_kind=selection.geography_kind, geography_id=selection.geography_id, county_vintage=selection.county_vintage, @@ -284,17 +284,15 @@ def build_spm_provenance( runtime_versions: SPMRuntimeVersions, ) -> SPMProvenance: """Build one compact receipt from resolved calculation inputs.""" - _require_complete_selection(selection) - assert selection.forecast_content_sha256 is not None - assert selection.scenario is not None - if forecast_sha256 != selection.forecast_content_sha256: + forecast_content_sha256, scenario = _require_complete_selection(selection) + if forecast_sha256 != forecast_content_sha256: msg = "SPM receipt artifact hash differs from the selection" raise ValueError(msg) return SPMProvenance( schema_version="canonical-spm-provenance-v2", forecast_id=forecast_id, forecast_sha256=forecast_sha256, - scenario=selection.scenario, + scenario=scenario, geography_kind=selection.geography_kind, geography_id=selection.geography_id, county_vintage=selection.county_vintage, diff --git a/projects/policyengine-simulation-executor/fixtures/test_simulation_output_builder.py b/projects/policyengine-simulation-executor/fixtures/test_simulation_output_builder.py index 2d091ee87..14f6c642a 100644 --- a/projects/policyengine-simulation-executor/fixtures/test_simulation_output_builder.py +++ b/projects/policyengine-simulation-executor/fixtures/test_simulation_output_builder.py @@ -24,7 +24,9 @@ def __init__(self, payload: dict[str, object]) -> None: def model_dump(self, *, mode: str) -> dict[str, object]: """Return the payload; fails unless the adapter asks for JSON mode.""" - assert mode == "json" + if mode != "json": + msg = f"The adapter must dump outputs in JSON mode, not {mode!r}" + raise AssertionError(msg) return self.payload diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_qualification.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_qualification.py index 0522f525c..34473ae93 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_qualification.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_qualification.py @@ -241,11 +241,11 @@ def qualify_report_parity( run_simulation_impl, ) - local_runtime = _local_qualification_runtime() + qualification_runtime = _local_qualification_runtime() + local_runtime = qualification_runtime def run_existing(request: dict[str, Any]) -> Mapping[str, Any]: - assert local_runtime is not None - return run_simulation_impl(request, runtime=local_runtime) + return run_simulation_impl(request, runtime=qualification_runtime) existing_runner = run_existing if single_simulation_runner is None: diff --git a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/smoke_app.py b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/smoke_app.py index 6b53b8312..dcd62c39f 100644 --- a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/smoke_app.py +++ b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/smoke_app.py @@ -70,6 +70,10 @@ def main() -> None: """Run the in-image smoke remotely; fail unless modules and /health loaded.""" report = smoke_import_gateway.remote() print(report) - assert report["modules_imported"] > 0 - assert any("/health" in route for route in report["routes"]) + if report["modules_imported"] <= 0: + msg = f"Gateway image smoke imported no modules: {report!r}" + raise RuntimeError(msg) + if not any("/health" in route for route in report["routes"]): + msg = f"Gateway image smoke found no /health route: {report!r}" + raise RuntimeError(msg) print("gateway image smoke OK") diff --git a/ruff.toml b/ruff.toml index 1aa85b582..a24250a3a 100644 --- a/ruff.toml +++ b/ruff.toml @@ -28,6 +28,9 @@ ignore = ["D203", "D213"] # Tests may use pytest's bare assert, and need no docstrings, annotations or # named constants for magic numbers. Every other rule applies to tests. "**/tests/**" = ["ANN", "D1", "PLR2004", "S101"] +# The deployed-service test suite (pytest, run against a live candidate) gets +# the same test exemptions. +"**/authenticated_tests/**" = ["ANN", "D1", "PLR2004", "S101"] [lint.flake8-type-checking] # Classes whose annotations Pydantic evaluates at runtime, so their imports must From e3292e767bf232914a7a1528d427507a01540c68 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:36:14 +0400 Subject: [PATCH 04/16] Keep command-line output for ruff T20 Every print the rule finds belongs to a command-line script or a Modal local entrypoint, whose stdout is its output, so keep each print. Files with one to three prints mark each with a reasoned suppression; the routing-state publisher and the policyengine package updater send their output through one small helper that holds the only print. Co-Authored-By: Claude Opus 5.5 --- .../scripts/update_policyengine_package.py | 13 +++-- .../generate_openapi.py | 2 +- .../stage12_infrastructure.py | 2 +- .../scripts/dump_package_version.py | 2 +- .../src/modal/smoke_app.py | 4 +- .../modal/utils/extract_bundle_versions.py | 4 +- .../src/modal/utils/extract_stage12_bundle.py | 4 +- .../src/modal/utils/qualify_stage12_parity.py | 2 +- .../src/modal/utils/record_deployment.py | 2 +- .../modal/utils/update_v2_version_manifest.py | 2 +- .../modal/utils/update_version_registry.py | 49 ++++++++++--------- .../src/modal/utils/validate_hf_access.py | 2 +- .../modal/utils/validate_v2_worker_release.py | 2 +- .../modal/v2_static_runtime_files_smoke.py | 2 +- .../generate_openapi.py | 2 +- .../generate_openapi.py | 2 +- .../smoke_app.py | 4 +- scripts/run_quality_guards.py | 6 +-- 18 files changed, 58 insertions(+), 48 deletions(-) diff --git a/.github/scripts/update_policyengine_package.py b/.github/scripts/update_policyengine_package.py index f570b34ec..fc8d3936b 100644 --- a/.github/scripts/update_policyengine_package.py +++ b/.github/scripts/update_policyengine_package.py @@ -32,6 +32,11 @@ def _json_from_stdin() -> object: return json.load(sys.stdin) +def _write_result(result: object) -> None: + """Print one result line for update-policyengine-package.sh to capture.""" + print(result) # noqa: T201 # command-line script: the shell script reads stdout + + def _requirement_name(requirement: str) -> str: return re.split(r"[\s\[<>=!~;@]", requirement, maxsplit=1)[0] @@ -155,7 +160,7 @@ def find_issue(title: str) -> None: and isinstance(item.get("number"), int) ) if matches: - print(matches[0]) + _write_result(matches[0]) def verify_issue(expected_number: int, expected_title: str) -> None: @@ -191,7 +196,7 @@ def latest_version(package: str) -> None: if not isinstance(version, str) or not version: msg = f"PyPI response has no latest version for {package}" raise SystemExit(msg) - print(version) + _write_result(version) def bundle_versions() -> None: @@ -264,7 +269,7 @@ def certificate_sha256(country: str) -> str: "uk_certificate_sha256": certificate_sha256("uk"), } for name, version in outputs.items(): - print(f"{name}={version}") + _write_result(f"{name}={version}") #: Release-dispatch fields compared with the installed bundle: (payload key @@ -398,7 +403,7 @@ def main() -> None: """ args = _parser().parse_args() if args.command == "current-version": - print(current_version(args.pyproject, args.package)) + _write_result(current_version(args.pyproject, args.package)) elif args.command == "update-requirements": update_requirements(args.pyproject, args.package, args.current, args.latest) elif args.command == "find-issue": diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/generate_openapi.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/generate_openapi.py index 70f1e9066..3e985410c 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/generate_openapi.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/generate_openapi.py @@ -20,7 +20,7 @@ def main() -> None: json.dumps(create_app().openapi(), indent=2) + "\n", encoding="utf-8", ) - print(f"OpenAPI spec written to {output}") + print(f"OpenAPI spec written to {output}") # noqa: T201 # command-line script: reports where it wrote the spec if __name__ == "__main__": diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_infrastructure.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_infrastructure.py index ccae557c9..09a36cd8b 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_infrastructure.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_infrastructure.py @@ -36,7 +36,7 @@ def main() -> None: ) except (Stage12RuntimeAccessError, ValueError) as error: raise SystemExit(str(error)) from None - print( + print( # noqa: T201 # command-line check: stdout reports the verified access "Shared API v2 runtime database access verified for Stage 12: " f"environment={args.environment}, role={args.expected_role}" ) diff --git a/projects/policyengine-simulation-executor/scripts/dump_package_version.py b/projects/policyengine-simulation-executor/scripts/dump_package_version.py index 32732e013..943fe1a8c 100644 --- a/projects/policyengine-simulation-executor/scripts/dump_package_version.py +++ b/projects/policyengine-simulation-executor/scripts/dump_package_version.py @@ -34,5 +34,5 @@ def get_package_version_from_lock( if version is None: sys.exit(1) -print(version) +print(version) # noqa: T201 # command-line script: the version on stdout is its output sys.exit(0) diff --git a/projects/policyengine-simulation-executor/src/modal/smoke_app.py b/projects/policyengine-simulation-executor/src/modal/smoke_app.py index 65ad946e8..40320881c 100644 --- a/projects/policyengine-simulation-executor/src/modal/smoke_app.py +++ b/projects/policyengine-simulation-executor/src/modal/smoke_app.py @@ -169,5 +169,5 @@ def smoke_import_executor() -> dict: def main() -> None: """Run the image smoke remotely and print its report.""" report = smoke_import_executor.remote() - print(report) - print("executor image smoke OK") + print(report) # noqa: T201 # Modal local entrypoint: runs locally, and stdout is the smoke's report + print("executor image smoke OK") # noqa: T201 # Modal local entrypoint: runs locally, and stdout is the smoke's report diff --git a/projects/policyengine-simulation-executor/src/modal/utils/extract_bundle_versions.py b/projects/policyengine-simulation-executor/src/modal/utils/extract_bundle_versions.py index 4e41068fa..3d5c263e8 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/extract_bundle_versions.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/extract_bundle_versions.py @@ -39,7 +39,7 @@ def main() -> None: if "--shell" in sys.argv[1:]: for key, value in outputs.items(): - print(f"{key}={value}") + print(f"{key}={value}") # noqa: T201 # command-line script: deployment scripts read stdout return github_output = os.environ.get("GITHUB_OUTPUT") @@ -49,7 +49,7 @@ def main() -> None: for key, value in outputs.items(): file.write(f"{key}={value}\n") - print( + print( # noqa: T201 # command-line script: deployment scripts read stdout "Deploying with policyengine.py bundle " f"{outputs['policyengine_version']}: " f"policyengine-core={outputs['policyengine_core_version']}, " diff --git a/projects/policyengine-simulation-executor/src/modal/utils/extract_stage12_bundle.py b/projects/policyengine-simulation-executor/src/modal/utils/extract_stage12_bundle.py index 3850ed768..11cd6dbaa 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/extract_stage12_bundle.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/extract_stage12_bundle.py @@ -66,10 +66,10 @@ def main() -> None: + "\n" ) if args.output is None: - print(rendered, end="") + print(rendered, end="") # noqa: T201 # command-line script: stdout is its output else: args.output.write_text(rendered, encoding="utf-8") - print(f"bundle_manifest_sha256={resolved.bundle_manifest_sha256}") + print(f"bundle_manifest_sha256={resolved.bundle_manifest_sha256}") # noqa: T201 # command-line script: stdout is its output if __name__ == "__main__": diff --git a/projects/policyengine-simulation-executor/src/modal/utils/qualify_stage12_parity.py b/projects/policyengine-simulation-executor/src/modal/utils/qualify_stage12_parity.py index 994750336..42c5864f3 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/qualify_stage12_parity.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/qualify_stage12_parity.py @@ -61,7 +61,7 @@ def main() -> None: simulation_tolerances=_tolerances(arguments.simulation_tolerances), aggregate_tolerances=_tolerances(arguments.aggregate_tolerances), ) - print(receipt.model_dump_json()) + print(receipt.model_dump_json()) # noqa: T201 # command-line script: the receipt JSON on stdout is its output if __name__ == "__main__": diff --git a/projects/policyengine-simulation-executor/src/modal/utils/record_deployment.py b/projects/policyengine-simulation-executor/src/modal/utils/record_deployment.py index 3b0426453..9efd1beb4 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/record_deployment.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/record_deployment.py @@ -64,7 +64,7 @@ def main() -> None: # The store speaks JSON-able mappings; models dump at the call site # (same discipline as publish_manifest_impl). ArtifactStore().write_deployed_marker(args.environment, marker.model_dump()) - print( + print( # noqa: T201 # command-line script: reports the recorded marker f"Recorded deployed/{args.environment}.json for manifest {args.manifest_digest}" ) diff --git a/projects/policyengine-simulation-executor/src/modal/utils/update_v2_version_manifest.py b/projects/policyengine-simulation-executor/src/modal/utils/update_v2_version_manifest.py index c695073e7..632e8e87b 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/update_v2_version_manifest.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/update_v2_version_manifest.py @@ -81,7 +81,7 @@ def main() -> None: worker=worker, force_default=args.force_default, ) - print( + print( # noqa: T201 # command-line script: the manifest JSON on stdout is its output json.dumps( manifest.model_dump(mode="json"), sort_keys=True, diff --git a/projects/policyengine-simulation-executor/src/modal/utils/update_version_registry.py b/projects/policyengine-simulation-executor/src/modal/utils/update_version_registry.py index 6fef5ef89..c1265f2e5 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/update_version_registry.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/update_version_registry.py @@ -1085,6 +1085,11 @@ def restore_routing_state( return next_state +def _report(line: str = "") -> None: + """Print one line of the command's progress report.""" + print(line) # noqa: T201 # command-line script: stdout is its progress report + + def main() -> None: """Publish a deployed app's routes, or seed the state from legacy Dicts.""" parser = argparse.ArgumentParser( @@ -1355,19 +1360,19 @@ def active_state() -> object: parser.error("--expected-latest needs --restore-latest") if args.seed_active_from_legacy: - print( + _report( "Seeding active routing state from legacy Modal dicts in " f"environment: {args.environment}" ) state = seed_active_routing_state_from_legacy(environment=args.environment) - print() - print(f" {ROUTING_STATE_DICT_NAME}[{ROUTING_STATE_ACTIVE_KEY}]: updated") - print(f" policyengine routes: {len(state['routes']['policyengine'])}") - print(f" US routes: {len(state['routes']['us'])}") - print(f" UK routes: {len(state['routes']['uk'])}") - print(f" bundle manifests: {len(state['bundles'])}") - print() - print("Legacy routing state seed completed successfully.") + _report() + _report(f" {ROUTING_STATE_DICT_NAME}[{ROUTING_STATE_ACTIVE_KEY}]: updated") + _report(f" policyengine routes: {len(state['routes']['policyengine'])}") + _report(f" US routes: {len(state['routes']['us'])}") + _report(f" UK routes: {len(state['routes']['uk'])}") + _report(f" bundle manifests: {len(state['bundles'])}") + _report() + _report("Legacy routing state seed completed successfully.") return required_args = { @@ -1398,12 +1403,12 @@ def active_state() -> object: print(f"advances_latest={'true' if plan['advances_latest'] else 'false'}") return - print(f"Publishing routing state in Modal environment: {args.environment}") - print(f" App name: {args.app_name}") - print(f" policyengine.py version: {args.policyengine_version}") - print(f" US version: {args.us_version}") - print(f" UK version: {args.uk_version}") - print() + _report(f"Publishing routing state in Modal environment: {args.environment}") + _report(f" App name: {args.app_name}") + _report(f" policyengine.py version: {args.policyengine_version}") + _report(f" US version: {args.us_version}") + _report(f" UK version: {args.uk_version}") + _report() state = publish_routing_state( environment=args.environment, @@ -1413,13 +1418,13 @@ def active_state() -> object: uk_version=args.uk_version, force_latest=args.force_latest, ) - print() - print(f" {ROUTING_STATE_DICT_NAME}[{ROUTING_STATE_ACTIVE_KEY}]: updated") - print(f" latest.policyengine: {state['latest']['policyengine']}") - print(f" latest.us: {state['latest']['us']}") - print(f" latest.uk: {state['latest']['uk']}") - print() - print("Routing state published successfully.") + _report() + _report(f" {ROUTING_STATE_DICT_NAME}[{ROUTING_STATE_ACTIVE_KEY}]: updated") + _report(f" latest.policyengine: {state['latest']['policyengine']}") + _report(f" latest.us: {state['latest']['us']}") + _report(f" latest.uk: {state['latest']['uk']}") + _report() + _report("Routing state published successfully.") if __name__ == "__main__": diff --git a/projects/policyengine-simulation-executor/src/modal/utils/validate_hf_access.py b/projects/policyengine-simulation-executor/src/modal/utils/validate_hf_access.py index 895fc9549..27ebf3062 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/validate_hf_access.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/validate_hf_access.py @@ -15,7 +15,7 @@ def validate_local_access() -> HFCredentialAudit: def main() -> None: """Validate the CI credential and print its audit record as JSON.""" - print(validate_local_access().model_dump_json()) + print(validate_local_access().model_dump_json()) # noqa: T201 # command-line script: the audit record on stdout is its output if __name__ == "__main__": diff --git a/projects/policyengine-simulation-executor/src/modal/utils/validate_v2_worker_release.py b/projects/policyengine-simulation-executor/src/modal/utils/validate_v2_worker_release.py index 0192c50f4..579799482 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/validate_v2_worker_release.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/validate_v2_worker_release.py @@ -108,7 +108,7 @@ def main() -> None: + "\n", encoding="utf-8", ) - print(validation.validation_invocation_id) + print(validation.validation_invocation_id) # noqa: T201 # command-line script: logs the validation id for CI if __name__ == "__main__": diff --git a/projects/policyengine-simulation-executor/src/modal/v2_static_runtime_files_smoke.py b/projects/policyengine-simulation-executor/src/modal/v2_static_runtime_files_smoke.py index 47c15c8b9..3c022295a 100644 --- a/projects/policyengine-simulation-executor/src/modal/v2_static_runtime_files_smoke.py +++ b/projects/policyengine-simulation-executor/src/modal/v2_static_runtime_files_smoke.py @@ -71,4 +71,4 @@ def main() -> None: f"UK dataset boundary version: {observed!r}" ) raise RuntimeError(msg) - print("Stage 12 static runtime file smoke OK") + print("Stage 12 static runtime file smoke OK") # noqa: T201 # Modal local entrypoint: runs locally, and stdout is the smoke's report diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/generate_openapi.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/generate_openapi.py index 3bf906d7e..f7b8b069b 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/generate_openapi.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/generate_openapi.py @@ -13,4 +13,4 @@ with open(output_path, "w") as f: json.dump(openapi_spec, f, indent=4) -print(f"OpenAPI spec written to {output_path}") +print(f"OpenAPI spec written to {output_path}") # noqa: T201 # command-line script: reports where it wrote the spec diff --git a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/generate_openapi.py b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/generate_openapi.py index fea8f2a3b..5df2740f3 100644 --- a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/generate_openapi.py +++ b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/generate_openapi.py @@ -150,7 +150,7 @@ def main() -> None: with open(output_path, "w") as f: json.dump(openapi_spec, f, indent=2) - print(f"OpenAPI spec written to {output_path}") + print(f"OpenAPI spec written to {output_path}") # noqa: T201 # command-line script: reports where it wrote the spec if __name__ == "__main__": diff --git a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/smoke_app.py b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/smoke_app.py index dcd62c39f..647afeb65 100644 --- a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/smoke_app.py +++ b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/smoke_app.py @@ -69,11 +69,11 @@ def smoke_import_gateway() -> dict: def main() -> None: """Run the in-image smoke remotely; fail unless modules and /health loaded.""" report = smoke_import_gateway.remote() - print(report) + print(report) # noqa: T201 # Modal local entrypoint: runs locally, and stdout is the smoke's report if report["modules_imported"] <= 0: msg = f"Gateway image smoke imported no modules: {report!r}" raise RuntimeError(msg) if not any("/health" in route for route in report["routes"]): msg = f"Gateway image smoke found no /health route: {report!r}" raise RuntimeError(msg) - print("gateway image smoke OK") + print("gateway image smoke OK") # noqa: T201 # Modal local entrypoint: runs locally, and stdout is the smoke's report diff --git a/scripts/run_quality_guards.py b/scripts/run_quality_guards.py index c6d7f4cc1..919857305 100644 --- a/scripts/run_quality_guards.py +++ b/scripts/run_quality_guards.py @@ -30,13 +30,13 @@ def main() -> int: for name, check in GUARDS: violations = check() if not violations: - print(f"{name}: passed") + print(f"{name}: passed") # noqa: T201 # command-line guard runner: stdout is its report continue failed = True - print(f"{name}: failed") + print(f"{name}: failed") # noqa: T201 # command-line guard runner: stdout is its report for violation in violations: - print(f" - {violation}") + print(f" - {violation}") # noqa: T201 # command-line guard runner: stdout is its report return 1 if failed else 0 From 4915e60f5d70c57bcdcded3f1b7f549c6921276b Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:39:48 +0400 Subject: [PATCH 05/16] Route test subprocess calls through helpers for ruff S603 and S607 Each test module that runs repository scripts now does so through one helper that holds the only subprocess call, with reasoned suppressions: the scripts are repository-owned, their arguments are built by the tests, and bash and uv come from PATH as they do in the workflows. Resolving the executables in Python instead could pick a different binary, because subprocess searches the child's PATH, and some tests run scripts with an empty environment. Co-Authored-By: Claude Opus 5.5 --- .../tests/test_cloud_run_revision_cleanup.py | 23 +- .../tests/test_deployment_assets.py | 93 +++---- .../tests/test_stage12_infrastructure.py | 37 +-- .../tests/workflow_script_support.py | 2 +- ...est_policyengine_package_update_scripts.py | 4 +- .../integration/test_image_smoke_modal.py | 36 +-- .../tests/test_ci_complete_gate.py | 31 ++- .../tests/test_modal_scripts.py | 258 +++++------------- .../test_policyengine_dependency_source.py | 29 +- ...est_policyengine_package_update_scripts.py | 6 +- .../integration/test_image_smoke_modal.py | 4 +- 11 files changed, 192 insertions(+), 331 deletions(-) diff --git a/projects/policyengine-simulation-entry/tests/test_cloud_run_revision_cleanup.py b/projects/policyengine-simulation-entry/tests/test_cloud_run_revision_cleanup.py index 80cfecf78..2f9bc38fd 100644 --- a/projects/policyengine-simulation-entry/tests/test_cloud_run_revision_cleanup.py +++ b/projects/policyengine-simulation-entry/tests/test_cloud_run_revision_cleanup.py @@ -27,6 +27,22 @@ ) +def _run_bash( + *arguments: str | Path, + check: bool, + capture_output: bool = False, + env: dict[str, str] | None = None, +) -> subprocess.CompletedProcess[str]: + """Run bash on a repository deployment script, with text output.""" + return subprocess.run( # noqa: S603 # runs repository scripts with test-built arguments + ["bash", *arguments], # noqa: S607 # bash from PATH, as the workflows run these scripts + check=check, + capture_output=capture_output, + text=True, + env=env, + ) + + def _revision(name: str, created_at: str, *, ready: bool = True) -> dict: return { "metadata": { @@ -96,11 +112,10 @@ def _run_cleanup( ) revisions_json.write_text(json.dumps(revisions), encoding="utf-8") - return subprocess.run( - ["bash", CLEANUP_SCRIPT], + return _run_bash( + CLEANUP_SCRIPT, check=False, capture_output=True, - text=True, env=os.environ | { "GCLOUD_BIN": str(_write_fake_gcloud(tmp_path)), @@ -321,5 +336,5 @@ def test_revision_cleanup_runs_after_promoted_deployment_completion(): def test_revision_cleanup_script_has_valid_shell_syntax_and_is_executable(): - subprocess.run(["bash", "-n", CLEANUP_SCRIPT], check=True) + _run_bash("-n", CLEANUP_SCRIPT, check=True) assert os.access(CLEANUP_SCRIPT, os.X_OK) diff --git a/projects/policyengine-simulation-entry/tests/test_deployment_assets.py b/projects/policyengine-simulation-entry/tests/test_deployment_assets.py index df84ef1fb..b5af13c0b 100644 --- a/projects/policyengine-simulation-entry/tests/test_deployment_assets.py +++ b/projects/policyengine-simulation-entry/tests/test_deployment_assets.py @@ -58,10 +58,26 @@ ) +def _run_bash( + *arguments: str | Path, + check: bool, + capture_output: bool = False, + env: dict[str, str] | None = None, +) -> subprocess.CompletedProcess[str]: + """Run bash on a repository deployment script, with text output.""" + return subprocess.run( # noqa: S603 # runs repository scripts with test-built arguments + ["bash", *arguments], # noqa: S607 # bash from PATH, as the workflows run these scripts + check=check, + capture_output=capture_output, + text=True, + env=env, + ) + + def test_deployment_scripts_have_valid_shell_syntax(): - subprocess.run(["bash", "-n", TRAFFIC_SCRIPT], check=True) - subprocess.run(["bash", "-n", SMOKE_SCRIPT], check=True) - subprocess.run(["bash", "-n", CLOUD_RUN_DEPLOY_SCRIPT], check=True) + _run_bash("-n", TRAFFIC_SCRIPT, check=True) + _run_bash("-n", SMOKE_SCRIPT, check=True) + _run_bash("-n", CLOUD_RUN_DEPLOY_SCRIPT, check=True) assert os.access(SMOKE_SCRIPT, os.X_OK) assert os.access(CLOUD_RUN_DEPLOY_SCRIPT, os.X_OK) @@ -167,17 +183,13 @@ def test_stable_smoke_retries_the_previous_revision_until_target_is_served( [old_revision, old_revision, target_revision], ) - subprocess.run( - [ - "bash", - SMOKE_SCRIPT, - "https://simulation.example.test", - target_revision, - old_revision, - ], + _run_bash( + SMOKE_SCRIPT, + "https://simulation.example.test", + target_revision, + old_revision, check=True, capture_output=True, - text=True, env=_smoke_test_env(tmp_path, state_file, revisions_file), ) @@ -193,17 +205,13 @@ def test_stable_smoke_rejects_an_unexpected_revision_without_retrying(tmp_path: [unexpected_revision, target_revision], ) - result = subprocess.run( - [ - "bash", - SMOKE_SCRIPT, - "https://simulation.example.test", - target_revision, - old_revision, - ], + result = _run_bash( + SMOKE_SCRIPT, + "https://simulation.example.test", + target_revision, + old_revision, check=False, capture_output=True, - text=True, env=_smoke_test_env(tmp_path, state_file, revisions_file), ) @@ -220,16 +228,12 @@ def test_candidate_smoke_rejects_a_revision_mismatch_without_retrying(tmp_path: [old_revision, target_revision], ) - result = subprocess.run( - [ - "bash", - SMOKE_SCRIPT, - "https://candidate.example.test", - target_revision, - ], + result = _run_bash( + SMOKE_SCRIPT, + "https://candidate.example.test", + target_revision, check=False, capture_output=True, - text=True, env=_smoke_test_env(tmp_path, state_file, revisions_file), ) @@ -248,17 +252,13 @@ def test_stable_smoke_fails_when_the_previous_revision_does_not_converge( [old_revision, old_revision, old_revision], ) - result = subprocess.run( - [ - "bash", - SMOKE_SCRIPT, - "https://simulation.example.test", - target_revision, - old_revision, - ], + result = _run_bash( + SMOKE_SCRIPT, + "https://simulation.example.test", + target_revision, + old_revision, check=False, capture_output=True, - text=True, env=_smoke_test_env(tmp_path, state_file, revisions_file), ) @@ -493,7 +493,7 @@ def test_cloud_run_deployment_escapes_environment_and_pins_secret_versions(tmp_p arguments_capture = Path(command_environment["GCLOUD_ARGUMENTS_CAPTURE"]) env_capture = Path(command_environment["GCLOUD_ENV_FILE_CAPTURE"]) - subprocess.run( + subprocess.run( # noqa: S603 # runs the repository deploy script directly, as the workflow does [CLOUD_RUN_DEPLOY_SCRIPT], check=True, env=command_environment, @@ -1691,11 +1691,10 @@ def test_traffic_changes_are_exact_revision_validated_and_reversible(tmp_path): "SIMULATION_ENTRYPOINT_SERVICE": "policyengine-simulation-entry", } - subprocess.run( - ["bash", TRAFFIC_SCRIPT], + _run_bash( + TRAFFIC_SCRIPT, check=True, capture_output=True, - text=True, env={ **base_env, "SIMULATION_ENTRYPOINT_TARGET_REVISION": "policyengine-simulation-entry-00002-new", @@ -1707,11 +1706,10 @@ def test_traffic_changes_are_exact_revision_validated_and_reversible(tmp_path): == "policyengine-simulation-entry-00002-new" ) - subprocess.run( - ["bash", TRAFFIC_SCRIPT], + _run_bash( + TRAFFIC_SCRIPT, check=True, capture_output=True, - text=True, env={ **base_env, "SIMULATION_ENTRYPOINT_TARGET_REVISION": "policyengine-simulation-entry-00001-old", @@ -1742,11 +1740,10 @@ def test_traffic_change_refuses_an_intervening_promotion(tmp_path): ) fake_gcloud.chmod(0o755) - result = subprocess.run( - ["bash", TRAFFIC_SCRIPT], + result = _run_bash( + TRAFFIC_SCRIPT, check=False, capture_output=True, - text=True, env={ **os.environ, "GCLOUD_BIN": str(fake_gcloud), diff --git a/projects/policyengine-simulation-entry/tests/test_stage12_infrastructure.py b/projects/policyengine-simulation-entry/tests/test_stage12_infrastructure.py index 4eb899072..6833cad03 100644 --- a/projects/policyengine-simulation-entry/tests/test_stage12_infrastructure.py +++ b/projects/policyengine-simulation-entry/tests/test_stage12_infrastructure.py @@ -11,20 +11,23 @@ DEPLOY_WORKFLOW = REPO_ROOT / ".github/workflows/simulation-deploy.reusable.yml" -def test_infrastructure_validation_is_bounded_and_valid_shell() -> None: - result = subprocess.run( - ["bash", "-n", str(SCRIPT)], +def _run_bash( + *arguments: str, env: dict[str, str] | None = None +) -> subprocess.CompletedProcess[str]: + """Run bash on a repository script, capturing text output; a failure is returned.""" + return subprocess.run( # noqa: S603 # runs repository scripts with test-built arguments + ["bash", *arguments], # noqa: S607 # bash from PATH, as the workflows run these scripts capture_output=True, text=True, + env=env, check=False, ) + + +def test_infrastructure_validation_is_bounded_and_valid_shell() -> None: + result = _run_bash("-n", str(SCRIPT)) assert result.returncode == 0, result.stderr - sync_result = subprocess.run( - ["bash", "-n", str(SYNC_SCRIPT)], - capture_output=True, - text=True, - check=False, - ) + sync_result = _run_bash("-n", str(SYNC_SCRIPT)) assert sync_result.returncode == 0, sync_result.stderr source = SCRIPT.read_text(encoding="utf-8") assert "STAGE12_ENVIRONMENT" in source @@ -64,13 +67,7 @@ def test_infrastructure_validation_is_bounded_and_valid_shell() -> None: def test_infrastructure_validation_rejects_missing_configuration() -> None: - result = subprocess.run( - ["bash", str(SCRIPT)], - capture_output=True, - text=True, - env={}, - check=False, - ) + result = _run_bash(str(SCRIPT), env={}) assert result.returncode != 0 assert "STAGE12_ENVIRONMENT is required" in result.stderr @@ -98,13 +95,7 @@ def test_storage_validation_uses_only_the_modal_worker_credentials() -> None: def test_modal_secret_sync_rejects_missing_configuration() -> None: - result = subprocess.run( - ["bash", str(SYNC_SCRIPT), "staging"], - capture_output=True, - text=True, - env={}, - check=False, - ) + result = _run_bash(str(SYNC_SCRIPT), "staging", env={}) assert result.returncode != 0 assert "STAGE12_GCP_PROJECT_ID is required" in result.stderr diff --git a/projects/policyengine-simulation-entry/tests/workflow_script_support.py b/projects/policyengine-simulation-entry/tests/workflow_script_support.py index f9111816e..f6aa5141f 100644 --- a/projects/policyengine-simulation-entry/tests/workflow_script_support.py +++ b/projects/policyengine-simulation-entry/tests/workflow_script_support.py @@ -73,7 +73,7 @@ def run_script( cwd: Path | None = None, ) -> subprocess.CompletedProcess[str]: """Run ``.github/scripts/`` directly, as a workflow step does.""" - return subprocess.run( + return subprocess.run( # noqa: S603 # runs a repository script with test-built arguments [str(SCRIPTS_DIR / name), *args], capture_output=True, text=True, diff --git a/projects/policyengine-simulation-executor/fixtures/test_policyengine_package_update_scripts.py b/projects/policyengine-simulation-executor/fixtures/test_policyengine_package_update_scripts.py index 84b849f4c..edc93e3fc 100644 --- a/projects/policyengine-simulation-executor/fixtures/test_policyengine_package_update_scripts.py +++ b/projects/policyengine-simulation-executor/fixtures/test_policyengine_package_update_scripts.py @@ -250,8 +250,8 @@ def updater_env(fake_bin: Path, **extra: str) -> dict[str, str]: def run_updater(*args: str, env: dict[str, str]) -> subprocess.CompletedProcess[str]: """Run the updater from the repo root; a non-zero exit is returned, not raised.""" - return subprocess.run( - ["bash", str(SCRIPT), *args], + return subprocess.run( # noqa: S603 # runs the repository updater script with test-built arguments + ["bash", str(SCRIPT), *args], # noqa: S607 # bash from PATH, as the workflow runs the script cwd=REPO_ROOT, env=env, capture_output=True, diff --git a/projects/policyengine-simulation-executor/tests/integration/test_image_smoke_modal.py b/projects/policyengine-simulation-executor/tests/integration/test_image_smoke_modal.py index cf7919c9f..4437595ce 100644 --- a/projects/policyengine-simulation-executor/tests/integration/test_image_smoke_modal.py +++ b/projects/policyengine-simulation-executor/tests/integration/test_image_smoke_modal.py @@ -13,43 +13,27 @@ PROJECT_ROOT = Path(__file__).resolve().parents[2] -@pytest.mark.integration -def test_executor_image_smoke(): +def _run_modal_smoke(app_path: str) -> subprocess.CompletedProcess[str]: + """Run one smoke app with ``modal run`` in the ``MODAL_SMOKE_ENV`` environment.""" env_name = os.environ.get("MODAL_SMOKE_ENV", "staging") - result = subprocess.run( - [ - "uv", - "run", - "modal", - "run", - f"--env={env_name}", - "src/modal/smoke_app.py", - ], + return subprocess.run( # noqa: S603 # runs a repository smoke app through uv and Modal + ["uv", "run", "modal", "run", f"--env={env_name}", app_path], # noqa: S607 # uv from PATH, as CI runs it cwd=PROJECT_ROOT, capture_output=True, text=True, timeout=2400, ) + + +@pytest.mark.integration +def test_executor_image_smoke(): + result = _run_modal_smoke("src/modal/smoke_app.py") assert result.returncode == 0, result.stdout + result.stderr assert "executor image smoke OK" in result.stdout @pytest.mark.integration def test_stage12_static_runtime_file_image_smoke(): - env_name = os.environ.get("MODAL_SMOKE_ENV", "staging") - result = subprocess.run( - [ - "uv", - "run", - "modal", - "run", - f"--env={env_name}", - "src/modal/v2_static_runtime_files_smoke.py", - ], - cwd=PROJECT_ROOT, - capture_output=True, - text=True, - timeout=2400, - ) + result = _run_modal_smoke("src/modal/v2_static_runtime_files_smoke.py") assert result.returncode == 0, result.stdout + result.stderr assert "Stage 12 static runtime file smoke OK" in result.stdout diff --git a/projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py b/projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py index 798704db2..52b39b385 100644 --- a/projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py +++ b/projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py @@ -103,16 +103,25 @@ def test_rule_proof_gate_check() -> None: assert gate_violations(good) == [] +def _run_gate_script(env: dict[str, str]) -> subprocess.CompletedProcess[str]: + return subprocess.run( # noqa: S603 # runs the repository gate script with test-built arguments + ["bash", GATE_SCRIPT], # noqa: S607 # bash from PATH, as the workflow runs the script + cwd=REPO, + env=env, + text=True, + capture_output=True, + check=False, + ) + + def _run_gate( job_results: dict[str, str], *, pull_request_base_ref: str = "main", stack_base_ref: str = "", ) -> subprocess.CompletedProcess[str]: - return subprocess.run( - ["bash", GATE_SCRIPT], - cwd=REPO, - env={ + return _run_gate_script( + { "HOME": os.environ.get("HOME", ""), "PATH": os.environ["PATH"], "JOB_RESULTS": json.dumps( @@ -123,10 +132,7 @@ def _run_gate( ), "PULL_REQUEST_BASE_REF": pull_request_base_ref, "STACK_BASE_REF": stack_base_ref, - }, - text=True, - capture_output=True, - check=False, + } ) @@ -179,13 +185,8 @@ def test_gate_fails_when_a_job_was_skipped_on_full_ci() -> None: def test_gate_requires_the_job_results() -> None: - result = subprocess.run( - ["bash", GATE_SCRIPT], - cwd=REPO, - env={"PATH": os.environ["PATH"], "PULL_REQUEST_BASE_REF": "main"}, - text=True, - capture_output=True, - check=False, + result = _run_gate_script( + {"PATH": os.environ["PATH"], "PULL_REQUEST_BASE_REF": "main"} ) assert result.returncode != 0 diff --git a/projects/policyengine-simulation-executor/tests/test_modal_scripts.py b/projects/policyengine-simulation-executor/tests/test_modal_scripts.py index 3b9b9f45f..753a4a80a 100644 --- a/projects/policyengine-simulation-executor/tests/test_modal_scripts.py +++ b/projects/policyengine-simulation-executor/tests/test_modal_scripts.py @@ -6,6 +6,7 @@ import os import subprocess +from pathlib import Path import pytest from fixtures.test_modal_scripts import REPO_ROOT, SCRIPTS_DIR @@ -13,6 +14,22 @@ pytest_plugins = ("fixtures.test_modal_scripts",) +def _run_bash( + *arguments: str, + env: dict[str, str] | None = None, + cwd: Path | None = None, +) -> subprocess.CompletedProcess[str]: + """Run bash on a repository script, capturing text output; a failure is returned.""" + return subprocess.run( # noqa: S603 # runs repository scripts with test-built arguments + ["bash", *arguments], # noqa: S607 # bash from PATH, as the workflows run these scripts + capture_output=True, + text=True, + env=env, + cwd=cwd, + check=False, + ) + + class TestModalExtractVersions: """Tests for modal-extract-versions.sh.""" @@ -24,11 +41,7 @@ def test_script_exists(self): def test_script_is_executable_or_can_be_run_with_bash(self): """Script should be runnable with bash.""" - result = subprocess.run( - ["bash", "-n", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(self.script)) assert result.returncode == 0, f"Syntax error in script: {result.stderr}" def test_extracts_versions_from_policyengine_bundle(self, temp_github_output): @@ -41,12 +54,7 @@ def test_extracts_versions_from_policyengine_bundle(self, temp_github_output): env = os.environ.copy() env["GITHUB_OUTPUT"] = temp_github_output - result = subprocess.run( - ["bash", str(self.script), str(project_dir)], - capture_output=True, - text=True, - env=env, - ) + result = _run_bash(str(self.script), str(project_dir), env=env) assert result.returncode == 0, f"Script failed: {result.stderr}" @@ -101,28 +109,18 @@ def test_script_exists(self): def test_script_syntax(self): """Script should have valid bash syntax.""" - result = subprocess.run( - ["bash", "-n", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(self.script)) assert result.returncode == 0, f"Syntax error: {result.stderr}" def test_requires_base_url_argument(self): """Should fail when no URL is provided.""" - result = subprocess.run( - ["bash", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash(str(self.script)) assert result.returncode != 0, "Should fail without URL argument" def test_fails_on_unreachable_url(self): """Should fail when URL is unreachable.""" - result = subprocess.run( - ["bash", str(self.script), "http://localhost:99999/nonexistent", "1", "1"], - capture_output=True, - text=True, + result = _run_bash( + str(self.script), "http://localhost:99999/nonexistent", "1", "1" ) assert result.returncode != 0, "Should fail on unreachable URL" @@ -150,13 +148,7 @@ def test_requires_hf_token_before_running_modal(self, tmp_path): ) env.pop("PE_UK_PRIVATE_HF_READ_TOKEN", None) - result = subprocess.run( - ["bash", str(self.script), "staging"], - capture_output=True, - text=True, - env=env, - check=False, - ) + result = _run_bash(str(self.script), "staging", env=env) assert result.returncode != 0 assert "PE_UK_PRIVATE_HF_READ_TOKEN is required" in result.stderr @@ -180,13 +172,7 @@ def test_syncs_expected_hf_key_before_image_smokes(self, tmp_path): } ) - result = subprocess.run( - ["bash", str(self.script), "staging"], - capture_output=True, - text=True, - env=env, - check=False, - ) + result = _run_bash(str(self.script), "staging", env=env) assert result.returncode == 0, result.stderr calls = uv_calls_log.read_text(encoding="utf-8").splitlines() @@ -268,11 +254,7 @@ def test_script_exists(self): def test_script_syntax(self): """Script should have valid bash syntax.""" - result = subprocess.run( - ["bash", "-n", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(self.script)) assert result.returncode == 0, f"Syntax error: {result.stderr}" def test_generates_success_summary(self, temp_github_step_summary): @@ -280,17 +262,12 @@ def test_generates_success_summary(self, temp_github_step_summary): env = os.environ.copy() env["GITHUB_STEP_SUMMARY"] = temp_github_step_summary - result = subprocess.run( - [ - "bash", - str(self.script), - "success", - "https://beta.example.com", - "success", - "https://prod.example.com", - ], - capture_output=True, - text=True, + result = _run_bash( + str(self.script), + "success", + "https://beta.example.com", + "success", + "https://prod.example.com", env=env, ) @@ -312,17 +289,12 @@ def test_generates_skipped_summary(self, temp_github_step_summary): env = os.environ.copy() env["GITHUB_STEP_SUMMARY"] = temp_github_step_summary - result = subprocess.run( - [ - "bash", - str(self.script), - "skipped", - "", - "success", - "https://prod.example.com", - ], - capture_output=True, - text=True, + result = _run_bash( + str(self.script), + "skipped", + "", + "success", + "https://prod.example.com", env=env, ) @@ -345,20 +317,12 @@ def test_script_exists(self): def test_script_syntax(self): """Script should have valid bash syntax.""" - result = subprocess.run( - ["bash", "-n", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(self.script)) assert result.returncode == 0, f"Syntax error: {result.stderr}" def test_requires_modal_environment_argument(self): """Should fail when no modal environment is provided.""" - result = subprocess.run( - ["bash", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash(str(self.script)) assert result.returncode != 0, "Should fail without modal environment" def test_fails_when_gateway_auth_config_is_partial(self): @@ -371,12 +335,7 @@ def test_fails_when_gateway_auth_config_is_partial(self): env.pop("GATEWAY_AUTH_CLIENT_ID", None) env.pop("GATEWAY_AUTH_CLIENT_SECRET", None) - result = subprocess.run( - ["bash", str(self.script), "staging"], - capture_output=True, - text=True, - env=env, - ) + result = _run_bash(str(self.script), "staging", env=env) assert result.returncode != 0 assert "Gateway auth config is partial." in result.stderr @@ -395,12 +354,7 @@ def test_fails_when_auth_required_but_gateway_auth_vars_missing(self): ): env.pop(key, None) - result = subprocess.run( - ["bash", str(self.script), "staging"], - capture_output=True, - text=True, - env=env, - ) + result = _run_bash(str(self.script), "staging", env=env) assert result.returncode != 0 assert "GATEWAY_AUTH_REQUIRED is enabled" in result.stderr @@ -411,13 +365,7 @@ def test_requires_gcp_credentials(self): env["PE_UK_PRIVATE_HF_READ_TOKEN"] = "hf_test" env.pop("GCP_CREDENTIALS_JSON", None) - result = subprocess.run( - ["bash", str(self.script), "staging"], - capture_output=True, - text=True, - env=env, - check=False, - ) + result = _run_bash(str(self.script), "staging", env=env) assert result.returncode != 0 assert "GCP_CREDENTIALS_JSON is required" in result.stderr @@ -446,12 +394,7 @@ def test_creates_gateway_secret_with_normalized_issuer(self, tmp_path): } ) - result = subprocess.run( - ["bash", str(self.script), "staging"], - capture_output=True, - text=True, - env=env, - ) + result = _run_bash(str(self.script), "staging", env=env) assert result.returncode == 0, result.stderr calls = uv_calls_log.read_text() @@ -498,13 +441,7 @@ def test_gcp_secret_sync_failure_stops_deployment(self, tmp_path): ): env.pop(key, None) - result = subprocess.run( - ["bash", str(self.script), "staging"], - capture_output=True, - text=True, - env=env, - check=False, - ) + result = _run_bash(str(self.script), "staging", env=env) assert result.returncode != 0 calls = uv_calls_log.read_text() @@ -555,12 +492,7 @@ def _run_with_fake_uv( else: env["POLICYENGINE_ARTIFACT_BUCKET"] = bucket - result = subprocess.run( - ["bash", str(self.script), *args], - capture_output=True, - text=True, - env=env, - ) + result = _run_bash(str(self.script), *args, env=env) calls = ( log_path.read_text(encoding="utf-8").splitlines() if log_path.exists() @@ -577,20 +509,12 @@ def test_script_exists(self): def test_script_syntax(self): """Script should have valid bash syntax.""" - result = subprocess.run( - ["bash", "-n", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(self.script)) assert result.returncode == 0, f"Syntax error: {result.stderr}" def test_requires_modal_environment_argument(self): """Should fail when no modal environment is provided.""" - result = subprocess.run( - ["bash", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash(str(self.script)) assert result.returncode != 0, "Should fail without modal environment" def test_requires_artifact_bucket(self, tmp_path): @@ -784,12 +708,7 @@ def _run_with_fake_uv(self, tmp_path, *args, bucket="policyengine-sim-artifacts" else: env["POLICYENGINE_ARTIFACT_BUCKET"] = bucket - result = subprocess.run( - ["bash", str(self.script), *args], - capture_output=True, - text=True, - env=env, - ) + result = _run_bash(str(self.script), *args, env=env) calls = ( log_path.read_text(encoding="utf-8").splitlines() if log_path.exists() @@ -803,11 +722,7 @@ def test_script_exists(self): def test_requires_environment_argument(self): """Should fail with no arguments.""" - result = subprocess.run( - ["bash", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash(str(self.script)) assert result.returncode != 0 def test_requires_manifest_digest_argument(self, tmp_path): @@ -879,20 +794,12 @@ def test_script_exists(self): def test_script_syntax(self): """Script should have valid bash syntax.""" - result = subprocess.run( - ["bash", "-n", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(self.script)) assert result.returncode == 0, f"Syntax error: {result.stderr}" def test_requires_modal_environment_argument(self): """Should fail when no modal environment is provided.""" - result = subprocess.run( - ["bash", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash(str(self.script)) assert result.returncode != 0, "Should fail without modal environment" @@ -907,11 +814,7 @@ def test_script_exists(self): def test_script_syntax(self): """Script should have valid bash syntax.""" - result = subprocess.run( - ["bash", "-n", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(self.script)) assert result.returncode == 0, f"Syntax error: {result.stderr}" @@ -926,29 +829,17 @@ def test_script_exists(self): def test_script_syntax(self): """Script should have valid bash syntax.""" - result = subprocess.run( - ["bash", "-n", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(self.script)) assert result.returncode == 0, f"Syntax error: {result.stderr}" def test_requires_environment_argument(self): """Should fail when no environment is provided.""" - result = subprocess.run( - ["bash", str(self.script)], - capture_output=True, - text=True, - ) + result = _run_bash(str(self.script)) assert result.returncode != 0, "Should fail without environment" def test_requires_base_url_argument(self): """Should fail when no base URL is provided.""" - result = subprocess.run( - ["bash", str(self.script), "beta"], - capture_output=True, - text=True, - ) + result = _run_bash(str(self.script), "beta") assert result.returncode != 0, "Should fail without base URL" def test_fails_when_auth_config_is_partial(self): @@ -959,12 +850,7 @@ def test_fails_when_auth_config_is_partial(self): env.pop("SIMULATION_TEST_AUTH_CLIENT_ID", None) env.pop("SIMULATION_TEST_AUTH_CLIENT_SECRET", None) - result = subprocess.run( - ["bash", str(self.script), "beta", "https://example.com"], - capture_output=True, - text=True, - env=env, - ) + result = _run_bash(str(self.script), "beta", "https://example.com", env=env) assert result.returncode != 0 assert "Simulation integration-test auth config is partial." in result.stderr @@ -981,12 +867,7 @@ def test_fails_when_auth_required_but_auth_vars_missing(self): ): env.pop(key, None) - result = subprocess.run( - ["bash", str(self.script), "beta", "https://example.com"], - capture_output=True, - text=True, - env=env, - ) + result = _run_bash(str(self.script), "beta", "https://example.com", env=env) assert result.returncode != 0 assert "SIMULATION_TEST_AUTH_REQUIRED is enabled" in result.stderr @@ -1018,17 +899,12 @@ def test_exports_us_and_uk_model_versions_to_integration_tests(self, tmp_path): ): env.pop(key, None) - result = subprocess.run( - [ - "bash", - str(self.script), - "prod", - "https://example.com", - "1.690.7", - "2.88.20", - ], - capture_output=True, - text=True, + result = _run_bash( + str(self.script), + "prod", + "https://example.com", + "1.690.7", + "2.88.20", env=env, cwd=REPO_ROOT, ) @@ -1060,11 +936,7 @@ def test_all_scripts_have_strict_mode(self, all_modal_scripts): def test_all_scripts_have_valid_syntax(self, all_modal_scripts): """All scripts should pass bash syntax check.""" for script in all_modal_scripts: - result = subprocess.run( - ["bash", "-n", str(script)], - capture_output=True, - text=True, - ) + result = _run_bash("-n", str(script)) assert result.returncode == 0, ( f"{script.name} has syntax errors: {result.stderr}" ) diff --git a/projects/policyengine-simulation-executor/tests/test_policyengine_dependency_source.py b/projects/policyengine-simulation-executor/tests/test_policyengine_dependency_source.py index 1450fe5b7..7cafeec5c 100644 --- a/projects/policyengine-simulation-executor/tests/test_policyengine_dependency_source.py +++ b/projects/policyengine-simulation-executor/tests/test_policyengine_dependency_source.py @@ -96,6 +96,19 @@ def _modal_import_env() -> dict[str, str]: return env +def _run_python_snippet( + code: str, env: dict[str, str] +) -> subprocess.CompletedProcess[str]: + """Run ``code`` in a fresh interpreter from the project root, capturing text output.""" + return subprocess.run( # noqa: S603 # runs a test-owned snippet with this interpreter + [sys.executable, "-c", code], + cwd=REPO_ROOT, + env=env, + capture_output=True, + text=True, + ) + + def test_modal_app_remote_import_uses_image_version_env(): code = """ import json @@ -124,13 +137,7 @@ def fail(message): """ env = _modal_import_env() env.update(VERSION_ENV) - result = subprocess.run( - [sys.executable, "-c", code], - cwd=REPO_ROOT, - env=env, - capture_output=True, - text=True, - ) + result = _run_python_snippet(code, env) assert result.returncode == 0, result.stderr output = result.stdout.strip() @@ -147,13 +154,7 @@ def test_modal_app_remote_import_fails_clearly_without_version_env(): import src.modal.app """ - result = subprocess.run( - [sys.executable, "-c", code], - cwd=REPO_ROOT, - env=_modal_import_env(), - capture_output=True, - text=True, - ) + result = _run_python_snippet(code, _modal_import_env()) assert result.returncode != 0 assert "POLICYENGINE_VERSION must be set" in result.stderr diff --git a/projects/policyengine-simulation-executor/tests/test_policyengine_package_update_scripts.py b/projects/policyengine-simulation-executor/tests/test_policyengine_package_update_scripts.py index a627dd5a8..21971edfc 100644 --- a/projects/policyengine-simulation-executor/tests/test_policyengine_package_update_scripts.py +++ b/projects/policyengine-simulation-executor/tests/test_policyengine_package_update_scripts.py @@ -23,8 +23,8 @@ def test_update_policyengine_package_script_has_valid_bash_syntax() -> None: - result = subprocess.run( - ["bash", "-n", str(SCRIPT)], + result = subprocess.run( # noqa: S603 # checks the syntax of a repository script + ["bash", "-n", str(SCRIPT)], # noqa: S607 # bash from PATH, as the workflow runs the script capture_output=True, text=True, check=False, @@ -41,7 +41,7 @@ def test_update_policyengine_package_uses_a_separate_python_helper() -> None: assert "python3 -c" not in source assert "python -c" not in source assert "<<'PY'" not in source - result = subprocess.run( + result = subprocess.run( # noqa: S603 # runs the repository helper script with this interpreter [sys.executable, str(helper), "--help"], capture_output=True, text=True, diff --git a/projects/policyengine-simulation-gateway/tests/integration/test_image_smoke_modal.py b/projects/policyengine-simulation-gateway/tests/integration/test_image_smoke_modal.py index f2ceb0be2..ae88bb2bc 100644 --- a/projects/policyengine-simulation-gateway/tests/integration/test_image_smoke_modal.py +++ b/projects/policyengine-simulation-gateway/tests/integration/test_image_smoke_modal.py @@ -16,8 +16,8 @@ @pytest.mark.integration def test_gateway_image_smoke(): env_name = os.environ.get("MODAL_SMOKE_ENV", "staging") - result = subprocess.run( - [ + result = subprocess.run( # noqa: S603 # runs the repository smoke app through uv and Modal + [ # noqa: S607 # uv from PATH, as CI runs it "uv", "run", "modal", From 47e5acd701c69c612b46d9506ef8cd5f5214190f Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:42:49 +0400 Subject: [PATCH 06/16] Fix the remaining ruff S findings Build the GCP metadata request inline from its constant http URL, with the same Metadata-Flavor header, so S310 can see the scheme. Keep every other finding with a reasoned suppression: - S105/S106: fake tokens in tests (one module constant per test module where a value repeats), and names or fingerprints that only look like secrets. - S108: the executor's fixed data directories inside the Modal container, and a lookup path the tests' fakes only pass along. - S110: add S110 to the existing reasoned BLE001 suppressions. - S102: the image tests execute repository-owned app source as Modal's /root/app.py. - S301: the SPM tests round-trip errors they built or raised. - S310: the Hub API request URL is built from the HF_ENDPOINT base and quoted path parts. Co-Authored-By: Claude Opus 5.5 --- .../policyengine_fastapi/opentelemetry/gcp.py | 14 ++++++++++---- .../hf_dataset.py | 4 ++-- .../tests/test_hf_dataset.py | 6 ++++-- .../errors.py | 2 +- .../src/modal/app.py | 2 +- .../hf_access_validation.py | 4 ++-- .../release_bundle.py | 2 +- .../simulation_runtime.py | 4 ++-- .../tests/test_canonical_spm.py | 4 ++-- .../tests/test_hf_access_validation.py | 2 +- .../tests/test_modal_bundle_image.py | 2 +- .../tests/test_modal_scripts.py | 16 +++++++++------- .../tests/test_simulation_output_builder.py | 6 ++++-- .../tests/test_simulation_output_geographic.py | 6 ++++-- .../tests/test_validate_hf_access.py | 4 ++-- .../src/policyengine_simulation_gateway/auth.py | 2 +- .../tests/test_modal_gateway_image.py | 2 +- 17 files changed, 48 insertions(+), 34 deletions(-) diff --git a/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py b/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py index b96ad4f4d..fc2b13c21 100644 --- a/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py +++ b/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py @@ -51,11 +51,17 @@ def _get_project_id() -> str: def _get_project_id_from_metadata() -> str | None: from urllib import request - url = "http://metadata.google.internal/computeMetadata/v1/project/project-id" - req = request.Request(url) - req.add_header("Metadata-Flavor", "Google") try: - return request.urlopen(req).read().decode() + return ( + request.urlopen( + request.Request( + "http://metadata.google.internal/computeMetadata/v1/project/project-id", + headers={"Metadata-Flavor": "Google"}, + ) + ) + .read() + .decode() + ) except Exception as err: # noqa: BLE001 # best-effort probe: any failure falls back to GOOGLE_CLOUD_PROJECT log.info( "Unable to get the google project id from a local metadata service: %s", diff --git a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/hf_dataset.py b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/hf_dataset.py index d76e67da7..49b053282 100644 --- a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/hf_dataset.py +++ b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/hf_dataset.py @@ -83,9 +83,9 @@ def _fetch_hf_dataset_revision( if token is not None: headers["Authorization"] = f"Bearer {token}" - request = Request(url, headers=headers) + request = Request(url, headers=headers) # noqa: S310 # Hub API URL: the HF_ENDPOINT base (https by default) and quoted path parts try: - with urlopen(request, timeout=HF_REQUEST_TIMEOUT_SECONDS) as response: + with urlopen(request, timeout=HF_REQUEST_TIMEOUT_SECONDS) as response: # noqa: S310 # the Hub API request built above return json.loads(response.read().decode("utf-8")) except HTTPError as exc: detail = exc.reason or f"HTTP {exc.code}" diff --git a/libs/policyengine-simulation-contract/tests/test_hf_dataset.py b/libs/policyengine-simulation-contract/tests/test_hf_dataset.py index ab2473134..336d229db 100644 --- a/libs/policyengine-simulation-contract/tests/test_hf_dataset.py +++ b/libs/policyengine-simulation-contract/tests/test_hf_dataset.py @@ -13,6 +13,8 @@ with_hf_revision, ) +RUNTIME_TOKEN = "runtime-token" # noqa: S105 # fake token; the Hub fetch is stubbed + class _FakeResponse: def __init__(self, payload: dict): @@ -75,12 +77,12 @@ def fake_fetch(repo_id, revision, token): return {"siblings": [{"rfilename": "dataset.h5"}]} monkeypatch.setattr(hf_dataset, "_fetch_hf_dataset_revision", fake_fetch) - monkeypatch.setenv("HUGGING_FACE_TOKEN", "runtime-token") + monkeypatch.setenv("HUGGING_FACE_TOKEN", RUNTIME_TOKEN) monkeypatch.setenv("PE_UK_PRIVATE_HF_READ_TOKEN", "deployment-token") monkeypatch.setenv("HF_TOKEN", "legacy-token") assert validate_hf_dataset_uri(dataset_uri) == dataset_uri - assert seen["token"] == "runtime-token" + assert seen["token"] == RUNTIME_TOKEN def test_validate_hf_dataset_uri_ignores_non_runtime_credential_names(monkeypatch): diff --git a/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/errors.py b/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/errors.py index dc3a4f9d1..119c3b3f5 100644 --- a/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/errors.py +++ b/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/errors.py @@ -91,7 +91,7 @@ def log_and_redact_exception( "gateway_error_redacted", attributes=payload, ) - except Exception: # noqa: BLE001 # pragma: no cover - defensive, never raise from logger + except Exception: # noqa: BLE001, S110 # pragma: no cover - defensive, never raise from logger pass return f"{GENERIC_JOB_FAILURE_MESSAGE} (correlation_id={correlation_id})" diff --git a/projects/policyengine-simulation-executor/src/modal/app.py b/projects/policyengine-simulation-executor/src/modal/app.py index bb15a0318..cf27f5645 100644 --- a/projects/policyengine-simulation-executor/src/modal/app.py +++ b/projects/policyengine-simulation-executor/src/modal/app.py @@ -259,7 +259,7 @@ def _configure_modal_observability( def _set_modal_call_attributes(runtime: "ObservabilityRuntime") -> None: try: runtime.set_context(function_call_id=modal.current_function_call_id()) - except Exception: # noqa: BLE001 # telemetry is non-fatal: a failed attribute must never fail the Modal call + except Exception: # noqa: BLE001, S110 # telemetry is non-fatal: a failed attribute must never fail the Modal call pass diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/hf_access_validation.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/hf_access_validation.py index 30358bb9b..d08f89188 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/hf_access_validation.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/hf_access_validation.py @@ -16,8 +16,8 @@ ) HF_RUNTIME_ENV_NAME = "HUGGING_FACE_TOKEN" -HF_MODAL_SECRET_NAME = "pe-uk-private-hf-read-token" -EXPECTED_HF_TOKEN_DISPLAY_NAME = "pe-uk-private-hf-read-token" +HF_MODAL_SECRET_NAME = "pe-uk-private-hf-read-token" # noqa: S105 # the Modal secret's name, not its value +EXPECTED_HF_TOKEN_DISPLAY_NAME = "pe-uk-private-hf-read-token" # noqa: S105 # the token's Hugging Face display name, not its value class HFAccessTarget(BaseModel): diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/release_bundle.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/release_bundle.py index feed3409a..2c2aadf7c 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/release_bundle.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/release_bundle.py @@ -446,7 +446,7 @@ def _receipt_path() -> Path: explicit = os.environ.get("POLICYENGINE_BUNDLE_RECEIPT") if explicit: return Path(explicit) - data_folder = os.environ.get("POLICYENGINE_DATA_FOLDER", "/tmp/policyengine-data") + data_folder = os.environ.get("POLICYENGINE_DATA_FOLDER", "/tmp/policyengine-data") # noqa: S108 # fixed data directory inside the Modal container return Path(data_folder) / BUNDLE_RECEIPT_FILENAME diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_runtime.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_runtime.py index 4d2ddd118..5e3d9d079 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_runtime.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_runtime.py @@ -647,13 +647,13 @@ def resolve_data_folder() -> str: this folder for ``ensure_datasets``/``Simulation.ensure()`` to find, so any divergence from ``_load_dataset``'s resolution strands them. """ - return os.environ.get("POLICYENGINE_DATA_FOLDER", "/tmp/policyengine-data") + return os.environ.get("POLICYENGINE_DATA_FOLDER", "/tmp/policyengine-data") # noqa: S108 # fixed data directory inside the Modal container def _nondefault_data_folder(country: str, name: str, uri: str) -> str: """Separate managed datasets even if their source filenames have the same stem.""" identity = hashlib.sha256(f"{country}:{name}:{uri}".encode()).hexdigest() - return f"/tmp/policyengine-alternate-data/{identity}" + return f"/tmp/policyengine-alternate-data/{identity}" # noqa: S108 # fixed data directory inside the Modal container def _resolve_dataset_selection( diff --git a/projects/policyengine-simulation-executor/tests/test_canonical_spm.py b/projects/policyengine-simulation-executor/tests/test_canonical_spm.py index 11d897bed..c03b418fc 100644 --- a/projects/policyengine-simulation-executor/tests/test_canonical_spm.py +++ b/projects/policyengine-simulation-executor/tests/test_canonical_spm.py @@ -516,7 +516,7 @@ def test_an_unavailable_capability_is_rederived_every_time(self, monkeypatch): ) def test_actual_http_formula_error_contract(monkeypatch, code, observability_runtime): error = SPMInputError(code, "Explicit input is required") - assert pickle.loads(pickle.dumps(error)).to_dict() == error.to_dict() + assert pickle.loads(pickle.dumps(error)).to_dict() == error.to_dict() # noqa: S301 # round-trips an object the test built monkeypatch.setattr( "policyengine_simulation_executor.simulation.run_simulation_impl", Mock(side_effect=error), @@ -834,7 +834,7 @@ def __init__(self, code, message): simulation_runtime.run_simulation_impl( {"country": "us"}, runtime=observability_runtime ) - transported = pickle.loads(pickle.dumps(caught.value)) + transported = pickle.loads(pickle.dumps(caught.value)) # noqa: S301 # round-trips an object the test raised assert transported.to_dict() == {"code": code, "message": "Observed input required"} diff --git a/projects/policyengine-simulation-executor/tests/test_hf_access_validation.py b/projects/policyengine-simulation-executor/tests/test_hf_access_validation.py index 0ef6f8b92..a24940b1f 100644 --- a/projects/policyengine-simulation-executor/tests/test_hf_access_validation.py +++ b/projects/policyengine-simulation-executor/tests/test_hf_access_validation.py @@ -54,7 +54,7 @@ def test_bundle_access_plan_covers_both_private_repositories() -> None: def test_validation_returns_only_safe_identity_and_artifact_metadata() -> None: - token = "private-test-token" + token = "private-test-token" # noqa: S105 # fake token; the loaders are stubbed target = _target() result = validate_uk_private_hf_access( diff --git a/projects/policyengine-simulation-executor/tests/test_modal_bundle_image.py b/projects/policyengine-simulation-executor/tests/test_modal_bundle_image.py index 8d57ad6d6..af5682dd1 100644 --- a/projects/policyengine-simulation-executor/tests/test_modal_bundle_image.py +++ b/projects/policyengine-simulation-executor/tests/test_modal_bundle_image.py @@ -301,7 +301,7 @@ def test_app_module_imports_at_container_entrypoint_path(monkeypatch): ) module = importlib.util.module_from_spec(spec) module.__file__ = "/root/app.py" - exec(code, module.__dict__) + exec(code, module.__dict__) # noqa: S102 # executes repository-owned app source as Modal's /root/app.py assert module.APP_NAME.startswith("policyengine-simulation-py") fetch_calls = [ diff --git a/projects/policyengine-simulation-executor/tests/test_modal_scripts.py b/projects/policyengine-simulation-executor/tests/test_modal_scripts.py index 753a4a80a..9600d5af5 100644 --- a/projects/policyengine-simulation-executor/tests/test_modal_scripts.py +++ b/projects/policyengine-simulation-executor/tests/test_modal_scripts.py @@ -13,6 +13,8 @@ pytest_plugins = ("fixtures.test_modal_scripts",) +FAKE_HF_TOKEN = "hf_test" # noqa: S105 # fake token; the scripts under test run stubbed commands + def _run_bash( *arguments: str, @@ -168,7 +170,7 @@ def test_syncs_expected_hf_key_before_image_smokes(self, tmp_path): { "PATH": f"{fake_bin}:{env['PATH']}", "UV_CALLS_LOG": str(uv_calls_log), - "PE_UK_PRIVATE_HF_READ_TOKEN": "hf_test", + "PE_UK_PRIVATE_HF_READ_TOKEN": FAKE_HF_TOKEN, } ) @@ -178,7 +180,7 @@ def test_syncs_expected_hf_key_before_image_smokes(self, tmp_path): calls = uv_calls_log.read_text(encoding="utf-8").splitlines() assert calls[0] == ( "run modal secret create pe-uk-private-hf-read-token " - "HUGGING_FACE_TOKEN=hf_test --env=staging --force" + f"HUGGING_FACE_TOKEN={FAKE_HF_TOKEN} --env=staging --force" ) assert len(calls) == 4 @@ -328,7 +330,7 @@ def test_requires_modal_environment_argument(self): def test_fails_when_gateway_auth_config_is_partial(self): """Should fail before touching Modal when auth config is partial.""" env = os.environ.copy() - env["PE_UK_PRIVATE_HF_READ_TOKEN"] = "hf_test" + env["PE_UK_PRIVATE_HF_READ_TOKEN"] = FAKE_HF_TOKEN env["GCP_CREDENTIALS_JSON"] = '{"type":"service_account"}' env["GATEWAY_AUTH_ISSUER"] = "https://tenant.auth0.com" env.pop("GATEWAY_AUTH_AUDIENCE", None) @@ -343,7 +345,7 @@ def test_fails_when_gateway_auth_config_is_partial(self): def test_fails_when_auth_required_but_gateway_auth_vars_missing(self): """Required auth must refuse deploy when the GitHub secrets are absent.""" env = os.environ.copy() - env["PE_UK_PRIVATE_HF_READ_TOKEN"] = "hf_test" + env["PE_UK_PRIVATE_HF_READ_TOKEN"] = FAKE_HF_TOKEN env["GCP_CREDENTIALS_JSON"] = '{"type":"service_account"}' env["GATEWAY_AUTH_REQUIRED"] = "1" for key in ( @@ -362,7 +364,7 @@ def test_fails_when_auth_required_but_gateway_auth_vars_missing(self): def test_requires_gcp_credentials(self): """Artifact credential synchronization must fail before deployment.""" env = os.environ.copy() - env["PE_UK_PRIVATE_HF_READ_TOKEN"] = "hf_test" + env["PE_UK_PRIVATE_HF_READ_TOKEN"] = FAKE_HF_TOKEN env.pop("GCP_CREDENTIALS_JSON", None) result = _run_bash(str(self.script), "staging", env=env) @@ -384,7 +386,7 @@ def test_creates_gateway_secret_with_normalized_issuer(self, tmp_path): { "PATH": f"{fake_bin}:{env['PATH']}", "UV_CALLS_LOG": str(uv_calls_log), - "PE_UK_PRIVATE_HF_READ_TOKEN": "hf_test", + "PE_UK_PRIVATE_HF_READ_TOKEN": FAKE_HF_TOKEN, "GCP_CREDENTIALS_JSON": '{"type":"service_account"}', "GATEWAY_AUTH_ISSUER": "https://tenant.auth0.com", "GATEWAY_AUTH_AUDIENCE": "https://simulation-api-beta.policyengine.org", @@ -428,7 +430,7 @@ def test_gcp_secret_sync_failure_stops_deployment(self, tmp_path): { "PATH": f"{fake_bin}:{env['PATH']}", "UV_CALLS_LOG": str(uv_calls_log), - "PE_UK_PRIVATE_HF_READ_TOKEN": "hf_test", + "PE_UK_PRIVATE_HF_READ_TOKEN": FAKE_HF_TOKEN, "GCP_CREDENTIALS_JSON": '{"type":"service_account"}', } ) diff --git a/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py b/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py index 6b157f1fd..4ddfaa513 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py @@ -80,6 +80,8 @@ ) from policyengine_simulation_observability.stages import Stage +CONSTITUENCY_CSV_PATH = "/tmp/constituencies_2024.csv" # noqa: S108 # a path string the fakes pass along; nothing opens it + def _params_with_spm(params, selection): """Return request parameters after the runtime resolves SPM settings.""" @@ -1773,7 +1775,7 @@ def compute(baseline_simulation, reform_simulation, **kwargs): assert baseline_simulation is baseline assert reform_simulation is reform assert kwargs == { - "constituency_csv_path": "/tmp/constituencies_2024.csv", + "constituency_csv_path": CONSTITUENCY_CSV_PATH, "download_missing_assets": False, } return SimpleNamespace(constituency_results=expected) @@ -1787,7 +1789,7 @@ def compute(baseline_simulation, reform_simulation, **kwargs): monkeypatch.setattr( "policyengine_simulation_executor.simulation_output_geographic." "_required_uk_geography_lookup_csv_path", - lambda spec: "/tmp/constituencies_2024.csv", + lambda spec: CONSTITUENCY_CSV_PATH, ) result = _simulation_output_builder( diff --git a/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py b/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py index 74b05af23..18d1442f7 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py @@ -14,6 +14,8 @@ ) from policyengine_simulation_executor import simulation_output_geographic +CONSTITUENCY_CSV_PATH = "/tmp/constituencies_2024.csv" # noqa: S108 # a path string the fakes pass along; nothing opens it + def _complete_constituency_record() -> dict[str, object]: return { @@ -54,7 +56,7 @@ def resolve(spec, **kwargs): assert spec is CONSTITUENCY_ASSET_SPEC assert kwargs == {"download_missing_assets": True} observed.append("lookup-resolved") - return "/tmp/constituencies_2024.csv" + return CONSTITUENCY_CSV_PATH monkeypatch.setattr( "policyengine_simulation_executor.simulation_runtime.setup_gcp_credentials", @@ -69,7 +71,7 @@ def resolve(spec, **kwargs): CONSTITUENCY_ASSET_SPEC ) - assert result == "/tmp/constituencies_2024.csv" + assert result == CONSTITUENCY_CSV_PATH assert observed == ["credentials-entered", "lookup-resolved"] diff --git a/projects/policyengine-simulation-executor/tests/test_validate_hf_access.py b/projects/policyengine-simulation-executor/tests/test_validate_hf_access.py index e74a50439..30a980436 100644 --- a/projects/policyengine-simulation-executor/tests/test_validate_hf_access.py +++ b/projects/policyengine-simulation-executor/tests/test_validate_hf_access.py @@ -12,9 +12,9 @@ def _audit() -> HFCredentialAudit: return HFCredentialAudit( - token_fingerprint="abc123def456", + token_fingerprint="abc123def456", # noqa: S106 # a fake hash prefix, not a token account="policyengine-runtime-data-reader", - token_display_name="pe-uk-private-hf-read-token", + token_display_name="pe-uk-private-hf-read-token", # noqa: S106 # a display name, not a token artifacts=(), ) diff --git a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/auth.py b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/auth.py index f0ed3fb03..e4d042b1f 100644 --- a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/auth.py +++ b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/auth.py @@ -186,7 +186,7 @@ def enforce_production_auth_guard(runtime: ObservabilityRuntime) -> None: "ack_value_present": True, }, ) - except Exception: # noqa: BLE001 # pragma: no cover - observability must never block startup + except Exception: # noqa: BLE001, S110 # pragma: no cover - observability must never block startup pass diff --git a/projects/policyengine-simulation-gateway/tests/test_modal_gateway_image.py b/projects/policyengine-simulation-gateway/tests/test_modal_gateway_image.py index 907630ee0..5d8ac0a84 100644 --- a/projects/policyengine-simulation-gateway/tests/test_modal_gateway_image.py +++ b/projects/policyengine-simulation-gateway/tests/test_modal_gateway_image.py @@ -166,6 +166,6 @@ def test_app_module_imports_at_container_entrypoint_path(monkeypatch): ) module = importlib.util.module_from_spec(spec) module.__file__ = "/root/app.py" - exec(code, module.__dict__) + exec(code, module.__dict__) # noqa: S102 # executes repository-owned app source as Modal's /root/app.py assert module.app.name == "policyengine-simulation-gateway" From f1bfefd278a165dc1dda3a3a0d21fcc6c3e8625a Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:43:03 +0400 Subject: [PATCH 07/16] Enable ruff group 6 (security, logging and debugging leftovers) Select DTZ, ERA, EXE, FIX, G, LOG, S, T20 and TD. The earlier commits on this branch fixed or suppressed, with a reason, every finding outside the shared scripts/rules tool. Co-Authored-By: Claude Opus 5.5 --- ruff.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ruff.toml b/ruff.toml index a24250a3a..628d82e0d 100644 --- a/ruff.toml +++ b/ruff.toml @@ -19,6 +19,8 @@ select = [ "D", # Group 5: exceptions and messages. "BLE", "EM", "TRY", + # Group 6: security, logging and debugging leftovers. + "DTZ", "ERA", "EXE", "FIX", "G", "LOG", "S", "T20", "TD", ] # Mutually exclusive pydocstyle pairs: keep D211 (no blank line before a class # docstring) and D212 (summary on the first line). From 3bfa406753a480265507514d7f7e1271047654d2 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:46:05 +0400 Subject: [PATCH 08/16] Register group 6 and the deployed-test ruff exemption sim.quality.ruff-lint now lists DTZ, ERA, EXE, FIX, G, LOG, S, T20 and TD. The authenticated_tests exemption gets its own rule, since a ruff per-file-ignore rule names one pattern. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 3 ++- changelog_entry.yaml | 1 + docs/generated/rules.md | 19 ++++++++++++------- rules/registry.toml | 21 ++++++++++++++++++++- 4 files changed, 35 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d6c088c79..6af452363 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -140,10 +140,11 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. ### Code quality and tests +- `sim.quality.deployed-tests-ruff-exemptions`: The deployed-service test suite (projects/*/authenticated_tests, pytest run against a live candidate) gets the same ruff exemptions as unit tests (S101, D1, ANN, PLR2004); every other ruff rule applies to it. (blocking: ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/authenticated_tests/**`) - `sim.quality.every-project-tested`: Every project and library with unit tests runs them in the PR test matrix. (doc-only, not checked: follow it yourself) - `sim.quality.ruff-format-src`: Source code under projects/*/src and libs/*/src is formatted with ruff format. (blocking: ruff format) - `sim.quality.ruff-format-tests`: Test code is formatted with ruff format too; CI currently checks only src directories. (doc-only, not checked: follow it yourself) -- `sim.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in ruff.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (ANN, BLE, D, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, TRY, UP, W)) +- `sim.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in ruff.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (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)) - `sim.quality.runtime-evaluated-models-resolve`: Every Pydantic model is runtime-evaluated for ruff: one of its bases, read through the imports of the model's own module, is listed in ruff.toml's runtime-evaluated-base-classes or is a runtime-evaluated class in that same module; and every Pydantic model resolves its annotations once its package is imported. (blocking: pytest `projects/policyengine-simulation-entry/tests/test_runtime_evaluated_annotations.py`, `projects/policyengine-simulation-executor/tests/test_runtime_evaluated_annotations.py`) - `sim.quality.tests-pass`: Each project's unit tests pass in its own locked environment. (blocking: CI job `test`) - `sim.quality.tests-ruff-exemptions`: Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004); every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/tests/**`) diff --git a/changelog_entry.yaml b/changelog_entry.yaml index 4d1de430b..03c3e7cac 100644 --- a/changelog_entry.yaml +++ b/changelog_entry.yaml @@ -4,6 +4,7 @@ - Made CLAUDE.md and the Copilot instructions thin pointers to AGENTS.md, required workflow logic longer than one line to live in tested and shellchecked scripts, and registered the tracking-issue rule - Added a rules registry (rules/registry.toml) with a standard-library tool that validates it, checks every rule is enforced where it claims, and renders docs/generated/rules.md and the Rules block in AGENTS.md changed: + - Enabled ruff's security, logging and debugging rules (S, T20, LOG, G, DTZ, ERA, TD, FIX, EXE); asserts outside tests became explicit checks, command-line scripts keep their printed output, test subprocess calls go through helpers, and every deliberate exception carries a reasoned suppression; the gateway's auth misconfiguration log no longer repeats its message, and the deployed-service tests share the unit-test ruff exemptions - Enabled ruff's exception rules (TRY, EM, BLE); exception messages are built before the raise, raises inside try blocks moved into helpers, and every deliberate catch-all or traceback-free error log carries a reasoned suppression; message text is unchanged, and error types change only in an operator script's two type checks - Enabled ruff's docstring rules (D) and documented every module, class and function outside tests; published contract models and routes now carry these descriptions in their JSON schema and OpenAPI, with no structural schema change - Enabled ruff's annotation rules (ANN) and annotated every function outside tests; test files are exempt from ANN, missing docstrings, magic numbers and bare-assert rules diff --git a/docs/generated/rules.md b/docs/generated/rules.md index f2daf773d..497ac7d09 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -8,11 +8,11 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf | Tier | enforced | ratcheted | advisory | doc-only | Total | | --- | ---: | ---: | ---: | ---: | ---: | -| mechanical | 35 | 1 | 0 | 14 | 50 | +| mechanical | 36 | 1 | 0 | 14 | 51 | | metric | 0 | 0 | 0 | 0 | 0 | | judgment | 0 | 0 | 0 | 0 | 0 | -**Blocking:** 36 of 50 rules. +**Blocking:** 37 of 51 rules. ## Gaps @@ -246,24 +246,27 @@ flowchart LR | Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | | --- | --- | --- | --- | --- | --- | --- | --- | +| `sim.quality.deployed-tests-ruff-exemptions` | The deployed-service test suite (projects/*/authenticated_tests, pytest run against a live candidate) gets the same ruff exemptions as unit tests (S101, D1, ANN, PLR2004); every other ruff rule applies to it. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/authenticated_tests/**` | `pr.yml:lint` | — | | `sim.quality.every-project-tested` | Every project and library with unit tests runs them in the PR test matrix. | mechanical | doc-only | no | nothing | — | — | | `sim.quality.ruff-format-src` | Source code under projects/*/src and libs/*/src is formatted with ruff format. | mechanical | enforced | yes | ruff format | `pr.yml:lint` | — | | `sim.quality.ruff-format-tests` | Test code is formatted with ruff format too; CI currently checks only src directories. | mechanical | doc-only | no | nothing | — | — | -| `sim.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in ruff.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. | mechanical | enforced | yes | ruff check (ANN, BLE, D, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, TRY, UP, W) | `pr.yml:lint` | — | +| `sim.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in ruff.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. | mechanical | enforced | yes | ruff check (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` | — | | `sim.quality.runtime-evaluated-models-resolve` | Every Pydantic model is runtime-evaluated for ruff: one of its bases, read through the imports of the model's own module, is listed in ruff.toml's runtime-evaluated-base-classes or is a runtime-evaluated class in that same module; and every Pydantic model resolves its annotations once its package is imported. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_runtime_evaluated_annotations.py`, `projects/policyengine-simulation-executor/tests/test_runtime_evaluated_annotations.py` | `pr.yml:test` | yes | | `sim.quality.tests-pass` | Each project's unit tests pass in its own locked environment. | mechanical | enforced | yes | CI job `test` | `pr.yml:test` | — | | `sim.quality.tests-ruff-exemptions` | Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004); every other ruff rule applies to tests. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/tests/**` | `pr.yml:lint` | — | ```mermaid flowchart LR + r_sim_quality_deployed_tests_ruff_exemptions("sim.quality.deployed-tests-ruff-exemptions (enforced)") + e_ruff_per_file_ignore_of_ANN__D1__PLR2004__S101_for____authenticated_tests___["ruff per-file ignore of ANN, D1, PLR2004, S101 for **/authenticated_tests/**"] + j_CI_pr_yml_lint["CI pr.yml:lint"] r_sim_quality_every_project_tested("sim.quality.every-project-tested (doc-only)") gap_unenforced>"not enforced"] r_sim_quality_ruff_format_src("sim.quality.ruff-format-src (enforced)") e_ruff_format["ruff format"] - j_CI_pr_yml_lint["CI pr.yml:lint"] r_sim_quality_ruff_format_tests("sim.quality.ruff-format-tests (doc-only)") r_sim_quality_ruff_lint("sim.quality.ruff-lint (enforced)") - e_ruff_check__ANN__BLE__D__E4__E7__E9__EM__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__TRY__UP__W_["ruff check (ANN, BLE, D, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, 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_["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)"] r_sim_quality_runtime_evaluated_models_resolve("sim.quality.runtime-evaluated-models-resolve (enforced)") e_pytest_projects_policyengine_simulation_entry_tests_test_runtime_evaluated_annotations_py__projects_policyengine_simulation_executor_tests_test_runtime_evaluated_annotations_py["pytest projects/policyengine-simulation-entry/tests/test_runtime_evaluated_annotations.py, projects/policyengine-simulation-executor/tests/test_runtime_evaluated_annotations.py"] j_CI_pr_yml_test["CI pr.yml:test"] @@ -271,12 +274,14 @@ flowchart LR e_CI_job_test["CI job test"] r_sim_quality_tests_ruff_exemptions("sim.quality.tests-ruff-exemptions (enforced)") e_ruff_per_file_ignore_of_ANN__D1__PLR2004__S101_for____tests___["ruff per-file ignore of ANN, D1, PLR2004, S101 for **/tests/**"] + r_sim_quality_deployed_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__PLR2004__S101_for____authenticated_tests___ + e_ruff_per_file_ignore_of_ANN__D1__PLR2004__S101_for____authenticated_tests___ --> j_CI_pr_yml_lint r_sim_quality_every_project_tested -.-> gap_unenforced r_sim_quality_ruff_format_src --> e_ruff_format e_ruff_format --> j_CI_pr_yml_lint r_sim_quality_ruff_format_tests -.-> gap_unenforced - r_sim_quality_ruff_lint --> e_ruff_check__ANN__BLE__D__E4__E7__E9__EM__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__TRY__UP__W_ - e_ruff_check__ANN__BLE__D__E4__E7__E9__EM__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__TRY__UP__W_ --> j_CI_pr_yml_lint + r_sim_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_sim_quality_runtime_evaluated_models_resolve --> e_pytest_projects_policyengine_simulation_entry_tests_test_runtime_evaluated_annotations_py__projects_policyengine_simulation_executor_tests_test_runtime_evaluated_annotations_py e_pytest_projects_policyengine_simulation_entry_tests_test_runtime_evaluated_annotations_py__projects_policyengine_simulation_executor_tests_test_runtime_evaluated_annotations_py --> j_CI_pr_yml_test r_sim_quality_tests_pass --> e_CI_job_test diff --git a/rules/registry.toml b/rules/registry.toml index 17cc3f656..a660957b7 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -733,7 +733,7 @@ autofix = true [rule.enforcer] kind = "ruff" mode = "lint" -codes = ["ANN", "BLE", "D", "E4", "E7", "E9", "EM", "F", "FLY", "FURB", "I", "PIE", "PYI", "RET", "RSE", "TC", "TRY", "UP", "W"] +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"] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "lint" @@ -758,6 +758,25 @@ workflow = ".github/workflows/pr.yml" job = "lint" invocation = "ruff check ." +[[rule]] +id = "sim.quality.deployed-tests-ruff-exemptions" +category = "quality" +statement = "The deployed-service test suite (projects/*/authenticated_tests, pytest run against a live candidate) gets the same ruff exemptions as unit tests (S101, D1, ANN, PLR2004); every other ruff rule applies to it." +rationale = "docs/engineering/skills/rules-registry.md#baselines" +scope = ["projects/*/authenticated_tests/**/*.py"] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "per-file-ignore" +pattern = "**/authenticated_tests/**" +codes = ["ANN", "D1", "PLR2004", "S101"] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "lint" +invocation = "ruff check ." + [[rule]] id = "sim.quality.tests-pass" category = "quality" From 081dfd3ae757b4a90a7aedca40ec344f310d0288 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:51:14 +0400 Subject: [PATCH 09/16] Sync the rules tool from the Public API's group 6 The shared scripts/rules tool takes the Public API's group 6 fixes: explicit checks instead of asserts, and reasoned suppressions for its git and shellcheck subprocess calls, prints and calendar dates. The rendered rules page picks up the tool's new hash. Co-Authored-By: Claude Opus 5.5 --- scripts/rules/baseline.py | 8 ++++---- scripts/rules/ci_scripts.py | 12 ++++++++---- scripts/rules/cli.py | 19 ++++++++++++------- scripts/rules/guard.py | 2 +- scripts/rules/tests/test_guard.py | 6 +++--- 5 files changed, 28 insertions(+), 19 deletions(-) diff --git a/scripts/rules/baseline.py b/scripts/rules/baseline.py index 5bc3feb90..4ebe33fac 100644 --- a/scripts/rules/baseline.py +++ b/scripts/rules/baseline.py @@ -28,16 +28,16 @@ def load_base_registry(root: Path, ref: str) -> Registry | None: Raise BaseUnavailable when ``ref`` does not name a commit. """ - verify = subprocess.run( - ["git", "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"], + verify = subprocess.run( # noqa: S603 # runs git with a fixed command and no shell + ["git", "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"], # noqa: S607 # git comes from PATH, as in any checkout the tool runs in cwd=root, capture_output=True, text=True, ) if verify.returncode != 0: raise BaseUnavailable(ref) - shown = subprocess.run( - ["git", "show", f"{ref}:{REGISTRY_PATH.as_posix()}"], + shown = subprocess.run( # noqa: S603 # runs git with a fixed command and no shell + ["git", "show", f"{ref}:{REGISTRY_PATH.as_posix()}"], # noqa: S607 # git comes from PATH, as in any checkout the tool runs in cwd=root, capture_output=True, text=True, diff --git a/scripts/rules/ci_scripts.py b/scripts/rules/ci_scripts.py index 0a0edecd2..e252cea64 100644 --- a/scripts/rules/ci_scripts.py +++ b/scripts/rules/ci_scripts.py @@ -96,7 +96,7 @@ def shellcheck_failures(config: CiScripts, root: Path) -> set[str] | None: ) if not scripts: return set() - result = subprocess.run( + result = subprocess.run( # noqa: S603 # runs the resolved shellcheck on this repository's scripts [shellcheck, "--format=gcc", *scripts], cwd=root, capture_output=True, @@ -149,7 +149,9 @@ def compare(registry: Registry, findings: Findings, root: Path) -> list[Violatio A check that could not run is skipped rather than reported. """ config = registry.ci_scripts - assert config is not None + if config is None: + msg = "the registry has no [ci_scripts] table" + raise TypeError(msg) checks = ( ( "multiline", @@ -197,7 +199,9 @@ def compare(registry: Registry, findings: Findings, root: Path) -> list[Violatio def write_baselines(registry: Registry, findings: Findings, root: Path) -> None: """Record today's findings as the baselines (used once, when adopting the rule).""" config = registry.ci_scripts - assert config is not None + if config is None: + msg = "the registry has no [ci_scripts] table" + raise TypeError(msg) for found, path in ( (findings.multiline, config.multiline_baseline), (findings.untested, config.untested_baseline), @@ -243,5 +247,5 @@ def check() -> list[str]: """Adapter for scripts/run_quality_guards.py.""" violations, notes = run_check(REPO_ROOT) for note in notes: - print(f"note: {note}") + print(f"note: {note}") # noqa: T201 # the guard adapter prints its notes return [str(violation) for violation in violations] diff --git a/scripts/rules/cli.py b/scripts/rules/cli.py index a4134cacb..9df65da56 100644 --- a/scripts/rules/cli.py +++ b/scripts/rules/cli.py @@ -14,12 +14,17 @@ from scripts.rules.validate import validate +def _report(message: str) -> None: + """Print one line of the command's output.""" + print(message) # noqa: T201 # reporting the result is this command's job + + def _print(violations: list, notes: list[str] | None = None) -> int: for note in notes or []: - print(f"note: {note}") + _report(f"note: {note}") for violation in violations: - print(f" - {violation}") - print("rules: failed" if violations else "rules: passed") + _report(f" - {violation}") + _report("rules: failed" if violations else "rules: passed") return 1 if violations else 0 @@ -55,7 +60,7 @@ def main(argv: list[str] | None = None) -> int: ci_scripts.write_baselines( registry, ci_scripts.find(registry.ci_scripts, root), root ) - print("wrote ci_scripts baselines; update the rules' baseline counts") + _report("wrote ci_scripts baselines; update the rules' baseline counts") return 0 violations, notes = ci_scripts.run_check(root) return _print(violations, notes) @@ -68,12 +73,12 @@ def main(argv: list[str] | None = None) -> int: registry, violations = load_registry(root) if registry is None: return _print(violations) - violations += validate(registry, root, date.today()) + violations += validate(registry, root, date.today()) # noqa: DTZ011 # review and expiry dates are calendar days, checked against the local date if args.command == "render": if violations: - print("render refused: fix the registry first") + _report("render refused: fix the registry first") return _print(violations) for path in write_outputs(registry, root): - print(f"wrote {path.relative_to(root)}") + _report(f"wrote {path.relative_to(root)}") return 0 return _print(violations) diff --git a/scripts/rules/guard.py b/scripts/rules/guard.py index 3efa3d4f5..092425003 100644 --- a/scripts/rules/guard.py +++ b/scripts/rules/guard.py @@ -89,7 +89,7 @@ def run_guard( if registry is None: return result - result.violations += validate(registry, root, today or date.today()) + result.violations += validate(registry, root, today or date.today()) # noqa: DTZ011 # review and expiry dates are calendar days, checked against the local date for rule in registry.rules: result.violations += resolve_enforcer(rule, root) result.violations += resolve_ci(rule, root) diff --git a/scripts/rules/tests/test_guard.py b/scripts/rules/tests/test_guard.py index 400a68b7e..731f65d49 100644 --- a/scripts/rules/tests/test_guard.py +++ b/scripts/rules/tests/test_guard.py @@ -589,9 +589,9 @@ def test_compares_against_git_base_ref(self) -> None: repo.write("rules/baselines/ratcheted.txt", "pkg/routes.py\n") repo.render() git = ["git", "-c", "user.email=t@example.com", "-c", "user.name=t"] - subprocess.run(["git", "init", "-q"], cwd=repo.root, check=True) - subprocess.run([*git, "add", "-A"], cwd=repo.root, check=True) - subprocess.run([*git, "commit", "-qm", "base"], cwd=repo.root, check=True) + subprocess.run(["git", "init", "-q"], cwd=repo.root, check=True) # noqa: S607 # git comes from PATH in the temporary test repository + subprocess.run([*git, "add", "-A"], cwd=repo.root, check=True) # noqa: S603 # a fixed git command in the temporary test repository + subprocess.run([*git, "commit", "-qm", "base"], cwd=repo.root, check=True) # noqa: S603 # a fixed git command in the temporary test repository repo.edit("rules/registry.toml", "count = 1", "count = 2") repo.write("rules/baselines/ratcheted.txt", "pkg/routes.py\npkg/other.py\n") repo.render() From 32e77049fcff825856fcc84860bfa81b22b3b3bb Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 14 Oct 2026 20:44:30 +0400 Subject: [PATCH 10/16] Fix the ruff group 6 findings in the code #743 and #744 added #743 and #744 landed on main after this stack branched. Group 6, which this PR enables, flags their new code; each fix follows this PR's conventions: - T201: the routing publisher's new commands report through its _report helper, the package updater's dispatch check through _write_result, and the sentinel CLI gets one _report helper that holds its only print. - S603/S607: the deployment-asset tests route #744's new bash calls through _run_bash, which now takes cwd; the rollback tests and the package-update helper tests get one helper each that holds the only subprocess call, with reasoned suppressions. - S310: the sentinel's HTTP helper opens the deployed entrypoint or the test auth issuer URL that the workflow passes; reasoned suppressions. - S106: a fake bearer token in the sentinel tests. Co-Authored-By: Claude Opus 5.5 --- .../scripts/update_policyengine_package.py | 6 +- .../tests/test_deployment_assets.py | 55 ++++++++------ .../tests/test_simulation_rollback.py | 31 ++++++-- .../src/modal/utils/release_sentinel.py | 19 +++-- .../modal/utils/update_version_registry.py | 36 ++++----- ...est_policyengine_package_update_scripts.py | 75 +++++++++---------- .../tests/test_release_sentinel.py | 2 +- 7 files changed, 126 insertions(+), 98 deletions(-) diff --git a/.github/scripts/update_policyengine_package.py b/.github/scripts/update_policyengine_package.py index fc8d3936b..785d0e96b 100644 --- a/.github/scripts/update_policyengine_package.py +++ b/.github/scripts/update_policyengine_package.py @@ -358,9 +358,9 @@ def verify_dispatch() -> None: ): msg = f"The release dispatch says the UK pair was not promotable ({verdict})" raise SystemExit(msg) - print(f"dispatch_verified={'true' if compared else 'false'}") - print(f"uk_qualification_verdict={verdict}") - print(f"uk_qualification_receipt_sha256={receipt_sha256}") + _write_result(f"dispatch_verified={'true' if compared else 'false'}") + _write_result(f"uk_qualification_verdict={verdict}") + _write_result(f"uk_qualification_receipt_sha256={receipt_sha256}") def _parser() -> argparse.ArgumentParser: diff --git a/projects/policyengine-simulation-entry/tests/test_deployment_assets.py b/projects/policyengine-simulation-entry/tests/test_deployment_assets.py index b5af13c0b..0fd04d522 100644 --- a/projects/policyengine-simulation-entry/tests/test_deployment_assets.py +++ b/projects/policyengine-simulation-entry/tests/test_deployment_assets.py @@ -63,6 +63,7 @@ def _run_bash( check: bool, capture_output: bool = False, env: dict[str, str] | None = None, + cwd: Path | None = None, ) -> subprocess.CompletedProcess[str]: """Run bash on a repository deployment script, with text output.""" return subprocess.run( # noqa: S603 # runs repository scripts with test-built arguments @@ -71,6 +72,7 @@ def _run_bash( capture_output=capture_output, text=True, env=env, + cwd=cwd, ) @@ -1220,12 +1222,16 @@ def test_a_failing_sentinel_fails_its_step(tmp_path, job): script = step["run"].replace( "${{ needs.deploy_entrypoint.outputs.candidate_url }}", "https://candidate" ) - result = subprocess.run( + result = _run_bash( # GitHub runs `run` blocks with bash -e -o pipefail. - ["bash", "--noprofile", "--norc", "-eo", "pipefail", "-c", script], + "--noprofile", + "--norc", + "-eo", + "pipefail", + "-c", + script, check=False, capture_output=True, - text=True, env={ "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", "RUNNER_TEMP": str(tmp_path), @@ -1294,11 +1300,15 @@ def _run_routing_step_with_a_failing_command(tmp_path, step, failed_command, **e plan = runner_temp / "routing-plans" / "routing-plan-prod-1" / "plan.json" plan.parent.mkdir(parents=True) plan.write_text("{}") - result = subprocess.run( - ["bash", "--noprofile", "--norc", "-eo", "pipefail", "-c", step["run"]], + result = _run_bash( + "--noprofile", + "--norc", + "-eo", + "pipefail", + "-c", + step["run"], check=False, capture_output=True, - text=True, env={ "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", "RUNNER_TEMP": str(runner_temp), @@ -1548,19 +1558,15 @@ def test_failed_release_gate_cannot_report_a_successful_release(tmp_path, failed } env[failed_gate] = "failure" # A later step cannot run after this gate under Actions' bash -e behavior. - result = subprocess.run( - [ - "bash", - "-eo", - "pipefail", - "-c", - gate["run"] + '\ntouch "$1"', - "gate", - str(marker), - ], + result = _run_bash( + "-eo", + "pipefail", + "-c", + gate["run"] + '\ntouch "$1"', + "gate", + str(marker), check=False, capture_output=True, - text=True, env=env, cwd=REPOSITORY_ROOT, ) @@ -1591,11 +1597,13 @@ def test_release_success_requires_completed_finalization_for_published_routing( ] if step.get("name") == "Check deployment result" ) - completed = subprocess.run( - ["bash", "-eo", "pipefail", "-c", gate["run"]], + completed = _run_bash( + "-eo", + "pipefail", + "-c", + gate["run"], check=False, capture_output=True, - text=True, cwd=REPOSITORY_ROOT, env={ "PATH": os.environ["PATH"], @@ -1822,11 +1830,12 @@ def _run_identity_smoke(tmp_path: Path, versions: dict, **expected: str): "FAKE_CURL_VERSIONS": str(versions_file), **expected, } - return subprocess.run( - ["bash", SMOKE_SCRIPT, "https://simulation.example.test", revision], + return _run_bash( + SMOKE_SCRIPT, + "https://simulation.example.test", + revision, check=False, capture_output=True, - text=True, env=env, ) diff --git a/projects/policyengine-simulation-entry/tests/test_simulation_rollback.py b/projects/policyengine-simulation-entry/tests/test_simulation_rollback.py index 972e43b3d..ddb099729 100644 --- a/projects/policyengine-simulation-entry/tests/test_simulation_rollback.py +++ b/projects/policyengine-simulation-entry/tests/test_simulation_rollback.py @@ -30,12 +30,27 @@ ) +def _run_bash( + *arguments: str | Path, + check: bool, + capture_output: bool = False, + env: dict[str, str] | None = None, +) -> subprocess.CompletedProcess[str]: + """Run bash on a repository rollback script or step, with text output.""" + return subprocess.run( # noqa: S603 # runs repository scripts with test-built arguments + ["bash", *arguments], # noqa: S607 # bash from PATH, as the workflows run these scripts + check=check, + capture_output=capture_output, + text=True, + env=env, + ) + + def _validate(**env: str) -> subprocess.CompletedProcess[str]: - return subprocess.run( - ["bash", VALIDATOR], + return _run_bash( + VALIDATOR, check=False, capture_output=True, - text=True, env={"PATH": os.environ["PATH"], **env}, ) @@ -306,11 +321,13 @@ def _run_restore( bin_dir = tmp_path / "bin" bin_dir.mkdir() _fake_gh(bin_dir, attempts, kept, inventory) - return subprocess.run( - ["bash", "-eo", "pipefail", "-c", _restore_script()], + return _run_bash( + "-eo", + "pipefail", + "-c", + _restore_script(), check=False, capture_output=True, - text=True, env={ "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", "RUNNER_TEMP": str(runner_temp), @@ -400,5 +417,5 @@ def test_rollbacks_share_the_deployment_concurrency_group(): def test_validator_has_valid_shell_syntax_and_is_executable(): - subprocess.run(["bash", "-n", VALIDATOR], check=True) + _run_bash("-n", VALIDATOR, check=True) assert os.access(VALIDATOR, os.X_OK) diff --git a/projects/policyengine-simulation-executor/src/modal/utils/release_sentinel.py b/projects/policyengine-simulation-executor/src/modal/utils/release_sentinel.py index ea776a9ad..9563b0107 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/release_sentinel.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/release_sentinel.py @@ -184,14 +184,14 @@ def _http( timeout: float = 60, ) -> tuple[int, Mapping[str, str], Any]: data = json.dumps(body).encode() if body is not None else None - request = urllib.request.Request(url, data=data, method=method) + request = urllib.request.Request(url, data=data, method=method) # noqa: S310 # the deployed entrypoint or the test auth issuer, from the workflow request.add_header("Accept", "application/json") if data is not None: request.add_header("Content-Type", "application/json") if token: request.add_header("Authorization", f"Bearer {token}") try: - with urllib.request.urlopen(request, timeout=timeout) as response: + with urllib.request.urlopen(request, timeout=timeout) as response: # noqa: S310 # the request built above status, headers, raw = response.status, response.headers, response.read() except urllib.error.HTTPError as error: status, headers, raw = error.code, error.headers, error.read() @@ -323,6 +323,11 @@ def check_deployment( return problems +def _report(line: str, *, error: bool = False) -> None: + """Print one line of the command's report, to stderr for a failure.""" + print(line, file=sys.stderr if error else sys.stdout) # noqa: T201 # command-line script: stdout and stderr are its report + + def main(argv: list[str] | None = None) -> int: """Run the ``expected`` or ``check`` command and return its exit status.""" parser = argparse.ArgumentParser(description=__doc__.split("\n\n")[0]) @@ -340,7 +345,7 @@ def main(argv: list[str] | None = None) -> int: with open(args.output, "w") as handle: json.dump(expected, handle, indent=2, sort_keys=True) for case in expected["cases"]: - print(f"{case['case_id']} ({case['seconds']}s): {case['values']}") + _report(f"{case['case_id']} ({case['seconds']}s): {case['values']}") return 0 with open(args.expected) as handle: @@ -352,13 +357,11 @@ def main(argv: list[str] | None = None) -> int: timeout_seconds=args.timeout_seconds, ) if problems: - print( - "The deployed API does not reproduce the bundle's numbers:", file=sys.stderr - ) + _report("The deployed API does not reproduce the bundle's numbers:", error=True) for problem in problems: - print(f" {problem}", file=sys.stderr) + _report(f" {problem}", error=True) return 1 - print( + _report( f"{args.base_url} reproduces {len(expected['cases'])} sentinel case(s) for " f"policyengine {expected['policyengine_version']}, UK dataset " f"{expected['uk']['dataset_sha256']}" diff --git a/projects/policyengine-simulation-executor/src/modal/utils/update_version_registry.py b/projects/policyengine-simulation-executor/src/modal/utils/update_version_registry.py index c1265f2e5..35bc087c7 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/update_version_registry.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/update_version_registry.py @@ -1245,17 +1245,17 @@ def active_state() -> object: return snapshot.get(ROUTING_STATE_ACTIVE_KEY) if args.print_latest: - print(read_latest_policyengine_version(environment=args.environment) or "") + _report(read_latest_policyengine_version(environment=args.environment) or "") return if args.print_identity: for key, value in latest_release_identity(active_state()).items(): - print(f"{key}={value}") + _report(f"{key}={value}") return if args.print_pending_plan: store = _routing_store(args.environment, create_if_missing=False) - print(json.dumps(_unfinished_publication(store), indent=2, sort_keys=True)) + _report(json.dumps(_unfinished_publication(store), indent=2, sort_keys=True)) return writes_routing = ( @@ -1284,10 +1284,10 @@ def active_state() -> object: store = _routing_store(args.environment, create_if_missing=False) pending = _unfinished_publication(store) if pending is None: - print("No pending routing publication; routing is unchanged.") + _report("No pending routing publication; routing is unchanged.") else: - print("Pending routing publication:") - print( + _report("Pending routing publication:") + _report( json.dumps(store.get(ROUTING_PUBLICATION_KEY), indent=2, sort_keys=True) ) outcome = finalize_routing_publication( @@ -1297,10 +1297,10 @@ def active_state() -> object: publication_run=args.publication_run, ) if outcome == "foreign-publication": - print( + _report( "Pending routing publication belongs to another run or environment; routing is unchanged." ) - print(f"Routing publication finalization: {outcome}") + _report(f"Routing publication finalization: {outcome}") if args.github_output: with open(args.github_output, "a") as handle: handle.write(f"outcome={outcome}\n") @@ -1311,7 +1311,7 @@ def active_state() -> object: plan = json.load(handle) operation = record_routing_plan if args.record_plan else complete_routing_plan operation(environment=args.environment, plan=plan) - print( + _report( f"Routing publication {plan['publication_id']}: " f"{'pending' if args.record_plan else 'completed'}" ) @@ -1324,7 +1324,7 @@ def active_state() -> object: environment=args.environment, plan=plan, ) - print( + _report( f"Published policyengine {plan['policyengine_version']} routing " f"({plan['after_sha256']}) in Modal environment {args.environment}" ) @@ -1339,7 +1339,7 @@ def active_state() -> object: environment=args.environment, plans=plans, ) - print( + _report( f"Routing for policyengine {plans[-1]['policyengine_version']}: " f"{outcome} in Modal environment {args.environment}" ) @@ -1351,10 +1351,10 @@ def active_state() -> object: policyengine_version=args.restore_latest, expected_latest=args.expected_latest, ) - print(f"Restored routing state in Modal environment: {args.environment}") - print(f" latest.policyengine: {state['latest']['policyengine']}") - print(f" latest.us: {state['latest']['us']}") - print(f" latest.uk: {state['latest']['uk']}") + _report(f"Restored routing state in Modal environment: {args.environment}") + _report(f" latest.policyengine: {state['latest']['policyengine']}") + _report(f" latest.us: {state['latest']['us']}") + _report(f" latest.uk: {state['latest']['uk']}") return if args.expected_latest: parser.error("--expected-latest needs --restore-latest") @@ -1398,9 +1398,9 @@ def active_state() -> object: if args.plan_out: with open(args.plan_out, "w") as handle: json.dump(plan, handle, indent=2, sort_keys=True) - print(f"before_sha256={plan['before_sha256']}") - print(f"after_sha256={plan['after_sha256']}") - print(f"advances_latest={'true' if plan['advances_latest'] else 'false'}") + _report(f"before_sha256={plan['before_sha256']}") + _report(f"after_sha256={plan['after_sha256']}") + _report(f"advances_latest={'true' if plan['advances_latest'] else 'false'}") return _report(f"Publishing routing state in Modal environment: {args.environment}") diff --git a/projects/policyengine-simulation-executor/tests/test_policyengine_package_update_scripts.py b/projects/policyengine-simulation-executor/tests/test_policyengine_package_update_scripts.py index 21971edfc..835239f39 100644 --- a/projects/policyengine-simulation-executor/tests/test_policyengine_package_update_scripts.py +++ b/projects/policyengine-simulation-executor/tests/test_policyengine_package_update_scripts.py @@ -403,15 +403,30 @@ def test_update_policyengine_package_rejects_direct_component_requirements( ) +def _run_helper( + command: str, + *, + check: bool = False, + input_text: str | None = None, + env: dict[str, str] | None = None, +) -> subprocess.CompletedProcess[str]: + """Run one command of the repository's Python update helper.""" + return subprocess.run( # noqa: S603 # runs the repository helper script with this interpreter + [sys.executable, str(HELPER), command], + input=input_text, + capture_output=True, + text=True, + check=check, + env=env, + ) + + def _verify(payload: str) -> subprocess.CompletedProcess[str]: import os - return subprocess.run( - [sys.executable, str(HELPER), "verify-dispatch"], - input=INSTALLED, - capture_output=True, - text=True, - check=False, + return _run_helper( + "verify-dispatch", + input_text=INSTALLED, env={**os.environ, "RELEASE_PAYLOAD": payload}, ) @@ -476,12 +491,9 @@ def test_verify_dispatch_compares_the_certificate_when_installed_reports_one() - ) def verify(payload: str) -> subprocess.CompletedProcess[str]: - return subprocess.run( - [sys.executable, str(HELPER), "verify-dispatch"], - input=installed, - capture_output=True, - text=True, - check=False, + return _run_helper( + "verify-dispatch", + input_text=installed, env={**os.environ, "RELEASE_PAYLOAD": payload}, ) @@ -496,12 +508,9 @@ def test_verify_dispatch_fails_closed_when_the_installed_bundle_cannot_answer() import os installed = INSTALLED.replace("uk_dataset_sha256=" + "a" * 64, "uk_dataset_sha256=") - result = subprocess.run( - [sys.executable, str(HELPER), "verify-dispatch"], - input=installed, - capture_output=True, - text=True, - check=False, + result = _run_helper( + "verify-dispatch", + input_text=installed, env={**os.environ, "RELEASE_PAYLOAD": _payload()}, ) assert result.returncode != 0 @@ -589,10 +598,8 @@ def test_bundle_versions_matches_policyengine_release_identity() -> None: bundle = get_current_bundle() record = bundle["data_releases"]["uk"] - output = subprocess.run( - [sys.executable, str(HELPER), "bundle-versions"], - capture_output=True, - text=True, + output = _run_helper( + "bundle-versions", check=True, ).stdout installed = dict(line.split("=", 1) for line in output.splitlines() if "=" in line) @@ -608,12 +615,9 @@ def test_bundle_versions_matches_policyengine_release_identity() -> None: } }, } - result = subprocess.run( - [sys.executable, str(HELPER), "verify-dispatch"], - input=output, - capture_output=True, - text=True, - check=False, + result = _run_helper( + "verify-dispatch", + input_text=output, env={**os.environ, "RELEASE_PAYLOAD": json.dumps(payload)}, ) assert result.returncode == 0, result.stderr @@ -641,10 +645,8 @@ def test_bundle_versions_reports_the_receipt_certificate_digest(tmp_path) -> Non "sys.modules['policyengine.provenance.receipt'] = module\n" ) env = {**os.environ, "PYTHONPATH": str(stand_in)} - output = subprocess.run( - [sys.executable, str(HELPER), "bundle-versions"], - capture_output=True, - text=True, + output = _run_helper( + "bundle-versions", check=True, env=env, ).stdout @@ -657,12 +659,9 @@ def test_bundle_versions_reports_the_receipt_certificate_digest(tmp_path) -> Non record = get_current_bundle()["data_releases"]["uk"] expected = hashlib.sha256(json.dumps(record, sort_keys=True).encode()).hexdigest() assert installed["uk_certificate_sha256"] == expected - mismatch = subprocess.run( - [sys.executable, str(HELPER), "verify-dispatch"], - input=output, - capture_output=True, - text=True, - check=False, + mismatch = _run_helper( + "verify-dispatch", + input_text=output, env={ **os.environ, "RELEASE_PAYLOAD": json.dumps( diff --git a/projects/policyengine-simulation-executor/tests/test_release_sentinel.py b/projects/policyengine-simulation-executor/tests/test_release_sentinel.py index 3d0c125a5..bf52aa4da 100644 --- a/projects/policyengine-simulation-executor/tests/test_release_sentinel.py +++ b/projects/policyengine-simulation-executor/tests/test_release_sentinel.py @@ -187,7 +187,7 @@ def _check(api, **kwargs): problems = sentinel.check_deployment( "https://candidate.example/", expected, - token="t0ken", + token="t0ken", # noqa: S106 # fake bearer token; the HTTP calls are stubbed http=api, sleep=sleeps.append, **kwargs, From 8d5850f8b41fc0d8ab45be27d6461e4c89f4603b Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 15 Oct 2026 03:01:54 +0400 Subject: [PATCH 11/16] Add S110 to the cancel suppression #795 added #795 landed on main after this stack branched. S110, which this PR enables, flags the try-except-pass in its _cancel_call, which now holds the best-effort cancel loop this PR suppressed; like the other reasoned BLE001 suppressions, it gains S110. Co-Authored-By: Claude Opus 5.5 --- .../src/modal/segmented_national.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/policyengine-simulation-executor/src/modal/segmented_national.py b/projects/policyengine-simulation-executor/src/modal/segmented_national.py index f01a0b577..d66b0775a 100644 --- a/projects/policyengine-simulation-executor/src/modal/segmented_national.py +++ b/projects/policyengine-simulation-executor/src/modal/segmented_national.py @@ -524,7 +524,7 @@ def _fail_if_budget_exhausted( def _cancel_call(call: modal.FunctionCall) -> None: try: call.cancel() - except Exception: # noqa: BLE001 # best-effort cleanup: a failed cancel must not mask the failure being raised, and superseded calls are cancelled again later + except Exception: # noqa: BLE001, S110 # best-effort cleanup: a failed cancel must not mask the failure being raised, and superseded calls are cancelled again later pass def _cancel_superseded_children(self) -> None: From fa94a3b8cd59390a6c11284793effdd627dbb795 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 15 Oct 2026 03:02:11 +0400 Subject: [PATCH 12/16] Fix the ruff group 6 findings in the code #791 added #791 landed on main after this stack branched. S105, which this PR enables, flags two values that only look like secrets, and each keeps a reasoned suppression: the redaction pattern for secret-looking key names in the Stage 12 failure-cause module, and the fake database password its tests check redaction removes. Co-Authored-By: Claude Opus 5.5 --- .../stage12_runtime/failure_causes.py | 2 +- .../tests/test_stage12_failure_causes.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/failure_causes.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/failure_causes.py index 9f7baa350..1fa12f85c 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/failure_causes.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/failure_causes.py @@ -86,7 +86,7 @@ ) _BEARER_TOKEN = re.compile(r"(?i)\bbearer\s+[A-Za-z0-9._~+/=-]+") _SECRET_KEY = ( - r"[\w-]*(?:password|passwd|pwd|secret|token|api[_-]?key|access[_-]?key" + r"[\w-]*(?:password|passwd|pwd|secret|token|api[_-]?key|access[_-]?key" # noqa: S105 # a pattern for secret-looking key names, not a secret r"|private[_-]?key|credentials?|signature|sig)" ) _SECRET_ASSIGNMENT = re.compile( diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_failure_causes.py b/projects/policyengine-simulation-executor/tests/test_stage12_failure_causes.py index b6636d9a5..03e37f4c1 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_failure_causes.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_failure_causes.py @@ -20,7 +20,7 @@ from pydantic import BaseModel, TypeAdapter, ValidationError from sqlalchemy.exc import IntegrityError, OperationalError -DATABASE_PASSWORD = "pooler-password-value" +DATABASE_PASSWORD = "pooler-password-value" # noqa: S105 # fake password; the tests check that redaction removes it DATABASE_URL = ( f"postgresql://postgres.project:{DATABASE_PASSWORD}" "@pooler.example.com:5432/postgres" From c21c2be8cabead2cc6a9d0e66f5265cdd572942e Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 01:00:32 +0400 Subject: [PATCH 13/16] Fix the ruff group 6 findings in the code #758 added #758 landed on main after this stack branched. S603, which this PR enables, flags the two new deployment tests that run the Cloud Run deploy script directly to check permission enforcement defaults and rejections. They carry the same reasoned suppression as the existing direct run of that script. Co-Authored-By: Claude Opus 5.5 --- .../tests/test_deployment_assets.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/projects/policyengine-simulation-entry/tests/test_deployment_assets.py b/projects/policyengine-simulation-entry/tests/test_deployment_assets.py index 0fd04d522..32966f711 100644 --- a/projects/policyengine-simulation-entry/tests/test_deployment_assets.py +++ b/projects/policyengine-simulation-entry/tests/test_deployment_assets.py @@ -548,7 +548,7 @@ def test_cloud_run_deployment_defaults_permission_enforcement_to_log_only( configured ) - subprocess.run( + subprocess.run( # noqa: S603 # runs the repository deploy script directly, as the workflow does [CLOUD_RUN_DEPLOY_SCRIPT], check=True, env=command_environment, @@ -568,7 +568,7 @@ def test_cloud_run_deployment_rejects_invalid_permission_enforcement( command_environment = _cloud_run_deploy_environment(tmp_path) command_environment["SIMULATION_ENTRYPOINT_ENFORCE_PERMISSIONS_VALUE"] = configured - result = subprocess.run( + result = subprocess.run( # noqa: S603 # runs the repository deploy script directly, as the workflow does [CLOUD_RUN_DEPLOY_SCRIPT], check=False, env=command_environment, From fa3df589bc8e9265bd36eadccb10bb0a546e0fa4 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 01:00:32 +0400 Subject: [PATCH 14/16] Fix the ruff group 6 findings in the code #781 added #781 landed on main after this stack branched. Group 6, which this PR enables, flags two of its additions: - S110: the Modal app's stage-duration telemetry swallows a failed set_context, so its suppression adds S110 beside BLE001, as the call attributes helper above it does; - DTZ001: the contract test that the job record rejects a naive time builds that naive time on purpose, so it carries a reasoned suppression. Co-Authored-By: Claude Opus 5.5 --- .../policyengine-simulation-contract/tests/test_job_progress.py | 2 +- projects/policyengine-simulation-executor/src/modal/app.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/policyengine-simulation-contract/tests/test_job_progress.py b/libs/policyengine-simulation-contract/tests/test_job_progress.py index d45335c58..63cc30051 100644 --- a/libs/policyengine-simulation-contract/tests/test_job_progress.py +++ b/libs/policyengine-simulation-contract/tests/test_job_progress.py @@ -341,7 +341,7 @@ def test__record_without_attempts_counts_one_attempt(self): def test__record_requires_timezone_aware_times(self): with pytest.raises(ValidationError): - _record(started_at=datetime(2026, 10, 13, 12, 0, 30)) + _record(started_at=datetime(2026, 10, 13, 12, 0, 30)) # noqa: DTZ001 # the naive time the record must reject class TestJobProgressState: diff --git a/projects/policyengine-simulation-executor/src/modal/app.py b/projects/policyengine-simulation-executor/src/modal/app.py index cf27f5645..97734e86b 100644 --- a/projects/policyengine-simulation-executor/src/modal/app.py +++ b/projects/policyengine-simulation-executor/src/modal/app.py @@ -314,7 +314,7 @@ def _record_stage_durations( for stage, seconds in durations.items() }, ) - except Exception: # noqa: BLE001 # telemetry is non-fatal + except Exception: # noqa: BLE001, S110 # telemetry is non-fatal pass From 0964c80188f0827178dbce7a50e67b1e15142060 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 09:44:12 +0400 Subject: [PATCH 15/16] Allow file-level ruff exceptions only in tests, CI/CD and command-line scripts A ruff per-file-ignores entry switches rules off for whole files. That is now acceptable where it's the norm: tests, CI/CD scripts under .github, and command-line scripts (under a scripts directory, or modules run through if __name__ == "__main__"). Everywhere else a kept finding still needs an inline suppression with its reason. The rules guard already requires a per-file-ignore rule for every entry. It now also expands each entry's pattern the way ruff does (globset syntax, and a slash-less pattern also matches file names) and fails when the pattern covers any other file, is negated, or matches nothing. scripts/rules/file_exceptions.py holds the definitions, and the registry doc describes the policy. Co-Authored-By: Claude Opus 5.5 --- docs/engineering/skills/rules-registry.md | 26 +++- scripts/rules/file_exceptions.py | 160 ++++++++++++++++++++++ scripts/rules/resolve.py | 47 +++++++ scripts/rules/tests/test_guard.py | 88 ++++++++++++ 4 files changed, 319 insertions(+), 2 deletions(-) create mode 100644 scripts/rules/file_exceptions.py diff --git a/docs/engineering/skills/rules-registry.md b/docs/engineering/skills/rules-registry.md index b4b843a05..a8ee8c544 100644 --- a/docs/engineering/skills/rules-registry.md +++ b/docs/engineering/skills/rules-registry.md @@ -69,7 +69,7 @@ gap is visible, rather than leaving it out. | --- | --- | --- | | `pytest` | `nodes` (`path::test` or `path::Class::test`), optional `project` (sub-project directory the paths are relative to) | Each test exists, and the CI job's command collects it | | `quality-guard` | `name`, optional `runner` | The name is in `GUARDS` in `scripts/run_quality_guards.py` | -| `ruff` | `mode` = `format`, `lint` (with `codes`) or `per-file-ignore` (with `pattern`, `codes`) | Codes are selected / the ignore entry exists | +| `ruff` | `mode` = `format`, `lint` (with `codes`) or `per-file-ignore` (with `pattern`, `codes`) | Codes are selected / the ignore entry exists and covers only [files allowed file-level exceptions](#file-level-exceptions) | | `mypy` | `paths` | Each path is in `[tool.mypy].files` | | `workflow-job` | optional `step` | The CI job exists (the job itself is the check) | | `pyright`, `import-linter` | `config` (+ `contract` for Import Linter) | The config file exists | @@ -117,13 +117,35 @@ fail the guard yet. - **Ruff is never ratcheted.** Adopting a ruff rule family is one standalone PR that changes the select list and fixes every resulting violation. Don't use - `--add-noqa` or blanket per-file ignores. + `--add-noqa`. A finding that is kept gets an inline `# noqa: CODE # reason`, + except in the files below. - Other enforcers (mypy, Pyright, Import Linter, custom guards) may start `ratcheted` with a baseline file of tolerated violations. Baselines may only shrink: the guard compares counts against the base branch. - Moving a rule from enforced/ratcheted to advisory/doc-only needs an explicit `[rule.downgrade] reason`. +## File-level exceptions + +A ruff `per-file-ignores` entry switches rules off for whole files. It is +allowed only where that's the norm rather than an exception to it: + +- **tests**: files under a `tests` or `authenticated_tests` directory; +- **CI/CD scripts**: files under `.github`; +- **command-line scripts**: files under a `scripts` directory, or modules that + run as a program through `if __name__ == "__main__":`. + +Everywhere else, keep a finding only with an inline suppression and its reason. + +Each entry needs a `per-file-ignore` rule (see below). The guard expands the +entry's pattern like ruff does and fails when the pattern: + +- covers any other file; +- is negated; +- matches nothing. + +`scripts/rules/file_exceptions.py` holds these definitions. + ## Hidden rules The guard fails when something enforces a rule that the registry doesn't diff --git a/scripts/rules/file_exceptions.py b/scripts/rules/file_exceptions.py new file mode 100644 index 000000000..2bca805b4 --- /dev/null +++ b/scripts/rules/file_exceptions.py @@ -0,0 +1,160 @@ +"""Where file-level lint exceptions are allowed. + +A ruff ``per-file-ignores`` entry switches rules off for whole files, so it is +allowed only for files where that is the norm rather than an exception to it: + +- **tests**: files under a ``tests`` or ``authenticated_tests`` directory; +- **CI/CD scripts**: files under ``.github``; +- **command-line scripts**: files under a ``scripts`` directory, or Python + modules that run as a program through ``if __name__ == "__main__":``. + +Everywhere else a kept finding gets an inline suppression with its reason. See +docs/engineering/skills/rules-registry.md#file-level-exceptions. +""" + +from __future__ import annotations + +import ast +import os +import re +from functools import cache +from pathlib import Path + +TEST_DIRECTORIES = frozenset({"tests", "authenticated_tests"}) +CI_DIRECTORY = ".github" +SCRIPT_DIRECTORY = "scripts" +LINTED_SUFFIXES = (".py", ".pyi") +# Directories that hold no source of this repository. +SKIPPED_DIRECTORIES = frozenset( + { + ".git", + ".venv", + "venv", + "node_modules", + "__pycache__", + ".mypy_cache", + ".ruff_cache", + } +) + + +# One piece of a ruff (globset) pattern: a wildcard, a character class, an +# alternation character, or a run of literal characters. +GLOB_PIECE = re.compile(r"\*\*/|\*\*|\*|\?|\[!?[^\]]*\]|[{},\[]|[^*?\[{},]+") +# As in ruff, "*" and "?" also match "/". +WILDCARDS = {"**/": "(?:.*/)?", "**": ".*", "*": ".*", "?": "."} + + +def _character_class(piece: str) -> str: + if piece == "[": + return re.escape(piece) + body = piece[1:-1].replace("\\", "\\\\") + if body.startswith("!"): + body = "^" + body[1:] + return f"[{body}]" + + +def glob_regex(pattern: str) -> re.Pattern[str]: + """Translate a ruff (globset) file pattern into a regex over relative paths. + + Supports ``**``, ``*``, ``?``, ``[...]`` and ``{a,b}`` alternatives. As in + ruff, ``*`` and ``?`` also match ``/``, so ``pkg/*.py`` covers + ``pkg/sub/module.py`` too. + """ + parts = [] + open_alternations = 0 + for piece in GLOB_PIECE.findall(pattern): + if piece in WILDCARDS: + parts.append(WILDCARDS[piece]) + elif piece.startswith("["): + parts.append(_character_class(piece)) + elif piece == "{": + parts.append("(?:") + open_alternations += 1 + elif piece == "}" and open_alternations: + parts.append(")") + open_alternations -= 1 + elif piece == "," and open_alternations: + parts.append("|") + else: + parts.append(re.escape(piece)) + return re.compile("".join(parts) + "$") + + +@cache +def linted_files(root: Path) -> tuple[str, ...]: + """Return the repository's Python files as sorted root-relative POSIX paths.""" + found = [] + for directory, subdirectories, files in os.walk(root): + subdirectories[:] = [ + name + for name in subdirectories + if name not in SKIPPED_DIRECTORIES + and (not name.startswith(".") or name == CI_DIRECTORY) + ] + found.extend( + Path(directory, name).relative_to(root).as_posix() + for name in files + if name.endswith(LINTED_SUFFIXES) + ) + return tuple(sorted(found)) + + +def matching_files(root: Path, pattern: str) -> list[str]: + """Return the files a ruff ``per-file-ignores`` pattern applies to. + + Like ruff, a pattern matches either a file's path or its name. + """ + regex = glob_regex(pattern) + return [ + path + for path in linted_files(root) + if regex.match(path) or regex.match(path.rsplit("/", 1)[-1]) + ] + + +def runs_as_program(root: Path, path: str) -> bool: + """Return whether a module has a top-level ``if __name__ == "__main__":``.""" + try: + tree = ast.parse((root / path).read_text(encoding="utf-8")) + except (OSError, SyntaxError, UnicodeDecodeError): + return False + for node in tree.body: + if ( + isinstance(node, ast.If) + and isinstance(node.test, ast.Compare) + and isinstance(node.test.left, ast.Name) + and node.test.left.id == "__name__" + and any( + isinstance(comparator, ast.Constant) and comparator.value == "__main__" + for comparator in node.test.comparators + ) + ): + return True + return False + + +def allows_file_level_exceptions(root: Path, path: str) -> bool: + """Return whether a file is a test, a CI/CD script or a command-line script.""" + directories = path.split("/")[:-1] + return ( + any(directory in TEST_DIRECTORIES for directory in directories) + or (bool(directories) and directories[0] == CI_DIRECTORY) + or SCRIPT_DIRECTORY in directories + or runs_as_program(root, path) + ) + + +def out_of_scope_files(root: Path, pattern: str) -> list[str] | None: + """Return the files a pattern covers that may not take file-level exceptions. + + ``None`` means the pattern can't be checked: it is negated, so it covers + every file it doesn't name. + """ + if pattern.startswith("!"): + return None + return [ + path + for path in matching_files(root, pattern) + if not allows_file_level_exceptions(root, path) + ] diff --git a/scripts/rules/resolve.py b/scripts/rules/resolve.py index ed2bba559..d00d15e91 100644 --- a/scripts/rules/resolve.py +++ b/scripts/rules/resolve.py @@ -13,6 +13,11 @@ from functools import cache from typing import TYPE_CHECKING, Any +from scripts.rules.file_exceptions import ( + linted_files, + matching_files, + out_of_scope_files, +) from scripts.rules.model import CiRef, Kind, Rule, Violation from scripts.rules.workflows import JobBlock, parse_jobs @@ -23,6 +28,8 @@ DEFAULT_RUNNER = "scripts/run_quality_guards.py" RUFF_DEFAULT_SELECT = ("E4", "E7", "E9", "F") PROOF_CI_FIELDS = ("workflow", "job", "invocation") +# How many offending files a file-level exception violation names. +SHOWN_OUT_OF_SCOPE_FILES = 3 # --- shared readers ------------------------------------------------------- @@ -38,6 +45,7 @@ def clear_caches() -> None: _read.cache_clear() workflow_jobs.cache_clear() test_names.cache_clear() + linted_files.cache_clear() @cache @@ -264,6 +272,45 @@ def _resolve_ruff(rule: Rule, root: Path) -> list[Violation]: "match enforcer.pattern/codes to the ruff config", ) ] + return _file_level_exception_scope(rule, root, pattern) + return [] + + +def _file_level_exception_scope( + rule: Rule, root: Path, pattern: str +) -> list[Violation]: + """Check a per-file ignore covers only tests, CI/CD and command-line scripts.""" + out_of_scope = out_of_scope_files(root, pattern) + if out_of_scope is None: + return [ + _v( + rule, + f"ruff per-file-ignores pattern {pattern!r} is negated", + "name the files the exception covers", + ) + ] + if not matching_files(root, pattern): + return [ + _v( + rule, + f"ruff per-file-ignores pattern {pattern!r} matches no files", + "fix the pattern, or remove the exception", + ) + ] + if out_of_scope: + shown = ", ".join(out_of_scope[:SHOWN_OUT_OF_SCOPE_FILES]) + hidden = len(out_of_scope) - SHOWN_OUT_OF_SCOPE_FILES + more = f" and {hidden} more" if hidden > 0 else "" + return [ + _v( + rule, + f"ruff per-file-ignores {pattern!r} covers files that may not take " + f"file-level exceptions: {shown}{more}", + "file-level exceptions are only for tests, CI/CD scripts and " + "command-line scripts; narrow the pattern and suppress other " + "findings inline with a reason", + ) + ] return [] diff --git a/scripts/rules/tests/test_guard.py b/scripts/rules/tests/test_guard.py index 731f65d49..6c80c0c3a 100644 --- a/scripts/rules/tests/test_guard.py +++ b/scripts/rules/tests/test_guard.py @@ -38,6 +38,34 @@ def with_rule(extra: str) -> dict[str, str]: return {"rules/registry.toml": REGISTRY + extra} +def per_file_ignore_rule(name: str, pattern: str) -> str: + """Return a registry rule describing a ruff per-file ignore of E501.""" + return f""" +[[rule]] +id = "ex.quality.{name}-ignores" +category = "quality" +statement = "Long lines are allowed here." +rationale = "docs/guide.md" +scope = ["rules/registry.toml"] +tier = "mechanical" +status = "advisory" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "per-file-ignore" +pattern = "{pattern}" +codes = ["E501"] +""" + + +def per_file_ignore_config(pattern: str) -> str: + """Return a pyproject.toml whose ruff config ignores E501 for ``pattern``.""" + return ( + '[tool.mypy]\nfiles = ["pkg"]\n\n' + f'[tool.ruff.lint.per-file-ignores]\n"{pattern}" = ["E501"]\n' + ) + + class ValidRepositoryTest(unittest.TestCase): def test_valid_repository_passes(self) -> None: with Repo() as repo: @@ -442,6 +470,66 @@ def test_unregistered_ruff_ignore(self) -> None: ) self.assertViolation(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' + allowed = { + "tests/**": "tests", + ".github/**": "ci", + "scripts/**": "scripts", + "pkg/cli.py": "cli", + } + for pattern, name in allowed.items(): + with ( + self.subTest(pattern=pattern), + Repo( + { + "pkg/cli.py": program, + ".github/scripts/release.py": "print(1)\n", + **with_rule(per_file_ignore_rule(name, pattern)), + } + ) as repo, + ): + repo.write("pyproject.toml", per_file_ignore_config(pattern)) + self.assertEqual([m for m in repo.messages() if "per-file" in m], []) + + 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( + repo, + "covers files that may not take file-level exceptions: pkg/routes.py", + ) + + def test_file_level_exception_needs_a_program_outside_scripts(self) -> None: + # A module with a main() that is never run as a program isn't a + # command-line script. + with Repo( + { + "pkg/cli.py": "def main():\n pass\n", + **with_rule(per_file_ignore_rule("cli", "pkg/cli.py")), + } + ) 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") + + 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( + 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") + + 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") + def test_unregistered_ci_job(self) -> None: with Repo() as repo: text = repo.read(".github/workflows/pr.yml") From 39710a9bb4d3313384c93d8375f4c0a72c3078af Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 09:48:16 +0400 Subject: [PATCH 16/16] Move group 6's test, CI and script suppressions to file-level exceptions Tests, CI/CD scripts and command-line scripts now take their deliberate group 6 exceptions as registered per-file-ignores, each code chosen from the suppressions those files already carried: - tests and the deployed-service tests: S102, S105, S106, S108, S301, S603, S607, DTZ001 and DTZ007, added to their existing exemptions; - .github: T201; - scripts directories: DTZ011, S603, S607 and T201; - programs under src (the Modal deployment utilities, and the OpenAPI and Stage 12 infrastructure commands): S310 and T201. That removes 75 inline codes from 44 files. Application code keeps its inline suppressions and reasons. Comments only: no code changes. Co-Authored-By: Claude Opus 5.5 --- .../scripts/update_policyengine_package.py | 2 +- AGENTS.md | 7 +- changelog_entry.yaml | 2 +- docs/generated/rules.md | 37 +++++++--- .../tests/test_hf_dataset.py | 2 +- .../tests/test_job_progress.py | 2 +- .../generate_openapi.py | 2 +- .../stage12_infrastructure.py | 2 +- .../tests/test_cloud_run_revision_cleanup.py | 4 +- .../tests/test_deployment_assets.py | 10 +-- .../tests/test_simulation_rollback.py | 4 +- .../tests/test_stage12_infrastructure.py | 4 +- .../tests/workflow_script_support.py | 2 +- .../scripts/dump_package_version.py | 2 +- .../modal/utils/extract_bundle_versions.py | 4 +- .../src/modal/utils/extract_stage12_bundle.py | 4 +- .../src/modal/utils/qualify_stage12_parity.py | 2 +- .../src/modal/utils/record_deployment.py | 2 +- .../src/modal/utils/release_sentinel.py | 6 +- .../modal/utils/update_v2_version_manifest.py | 2 +- .../modal/utils/update_version_registry.py | 2 +- .../src/modal/utils/validate_hf_access.py | 2 +- .../modal/utils/validate_v2_worker_release.py | 2 +- .../integration/test_image_smoke_modal.py | 4 +- .../tests/test_canonical_spm.py | 4 +- .../tests/test_ci_complete_gate.py | 4 +- .../tests/test_hf_access_validation.py | 2 +- .../tests/test_modal_bundle_image.py | 2 +- .../tests/test_modal_scripts.py | 6 +- .../test_policyengine_dependency_source.py | 2 +- ...est_policyengine_package_update_scripts.py | 8 +-- .../tests/test_release_sentinel.py | 2 +- .../tests/test_simulation_output_builder.py | 2 +- .../test_simulation_output_geographic.py | 2 +- .../tests/test_simulation_runtime_policy.py | 4 +- .../tests/test_stage12_failure_causes.py | 2 +- .../tests/test_stage12_runtime.py | 2 +- .../tests/test_validate_hf_access.py | 4 +- .../generate_openapi.py | 2 +- .../integration/test_image_smoke_modal.py | 4 +- .../tests/test_modal_gateway_image.py | 2 +- ruff.toml | 27 +++++++- rules/registry.toml | 69 +++++++++++++++++-- scripts/rules/baseline.py | 8 +-- scripts/rules/ci_scripts.py | 4 +- scripts/rules/cli.py | 4 +- scripts/rules/guard.py | 2 +- scripts/rules/tests/test_guard.py | 6 +- scripts/run_quality_guards.py | 6 +- 49 files changed, 194 insertions(+), 98 deletions(-) diff --git a/.github/scripts/update_policyengine_package.py b/.github/scripts/update_policyengine_package.py index 785d0e96b..af723b329 100644 --- a/.github/scripts/update_policyengine_package.py +++ b/.github/scripts/update_policyengine_package.py @@ -34,7 +34,7 @@ def _json_from_stdin() -> object: def _write_result(result: object) -> None: """Print one result line for update-policyengine-package.sh to capture.""" - print(result) # noqa: T201 # command-line script: the shell script reads stdout + print(result) def _requirement_name(requirement: str) -> str: diff --git a/AGENTS.md b/AGENTS.md index 6af452363..15ba224be 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -140,14 +140,17 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. ### Code quality and tests -- `sim.quality.deployed-tests-ruff-exemptions`: The deployed-service test suite (projects/*/authenticated_tests, pytest run against a live candidate) gets the same ruff exemptions as unit tests (S101, D1, ANN, PLR2004); every other ruff rule applies to it. (blocking: ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/authenticated_tests/**`) +- `sim.quality.ci-cd-scripts-ruff-exemptions`: CI/CD scripts under .github may print their output (T201); every other ruff rule applies to them. (blocking: ruff per-file ignore of T201 for `.github/**`) +- `sim.quality.command-line-scripts-ruff-exemptions`: Command-line scripts under a scripts directory may print their output (T201), run other programs (S603, S607) and use today's date (DTZ011); every other ruff rule applies to them. (blocking: ruff per-file ignore of DTZ011, S603, S607, T201 for `**/scripts/**`) +- `sim.quality.deployed-tests-ruff-exemptions`: The deployed-service test suite (projects/*/authenticated_tests, pytest run against a live candidate) gets the same ruff exemptions as unit tests (sim.quality.tests-ruff-exemptions); every other ruff rule applies to it. (blocking: ruff per-file ignore of ANN, D1, DTZ001, DTZ007, PLR2004, S101, S102, S105, S106, S108, S301, S603, S607 for `**/authenticated_tests/**`) - `sim.quality.every-project-tested`: Every project and library with unit tests runs them in the PR test matrix. (doc-only, not checked: follow it yourself) - `sim.quality.ruff-format-src`: Source code under projects/*/src and libs/*/src is formatted with ruff format. (blocking: ruff format) - `sim.quality.ruff-format-tests`: Test code is formatted with ruff format too; CI currently checks only src directories. (doc-only, not checked: follow it yourself) - `sim.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in ruff.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (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)) - `sim.quality.runtime-evaluated-models-resolve`: Every Pydantic model is runtime-evaluated for ruff: one of its bases, read through the imports of the model's own module, is listed in ruff.toml's runtime-evaluated-base-classes or is a runtime-evaluated class in that same module; and every Pydantic model resolves its annotations once its package is imported. (blocking: pytest `projects/policyengine-simulation-entry/tests/test_runtime_evaluated_annotations.py`, `projects/policyengine-simulation-executor/tests/test_runtime_evaluated_annotations.py`) +- `sim.quality.src-programs-ruff-exemptions`: Programs under src (the Modal deployment utilities, and the OpenAPI and Stage 12 infrastructure commands) may print their output (T201) and open URLs from their arguments (S310); every other ruff rule applies to them. (blocking: ruff per-file ignore of S310, T201 for `projects/*/src/{modal/utils/*,policyengine_simulation_entry/generate_openapi,policyengine_simulation_entry/stage12_infrastructure,policyengine_simulation_gateway/generate_openapi}.py`) - `sim.quality.tests-pass`: Each project's unit tests pass in its own locked environment. (blocking: CI job `test`) -- `sim.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); every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/tests/**`) +- `sim.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 run repository scripts (S603, S607) and use fake credentials (S105, S106), temporary paths (S108), naive datetimes (DTZ001, DTZ007), pickle (S301) and exec (S102). Every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, DTZ001, DTZ007, PLR2004, S101, S102, S105, S106, S108, S301, S603, S607 for `**/tests/**`) ### CI workflows and scripts diff --git a/changelog_entry.yaml b/changelog_entry.yaml index 03c3e7cac..99adea86d 100644 --- a/changelog_entry.yaml +++ b/changelog_entry.yaml @@ -4,7 +4,7 @@ - Made CLAUDE.md and the Copilot instructions thin pointers to AGENTS.md, required workflow logic longer than one line to live in tested and shellchecked scripts, and registered the tracking-issue rule - Added a rules registry (rules/registry.toml) with a standard-library tool that validates it, checks every rule is enforced where it claims, and renders docs/generated/rules.md and the Rules block in AGENTS.md changed: - - Enabled ruff's security, logging and debugging rules (S, T20, LOG, G, DTZ, ERA, TD, FIX, EXE); asserts outside tests became explicit checks, command-line scripts keep their printed output, test subprocess calls go through helpers, and every deliberate exception carries a reasoned suppression; the gateway's auth misconfiguration log no longer repeats its message, and the deployed-service tests share the unit-test ruff exemptions + - Enabled ruff's security, logging and debugging rules (S, T20, LOG, G, DTZ, ERA, TD, FIX, EXE); asserts outside tests became explicit checks, command-line scripts keep their printed output, test subprocess calls go through helpers, and every deliberate exception carries a reasoned suppression; the gateway's auth misconfiguration log no longer repeats its message, and the deployed-service tests share the unit-test ruff exemptions; tests, CI/CD scripts and command-line scripts take their deliberate exceptions as registered file-level ruff ignores instead of inline suppressions, and the rules guard rejects file-level ignores anywhere else - Enabled ruff's exception rules (TRY, EM, BLE); exception messages are built before the raise, raises inside try blocks moved into helpers, and every deliberate catch-all or traceback-free error log carries a reasoned suppression; message text is unchanged, and error types change only in an operator script's two type checks - Enabled ruff's docstring rules (D) and documented every module, class and function outside tests; published contract models and routes now carry these descriptions in their JSON schema and OpenAPI, with no structural schema change - Enabled ruff's annotation rules (ANN) and annotated every function outside tests; test files are exempt from ANN, missing docstrings, magic numbers and bare-assert rules diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 497ac7d09..3be6b78e9 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -8,11 +8,11 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf | Tier | enforced | ratcheted | advisory | doc-only | Total | | --- | ---: | ---: | ---: | ---: | ---: | -| mechanical | 36 | 1 | 0 | 14 | 51 | +| mechanical | 39 | 1 | 0 | 14 | 54 | | metric | 0 | 0 | 0 | 0 | 0 | | judgment | 0 | 0 | 0 | 0 | 0 | -**Blocking:** 37 of 51 rules. +**Blocking:** 40 of 54 rules. ## Gaps @@ -246,20 +246,27 @@ flowchart LR | Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | | --- | --- | --- | --- | --- | --- | --- | --- | -| `sim.quality.deployed-tests-ruff-exemptions` | The deployed-service test suite (projects/*/authenticated_tests, pytest run against a live candidate) gets the same ruff exemptions as unit tests (S101, D1, ANN, PLR2004); every other ruff rule applies to it. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/authenticated_tests/**` | `pr.yml:lint` | — | +| `sim.quality.ci-cd-scripts-ruff-exemptions` | CI/CD scripts under .github may print their output (T201); every other ruff rule applies to them. | mechanical | enforced | yes | ruff per-file ignore of T201 for `.github/**` | `pr.yml:lint` | — | +| `sim.quality.command-line-scripts-ruff-exemptions` | Command-line scripts under a scripts directory may print their output (T201), run other programs (S603, S607) and use today's date (DTZ011); every other ruff rule applies to them. | mechanical | enforced | yes | ruff per-file ignore of DTZ011, S603, S607, T201 for `**/scripts/**` | `pr.yml:lint` | — | +| `sim.quality.deployed-tests-ruff-exemptions` | The deployed-service test suite (projects/*/authenticated_tests, pytest run against a live candidate) gets the same ruff exemptions as unit tests (sim.quality.tests-ruff-exemptions); every other ruff rule applies to it. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, DTZ001, DTZ007, PLR2004, S101, S102, S105, S106, S108, S301, S603, S607 for `**/authenticated_tests/**` | `pr.yml:lint` | — | | `sim.quality.every-project-tested` | Every project and library with unit tests runs them in the PR test matrix. | mechanical | doc-only | no | nothing | — | — | | `sim.quality.ruff-format-src` | Source code under projects/*/src and libs/*/src is formatted with ruff format. | mechanical | enforced | yes | ruff format | `pr.yml:lint` | — | | `sim.quality.ruff-format-tests` | Test code is formatted with ruff format too; CI currently checks only src directories. | mechanical | doc-only | no | nothing | — | — | | `sim.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in ruff.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. | mechanical | enforced | yes | ruff check (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` | — | | `sim.quality.runtime-evaluated-models-resolve` | Every Pydantic model is runtime-evaluated for ruff: one of its bases, read through the imports of the model's own module, is listed in ruff.toml's runtime-evaluated-base-classes or is a runtime-evaluated class in that same module; and every Pydantic model resolves its annotations once its package is imported. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_runtime_evaluated_annotations.py`, `projects/policyengine-simulation-executor/tests/test_runtime_evaluated_annotations.py` | `pr.yml:test` | yes | +| `sim.quality.src-programs-ruff-exemptions` | Programs under src (the Modal deployment utilities, and the OpenAPI and Stage 12 infrastructure commands) may print their output (T201) and open URLs from their arguments (S310); every other ruff rule applies to them. | mechanical | enforced | yes | ruff per-file ignore of S310, T201 for `projects/*/src/{modal/utils/*,policyengine_simulation_entry/generate_openapi,policyengine_simulation_entry/stage12_infrastructure,policyengine_simulation_gateway/generate_openapi}.py` | `pr.yml:lint` | — | | `sim.quality.tests-pass` | Each project's unit tests pass in its own locked environment. | mechanical | enforced | yes | CI job `test` | `pr.yml:test` | — | -| `sim.quality.tests-ruff-exemptions` | Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004); every other ruff rule applies to tests. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/tests/**` | `pr.yml:lint` | — | +| `sim.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 run repository scripts (S603, S607) and use fake credentials (S105, S106), temporary paths (S108), naive datetimes (DTZ001, DTZ007), pickle (S301) and exec (S102). Every other ruff rule applies to tests. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, DTZ001, DTZ007, PLR2004, S101, S102, S105, S106, S108, S301, S603, S607 for `**/tests/**` | `pr.yml:lint` | — | ```mermaid flowchart LR - r_sim_quality_deployed_tests_ruff_exemptions("sim.quality.deployed-tests-ruff-exemptions (enforced)") - e_ruff_per_file_ignore_of_ANN__D1__PLR2004__S101_for____authenticated_tests___["ruff per-file ignore of ANN, D1, PLR2004, S101 for **/authenticated_tests/**"] + r_sim_quality_ci_cd_scripts_ruff_exemptions("sim.quality.ci-cd-scripts-ruff-exemptions (enforced)") + e_ruff_per_file_ignore_of_T201_for__github___["ruff per-file ignore of T201 for .github/**"] j_CI_pr_yml_lint["CI pr.yml:lint"] + r_sim_quality_command_line_scripts_ruff_exemptions("sim.quality.command-line-scripts-ruff-exemptions (enforced)") + e_ruff_per_file_ignore_of_DTZ011__S603__S607__T201_for____scripts___["ruff per-file ignore of DTZ011, S603, S607, T201 for **/scripts/**"] + r_sim_quality_deployed_tests_ruff_exemptions("sim.quality.deployed-tests-ruff-exemptions (enforced)") + e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ007__PLR2004__S101__S102__S105__S106__S108__S301__S603__S607_for____authenticated_tests___["ruff per-file ignore of ANN, D1, DTZ001, DTZ007, PLR2004, S101, S102, S105, S106, S108, S301, S603, S607 for **/authenticated_tests/**"] r_sim_quality_every_project_tested("sim.quality.every-project-tested (doc-only)") gap_unenforced>"not enforced"] r_sim_quality_ruff_format_src("sim.quality.ruff-format-src (enforced)") @@ -270,12 +277,18 @@ flowchart LR r_sim_quality_runtime_evaluated_models_resolve("sim.quality.runtime-evaluated-models-resolve (enforced)") e_pytest_projects_policyengine_simulation_entry_tests_test_runtime_evaluated_annotations_py__projects_policyengine_simulation_executor_tests_test_runtime_evaluated_annotations_py["pytest projects/policyengine-simulation-entry/tests/test_runtime_evaluated_annotations.py, projects/policyengine-simulation-executor/tests/test_runtime_evaluated_annotations.py"] j_CI_pr_yml_test["CI pr.yml:test"] + r_sim_quality_src_programs_ruff_exemptions("sim.quality.src-programs-ruff-exemptions (enforced)") + e_ruff_per_file_ignore_of_S310__T201_for_projects___src__modal_utils___policyengine_simulation_entry_generate_openapi_policyengine_simulation_entry_stage12_infrastructure_policyengine_simulation_gateway_generate_openapi__py["ruff per-file ignore of S310, T201 for projects/*/src/{modal/utils/*,policyengine_simulation_entry/generate_openapi,policyengine_simulation_entry/stage12_infrastructure,policyengine_simulation_gateway/generate_openapi}.py"] r_sim_quality_tests_pass("sim.quality.tests-pass (enforced)") e_CI_job_test["CI job test"] r_sim_quality_tests_ruff_exemptions("sim.quality.tests-ruff-exemptions (enforced)") - e_ruff_per_file_ignore_of_ANN__D1__PLR2004__S101_for____tests___["ruff per-file ignore of ANN, D1, PLR2004, S101 for **/tests/**"] - r_sim_quality_deployed_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__PLR2004__S101_for____authenticated_tests___ - e_ruff_per_file_ignore_of_ANN__D1__PLR2004__S101_for____authenticated_tests___ --> j_CI_pr_yml_lint + e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ007__PLR2004__S101__S102__S105__S106__S108__S301__S603__S607_for____tests___["ruff per-file ignore of ANN, D1, DTZ001, DTZ007, PLR2004, S101, S102, S105, S106, S108, S301, S603, S607 for **/tests/**"] + r_sim_quality_ci_cd_scripts_ruff_exemptions --> e_ruff_per_file_ignore_of_T201_for__github___ + e_ruff_per_file_ignore_of_T201_for__github___ --> j_CI_pr_yml_lint + r_sim_quality_command_line_scripts_ruff_exemptions --> e_ruff_per_file_ignore_of_DTZ011__S603__S607__T201_for____scripts___ + e_ruff_per_file_ignore_of_DTZ011__S603__S607__T201_for____scripts___ --> j_CI_pr_yml_lint + r_sim_quality_deployed_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ007__PLR2004__S101__S102__S105__S106__S108__S301__S603__S607_for____authenticated_tests___ + e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ007__PLR2004__S101__S102__S105__S106__S108__S301__S603__S607_for____authenticated_tests___ --> j_CI_pr_yml_lint r_sim_quality_every_project_tested -.-> gap_unenforced r_sim_quality_ruff_format_src --> e_ruff_format e_ruff_format --> j_CI_pr_yml_lint @@ -284,10 +297,12 @@ flowchart LR 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_sim_quality_runtime_evaluated_models_resolve --> e_pytest_projects_policyengine_simulation_entry_tests_test_runtime_evaluated_annotations_py__projects_policyengine_simulation_executor_tests_test_runtime_evaluated_annotations_py e_pytest_projects_policyengine_simulation_entry_tests_test_runtime_evaluated_annotations_py__projects_policyengine_simulation_executor_tests_test_runtime_evaluated_annotations_py --> j_CI_pr_yml_test + r_sim_quality_src_programs_ruff_exemptions --> e_ruff_per_file_ignore_of_S310__T201_for_projects___src__modal_utils___policyengine_simulation_entry_generate_openapi_policyengine_simulation_entry_stage12_infrastructure_policyengine_simulation_gateway_generate_openapi__py + e_ruff_per_file_ignore_of_S310__T201_for_projects___src__modal_utils___policyengine_simulation_entry_generate_openapi_policyengine_simulation_entry_stage12_infrastructure_policyengine_simulation_gateway_generate_openapi__py --> j_CI_pr_yml_lint r_sim_quality_tests_pass --> e_CI_job_test e_CI_job_test --> j_CI_pr_yml_test - r_sim_quality_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__PLR2004__S101_for____tests___ - e_ruff_per_file_ignore_of_ANN__D1__PLR2004__S101_for____tests___ --> j_CI_pr_yml_lint + r_sim_quality_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ007__PLR2004__S101__S102__S105__S106__S108__S301__S603__S607_for____tests___ + e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ007__PLR2004__S101__S102__S105__S106__S108__S301__S603__S607_for____tests___ --> j_CI_pr_yml_lint ``` ## CI workflows and scripts diff --git a/libs/policyengine-simulation-contract/tests/test_hf_dataset.py b/libs/policyengine-simulation-contract/tests/test_hf_dataset.py index 336d229db..5d24d630a 100644 --- a/libs/policyengine-simulation-contract/tests/test_hf_dataset.py +++ b/libs/policyengine-simulation-contract/tests/test_hf_dataset.py @@ -13,7 +13,7 @@ with_hf_revision, ) -RUNTIME_TOKEN = "runtime-token" # noqa: S105 # fake token; the Hub fetch is stubbed +RUNTIME_TOKEN = "runtime-token" class _FakeResponse: diff --git a/libs/policyengine-simulation-contract/tests/test_job_progress.py b/libs/policyengine-simulation-contract/tests/test_job_progress.py index 63cc30051..d45335c58 100644 --- a/libs/policyengine-simulation-contract/tests/test_job_progress.py +++ b/libs/policyengine-simulation-contract/tests/test_job_progress.py @@ -341,7 +341,7 @@ def test__record_without_attempts_counts_one_attempt(self): def test__record_requires_timezone_aware_times(self): with pytest.raises(ValidationError): - _record(started_at=datetime(2026, 10, 13, 12, 0, 30)) # noqa: DTZ001 # the naive time the record must reject + _record(started_at=datetime(2026, 10, 13, 12, 0, 30)) class TestJobProgressState: diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/generate_openapi.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/generate_openapi.py index 3e985410c..70f1e9066 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/generate_openapi.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/generate_openapi.py @@ -20,7 +20,7 @@ def main() -> None: json.dumps(create_app().openapi(), indent=2) + "\n", encoding="utf-8", ) - print(f"OpenAPI spec written to {output}") # noqa: T201 # command-line script: reports where it wrote the spec + print(f"OpenAPI spec written to {output}") if __name__ == "__main__": diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_infrastructure.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_infrastructure.py index 09a36cd8b..ccae557c9 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_infrastructure.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_infrastructure.py @@ -36,7 +36,7 @@ def main() -> None: ) except (Stage12RuntimeAccessError, ValueError) as error: raise SystemExit(str(error)) from None - print( # noqa: T201 # command-line check: stdout reports the verified access + print( "Shared API v2 runtime database access verified for Stage 12: " f"environment={args.environment}, role={args.expected_role}" ) diff --git a/projects/policyengine-simulation-entry/tests/test_cloud_run_revision_cleanup.py b/projects/policyengine-simulation-entry/tests/test_cloud_run_revision_cleanup.py index 2f9bc38fd..e399b24ab 100644 --- a/projects/policyengine-simulation-entry/tests/test_cloud_run_revision_cleanup.py +++ b/projects/policyengine-simulation-entry/tests/test_cloud_run_revision_cleanup.py @@ -34,8 +34,8 @@ def _run_bash( env: dict[str, str] | None = None, ) -> subprocess.CompletedProcess[str]: """Run bash on a repository deployment script, with text output.""" - return subprocess.run( # noqa: S603 # runs repository scripts with test-built arguments - ["bash", *arguments], # noqa: S607 # bash from PATH, as the workflows run these scripts + return subprocess.run( + ["bash", *arguments], check=check, capture_output=capture_output, text=True, diff --git a/projects/policyengine-simulation-entry/tests/test_deployment_assets.py b/projects/policyengine-simulation-entry/tests/test_deployment_assets.py index 32966f711..58a0d8f50 100644 --- a/projects/policyengine-simulation-entry/tests/test_deployment_assets.py +++ b/projects/policyengine-simulation-entry/tests/test_deployment_assets.py @@ -66,8 +66,8 @@ def _run_bash( cwd: Path | None = None, ) -> subprocess.CompletedProcess[str]: """Run bash on a repository deployment script, with text output.""" - return subprocess.run( # noqa: S603 # runs repository scripts with test-built arguments - ["bash", *arguments], # noqa: S607 # bash from PATH, as the workflows run these scripts + return subprocess.run( + ["bash", *arguments], check=check, capture_output=capture_output, text=True, @@ -495,7 +495,7 @@ def test_cloud_run_deployment_escapes_environment_and_pins_secret_versions(tmp_p arguments_capture = Path(command_environment["GCLOUD_ARGUMENTS_CAPTURE"]) env_capture = Path(command_environment["GCLOUD_ENV_FILE_CAPTURE"]) - subprocess.run( # noqa: S603 # runs the repository deploy script directly, as the workflow does + subprocess.run( [CLOUD_RUN_DEPLOY_SCRIPT], check=True, env=command_environment, @@ -548,7 +548,7 @@ def test_cloud_run_deployment_defaults_permission_enforcement_to_log_only( configured ) - subprocess.run( # noqa: S603 # runs the repository deploy script directly, as the workflow does + subprocess.run( [CLOUD_RUN_DEPLOY_SCRIPT], check=True, env=command_environment, @@ -568,7 +568,7 @@ def test_cloud_run_deployment_rejects_invalid_permission_enforcement( command_environment = _cloud_run_deploy_environment(tmp_path) command_environment["SIMULATION_ENTRYPOINT_ENFORCE_PERMISSIONS_VALUE"] = configured - result = subprocess.run( # noqa: S603 # runs the repository deploy script directly, as the workflow does + result = subprocess.run( [CLOUD_RUN_DEPLOY_SCRIPT], check=False, env=command_environment, diff --git a/projects/policyengine-simulation-entry/tests/test_simulation_rollback.py b/projects/policyengine-simulation-entry/tests/test_simulation_rollback.py index ddb099729..1288e79f3 100644 --- a/projects/policyengine-simulation-entry/tests/test_simulation_rollback.py +++ b/projects/policyengine-simulation-entry/tests/test_simulation_rollback.py @@ -37,8 +37,8 @@ def _run_bash( env: dict[str, str] | None = None, ) -> subprocess.CompletedProcess[str]: """Run bash on a repository rollback script or step, with text output.""" - return subprocess.run( # noqa: S603 # runs repository scripts with test-built arguments - ["bash", *arguments], # noqa: S607 # bash from PATH, as the workflows run these scripts + return subprocess.run( + ["bash", *arguments], check=check, capture_output=capture_output, text=True, diff --git a/projects/policyengine-simulation-entry/tests/test_stage12_infrastructure.py b/projects/policyengine-simulation-entry/tests/test_stage12_infrastructure.py index 6833cad03..c2d58faa9 100644 --- a/projects/policyengine-simulation-entry/tests/test_stage12_infrastructure.py +++ b/projects/policyengine-simulation-entry/tests/test_stage12_infrastructure.py @@ -15,8 +15,8 @@ def _run_bash( *arguments: str, env: dict[str, str] | None = None ) -> subprocess.CompletedProcess[str]: """Run bash on a repository script, capturing text output; a failure is returned.""" - return subprocess.run( # noqa: S603 # runs repository scripts with test-built arguments - ["bash", *arguments], # noqa: S607 # bash from PATH, as the workflows run these scripts + return subprocess.run( + ["bash", *arguments], capture_output=True, text=True, env=env, diff --git a/projects/policyengine-simulation-entry/tests/workflow_script_support.py b/projects/policyengine-simulation-entry/tests/workflow_script_support.py index f6aa5141f..f9111816e 100644 --- a/projects/policyengine-simulation-entry/tests/workflow_script_support.py +++ b/projects/policyengine-simulation-entry/tests/workflow_script_support.py @@ -73,7 +73,7 @@ def run_script( cwd: Path | None = None, ) -> subprocess.CompletedProcess[str]: """Run ``.github/scripts/`` directly, as a workflow step does.""" - return subprocess.run( # noqa: S603 # runs a repository script with test-built arguments + return subprocess.run( [str(SCRIPTS_DIR / name), *args], capture_output=True, text=True, diff --git a/projects/policyengine-simulation-executor/scripts/dump_package_version.py b/projects/policyengine-simulation-executor/scripts/dump_package_version.py index 943fe1a8c..32732e013 100644 --- a/projects/policyengine-simulation-executor/scripts/dump_package_version.py +++ b/projects/policyengine-simulation-executor/scripts/dump_package_version.py @@ -34,5 +34,5 @@ def get_package_version_from_lock( if version is None: sys.exit(1) -print(version) # noqa: T201 # command-line script: the version on stdout is its output +print(version) sys.exit(0) diff --git a/projects/policyengine-simulation-executor/src/modal/utils/extract_bundle_versions.py b/projects/policyengine-simulation-executor/src/modal/utils/extract_bundle_versions.py index 3d5c263e8..4e41068fa 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/extract_bundle_versions.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/extract_bundle_versions.py @@ -39,7 +39,7 @@ def main() -> None: if "--shell" in sys.argv[1:]: for key, value in outputs.items(): - print(f"{key}={value}") # noqa: T201 # command-line script: deployment scripts read stdout + print(f"{key}={value}") return github_output = os.environ.get("GITHUB_OUTPUT") @@ -49,7 +49,7 @@ def main() -> None: for key, value in outputs.items(): file.write(f"{key}={value}\n") - print( # noqa: T201 # command-line script: deployment scripts read stdout + print( "Deploying with policyengine.py bundle " f"{outputs['policyengine_version']}: " f"policyengine-core={outputs['policyengine_core_version']}, " diff --git a/projects/policyengine-simulation-executor/src/modal/utils/extract_stage12_bundle.py b/projects/policyengine-simulation-executor/src/modal/utils/extract_stage12_bundle.py index 11cd6dbaa..3850ed768 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/extract_stage12_bundle.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/extract_stage12_bundle.py @@ -66,10 +66,10 @@ def main() -> None: + "\n" ) if args.output is None: - print(rendered, end="") # noqa: T201 # command-line script: stdout is its output + print(rendered, end="") else: args.output.write_text(rendered, encoding="utf-8") - print(f"bundle_manifest_sha256={resolved.bundle_manifest_sha256}") # noqa: T201 # command-line script: stdout is its output + print(f"bundle_manifest_sha256={resolved.bundle_manifest_sha256}") if __name__ == "__main__": diff --git a/projects/policyengine-simulation-executor/src/modal/utils/qualify_stage12_parity.py b/projects/policyengine-simulation-executor/src/modal/utils/qualify_stage12_parity.py index 42c5864f3..994750336 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/qualify_stage12_parity.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/qualify_stage12_parity.py @@ -61,7 +61,7 @@ def main() -> None: simulation_tolerances=_tolerances(arguments.simulation_tolerances), aggregate_tolerances=_tolerances(arguments.aggregate_tolerances), ) - print(receipt.model_dump_json()) # noqa: T201 # command-line script: the receipt JSON on stdout is its output + print(receipt.model_dump_json()) if __name__ == "__main__": diff --git a/projects/policyengine-simulation-executor/src/modal/utils/record_deployment.py b/projects/policyengine-simulation-executor/src/modal/utils/record_deployment.py index 9efd1beb4..3b0426453 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/record_deployment.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/record_deployment.py @@ -64,7 +64,7 @@ def main() -> None: # The store speaks JSON-able mappings; models dump at the call site # (same discipline as publish_manifest_impl). ArtifactStore().write_deployed_marker(args.environment, marker.model_dump()) - print( # noqa: T201 # command-line script: reports the recorded marker + print( f"Recorded deployed/{args.environment}.json for manifest {args.manifest_digest}" ) diff --git a/projects/policyengine-simulation-executor/src/modal/utils/release_sentinel.py b/projects/policyengine-simulation-executor/src/modal/utils/release_sentinel.py index 9563b0107..c930145f9 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/release_sentinel.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/release_sentinel.py @@ -184,14 +184,14 @@ def _http( timeout: float = 60, ) -> tuple[int, Mapping[str, str], Any]: data = json.dumps(body).encode() if body is not None else None - request = urllib.request.Request(url, data=data, method=method) # noqa: S310 # the deployed entrypoint or the test auth issuer, from the workflow + request = urllib.request.Request(url, data=data, method=method) request.add_header("Accept", "application/json") if data is not None: request.add_header("Content-Type", "application/json") if token: request.add_header("Authorization", f"Bearer {token}") try: - with urllib.request.urlopen(request, timeout=timeout) as response: # noqa: S310 # the request built above + with urllib.request.urlopen(request, timeout=timeout) as response: status, headers, raw = response.status, response.headers, response.read() except urllib.error.HTTPError as error: status, headers, raw = error.code, error.headers, error.read() @@ -325,7 +325,7 @@ def check_deployment( def _report(line: str, *, error: bool = False) -> None: """Print one line of the command's report, to stderr for a failure.""" - print(line, file=sys.stderr if error else sys.stdout) # noqa: T201 # command-line script: stdout and stderr are its report + print(line, file=sys.stderr if error else sys.stdout) def main(argv: list[str] | None = None) -> int: diff --git a/projects/policyengine-simulation-executor/src/modal/utils/update_v2_version_manifest.py b/projects/policyengine-simulation-executor/src/modal/utils/update_v2_version_manifest.py index 632e8e87b..c695073e7 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/update_v2_version_manifest.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/update_v2_version_manifest.py @@ -81,7 +81,7 @@ def main() -> None: worker=worker, force_default=args.force_default, ) - print( # noqa: T201 # command-line script: the manifest JSON on stdout is its output + print( json.dumps( manifest.model_dump(mode="json"), sort_keys=True, diff --git a/projects/policyengine-simulation-executor/src/modal/utils/update_version_registry.py b/projects/policyengine-simulation-executor/src/modal/utils/update_version_registry.py index 35bc087c7..9ab9f69f0 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/update_version_registry.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/update_version_registry.py @@ -1087,7 +1087,7 @@ def restore_routing_state( def _report(line: str = "") -> None: """Print one line of the command's progress report.""" - print(line) # noqa: T201 # command-line script: stdout is its progress report + print(line) def main() -> None: diff --git a/projects/policyengine-simulation-executor/src/modal/utils/validate_hf_access.py b/projects/policyengine-simulation-executor/src/modal/utils/validate_hf_access.py index 27ebf3062..895fc9549 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/validate_hf_access.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/validate_hf_access.py @@ -15,7 +15,7 @@ def validate_local_access() -> HFCredentialAudit: def main() -> None: """Validate the CI credential and print its audit record as JSON.""" - print(validate_local_access().model_dump_json()) # noqa: T201 # command-line script: the audit record on stdout is its output + print(validate_local_access().model_dump_json()) if __name__ == "__main__": diff --git a/projects/policyengine-simulation-executor/src/modal/utils/validate_v2_worker_release.py b/projects/policyengine-simulation-executor/src/modal/utils/validate_v2_worker_release.py index 579799482..0192c50f4 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/validate_v2_worker_release.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/validate_v2_worker_release.py @@ -108,7 +108,7 @@ def main() -> None: + "\n", encoding="utf-8", ) - print(validation.validation_invocation_id) # noqa: T201 # command-line script: logs the validation id for CI + print(validation.validation_invocation_id) if __name__ == "__main__": diff --git a/projects/policyengine-simulation-executor/tests/integration/test_image_smoke_modal.py b/projects/policyengine-simulation-executor/tests/integration/test_image_smoke_modal.py index 4437595ce..843a966f0 100644 --- a/projects/policyengine-simulation-executor/tests/integration/test_image_smoke_modal.py +++ b/projects/policyengine-simulation-executor/tests/integration/test_image_smoke_modal.py @@ -16,8 +16,8 @@ def _run_modal_smoke(app_path: str) -> subprocess.CompletedProcess[str]: """Run one smoke app with ``modal run`` in the ``MODAL_SMOKE_ENV`` environment.""" env_name = os.environ.get("MODAL_SMOKE_ENV", "staging") - return subprocess.run( # noqa: S603 # runs a repository smoke app through uv and Modal - ["uv", "run", "modal", "run", f"--env={env_name}", app_path], # noqa: S607 # uv from PATH, as CI runs it + return subprocess.run( + ["uv", "run", "modal", "run", f"--env={env_name}", app_path], cwd=PROJECT_ROOT, capture_output=True, text=True, diff --git a/projects/policyengine-simulation-executor/tests/test_canonical_spm.py b/projects/policyengine-simulation-executor/tests/test_canonical_spm.py index c03b418fc..11d897bed 100644 --- a/projects/policyengine-simulation-executor/tests/test_canonical_spm.py +++ b/projects/policyengine-simulation-executor/tests/test_canonical_spm.py @@ -516,7 +516,7 @@ def test_an_unavailable_capability_is_rederived_every_time(self, monkeypatch): ) def test_actual_http_formula_error_contract(monkeypatch, code, observability_runtime): error = SPMInputError(code, "Explicit input is required") - assert pickle.loads(pickle.dumps(error)).to_dict() == error.to_dict() # noqa: S301 # round-trips an object the test built + assert pickle.loads(pickle.dumps(error)).to_dict() == error.to_dict() monkeypatch.setattr( "policyengine_simulation_executor.simulation.run_simulation_impl", Mock(side_effect=error), @@ -834,7 +834,7 @@ def __init__(self, code, message): simulation_runtime.run_simulation_impl( {"country": "us"}, runtime=observability_runtime ) - transported = pickle.loads(pickle.dumps(caught.value)) # noqa: S301 # round-trips an object the test raised + transported = pickle.loads(pickle.dumps(caught.value)) assert transported.to_dict() == {"code": code, "message": "Observed input required"} diff --git a/projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py b/projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py index 52b39b385..94fec7bb4 100644 --- a/projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py +++ b/projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py @@ -104,8 +104,8 @@ def test_rule_proof_gate_check() -> None: def _run_gate_script(env: dict[str, str]) -> subprocess.CompletedProcess[str]: - return subprocess.run( # noqa: S603 # runs the repository gate script with test-built arguments - ["bash", GATE_SCRIPT], # noqa: S607 # bash from PATH, as the workflow runs the script + return subprocess.run( + ["bash", GATE_SCRIPT], cwd=REPO, env=env, text=True, diff --git a/projects/policyengine-simulation-executor/tests/test_hf_access_validation.py b/projects/policyengine-simulation-executor/tests/test_hf_access_validation.py index a24940b1f..0ef6f8b92 100644 --- a/projects/policyengine-simulation-executor/tests/test_hf_access_validation.py +++ b/projects/policyengine-simulation-executor/tests/test_hf_access_validation.py @@ -54,7 +54,7 @@ def test_bundle_access_plan_covers_both_private_repositories() -> None: def test_validation_returns_only_safe_identity_and_artifact_metadata() -> None: - token = "private-test-token" # noqa: S105 # fake token; the loaders are stubbed + token = "private-test-token" target = _target() result = validate_uk_private_hf_access( diff --git a/projects/policyengine-simulation-executor/tests/test_modal_bundle_image.py b/projects/policyengine-simulation-executor/tests/test_modal_bundle_image.py index af5682dd1..8d57ad6d6 100644 --- a/projects/policyengine-simulation-executor/tests/test_modal_bundle_image.py +++ b/projects/policyengine-simulation-executor/tests/test_modal_bundle_image.py @@ -301,7 +301,7 @@ def test_app_module_imports_at_container_entrypoint_path(monkeypatch): ) module = importlib.util.module_from_spec(spec) module.__file__ = "/root/app.py" - exec(code, module.__dict__) # noqa: S102 # executes repository-owned app source as Modal's /root/app.py + exec(code, module.__dict__) assert module.APP_NAME.startswith("policyengine-simulation-py") fetch_calls = [ diff --git a/projects/policyengine-simulation-executor/tests/test_modal_scripts.py b/projects/policyengine-simulation-executor/tests/test_modal_scripts.py index 9600d5af5..150748110 100644 --- a/projects/policyengine-simulation-executor/tests/test_modal_scripts.py +++ b/projects/policyengine-simulation-executor/tests/test_modal_scripts.py @@ -13,7 +13,7 @@ pytest_plugins = ("fixtures.test_modal_scripts",) -FAKE_HF_TOKEN = "hf_test" # noqa: S105 # fake token; the scripts under test run stubbed commands +FAKE_HF_TOKEN = "hf_test" def _run_bash( @@ -22,8 +22,8 @@ def _run_bash( cwd: Path | None = None, ) -> subprocess.CompletedProcess[str]: """Run bash on a repository script, capturing text output; a failure is returned.""" - return subprocess.run( # noqa: S603 # runs repository scripts with test-built arguments - ["bash", *arguments], # noqa: S607 # bash from PATH, as the workflows run these scripts + return subprocess.run( + ["bash", *arguments], capture_output=True, text=True, env=env, diff --git a/projects/policyengine-simulation-executor/tests/test_policyengine_dependency_source.py b/projects/policyengine-simulation-executor/tests/test_policyengine_dependency_source.py index 7cafeec5c..ab3fc4828 100644 --- a/projects/policyengine-simulation-executor/tests/test_policyengine_dependency_source.py +++ b/projects/policyengine-simulation-executor/tests/test_policyengine_dependency_source.py @@ -100,7 +100,7 @@ def _run_python_snippet( code: str, env: dict[str, str] ) -> subprocess.CompletedProcess[str]: """Run ``code`` in a fresh interpreter from the project root, capturing text output.""" - return subprocess.run( # noqa: S603 # runs a test-owned snippet with this interpreter + return subprocess.run( [sys.executable, "-c", code], cwd=REPO_ROOT, env=env, diff --git a/projects/policyengine-simulation-executor/tests/test_policyengine_package_update_scripts.py b/projects/policyengine-simulation-executor/tests/test_policyengine_package_update_scripts.py index 835239f39..85d095d2d 100644 --- a/projects/policyengine-simulation-executor/tests/test_policyengine_package_update_scripts.py +++ b/projects/policyengine-simulation-executor/tests/test_policyengine_package_update_scripts.py @@ -23,8 +23,8 @@ def test_update_policyengine_package_script_has_valid_bash_syntax() -> None: - result = subprocess.run( # noqa: S603 # checks the syntax of a repository script - ["bash", "-n", str(SCRIPT)], # noqa: S607 # bash from PATH, as the workflow runs the script + result = subprocess.run( + ["bash", "-n", str(SCRIPT)], capture_output=True, text=True, check=False, @@ -41,7 +41,7 @@ def test_update_policyengine_package_uses_a_separate_python_helper() -> None: assert "python3 -c" not in source assert "python -c" not in source assert "<<'PY'" not in source - result = subprocess.run( # noqa: S603 # runs the repository helper script with this interpreter + result = subprocess.run( [sys.executable, str(helper), "--help"], capture_output=True, text=True, @@ -411,7 +411,7 @@ def _run_helper( env: dict[str, str] | None = None, ) -> subprocess.CompletedProcess[str]: """Run one command of the repository's Python update helper.""" - return subprocess.run( # noqa: S603 # runs the repository helper script with this interpreter + return subprocess.run( [sys.executable, str(HELPER), command], input=input_text, capture_output=True, diff --git a/projects/policyengine-simulation-executor/tests/test_release_sentinel.py b/projects/policyengine-simulation-executor/tests/test_release_sentinel.py index bf52aa4da..3d0c125a5 100644 --- a/projects/policyengine-simulation-executor/tests/test_release_sentinel.py +++ b/projects/policyengine-simulation-executor/tests/test_release_sentinel.py @@ -187,7 +187,7 @@ def _check(api, **kwargs): problems = sentinel.check_deployment( "https://candidate.example/", expected, - token="t0ken", # noqa: S106 # fake bearer token; the HTTP calls are stubbed + token="t0ken", http=api, sleep=sleeps.append, **kwargs, diff --git a/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py b/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py index 4ddfaa513..500b24477 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py @@ -80,7 +80,7 @@ ) from policyengine_simulation_observability.stages import Stage -CONSTITUENCY_CSV_PATH = "/tmp/constituencies_2024.csv" # noqa: S108 # a path string the fakes pass along; nothing opens it +CONSTITUENCY_CSV_PATH = "/tmp/constituencies_2024.csv" def _params_with_spm(params, selection): diff --git a/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py b/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py index 18d1442f7..0a5f65343 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py @@ -14,7 +14,7 @@ ) from policyengine_simulation_executor import simulation_output_geographic -CONSTITUENCY_CSV_PATH = "/tmp/constituencies_2024.csv" # noqa: S108 # a path string the fakes pass along; nothing opens it +CONSTITUENCY_CSV_PATH = "/tmp/constituencies_2024.csv" def _complete_constituency_record() -> dict[str, object]: diff --git a/projects/policyengine-simulation-executor/tests/test_simulation_runtime_policy.py b/projects/policyengine-simulation-executor/tests/test_simulation_runtime_policy.py index 6845e540d..a5ee58b1d 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_runtime_policy.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_runtime_policy.py @@ -27,7 +27,7 @@ def _naive_policy_date(year: int, month: int, day: int) -> datetime: policyengine.py parses policy period keys with ``strptime`` and no zone, so its ``ParameterValue`` dates are naive; an aware value never compares equal. """ - return datetime(year, month, day) # noqa: DTZ001 # naive like the dates under test + return datetime(year, month, day) @pytest.fixture(scope="module") @@ -69,7 +69,7 @@ def test_uk_interval_policy_preserves_inclusive_stop_date(uk, stop): assert value.parameter.tax_benefit_model_version is uk.model assert value.parameter.data_type is float assert value.start_date == _naive_policy_date(2027, 4, 6) - assert value.end_date == datetime.strptime(stop, "%Y-%m-%d") # noqa: DTZ007 # policyengine.py's ParameterValue dates are naive + assert value.end_date == datetime.strptime(stop, "%Y-%m-%d") assert value.value == 0.21 assert reform_dict_from_parameter_values(policy.parameter_values) == raw diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_failure_causes.py b/projects/policyengine-simulation-executor/tests/test_stage12_failure_causes.py index 03e37f4c1..b6636d9a5 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_failure_causes.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_failure_causes.py @@ -20,7 +20,7 @@ from pydantic import BaseModel, TypeAdapter, ValidationError from sqlalchemy.exc import IntegrityError, OperationalError -DATABASE_PASSWORD = "pooler-password-value" # noqa: S105 # fake password; the tests check that redaction removes it +DATABASE_PASSWORD = "pooler-password-value" DATABASE_URL = ( f"postgresql://postgres.project:{DATABASE_PASSWORD}" "@pooler.example.com:5432/postgres" diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py b/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py index e6f930aad..f76dbeda3 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py @@ -1071,7 +1071,7 @@ def serialize(self): for role, stop in (("baseline", "2026-12-31"), ("reform", "2027-12-31")): (value,) = observed[role].policy.parameter_values - assert value.end_date == datetime.strptime(stop, "%Y-%m-%d"), role # noqa: DTZ007 # policyengine.py's ParameterValue dates are naive + assert value.end_date == datetime.strptime(stop, "%Y-%m-%d"), role def _uk_report() -> ReportExecutionInput: diff --git a/projects/policyengine-simulation-executor/tests/test_validate_hf_access.py b/projects/policyengine-simulation-executor/tests/test_validate_hf_access.py index 30a980436..e74a50439 100644 --- a/projects/policyengine-simulation-executor/tests/test_validate_hf_access.py +++ b/projects/policyengine-simulation-executor/tests/test_validate_hf_access.py @@ -12,9 +12,9 @@ def _audit() -> HFCredentialAudit: return HFCredentialAudit( - token_fingerprint="abc123def456", # noqa: S106 # a fake hash prefix, not a token + token_fingerprint="abc123def456", account="policyengine-runtime-data-reader", - token_display_name="pe-uk-private-hf-read-token", # noqa: S106 # a display name, not a token + token_display_name="pe-uk-private-hf-read-token", artifacts=(), ) diff --git a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/generate_openapi.py b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/generate_openapi.py index 5df2740f3..fea8f2a3b 100644 --- a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/generate_openapi.py +++ b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/generate_openapi.py @@ -150,7 +150,7 @@ def main() -> None: with open(output_path, "w") as f: json.dump(openapi_spec, f, indent=2) - print(f"OpenAPI spec written to {output_path}") # noqa: T201 # command-line script: reports where it wrote the spec + print(f"OpenAPI spec written to {output_path}") if __name__ == "__main__": diff --git a/projects/policyengine-simulation-gateway/tests/integration/test_image_smoke_modal.py b/projects/policyengine-simulation-gateway/tests/integration/test_image_smoke_modal.py index ae88bb2bc..f2ceb0be2 100644 --- a/projects/policyengine-simulation-gateway/tests/integration/test_image_smoke_modal.py +++ b/projects/policyengine-simulation-gateway/tests/integration/test_image_smoke_modal.py @@ -16,8 +16,8 @@ @pytest.mark.integration def test_gateway_image_smoke(): env_name = os.environ.get("MODAL_SMOKE_ENV", "staging") - result = subprocess.run( # noqa: S603 # runs the repository smoke app through uv and Modal - [ # noqa: S607 # uv from PATH, as CI runs it + result = subprocess.run( + [ "uv", "run", "modal", diff --git a/projects/policyengine-simulation-gateway/tests/test_modal_gateway_image.py b/projects/policyengine-simulation-gateway/tests/test_modal_gateway_image.py index 5d8ac0a84..907630ee0 100644 --- a/projects/policyengine-simulation-gateway/tests/test_modal_gateway_image.py +++ b/projects/policyengine-simulation-gateway/tests/test_modal_gateway_image.py @@ -166,6 +166,6 @@ def test_app_module_imports_at_container_entrypoint_path(monkeypatch): ) module = importlib.util.module_from_spec(spec) module.__file__ = "/root/app.py" - exec(code, module.__dict__) # noqa: S102 # executes repository-owned app source as Modal's /root/app.py + exec(code, module.__dict__) assert module.app.name == "policyengine-simulation-gateway" diff --git a/ruff.toml b/ruff.toml index 628d82e0d..5afddf284 100644 --- a/ruff.toml +++ b/ruff.toml @@ -27,12 +27,33 @@ select = [ ignore = ["D203", "D213"] [lint.per-file-ignores] +# File-level exceptions are allowed only for tests, CI/CD scripts and +# command-line scripts; the rules guard rejects any other pattern. See +# 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. Every other rule applies to tests. -"**/tests/**" = ["ANN", "D1", "PLR2004", "S101"] +# named constants for magic numbers. They also run repository scripts and use +# fake credentials, temporary paths, naive datetimes, pickle and exec on +# purpose. Every other rule applies to tests. +"**/tests/**" = [ + "ANN", "D1", "DTZ001", "DTZ007", "PLR2004", + "S101", "S102", "S105", "S106", "S108", "S301", "S603", "S607", +] # The deployed-service test suite (pytest, run against a live candidate) gets # the same test exemptions. -"**/authenticated_tests/**" = ["ANN", "D1", "PLR2004", "S101"] +"**/authenticated_tests/**" = [ + "ANN", "D1", "DTZ001", "DTZ007", "PLR2004", + "S101", "S102", "S105", "S106", "S108", "S301", "S603", "S607", +] +# CI/CD scripts report on stdout. +".github/**" = ["T201"] +# Command-line scripts report on stdout, run other programs and use today's +# date. +"**/scripts/**" = ["DTZ011", "S603", "S607", "T201"] +# Programs under src: the Modal deployment utilities, and the OpenAPI and +# Stage 12 infrastructure commands. They report on stdout, and the release +# sentinel opens URLs from its arguments. +"projects/*/src/{modal/utils/*,policyengine_simulation_entry/generate_openapi,policyengine_simulation_entry/stage12_infrastructure,policyengine_simulation_gateway/generate_openapi}.py" = ["S310", "T201"] [lint.flake8-type-checking] # Classes whose annotations Pydantic evaluates at runtime, so their imports must diff --git a/rules/registry.toml b/rules/registry.toml index a660957b7..99fa344c5 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -742,8 +742,8 @@ invocation = "ruff check ." [[rule]] id = "sim.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); every other ruff rule applies to tests." -rationale = "docs/engineering/skills/rules-registry.md#baselines" +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 run repository scripts (S603, S607) and use fake credentials (S105, S106), temporary paths (S108), naive datetimes (DTZ001, DTZ007), pickle (S301) and exec (S102). Every other ruff rule applies to tests." +rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" scope = ["projects/*/tests/**/*.py", "libs/*/tests/**/*.py", "scripts/rules/tests/**/*.py"] tier = "mechanical" status = "enforced" @@ -752,7 +752,7 @@ autofix = false kind = "ruff" mode = "per-file-ignore" pattern = "**/tests/**" -codes = ["ANN", "D1", "PLR2004", "S101"] +codes = ["ANN", "D1", "DTZ001", "DTZ007", "PLR2004", "S101", "S102", "S105", "S106", "S108", "S301", "S603", "S607"] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "lint" @@ -761,8 +761,8 @@ invocation = "ruff check ." [[rule]] id = "sim.quality.deployed-tests-ruff-exemptions" category = "quality" -statement = "The deployed-service test suite (projects/*/authenticated_tests, pytest run against a live candidate) gets the same ruff exemptions as unit tests (S101, D1, ANN, PLR2004); every other ruff rule applies to it." -rationale = "docs/engineering/skills/rules-registry.md#baselines" +statement = "The deployed-service test suite (projects/*/authenticated_tests, pytest run against a live candidate) gets the same ruff exemptions as unit tests (sim.quality.tests-ruff-exemptions); every other ruff rule applies to it." +rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" scope = ["projects/*/authenticated_tests/**/*.py"] tier = "mechanical" status = "enforced" @@ -771,7 +771,64 @@ autofix = false kind = "ruff" mode = "per-file-ignore" pattern = "**/authenticated_tests/**" -codes = ["ANN", "D1", "PLR2004", "S101"] +codes = ["ANN", "D1", "DTZ001", "DTZ007", "PLR2004", "S101", "S102", "S105", "S106", "S108", "S301", "S603", "S607"] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "lint" +invocation = "ruff check ." + +[[rule]] +id = "sim.quality.ci-cd-scripts-ruff-exemptions" +category = "quality" +statement = "CI/CD scripts under .github may print their output (T201); every other ruff rule applies to them." +rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" +scope = [".github/scripts/*.py"] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "per-file-ignore" +pattern = ".github/**" +codes = ["T201"] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "lint" +invocation = "ruff check ." + +[[rule]] +id = "sim.quality.command-line-scripts-ruff-exemptions" +category = "quality" +statement = "Command-line scripts under a scripts directory may print their output (T201), run other programs (S603, S607) and use today's date (DTZ011); every other ruff rule applies to them." +rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" +scope = ["scripts/**/*.py", "projects/*/scripts/*.py"] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "per-file-ignore" +pattern = "**/scripts/**" +codes = ["DTZ011", "S603", "S607", "T201"] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "lint" +invocation = "ruff check ." + +[[rule]] +id = "sim.quality.src-programs-ruff-exemptions" +category = "quality" +statement = "Programs under src (the Modal deployment utilities, and the OpenAPI and Stage 12 infrastructure commands) may print their output (T201) and open URLs from their arguments (S310); every other ruff rule applies to them." +rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" +scope = ["projects/policyengine-simulation-executor/src/modal/utils/*.py", "projects/policyengine-simulation-entry/src/policyengine_simulation_entry/generate_openapi.py", "projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_infrastructure.py", "projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/generate_openapi.py"] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "per-file-ignore" +pattern = "projects/*/src/{modal/utils/*,policyengine_simulation_entry/generate_openapi,policyengine_simulation_entry/stage12_infrastructure,policyengine_simulation_gateway/generate_openapi}.py" +codes = ["S310", "T201"] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "lint" diff --git a/scripts/rules/baseline.py b/scripts/rules/baseline.py index 4ebe33fac..5bc3feb90 100644 --- a/scripts/rules/baseline.py +++ b/scripts/rules/baseline.py @@ -28,16 +28,16 @@ def load_base_registry(root: Path, ref: str) -> Registry | None: Raise BaseUnavailable when ``ref`` does not name a commit. """ - verify = subprocess.run( # noqa: S603 # runs git with a fixed command and no shell - ["git", "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"], # noqa: S607 # git comes from PATH, as in any checkout the tool runs in + verify = subprocess.run( + ["git", "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"], cwd=root, capture_output=True, text=True, ) if verify.returncode != 0: raise BaseUnavailable(ref) - shown = subprocess.run( # noqa: S603 # runs git with a fixed command and no shell - ["git", "show", f"{ref}:{REGISTRY_PATH.as_posix()}"], # noqa: S607 # git comes from PATH, as in any checkout the tool runs in + shown = subprocess.run( + ["git", "show", f"{ref}:{REGISTRY_PATH.as_posix()}"], cwd=root, capture_output=True, text=True, diff --git a/scripts/rules/ci_scripts.py b/scripts/rules/ci_scripts.py index e252cea64..571b63ac2 100644 --- a/scripts/rules/ci_scripts.py +++ b/scripts/rules/ci_scripts.py @@ -96,7 +96,7 @@ def shellcheck_failures(config: CiScripts, root: Path) -> set[str] | None: ) if not scripts: return set() - result = subprocess.run( # noqa: S603 # runs the resolved shellcheck on this repository's scripts + result = subprocess.run( [shellcheck, "--format=gcc", *scripts], cwd=root, capture_output=True, @@ -247,5 +247,5 @@ def check() -> list[str]: """Adapter for scripts/run_quality_guards.py.""" violations, notes = run_check(REPO_ROOT) for note in notes: - print(f"note: {note}") # noqa: T201 # the guard adapter prints its notes + print(f"note: {note}") return [str(violation) for violation in violations] diff --git a/scripts/rules/cli.py b/scripts/rules/cli.py index 9df65da56..78ebf6149 100644 --- a/scripts/rules/cli.py +++ b/scripts/rules/cli.py @@ -16,7 +16,7 @@ def _report(message: str) -> None: """Print one line of the command's output.""" - print(message) # noqa: T201 # reporting the result is this command's job + print(message) def _print(violations: list, notes: list[str] | None = None) -> int: @@ -73,7 +73,7 @@ def main(argv: list[str] | None = None) -> int: registry, violations = load_registry(root) if registry is None: return _print(violations) - violations += validate(registry, root, date.today()) # noqa: DTZ011 # review and expiry dates are calendar days, checked against the local date + violations += validate(registry, root, date.today()) if args.command == "render": if violations: _report("render refused: fix the registry first") diff --git a/scripts/rules/guard.py b/scripts/rules/guard.py index 092425003..3efa3d4f5 100644 --- a/scripts/rules/guard.py +++ b/scripts/rules/guard.py @@ -89,7 +89,7 @@ def run_guard( if registry is None: return result - result.violations += validate(registry, root, today or date.today()) # noqa: DTZ011 # review and expiry dates are calendar days, checked against the local date + result.violations += validate(registry, root, today or date.today()) for rule in registry.rules: result.violations += resolve_enforcer(rule, root) result.violations += resolve_ci(rule, root) diff --git a/scripts/rules/tests/test_guard.py b/scripts/rules/tests/test_guard.py index 6c80c0c3a..00151ed23 100644 --- a/scripts/rules/tests/test_guard.py +++ b/scripts/rules/tests/test_guard.py @@ -677,9 +677,9 @@ def test_compares_against_git_base_ref(self) -> None: repo.write("rules/baselines/ratcheted.txt", "pkg/routes.py\n") repo.render() git = ["git", "-c", "user.email=t@example.com", "-c", "user.name=t"] - subprocess.run(["git", "init", "-q"], cwd=repo.root, check=True) # noqa: S607 # git comes from PATH in the temporary test repository - subprocess.run([*git, "add", "-A"], cwd=repo.root, check=True) # noqa: S603 # a fixed git command in the temporary test repository - subprocess.run([*git, "commit", "-qm", "base"], cwd=repo.root, check=True) # noqa: S603 # a fixed git command in the temporary test repository + subprocess.run(["git", "init", "-q"], cwd=repo.root, check=True) + subprocess.run([*git, "add", "-A"], cwd=repo.root, check=True) + subprocess.run([*git, "commit", "-qm", "base"], cwd=repo.root, check=True) repo.edit("rules/registry.toml", "count = 1", "count = 2") repo.write("rules/baselines/ratcheted.txt", "pkg/routes.py\npkg/other.py\n") repo.render() diff --git a/scripts/run_quality_guards.py b/scripts/run_quality_guards.py index 919857305..c6d7f4cc1 100644 --- a/scripts/run_quality_guards.py +++ b/scripts/run_quality_guards.py @@ -30,13 +30,13 @@ def main() -> int: for name, check in GUARDS: violations = check() if not violations: - print(f"{name}: passed") # noqa: T201 # command-line guard runner: stdout is its report + print(f"{name}: passed") continue failed = True - print(f"{name}: failed") # noqa: T201 # command-line guard runner: stdout is its report + print(f"{name}: failed") for violation in violations: - print(f" - {violation}") # noqa: T201 # command-line guard runner: stdout is its report + print(f" - {violation}") return 1 if failed else 0