From 4d331d79b90a795c697f1be5db37dd8b032b5d03 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:45:27 +0400 Subject: [PATCH 01/23] Fix ruff TRY, EM and BLE findings in the FastAPI library Exception messages move into a msg variable before the raise (EM), with the text unchanged. The JWT decoder re-raises with a bare raise (TRY201). Its two JWKS lookup error logs stay traceback-free under a TRY400 noqa, because callers control the token and a traceback per bad token would let them flood the logs. The token decode and the GCP project-id probe keep except Exception under a BLE001 noqa: any decode failure must become a 403, and any probe failure falls back to GOOGLE_CLOUD_PROJECT. Co-Authored-By: Claude Opus 5.5 --- .../src/policyengine_fastapi/auth/jwt_decoder.py | 10 +++++----- .../src/policyengine_fastapi/observability/config.py | 5 ++--- .../src/policyengine_fastapi/opentelemetry/gcp.py | 2 +- libs/policyengine-fastapi/tests/test_observability.py | 6 ++++-- 4 files changed, 12 insertions(+), 11 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 a63578fb3..cdc6ce37c 100644 --- a/libs/policyengine-fastapi/src/policyengine_fastapi/auth/jwt_decoder.py +++ b/libs/policyengine-fastapi/src/policyengine_fastapi/auth/jwt_decoder.py @@ -45,10 +45,10 @@ def __call__( """ try: return self._validate(token) - except HTTPException as err: + except HTTPException: if self.auto_error: LOG.info("Invalid auth token: Throwing 4XX error") - raise err + raise LOG.info("Ignoring invalid auth token") return None @@ -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}") + 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 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}") + 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 raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) try: @@ -77,7 +77,7 @@ def _validate(self, token: HTTPAuthorizationCredentials | None) -> dict[str, str audience=self.audience, issuer=self.issuer, ) - except Exception as error: + 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}") raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) return payload diff --git a/libs/policyengine-fastapi/src/policyengine_fastapi/observability/config.py b/libs/policyengine-fastapi/src/policyengine_fastapi/observability/config.py index 9e1325481..a8da2d8a9 100644 --- a/libs/policyengine-fastapi/src/policyengine_fastapi/observability/config.py +++ b/libs/policyengine-fastapi/src/policyengine_fastapi/observability/config.py @@ -24,9 +24,8 @@ def parse_header_value_pairs(raw: str | None) -> dict[str, str]: continue key, separator, value = candidate.partition("=") if not separator: - raise ValueError( - "Expected OTLP headers in key=value format separated by commas" - ) + msg = "Expected OTLP headers in key=value format separated by commas" + raise ValueError(msg) headers[key.strip()] = value.strip() return headers diff --git a/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py b/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py index 147433112..d2312e5a3 100644 --- a/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py +++ b/libs/policyengine-fastapi/src/policyengine_fastapi/opentelemetry/gcp.py @@ -56,7 +56,7 @@ def _get_project_id_from_metadata() -> str | None: req.add_header("Metadata-Flavor", "Google") try: return request.urlopen(req).read().decode() - except Exception as err: + 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}" ) diff --git a/libs/policyengine-fastapi/tests/test_observability.py b/libs/policyengine-fastapi/tests/test_observability.py index 9c3513428..c6de6efd3 100644 --- a/libs/policyengine-fastapi/tests/test_observability.py +++ b/libs/policyengine-fastapi/tests/test_observability.py @@ -35,7 +35,8 @@ def test_parse_header_value_pairs__raises_for_invalid_pair(): except ValueError as error: assert "key=value" in str(error) else: - raise AssertionError("Expected invalid OTLP header parsing to fail") + msg = "Expected invalid OTLP header parsing to fail" + raise AssertionError(msg) def test_observability_config_disabled__returns_disabled_defaults(): @@ -120,7 +121,8 @@ def test_contract_models__reject_extra_fields(): except ValidationError as error: assert "unexpected" in str(error) else: - raise AssertionError("Expected extra field validation to fail") + msg = "Expected extra field validation to fail" + raise AssertionError(msg) def test_noop_observability__accepts_calls_without_side_effects(): From 71dc2718a892aadf74214bcf89debb5a239a2e06 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:45:33 +0400 Subject: [PATCH 02/23] Fix ruff TRY and EM findings in the simulation contract library Exception messages move into a msg variable before the raise (EM), with the text unchanged; Pydantic validators keep raising ValueError, so callers still get the same ValidationError. resolve_spm_selection and validate_spm_result move their try bodies into private helpers so no raise sits inside the try (TRY301, TRY300); the same ValueErrors still become the same SPMInputError, chained from the original. Co-Authored-By: Claude Opus 5.5 --- .../dataset_uri.py | 6 +- .../gateway_models.py | 20 +- .../hf_dataset.py | 18 +- .../policyengine_simulation_contract/spm.py | 195 +++++++++++------- .../stage12_execution.py | 183 +++++++++------- .../stage12_manifest.py | 53 +++-- .../tests/conftest.py | 3 +- .../tests/test_dataset_uri.py | 5 +- 8 files changed, 292 insertions(+), 191 deletions(-) diff --git a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/dataset_uri.py b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/dataset_uri.py index c731db0c0..9879d50d3 100644 --- a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/dataset_uri.py +++ b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/dataset_uri.py @@ -20,7 +20,8 @@ def split_dataset_revision(dataset_uri: str) -> tuple[str, str | None]: return dataset_uri, None dataset_without_revision, revision = dataset_uri.rsplit("@", maxsplit=1) if not dataset_without_revision or not revision: - raise ValueError(f"Invalid dataset revision reference: {dataset_uri}") + msg = f"Invalid dataset revision reference: {dataset_uri}" + raise ValueError(msg) return dataset_without_revision, revision @@ -39,11 +40,12 @@ def select_dataset_revision( and requested_data_version is not None and requested_revision != requested_data_version ): - raise ValueError( + msg = ( "Conflicting dataset revisions: " f"data requests {requested_revision!r} but data_version is " f"{requested_data_version!r}" ) + raise ValueError(msg) return requested_revision or requested_data_version diff --git a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/gateway_models.py b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/gateway_models.py index 4e3842e69..b57e3b057 100644 --- a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/gateway_models.py +++ b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/gateway_models.py @@ -83,10 +83,11 @@ def _enforce_max_payload_size(value: object) -> object: # only guards against well-formed hostile payloads. return value if encoded_length > MAX_GATEWAY_REQUEST_BYTES: - raise ValueError( + msg = ( f"Request body is too large ({encoded_length} bytes); the gateway " f"accepts at most {MAX_GATEWAY_REQUEST_BYTES} bytes." ) + raise ValueError(msg) return value @@ -160,7 +161,8 @@ class SimulationRequest(GatewayRequestBase): def region_xor_region_group(self) -> "SimulationRequest": """Reject a request that sets both ``region`` and ``region_group``.""" if self.region is not None and self.region_group is not None: - raise ValueError("Provide `region` or `region_group`, not both.") + msg = "Provide `region` or `region_group`, not both." + raise ValueError(msg) return self @@ -290,7 +292,8 @@ def validate_start_year(cls, value: str) -> str: try: return str(int(value)) except (TypeError, ValueError) as exc: - raise ValueError("start_year must be an integer year") from exc + msg = "start_year must be an integer year" + raise ValueError(msg) from exc @model_validator(mode="after") def validate_end_year(self) -> "BudgetWindowBatchRequest": @@ -300,17 +303,18 @@ def validate_end_year(self) -> "BudgetWindowBatchRequest": impacts; those requests belong on the single-year endpoint. """ if self.region_group is not None: - raise ValueError("Provide `region` or `region_group`, not both.") + msg = "Provide `region` or `region_group`, not both." + raise ValueError(msg) end_year = int(self.start_year) + self.window_size - 1 if end_year > self.MAX_END_YEAR: - raise ValueError( - f"budget-window end_year must be {self.MAX_END_YEAR} or earlier" - ) + msg = f"budget-window end_year must be {self.MAX_END_YEAR} or earlier" + raise ValueError(msg) if self.include_cliffs is True: - raise ValueError( + msg = ( "budget-window cliff impacts are not supported; use the single-year " "simulation endpoint with include_cliffs=true" ) + raise ValueError(msg) return self 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 bf4de1b84..d76e67da7 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 @@ -60,9 +60,8 @@ def parse_hf_dataset_uri(dataset_uri: str) -> HFDatasetReference | None: ) parts = path_with_repo.split("/", maxsplit=2) if len(parts) != 3 or not all(parts): - raise HuggingFaceDatasetReferenceError( - f"Invalid Hugging Face dataset URI: {dataset_uri!r}" - ) + msg = f"Invalid Hugging Face dataset URI: {dataset_uri!r}" + raise HuggingFaceDatasetReferenceError(msg) return HFDatasetReference( repo_id=f"{parts[0]}/{parts[1]}", path=parts[2], @@ -90,15 +89,17 @@ def _fetch_hf_dataset_revision( return json.loads(response.read().decode("utf-8")) except HTTPError as exc: detail = exc.reason or f"HTTP {exc.code}" - raise HuggingFaceDatasetReferenceError( + msg = ( f"Hugging Face dataset revision {repo_id}@{revision} was not found: " f"{detail}" - ) from exc + ) + raise HuggingFaceDatasetReferenceError(msg) from exc except (OSError, URLError, json.JSONDecodeError) as exc: - raise HuggingFaceDatasetReferenceError( + msg = ( f"Unable to validate Hugging Face dataset revision " f"{repo_id}@{revision}: {exc}" - ) from exc + ) + raise HuggingFaceDatasetReferenceError(msg) from exc def _siblings_contain_path(payload: dict[str, Any], path: str) -> bool | None: @@ -127,10 +128,11 @@ def validate_hf_dataset_uri(dataset_uri: str) -> str: payload = _fetch_hf_dataset_revision(parsed.repo_id, parsed.revision, _hf_token()) contains_path = _siblings_contain_path(payload, parsed.path) if contains_path is False: - raise HuggingFaceDatasetReferenceError( + msg = ( f"Hugging Face dataset revision {parsed.repo_id}@{parsed.revision} " f"does not contain artifact {parsed.path!r}" ) + raise HuggingFaceDatasetReferenceError(msg) return dataset_uri 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 10e41246e..ccb5f7bf1 100644 --- a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/spm.py +++ b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/spm.py @@ -81,7 +81,8 @@ def validate_as_of(cls, value: str | None) -> str | None: """Require ``as_of`` to be a canonical ``YYYY-MM-DD`` date when given.""" if value is not None: if date.fromisoformat(value).isoformat() != value: - raise ValueError("as_of must be an ISO calendar date (YYYY-MM-DD)") + msg = "as_of must be an ISO calendar date (YYYY-MM-DD)" + raise ValueError(msg) return value @model_validator(mode="after") @@ -97,9 +98,11 @@ def validate_location(self) -> Self: return self if self.geography_kind == "metro": if not self.geography_id or not self.geography_id.strip(): - raise ValueError("An SPM area selection requires geography_id") + msg = "An SPM area selection requires geography_id" + raise ValueError(msg) elif self.geography_id is not None: - raise ValueError("Only an SPM area selection accepts geography_id") + msg = "Only an SPM area selection accepts geography_id" + raise ValueError(msg) return self @@ -136,7 +139,8 @@ def serialize_resolved_configuration( # noqa: ANN201 # a return annotation wou def validate_as_of(cls, value: str | None) -> str | None: """Require ``as_of`` to be a canonical ``YYYY-MM-DD`` date when given.""" if value is not None and date.fromisoformat(value).isoformat() != value: - raise ValueError("as_of must be an ISO calendar date (YYYY-MM-DD)") + msg = "as_of must be an ISO calendar date (YYYY-MM-DD)" + raise ValueError(msg) return value @model_validator(mode="after") @@ -144,9 +148,11 @@ def validate_location(self) -> "SPMResolvedConfiguration": """Require ``geography_id`` for metro selections and forbid it otherwise.""" if self.geography_kind == "metro": if self.geography_id is None or not self.geography_id.strip(): - raise ValueError("An SPM area selection requires geography_id") + msg = "An SPM area selection requires geography_id" + raise ValueError(msg) elif self.geography_id is not None: - raise ValueError("Only an SPM area selection accepts geography_id") + msg = "Only an SPM area selection accepts geography_id" + raise ValueError(msg) return self @@ -187,9 +193,11 @@ def serialize_provenance( # noqa: ANN201 # a return annotation would become th def validate_years(cls, value: tuple[str, ...]) -> tuple[str, ...]: """Require four-digit years, sorted ascending with no duplicates.""" if any(re.fullmatch(r"[0-9]{4}", year) is None for year in value): - raise ValueError("SPM provenance years must use four-digit years") + msg = "SPM provenance years must use four-digit years" + raise ValueError(msg) if value != tuple(sorted(set(value))): - raise ValueError("SPM provenance years must be sorted and unique") + msg = "SPM provenance years must be sorted and unique" + raise ValueError(msg) return value @model_validator(mode="after") @@ -197,9 +205,11 @@ def validate_provenance_location(self) -> "SPMProvenance": """Require ``geography_id`` for metro receipts and forbid it otherwise.""" if self.geography_kind == "metro": if self.geography_id is None or not self.geography_id.strip(): - raise ValueError("An SPM area receipt requires geography_id") + msg = "An SPM area receipt requires geography_id" + raise ValueError(msg) elif self.geography_id is not None: - raise ValueError("Only an SPM area receipt accepts geography_id") + msg = "Only an SPM area receipt accepts geography_id" + raise ValueError(msg) return self @@ -227,9 +237,11 @@ def _require_complete_selection(selection: SPMSelection) -> None: "Resolved SPM selection is missing " + ", ".join(sorted(missing)) ) if selection.forecast_content_sha256 is None: - raise ValueError("Resolved SPM selection has no artifact hash") + msg = "Resolved SPM selection has no artifact hash" + raise ValueError(msg) if selection.scenario is None: - raise ValueError("Resolved SPM selection has no scenario") + msg = "Resolved SPM selection has no scenario" + raise ValueError(msg) def _resolved_spm_configuration( @@ -276,7 +288,8 @@ def build_spm_provenance( assert selection.forecast_content_sha256 is not None assert selection.scenario is not None if forecast_sha256 != selection.forecast_content_sha256: - raise ValueError("SPM receipt artifact hash differs from the selection") + msg = "SPM receipt artifact hash differs from the selection" + raise ValueError(msg) return SPMProvenance( schema_version="canonical-spm-provenance-v2", forecast_id=forecast_id, @@ -303,9 +316,8 @@ def validate_spm_calculation_provenance( receipt_configuration = _resolved_spm_configuration_from_provenance(receipt) for field_name in SPMResolvedConfiguration.model_fields: if getattr(receipt_configuration, field_name) != getattr(resolved, field_name): - raise ValueError( - f"SPM receipt {field_name} differs from the resolved selection" - ) + msg = f"SPM receipt {field_name} differs from the resolved selection" + raise ValueError(msg) return receipt @@ -385,9 +397,8 @@ def pinned_defaults(self) -> Self: default, since ``SPMSelection`` serialises only the fields that were set. """ if not self.defaults.forecast_content_sha256 or not self.defaults.scenario: - raise ValueError( - "Certified SPM defaults must pin artifact hash and scenario" - ) + msg = "Certified SPM defaults must pin artifact hash and scenario" + raise ValueError(msg) self.defaults = SPMSelection( **{name: getattr(self.defaults, name) for name in SPMSelection.model_fields} ) @@ -416,7 +427,8 @@ class SPMComparisonProvenance(BaseModel): def validate_compatible_receipts(self) -> "SPMComparisonProvenance": """Reject comparisons whose baseline and reform receipts differ.""" if self.baseline.receipt != self.reform.receipt: - raise ValueError("baseline and reform SPM receipts differ") + msg = "baseline and reform SPM receipts differ" + raise ValueError(msg) return self @@ -425,10 +437,12 @@ def _collapse_spm_receipts( receipts: Sequence[SPMProvenance], ) -> SPMExecutionProvenance: if not receipts: - raise ValueError(f"{side} receipts must not be empty") + msg = f"{side} receipts must not be empty" + raise ValueError(msg) first = receipts[0] if any(receipt != first for receipt in receipts[1:]): - raise ValueError(f"{side} receipts differ") + msg = f"{side} receipts differ" + raise ValueError(msg) return SPMExecutionProvenance(receipt=first, execution_count=len(receipts)) @@ -441,7 +455,8 @@ def build_spm_comparison_provenance( baseline = _collapse_spm_receipts("baseline", baseline_receipts) reform = _collapse_spm_receipts("reform", reform_receipts) if baseline.receipt != reform.receipt: - raise ValueError("baseline and reform SPM receipts differ") + msg = "baseline and reform SPM receipts differ" + raise ValueError(msg) return SPMComparisonProvenance( schema_version="canonical-spm-comparison-v2", baseline=baseline, @@ -449,6 +464,36 @@ def build_spm_comparison_provenance( ) +def _certified_spm_selection( + capability: SPMCapability | Mapping[str, object], + selection: SPMSelection | Mapping[str, object] | None, +) -> JsonObject: + """Merge a selection over the certified capability's defaults. + + Raises ``ValueError`` for an invalid capability or selection, or for a + selection that names a different forecast artifact. + """ + cap = SPMCapability.model_validate(capability) + chosen = SPMSelection.model_validate({} if selection is None else selection) + defaults = cap.defaults + if chosen.forecast_content_sha256 not in ( + None, + defaults.forecast_content_sha256, + ): + msg = "SPM selection does not match the certified bundle artifact hash" + raise ValueError(msg) + values = {name: getattr(defaults, name) for name in SPMSelection.model_fields} + values.update(chosen.model_dump(exclude_unset=True)) + if ( + "geography_kind" in chosen.model_fields_set + and chosen.geography_kind != defaults.geography_kind + ): + values["geography_id"] = chosen.geography_id + values["forecast_content_sha256"] = defaults.forecast_content_sha256 + values["scenario"] = chosen.scenario or defaults.scenario + return SPMSelection.model_validate(values).model_dump(mode="json") + + def resolve_spm_selection( country: str, selection: SPMSelection | Mapping[str, object] | None, @@ -461,9 +506,8 @@ def resolve_spm_selection( """Resolve only certified metadata; unrecognized future bundles fail closed.""" if country.lower() != "us": if selection is not None: - raise SPMInputError( - "SPM_SETTINGS_INVALID", "SPM settings are supported only for the US" - ) + msg = "SPM_SETTINGS_INVALID" + raise SPMInputError(msg, "SPM settings are supported only for the US") return None if capability is None: # Existing versioned worker bundles predate canonical SPM. This does @@ -497,33 +541,51 @@ def resolve_spm_selection( ) if selection is None and historical: return None + msg = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - "SPM_CONFIGURATION_UNAVAILABLE", + msg, "This worker bundle has no certified canonical SPM capability", ) try: - cap = SPMCapability.model_validate(capability) - chosen = SPMSelection.model_validate({} if selection is None else selection) - defaults = cap.defaults - if chosen.forecast_content_sha256 not in ( - None, - defaults.forecast_content_sha256, - ): - raise ValueError( - "SPM selection does not match the certified bundle artifact hash" - ) - values = {name: getattr(defaults, name) for name in SPMSelection.model_fields} - values.update(chosen.model_dump(exclude_unset=True)) + return _certified_spm_selection(capability, selection) + except ValueError as exc: + msg = "SPM_SETTINGS_INVALID" + raise SPMInputError(msg, str(exc)) from exc + + +def _validated_spm_provenance( + result: dict, + selection: object, + *, + expected_year: int | str | None, +) -> SPMComparisonProvenance: + """Return the result's SPM provenance once it matches ``selection``. + + Raises ``ValueError`` when the provenance is missing, incomplete, does not + match the selection, or does not cover ``expected_year``. + """ + chosen = _resolved_spm_configuration(SPMSelection.model_validate(selection)) + try: + provenance = SPMComparisonProvenance.model_validate( + result.get("spm_provenance") + ) + except ValidationError as exc: + if any(error["type"] == "missing" for error in exc.errors()): + msg = "Result has no complete resolved SPM provenance" + raise ValueError(msg) from exc + raise + for execution in (provenance.baseline, provenance.reform): + validate_spm_calculation_provenance( + config=SPMSelection.model_validate(chosen.model_dump(mode="json")), + receipt=execution.receipt, + ) if ( - "geography_kind" in chosen.model_fields_set - and chosen.geography_kind != defaults.geography_kind + expected_year is not None + and str(expected_year) not in execution.receipt.years ): - values["geography_id"] = chosen.geography_id - values["forecast_content_sha256"] = defaults.forecast_content_sha256 - values["scenario"] = chosen.scenario or defaults.scenario - return SPMSelection.model_validate(values).model_dump(mode="json") - except ValueError as exc: - raise SPMInputError("SPM_SETTINGS_INVALID", str(exc)) from exc + msg = "Result SPM provenance does not cover the requested year" + raise ValueError(msg) + return provenance def validate_spm_result( @@ -531,44 +593,24 @@ def validate_spm_result( ) -> SPMComparisonProvenance | None: """Do not accept incomplete or mixed-method child/cached output.""" if "spm_config" in result: + msg = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - "SPM_CONFIGURATION_UNAVAILABLE", + msg, "Completed results must not contain legacy spm_config metadata", ) if selection is None: if result.get("spm_provenance") is not None: + msg = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - "SPM_CONFIGURATION_UNAVAILABLE", + msg, "Unexpected SPM receipt for a historical result", ) return None try: - chosen = _resolved_spm_configuration(SPMSelection.model_validate(selection)) - try: - provenance = SPMComparisonProvenance.model_validate( - result.get("spm_provenance") - ) - except ValidationError as exc: - if any(error["type"] == "missing" for error in exc.errors()): - raise ValueError( - "Result has no complete resolved SPM provenance" - ) from exc - raise - for execution in (provenance.baseline, provenance.reform): - validate_spm_calculation_provenance( - config=SPMSelection.model_validate(chosen.model_dump(mode="json")), - receipt=execution.receipt, - ) - if ( - expected_year is not None - and str(expected_year) not in execution.receipt.years - ): - raise ValueError( - "Result SPM provenance does not cover the requested year" - ) - return provenance + return _validated_spm_provenance(result, selection, expected_year=expected_year) except ValueError as exc: - raise SPMInputError("SPM_CONFIGURATION_UNAVAILABLE", str(exc)) from exc + msg = "SPM_CONFIGURATION_UNAVAILABLE" + raise SPMInputError(msg, str(exc)) from exc def combine_spm_results( @@ -592,9 +634,8 @@ def combine_spm_results( validated: list[SPMComparisonProvenance] = [] for receipt in receipts: if receipt is None: - raise SPMInputError( - "SPM_CONFIGURATION_UNAVAILABLE", "Missing canonical SPM receipt" - ) + msg = "SPM_CONFIGURATION_UNAVAILABLE" + raise SPMInputError(msg, "Missing canonical SPM receipt") validated.append(receipt) combined = build_spm_comparison_provenance( baseline_receipts=[ diff --git a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/stage12_execution.py b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/stage12_execution.py index a125451d7..dd4e6ca42 100644 --- a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/stage12_execution.py +++ b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/stage12_execution.py @@ -200,9 +200,11 @@ class GeographySelection(StrictContractModel): def require_complete_filter(self) -> GeographySelection: """Require the filter field and value together, and both for a strategy.""" if (self.filter_field is None) != (self.filter_value is None): - raise ValueError("filter_field and filter_value must be supplied together") + msg = "filter_field and filter_value must be supplied together" + raise ValueError(msg) if self.filter_strategy is not None and self.filter_field is None: - raise ValueError("filter_strategy requires filter_field and filter_value") + msg = "filter_strategy requires filter_field and filter_value" + raise ValueError(msg) return self @@ -235,9 +237,8 @@ def reject_combined_policy_input(cls, value: object) -> object: ``ReportExecutionInput``. """ if isinstance(value, dict) and ({"baseline", "reform"} & value.keys()): - raise ValueError( - "single-simulation input cannot contain baseline or reform fields" - ) + msg = "single-simulation input cannot contain baseline or reform fields" + raise ValueError(msg) return value @@ -269,11 +270,11 @@ def require_complete_aggregate_profile( ) -> tuple[ReportAggregate, ...]: """Require every ``ReportAggregate``, once each and in declaration order.""" if len(value) != len(set(value)): - raise ValueError("report output aggregates must be unique") + msg = "report output aggregates must be unique" + raise ValueError(msg) if value != tuple(ReportAggregate): - raise ValueError( - "Stage 12 currently requires the complete aggregate profile" - ) + msg = "Stage 12 currently requires the complete aggregate profile" + raise ValueError(msg) return value @@ -297,9 +298,11 @@ class EntityOutputPlan(StrictContractModel): def require_canonical_variables(cls, value: tuple[str, ...]) -> tuple[str, ...]: """Require variable names to be unique and sorted, so plans hash stably.""" if len(value) != len(set(value)): - raise ValueError("output-plan variables must be unique") + msg = "output-plan variables must be unique" + raise ValueError(msg) if value != tuple(sorted(value)): - raise ValueError("output-plan variables must use canonical order") + msg = "output-plan variables must use canonical order" + raise ValueError(msg) return value @model_validator(mode="after") @@ -309,17 +312,16 @@ def require_variable_subsets(self) -> EntityOutputPlan: Both must also be subsets of ``materialized_variables``. """ if not set(self.additional_variables).issubset(self.materialized_variables): - raise ValueError( - "additional output-plan variables must be a materialized subset" - ) + msg = "additional output-plan variables must be a materialized subset" + raise ValueError(msg) if not set(self.dataset_variables).issubset(self.materialized_variables): - raise ValueError( - "dataset output-plan variables must be a materialized subset" - ) + msg = "dataset output-plan variables must be a materialized subset" + raise ValueError(msg) if set(self.additional_variables).intersection(self.dataset_variables): - raise ValueError( + msg = ( "calculated additional variables and dataset variables must be disjoint" ) + raise ValueError(msg) return self @@ -340,9 +342,11 @@ def require_canonical_entities( """Require entity plans to be unique and sorted by entity name.""" names = tuple(entity.entity for entity in value) if len(names) != len(set(names)): - raise ValueError("output-plan entities must be unique") + msg = "output-plan entities must be unique" + raise ValueError(msg) if names != tuple(sorted(names)): - raise ValueError("output-plan entities must use canonical order") + msg = "output-plan entities must use canonical order" + raise ValueError(msg) return value @@ -367,7 +371,8 @@ class PlannedSimulationExecutionInput(SimulationExecutionInput): def require_output_plan_country(self) -> PlannedSimulationExecutionInput: """Reject an output plan built for a different country than the geography.""" if self.output_plan.country != self.geography.country: - raise ValueError("output plan country must match simulation geography") + msg = "output plan country must match simulation geography" + raise ValueError(msg) return self @@ -388,7 +393,8 @@ class RowIdentity(StrictContractModel): def require_unique_columns(cls, value: tuple[str, ...]) -> tuple[str, ...]: """Reject repeated identifier column names.""" if len(value) != len(set(value)): - raise ValueError("row identifier columns must be unique") + msg = "row identifier columns must be unique" + raise ValueError(msg) return value @@ -433,15 +439,20 @@ def require_aligned_simulations(self) -> ReportExecutionInput: distinct execution ids, and everything but the policy must match. """ if self.baseline.role is not SimulationRole.BASELINE: - raise ValueError("baseline input must use the baseline role") + msg = "baseline input must use the baseline role" + raise ValueError(msg) if self.reform.role is not SimulationRole.REFORM: - raise ValueError("reform input must use the reform role") + msg = "reform input must use the reform role" + raise ValueError(msg) if self.baseline.evaluation_id != self.evaluation_id: - raise ValueError("baseline evaluation_id must match the report") + msg = "baseline evaluation_id must match the report" + raise ValueError(msg) if self.reform.evaluation_id != self.evaluation_id: - raise ValueError("reform evaluation_id must match the report") + msg = "reform evaluation_id must match the report" + raise ValueError(msg) if self.baseline.simulation_execution_id == self.reform.simulation_execution_id: - raise ValueError("baseline and reform simulation identifiers must differ") + msg = "baseline and reform simulation identifiers must differ" + raise ValueError(msg) for field_name in ( "population", "year", @@ -450,11 +461,11 @@ def require_aligned_simulations(self) -> ReportExecutionInput: "bundle", ): if getattr(self.baseline, field_name) != getattr(self.reform, field_name): - raise ValueError( - f"baseline and reform {field_name} must match before execution" - ) + msg = f"baseline and reform {field_name} must match before execution" + raise ValueError(msg) if len(self.requested_aggregates) != len(set(self.requested_aggregates)): - raise ValueError("requested aggregates must be unique") + msg = "requested aggregates must be unique" + raise ValueError(msg) return self @@ -492,14 +503,16 @@ def require_unique_aggregates( ) -> tuple[ReportAggregate, ...]: """Reject repeated aggregate names.""" if len(value) != len(set(value)): - raise ValueError("requested aggregates must be unique") + msg = "requested aggregates must be unique" + raise ValueError(msg) return value @model_validator(mode="after") def validate_spm_provenance(self) -> AggregateReportArtifactPayload: """Reject duplicate or noncanonical SPM data in aggregate artifacts.""" if "spm_config" in self.result: - raise ValueError("aggregate result must not include legacy spm_config") + msg = "aggregate result must not include legacy spm_config" + raise ValueError(msg) # Reports without US SPM calculations serialize this optional field # as null. Validate a supplied record, not the absence of one. provenance = self.result.get("spm_provenance") @@ -527,19 +540,21 @@ def validate_presence(self) -> ResultDifference: only an absolute delta may stand alone (a zero production value). """ if not self.production_present and not self.stage12_present: - raise ValueError( - "a result difference must be present in at least one result" - ) + msg = "a result difference must be present in at least one result" + raise ValueError(msg) if not self.production_present and self.production_value is not None: - raise ValueError("an absent production value must be null") + msg = "an absent production value must be null" + raise ValueError(msg) if not self.stage12_present and self.stage12_value is not None: - raise ValueError("an absent Stage 12 value must be null") + msg = "an absent Stage 12 value must be null" + raise ValueError(msg) # A zero production value has no finite relative delta, so an absolute # delta with no relative delta is the only permitted partial pair. if (self.absolute_delta is None) != (self.relative_delta is None) and ( self.absolute_delta is None or self.relative_delta is not None ): - raise ValueError("numeric deltas must be supplied together") + msg = "numeric deltas must be supplied together" + raise ValueError(msg) return self @@ -560,12 +575,15 @@ class ResultComparisonArtifactPayload(StrictContractModel): def validate_differences(self) -> ResultComparisonArtifactPayload: """Require a timezone-aware timestamp and a count and status that match.""" if self.compared_at.tzinfo is None: - raise ValueError("comparison timestamp must include a timezone") + msg = "comparison timestamp must include a timezone" + raise ValueError(msg) if self.difference_count != len(self.differences): - raise ValueError("difference_count must equal the number of differences") + msg = "difference_count must equal the number of differences" + raise ValueError(msg) expected_status = "matched" if not self.differences else "different" if self.status != expected_status: - raise ValueError("comparison status does not match its differences") + msg = "comparison status does not match its differences" + raise ValueError(msg) return self @@ -612,12 +630,15 @@ def validate_retention(self) -> Stage12InvocationContext: def _validate_retention(created_at: datetime, retention_expires_at: datetime) -> None: if created_at.tzinfo is None or retention_expires_at.tzinfo is None: - raise ValueError("comparison retention timestamps must include a timezone") + msg = "comparison retention timestamps must include a timezone" + raise ValueError(msg) retention = retention_expires_at - created_at if retention <= timedelta(0): - raise ValueError("comparison retention must be greater than zero") + msg = "comparison retention must be greater than zero" + raise ValueError(msg) if retention > MAX_COMPARISON_RUN_ARTIFACT_RETENTION: - raise ValueError("comparison retention must not exceed 30 days") + msg = "comparison retention must not exceed 30 days" + raise ValueError(msg) class ComparisonRunLifecycleStatus(StrEnum): @@ -713,14 +734,18 @@ def validate_state(self) -> ComparisonReportRecord: if any(value is not None for value in output_fields) and not all( value is not None for value in output_fields ): - raise ValueError("aggregate output fields must be supplied together") + msg = "aggregate output fields must be supplied together" + raise ValueError(msg) if self.status is ComparisonRunLifecycleStatus.SUCCEEDED: if self.aggregation_status is not ComparisonRunAggregationStatus.SUCCEEDED: - raise ValueError("a successful report requires successful aggregation") + msg = "a successful report requires successful aggregation" + raise ValueError(msg) if not all(value is not None for value in output_fields): - raise ValueError("a successful report requires an aggregate output") + msg = "a successful report requires an aggregate output" + raise ValueError(msg) if self.completed_at is None: - raise ValueError("a successful report requires completed_at") + msg = "a successful report requires completed_at" + raise ValueError(msg) if ( self.status in { @@ -729,7 +754,8 @@ def validate_state(self) -> ComparisonReportRecord: } and self.error_code is None ): - raise ValueError("a failed or skipped report requires an error_code") + msg = "a failed or skipped report requires an error_code" + raise ValueError(msg) comparison_output_fields = ( self.comparison_output_uri, self.comparison_output_sha256, @@ -741,38 +767,43 @@ def validate_state(self) -> ComparisonReportRecord: ResultComparisonStatus.RUNNING, }: if any(value is not None for value in comparison_output_fields): - raise ValueError( - "an incomplete comparison cannot reference an output artifact" - ) + msg = "an incomplete comparison cannot reference an output artifact" + raise ValueError(msg) if self.comparison_completed_at is not None: - raise ValueError("an incomplete comparison cannot have completed_at") + msg = "an incomplete comparison cannot have completed_at" + raise ValueError(msg) if self.comparison_error_code is not None: - raise ValueError("an incomplete comparison cannot have an error_code") + msg = "an incomplete comparison cannot have an error_code" + raise ValueError(msg) if self.comparison_error_summary is not None: - raise ValueError( - "an incomplete comparison cannot have an error_summary" - ) + msg = "an incomplete comparison cannot have an error_summary" + raise ValueError(msg) elif self.comparison_status in { ResultComparisonStatus.MATCHED, ResultComparisonStatus.DIFFERENT, }: if not all(value is not None for value in comparison_output_fields): - raise ValueError("a completed comparison requires an output artifact") + msg = "a completed comparison requires an output artifact" + raise ValueError(msg) if self.comparison_completed_at is None: - raise ValueError("a completed comparison requires completed_at") + msg = "a completed comparison requires completed_at" + raise ValueError(msg) if self.comparison_error_code is not None: - raise ValueError("a completed comparison cannot have an error_code") + msg = "a completed comparison cannot have an error_code" + raise ValueError(msg) if self.comparison_error_summary is not None: - raise ValueError("a completed comparison cannot have an error_summary") + msg = "a completed comparison cannot have an error_summary" + raise ValueError(msg) else: if any(value is not None for value in comparison_output_fields): - raise ValueError( - "a failed comparison cannot reference an output artifact" - ) + msg = "a failed comparison cannot reference an output artifact" + raise ValueError(msg) if self.comparison_completed_at is None: - raise ValueError("a failed comparison requires completed_at") + msg = "a failed comparison requires completed_at" + raise ValueError(msg) if self.comparison_error_code is None: - raise ValueError("a failed comparison requires an error_code") + msg = "a failed comparison requires an error_code" + raise ValueError(msg) return self @@ -824,17 +855,22 @@ def validate_state(self) -> ComparisonSimulationRecord: if any(value is not None for value in output_fields) and not all( value is not None for value in output_fields ): - raise ValueError("simulation output fields must be supplied together") + msg = "simulation output fields must be supplied together" + raise ValueError(msg) if self.row_identity_columns is not None: if not self.row_identity_columns: - raise ValueError("row_identity_columns must not be empty") + msg = "row_identity_columns must not be empty" + raise ValueError(msg) if len(self.row_identity_columns) != len(set(self.row_identity_columns)): - raise ValueError("row_identity_columns must be unique") + msg = "row_identity_columns must be unique" + raise ValueError(msg) if self.status is ComparisonRunLifecycleStatus.SUCCEEDED: if not all(value is not None for value in output_fields): - raise ValueError("a successful simulation requires an output") + msg = "a successful simulation requires an output" + raise ValueError(msg) if self.completed_at is None: - raise ValueError("a successful simulation requires completed_at") + msg = "a successful simulation requires completed_at" + raise ValueError(msg) if ( self.status in { @@ -843,7 +879,8 @@ def validate_state(self) -> ComparisonSimulationRecord: } and self.error_code is None ): - raise ValueError("a failed or skipped simulation requires an error_code") + msg = "a failed or skipped simulation requires an error_code" + raise ValueError(msg) return self diff --git a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/stage12_manifest.py b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/stage12_manifest.py index 9301ec51e..009c29112 100644 --- a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/stage12_manifest.py +++ b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/stage12_manifest.py @@ -82,24 +82,32 @@ def validate_worker(self) -> V2WorkerVersion: each with a validation invocation. """ if not self.application_name.startswith(V2_APPLICATION_PREFIX): - raise ValueError("v2 application uses an invalid name prefix") + msg = "v2 application uses an invalid name prefix" + raise ValueError(msg) if self.application_name.startswith(V1_APPLICATION_PREFIX): - raise ValueError("v2 application name collides with the v1 prefix") + msg = "v2 application name collides with the v1 prefix" + raise ValueError(msg) expected_name = v2_application_name(self.bundle.policyengine_version) if self.application_name != expected_name: - raise ValueError("v2 application name does not match its bundle version") + msg = "v2 application name does not match its bundle version" + raise ValueError(msg) if self.validation.application_name != self.application_name: - raise ValueError("worker validation names another application") + msg = "worker validation names another application" + raise ValueError(msg) if self.validation.bundle_manifest_sha256 != self.bundle_manifest_sha256: - raise ValueError("worker validation names another bundle digest") + msg = "worker validation names another bundle digest" + raise ValueError(msg) countries = tuple(worker.country for worker in self.countries) bundle_countries = tuple(country.country for country in self.bundle.countries) if len(countries) != len(set(countries)): - raise ValueError("v2 worker countries must be unique") + msg = "v2 worker countries must be unique" + raise ValueError(msg) if countries != bundle_countries: - raise ValueError("v2 worker countries do not match the normalized bundle") + msg = "v2 worker countries do not match the normalized bundle" + raise ValueError(msg) if set(self.validation.country_validation_invocation_ids) != set(countries): - raise ValueError("worker validation does not cover every country") + msg = "worker validation does not cover every country" + raise ValueError(msg) return self @@ -119,12 +127,15 @@ class V2VersionManifest(StrictManifestModel): def validate_versions(self) -> V2VersionManifest: """Check the default has an entry and each key is its bundle's version.""" if not self.versions: - raise ValueError("v2 manifest must contain a worker version") + msg = "v2 manifest must contain a worker version" + raise ValueError(msg) if self.default_version not in self.versions: - raise ValueError("v2 default version has no worker entry") + msg = "v2 default version has no worker entry" + raise ValueError(msg) for version, worker in self.versions.items(): if version != worker.bundle.policyengine_version: - raise ValueError("v2 manifest version key does not match its bundle") + msg = "v2 manifest version key does not match its bundle" + raise ValueError(msg) return self @@ -149,12 +160,14 @@ def v2_application_name(policyengine_version: str) -> str: if not policyengine_version or not re.fullmatch( r"[A-Za-z0-9][A-Za-z0-9._+-]*", policyengine_version ): - raise ValueError("PolicyEngine.py version cannot form a Modal application name") + msg = "PolicyEngine.py version cannot form a Modal application name" + raise ValueError(msg) name = V2_APPLICATION_PREFIX + policyengine_version.replace(".", "-").replace( "+", "-" ) if len(name) > 64: - raise ValueError("v2 Modal application name exceeds 64 characters") + msg = "v2 Modal application name exceeds 64 characters" + raise ValueError(msg) return name @@ -255,7 +268,8 @@ def load(self) -> V2VersionManifest: manifest = V2VersionManifest.model_validate(value) except (TypeError, ValueError): if self._last_valid is None: - raise ValueError("no valid v2 version manifest is available") from None + msg = "no valid v2 version manifest is available" + raise ValueError(msg) from None return self._last_valid self._last_valid = manifest return manifest @@ -270,9 +284,8 @@ def resolve(self, version: str | None = None) -> tuple[str, V2WorkerVersion, str try: worker = manifest.versions[selected] except KeyError: - raise ValueError( - f"v2 worker version {selected!r} is not available" - ) from None + msg = f"v2 worker version {selected!r} is not available" + raise ValueError(msg) from None return selected, worker, manifest_digest(manifest) @@ -282,9 +295,11 @@ def assert_separate_manifest_names(*, v1_name: str, v2_name: str) -> None: This keeps a v2 manifest publish from overwriting the v1 routing state. """ if not v1_name or not v2_name: - raise ValueError("v1 and v2 manifest names must be non-empty") + msg = "v1 and v2 manifest names must be non-empty" + raise ValueError(msg) if v1_name == v2_name: - raise ValueError("v1 and v2 manifests must use different storage names") + msg = "v1 and v2 manifests must use different storage names" + raise ValueError(msg) def validate_manifest_has_no_credentials(value: Mapping[str, object]) -> None: diff --git a/libs/policyengine-simulation-contract/tests/conftest.py b/libs/policyengine-simulation-contract/tests/conftest.py index 7ccaff8cb..920578f17 100644 --- a/libs/policyengine-simulation-contract/tests/conftest.py +++ b/libs/policyengine-simulation-contract/tests/conftest.py @@ -37,7 +37,8 @@ def from_name(name: str, create_if_missing: bool = False): if create_if_missing and name not in mock_dicts: mock_dicts[name] = {} if name not in mock_dicts: - raise KeyError(f"Mock dict not configured for: {name}") + msg = f"Mock dict not configured for: {name}" + raise KeyError(msg) return MockDict(mock_dicts[name]) class MockModal: diff --git a/libs/policyengine-simulation-contract/tests/test_dataset_uri.py b/libs/policyengine-simulation-contract/tests/test_dataset_uri.py index 8a873e5e6..e06019764 100644 --- a/libs/policyengine-simulation-contract/tests/test_dataset_uri.py +++ b/libs/policyengine-simulation-contract/tests/test_dataset_uri.py @@ -12,9 +12,8 @@ def test_runtime_dataset_uri_preserves_populace_build_revision_when_package_diff ) def reject_hf_validation(dataset_uri: str, revision: str) -> str: - raise AssertionError( - f"HF validation should not run for trusted bundle data: {dataset_uri}@{revision}" - ) + msg = f"HF validation should not run for trusted bundle data: {dataset_uri}@{revision}" + raise AssertionError(msg) monkeypatch.setattr( "policyengine_simulation_contract.dataset_uri.with_hf_revision", From 358486f9f56df54e209dcbd7fff6473dd22a3966 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:45:33 +0400 Subject: [PATCH 03/23] Fix ruff TRY, EM and BLE findings in the simulation observability library Exception messages move into a msg variable before the raise (EM), with the text unchanged. log_and_redact_exception's fallback log calls logger.exception with the same explicit exc_info, so its output is unchanged (TRY400). Its telemetry catch-all keeps except Exception under a BLE001 noqa, because the helper must never raise. Co-Authored-By: Claude Opus 5.5 --- .../src/policyengine_simulation_observability/errors.py | 4 ++-- .../src/policyengine_simulation_observability/stages.py | 5 ++--- .../src/policyengine_simulation_observability/telemetry.py | 3 ++- 3 files changed, 6 insertions(+), 6 deletions(-) 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 f90428554..dc3a4f9d1 100644 --- a/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/errors.py +++ b/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/errors.py @@ -77,7 +77,7 @@ def log_and_redact_exception( extra=payload, ) except Exception: # pragma: no cover - context key clashed with LogRecord - logger.error( + logger.exception( "Gateway %s failed (correlation_id=%s)", scope, correlation_id, @@ -91,7 +91,7 @@ def log_and_redact_exception( "gateway_error_redacted", attributes=payload, ) - except Exception: # pragma: no cover - defensive, never raise from logger + except Exception: # noqa: BLE001 # pragma: no cover - defensive, never raise from logger pass return f"{GENERIC_JOB_FAILURE_MESSAGE} (correlation_id={correlation_id})" diff --git a/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/stages.py b/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/stages.py index c042b8dca..dc7dc7f74 100644 --- a/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/stages.py +++ b/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/stages.py @@ -117,9 +117,8 @@ class StagePlan: def name(self, stage: Stage) -> str: """Return a configured stage name, rejecting unregistered use.""" if stage not in self.stages: - raise ValueError( - f"{stage.value!r} is not registered for {self.configuration.value!r}" - ) + msg = f"{stage.value!r} is not registered for {self.configuration.value!r}" + raise ValueError(msg) return stage.value diff --git a/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/telemetry.py b/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/telemetry.py index 409f58ad7..137b3f593 100644 --- a/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/telemetry.py +++ b/libs/policyengine-simulation-observability/src/policyengine_simulation_observability/telemetry.py @@ -44,7 +44,8 @@ def validate_observability_id(cls, value: str | None) -> str | None: return None normalized = normalize_observability_id(value) if normalized is None: - raise ValueError("observability_id must be a UUID") + msg = "observability_id must be a UUID" + raise ValueError(msg) return normalized From 7d6b137274f87e02017924e25dae5a8cb013f566 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:45:33 +0400 Subject: [PATCH 04/23] Fix ruff EM findings in the Stage 12 persistence library Exception messages move into a msg variable before the raise, with the text unchanged. Co-Authored-By: Claude Opus 5.5 --- .../policyengine_stage12_persistence/store.py | 57 ++++----- .../validation.py | 110 ++++++++++-------- .../validators.py | 22 ++-- 3 files changed, 102 insertions(+), 87 deletions(-) diff --git a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/store.py b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/store.py index 87996a455..df8658561 100644 --- a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/store.py +++ b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/store.py @@ -104,15 +104,19 @@ def normalized_database_url(database_url: str) -> URL: """Select psycopg 3 without changing the stored secret's URL format.""" if not database_url or database_url != database_url.strip(): - raise ValueError("Stage 12 database URL is missing or malformed") + msg = "Stage 12 database URL is missing or malformed" + raise ValueError(msg) try: parsed = make_url(database_url) except Exception as error: - raise ValueError("Stage 12 database URL is malformed") from error + msg = "Stage 12 database URL is malformed" + raise ValueError(msg) from error if parsed.drivername != "postgresql": - raise ValueError("Stage 12 database URL must use postgresql://") + msg = "Stage 12 database URL must use postgresql://" + raise ValueError(msg) if not parsed.username or not parsed.host or not parsed.database: - raise ValueError("Stage 12 database URL is incomplete") + msg = "Stage 12 database URL is incomplete" + raise ValueError(msg) return parsed.set(drivername="postgresql+psycopg") @@ -199,7 +203,8 @@ def __init__( """ if engine is None: if database_url is None: - raise ValueError("Stage 12 database URL is required") + msg = "Stage 12 database URL is required" + raise ValueError(msg) engine = create_stage12_engine(database_url) self.engine = engine self._open_connection = open_connection @@ -258,9 +263,10 @@ def create_or_resolve_report( .one_or_none() ) if existing is None: - raise ValueError( + msg = ( "comparison report conflict did not resolve to an existing identity" ) + raise ValueError(msg) resolved = _report(existing) require_report_conflict_matches(resolved, record) return ComparisonReportPersistenceResult(record=resolved, created=False) @@ -278,7 +284,8 @@ def get_report(self, evaluation_id: UUID) -> ComparisonReportRecord: .one_or_none() ) if row is None: - raise LookupError(f"comparison report {evaluation_id} does not exist") + msg = f"comparison report {evaluation_id} does not exist" + raise LookupError(msg) return _report(row) def list_simulations( @@ -321,9 +328,8 @@ def replace_report( .one_or_none() ) if row is None: - raise LookupError( - f"comparison report {record.evaluation_id} does not exist" - ) + msg = f"comparison report {record.evaluation_id} does not exist" + raise LookupError(msg) existing = _report(row) require_report_identity(existing, record) require_result_comparison_unchanged(existing, record) @@ -370,9 +376,8 @@ def replace_report_result_comparison( .one_or_none() ) if row is None: - raise LookupError( - f"comparison report {record.evaluation_id} does not exist" - ) + msg = f"comparison report {record.evaluation_id} does not exist" + raise LookupError(msg) existing = _report(row) require_comparison_update_only(existing, record) require_result_comparison_transition( @@ -426,9 +431,8 @@ def create_or_resolve_simulation( .one_or_none() ) if parent_row is None: - raise LookupError( - f"comparison report {record.evaluation_id} does not exist" - ) + msg = f"comparison report {record.evaluation_id} does not exist" + raise LookupError(msg) require_simulation_parent_matches(_report(parent_row), record) created = connection.execute(statement).mappings().one_or_none() if created is not None: @@ -450,9 +454,8 @@ def create_or_resolve_simulation( .one_or_none() ) if existing is None: - raise ValueError( - "comparison simulation conflict did not resolve to an existing identity" - ) + msg = "comparison simulation conflict did not resolve to an existing identity" + raise ValueError(msg) resolved = _simulation(existing) require_simulation_conflict_matches(resolved, record) return ComparisonSimulationPersistenceResult( @@ -477,9 +480,8 @@ def get_simulation( .one_or_none() ) if row is None: - raise LookupError( - f"comparison simulation {simulation_execution_id} does not exist" - ) + msg = f"comparison simulation {simulation_execution_id} does not exist" + raise LookupError(msg) return _simulation(row) def replace_simulation( @@ -507,10 +509,11 @@ def replace_simulation( .one_or_none() ) if row is None: - raise LookupError( + msg = ( "comparison simulation " f"{record.simulation_execution_id} does not exist" ) + raise LookupError(msg) existing = _simulation(row) require_simulation_identity(existing, record) require_lifecycle_transition(existing.status, record.status) @@ -576,10 +579,10 @@ def attach_simulation_invocation( .one_or_none() ) if row is None: - raise LookupError( - f"comparison simulation {simulation_execution_id} does not exist" - ) + msg = f"comparison simulation {simulation_execution_id} does not exist" + raise LookupError(msg) resolved = _simulation(row) if resolved.modal_invocation_id != modal_invocation_id: - raise ValueError("comparison simulation invocation identity changed") + msg = "comparison simulation invocation identity changed" + raise ValueError(msg) return resolved diff --git a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/validation.py b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/validation.py index 9f0e36ed4..d1f0be1a8 100644 --- a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/validation.py +++ b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/validation.py @@ -45,28 +45,35 @@ class Stage12RuntimeAccessError(RuntimeError): def _require_column_type(expected: object, actual: object, *, column: str) -> None: if isinstance(expected, ENUM): if not isinstance(actual, ENUM): - raise Stage12RuntimeAccessError(f"{column} has an unexpected type") + msg = f"{column} has an unexpected type" + raise Stage12RuntimeAccessError(msg) if expected.name != actual.name or tuple(expected.enums) != tuple(actual.enums): - raise Stage12RuntimeAccessError(f"{column} has an unexpected enum") + msg = f"{column} has an unexpected enum" + raise Stage12RuntimeAccessError(msg) return if isinstance(expected, UUID): if not isinstance(actual, UUID): - raise Stage12RuntimeAccessError(f"{column} has an unexpected type") + msg = f"{column} has an unexpected type" + raise Stage12RuntimeAccessError(msg) return if isinstance(expected, JSON): if not isinstance(actual, JSON): - raise Stage12RuntimeAccessError(f"{column} has an unexpected type") + msg = f"{column} has an unexpected type" + raise Stage12RuntimeAccessError(msg) return if isinstance(expected, DateTime): if not isinstance(actual, DateTime) or not actual.timezone: - raise Stage12RuntimeAccessError(f"{column} has an unexpected type") + msg = f"{column} has an unexpected type" + raise Stage12RuntimeAccessError(msg) return if isinstance(expected, String): if not isinstance(actual, String) or expected.length != actual.length: - raise Stage12RuntimeAccessError(f"{column} has an unexpected type") + msg = f"{column} has an unexpected type" + raise Stage12RuntimeAccessError(msg) return if isinstance(expected, Integer) and not isinstance(actual, Integer): - raise Stage12RuntimeAccessError(f"{column} has an unexpected type") + msg = f"{column} has an unexpected type" + raise Stage12RuntimeAccessError(msg) def _validate_table_schema( @@ -84,15 +91,13 @@ def _validate_table_schema( } declared_columns = list(table.c) # type: ignore[attr-defined] if set(live_columns) != {item.name for item in declared_columns}: - raise Stage12RuntimeAccessError( - f"{table_name} columns do not match the synchronized mapping" - ) + msg = f"{table_name} columns do not match the synchronized mapping" + raise Stage12RuntimeAccessError(msg) for declared in declared_columns: live = live_columns[declared.name] if declared.nullable != live["nullable"]: - raise Stage12RuntimeAccessError( - f"{table_name}.{declared.name} has unexpected nullability" - ) + msg = f"{table_name}.{declared.name} has unexpected nullability" + raise Stage12RuntimeAccessError(msg) _require_column_type( declared.type, live["type"], @@ -102,7 +107,8 @@ def _validate_table_schema( primary_key = inspector.get_pk_constraint(table_name, schema="public") declared_primary_key = [column.name for column in table.primary_key] # type: ignore[attr-defined] if primary_key.get("constrained_columns") != declared_primary_key: - raise Stage12RuntimeAccessError(f"{table_name} has an unexpected primary key") + msg = f"{table_name} has an unexpected primary key" + raise Stage12RuntimeAccessError(msg) unique_constraints = { item.get("name"): tuple(item.get("column_names") or ()) @@ -116,18 +122,16 @@ def _validate_table_schema( if unique_constraints.get(identity_constraint) != tuple( column.name for column in declared_unique.columns ): - raise Stage12RuntimeAccessError( - f"{table_name} has an unexpected identity constraint" - ) + msg = f"{table_name} has an unexpected identity constraint" + raise Stage12RuntimeAccessError(msg) live_checks = { item.get("name") for item in inspector.get_check_constraints(table_name, schema="public") } if not check_constraints.issubset(live_checks): - raise Stage12RuntimeAccessError( - f"{table_name} is missing required check constraints" - ) + msg = f"{table_name} is missing required check constraints" + raise Stage12RuntimeAccessError(msg) live_indexes = { item.get("name"): tuple(item.get("column_names") or ()) @@ -139,9 +143,8 @@ def _validate_table_schema( } for name, columns in declared_indexes.items(): if live_indexes.get(name) != columns: - raise Stage12RuntimeAccessError( - f"{table_name} is missing the expected {name} index" - ) + msg = f"{table_name} is missing the expected {name} index" + raise Stage12RuntimeAccessError(msg) def _validate_foreign_key(connection: Connection) -> None: @@ -158,16 +161,16 @@ def _validate_foreign_key(connection: Connection) -> None: None, ) if expected is None: - raise Stage12RuntimeAccessError("simulation parent foreign key is missing") + msg = "simulation parent foreign key is missing" + raise Stage12RuntimeAccessError(msg) if ( expected.get("referred_schema") not in {None, "public"} or expected.get("referred_table") != REPORT_TABLE_NAME or expected.get("referred_columns") != ["evaluation_id"] or (expected.get("options") or {}).get("ondelete") != "CASCADE" ): - raise Stage12RuntimeAccessError( - "simulation parent foreign key has an unexpected definition" - ) + msg = "simulation parent foreign key has an unexpected definition" + raise Stage12RuntimeAccessError(msg) def _has_schema_privilege(connection: Connection, privilege: str) -> bool: @@ -198,24 +201,23 @@ def _has_table_privilege( def _validate_permissions(connection: Connection, *, expected_role: str) -> None: current_role = connection.execute(select(func.current_user())).scalar_one() if current_role != expected_role: - raise Stage12RuntimeAccessError( - "Stage 12 credential authenticated as an unexpected role" - ) + msg = "Stage 12 credential authenticated as an unexpected role" + raise Stage12RuntimeAccessError(msg) if not _has_schema_privilege(connection, "USAGE"): - raise Stage12RuntimeAccessError("runtime role lacks public-schema USAGE") + msg = "runtime role lacks public-schema USAGE" + raise Stage12RuntimeAccessError(msg) if _has_schema_privilege(connection, "CREATE"): - raise Stage12RuntimeAccessError("runtime role has public-schema CREATE") + msg = "runtime role has public-schema CREATE" + raise Stage12RuntimeAccessError(msg) for table_name in (REPORT_TABLE_NAME, SIMULATION_TABLE_NAME): for privilege in REQUIRED_TABLE_PRIVILEGES: if not _has_table_privilege(connection, table_name, privilege): - raise Stage12RuntimeAccessError( - f"runtime role lacks {privilege} on {table_name}" - ) + msg = f"runtime role lacks {privilege} on {table_name}" + raise Stage12RuntimeAccessError(msg) for privilege in FORBIDDEN_TABLE_PRIVILEGES: if _has_table_privilege(connection, table_name, privilege): - raise Stage12RuntimeAccessError( - f"runtime role unexpectedly has {privilege} on {table_name}" - ) + msg = f"runtime role unexpectedly has {privilege} on {table_name}" + raise Stage12RuntimeAccessError(msg) def _exercise_runtime_dml(connection: Connection, *, environment: str) -> None: @@ -273,14 +275,16 @@ def _exercise_runtime_dml(connection: Connection, *, environment: str) -> None: .returning(comparison_reports.c.evaluation_id) ).scalar_one() if inserted_report != evaluation_id: - raise Stage12RuntimeAccessError("report DML canary insert failed") + msg = "report DML canary insert failed" + raise Stage12RuntimeAccessError(msg) selected_report = connection.execute( select(comparison_reports.c.evaluation_id).where( comparison_reports.c.evaluation_id == evaluation_id ) ).scalar_one() if selected_report != evaluation_id: - raise Stage12RuntimeAccessError("report DML canary select failed") + msg = "report DML canary select failed" + raise Stage12RuntimeAccessError(msg) updated_report = connection.execute( update(comparison_reports) .where(comparison_reports.c.evaluation_id == evaluation_id) @@ -288,7 +292,8 @@ def _exercise_runtime_dml(connection: Connection, *, environment: str) -> None: .returning(comparison_reports.c.evaluation_id) ).scalar_one() if updated_report != evaluation_id: - raise Stage12RuntimeAccessError("report DML canary update failed") + msg = "report DML canary update failed" + raise Stage12RuntimeAccessError(msg) simulation_values = { "simulation_execution_id": simulation_execution_id, @@ -322,14 +327,16 @@ def _exercise_runtime_dml(connection: Connection, *, environment: str) -> None: .returning(comparison_simulations.c.simulation_execution_id) ).scalar_one() if inserted_simulation != simulation_execution_id: - raise Stage12RuntimeAccessError("simulation DML canary insert failed") + msg = "simulation DML canary insert failed" + raise Stage12RuntimeAccessError(msg) selected_simulation = connection.execute( select(comparison_simulations.c.simulation_execution_id).where( comparison_simulations.c.simulation_execution_id == simulation_execution_id ) ).scalar_one() if selected_simulation != simulation_execution_id: - raise Stage12RuntimeAccessError("simulation DML canary select failed") + msg = "simulation DML canary select failed" + raise Stage12RuntimeAccessError(msg) updated_simulation = connection.execute( update(comparison_simulations) .where( @@ -339,7 +346,8 @@ def _exercise_runtime_dml(connection: Connection, *, environment: str) -> None: .returning(comparison_simulations.c.simulation_execution_id) ).scalar_one() if updated_simulation != simulation_execution_id: - raise Stage12RuntimeAccessError("simulation DML canary update failed") + msg = "simulation DML canary update failed" + raise Stage12RuntimeAccessError(msg) deleted_simulation = connection.execute( delete(comparison_simulations) .where( @@ -348,14 +356,16 @@ def _exercise_runtime_dml(connection: Connection, *, environment: str) -> None: .returning(comparison_simulations.c.simulation_execution_id) ).scalar_one() if deleted_simulation != simulation_execution_id: - raise Stage12RuntimeAccessError("simulation DML canary delete failed") + msg = "simulation DML canary delete failed" + raise Stage12RuntimeAccessError(msg) deleted_report = connection.execute( delete(comparison_reports) .where(comparison_reports.c.evaluation_id == evaluation_id) .returning(comparison_reports.c.evaluation_id) ).scalar_one() if deleted_report != evaluation_id: - raise Stage12RuntimeAccessError("report DML canary delete failed") + msg = "report DML canary delete failed" + raise Stage12RuntimeAccessError(msg) def validate_runtime_database( @@ -366,7 +376,8 @@ def validate_runtime_database( ) -> None: """Inspect the live schema and roll back a complete typed DML canary.""" if environment not in {"staging", "production"}: - raise Stage12RuntimeAccessError("environment must be staging or production") + msg = "environment must be staging or production" + raise Stage12RuntimeAccessError(msg) try: with engine.connect() as connection: _validate_permissions(connection, expected_role=expected_role) @@ -394,6 +405,5 @@ def validate_runtime_database( except Stage12RuntimeAccessError: raise except SQLAlchemyError as error: - raise Stage12RuntimeAccessError( - f"Stage 12 runtime database verification failed ({type(error).__name__})" - ) from None + msg = f"Stage 12 runtime database verification failed ({type(error).__name__})" + raise Stage12RuntimeAccessError(msg) from None diff --git a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/validators.py b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/validators.py index f7bcdf264..1d60019d7 100644 --- a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/validators.py +++ b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/validators.py @@ -109,9 +109,8 @@ def _require_equal_fields( ) -> None: for field_name in fields: if getattr(existing, field_name) != getattr(candidate, field_name): - raise ValueError( - f"comparison-run field {field_name} is immutable for an existing identity" - ) + msg = f"comparison-run field {field_name} is immutable for an existing identity" + raise ValueError(msg) def require_report_identity( @@ -158,7 +157,8 @@ def require_simulation_parent_matches( Versions, the Modal application, creation time and retention must all match. """ if child.evaluation_id != parent.evaluation_id: - raise ValueError("comparison child names a different parent evaluation_id") + msg = "comparison child names a different parent evaluation_id" + raise ValueError(msg) for field_name in ( "contract_version", "worker_version", @@ -168,7 +168,8 @@ def require_simulation_parent_matches( "retention_expires_at", ): if getattr(child, field_name) != getattr(parent, field_name): - raise ValueError(f"comparison child {field_name} does not match its parent") + msg = f"comparison child {field_name} does not match its parent" + raise ValueError(msg) def require_successful_report_replay( @@ -316,10 +317,11 @@ def require_lifecycle_transition( Repeating the current status is allowed; SUCCEEDED and SKIPPED are terminal. """ if candidate not in ALLOWED_LIFECYCLE_TRANSITIONS[current]: - raise ValueError( + msg = ( "comparison-run lifecycle cannot move " f"from {current.value} to {candidate.value}" ) + raise ValueError(msg) def require_aggregation_transition( @@ -332,10 +334,11 @@ def require_aggregation_transition( return to RUNNING for a retry. """ if candidate not in ALLOWED_AGGREGATION_TRANSITIONS[current]: - raise ValueError( + msg = ( "comparison-run aggregation cannot move " f"from {current.value} to {candidate.value}" ) + raise ValueError(msg) def require_result_comparison_transition( @@ -347,6 +350,5 @@ def require_result_comparison_transition( NOT_REQUESTED never changes, and MATCHED, DIFFERENT and FAILED are terminal. """ if candidate not in ALLOWED_RESULT_COMPARISON_TRANSITIONS[current]: - raise ValueError( - f"result comparison cannot move from {current.value} to {candidate.value}" - ) + msg = f"result comparison cannot move from {current.value} to {candidate.value}" + raise ValueError(msg) From 2df493ee25ef3416d9eaf037d04abde9a905a53a Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:45:39 +0400 Subject: [PATCH 05/23] Fix ruff TRY, EM and BLE findings in the simulation entrypoint Exception messages move into a msg variable before the raise (EM), with the text unchanged. forward() returns the proxied response from an else block (TRY300). The four Stage 12 diagnostic error logs stay traceback-free under a TRY400 noqa: three record only the error type, so a traceback could carry request or calculation data, and the dispatch timeout is fully described by its timeout. The Stage 12 context capture keeps its telemetry catch-all under a BLE001 noqa. Co-Authored-By: Claude Opus 5.5 --- .../authenticated_tests/test_deployed_auth.py | 6 ++- .../src/policyengine_simulation_entry/app.py | 11 ++-- .../policyengine_simulation_entry/backend.py | 25 +++++---- .../policyengine_simulation_entry/config.py | 53 +++++++++---------- .../stage12_backend.py | 13 ++--- .../tests/test_app.py | 12 +++-- .../tests/test_auth.py | 3 +- .../tests/test_backend.py | 6 ++- .../tests/test_stage12_backend.py | 3 +- 9 files changed, 73 insertions(+), 59 deletions(-) diff --git a/projects/policyengine-simulation-entry/authenticated_tests/test_deployed_auth.py b/projects/policyengine-simulation-entry/authenticated_tests/test_deployed_auth.py index ed5c1335e..9f22b8c37 100644 --- a/projects/policyengine-simulation-entry/authenticated_tests/test_deployed_auth.py +++ b/projects/policyengine-simulation-entry/authenticated_tests/test_deployed_auth.py @@ -21,7 +21,8 @@ def _required(name: str) -> str: value = os.environ.get(name, "").strip() if not value: - raise RuntimeError(f"{name} is required for authenticated deployment tests.") + msg = f"{name} is required for authenticated deployment tests." + raise RuntimeError(msg) return value @@ -70,7 +71,8 @@ def access_token() -> str: response.raise_for_status() token = response.json().get("access_token") if not isinstance(token, str) or not token: - raise RuntimeError("Auth0 response omitted access_token.") + msg = "Auth0 response omitted access_token." + raise RuntimeError(msg) return token diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py index 014b7fd22..7eb1265a0 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py @@ -379,7 +379,6 @@ async def forward( response.headers[OBSERVABILITY_ID_HEADER] = observability_id elif OBSERVABILITY_ID_HEADER in response.headers: del response.headers[OBSERVABILITY_ID_HEADER] - return response except BackendTimeout: runtime.event( "simulation_entry_backend_timeout", @@ -414,6 +413,8 @@ async def forward( **BACKEND_RESPONSE_HEADER, }, ) + else: + return response def temporary_submission_payload( report: ComparisonReportRecord, @@ -483,7 +484,7 @@ async def submit_temporary_stage12_report( # This temporary diagnostic route must convert every integration # failure into an HTTP response rather than crash the application. except Exception as error: # noqa: BLE001 - logger.error( + logger.error( # noqa: TRY400 # Stage 12 failure logs carry only the error type; a traceback could expose request or calculation data "stage12_direct_submission_failed", extra={ "request_id": request.state.request_id, @@ -536,7 +537,7 @@ async def get_temporary_stage12_report( # Reading temporary diagnostic state crosses an internal HTTP boundary # whose concrete exception types are implementation details. except Exception as error: # noqa: BLE001 - logger.error( + logger.error( # noqa: TRY400 # Stage 12 failure logs carry only the error type; a traceback could expose request or calculation data "stage12_direct_status_failed", extra={ "request_id": request.state.request_id, @@ -621,7 +622,7 @@ async def submit_comparison( timeout=STAGE12_MODAL_SUBMISSION_TIMEOUT_SECONDS, ) except TimeoutError: - logger.error( + logger.error( # noqa: TRY400 # an expected timeout, fully described by timeout_seconds; the traceback would show only asyncio cancellation "stage12_comparison_dispatch_timed_out", extra={ "request_id": request.state.request_id, @@ -631,7 +632,7 @@ async def submit_comparison( # Automatic Stage 12 work must never change the production v1 # response, including for unexpected integration failures. except Exception as error: # noqa: BLE001 - logger.error( + logger.error( # noqa: TRY400 # Stage 12 failure logs carry only the error type; a traceback could expose request or calculation data "stage12_comparison_dispatch_failed", extra={ "request_id": request.state.request_id, diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/backend.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/backend.py index f6a0edc91..81842927f 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/backend.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/backend.py @@ -116,7 +116,8 @@ async def get_token(self) -> str: ): await self._fetch() if self._token is None: # pragma: no cover - raise BackendAuthenticationError("Auth0 returned no token.") + msg = "Auth0 returned no token." + raise BackendAuthenticationError(msg) return self._token def invalidate(self) -> None: @@ -142,9 +143,8 @@ async def _fetch(self) -> None: except BackendAuthenticationError: raise except (httpx.HTTPError, TypeError, ValueError, ValidationError) as exc: - raise BackendAuthenticationError( - "Unable to obtain old-gateway credentials." - ) from exc + msg = "Unable to obtain old-gateway credentials." + raise BackendAuthenticationError(msg) from exc class OldGatewayBackend: @@ -195,7 +195,8 @@ async def close(self) -> None: def _runtime(self) -> tuple[httpx.AsyncClient, ClientCredentialsTokenProvider]: if self.client is None or self.token_provider is None: - raise BackendUnavailable("Old-gateway backend is not started.") + msg = "Old-gateway backend is not started." + raise BackendUnavailable(msg) return self.client, self.token_provider async def ready(self) -> bool: @@ -246,11 +247,14 @@ async def request( if connection_retries_remaining: connection_retries_remaining -= 1 continue - raise BackendUnavailable("Old gateway is unavailable.") from exc + msg = "Old gateway is unavailable." + raise BackendUnavailable(msg) from exc except httpx.TimeoutException as exc: - raise BackendTimeout("Old gateway timed out.") from exc + msg = "Old gateway timed out." + raise BackendTimeout(msg) from exc except httpx.RequestError as exc: - raise BackendUnavailable("Old gateway is unavailable.") from exc + msg = "Old gateway is unavailable." + raise BackendUnavailable(msg) from exc if response.status_code in {401, 403}: if auth_refreshes_remaining: @@ -260,9 +264,8 @@ async def request( # 403 before endpoint processing. One credential-refresh # replay is therefore safe for POST as well as GET. continue - raise BackendAuthenticationError( - "Old gateway rejected refreshed service credentials." - ) + msg = "Old gateway rejected refreshed service credentials." + raise BackendAuthenticationError(msg) return BackendResponse( status_code=response.status_code, diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/config.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/config.py index 7ebc60617..62bbed40c 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/config.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/config.py @@ -32,7 +32,8 @@ def _binary_flag(name: str) -> bool: return False if value == "1": return True - raise ConfigurationError(f"{name} must be exactly 0 or 1.") + msg = f"{name} must be exactly 0 or 1." + raise ConfigurationError(msg) def _normalized_issuer(value: str) -> str: @@ -53,13 +54,14 @@ def _validate_https_url( or parsed.username is not None or parsed.password is not None ): - raise ConfigurationError(f"{name} must be an absolute HTTPS URL.") + msg = f"{name} must be an absolute HTTPS URL." + raise ConfigurationError(msg) if parsed.params or parsed.query or parsed.fragment: - raise ConfigurationError( - f"{name} must not contain parameters, a query string, or a fragment." - ) + msg = f"{name} must not contain parameters, a query string, or a fragment." + raise ConfigurationError(msg) if not allow_path and parsed.path not in {"", "/"}: - raise ConfigurationError(f"{name} must not contain a path.") + msg = f"{name} must not contain a path." + raise ConfigurationError(msg) @dataclass(frozen=True) @@ -164,18 +166,15 @@ def validate(self) -> None: configured all together or not at all. """ if self.production and not self.auth_required: - raise ConfigurationError( - "Caller authentication cannot be disabled in production." - ) + msg = "Caller authentication cannot be disabled in production." + raise ConfigurationError(msg) if bool(self.auth_issuer) != bool(self.auth_audience): - raise ConfigurationError( - "Set both SIMULATION_ENTRYPOINT_AUTH_ISSUER and SIMULATION_ENTRYPOINT_AUTH_AUDIENCE." - ) + msg = "Set both SIMULATION_ENTRYPOINT_AUTH_ISSUER and SIMULATION_ENTRYPOINT_AUTH_AUDIENCE." + raise ConfigurationError(msg) if self.auth_required and not self.auth_issuer: - raise ConfigurationError( - "Caller authentication is required but issuer/audience are missing." - ) + msg = "Caller authentication is required but issuer/audience are missing." + raise ConfigurationError(msg) if self.auth_issuer: _validate_https_url( "SIMULATION_ENTRYPOINT_AUTH_ISSUER", @@ -191,9 +190,8 @@ def validate(self) -> None: } missing = [name for name, value in required_backend.items() if not value] if missing: - raise ConfigurationError( - f"Missing old-gateway configuration: {', '.join(missing)}." - ) + msg = f"Missing old-gateway configuration: {', '.join(missing)}." + raise ConfigurationError(msg) _validate_https_url( "OLD_GATEWAY_AUTH_ISSUER", @@ -201,7 +199,8 @@ def validate(self) -> None: ) if self.connect_timeout_seconds <= 0 or self.request_timeout_seconds <= 0: - raise ConfigurationError("Old-gateway timeouts must be positive.") + msg = "Old-gateway timeouts must be positive." + raise ConfigurationError(msg) _validate_https_url( "OLD_GATEWAY_URL", @@ -213,9 +212,8 @@ def validate(self) -> None: if upstream_hostname is None or not upstream_hostname.endswith( MODAL_GATEWAY_HOST_SUFFIX ): - raise ConfigurationError( - "OLD_GATEWAY_URL must point to the existing Modal gateway." - ) + msg = "OLD_GATEWAY_URL must point to the existing Modal gateway." + raise ConfigurationError(msg) try: assert_separate_manifest_names( @@ -225,7 +223,8 @@ def validate(self) -> None: except ValueError as error: raise ConfigurationError(str(error)) from error if self.stage12_retention_days != 30: - raise ConfigurationError("STAGE12_RETENTION_DAYS must be exactly 30.") + msg = "STAGE12_RETENTION_DAYS must be exactly 30." + raise ConfigurationError(msg) required_stage12 = { "STAGE12_V2_MANIFEST_ENVIRONMENT": self.stage12_v2_manifest_environment, "STAGE12_DATABASE_URL": self.stage12_database_url, @@ -240,13 +239,13 @@ def validate(self) -> None: "Stage 12 resources require: " + ", ".join(missing_stage12) + "." ) if self.stage12_enabled and not self.stage12_resources_configured: - raise ConfigurationError( + msg = ( "STAGE12_ENABLED=1 requires the Stage 12 manifest, database, " "and artifact resources." ) + raise ConfigurationError(msg) if self.stage12_database_url and not self.stage12_database_url.startswith( "postgresql://" ): - raise ConfigurationError( - "STAGE12_DATABASE_URL must use the canonical postgresql:// scheme." - ) + msg = "STAGE12_DATABASE_URL must use the canonical postgresql:// scheme." + raise ConfigurationError(msg) diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_backend.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_backend.py index 63c474296..9da56afb9 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_backend.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_backend.py @@ -141,9 +141,8 @@ async def spawn( ) invocation_id = getattr(call, "object_id", None) if not isinstance(invocation_id, str) or not invocation_id: - raise RuntimeError( - "Modal report dispatch returned no invocation identifier" - ) + msg = "Modal report dispatch returned no invocation identifier" + raise RuntimeError(msg) return invocation_id @@ -272,7 +271,8 @@ def _get_temporary_report( ) -> tuple[ComparisonReportRecord, tuple[ComparisonSimulationRecord, ...]]: report = self._store.get_report(evaluation_id) if report.environment != self._settings.environment: - raise LookupError(f"comparison report {evaluation_id} does not exist") + msg = f"comparison report {evaluation_id} does not exist" + raise LookupError(msg) return report, self._store.list_simulations(evaluation_id) def _resolve_report( @@ -397,7 +397,8 @@ def _prepare_automatic_report( response.get("job_id") if isinstance(response, dict) else None ) if not isinstance(production_identity, str) or not production_identity: - raise ValueError("accepted production response has no job identifier") + msg = "accepted production response has no job identifier" + raise ValueError(msg) version, worker, manifest_sha256 = self._manifest_loader.resolve( self._settings.stage12_v2_worker_version @@ -474,7 +475,7 @@ async def _spawn_report( ) try: raw_context = self._runtime.capture_context() - except Exception: + except Exception: # noqa: BLE001 # telemetry is non-fatal: a failed context capture dispatches without one raw_context = None captured_context = dict(raw_context) if isinstance(raw_context, dict) else {} captured_context.pop("observability_id", None) diff --git a/projects/policyengine-simulation-entry/tests/test_app.py b/projects/policyengine-simulation-entry/tests/test_app.py index eb3835f19..9baef55dc 100644 --- a/projects/policyengine-simulation-entry/tests/test_app.py +++ b/projects/policyengine-simulation-entry/tests/test_app.py @@ -84,7 +84,8 @@ async def submit_temporary_report(self, **submission): async def get_temporary_report(self, evaluation_id): self.polls.append(evaluation_id) if evaluation_id != self.report.evaluation_id: - raise LookupError("missing report") + msg = "missing report" + raise LookupError(msg) return self.report, self.simulations @@ -227,7 +228,8 @@ async def dispatch_after_production(self, **call): def test_comparison_dispatch_failure_cannot_change_production_response(backend): class FailingComparisonBackend: async def dispatch_after_production(self, **_): - raise RuntimeError("comparison unavailable") + msg = "comparison unavailable" + raise RuntimeError(msg) payload = { "job_id": "fc-123", @@ -395,7 +397,8 @@ def test_temporary_stage12_poll_not_found_has_short_retry_hint(backend): class MissingComparisonBackend(TemporaryComparisonBackend): async def get_temporary_report(self, evaluation_id): self.polls.append(evaluation_id) - raise LookupError("not created") + msg = "not created" + raise LookupError(msg) comparison = MissingComparisonBackend() app = create_app( @@ -897,7 +900,8 @@ async def request(self, *args, **kwargs): def test_unexpected_failure_preserves_correlation_headers_and_request_log(caplog): class FailingBackend(FakeBackend): async def request(self, *args, **kwargs): - raise RuntimeError("unexpected backend failure") + msg = "unexpected backend failure" + raise RuntimeError(msg) app = create_app( settings=make_settings(), diff --git a/projects/policyengine-simulation-entry/tests/test_auth.py b/projects/policyengine-simulation-entry/tests/test_auth.py index 49c2f4a80..b3e4549db 100644 --- a/projects/policyengine-simulation-entry/tests/test_auth.py +++ b/projects/policyengine-simulation-entry/tests/test_auth.py @@ -72,7 +72,8 @@ def __call__(self, token, runtime): def test_public_routes_do_not_require_token(monkeypatch): class Decoder: def __call__(self, token, runtime): - raise AssertionError("public route attempted authentication") + msg = "public route attempted authentication" + raise AssertionError(msg) monkeypatch.setattr(auth_module, "_decoder", lambda issuer, audience: Decoder()) app = create_app(settings=make_settings(), backend=FakeBackend()) diff --git a/projects/policyengine-simulation-entry/tests/test_backend.py b/projects/policyengine-simulation-entry/tests/test_backend.py index 85094b6c0..e28cb2a1d 100644 --- a/projects/policyengine-simulation-entry/tests/test_backend.py +++ b/projects/policyengine-simulation-entry/tests/test_backend.py @@ -199,7 +199,8 @@ def handler(request: httpx.Request) -> httpx.Response: json={"access_token": "token", "expires_in": 3600}, ) gateway_attempts += 1 - raise httpx.ConnectError("unavailable", request=request) + msg = "unavailable" + raise httpx.ConnectError(msg, request=request) backend = OldGatewayBackend( make_settings(), @@ -232,7 +233,8 @@ def handler(request: httpx.Request) -> httpx.Response: ) gateway_attempts += 1 if gateway_attempts == 1: - raise httpx.ConnectError("unavailable", request=request) + msg = "unavailable" + raise httpx.ConnectError(msg, request=request) return httpx.Response(200, json={"status": "complete"}) backend = OldGatewayBackend( diff --git a/projects/policyengine-simulation-entry/tests/test_stage12_backend.py b/projects/policyengine-simulation-entry/tests/test_stage12_backend.py index 55cdd73fe..b18cc02a8 100644 --- a/projects/policyengine-simulation-entry/tests/test_stage12_backend.py +++ b/projects/policyengine-simulation-entry/tests/test_stage12_backend.py @@ -44,7 +44,8 @@ def __init__(self, *, current: ComparisonReportRecord | None = None): def get_report(self, evaluation_id): self.events.append(("get_report", evaluation_id)) if self.current is None or self.current.evaluation_id != evaluation_id: - raise LookupError("missing report") + msg = "missing report" + raise LookupError(msg) return self.current def list_simulations(self, evaluation_id): From c5f1966e41abc54add3b5998d94507c9005ecc0b Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:45:39 +0400 Subject: [PATCH 06/23] Fix ruff TRY, EM and BLE findings in the simulation gateway Exception messages move into a msg variable before the raise (EM), with the text unchanged. The auth misconfiguration log keeps its message and gains a traceback through exc_info=True (TRY400). _runtime keeps RuntimeError and _resolve_policyengine_route keeps ValueError under a TRY004 noqa, because handlers and callers depend on those types. The job-status and budget-window handlers keep their catch-alls under a BLE001 noqa: call.get re-raises whatever the worker raised, and every failure must become a failed-job response or fall back to the seed state. The auth-bypass event keeps its catch-all too. The golden OpenAPI spec is unchanged. Co-Authored-By: Claude Opus 5.5 --- .../fixtures/gateway_endpoints.py | 6 ++- .../policyengine_simulation_gateway/auth.py | 19 +++++---- .../endpoints.py | 42 ++++++++++++------- .../generate_openapi.py | 21 ++++++---- .../tests/test_endpoints.py | 5 +-- 5 files changed, 59 insertions(+), 34 deletions(-) diff --git a/projects/policyengine-simulation-gateway/fixtures/gateway_endpoints.py b/projects/policyengine-simulation-gateway/fixtures/gateway_endpoints.py index a87b88f3e..1e01a6454 100644 --- a/projects/policyengine-simulation-gateway/fixtures/gateway_endpoints.py +++ b/projects/policyengine-simulation-gateway/fixtures/gateway_endpoints.py @@ -196,7 +196,8 @@ def items(self) -> ItemsView[str, object]: @classmethod def from_name(cls, name: str) -> Self: """Mock from_name that returns a MockDict based on name.""" - raise NotImplementedError("Mock not configured") + msg = "Mock not configured" + raise NotImplementedError(msg) class MockFunctionCall: @@ -375,7 +376,8 @@ def from_name(name: str, create_if_missing: bool = False) -> MockDict: if create_if_missing and name not in mock_dicts: mock_dicts[name] = {} if name not in mock_dicts: - raise KeyError(f"Mock dict not configured for: {name}") + msg = f"Mock dict not configured for: {name}" + raise KeyError(msg) return MockDict(mock_dicts[name]) class MockModalFunction: 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 ee0a27a71..d7f942bf1 100644 --- a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/auth.py +++ b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/auth.py @@ -98,11 +98,12 @@ def _get_decoder() -> JWTDecoder: issuer = os.environ.get(GATEWAY_AUTH_ISSUER_ENV) audience = os.environ.get(GATEWAY_AUTH_AUDIENCE_ENV) if not issuer or not audience: - raise RuntimeError( + msg = ( "Gateway auth misconfigured: set " f"{GATEWAY_AUTH_ISSUER_ENV} and {GATEWAY_AUTH_AUDIENCE_ENV} or " f"{GATEWAY_AUTH_DISABLED_ENV}=1 for local/test use." ) + raise RuntimeError(msg) if not issuer.endswith("/"): issuer = issuer + "/" return _build_decoder(issuer, audience) @@ -153,18 +154,20 @@ def enforce_production_auth_guard(runtime: ObservabilityRuntime) -> None: ack = os.environ.get(GATEWAY_AUTH_DISABLED_ACK_ENV, "") if modal_env is None or modal_env.lower() in PRODUCTION_MODAL_ENVIRONMENTS: - raise AuthDisabledInProductionError( + msg = ( f"Refusing to start gateway with {GATEWAY_AUTH_DISABLED_ENV}=1 " f"when {MODAL_ENVIRONMENT_ENV}={modal_env!r}. " "Disabling auth is only permitted in ephemeral dev environments." ) + raise AuthDisabledInProductionError(msg) if ack != GATEWAY_AUTH_DISABLED_ACK_VALUE: - raise AuthDisabledWithoutAckError( + msg = ( f"Refusing to start gateway with {GATEWAY_AUTH_DISABLED_ENV}=1 " f"unless {GATEWAY_AUTH_DISABLED_ACK_ENV}=" f"{GATEWAY_AUTH_DISABLED_ACK_VALUE} is also set." ) + raise AuthDisabledWithoutAckError(msg) banner = ( "\n" @@ -183,7 +186,7 @@ def enforce_production_auth_guard(runtime: ObservabilityRuntime) -> None: "ack_value_present": True, }, ) - except Exception: # pragma: no cover - observability must never block startup + except Exception: # noqa: BLE001 # pragma: no cover - observability must never block startup pass @@ -204,19 +207,21 @@ def enforce_auth_configured_guard() -> None: audience = os.environ.get(GATEWAY_AUTH_AUDIENCE_ENV) if bool(issuer) != bool(audience): - raise AuthMisconfiguredError( + msg = ( "Gateway auth is partially configured: set both " f"{GATEWAY_AUTH_ISSUER_ENV} and {GATEWAY_AUTH_AUDIENCE_ENV}, " "or clear both." ) + raise AuthMisconfiguredError(msg) if _auth_required() and (not issuer or not audience): - raise AuthMisconfiguredError( + msg = ( "Gateway auth is required but " f"{GATEWAY_AUTH_ISSUER_ENV}/{GATEWAY_AUTH_AUDIENCE_ENV} are not set " "in the container environment. Verify the " "'policyengine-gateway-auth' Modal secret is synced correctly." ) + raise AuthMisconfiguredError(msg) def require_auth( @@ -260,7 +265,7 @@ def require_auth( try: decoder = _get_decoder() except RuntimeError as exc: - logger.error("Gateway auth misconfigured: %s", exc) + logger.error("Gateway auth misconfigured: %s", exc, exc_info=True) raise HTTPException( status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail="Gateway authentication is not configured.", diff --git a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/endpoints.py b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/endpoints.py index 7271938ab..334356309 100644 --- a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/endpoints.py +++ b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/endpoints.py @@ -119,7 +119,8 @@ def _job_metadata_store() -> "modal.Dict": def _runtime(request: Request) -> ObservabilityRuntime: runtime = getattr(request.app.state, "policyengine_observability", None) if not isinstance(runtime, ObservabilityRuntime): - raise RuntimeError("Simulation gateway observability is not configured") + msg = "Simulation gateway observability is not configured" + raise RuntimeError(msg) # noqa: TRY004 # a missing runtime is a configuration error, not a bad argument; FastAPI handlers surface it unchanged return runtime @@ -296,10 +297,11 @@ def _policyengine_version_for_app( if len(matching) == 1 and not unclassified: return matching[0] if len(candidates) > 1: - raise ValueError( + msg = ( f"Ambiguous bundle for {country} version {model_version}; pass " "policyengine_version to select a bundle explicitly" ) + raise ValueError(msg) if not candidates: return None version = next(iter(candidates)) @@ -334,7 +336,8 @@ def _resolve_policyengine_route( ) -> RouteResolution: app_name = _routing_state_routes(state, "policyengine").get(policyengine_version) if not isinstance(app_name, str): - raise ValueError(f"Unknown policyengine.py version {policyengine_version}") + msg = f"Unknown policyengine.py version {policyengine_version}" + raise ValueError(msg) # noqa: TRY004 # an unknown version is a request error; route resolution callers catch ValueError return RouteResolution( app_name=app_name, response_version=response_version or policyengine_version, @@ -390,11 +393,12 @@ def _validate_legacy_version_matches_bundle( return model_version = country_bundle.get("model_version") if isinstance(model_version, str) and requested_version != model_version: - raise ValueError( + msg = ( f"Requested {country} version {requested_version} does not match " f"policyengine.py bundle {manifest.get('policyengine_version')} " f"({country} model version {model_version})" ) + raise ValueError(msg) def _resolve_from_active_state( @@ -433,7 +437,8 @@ def _resolve_from_active_state( ) if country_resolution is not None: return country_resolution - raise ValueError("Routing state does not define a latest route") + msg = "Routing state does not define a latest route" + raise ValueError(msg) country_resolution = _resolve_country_route( state, @@ -443,10 +448,11 @@ def _resolve_from_active_state( policyengine_app = _routing_state_routes(state, "policyengine").get(version) if country_resolution is not None and isinstance(policyengine_app, str): if country_resolution.app_name != policyengine_app: - raise ValueError( + msg = ( f"Ambiguous version {version} for country {country}; pass " "policyengine_version to select a bundle explicitly" ) + raise ValueError(msg) return _resolve_policyengine_route( state, policyengine_version=version, @@ -459,7 +465,8 @@ def _resolve_from_active_state( policyengine_version=version, ) - raise ValueError(f"Unknown version {version} for country {country}") + msg = f"Unknown version {version} for country {country}" + raise ValueError(msg) def _resolve_from_legacy_dicts( @@ -471,11 +478,13 @@ def _resolve_from_legacy_dicts( if policyengine_version is not None: policyengine_versions = _optional_modal_dict(POLICYENGINE_VERSION_DICT_NAME) if policyengine_versions is None: - raise ValueError(f"Unknown policyengine.py version {policyengine_version}") + msg = f"Unknown policyengine.py version {policyengine_version}" + raise ValueError(msg) try: app_name = policyengine_versions[policyengine_version] except KeyError: - raise ValueError(f"Unknown policyengine.py version {policyengine_version}") + msg = f"Unknown policyengine.py version {policyengine_version}" + raise ValueError(msg) return RouteResolution( app_name=app_name, response_version=policyengine_version, @@ -505,7 +514,8 @@ def _resolve_from_legacy_dicts( policyengine_version=version, bundle_manifest={}, ) - raise ValueError(f"Unknown version {resolved_version} for country {country}") + msg = f"Unknown version {resolved_version} for country {country}" + raise ValueError(msg) return RouteResolution( app_name=app_name, @@ -559,8 +569,9 @@ def _build_policyengine_bundle( try: capability = SPMCapability.model_validate(capability) except ValueError as exc: + msg = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - "SPM_CONFIGURATION_UNAVAILABLE", + msg, "This worker bundle has invalid canonical SPM capability metadata", ) from exc return PolicyEngineBundle( @@ -817,7 +828,8 @@ def resolve_route( """Resolve a country/package or policyengine.py version to a Modal app.""" country_lower = country.lower() if country_lower not in ("us", "uk"): - raise ValueError(f"Unknown country: {country}") + msg = f"Unknown country: {country}" + raise ValueError(msg) state = _active_routing_state() if state: @@ -1115,7 +1127,7 @@ def with_progress(status: JobStatus) -> dict: running_metadata = with_progress("running") with runtime.span(ANNUAL_IMPACT_STAGES.name(Stage.RESPONSE_SERIALIZATION)): return running_job_response(running_metadata, headers=response_headers) - except Exception as exc: + except Exception as exc: # noqa: BLE001 # call.get re-raises whatever the worker raised; every failure becomes a failed-job response, logged with its traceback by log_and_redact_exception if _is_modal_job_not_found(exc): runtime.record_exception(exc, handled=True, status_code=404) raise HTTPException(status_code=404, detail=f"Job not found: {job_id}") @@ -1185,7 +1197,7 @@ async def get_budget_window_job_status( try: with runtime.span(BUDGET_WINDOW_STAGES.name(Stage.MODAL_JOB_STATUS_POLL)): call = modal.FunctionCall.from_id(batch_job_id) - except Exception as exc: + except Exception as exc: # noqa: BLE001 # any lookup failure degrades to the seed state; logged below with its traceback at warning level # The endpoint degrades gracefully to the seed state (a successful # 202/200 response), so this must NOT be recorded as a request # error — recording the exception would emit a request failure and @@ -1221,7 +1233,7 @@ async def get_budget_window_job_status( return batch_status_response( build_batch_status_response(seed_state), headers=response_headers ) - except Exception as exc: + except Exception as exc: # noqa: BLE001 # call.get re-raises whatever the batch worker raised; every failure is persisted as a failed batch, logged with its traceback by log_and_redact_exception # Persist the failure so subsequent polls don't resurrect the # "submitted" status from the seed store (#448). We deliberately # overwrite the main job store entry as well as the seed so either 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 14770300d..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 @@ -49,7 +49,8 @@ async def submit_simulation(request: SimulationRequest) -> JobSubmitResponse: Routes to the appropriate simulation app based on country and version. Returns immediately with a job_id for polling. """ - raise NotImplementedError("Stub for OpenAPI generation") + msg = "Stub for OpenAPI generation" + raise NotImplementedError(msg) @app.post( "/simulate/economy/budget-window", @@ -66,7 +67,8 @@ async def submit_budget_window_batch( Returns immediately with a parent batch job ID for polling. """ - raise NotImplementedError("Stub for OpenAPI generation") + msg = "Stub for OpenAPI generation" + raise NotImplementedError(msg) @app.get( "/jobs/{job_id}", @@ -88,7 +90,8 @@ async def get_job_status(job_id: str) -> JobStatusResponse: - 500 with status="failed" and error on failure """ - raise NotImplementedError("Stub for OpenAPI generation") + msg = "Stub for OpenAPI generation" + raise NotImplementedError(msg) @app.get( "/budget-window-jobs/{batch_job_id}", @@ -107,17 +110,20 @@ async def get_budget_window_job_status( batch_job_id: str, ) -> BudgetWindowBatchStatusResponse: """Poll for budget-window batch status.""" - raise NotImplementedError("Stub for OpenAPI generation") + msg = "Stub for OpenAPI generation" + raise NotImplementedError(msg) @app.get("/versions", response_model=VersionsResponse) async def list_versions() -> VersionsResponse: """List all available routing versions.""" - raise NotImplementedError("Stub for OpenAPI generation") + msg = "Stub for OpenAPI generation" + raise NotImplementedError(msg) @app.get("/versions/{kind}", response_model=VersionMap) async def get_country_versions(kind: str) -> VersionMap: """Get available versions for policyengine, US, or UK routing.""" - raise NotImplementedError("Stub for OpenAPI generation") + msg = "Stub for OpenAPI generation" + raise NotImplementedError(msg) @app.get("/health", response_model=HealthResponse) async def health() -> HealthResponse: @@ -127,7 +133,8 @@ async def health() -> HealthResponse: @app.post("/ping", response_model=PingResponse) async def ping(request: PingRequest) -> PingResponse: """Verify the API is able to receive and process requests.""" - raise NotImplementedError("Stub for OpenAPI generation") + msg = "Stub for OpenAPI generation" + raise NotImplementedError(msg) return app diff --git a/projects/policyengine-simulation-gateway/tests/test_endpoints.py b/projects/policyengine-simulation-gateway/tests/test_endpoints.py index 0fa500987..be8a30f73 100644 --- a/projects/policyengine-simulation-gateway/tests/test_endpoints.py +++ b/projects/policyengine-simulation-gateway/tests/test_endpoints.py @@ -760,9 +760,8 @@ def test__given_uk_submission_without_data_and_manifest_commit__then_uses_popula mock_modal["dicts"]["simulation-api-routing-state"] = {"active": state} def reject_revision(dataset_uri, revision): - raise HuggingFaceDatasetReferenceError( - f"HF validation should not run for private GCS data: {dataset_uri}@{revision}" - ) + msg = f"HF validation should not run for private GCS data: {dataset_uri}@{revision}" + raise HuggingFaceDatasetReferenceError(msg) monkeypatch.setattr( "policyengine_simulation_contract.dataset_uri.with_hf_revision", From 5aaa4f96ada73131f0729bce9737341c79d1d71c Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:45:46 +0400 Subject: [PATCH 07/23] Fix ruff TRY, EM and BLE findings in the simulation executor Exception messages move into a msg variable before the raise (EM), with the text unchanged. Raises inside try bodies move into private helpers (_parse_policy_period_dates, _certified_us_spm_capability, _require_artifact_digest, _require_local_authority_column), so the same exceptions reach the same handlers (TRY301, TRY300); dump_package_version narrows its try to the file read. qualify_stage12_parity, a local operator CLI that nothing calls or catches, now raises TypeError for a JSON value that is not an object. The other TRY004 sites keep their types under a noqa: they cross a Modal boundary, are documented or asserted by tests, or are recorded by type name in telemetry and Stage 12 error summaries. The Stage 12 comparison failure log stays traceback-free under a TRY400 noqa. Worker, cleanup and telemetry catch-alls keep except Exception under a BLE001 noqa. The Modal images copy src/modal and policyengine_simulation_executor, so their layers change like any source edit; src/modal/app.py gains no future import. Co-Authored-By: Claude Opus 5.5 --- .../scripts/dump_package_version.py | 12 +- .../src/modal/_image_setup.py | 12 +- .../src/modal/app.py | 11 +- .../src/modal/budget_window_context.py | 12 +- .../src/modal/budget_window_results.py | 8 +- .../src/modal/budget_window_scheduler.py | 4 +- .../src/modal/segmented_national.py | 8 +- .../src/modal/smoke_app.py | 3 +- .../src/modal/utils/extract_stage12_bundle.py | 6 +- .../src/modal/utils/qualify_stage12_parity.py | 9 +- .../modal/utils/update_version_registry.py | 23 ++-- .../modal/utils/validate_v2_worker_release.py | 11 +- .../src/modal/v2_app.py | 6 +- .../modal/v2_static_runtime_files_smoke.py | 9 +- .../artifact_store.py | 5 +- .../hf_access_validation.py | 31 ++--- .../precompute.py | 27 +++-- .../release_bundle.py | 31 ++--- .../segmented_national_reduce.py | 11 +- .../simulation_output_geographic.py | 36 ++++-- .../simulation_runtime.py | 75 +++++++----- .../policyengine_simulation_executor/spm.py | 93 +++++++++------ .../stage12_artifacts.py | 42 ++++--- .../stage12_bundle/assertions.py | 6 +- .../stage12_bundle/manifest.py | 18 ++- .../stage12_bundle/parsing.py | 49 ++++---- .../stage12_parity.py | 48 +++++--- .../stage12_qualification.py | 33 ++++-- .../stage12_runtime/aggregation.py | 36 ++++-- .../stage12_runtime/comparison.py | 2 +- .../stage12_runtime/coordination.py | 73 ++++++++---- .../stage12_runtime/output_planning.py | 18 ++- .../stage12_runtime/simulation.py | 41 ++++--- .../stage12_worker_validation.py | 24 ++-- .../uk_local_authority_metadata.py | 107 +++++++++--------- .../tests/test_artifact_store.py | 6 +- .../tests/test_baseline_artifacts.py | 17 ++- .../tests/test_budget_window_scheduler.py | 3 +- .../tests/test_canonical_spm.py | 3 +- .../tests/test_gcp_credentials.py | 3 +- .../tests/test_segmented_national.py | 9 +- .../tests/test_segmented_national_reduce.py | 3 +- .../tests/test_simulation_output_builder.py | 9 +- .../tests/test_stage12_modal_app.py | 3 +- .../tests/test_stage12_output_planning.py | 3 +- .../tests/test_stage12_runtime.py | 15 ++- 46 files changed, 614 insertions(+), 400 deletions(-) diff --git a/projects/policyengine-simulation-executor/scripts/dump_package_version.py b/projects/policyengine-simulation-executor/scripts/dump_package_version.py index f704f5fd8..32732e013 100644 --- a/projects/policyengine-simulation-executor/scripts/dump_package_version.py +++ b/projects/policyengine-simulation-executor/scripts/dump_package_version.py @@ -19,15 +19,15 @@ def get_package_version_from_lock( try: with open(lock_file_path, "rb") as f: lock_data = tomllib.load(f) - - for package in lock_data.get("package", []): - if package.get("name") == package_name: - return package.get("version") - - return None except FileNotFoundError: return None + for package in lock_data.get("package", []): + if package.get("name") == package_name: + return package.get("version") + + return None + version = get_package_version_from_lock(sys.argv[2], sys.argv[1]) diff --git a/projects/policyengine-simulation-executor/src/modal/_image_setup.py b/projects/policyengine-simulation-executor/src/modal/_image_setup.py index 4451ee676..82290a293 100644 --- a/projects/policyengine-simulation-executor/src/modal/_image_setup.py +++ b/projects/policyengine-simulation-executor/src/modal/_image_setup.py @@ -48,12 +48,13 @@ def fetch_artifacts( logger = logging.getLogger(__name__) if not manifest: - raise RuntimeError( + msg = ( "No artifact manifest was resolved on the deploying machine. " "Set POLICYENGINE_MANIFEST_DIGEST (from the precompute run's " "MANIFEST_DIGEST= output line) and POLICYENGINE_ARTIFACT_BUCKET, " "with GCP credentials available, then redeploy." ) + raise RuntimeError(msg) # Freshness gate: refuse to bake artifacts computed for a different # version-set than the one this image installs. @@ -66,12 +67,13 @@ def fetch_artifacts( for env_var, manifest_value in expected_env.items(): image_value = os.environ.get(env_var) if image_value != manifest_value: - raise RuntimeError( + msg = ( f"Stale artifact manifest: the image has " f"{env_var}={image_value!r} but the manifest was computed " f"for {manifest_value!r}. Re-run the precompute for the " "current version-set." ) + raise RuntimeError(msg) installed_data_version = None receipt_path = os.environ.get("POLICYENGINE_BUNDLE_RECEIPT") @@ -92,12 +94,13 @@ def fetch_artifacts( country, ) elif installed_data_version != receipt["data_version"]: - raise RuntimeError( + msg = ( f"Stale artifact manifest: installed {country} data version is " f"{installed_data_version!r} but the manifest was computed for " f"{receipt['data_version']!r}. Re-run the precompute for the " "current version-set." ) + raise RuntimeError(msg) if client is None: creds_kwargs = {} @@ -166,7 +169,8 @@ def fetch_artifacts( if not (data_folder / artifact["filename"]).exists() ] if absent: - raise RuntimeError(f"Artifact fetch did not produce expected files: {absent}") + msg = f"Artifact fetch did not produce expected files: {absent}" + raise RuntimeError(msg) def snapshot_models() -> None: diff --git a/projects/policyengine-simulation-executor/src/modal/app.py b/projects/policyengine-simulation-executor/src/modal/app.py index af34fd99f..6d2ff4426 100644 --- a/projects/policyengine-simulation-executor/src/modal/app.py +++ b/projects/policyengine-simulation-executor/src/modal/app.py @@ -49,10 +49,11 @@ def _version_from_env_or_local_bundle_package(env_var: str, package: str) -> str return value if modal.is_local(): return get_bundled_package_version(package) - raise RuntimeError( + msg = ( f"{env_var} must be set in the Modal image environment for remote " "simulation workers." ) + raise RuntimeError(msg) def _version_from_env_or_local_bundle(env_var: str, country: str) -> str: @@ -61,10 +62,11 @@ def _version_from_env_or_local_bundle(env_var: str, country: str) -> str: return value if modal.is_local(): return get_bundled_country_model_version(country) - raise RuntimeError( + msg = ( f"{env_var} must be set in the Modal image environment for remote " "simulation workers." ) + raise RuntimeError(msg) POLICYENGINE_VERSION = _version_from_env_or_local_bundle_package( @@ -156,10 +158,11 @@ def _deploy_time_artifact_inputs() -> tuple[str, dict | None]: store = ArtifactStore(bucket or None) payload = store.read_manifest(digest) if payload is None: - raise RuntimeError( + msg = ( f"Artifact manifest {digest} is not in the store: the deploy " "must consume a digest published by the precompute run." ) + raise RuntimeError(msg) # Validate on the runner so a corrupt or foreign manifest fails the # deploy here, not mid-image-build. manifest = ArtifactManifest.model_validate(payload) @@ -256,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: + except Exception: # noqa: BLE001 # telemetry is non-fatal: a failed attribute must never fail the Modal call pass diff --git a/projects/policyengine-simulation-executor/src/modal/budget_window_context.py b/projects/policyengine-simulation-executor/src/modal/budget_window_context.py index 6d8199725..f3ecafb42 100644 --- a/projects/policyengine-simulation-executor/src/modal/budget_window_context.py +++ b/projects/policyengine-simulation-executor/src/modal/budget_window_context.py @@ -64,18 +64,22 @@ def build_batch_context( request = BudgetWindowBatchRequest.model_validate(params) metadata = params.get("_metadata") if not isinstance(metadata, dict): - raise ValueError("Missing internal batch metadata") + msg = "Missing internal batch metadata" + raise ValueError(msg) # noqa: TRY004 # documented ValueError for malformed batch metadata, raised across the run_budget_window_batch Modal boundary resolved_app_name = metadata.get("resolved_app_name") resolved_version = metadata.get("resolved_version") bundle_payload = metadata.get("policyengine_bundle") if not isinstance(resolved_app_name, str) or not resolved_app_name: - raise ValueError("Missing resolved_app_name in batch metadata") + msg = "Missing resolved_app_name in batch metadata" + raise ValueError(msg) if not isinstance(resolved_version, str) or not resolved_version: - raise ValueError("Missing resolved_version in batch metadata") + msg = "Missing resolved_version in batch metadata" + raise ValueError(msg) if not isinstance(bundle_payload, dict): - raise ValueError("Missing policyengine_bundle in batch metadata") + msg = "Missing policyengine_bundle in batch metadata" + raise ValueError(msg) # noqa: TRY004 # documented ValueError for malformed batch metadata, raised across the run_budget_window_batch Modal boundary return BudgetWindowBatchContext( batch_job_id=batch_job_id, diff --git a/projects/policyengine-simulation-executor/src/modal/budget_window_results.py b/projects/policyengine-simulation-executor/src/modal/budget_window_results.py index 719a5651b..4c1c06ae1 100644 --- a/projects/policyengine-simulation-executor/src/modal/budget_window_results.py +++ b/projects/policyengine-simulation-executor/src/modal/budget_window_results.py @@ -48,7 +48,8 @@ def extract_annual_impact( receipt = validate_spm_result(child_result, spm, expected_year=simulation_year) budget = child_result.get("budget", {}) if not isinstance(budget, dict): - raise ValueError("Malformed budget-window child result: missing budget object") + msg = "Malformed budget-window child result: missing budget object" + raise ValueError(msg) # noqa: TRY004 # a malformed child result is a data error, like the missing-figure ValueError below; log_and_redact_exception records the type name missing_keys = [ key @@ -57,9 +58,8 @@ def extract_annual_impact( ] if missing_keys: missing = ", ".join(f"budget.{key}" for key in missing_keys) - raise ValueError( - f"Malformed budget-window child result: missing numeric {missing}" - ) + msg = f"Malformed budget-window child result: missing numeric {missing}" + raise ValueError(msg) tax_revenue_impact = budget["tax_revenue_impact"] # UK worker results omit the state fiscal layer entirely; coerce to 0.0 diff --git a/projects/policyengine-simulation-executor/src/modal/budget_window_scheduler.py b/projects/policyengine-simulation-executor/src/modal/budget_window_scheduler.py index 8d4e95c56..0947b09d4 100644 --- a/projects/policyengine-simulation-executor/src/modal/budget_window_scheduler.py +++ b/projects/policyengine-simulation-executor/src/modal/budget_window_scheduler.py @@ -238,7 +238,7 @@ def poll_running_children_once(self) -> bool: child_result = handle.call.get(timeout=0) except TimeoutError: continue - except Exception as exc: + except Exception as exc: # noqa: BLE001 # call.get re-raises whatever the child worker raised; every failure fails the batch, logged with its traceback by log_and_redact_exception detail = spm_error_detail(exc) if detail: self.state.errors = [detail] @@ -277,7 +277,7 @@ def poll_running_children_once(self) -> bool: child_result=child_result, spm=self.state.request_payload.get("spm"), ) - except Exception as exc: + except Exception as exc: # noqa: BLE001 # any unparseable child result fails the batch, logged with its traceback by log_and_redact_exception detail = spm_error_detail(exc) if detail: self.state.errors = [detail] diff --git a/projects/policyengine-simulation-executor/src/modal/segmented_national.py b/projects/policyengine-simulation-executor/src/modal/segmented_national.py index 1603daccd..5a16eae36 100644 --- a/projects/policyengine-simulation-executor/src/modal/segmented_national.py +++ b/projects/policyengine-simulation-executor/src/modal/segmented_national.py @@ -223,7 +223,8 @@ def __init__( self.country = str(params.get("country") or "us").lower() groups = national_region_groups(self.country) if not groups: - raise ValueError(f"No national partition for country {self.country!r}") + msg = f"No national partition for country {self.country!r}" + raise ValueError(msg) self.groups = groups # Lazy handle, no RPC until first spawn (budget-window precedent). self.child_func = self.modal.Function.from_name(app_name, SEGMENT_FUNCTION_NAME) @@ -376,10 +377,11 @@ def _collect(self, children: list[ChildExecution]) -> list[dict]: if child.attempt > 1 else "" ) - raise RuntimeError( + msg = ( "Segmented national child failed for group " f"{child.group_label}{retry_note}: {redacted}" - ) from None + ) + raise RuntimeError(msg) from None pending.discard(index) poll_errors.pop(index, None) progress_made = True diff --git a/projects/policyengine-simulation-executor/src/modal/smoke_app.py b/projects/policyengine-simulation-executor/src/modal/smoke_app.py index f70815a48..65ad946e8 100644 --- a/projects/policyengine-simulation-executor/src/modal/smoke_app.py +++ b/projects/policyengine-simulation-executor/src/modal/smoke_app.py @@ -99,7 +99,8 @@ def _validate_installed_uk_local_authority_dataset() -> str: """Require the installed UK dataset to match one packaged boundary version.""" dataset_path = resolve_local_bundle_dataset_path("uk", None) if dataset_path is None: - raise RuntimeError("installed certified UK dataset is unavailable") + msg = "installed certified UK dataset is unavailable" + raise RuntimeError(msg) metadata = detect_uk_local_authority_metadata_from_hdf(dataset_path) return metadata.boundary_version.value 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 881a54fd2..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 @@ -17,9 +17,11 @@ def _expectations(values: list[str]) -> dict[str, str]: for value in values: name, separator, expected = value.partition("=") if not separator or not name or not expected: - raise ValueError("--expect values must use non-empty NAME=VALUE syntax") + msg = "--expect values must use non-empty NAME=VALUE syntax" + raise ValueError(msg) if name in expectations: - raise ValueError(f"Duplicate Stage 12 bundle assertion {name!r}") + msg = f"Duplicate Stage 12 bundle assertion {name!r}" + raise ValueError(msg) expectations[name] = expected return expectations 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 cc6085d4b..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 @@ -16,7 +16,8 @@ def _json_object(path: Path) -> dict[str, Any]: value = json.loads(path.read_text(encoding="utf-8")) if not isinstance(value, dict): - raise ValueError(f"{path} must contain one JSON object") + msg = f"{path} must contain one JSON object" + raise TypeError(msg) return value @@ -27,10 +28,12 @@ def _tolerances(path: Path | None) -> dict[str, NumericalTolerance]: tolerances = {} for field, configuration in value.items(): if not isinstance(configuration, dict): - raise ValueError(f"tolerance for {field!r} must be an object") + msg = f"tolerance for {field!r} must be an object" + raise TypeError(msg) unknown = set(configuration) - {"absolute", "relative"} if unknown: - raise ValueError(f"tolerance for {field!r} has unknown fields") + msg = f"tolerance for {field!r} has unknown fields" + raise ValueError(msg) tolerances[field] = NumericalTolerance( absolute=float(configuration.get("absolute", 0.0)), relative=float(configuration.get("relative", 0.0)), 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 8d7e5a334..fcd045843 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 @@ -220,7 +220,8 @@ def validate_routing_state( a manifest unless its version is in ``required_policyengine_manifests``. """ if state.get("schema_version") != 1: - raise ValueError("Routing state schema_version must be 1") + msg = "Routing state schema_version must be 1" + raise ValueError(msg) required_manifests = set(required_policyengine_manifests) required_latest_routes = ["us", "uk"] @@ -230,31 +231,34 @@ def validate_routing_state( for route_name in required_latest_routes: latest = state["latest"].get(route_name) if not latest: - raise ValueError(f"Routing state latest.{route_name} is missing") + msg = f"Routing state latest.{route_name} is missing" + raise ValueError(msg) if latest not in state["routes"][route_name]: - raise ValueError( - f"Routing state latest.{route_name}={latest!r} has no route" - ) + msg = f"Routing state latest.{route_name}={latest!r} has no route" + raise ValueError(msg) for policyengine_version, app_name in state["routes"]["policyengine"].items(): manifest = state["bundles"].get(policyengine_version) if not isinstance(manifest, dict): if policyengine_version in required_manifests: - raise ValueError( + msg = ( "Routing state policyengine route " f"{policyengine_version!r} has no bundle manifest" ) + raise ValueError(msg) continue if manifest.get("policyengine_version") != policyengine_version: - raise ValueError( + msg = ( "Routing state bundle manifest key does not match " f"policyengine_version for {policyengine_version!r}" ) + raise ValueError(msg) if manifest.get("app_name") != app_name: - raise ValueError( + msg = ( "Routing state bundle manifest app_name does not match route " f"for {policyengine_version!r}" ) + raise ValueError(msg) def _assert_version_matches_manifest( @@ -264,10 +268,11 @@ def _assert_version_matches_manifest( manifest_version: str, ) -> None: if requested_version != manifest_version: - raise ValueError( + msg = ( f"{requested_name} {requested_version!r} does not match the " f"policyengine.py bundle manifest value {manifest_version!r}" ) + raise ValueError(msg) def build_next_routing_state( 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 b430a7f97..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 @@ -36,7 +36,8 @@ def _spawn_validation( result = call.get(timeout=1_000) invocation_id = getattr(call, "object_id", None) if not isinstance(invocation_id, str) or not invocation_id: - raise RuntimeError(f"{function_name} returned no Modal invocation identifier") + msg = f"{function_name} returned no Modal invocation identifier" + raise RuntimeError(msg) return result, invocation_id @@ -57,7 +58,8 @@ def validate_release(*, environment: str) -> V2WorkerValidation: environment=environment, ) if not isinstance(result, dict) or result.get("validated") is not True: - raise RuntimeError(f"{function_name} did not report successful validation") + msg = f"{function_name} did not report successful validation" + raise RuntimeError(msg) expected = { "country": country_bundle.country, "policyengine_version": resolved.bundle.policyengine_version, @@ -74,9 +76,8 @@ def validate_release(*, environment: str) -> V2WorkerValidation: if result.get(key) != expected_value } if mismatches: - raise RuntimeError( - f"{function_name} reported bundle values that differ from deployment" - ) + msg = f"{function_name} reported bundle values that differ from deployment" + raise RuntimeError(msg) invocation_ids[country_bundle.country] = invocation_id return V2WorkerValidation( validated=True, diff --git a/projects/policyengine-simulation-executor/src/modal/v2_app.py b/projects/policyengine-simulation-executor/src/modal/v2_app.py index 9a15e1583..f7c04b6af 100644 --- a/projects/policyengine-simulation-executor/src/modal/v2_app.py +++ b/projects/policyengine-simulation-executor/src/modal/v2_app.py @@ -73,7 +73,8 @@ def _external_assertions(environment: dict[str, str]) -> dict[str, str]: _external_assertions(dict(os.environ)), ) if os.environ.get("MODAL_APP_NAME") not in {None, "", APP_NAME}: - raise RuntimeError("MODAL_APP_NAME does not match the bundle-derived v2 name") + msg = "MODAL_APP_NAME does not match the bundle-derived v2 name" + raise RuntimeError(msg) app = modal.App(APP_NAME) @@ -311,7 +312,8 @@ def coordinate_report( coordinator_invocation_id = modal.current_function_call_id() if coordinator_invocation_id is None: - raise RuntimeError("Modal coordinator invocation identifier is unavailable") + msg = "Modal coordinator invocation identifier is unavailable" + raise RuntimeError(msg) runtime = init_process_observability( service_name="policyengine-stage12-coordinator", service_role="stage12_report_coordinator", 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 a86d94a38..47c15c8b9 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 @@ -25,7 +25,8 @@ def validate_installed_uk_local_authority_dataset() -> str: """ dataset_path = resolve_local_bundle_dataset_path("uk", None) if dataset_path is None: - raise RuntimeError("installed certified UK dataset is unavailable") + msg = "installed certified UK dataset is unavailable" + raise RuntimeError(msg) return detect_uk_local_authority_metadata_from_hdf( dataset_path ).boundary_version.value @@ -59,13 +60,15 @@ def main() -> None: "lad23_coordinates": 361, } if {key: observed.get(key) for key in expected_counts} != expected_counts: - raise RuntimeError( + msg = ( f"Stage 12 static runtime file smoke returned invalid counts: {observed!r}" ) + raise RuntimeError(msg) supported_versions = {version.value for version in UKLocalAuthorityBoundaryVersion} if observed.get("installed_dataset_boundary_version") not in supported_versions: - raise RuntimeError( + msg = ( "Stage 12 static runtime file smoke could not identify the installed " f"UK dataset boundary version: {observed!r}" ) + raise RuntimeError(msg) print("Stage 12 static runtime file smoke OK") diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/artifact_store.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/artifact_store.py index 12ef86cd0..98476ba85 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/artifact_store.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/artifact_store.py @@ -43,9 +43,8 @@ def resolve_bucket_name(explicit: str | None = None) -> str: """ bucket = explicit or os.environ.get(ARTIFACT_BUCKET_ENV) if not bucket: - raise RuntimeError( - f"No artifact bucket configured: pass one or set {ARTIFACT_BUCKET_ENV}." - ) + msg = f"No artifact bucket configured: pass one or set {ARTIFACT_BUCKET_ENV}." + raise RuntimeError(msg) return bucket 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 76db81c62..30358bb9b 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 @@ -110,19 +110,19 @@ def build_uk_bundle_access_plan() -> tuple[HFAccessTarget, ...]: if parsed is None: continue if parsed.revision is None: - raise RuntimeError( - f"UK bundle dataset {dataset_name!r} does not pin a revision" - ) + msg = f"UK bundle dataset {dataset_name!r} does not pin a revision" + raise RuntimeError(msg) repo_type = bundle.dataset_repo_types.get(dataset_name) if repo_type == "model": validated_repo_type: Literal["model", "dataset"] = "model" elif repo_type == "dataset": validated_repo_type = "dataset" else: - raise RuntimeError( + msg = ( f"UK bundle dataset {dataset_name!r} has unsupported Hugging Face " f"repository type {repo_type!r}" ) + raise RuntimeError(msg) target = HFAccessTarget( repo_id=parsed.repo_id, repo_type=validated_repo_type, @@ -132,7 +132,8 @@ def build_uk_bundle_access_plan() -> tuple[HFAccessTarget, ...]: key = (target.repo_id, target.repo_type, target.path, target.revision) targets[key] = target if not targets: - raise RuntimeError("The UK .py bundle contains no Hugging Face artifacts") + msg = "The UK .py bundle contains no Hugging Face artifacts" + raise RuntimeError(msg) return tuple(targets[key] for key in sorted(targets)) @@ -170,18 +171,20 @@ def validate_uk_private_hf_access( """Validate identity, read-only permissions, and every planned artifact.""" token = environment.get(HF_RUNTIME_ENV_NAME) if not token: - raise RuntimeError(f"{HF_RUNTIME_ENV_NAME} is required") + msg = f"{HF_RUNTIME_ENV_NAME} is required" + raise RuntimeError(msg) if not targets: - raise RuntimeError("At least one Hugging Face artifact must be checked") + msg = "At least one Hugging Face artifact must be checked" + raise RuntimeError(msg) identity = HFWhoAmIIdentity.model_validate(whoami_loader(token)) token_identity = identity.auth.access_token if token_identity.display_name != EXPECTED_HF_TOKEN_DISPLAY_NAME: - raise RuntimeError( - f"Unexpected Hugging Face token identity: {token_identity.display_name!r}" - ) + msg = f"Unexpected Hugging Face token identity: {token_identity.display_name!r}" + raise RuntimeError(msg) if token_identity.role != "fineGrained": - raise RuntimeError("The UK private-data token must be fine-grained") + msg = "The UK private-data token must be fine-grained" + raise RuntimeError(msg) permissions = set(token_identity.fine_grained.global_permissions) permissions.update( @@ -190,9 +193,11 @@ def validate_uk_private_hf_access( for permission in scope.permissions ) if "repo.content.read" not in permissions: - raise RuntimeError("The UK private-data token has no repository read scope") + msg = "The UK private-data token has no repository read scope" + raise RuntimeError(msg) if any("write" in permission for permission in permissions): - raise RuntimeError("The UK private-data token must not have write permission") + msg = "The UK private-data token must not have write permission" + raise RuntimeError(msg) artifacts = tuple(metadata_loader(target, token) for target in targets) return HFCredentialAudit( diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/precompute.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/precompute.py index 77a765289..9b147d908 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/precompute.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/precompute.py @@ -139,11 +139,12 @@ def cohort_identity(year: int, group: list[str]) -> BaselineArtifactIdentity: year=year, ) if identity is None: - raise RuntimeError( + msg = ( f"Cohort request for year {year} group {group} does not qualify " "for a deterministic baseline id — writer and reader predicates " "have diverged." ) + raise RuntimeError(msg) return identity @@ -200,7 +201,8 @@ def plan_artifacts_impl(bucket: str) -> PrecomputePlan: store = ArtifactStore(bucket) groups = national_region_groups(PRECOMPUTE_COUNTRY) if not groups: - raise RuntimeError(f"No national partition for {PRECOMPUTE_COUNTRY!r}") + msg = f"No national partition for {PRECOMPUTE_COUNTRY!r}" + raise RuntimeError(msg) datasets: list[DatasetPlanEntry] = [] baselines: list[BaselinePlanEntry] = [] @@ -258,11 +260,12 @@ def build_dataset_impl(bucket: str, expected: DatasetPlanEntry) -> DatasetBuildR identity = collect_dataset_identity(PRECOMPUTE_COUNTRY, expected.year) if identity.store_path != expected.path: - raise RuntimeError( + msg = ( "Planned and in-container dataset identities disagree " f"({expected.path} != {identity.store_path}); refusing to upload " "under a mismatched key." ) + raise RuntimeError(msg) data_folder = resolve_data_folder() country_module = _country_module(PRECOMPUTE_COUNTRY) @@ -276,7 +279,8 @@ def build_dataset_impl(bucket: str, expected: DatasetPlanEntry) -> DatasetBuildR local_file = Path(data_folder) / identity.filename if not local_file.exists(): - raise RuntimeError(f"ensure_datasets produced no file at {local_file}") + msg = f"ensure_datasets produced no file at {local_file}" + raise RuntimeError(msg) uploaded = ArtifactStore(bucket).upload_file(identity.store_path, local_file) return DatasetBuildResult( year=expected.year, @@ -328,19 +332,21 @@ def _prepare_cohort_baseline( region_code=resolution.code, ) if baseline.id != expected.simulation_id: - raise RuntimeError( + msg = ( "Planned and in-container baseline ids disagree " f"({expected.simulation_id} != {baseline.id}); refusing to act " "under a mismatched key." ) + raise RuntimeError(msg) planned_storage_id = Path(expected.path).stem storage_id = getattr(baseline, "storage_id", baseline.id) if storage_id != planned_storage_id: - raise RuntimeError( + msg = ( "Planned and in-container baseline storage ids disagree " f"({planned_storage_id} != {storage_id}); refusing to act " "under a mismatched key." ) + raise RuntimeError(msg) # The extras economic_impact_analysis applies unconditionally before # ensure(); the artifact must carry them or every request would fail @@ -363,10 +369,11 @@ def compute_baseline_impl( store, data_folder, baseline = _prepare_cohort_baseline(bucket, expected) if not isinstance(baseline, ArtifactBaselineSimulation): - raise RuntimeError( + msg = ( "Precompute built a plain Simulation — the qualifying predicate " "rejected the cohort request shape." ) + raise RuntimeError(msg) # noqa: TRY004 # a precompute invariant failure, not a bad argument; raised across the compute_baseline Modal boundary and asserted by test_precompute started = time.monotonic() baseline.ensure() @@ -376,7 +383,8 @@ def compute_baseline_impl( getattr(baseline, "storage_id", baseline.id) ) if not artifact_file.exists(): - raise RuntimeError(f"ensure() left no artifact at {artifact_file}") + msg = f"ensure() left no artifact at {artifact_file}" + raise RuntimeError(msg) uploaded = store.upload_file(expected.path, artifact_file) return BaselineComputeResult( year=expected.year, @@ -413,10 +421,11 @@ def verify_determinism_impl( store.download_file(expected.path, artifact_file) baseline.ensure() if baseline.artifact_outcome != OUTCOME_HIT: - raise RuntimeError( + msg = ( "Determinism gate could not load the artifact it verifies " f"(outcome={baseline.artifact_outcome})" ) + raise RuntimeError(msg) selection = getattr(baseline, "spm_config", None) fresh = Simulation( 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 42daa350f..4c0eb5346 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 @@ -107,7 +107,8 @@ def _default_dataset_sha256( def _normalise_country(country: str) -> str: country = country.lower() if country not in SUPPORTED_COUNTRIES: - raise ValueError(f"Unsupported country: {country}") + msg = f"Unsupported country: {country}" + raise ValueError(msg) return country @@ -262,23 +263,23 @@ def get_bundled_package_version(package: str) -> str: """Return a runtime package version from policyengine.py's bundle manifest.""" bundle = _current_policyengine_bundle() if not isinstance(bundle, Mapping): - raise TypeError("PolicyEngine.py bundle metadata is unavailable") + msg = "PolicyEngine.py bundle metadata is unavailable" + raise TypeError(msg) packages = bundle.get("packages") if not isinstance(packages, Mapping): - raise TypeError("PolicyEngine.py bundle has no package mapping") + msg = "PolicyEngine.py bundle has no package mapping" + raise TypeError(msg) package_metadata = packages.get(package) if not isinstance(package_metadata, Mapping): - raise TypeError( - f"PolicyEngine.py bundle has no metadata for package {package!r}" - ) + msg = f"PolicyEngine.py bundle has no metadata for package {package!r}" + raise TypeError(msg) version = package_metadata.get("version") if not isinstance(version, str) or not version.strip(): - raise ValueError( - f"PolicyEngine.py bundle has no version for package {package!r}" - ) + msg = f"PolicyEngine.py bundle has no version for package {package!r}" + raise ValueError(msg) return version.strip() @@ -290,7 +291,8 @@ def _derive_data_package_version( """Return the data package version certified by policyengine.py.""" version = bundled_data_package.get("version") or manifest_data_package_version if not isinstance(version, str) or not version.strip(): - raise ValueError("PolicyEngine.py bundle has no data package version") + msg = "PolicyEngine.py bundle has no data package version" + raise ValueError(msg) return version.strip() @@ -398,7 +400,8 @@ def _split_requested_revision(requested_data: str) -> tuple[str, str | None]: return requested_data, None dataset_name, revision = requested_data.rsplit("@", maxsplit=1) if not dataset_name or not revision: - raise ValueError(f"Invalid dataset revision reference: {requested_data}") + msg = f"Invalid dataset revision reference: {requested_data}" + raise ValueError(msg) return dataset_name, revision @@ -421,10 +424,11 @@ def resolve_bundle_dataset_name(country: str, requested_data: str | None) -> str if revision is not None: uri = bundle.dataset_uris.get(requested_without_revision) if uri is None: - raise ValueError( + msg = ( "Unknown dataset revision reference " f"{requested_data!r} for country {bundle.country!r}" ) + raise ValueError(msg) return _with_hf_revision_unvalidated(uri, revision) return requested_without_revision @@ -575,10 +579,11 @@ def resolve_runtime_bundle_dataset_uri( dataset_uri = resolve_bundle_dataset_uri(country, requested_without_revision) if dataset_uri == requested_without_revision: if revision is not None: - raise ValueError( + msg = ( "Unknown dataset revision reference " f"{requested_data!r} for country {bundle.country!r}" ) + raise ValueError(msg) return requested_data return runtime_dataset_uri( diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/segmented_national_reduce.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/segmented_national_reduce.py index d3a87244f..669897178 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/segmented_national_reduce.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/segmented_national_reduce.py @@ -64,12 +64,14 @@ def concatenate_microdata( concatenated in child order with source dtypes restored. """ if not child_outputs: - raise ValueError("No child outputs to reduce") + msg = "No child outputs to reduce" + raise ValueError(msg) payloads = [] for index, child in enumerate(child_outputs): payload = child.get("_microdata") if not payload: - raise ValueError(f"Child {index} returned no _microdata payload") + msg = f"Child {index} returned no _microdata payload" + raise ValueError(msg) payloads.append(payload) entities = list(payloads[0]["baseline"]) @@ -136,9 +138,8 @@ def build_national_output( district table populates. No model recompute occurs. """ if country.lower() != "us": - raise ValueError( - f"No segmented-national reduce for country {country!r} (US only)" - ) + msg = f"No segmented-national reduce for country {country!r} (US only)" + raise ValueError(msg) sides = concatenate_microdata(child_outputs) datasets = { diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_geographic.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_geographic.py index 281e00107..f7c97449c 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_geographic.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_geographic.py @@ -45,10 +45,11 @@ def _required_uk_geography_lookup_csv_path(spec: UKGeographyAssetSpec) -> str: download_missing_assets=True, ) if path is None: - raise FileNotFoundError( + msg = ( f"Required UK {spec.geography_type} lookup CSV " f"{spec.lookup_csv_filename!r} could not be resolved" ) + raise FileNotFoundError(msg) return path @@ -61,17 +62,21 @@ def _complete_uk_geography_output( """Require every UK geography record to contain lookup-owned metadata.""" output = build_geographic_impact_output(value) if output is None or not output.root: - raise ValueError("UK geography output did not contain result records") + msg = "UK geography output did not contain result records" + raise ValueError(msg) for record in output.root: record_values = record.model_dump(mode="python") code = record_values.get(code_field) name = record_values.get(name_field) if not isinstance(code, str) or not code.strip(): - raise ValueError(f"UK geography output is missing {code_field}") + msg = f"UK geography output is missing {code_field}" + raise ValueError(msg) if not isinstance(name, str) or not name.strip() or name == code: - raise ValueError(f"UK geography {code!r} is missing a lookup name") + msg = f"UK geography {code!r} is missing a lookup name" + raise ValueError(msg) if record_values.get("x") is None or record_values.get("y") is None: - raise ValueError(f"UK geography {code!r} is missing lookup coordinates") + msg = f"UK geography {code!r} is missing lookup coordinates" + raise ValueError(msg) return output @@ -160,14 +165,17 @@ def _public_congressional_district_code(record: Mapping[str, Any]) -> str: if district_number is None: district_number = geoid % 100 if state_fips is None: - raise ValueError("Congressional district output is missing state FIPS") + msg = "Congressional district output is missing state FIPS" + raise ValueError(msg) state_abbreviation_by_fips, at_large_states = _policyengine_us_district_metadata() state_abbreviation = state_abbreviation_by_fips.get(state_fips) if state_abbreviation is None: - raise ValueError(f"Unknown state FIPS code: {state_fips}") + msg = f"Unknown state FIPS code: {state_fips}" + raise ValueError(msg) if district_number is None: - raise ValueError("Congressional district output is missing district number") + msg = "Congressional district output is missing district number" + raise ValueError(msg) public_district_number = ( 1 if state_abbreviation in at_large_states else district_number @@ -238,11 +246,12 @@ def _require_uk_area_column( separate run. """ if column not in household.columns: - raise ValueError( + msg = ( f"UK {breakdown} breakdowns need {column} on the simulated dataset, " "which carries no area codes; for such a dataset they need a run on " "the local-area dataset its bundle routes these regions to." ) + raise ValueError(msg) def build_uk_constituency_impact( @@ -293,7 +302,8 @@ def build_uk_local_authority_impact( baseline_household = _output_household(baseline) _require_uk_area_column(baseline_household, "la_code_oa", "local-authority") if uk_local_authority_metadata is None: - raise ValueError("UK local-authority boundary metadata is required") + msg = "UK local-authority boundary metadata is required" + raise ValueError(msg) from policyengine.outputs.uk_geography_impact import ( compute_longwise_uk_geography_impacts, @@ -319,13 +329,15 @@ def build_uk_local_authority_impact( for numeric_record in numeric_records: code = numeric_record.get("local_authority_code") if not isinstance(code, str) or not code: - raise ValueError("UK local-authority output contains an invalid code") + msg = "UK local-authority output contains an invalid code" + raise ValueError(msg) metadata = display_metadata.get(code) if metadata is None: - raise ValueError( + msg = ( f"UK local-authority code {code!r} is not part of the detected " f"{boundary_version.value.upper()} boundary version" ) + raise ValueError(msg) records.append( { **numeric_record, 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 93b9c2a5b..001bfd0ba 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 @@ -208,19 +208,25 @@ def _parse_year(params: dict[str, Any]) -> int: def _parse_policy_period(period_key: object) -> tuple[datetime, datetime | None]: """Parse effective dates or inclusive ``start.stop`` policy periods.""" text = str(period_key) - parts = text.split(".") try: - if len(parts) not in {1, 2}: - 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] - start = dates[0] - stop = dates[1] if len(dates) == 2 else None - if stop is not None and stop < start: - raise ValueError + return _parse_policy_period_dates(text) except ValueError: - raise ValueError(f"Invalid policy period key: {text!r}") from None + msg = f"Invalid policy period key: {text!r}" + raise ValueError(msg) from None + + +def _parse_policy_period_dates(text: str) -> tuple[datetime, datetime | None]: + """Parse a date or a ``start.stop`` range; raise a bare ``ValueError`` if invalid.""" + parts = text.split(".") + if len(parts) not in {1, 2}: + 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] + start = dates[0] + stop = dates[1] if len(dates) == 2 else None + if stop is not None and stop < start: + raise ValueError return start, stop @@ -421,9 +427,8 @@ def release(reference: ArtifactPathReference) -> tuple[str, str]: selected = manifest.datasets.get(dataset_selection.name) if selected is None: - raise ValueError( - f"UK dataset {dataset_selection.name!r} is not in the certified bundle" - ) + msg = f"UK dataset {dataset_selection.name!r} is not in the certified bundle" + raise ValueError(msg) certified_name = next( ( name @@ -433,12 +438,13 @@ def release(reference: ArtifactPathReference) -> tuple[str, str]: None, ) if certified_name is None: - raise ValueError( + msg = ( f"UK region {region!r} reweights households with {matrix_name}, but the " f"certified bundle has no {matrix_name} from the release of " f"{dataset_selection.name!r}; a matrix from another release does not " "line up with its households." ) + raise ValueError(msg) certified = materialize_dataset( "uk", certified_name, data_dir=default_download_dir() ) @@ -453,32 +459,36 @@ def release(reference: ArtifactPathReference) -> tuple[str, str]: simulation_output_geographic._required_uk_geography_lookup_csv_path(spec) paths = resolve_uk_geography_asset_paths(spec, download_missing_assets=False) if Path(paths.weight_matrix_path).resolve() != Path(certified.path).resolve(): - raise ValueError( + msg = ( f"UK region {region!r} would read {paths.weight_matrix_path}, not the " f"{matrix_name} certified with {dataset_selection.name!r} " f"({certified.path})." ) + raise ValueError(msg) year = str(dataset.year) with h5py.File(paths.weight_matrix_path, "r") as matrix: if year not in matrix: covered = ", ".join(sorted(matrix)) - raise ValueError( + msg = ( f"UK region {region!r} reweights households with {matrix_name}, " f"which has no weights for {year} (it covers {covered})." ) + raise ValueError(msg) weights = matrix[year] if not isinstance(weights, h5py.Dataset): - raise TypeError(f"{matrix_name} entry {year} is not a weight matrix") + msg = f"{matrix_name} entry {year} is not a weight matrix" + raise TypeError(msg) matrix_households = weights.shape[-1] households = len(pd.DataFrame(dataset.data.entity_data["household"])) if households != matrix_households: - raise ValueError( + msg = ( f"UK region {region!r} reweights households with {matrix_name}, " f"which was built for {matrix_households} households; the selected " f"UK dataset has {households}. Constituency and local-authority " "runs on this dataset need a local-area dataset that carries " "constituency and local-authority codes." ) + raise ValueError(msg) def _region_parent_dataset_reference( @@ -491,7 +501,8 @@ def _region_parent_dataset_reference( while isinstance(parent_code, str) and parent_code: if parent_code in visited: - raise ValueError(f"Region hierarchy cycle at {parent_code}") + msg = f"Region hierarchy cycle at {parent_code}" + raise ValueError(msg) visited.add(parent_code) parent_region = country_module.model.get_region(parent_code) @@ -516,10 +527,11 @@ def _reject_unscoped_us_place_region(region_code: str, region: Region) -> None: return if getattr(region, "scoping_strategy", None) is not None: return - raise ValueError( + msg = ( "US place regions are not yet supported for runtime simulation because " "policyengine.py does not expose place-level dataset scoping." ) + raise ValueError(msg) def _resolve_region( @@ -543,7 +555,8 @@ def _resolve_region( if region is None and country == "uk": region = _build_uk_weight_replacement_region(region_code) if region is None: - raise ValueError(f"Unsupported {country.upper()} region: {region_code}") + msg = f"Unsupported {country.upper()} region: {region_code}" + raise ValueError(msg) if country == "us": _reject_unscoped_us_place_region(region_code, region) @@ -592,20 +605,23 @@ def _resolve_region_group( for code in (params.get("region_group") or []) ] if not codes: - raise ValueError("region_group must contain at least one region code") + msg = "region_group must contain at least one region code" + raise ValueError(msg) members = [] for code in codes: region = country_module.model.get_region(code) if region is None: - raise ValueError(f"Unsupported {country.upper()} region in group: {code}") + msg = f"Unsupported {country.upper()} region in group: {code}" + raise ValueError(msg) scoping_strategy = getattr(region, "scoping_strategy", None) if not isinstance(scoping_strategy, RowFilterStrategy): - raise ValueError( + msg = ( f"Region group member '{code}' is not a row-filter region " f"(got {type(scoping_strategy).__name__}); region groups must " "preserve national weights." ) + raise ValueError(msg) # noqa: TRY004 # an unsupported region in the request; run_simulation_impl handles ValueError specially members.append(scoping_strategy) return RegionResolution( @@ -618,7 +634,8 @@ def _resolve_region_group( def _country_module(country: str) -> ModuleType: country = country.lower() if country not in {"us", "uk"}: - raise ValueError(f"Unsupported country: {country}") + msg = f"Unsupported country: {country}" + raise ValueError(msg) return import_module(f"policyengine.tax_benefit_models.{country}") @@ -646,7 +663,8 @@ def _resolve_dataset_selection( ) -> DatasetSelection: """Resolve the bundle's regional or default dataset to a managed name.""" if "data" in params or "data_version" in params: - raise ValueError("Dataset overrides are not supported") + msg = "Dataset overrides are not supported" + raise ValueError(msg) country = params.get("country", "us").lower() bundle = get_country_release_bundle(country) if region_resolution is not None and region_resolution.dataset_reference: @@ -666,10 +684,11 @@ def _resolve_dataset_selection( validate_hf=False, ): return DatasetSelection(name, uri, name == bundle.default_dataset) - raise ValueError( + msg = ( f"Unsupported dataset {dataset_reference!r} for country {country!r}; " "choose a name in the certified release manifest" ) + raise ValueError(msg) def _load_dataset( diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/spm.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/spm.py index 240fb9674..2dcc18383 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/spm.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/spm.py @@ -58,8 +58,9 @@ def runtime_spm_capability() -> SPMCapability | None: bundle = get_current_bundle() configured = bundle.get("measurements", {}).get("spm") except (ImportError, ValueError, TypeError, AttributeError) as exc: + msg = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - "SPM_CONFIGURATION_UNAVAILABLE", + msg, "The installed SPM bundle configuration is unavailable", ) from exc if configured is None: @@ -67,28 +68,40 @@ def runtime_spm_capability() -> SPMCapability | None: if "spm" not in Simulation.model_fields or not hasattr( Simulation, "spm_provenance" ): + msg = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - "SPM_CONFIGURATION_UNAVAILABLE", + msg, "The installed wrapper does not support canonical SPM", ) try: - from policyengine_us import Microsimulation - - if not ( - hasattr(Microsimulation, "spm_config") - and callable(getattr(Microsimulation, "spm_provenance", None)) - ): - raise ValueError("The installed US model does not support canonical SPM") - capability = SPMCapability(defaults=configured) - forecast = _forecast(capability.defaults.forecast_content_sha256) - forecast.entry( - forecast.years[0], - scenario=capability.defaults.scenario, - as_of=capability.defaults.as_of, - ) - return capability + return _certified_us_spm_capability(configured) except (ImportError, ValueError, TypeError, OSError) as exc: - raise SPMInputError("SPM_CONFIGURATION_UNAVAILABLE", str(exc)) from exc + msg = "SPM_CONFIGURATION_UNAVAILABLE" + raise SPMInputError(msg, str(exc)) from exc + + +def _certified_us_spm_capability(configured: object) -> SPMCapability: + """Certify the bundle's SPM defaults against the installed US model and forecast. + + Raises ``ImportError``, ``ValueError``, ``TypeError`` or ``OSError`` when the + US model, the configuration or the pinned forecast cannot support them. + """ + from policyengine_us import Microsimulation + + if not ( + hasattr(Microsimulation, "spm_config") + and callable(getattr(Microsimulation, "spm_provenance", None)) + ): + msg = "The installed US model does not support canonical SPM" + raise ValueError(msg) + capability = SPMCapability(defaults=configured) + forecast = _forecast(capability.defaults.forecast_content_sha256) + forecast.entry( + forecast.years[0], + scenario=capability.defaults.scenario, + as_of=capability.defaults.as_of, + ) + return capability @lru_cache(maxsize=64) @@ -114,7 +127,8 @@ def _prevalidate_selection( selection = json.loads(selection_json) forecast = _forecast(selection["forecast_content_sha256"]) if selection["county_vintage"] != "2020": - raise ValueError("Unsupported county vintage: use 2020") + msg = "Unsupported county vintage: use 2020" + raise ValueError(msg) provider = PolicyEngineSPMProvider( forecast, **{ @@ -139,7 +153,8 @@ def _prevalidate_selection( as_of=selection["as_of"], ) except ValueError as exc: - raise SPMInputError("SPM_GEOGRAPHY_UNAVAILABLE", str(exc)) from None + msg = "SPM_GEOGRAPHY_UNAVAILABLE" + raise SPMInputError(msg, str(exc)) from None # Bound at import, so clearing still works while a test has replaced one of @@ -204,15 +219,18 @@ def normalize_runtime_spm(params: dict[str, Any]) -> dict[str, Any] | None: # plain scenario error; use the same narrow translation as the # calculator's Frame and Axiom adapters, leaving other errors alone. if str(exc).startswith("Unknown forecast scenario:"): - raise SPMInputError("SPM_SCENARIO_UNAVAILABLE", str(exc)) from exc - raise SPMInputError("SPM_SETTINGS_INVALID", str(exc)) from exc + msg = "SPM_SCENARIO_UNAVAILABLE" + raise SPMInputError(msg, str(exc)) from exc + msg = "SPM_SETTINGS_INVALID" + raise SPMInputError(msg, str(exc)) from exc return selection def _required_string(source: Mapping[str, object], field_name: str) -> str: value = source.get(field_name) if not isinstance(value, str) or not value: - raise ValueError(f"SPM calculation receipt has no valid {field_name}") + msg = f"SPM calculation receipt has no valid {field_name}" + raise ValueError(msg) return value @@ -227,7 +245,8 @@ def compact_spm_provenance( package versions that belong on the simulation API wire contract. """ if not isinstance(country_receipt, Mapping): - raise TypeError("SPM calculation receipt must be an object") + msg = "SPM calculation receipt must be an object" + raise TypeError(msg) resolved = SPMSelection.model_validate(selection) forecast_sha256 = _required_string(country_receipt, "forecast_sha256") for field_name, expected_value in ( @@ -236,18 +255,20 @@ def compact_spm_provenance( ("geography_kind", resolved.geography_kind), ): if country_receipt.get(field_name) != expected_value: - raise ValueError( - f"SPM calculation receipt {field_name} differs from the selection" - ) + msg = f"SPM calculation receipt {field_name} differs from the selection" + raise ValueError(msg) raw_versions = country_receipt.get("runtime_versions") if not isinstance(raw_versions, Mapping): - raise TypeError("SPM calculation receipt has no runtime versions") + msg = "SPM calculation receipt has no runtime versions" + raise TypeError(msg) runtime_versions = SPMRuntimeVersions.model_validate(raw_versions) raw_years = country_receipt.get("years") if not isinstance(raw_years, Mapping): - raise TypeError("SPM calculation receipt has no executed years") + msg = "SPM calculation receipt has no executed years" + raise TypeError(msg) if any(not isinstance(year, str) for year in raw_years): - raise TypeError("SPM calculation receipt year keys must be strings") + msg = "SPM calculation receipt year keys must be strings" + raise TypeError(msg) return build_spm_provenance( forecast_id=_required_string(country_receipt, "forecast_id"), forecast_sha256=forecast_sha256, @@ -264,14 +285,16 @@ def simulation_spm_calculation_provenance( """Read and compact one simulation's resolved SPM receipt.""" resolved = SPMSelection.model_validate(selection) if getattr(simulation, "spm_config", None) != resolved.model_dump(mode="json"): + msg = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - "SPM_CONFIGURATION_UNAVAILABLE", + msg, "Simulation ignored the requested SPM selection", ) receipt = getattr(simulation, "spm_provenance", None) if not callable(receipt): + msg = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - "SPM_CONFIGURATION_UNAVAILABLE", + msg, "Simulation has no valid SPM calculation receipt", ) try: @@ -280,8 +303,9 @@ def simulation_spm_calculation_provenance( receipt=compact_spm_provenance(receipt(), resolved), ) except (TypeError, ValueError) as exc: + msg = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - "SPM_CONFIGURATION_UNAVAILABLE", + msg, "Simulation has no valid SPM calculation receipt", ) from exc @@ -310,8 +334,9 @@ def simulation_spm_result( if expected_year is not None and str(expected_year) not in ( comparison.baseline.receipt.years ): + msg = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - "SPM_CONFIGURATION_UNAVAILABLE", + msg, "Result SPM provenance does not cover the requested year", ) return { diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_artifacts.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_artifacts.py index 272c0ff89..72272360d 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_artifacts.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_artifacts.py @@ -62,9 +62,11 @@ def comparison_run_prefix( naive ``created_at``. """ if not environment or "/" in environment or ".." in environment: - raise ValueError("invalid Stage 12 artifact environment") + msg = "invalid Stage 12 artifact environment" + raise ValueError(msg) if created_at.tzinfo is None: - raise ValueError("Stage 12 artifact timestamp must include a timezone") + msg = "Stage 12 artifact timestamp must include a timezone" + raise ValueError(msg) return ( f"stage-12-runs/{environment}/{created_at:%Y}/{created_at:%m}/{evaluation_id}" ) @@ -101,9 +103,11 @@ def _frame_payload( source = source.reindex(sorted(source.columns), axis=1) identifier = PARQUET_CONTRACT.identifier_column_template.format(entity=entity) if identifier not in source.columns: - raise ValueError(f"simulation output entity {entity!r} has no {identifier}") + msg = f"simulation output entity {entity!r} has no {identifier}" + raise ValueError(msg) if source[identifier].duplicated().any(): - raise ValueError(f"simulation output entity {entity!r} has duplicate IDs") + msg = f"simulation output entity {entity!r} has duplicate IDs" + raise ValueError(msg) source = source.sort_values(identifier, kind="stable").reset_index(drop=True) dtypes[entity] = {column: str(dtype) for column, dtype in source.dtypes.items()} identifier_values[entity] = [ @@ -113,7 +117,8 @@ def _frame_payload( source.insert(0, PARQUET_CONTRACT.entity_column, entity) parts.append(source) if not parts: - raise ValueError("simulation output contains no entity tables") + msg = "simulation output contains no entity tables" + raise ValueError(msg) combined = pd.concat(parts, ignore_index=True, sort=False) columns = [ PARQUET_CONTRACT.entity_column, @@ -196,22 +201,24 @@ def deserialize_simulation_frames(payload: bytes) -> dict[str, pd.DataFrame]: metadata.get(PARQUET_CONTRACT.schema_version_metadata_key.encode()) != str(PARQUET_CONTRACT.payload_schema_version).encode() ): - raise ValueError("unsupported Stage 12 simulation artifact schema") + msg = "unsupported Stage 12 simulation artifact schema" + raise ValueError(msg) raw_dtypes = metadata.get(PARQUET_CONTRACT.dtype_metadata_key.encode()) if raw_dtypes is None: - raise ValueError("Stage 12 simulation artifact has no dtype metadata") + msg = "Stage 12 simulation artifact has no dtype metadata" + raise ValueError(msg) dtypes = json.loads(raw_dtypes) if not isinstance(dtypes, dict): - raise TypeError("Stage 12 simulation artifact dtype metadata is invalid") + msg = "Stage 12 simulation artifact dtype metadata is invalid" + raise TypeError(msg) combined = table.to_pandas() frames: dict[str, pd.DataFrame] = {} for entity in sorted(combined[PARQUET_CONTRACT.entity_column].unique()): entity_name = str(entity) entity_dtypes = dtypes.get(entity_name) if not isinstance(entity_dtypes, dict): - raise ValueError( - f"Stage 12 simulation artifact has no dtype schema for {entity_name}" - ) + msg = f"Stage 12 simulation artifact has no dtype schema for {entity_name}" + raise ValueError(msg) # noqa: TRY004 # documented ValueError for missing dtype metadata; the coordinator persists the type name as error_summary frame = combined.loc[combined[PARQUET_CONTRACT.entity_column] == entity].copy() frame = frame.sort_values(PARQUET_CONTRACT.row_order_column, kind="stable") frame = frame.drop( @@ -271,9 +278,8 @@ class Stage12ArtifactStore: def __init__(self, bucket_name: str, *, store: ArtifactStore | None = None) -> None: """Bind to ``bucket_name``, which must be non-empty (there is no default).""" if not bucket_name: - raise ValueError( - "Stage 12 requires an explicitly configured private bucket" - ) + msg = "Stage 12 requires an explicitly configured private bucket" + raise ValueError(msg) self.bucket_name = bucket_name self._store = store or ArtifactStore(bucket_name) @@ -293,9 +299,8 @@ def _write_immutable( if not created: existing = self._store.read_bytes(path) if existing is None or sha256(existing).hexdigest() != digest: - raise RuntimeError( - "immutable Stage 12 artifact path contains other data" - ) + msg = "immutable Stage 12 artifact path contains other data" + raise RuntimeError(msg) return ArtifactReference( uri=f"gs://{self.bucket_name}/{path}", media_type=ArtifactMediaType(content_type), @@ -383,7 +388,8 @@ def read(self, uri: str) -> bytes: """ expected_prefix = f"gs://{self.bucket_name}/" if not uri.startswith(expected_prefix): - raise ValueError("Stage 12 artifact URI names another bucket") + msg = "Stage 12 artifact URI names another bucket" + raise ValueError(msg) payload = self._store.read_bytes(uri.removeprefix(expected_prefix)) if payload is None: raise FileNotFoundError(uri) diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_bundle/assertions.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_bundle/assertions.py index 404b93593..0ddb9cd25 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_bundle/assertions.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_bundle/assertions.py @@ -48,8 +48,10 @@ def assert_expected_bundle_values( actual = assertion_values(bundle) for name, expected_value in expected.items(): if name not in actual: - raise Stage12BundleError(f"Unknown Stage 12 bundle assertion {name!r}") + msg = f"Unknown Stage 12 bundle assertion {name!r}" + raise Stage12BundleError(msg) if actual[name] != expected_value: - raise Stage12BundleError( + msg = ( f"Stage 12 bundle assertion {name!r} does not match the packaged bundle" ) + raise Stage12BundleError(msg) diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_bundle/manifest.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_bundle/manifest.py index 647c87faa..15c0d4356 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_bundle/manifest.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_bundle/manifest.py @@ -33,7 +33,8 @@ def _normalize_stage12_bundle( ) -> Stage12BundleManifest: bundle = require_mapping(raw_bundle, "root") if bundle.get("schema_version") != 2: - raise Stage12BundleError("PolicyEngine.py bundle schema_version must be 2") + msg = "PolicyEngine.py bundle schema_version must be 2" + raise Stage12BundleError(msg) policyengine_version = require_text( bundle.get("policyengine_version"), "policyengine_version", @@ -42,9 +43,8 @@ def _normalize_stage12_bundle( require_text(bundle.get("bundle_version"), "bundle_version") != policyengine_version ): - raise Stage12BundleError( - "PolicyEngine.py bundle_version does not match policyengine_version" - ) + msg = "PolicyEngine.py bundle_version does not match policyengine_version" + raise Stage12BundleError(msg) packages = require_mapping(bundle.get("packages"), "packages") packaged_policyengine_version, policyengine_requirement = require_exact_package( @@ -52,9 +52,8 @@ def _normalize_stage12_bundle( "policyengine", ) if packaged_policyengine_version != policyengine_version: - raise Stage12BundleError( - "PolicyEngine.py package version does not match the bundle version" - ) + msg = "PolicyEngine.py package version does not match the bundle version" + raise Stage12BundleError(msg) core_package_version, core_package_requirement = require_exact_package( packages, "policyengine-core", @@ -93,9 +92,8 @@ def normalize_stage12_bundle( supported_countries=supported_countries, ) except ValidationError as error: - raise Stage12BundleError( - "PolicyEngine.py bundle contains malformed values" - ) from error + msg = "PolicyEngine.py bundle contains malformed values" + raise Stage12BundleError(msg) from error def bundle_digest(bundle: Stage12BundleManifest) -> str: diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_bundle/parsing.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_bundle/parsing.py index bb1e66e14..c5581c299 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_bundle/parsing.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_bundle/parsing.py @@ -16,20 +16,19 @@ def require_mapping(value: object, path: str) -> Mapping[str, object]: """Return ``value`` as a string-keyed mapping, or raise naming ``path``.""" if not isinstance(value, Mapping): - raise Stage12BundleError(f"PolicyEngine.py bundle field {path} is missing") + msg = f"PolicyEngine.py bundle field {path} is missing" + raise Stage12BundleError(msg) return {str(key): item for key, item in value.items()} def require_text(value: object, path: str) -> str: """Return ``value`` if a non-empty string without outer whitespace, else raise.""" if not isinstance(value, str) or not value.strip(): - raise Stage12BundleError( - f"PolicyEngine.py bundle field {path} must be a non-empty string" - ) + msg = f"PolicyEngine.py bundle field {path} must be a non-empty string" + raise Stage12BundleError(msg) if value != value.strip(): - raise Stage12BundleError( - f"PolicyEngine.py bundle field {path} must not contain outer whitespace" - ) + msg = f"PolicyEngine.py bundle field {path} must not contain outer whitespace" + raise Stage12BundleError(msg) return value @@ -53,14 +52,12 @@ def require_exact_package( f"packages.{package_name}.install_requirement", ) if name != package_name: - raise Stage12BundleError( - f"PolicyEngine.py bundle package key {package_name!r} does not match its name" - ) + msg = f"PolicyEngine.py bundle package key {package_name!r} does not match its name" + raise Stage12BundleError(msg) expected_requirement = f"{name}=={version}" if requirement != expected_requirement: - raise Stage12BundleError( - f"PolicyEngine.py bundle package {name!r} is not exactly pinned" - ) + msg = f"PolicyEngine.py bundle package {name!r} is not exactly pinned" + raise Stage12BundleError(msg) return version, requirement @@ -92,9 +89,10 @@ def normalize_country_bundle( require_text(release.get("country_id"), f"data_releases.{country}.country_id") != country ): - raise Stage12BundleError( + msg = ( f"PolicyEngine.py bundle data release for {country!r} names another country" ) + raise Stage12BundleError(msg) if ( require_text( release.get("policyengine_version"), @@ -102,9 +100,8 @@ def normalize_country_bundle( ) != policyengine_version ): - raise Stage12BundleError( - f"PolicyEngine.py bundle data release for {country!r} names another PolicyEngine.py version" - ) + msg = f"PolicyEngine.py bundle data release for {country!r} names another PolicyEngine.py version" + raise Stage12BundleError(msg) release_model = require_mapping( release.get("model_package"), @@ -122,9 +119,8 @@ def normalize_country_bundle( ) != country_package_version ): - raise Stage12BundleError( - f"PolicyEngine.py bundle country package metadata is inconsistent for {country!r}" - ) + msg = f"PolicyEngine.py bundle country package metadata is inconsistent for {country!r}" + raise Stage12BundleError(msg) data_package = require_mapping( release.get("data_package"), @@ -202,13 +198,11 @@ def normalize_country_bundle( datasets_by_identity = {dataset.identity: dataset for dataset in datasets} selected_dataset = datasets_by_identity.get(default_dataset) if selected_dataset is None: - raise Stage12BundleError( - f"PolicyEngine.py bundle default dataset is absent for {country!r}" - ) + msg = f"PolicyEngine.py bundle default dataset is absent for {country!r}" + raise Stage12BundleError(msg) if selected_dataset.uri != default_dataset_uri: - raise Stage12BundleError( - f"PolicyEngine.py bundle default dataset URI is inconsistent for {country!r}" - ) + msg = f"PolicyEngine.py bundle default dataset URI is inconsistent for {country!r}" + raise Stage12BundleError(msg) certified = require_mapping( release.get("certified_data_artifact"), @@ -233,9 +227,10 @@ def normalize_country_bundle( "data_package.version": certified_package.get("version"), } if actual_certified_values != certified_values: - raise Stage12BundleError( + msg = ( f"PolicyEngine.py bundle certified artifact is inconsistent for {country!r}" ) + raise Stage12BundleError(msg) return Stage12CountryBundle( country=country, diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_parity.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_parity.py index dd8db1d13..6a4c8354d 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_parity.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_parity.py @@ -20,7 +20,8 @@ class NumericalTolerance: def __post_init__(self) -> None: """Reject negative tolerances with ``ValueError``.""" if self.absolute < 0 or self.relative < 0: - raise ValueError("numerical tolerances must be non-negative") + msg = "numerical tolerances must be non-negative" + raise ValueError(msg) class ParityMismatch(ValueError): @@ -45,9 +46,11 @@ def canonicalize_simulation_frames( identifier = f"{entity}_id" frame = pd.DataFrame(frames[entity]).copy() if identifier not in frame.columns: - raise ParityMismatch("missing_row_identity", entity) + msg = "missing_row_identity" + raise ParityMismatch(msg, entity) if frame[identifier].duplicated().any(): - raise ParityMismatch("duplicate_row_identity", entity) + msg = "duplicate_row_identity" + raise ParityMismatch(msg, entity) canonical[entity] = ( frame.sort_values(identifier, kind="stable") .reset_index(drop=True) @@ -71,23 +74,28 @@ def compare_simulation_frames( expected = canonicalize_simulation_frames(incumbent) actual = canonicalize_simulation_frames(candidate) if expected.keys() != actual.keys(): - raise ParityMismatch("entity_membership_mismatch", "simulation") + msg = "entity_membership_mismatch" + raise ParityMismatch(msg, "simulation") allowed = tolerances or {} for entity, expected_frame in expected.items(): actual_frame = actual[entity] if tuple(expected_frame.columns) != tuple(actual_frame.columns): - raise ParityMismatch("column_membership_mismatch", entity) + msg = "column_membership_mismatch" + raise ParityMismatch(msg, entity) if len(expected_frame) != len(actual_frame): - raise ParityMismatch("row_membership_mismatch", entity) + msg = "row_membership_mismatch" + raise ParityMismatch(msg, entity) identifier = f"{entity}_id" if not expected_frame[identifier].equals(actual_frame[identifier]): - raise ParityMismatch("row_identity_mismatch", entity) + msg = "row_identity_mismatch" + raise ParityMismatch(msg, entity) for column in expected_frame.columns: path = f"{entity}.{column}" expected_values = expected_frame[column] actual_values = actual_frame[column] if str(expected_values.dtype) != str(actual_values.dtype): - raise ParityMismatch("dtype_mismatch", path) + msg = "dtype_mismatch" + raise ParityMismatch(msg, path) if pd.api.types.is_numeric_dtype(expected_values.dtype): tolerance = allowed.get(path, NumericalTolerance()) if not np.allclose( @@ -97,9 +105,11 @@ def compare_simulation_frames( atol=tolerance.absolute, equal_nan=True, ): - raise ParityMismatch("numeric_value_mismatch", path) + msg = "numeric_value_mismatch" + raise ParityMismatch(msg, path) elif not expected_values.equals(actual_values): - raise ParityMismatch("value_mismatch", path) + msg = "value_mismatch" + raise ParityMismatch(msg, path) def compare_aggregate_reports( @@ -119,7 +129,8 @@ def compare_aggregate_reports( def compare(expected: object, actual: object, path: str) -> None: if isinstance(expected, Mapping): if not isinstance(actual, Mapping) or expected.keys() != actual.keys(): - raise ParityMismatch("field_membership_mismatch", path or "report") + msg = "field_membership_mismatch" + raise ParityMismatch(msg, path or "report") for key in sorted(expected): compare(expected[key], actual[key], f"{path}.{key}".lstrip(".")) return @@ -129,9 +140,11 @@ def compare(expected: object, actual: object, path: str) -> None: if not isinstance(actual, Sequence) or isinstance( actual, (str, bytes, bytearray) ): - raise ParityMismatch("value_type_mismatch", path) + msg = "value_type_mismatch" + raise ParityMismatch(msg, path) if len(expected) != len(actual): - raise ParityMismatch("list_length_mismatch", path) + msg = "list_length_mismatch" + raise ParityMismatch(msg, path) for index, value in enumerate(expected): compare(value, actual[index], f"{path}[{index}]") return @@ -148,11 +161,14 @@ def compare(expected: object, actual: object, path: str) -> None: rel_tol=tolerance.relative, abs_tol=tolerance.absolute, ): - raise ParityMismatch("numeric_value_mismatch", path) + msg = "numeric_value_mismatch" + raise ParityMismatch(msg, path) return if type(expected) is not type(actual): - raise ParityMismatch("value_type_mismatch", path) + msg = "value_type_mismatch" + raise ParityMismatch(msg, path) if expected != actual: - raise ParityMismatch("value_mismatch", path) + msg = "value_mismatch" + raise ParityMismatch(msg, path) compare(incumbent, candidate, "") 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 75c026831..292d5e356 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 @@ -131,15 +131,19 @@ def _validate_matching_input( if field == "spm" and expected_value is None and actual_value is None: continue if actual_value != expected_value: - raise ParityMismatch("normalized_input_mismatch", field) + msg = "normalized_input_mismatch" + raise ParityMismatch(msg, field) if existing_request.get("data") not in accepted_dataset_values: - raise ParityMismatch("dataset_provenance_mismatch", "data") + msg = "dataset_provenance_mismatch" + raise ParityMismatch(msg, "data") population = baseline.population if population.kind != "dataset": - raise ParityMismatch("population_kind_mismatch", "population.kind") + msg = "population_kind_mismatch" + raise ParityMismatch(msg, "population.kind") if population.artifact.uri != bundle.dataset.uri: + msg = "dataset_provenance_mismatch" raise ParityMismatch( - "dataset_provenance_mismatch", + msg, "population.artifact.uri", ) @@ -150,23 +154,27 @@ def _existing_frames( side: Literal["baseline", "reform"], ) -> dict[str, pd.DataFrame]: if not isinstance(microdata, Mapping): - raise ParityMismatch("missing_existing_microdata", "report._microdata") + msg = "missing_existing_microdata" + raise ParityMismatch(msg, "report._microdata") raw_side = microdata.get(side) raw_dtypes = microdata.get("dtypes") side_dtypes = raw_dtypes.get(side) if isinstance(raw_dtypes, Mapping) else None if not isinstance(raw_side, Mapping) or not isinstance(side_dtypes, Mapping): - raise ParityMismatch("invalid_existing_microdata", f"report._microdata.{side}") + msg = "invalid_existing_microdata" + raise ParityMismatch(msg, f"report._microdata.{side}") frames: dict[str, pd.DataFrame] = {} for entity, columns in raw_side.items(): entity_dtypes = side_dtypes.get(entity) if not isinstance(entity, str) or not isinstance(columns, dict): + msg = "invalid_existing_microdata" raise ParityMismatch( - "invalid_existing_microdata", + msg, f"report._microdata.{side}", ) if not isinstance(entity_dtypes, dict): + msg = "invalid_existing_dtypes" raise ParityMismatch( - "invalid_existing_dtypes", + msg, f"report._microdata.dtypes.{side}.{entity}", ) frames[entity] = rebuild_entity_frame(columns, entity_dtypes) @@ -266,13 +274,15 @@ def run_existing(request: dict[str, Any]) -> Mapping[str, Any]: bundle = report.baseline.bundle if existing_result.get("model_version") != bundle.country_package_version: + msg = "country_package_provenance_mismatch" raise ParityMismatch( - "country_package_provenance_mismatch", + msg, "report.model_version", ) if existing_result.get("data_version") != bundle.dataset.artifact_revision: + msg = "dataset_provenance_mismatch" raise ParityMismatch( - "dataset_provenance_mismatch", + msg, "report.data_version", ) @@ -310,7 +320,8 @@ def run_existing(request: dict[str, Any]) -> Mapping[str, Any]: ) v2_result = v2_report.get("result") if not isinstance(v2_result, Mapping): - raise ParityMismatch("invalid_v2_aggregate", "report.result") + msg = "invalid_v2_aggregate" + raise ParityMismatch(msg, "report.result") compare_aggregate_reports( existing_result, v2_result, diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/aggregation.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/aggregation.py index beda86dcf..90932edb1 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/aggregation.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/aggregation.py @@ -43,15 +43,19 @@ def validate_uk_local_authority_metadata( ) if region_code is None or uk_area_codes_required(region_code): - raise ValueError("UK simulation artifact metadata is missing") + msg = "UK simulation artifact metadata is missing" + raise ValueError(msg) return None if baseline is None or reform is None: - raise ValueError("UK simulation artifact metadata is missing on one side") + msg = "UK simulation artifact metadata is missing on one side" + raise ValueError(msg) if baseline != reform: - raise ValueError("UK simulation artifact boundary versions do not match") + msg = "UK simulation artifact boundary versions do not match" + raise ValueError(msg) return baseline if baseline is not None or reform is not None: - raise ValueError("non-UK simulation artifacts contain UK authority metadata") + msg = "non-UK simulation artifacts contain UK authority metadata" + raise ValueError(msg) return None @@ -67,24 +71,30 @@ def validate_aligned_outputs( output plan and row identities all line up. """ if baseline.role.value != "baseline" or reform.role.value != "reform": - raise ValueError("simulation artifacts have incorrect report roles") + msg = "simulation artifacts have incorrect report roles" + raise ValueError(msg) if ( baseline.evaluation_id != report.evaluation_id or reform.evaluation_id != report.evaluation_id ): - raise ValueError("simulation artifacts name another comparison run") + msg = "simulation artifacts name another comparison run" + raise ValueError(msg) if baseline.bundle != reform.bundle or baseline.bundle != report.baseline.bundle: - raise ValueError("simulation artifacts have incompatible bundle provenance") + msg = "simulation artifacts have incompatible bundle provenance" + raise ValueError(msg) if baseline.output_schema_version != reform.output_schema_version: - raise ValueError("simulation artifacts use incompatible output schemas") + msg = "simulation artifacts use incompatible output schemas" + raise ValueError(msg) expected_plan_sha256 = stage12_output_plan_sha256(output_plan) if ( baseline.output_plan_sha256 != expected_plan_sha256 or reform.output_plan_sha256 != expected_plan_sha256 ): - raise ValueError("simulation artifacts do not satisfy the report output plan") + msg = "simulation artifacts do not satisfy the report output plan" + raise ValueError(msg) if baseline.row_identity != reform.row_identity: - raise ValueError("simulation artifacts have incompatible stable row identities") + msg = "simulation artifacts have incompatible stable row identities" + raise ValueError(msg) def _dataset_from_frames( @@ -244,10 +254,12 @@ def build_spm_result( baseline_provenance = baseline_descriptor.calculation_provenance reform_provenance = reform_descriptor.calculation_provenance if baseline_provenance is None or reform_provenance is None: - raise ValueError("SPM calculation provenance is missing") + msg = "SPM calculation provenance is missing" + raise ValueError(msg) for receipt in (baseline_provenance, reform_provenance): if str(report.baseline.year) not in receipt.years: - raise ValueError("SPM calculation provenance does not cover report year") + msg = "SPM calculation provenance does not cover report year" + raise ValueError(msg) comparison = build_spm_comparison_provenance( baseline_receipts=[baseline_provenance], reform_receipts=[reform_provenance], diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/comparison.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/comparison.py index 13ae6d02a..be969dafd 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/comparison.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/comparison.py @@ -139,7 +139,7 @@ def compare_completed_report( "production_job_id": production_job_id, }, ) - logger.error( + logger.error( # noqa: TRY400 # Stage 12 failure logs carry only the error type; a traceback could expose request or calculation data "stage12_result_comparison_failed", extra={ "evaluation_id": str(parent.evaluation_id), diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/coordination.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/coordination.py index c92078a13..5fd4391d8 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/coordination.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/coordination.py @@ -122,26 +122,34 @@ def _validate_submitted_parent( } for field, value in expected.items(): if getattr(parent, field) != value: - raise ValueError(f"submitted parent {field} does not match invocation") + msg = f"submitted parent {field} does not match invocation" + raise ValueError(msg) if parent.status is not ComparisonRunLifecycleStatus.PENDING: - raise ValueError("submitted parent must be pending") + msg = "submitted parent must be pending" + raise ValueError(msg) if parent.aggregation_status is not ComparisonRunAggregationStatus.NOT_STARTED: - raise ValueError("submitted parent aggregation must not be started") + msg = "submitted parent aggregation must not be started" + raise ValueError(msg) if parent.coordinator_invocation_id is not None: - raise ValueError("submitted parent must not contain an invocation identifier") + msg = "submitted parent must not contain an invocation identifier" + raise ValueError(msg) production_job_id = context.production_function_call_id if production_job_id is None: if parent.production_identity != f"direct:{report.evaluation_id}": - raise ValueError("direct parent identity does not match its report") + msg = "direct parent identity does not match its report" + raise ValueError(msg) if parent.incumbent_execution_id is not None: - raise ValueError("direct parent must not name a production execution") + msg = "direct parent must not name a production execution" + raise ValueError(msg) if parent.comparison_status is not ResultComparisonStatus.NOT_REQUESTED: - raise ValueError("direct parent must not request result comparison") + msg = "direct parent must not request result comparison" + raise ValueError(msg) elif ( parent.production_identity != production_job_id or parent.incumbent_execution_id != production_job_id ): - raise ValueError("automatic parent does not match the production execution") + msg = "automatic parent does not match the production execution" + raise ValueError(msg) def _claim_parent( @@ -169,9 +177,8 @@ def _claim_parent( if persistence.created: return parent, True if parent.evaluation_id != submitted.evaluation_id: - raise ValueError( - "existing comparison identity has a different report identifier" - ) + msg = "existing comparison identity has a different report identifier" + raise ValueError(msg) if parent.status in { ComparisonRunLifecycleStatus.RUNNING, ComparisonRunLifecycleStatus.SUCCEEDED, @@ -231,6 +238,17 @@ def _mark_child_failed( return error_summary +def _require_artifact_digest( + payload: bytes, + expected_sha256: str, + *, + mismatch_message: str, +) -> None: + """Raise ``ValueError(mismatch_message)`` unless ``payload`` matches its digest.""" + if sha256(payload).hexdigest() != expected_sha256: + raise ValueError(mismatch_message) + + def coordinate_report( payload: object, context_payload: object, @@ -260,11 +278,14 @@ def coordinate_report( context = Stage12InvocationContext.model_validate(context_payload) submitted_parent = ComparisonReportRecord.model_validate(parent_payload) if application_name != context.modal_application: - raise ValueError("report coordinator application differs from dispatch context") + msg = "report coordinator application differs from dispatch context" + raise ValueError(msg) if report.baseline.bundle.policyengine_version != context.worker_version: - raise ValueError("report worker version differs from dispatch context") + msg = "report worker version differs from dispatch context" + raise ValueError(msg) if report.baseline.bundle.bundle_manifest_sha256 != context.bundle_manifest_sha256: - raise ValueError("report bundle digest differs from dispatch context") + msg = "report bundle digest differs from dispatch context" + raise ValueError(msg) _validate_submitted_parent( report=report, context=context, @@ -355,8 +376,11 @@ def coordinate_report( if child.status is ComparisonRunLifecycleStatus.SUCCEEDED: descriptor = descriptor_from_record(child, simulation) retained = artifact_storage.read(descriptor.artifact.uri) - if sha256(retained).hexdigest() != descriptor.artifact.content_sha256: - raise ValueError("retained child artifact digest mismatch") + _require_artifact_digest( + retained, + descriptor.artifact.content_sha256, + mismatch_message="retained child artifact digest mismatch", + ) descriptor = descriptor.model_copy( update={ "artifact": descriptor.artifact.model_copy( @@ -472,10 +496,16 @@ def coordinate_report( with stage_span(Stage.STAGE12_ARTIFACT_READ): baseline_payload = artifact_storage.read(baseline.artifact.uri) reform_payload = artifact_storage.read(reform.artifact.uri) - if sha256(baseline_payload).hexdigest() != baseline.artifact.content_sha256: - raise ValueError("baseline artifact digest mismatch") - if sha256(reform_payload).hexdigest() != reform.artifact.content_sha256: - raise ValueError("reform artifact digest mismatch") + _require_artifact_digest( + baseline_payload, + baseline.artifact.content_sha256, + mismatch_message="baseline artifact digest mismatch", + ) + _require_artifact_digest( + reform_payload, + reform.artifact.content_sha256, + mismatch_message="reform artifact digest mismatch", + ) baseline_frames = deserialize_simulation_frames(baseline_payload) reform_frames = deserialize_simulation_frames(reform_payload) uk_local_authority_metadata = validate_uk_local_authority_metadata( @@ -563,4 +593,5 @@ def coordinate_report( runtime=runtime, failure_scope="report_coordination", ) - raise RuntimeError("Stage 12 report coordination failed") from None + msg = "Stage 12 report coordination failed" + raise RuntimeError(msg) from None diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/output_planning.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/output_planning.py index b6e5f4c58..8c067732f 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/output_planning.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/output_planning.py @@ -93,7 +93,8 @@ def _planning_simulation( def _include_cliff_impacts(report: ReportExecutionInput) -> bool: value = report.baseline.options.get("include_cliffs", False) if not isinstance(value, bool): - raise TypeError("include_cliffs must be a boolean") + msg = "include_cliffs must be a boolean" + raise TypeError(msg) return value @@ -243,9 +244,8 @@ def apply_output_plan( } missing = {entity: variables for entity, variables in missing.items() if variables} if missing: - raise ValueError( - f"configured simulation does not satisfy output plan: {missing}" - ) + msg = f"configured simulation does not satisfy output plan: {missing}" + raise ValueError(msg) def validate_output_frames( @@ -257,9 +257,8 @@ def validate_output_frames( entity.entity for entity in output_plan.entities if entity.entity not in frames ] if missing_entities: - raise ValueError( - f"simulation output is missing planned entities: {missing_entities}" - ) + msg = f"simulation output is missing planned entities: {missing_entities}" + raise ValueError(msg) missing_columns = { entity.entity: sorted( set(entity.materialized_variables) - set(frames[entity.entity].columns) @@ -270,6 +269,5 @@ def validate_output_frames( entity: columns for entity, columns in missing_columns.items() if columns } if missing_columns: - raise ValueError( - f"simulation output is missing planned columns: {missing_columns}" - ) + msg = f"simulation output is missing planned columns: {missing_columns}" + raise ValueError(msg) diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/simulation.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/simulation.py index 039c48d11..16ef2f7ae 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/simulation.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/simulation.py @@ -75,25 +75,30 @@ def _require_context( required_country: CountryId, ) -> None: if simulation.geography.country != required_country: - raise ValueError("single-simulation input was sent to another country worker") + msg = "single-simulation input was sent to another country worker" + raise ValueError(msg) if simulation.bundle.policyengine_version != context.worker_version: - raise ValueError("single-simulation worker version does not match its context") + msg = "single-simulation worker version does not match its context" + raise ValueError(msg) if simulation.bundle.bundle_manifest_sha256 != context.bundle_manifest_sha256: - raise ValueError("single-simulation bundle digest does not match its context") + msg = "single-simulation bundle digest does not match its context" + raise ValueError(msg) if str(simulation.evaluation_id) not in context.artifact_prefix: - raise ValueError( - "single-simulation artifact prefix names another comparison run" - ) + msg = "single-simulation artifact prefix names another comparison run" + raise ValueError(msg) if simulation.output_plan.country != required_country: - raise ValueError("single-simulation output plan names another country") + msg = "single-simulation output plan names another country" + raise ValueError(msg) def _require_installed_bundle(simulation: SimulationExecutionInput) -> None: resolved = load_stage12_bundle() if resolved.bundle_manifest_sha256 != simulation.bundle.bundle_manifest_sha256: - raise RuntimeError("installed bundle digest differs from the simulation input") + msg = "installed bundle digest differs from the simulation input" + raise RuntimeError(msg) if resolved.bundle.policyengine_version != simulation.bundle.policyengine_version: - raise RuntimeError("installed PolicyEngine.py version differs from the input") + msg = "installed PolicyEngine.py version differs from the input" + raise RuntimeError(msg) country = next( item for item in resolved.bundle.countries @@ -114,7 +119,8 @@ def _require_installed_bundle(simulation: SimulationExecutionInput) -> None: "dataset_revision": simulation.bundle.dataset.artifact_revision, } if actual != expected: - raise RuntimeError("simulation provenance differs from the installed bundle") + msg = "simulation provenance differs from the installed bundle" + raise RuntimeError(msg) def calculate_simulation_frames( @@ -133,9 +139,11 @@ def calculate_simulation_frames( runtime=runtime, ) if simulation.population.kind != "dataset": - raise ValueError("Stage 12 society-wide worker requires a dataset population") + msg = "Stage 12 society-wide worker requires a dataset population" + raise ValueError(msg) if simulation.population.artifact.uri != simulation.bundle.dataset.uri: - raise ValueError("population artifact differs from bundle dataset provenance") + msg = "population artifact differs from bundle dataset provenance" + raise ValueError(msg) _require_installed_bundle(simulation) country = simulation.geography.country params: dict[str, Any] = { @@ -204,7 +212,8 @@ def calculate_simulation_frames( output_data = getattr(getattr(model, "output_dataset", None), "data", None) entity_data = getattr(output_data, "entity_data", None) if not isinstance(entity_data, Mapping): - raise TypeError("simulation produced no entity output tables") + msg = "simulation produced no entity output tables" + raise TypeError(msg) frames = {entity: pd.DataFrame(frame) for entity, frame in entity_data.items()} validate_output_frames(frames, simulation.output_plan) selection = getattr(model, "spm_config", None) @@ -248,7 +257,8 @@ def descriptor_from_record( or row_count is None or row_identity_sha256 is None ): - raise ValueError("successful child record has incomplete output metadata") + msg = "successful child record has incomplete output metadata" + raise ValueError(msg) from policyengine_simulation_contract.stage12_execution import ( ArtifactMediaType, ArtifactReference, @@ -398,4 +408,5 @@ def run_single_simulation( runtime=runtime, failure_scope="simulation_execution", ) - raise RuntimeError("Stage 12 simulation execution failed") from None + msg = "Stage 12 simulation execution failed" + raise RuntimeError(msg) from None diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_worker_validation.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_worker_validation.py index 4fc038708..37bb6a310 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_worker_validation.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_worker_validation.py @@ -49,9 +49,11 @@ def _check_dataset_access(path: str, expected_sha256: str) -> None: while chunk := stream.read(1024 * 1024): digest.update(chunk) if dataset_path.stat().st_size == 0: - raise RuntimeError("Stage 12 installed dataset artifact is empty") + msg = "Stage 12 installed dataset artifact is empty" + raise RuntimeError(msg) if digest.hexdigest() != expected_sha256: - raise RuntimeError("Stage 12 installed dataset artifact digest differs") + msg = "Stage 12 installed dataset artifact digest differs" + raise RuntimeError(msg) def _run_non_serving_calculation(country: CountryId) -> None: @@ -65,7 +67,8 @@ def _run_non_serving_calculation(country: CountryId) -> None: ) household = getattr(result, "household", None) if not isinstance(household, Mapping) or "household_net_income" not in household: - raise RuntimeError("Stage 12 validation calculation returned no net income") + msg = "Stage 12 validation calculation returned no net income" + raise RuntimeError(msg) def _check_uk_local_authority_resources() -> None: @@ -119,23 +122,20 @@ def validate_country_worker( import_module("policyengine_simulation_executor.stage12_runtime") resolved = load_stage12_bundle() if resolved.bundle_manifest_sha256 != expected_bundle_manifest_sha256: - raise RuntimeError( - "installed PolicyEngine.py bundle digest differs from deployment input" - ) + msg = "installed PolicyEngine.py bundle digest differs from deployment input" + raise RuntimeError(msg) try: country_bundle = next( item for item in resolved.bundle.countries if item.country == country ) except StopIteration: - raise RuntimeError( - f"installed PolicyEngine.py bundle does not support {country!r}" - ) from None + msg = f"installed PolicyEngine.py bundle does not support {country!r}" + raise RuntimeError(msg) from None import_module(f"policyengine.tax_benefit_models.{country}") installed_dataset_path = dataset_path_resolver(country) if installed_dataset_path is None: - raise RuntimeError( - f"installed certified dataset is unavailable for {country!r}" - ) + msg = f"installed certified dataset is unavailable for {country!r}" + raise RuntimeError(msg) selected_dataset = next( dataset for dataset in country_bundle.datasets diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/uk_local_authority_metadata.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/uk_local_authority_metadata.py index b51f66918..90e49a4f4 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/uk_local_authority_metadata.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/uk_local_authority_metadata.py @@ -132,24 +132,21 @@ def _rows( with _resource_path(filename).open("r", encoding="utf-8") as stream: reader = csv.DictReader(stream) if tuple(reader.fieldnames or ()) != expected_fields: - raise ValueError( - f"UK local-authority resource {filename!r} has invalid columns" - ) + msg = f"UK local-authority resource {filename!r} has invalid columns" + raise ValueError(msg) rows: list[dict[str, str]] = [] for raw in reader: if any(raw.get(field) is None for field in expected_fields): - raise ValueError( - f"UK local-authority resource {filename!r} has an incomplete row" - ) + msg = f"UK local-authority resource {filename!r} has an incomplete row" + raise ValueError(msg) rows.append({field: str(raw[field]).strip() for field in expected_fields}) return tuple(rows) def _require_code(code: str, filename: str) -> None: if not _CODE_PATTERN.fullmatch(code): - raise ValueError( - f"UK local-authority resource {filename!r} contains invalid code {code!r}" - ) + msg = f"UK local-authority resource {filename!r} contains invalid code {code!r}" + raise ValueError(msg) def _load_names() -> Mapping[str, str]: @@ -160,13 +157,11 @@ def _load_names() -> Mapping[str, str]: name = row["name"] _require_code(code, filename) if not name or name == code: - raise ValueError( - f"UK local-authority resource {filename!r} has no name for {code!r}" - ) + msg = f"UK local-authority resource {filename!r} has no name for {code!r}" + raise ValueError(msg) if code in names: - raise ValueError( - f"UK local-authority resource {filename!r} repeats {code!r}" - ) + msg = f"UK local-authority resource {filename!r} repeats {code!r}" + raise ValueError(msg) names[code] = name return MappingProxyType(names) @@ -177,16 +172,16 @@ def _load_coordinates(filename: str) -> Mapping[str, LocalAuthorityCoordinate]: code = row["code"] _require_code(code, filename) if code in coordinates: - raise ValueError( - f"UK local-authority resource {filename!r} repeats {code!r}" - ) + msg = f"UK local-authority resource {filename!r} repeats {code!r}" + raise ValueError(msg) try: coordinate = LocalAuthorityCoordinate(x=int(row["x"]), y=int(row["y"])) except ValueError as error: - raise ValueError( + msg = ( f"UK local-authority resource {filename!r} has invalid coordinates " f"for {code!r}" - ) from error + ) + raise ValueError(msg) from error coordinates[code] = coordinate return MappingProxyType(coordinates) @@ -199,19 +194,21 @@ def load_uk_local_authority_resources() -> UKLocalAuthorityResources: lad23 = _load_coordinates("coordinates_lad23.csv") supported_codes = lad22.keys() | lad23.keys() if names.keys() != supported_codes: - raise ValueError( - "UK local-authority names must exactly cover the supported boundary versions" - ) + msg = "UK local-authority names must exactly cover the supported boundary versions" + raise ValueError(msg) if lad22.keys() - lad23.keys() != _LAD22_ONLY_CODES: - raise ValueError("LAD22 distinguishing codes do not match its coordinate file") + msg = "LAD22 distinguishing codes do not match its coordinate file" + raise ValueError(msg) if lad23.keys() - lad22.keys() != _LAD23_ONLY_CODES: - raise ValueError("LAD23 distinguishing codes do not match its coordinate file") + msg = "LAD23 distinguishing codes do not match its coordinate file" + raise ValueError(msg) for code in lad22.keys() & lad23.keys(): if lad22[code] != lad23[code]: - raise ValueError( + msg = ( "UK local-authority coordinate differs between boundary versions " f"for {code!r}" ) + raise ValueError(msg) return UKLocalAuthorityResources( names=names, coordinates=MappingProxyType( @@ -232,17 +229,18 @@ def _normalise_observed_code(value: object) -> str | None: try: code = value.decode("utf-8").strip() except UnicodeDecodeError as error: - raise ValueError( - "UK dataset contains an invalid local-authority code" - ) from error + msg = "UK dataset contains an invalid local-authority code" + raise ValueError(msg) from error elif isinstance(value, str): code = value.strip() else: - raise TypeError("UK dataset contains a non-text local-authority code") + msg = "UK dataset contains a non-text local-authority code" + raise TypeError(msg) if not code: return None if not _CODE_PATTERN.fullmatch(code): - raise ValueError(f"UK dataset contains invalid local-authority code {code!r}") + msg = f"UK dataset contains invalid local-authority code {code!r}" + raise ValueError(msg) return code @@ -256,14 +254,14 @@ def detect_uk_local_authority_boundary_version( if (code := _normalise_observed_code(value)) is not None ) if not observed_codes: - raise ValueError("UK dataset contains no local-authority codes") + msg = "UK dataset contains no local-authority codes" + raise ValueError(msg) supported_codes = load_uk_local_authority_resources().names.keys() unsupported_codes = observed_codes - supported_codes if unsupported_codes: first = min(unsupported_codes) - raise ValueError( - f"UK dataset contains unsupported local-authority code {first!r}" - ) + msg = f"UK dataset contains unsupported local-authority code {first!r}" + raise ValueError(msg) matches = tuple( definition @@ -271,11 +269,11 @@ def detect_uk_local_authority_boundary_version( if observed_codes & definition.distinguishing_codes ) if len(matches) > 1: - raise ValueError("UK dataset mixes LAD22 and LAD23 local-authority codes") + msg = "UK dataset mixes LAD22 and LAD23 local-authority codes" + raise ValueError(msg) if not matches: - raise ValueError( - "UK dataset local-authority configuration cannot be identified" - ) + msg = "UK dataset local-authority configuration cannot be identified" + raise ValueError(msg) return UKLocalAuthorityMetadata(boundary_version=matches[0].boundary_version) @@ -331,10 +329,12 @@ def detect_uk_local_authority_metadata( data = getattr(dataset, "data", None) entity_data = getattr(data, "entity_data", None) if not isinstance(entity_data, Mapping): - raise TypeError("UK dataset contains no entity tables") + msg = "UK dataset contains no entity tables" + raise TypeError(msg) household = entity_data.get("household") if household is None: - raise ValueError("UK dataset contains no household table") + msg = "UK dataset contains no household table" + raise ValueError(msg) household_frame = pd.DataFrame(household) if "la_code_oa" not in household_frame: if uk_area_codes_required(region_code): @@ -349,13 +349,21 @@ def uk_hdf_household_has_local_authority_codes(dataset_path: str) -> bool: """Whether an installed UK HDF dataset's household table has ``la_code_oa``.""" with pd.HDFStore(dataset_path, mode="r") as store: if "household" not in store: - raise ValueError("UK dataset contains no household table") + msg = "UK dataset contains no household table" + raise ValueError(msg) household = store.select("household", stop=0) if not isinstance(household, pd.DataFrame): - raise TypeError("UK dataset household table is not a data frame") + msg = "UK dataset household table is not a data frame" + raise TypeError(msg) return "la_code_oa" in household.columns +def _require_local_authority_column(household_chunk: pd.DataFrame) -> None: + if "la_code_oa" not in household_chunk: + msg = "UK dataset household table contains no la_code_oa column" + raise ValueError(msg) + + def detect_uk_local_authority_metadata_from_hdf( dataset_path: str, ) -> UKLocalAuthorityMetadata: @@ -363,7 +371,8 @@ def detect_uk_local_authority_metadata_from_hdf( observed_codes: set[object] = set() with pd.HDFStore(dataset_path, mode="r") as store: if "/household" not in store.keys(): - raise ValueError("UK dataset contains no household table") + msg = "UK dataset contains no household table" + raise ValueError(msg) try: chunks = store.select( "household", @@ -371,15 +380,11 @@ def detect_uk_local_authority_metadata_from_hdf( chunksize=100_000, ) for chunk in chunks: - if "la_code_oa" not in chunk: - raise ValueError( - "UK dataset household table contains no la_code_oa column" - ) + _require_local_authority_column(chunk) observed_codes.update(chunk["la_code_oa"].drop_duplicates().tolist()) except (KeyError, TypeError, ValueError) as error: if "la_code_oa" in str(error): - raise ValueError( - "UK dataset household table contains no la_code_oa column" - ) from error + msg = "UK dataset household table contains no la_code_oa column" + raise ValueError(msg) from error raise return detect_uk_local_authority_boundary_version(observed_codes) diff --git a/projects/policyengine-simulation-executor/tests/test_artifact_store.py b/projects/policyengine-simulation-executor/tests/test_artifact_store.py index baa028cdc..c79e9ce8f 100644 --- a/projects/policyengine-simulation-executor/tests/test_artifact_store.py +++ b/projects/policyengine-simulation-executor/tests/test_artifact_store.py @@ -29,7 +29,8 @@ def exists(self): def _write(self, data, **kwargs): self.store.upload_calls.append((self.path, kwargs)) if kwargs.get("if_generation_match") == 0 and self.path in self.store.objects: - raise PreconditionFailed(f"object exists: {self.path}") + msg = f"object exists: {self.path}" + raise PreconditionFailed(msg) self.store.objects[self.path] = data def upload_from_filename(self, filename, **kwargs): @@ -46,7 +47,8 @@ def download_to_filename(self, filename): # the transport dies. with open(filename, "wb") as handle: handle.write(self.store.objects[self.path][:1]) - raise ConnectionError("stream reset") + msg = "stream reset" + raise ConnectionError(msg) with open(filename, "wb") as handle: handle.write(self.store.objects[self.path]) diff --git a/projects/policyengine-simulation-executor/tests/test_baseline_artifacts.py b/projects/policyengine-simulation-executor/tests/test_baseline_artifacts.py index f67216a25..0a32f4a44 100644 --- a/projects/policyengine-simulation-executor/tests/test_baseline_artifacts.py +++ b/projects/policyengine-simulation-executor/tests/test_baseline_artifacts.py @@ -139,7 +139,8 @@ def test_single_state_scoping_disqualifies(self, collected): def test_identity_collection_failure_degrades_to_random_id(self, monkeypatch): def boom(*args, **kwargs): - raise RuntimeError("manifest unavailable") + msg = "manifest unavailable" + raise RuntimeError(msg) monkeypatch.setattr(ak, "collect_baseline_identity", boom) assert ( @@ -181,9 +182,11 @@ def resolve_entity_variables(self, simulation): def load(self, simulation): self.calls.append("load") if self.load_result == "absent": - raise FileNotFoundError("no artifact") + msg = "no artifact" + raise FileNotFoundError(msg) if self.load_result == "broken": - raise OSError("corrupt h5") + msg = "corrupt h5" + raise OSError(msg) frames = ( _complete_frames() if self.load_result == "complete" @@ -526,7 +529,8 @@ def _kwargs(self): def test_qualifying_identity_propagates_collection_errors(self, monkeypatch): def boom(*args, **kwargs): - raise RuntimeError("receipt unreadable") + msg = "receipt unreadable" + raise RuntimeError(msg) monkeypatch.setattr(ak, "collect_baseline_identity", boom) with pytest.raises(RuntimeError, match="receipt unreadable"): @@ -534,7 +538,8 @@ def boom(*args, **kwargs): def test_deterministic_id_swallows_collection_errors(self, monkeypatch): def boom(*args, **kwargs): - raise RuntimeError("receipt unreadable") + msg = "receipt unreadable" + raise RuntimeError(msg) monkeypatch.setattr(ak, "collect_baseline_identity", boom) assert ba.deterministic_baseline_id(**self._kwargs()) is None @@ -809,7 +814,7 @@ def ensure(self): return try: self.tax_benefit_model_version.load(self) - except Exception: + except Exception: # noqa: BLE001 # mirrors .py's Simulation.ensure(), which runs and saves after any load failure self.run() self.save() # The wrapper files a snapshot, not the live object, so re-pointing diff --git a/projects/policyengine-simulation-executor/tests/test_budget_window_scheduler.py b/projects/policyengine-simulation-executor/tests/test_budget_window_scheduler.py index e44192659..072e15b0f 100644 --- a/projects/policyengine-simulation-executor/tests/test_budget_window_scheduler.py +++ b/projects/policyengine-simulation-executor/tests/test_budget_window_scheduler.py @@ -284,7 +284,8 @@ class MockModal: @staticmethod def current_function_call_id(): if runtime.current_parent_call_id is None: - raise RuntimeError("No active parent batch call") + msg = "No active parent batch call" + raise RuntimeError(msg) return runtime.current_parent_call_id monkeypatch.setattr(endpoints, "modal", MockModal) diff --git a/projects/policyengine-simulation-executor/tests/test_canonical_spm.py b/projects/policyengine-simulation-executor/tests/test_canonical_spm.py index b6c63cf6a..a52f9ac40 100644 --- a/projects/policyengine-simulation-executor/tests/test_canonical_spm.py +++ b/projects/policyengine-simulation-executor/tests/test_canonical_spm.py @@ -274,7 +274,8 @@ def test_sync_compatibility_endpoint_preserves_explicit_null( def capture(params, *, runtime): captured.append(params) - raise SPMInputError("SPM_GEOGRAPHY_REQUIRED", "intentional test stop") + msg = "SPM_GEOGRAPHY_REQUIRED" + raise SPMInputError(msg, "intentional test stop") monkeypatch.setattr(simulation, "run_simulation_impl", capture) app = FastAPI() diff --git a/projects/policyengine-simulation-executor/tests/test_gcp_credentials.py b/projects/policyengine-simulation-executor/tests/test_gcp_credentials.py index 530689baa..e4eb76cb2 100644 --- a/projects/policyengine-simulation-executor/tests/test_gcp_credentials.py +++ b/projects/policyengine-simulation-executor/tests/test_gcp_credentials.py @@ -43,7 +43,8 @@ def test_setup_gcp_credentials_cleans_up_on_exception( with pytest.raises(RuntimeError): with setup_gcp_credentials(observability_runtime): captured_path.append(os.environ["GOOGLE_APPLICATION_CREDENTIALS"]) - raise RuntimeError("boom") + msg = "boom" + raise RuntimeError(msg) assert captured_path assert not os.path.exists(captured_path[0]) diff --git a/projects/policyengine-simulation-executor/tests/test_segmented_national.py b/projects/policyengine-simulation-executor/tests/test_segmented_national.py index 164a97e97..6ab37e8fa 100644 --- a/projects/policyengine-simulation-executor/tests/test_segmented_national.py +++ b/projects/policyengine-simulation-executor/tests/test_segmented_national.py @@ -167,7 +167,8 @@ def get(self, timeout): self.before_each_poll() if self.errors_once > 0: self.errors_once -= 1 - raise ConnectionError("transient control-plane blip") + msg = "transient control-plane blip" + raise ConnectionError(msg) if self.error is not None: raise self.error if self.polls_until_ready > 0: @@ -205,7 +206,8 @@ def _spawn(self, payload, *, observability_context=None): self.fail_spawn_at is not None and len(self.spawned_payloads) == self.fail_spawn_at ): - raise ConnectionError("spawn RPC failed") + msg = "spawn RPC failed" + raise ConnectionError(msg) self.spawned_payloads.append(payload) self.spawned_observability_contexts.append(observability_context) return self._calls[len(self.spawned_payloads) - 1] @@ -928,7 +930,8 @@ def test__typed_spm_error_spawns_nothing(self, monkeypatch): # Resolution failures are cheap only if they happen before 20 # children are spawned. def unavailable(params): - raise SPMInputError("SPM_GEOGRAPHY_REQUIRED", "County required") + msg = "SPM_GEOGRAPHY_REQUIRED" + raise SPMInputError(msg, "County required") monkeypatch.setattr(executor_spm, "normalize_runtime_spm", unavailable) monkeypatch.setattr( diff --git a/projects/policyengine-simulation-executor/tests/test_segmented_national_reduce.py b/projects/policyengine-simulation-executor/tests/test_segmented_national_reduce.py index 2d0d57b4b..e335019c2 100644 --- a/projects/policyengine-simulation-executor/tests/test_segmented_national_reduce.py +++ b/projects/policyengine-simulation-executor/tests/test_segmented_national_reduce.py @@ -70,7 +70,8 @@ def test__rejects_empty_input(self): class TestPrecomputedSimulation: def test__ensure_never_computes(self, monkeypatch): def boom(self): - raise AssertionError("RECOMPUTE ATTEMPTED") + msg = "RECOMPUTE ATTEMPTED" + raise AssertionError(msg) monkeypatch.setattr(PrecomputedSimulation, "run", boom) simulation = PrecomputedSimulation() 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 06dd86f49..6b157f1fd 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py @@ -122,7 +122,8 @@ def __init__(self, household): self.output_dataset = _FakeOutputDataset(household) def ensure(self): - raise AssertionError("test data is already materialized") + msg = "test data is already materialized" + raise AssertionError(msg) class _TrackedSpan(AbstractContextManager): @@ -299,7 +300,8 @@ def compute(simulation): if simulation is baseline else REFORM_POVERTY_BY_RACE ) - raise AssertionError(f"Unexpected poverty output: {name}") + msg = f"Unexpected poverty output: {name}" + raise AssertionError(msg) return compute @@ -1740,7 +1742,8 @@ def test_builder_budgetary_impact_propagates_required_calculation_errors(monkeyp baseline, reform = _macro_baseline_reform() def fail_change_output_variable(*args, **kwargs): - raise RuntimeError("household_tax missing") + msg = "household_tax missing" + raise RuntimeError(msg) monkeypatch.setattr( "policyengine_simulation_executor.simulation_output_budget._change_output_variable", diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_modal_app.py b/projects/policyengine-simulation-executor/tests/test_stage12_modal_app.py index 811d90746..5f1f528a2 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_modal_app.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_modal_app.py @@ -249,7 +249,8 @@ def test_stage12_functions_close_runtime_after_application_error( monkeypatch.setattr(module, "init_process_observability", lambda **_: runtime) def fail(*_, **__): - raise LookupError("application failure") + msg = "application failure" + raise LookupError(msg) with pytest.raises(LookupError, match="application failure"): _invoke(monkeypatch, module, function_name, fail) diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_output_planning.py b/projects/policyengine-simulation-executor/tests/test_stage12_output_planning.py index f22cb7e8f..c0e5a196d 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_output_planning.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_output_planning.py @@ -190,7 +190,8 @@ def test_unrelated_policy_does_not_activate_labor_supply_outputs() -> None: def test_planning_is_deterministic_and_never_ensures_a_simulation(monkeypatch) -> None: def reject_ensure(_simulation): - raise AssertionError("output planning must not execute a simulation") + msg = "output planning must not execute a simulation" + raise AssertionError(msg) monkeypatch.setattr(Simulation, "ensure", reject_ensure) diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py b/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py index 9b5d9b248..fa7732040 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py @@ -716,7 +716,8 @@ def test_single_worker_exposes_only_a_bounded_failure() -> None: store.children[simulation.simulation_execution_id] = _child(simulation) def fail(_simulation): - raise RuntimeError("country package calculation failed") + msg = "country package calculation failed" + raise RuntimeError(msg) with pytest.raises( RuntimeError, match="Stage 12 simulation execution failed" @@ -1254,7 +1255,8 @@ def run(): started = time.monotonic() time.sleep(0.05) if role == self.fail_role: - raise RuntimeError("child calculation failed") + msg = "child calculation failed" + raise RuntimeError(msg) frames = _frames(100.0 if role == "baseline" else 120.0) if self.missing_column and role == "reform": frames["household"] = frames["household"].drop( @@ -1714,7 +1716,8 @@ def test_coordinator_rejects_artifacts_that_do_not_satisfy_the_shared_plan( def test_coordinator_records_failure_when_child_persistence_fails() -> None: class FailingChildStore(FakeStore): def create_or_resolve_simulation(self, record): - raise RuntimeError("database rejected the child record") + msg = "database rejected the child record" + raise RuntimeError(msg) store = FailingChildStore() @@ -2315,7 +2318,8 @@ def __init__(self, object_id): self.object_id = object_id def get(self, *, timeout=None): - raise TimeoutError("child result was not ready") + msg = "child result was not ready" + raise TimeoutError(msg) class TimeoutInvoker: @@ -2328,7 +2332,8 @@ def spawn(self, *, simulation, **_): return TimeoutCall(f"call-{role}") def restore(self, invocation_id): - raise AssertionError(f"unexpected restored invocation {invocation_id}") + msg = f"unexpected restored invocation {invocation_id}" + raise AssertionError(msg) def test_coordinator_records_bounded_timeout_without_an_aggregate() -> None: From bce1fb7639d5f0f233d0bf4218f1b58a722d72aa Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:45:46 +0400 Subject: [PATCH 08/23] Fix ruff EM findings in the integration tests and repo scripts Exception messages move into a msg variable before the raise, with the text unchanged. Co-Authored-By: Claude Opus 5.5 --- .../scripts/update_policyengine_package.py | 46 +++++++++++-------- .../tests/simulation/conftest.py | 9 ++-- .../tests/simulation/test_calculate.py | 9 ++-- 3 files changed, 40 insertions(+), 24 deletions(-) diff --git a/.github/scripts/update_policyengine_package.py b/.github/scripts/update_policyengine_package.py index c1bb21f2a..5c8240404 100644 --- a/.github/scripts/update_policyengine_package.py +++ b/.github/scripts/update_policyengine_package.py @@ -51,10 +51,11 @@ def current_version(pyproject: Path, package: str) -> str: if isinstance(dependency, str) and dependency.startswith(prefix) ] if len(matches) != 1: - raise SystemExit( + msg = ( f"Expected one {package}[models] requirement in project.dependencies; " f"found {matches!r}" ) + raise SystemExit(msg) return matches[0] @@ -86,9 +87,8 @@ def update_requirements( and dependency.startswith(f"{package}[models]==") ] if matches != [old_requirement]: - raise SystemExit( - f"Expected {old_requirement} in {location}; found {matches!r}" - ) + msg = f"Expected {old_requirement} in {location}; found {matches!r}" + raise SystemExit(msg) redundant_wrapper_requirements = [ dependency @@ -98,10 +98,11 @@ def update_requirements( and _requirement_name(dependency) == package ] if redundant_wrapper_requirements: - raise SystemExit( + msg = ( f"Expected only {old_requirement} for {package} in {location}; " f"found {redundant_wrapper_requirements!r}" ) + raise SystemExit(msg) direct_component_requirements = [ dependency @@ -110,18 +111,20 @@ def update_requirements( and _requirement_name(dependency) in COMPONENT_PACKAGES ] if direct_component_requirements: - raise SystemExit( + msg = ( f"Expected {location} to obtain component packages from " f"{package}[models]; found {direct_component_requirements!r}" ) + raise SystemExit(msg) old_pin = f'"{old_requirement}"' new_pin = f'"{new_requirement}"' if text.count(old_pin) != len(requirements): - raise SystemExit( + msg = ( f"Expected {old_pin} {len(requirements)} times in {pyproject}; " f"found {text.count(old_pin)}" ) + raise SystemExit(msg) with tempfile.NamedTemporaryFile( mode="w", @@ -139,7 +142,8 @@ def find_issue(title: str) -> None: """Print the lowest matching issue number from paginated GitHub output.""" pages = _json_from_stdin() if not isinstance(pages, list): - raise SystemExit("GitHub issue response must be a list of pages") + msg = "GitHub issue response must be a list of pages" + raise SystemExit(msg) matches = sorted( item["number"] for page in pages @@ -158,23 +162,26 @@ def verify_issue(expected_number: int, expected_title: str) -> None: """Validate the issue selected for an automated update.""" issue = _mapping(_json_from_stdin()) if issue.get("number") != expected_number: - raise SystemExit("Resolved update issue has an unexpected number") + msg = "Resolved update issue has an unexpected number" + raise SystemExit(msg) if issue.get("state") != "OPEN": - raise SystemExit("Resolved update issue is not open") + msg = "Resolved update issue is not open" + raise SystemExit(msg) if issue.get("title") != expected_title: - raise SystemExit("Resolved update issue has an unexpected title") + msg = "Resolved update issue has an unexpected title" + raise SystemExit(msg) def verify_pr(expected_repository: str) -> None: """Validate that an automated pull request is a canonical draft.""" pull_request = _mapping(_json_from_stdin()) if pull_request.get("isDraft") is not True: - raise SystemExit("Automated policyengine update PR is not a draft") + msg = "Automated policyengine update PR is not a draft" + raise SystemExit(msg) head_repository = _mapping(pull_request.get("headRepository")) if head_repository.get("nameWithOwner") != expected_repository: - raise SystemExit( - "Automated policyengine update PR is not from the canonical repository" - ) + msg = "Automated policyengine update PR is not from the canonical repository" + raise SystemExit(msg) def latest_version(package: str) -> None: @@ -182,7 +189,8 @@ def latest_version(package: str) -> None: payload = _mapping(_json_from_stdin()) version = _mapping(payload.get("info")).get("version") if not isinstance(version, str) or not version: - raise SystemExit(f"PyPI response has no latest version for {package}") + msg = f"PyPI response has no latest version for {package}" + raise SystemExit(msg) print(version) @@ -197,7 +205,8 @@ def bundle_versions() -> None: def package_version(name: str) -> str: version = _mapping(packages.get(name)).get("version") if not isinstance(version, str) or not version: - raise SystemExit(f"Bundle has no version for {name}") + msg = f"Bundle has no version for {name}" + raise SystemExit(msg) return version def data_release_version(country: str) -> str: @@ -206,7 +215,8 @@ def data_release_version(country: str) -> str: "version" ) if not isinstance(version, str) or not version: - raise SystemExit(f"Bundle has no data release version for {country}") + msg = f"Bundle has no data release version for {country}" + raise SystemExit(msg) return version def data_package_version(country: str) -> str: diff --git a/projects/policyengine-apis-integ/tests/simulation/conftest.py b/projects/policyengine-apis-integ/tests/simulation/conftest.py index c1e616b18..e1c2c808e 100644 --- a/projects/policyengine-apis-integ/tests/simulation/conftest.py +++ b/projects/policyengine-apis-integ/tests/simulation/conftest.py @@ -113,10 +113,11 @@ def _poll_budget_window_batch( return response.parsed if response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR: - raise AssertionError( + msg = ( "Budget-window batch failed: " f"{_decode_response_content(response.content)}" ) + raise AssertionError(msg) delay_seconds = poll_retry_delay_seconds( response.status_code, @@ -130,16 +131,18 @@ def _poll_budget_window_batch( time.sleep(min(delay_seconds, remaining_seconds)) continue - raise AssertionError( + msg = ( "Unexpected budget-window poll status " f"{response.status_code}: {_decode_response_content(response.content)}" ) + raise AssertionError(msg) - raise TimeoutError( + msg = ( f"Budget-window batch {batch_job_id} did not complete within " f"{max_wait_seconds}s; last response was " f"{last_status_code}: {_decode_response_content(last_content)}" ) + raise TimeoutError(msg) @pytest.fixture() diff --git a/projects/policyengine-apis-integ/tests/simulation/test_calculate.py b/projects/policyengine-apis-integ/tests/simulation/test_calculate.py index d4f9578f4..f5e8d1238 100644 --- a/projects/policyengine-apis-integ/tests/simulation/test_calculate.py +++ b/projects/policyengine-apis-integ/tests/simulation/test_calculate.py @@ -113,7 +113,8 @@ def poll_for_completion( return response.parsed if response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR: - raise AssertionError(f"Job failed: {response.content}") + msg = f"Job failed: {response.content}" + raise AssertionError(msg) delay_seconds = poll_retry_delay_seconds( response.status_code, @@ -128,9 +129,11 @@ def poll_for_completion( continue # Unexpected status code - raise AssertionError(f"Unexpected status code: {response.status_code}") + msg = f"Unexpected status code: {response.status_code}" + raise AssertionError(msg) - raise TimeoutError(f"Job {job_id} did not complete within {max_wait_seconds}s") + msg = f"Job {job_id} did not complete within {max_wait_seconds}s" + raise TimeoutError(msg) def assert_job_progress_reported(bodies: list[Mapping]) -> None: From 1b637a0948edf894b9b1644fd392593c4888297d Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:45:56 +0400 Subject: [PATCH 09/23] Enable ruff group 5 (exceptions and messages) Select flake8-blind-except (BLE), flake8-errmsg (EM) and tryceratops (TRY). The preceding commits fix every finding; deliberate catch-alls, kept exception types and traceback-free error logs carry an inline noqa with the reason. Tests get no exemption. Co-Authored-By: Claude Opus 5.5 --- ruff.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ruff.toml b/ruff.toml index d15a4735d..1aa85b582 100644 --- a/ruff.toml +++ b/ruff.toml @@ -17,6 +17,8 @@ select = [ "ANN", # Group 4: docstrings. "D", + # Group 5: exceptions and messages. + "BLE", "EM", "TRY", ] # Mutually exclusive pydocstyle pairs: keep D211 (no blank line before a class # docstring) and D212 (summary on the first line). From 514364e95ceac736b19d6e3a7c2dcb01f9a9d6f2 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:50:08 +0400 Subject: [PATCH 10/23] Give every BLE001 suppression an inline reason Enabling BLE activated suppressions that were written before the rule was on. Seven had no reason, or kept it only in a comment above; they now carry it inline, and the "- reason" ones use the " # reason" form. Co-Authored-By: Claude Opus 5.5 --- .../src/policyengine_simulation_entry/app.py | 12 ++++++------ .../policyengine_simulation_entry/stage12_backend.py | 2 +- .../release_bundle.py | 2 +- .../stage12_runtime/comparison.py | 2 +- .../stage12_runtime/coordination.py | 2 +- .../stage12_runtime/simulation.py | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py index 7eb1265a0..4651c0044 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py @@ -207,7 +207,7 @@ def bind_observability_id(request: Request, value: object) -> str | None: if observability_id is not None: try: runtime.set_context(observability_id=observability_id) - except Exception: # noqa: BLE001, S110 - telemetry is non-fatal + except Exception: # noqa: BLE001, S110 # telemetry is non-fatal pass return observability_id @@ -243,7 +243,7 @@ def adopt_backend_observability_id( "reason": "downstream_identifier_differed", }, ) - except Exception: # noqa: BLE001, S110 - telemetry is non-fatal + except Exception: # noqa: BLE001, S110 # telemetry is non-fatal pass return current return bind_observability_id(request, current or downstream) @@ -327,7 +327,7 @@ async def forward( event_identifiers: RequestIdentifiers = identifiers or {} try: runtime.set_context(backend="old_gateway", **event_identifiers) - except Exception: # noqa: BLE001, S110 - telemetry is non-fatal + except Exception: # noqa: BLE001, S110 # telemetry is non-fatal pass try: result = await runtime_backend.request( @@ -483,7 +483,7 @@ async def submit_temporary_stage12_report( ) # This temporary diagnostic route must convert every integration # failure into an HTTP response rather than crash the application. - except Exception as error: # noqa: BLE001 + except Exception as error: # noqa: BLE001 # every integration failure becomes a 503 logger.error( # noqa: TRY400 # Stage 12 failure logs carry only the error type; a traceback could expose request or calculation data "stage12_direct_submission_failed", extra={ @@ -536,7 +536,7 @@ async def get_temporary_stage12_report( ) # Reading temporary diagnostic state crosses an internal HTTP boundary # whose concrete exception types are implementation details. - except Exception as error: # noqa: BLE001 + except Exception as error: # noqa: BLE001 # any status-read failure becomes a 503 logger.error( # noqa: TRY400 # Stage 12 failure logs carry only the error type; a traceback could expose request or calculation data "stage12_direct_status_failed", extra={ @@ -631,7 +631,7 @@ async def submit_comparison( ) # Automatic Stage 12 work must never change the production v1 # response, including for unexpected integration failures. - except Exception as error: # noqa: BLE001 + except Exception as error: # noqa: BLE001 # Stage 12 dispatch must never change the v1 response logger.error( # noqa: TRY400 # Stage 12 failure logs carry only the error type; a traceback could expose request or calculation data "stage12_comparison_dispatch_failed", extra={ diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_backend.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_backend.py index 9da56afb9..33af3d078 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_backend.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/stage12_backend.py @@ -529,7 +529,7 @@ async def _spawn_report( ), }, ) - except Exception as error: # noqa: BLE001 + except Exception as error: # noqa: BLE001 # any dispatch failure is recorded on the parent run, then re-raised as TemporaryStage12DispatchFailed failed_at = datetime.now(UTC) failed = parent.model_copy( update={ 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 4c0eb5346..feed3409a 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 @@ -230,7 +230,7 @@ def _current_policyengine_bundle() -> Mapping | None: from policyengine.bundle import get_current_bundle return get_current_bundle() - except Exception: # noqa: BLE001 - preserve fallback for invalid legacy bundles + except Exception: # noqa: BLE001 # preserve the fallback for invalid legacy bundles return None diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/comparison.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/comparison.py index be969dafd..59954c130 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/comparison.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/comparison.py @@ -109,7 +109,7 @@ def compare_completed_report( ) # Result retrieval, validation, artifact storage, and persistence are all # deliberately isolated from the successful Stage 12 calculation. - except Exception as error: # noqa: BLE001 + except Exception as error: # noqa: BLE001 # comparison failures are recorded, never allowed to fail the Stage 12 calculation failed_at = datetime.now(UTC) try: latest = store.get_report(parent.evaluation_id) diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/coordination.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/coordination.py index 5fd4391d8..9ba01e9a0 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/coordination.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/coordination.py @@ -564,7 +564,7 @@ def coordinate_report( return descriptor.model_dump(mode="json") # Persist a redacted cause for any coordinator, child-call, aggregation, or # artifact failure before returning a stable exception to Modal. - except Exception as error: # noqa: BLE001 + except Exception as error: # noqa: BLE001 # every failure is persisted before a stable exception returns to Modal if error is not reported_child_failure: report_original_failure( error, diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/simulation.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/simulation.py index 16ef2f7ae..20f2874c1 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/simulation.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/simulation.py @@ -382,7 +382,7 @@ def run_single_simulation( return descriptor.model_dump(mode="json") # Persist a redacted cause for any country-package calculation failure # before returning a stable exception to Modal. - except Exception as error: # noqa: BLE001 + except Exception as error: # noqa: BLE001 # every calculation failure is persisted before a stable exception returns to Modal report_original_failure( error, runtime=runtime, From 1dfb1ffeb2cee1c44553442f364b4609551aa9ca Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:50:08 +0400 Subject: [PATCH 11/23] Name hoisted SPM error codes error_code, not msg The EM fix moves an exception's first literal argument into a variable called msg. SPMInputError's first argument is an error code, followed by the message, so those 20 variables are renamed error_code. The gateway file also moves its BLE001 suppression reason to the " # reason" form. Co-Authored-By: Claude Opus 5.5 --- .../policyengine_simulation_contract/spm.py | 30 +++++++------- .../policyengine_simulation_executor/spm.py | 40 +++++++++---------- .../tests/test_canonical_spm.py | 4 +- .../tests/test_segmented_national.py | 4 +- .../endpoints.py | 6 +-- 5 files changed, 43 insertions(+), 41 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 ccb5f7bf1..450950d69 100644 --- a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/spm.py +++ b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/spm.py @@ -506,8 +506,10 @@ def resolve_spm_selection( """Resolve only certified metadata; unrecognized future bundles fail closed.""" if country.lower() != "us": if selection is not None: - msg = "SPM_SETTINGS_INVALID" - raise SPMInputError(msg, "SPM settings are supported only for the US") + error_code = "SPM_SETTINGS_INVALID" + raise SPMInputError( + error_code, "SPM settings are supported only for the US" + ) return None if capability is None: # Existing versioned worker bundles predate canonical SPM. This does @@ -541,16 +543,16 @@ def resolve_spm_selection( ) if selection is None and historical: return None - msg = "SPM_CONFIGURATION_UNAVAILABLE" + error_code = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - msg, + error_code, "This worker bundle has no certified canonical SPM capability", ) try: return _certified_spm_selection(capability, selection) except ValueError as exc: - msg = "SPM_SETTINGS_INVALID" - raise SPMInputError(msg, str(exc)) from exc + error_code = "SPM_SETTINGS_INVALID" + raise SPMInputError(error_code, str(exc)) from exc def _validated_spm_provenance( @@ -593,24 +595,24 @@ def validate_spm_result( ) -> SPMComparisonProvenance | None: """Do not accept incomplete or mixed-method child/cached output.""" if "spm_config" in result: - msg = "SPM_CONFIGURATION_UNAVAILABLE" + error_code = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - msg, + error_code, "Completed results must not contain legacy spm_config metadata", ) if selection is None: if result.get("spm_provenance") is not None: - msg = "SPM_CONFIGURATION_UNAVAILABLE" + error_code = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - msg, + error_code, "Unexpected SPM receipt for a historical result", ) return None try: return _validated_spm_provenance(result, selection, expected_year=expected_year) except ValueError as exc: - msg = "SPM_CONFIGURATION_UNAVAILABLE" - raise SPMInputError(msg, str(exc)) from exc + error_code = "SPM_CONFIGURATION_UNAVAILABLE" + raise SPMInputError(error_code, str(exc)) from exc def combine_spm_results( @@ -634,8 +636,8 @@ def combine_spm_results( validated: list[SPMComparisonProvenance] = [] for receipt in receipts: if receipt is None: - msg = "SPM_CONFIGURATION_UNAVAILABLE" - raise SPMInputError(msg, "Missing canonical SPM receipt") + error_code = "SPM_CONFIGURATION_UNAVAILABLE" + raise SPMInputError(error_code, "Missing canonical SPM receipt") validated.append(receipt) combined = build_spm_comparison_provenance( baseline_receipts=[ diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/spm.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/spm.py index 2dcc18383..62ab5532a 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/spm.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/spm.py @@ -58,9 +58,9 @@ def runtime_spm_capability() -> SPMCapability | None: bundle = get_current_bundle() configured = bundle.get("measurements", {}).get("spm") except (ImportError, ValueError, TypeError, AttributeError) as exc: - msg = "SPM_CONFIGURATION_UNAVAILABLE" + error_code = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - msg, + error_code, "The installed SPM bundle configuration is unavailable", ) from exc if configured is None: @@ -68,16 +68,16 @@ def runtime_spm_capability() -> SPMCapability | None: if "spm" not in Simulation.model_fields or not hasattr( Simulation, "spm_provenance" ): - msg = "SPM_CONFIGURATION_UNAVAILABLE" + error_code = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - msg, + error_code, "The installed wrapper does not support canonical SPM", ) try: return _certified_us_spm_capability(configured) except (ImportError, ValueError, TypeError, OSError) as exc: - msg = "SPM_CONFIGURATION_UNAVAILABLE" - raise SPMInputError(msg, str(exc)) from exc + error_code = "SPM_CONFIGURATION_UNAVAILABLE" + raise SPMInputError(error_code, str(exc)) from exc def _certified_us_spm_capability(configured: object) -> SPMCapability: @@ -153,8 +153,8 @@ def _prevalidate_selection( as_of=selection["as_of"], ) except ValueError as exc: - msg = "SPM_GEOGRAPHY_UNAVAILABLE" - raise SPMInputError(msg, str(exc)) from None + error_code = "SPM_GEOGRAPHY_UNAVAILABLE" + raise SPMInputError(error_code, str(exc)) from None # Bound at import, so clearing still works while a test has replaced one of @@ -219,10 +219,10 @@ def normalize_runtime_spm(params: dict[str, Any]) -> dict[str, Any] | None: # plain scenario error; use the same narrow translation as the # calculator's Frame and Axiom adapters, leaving other errors alone. if str(exc).startswith("Unknown forecast scenario:"): - msg = "SPM_SCENARIO_UNAVAILABLE" - raise SPMInputError(msg, str(exc)) from exc - msg = "SPM_SETTINGS_INVALID" - raise SPMInputError(msg, str(exc)) from exc + error_code = "SPM_SCENARIO_UNAVAILABLE" + raise SPMInputError(error_code, str(exc)) from exc + error_code = "SPM_SETTINGS_INVALID" + raise SPMInputError(error_code, str(exc)) from exc return selection @@ -285,16 +285,16 @@ def simulation_spm_calculation_provenance( """Read and compact one simulation's resolved SPM receipt.""" resolved = SPMSelection.model_validate(selection) if getattr(simulation, "spm_config", None) != resolved.model_dump(mode="json"): - msg = "SPM_CONFIGURATION_UNAVAILABLE" + error_code = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - msg, + error_code, "Simulation ignored the requested SPM selection", ) receipt = getattr(simulation, "spm_provenance", None) if not callable(receipt): - msg = "SPM_CONFIGURATION_UNAVAILABLE" + error_code = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - msg, + error_code, "Simulation has no valid SPM calculation receipt", ) try: @@ -303,9 +303,9 @@ def simulation_spm_calculation_provenance( receipt=compact_spm_provenance(receipt(), resolved), ) except (TypeError, ValueError) as exc: - msg = "SPM_CONFIGURATION_UNAVAILABLE" + error_code = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - msg, + error_code, "Simulation has no valid SPM calculation receipt", ) from exc @@ -334,9 +334,9 @@ def simulation_spm_result( if expected_year is not None and str(expected_year) not in ( comparison.baseline.receipt.years ): - msg = "SPM_CONFIGURATION_UNAVAILABLE" + error_code = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - msg, + error_code, "Result SPM provenance does not cover the requested year", ) return { diff --git a/projects/policyengine-simulation-executor/tests/test_canonical_spm.py b/projects/policyengine-simulation-executor/tests/test_canonical_spm.py index a52f9ac40..11d897bed 100644 --- a/projects/policyengine-simulation-executor/tests/test_canonical_spm.py +++ b/projects/policyengine-simulation-executor/tests/test_canonical_spm.py @@ -274,8 +274,8 @@ def test_sync_compatibility_endpoint_preserves_explicit_null( def capture(params, *, runtime): captured.append(params) - msg = "SPM_GEOGRAPHY_REQUIRED" - raise SPMInputError(msg, "intentional test stop") + error_code = "SPM_GEOGRAPHY_REQUIRED" + raise SPMInputError(error_code, "intentional test stop") monkeypatch.setattr(simulation, "run_simulation_impl", capture) app = FastAPI() diff --git a/projects/policyengine-simulation-executor/tests/test_segmented_national.py b/projects/policyengine-simulation-executor/tests/test_segmented_national.py index 6ab37e8fa..473ccbd68 100644 --- a/projects/policyengine-simulation-executor/tests/test_segmented_national.py +++ b/projects/policyengine-simulation-executor/tests/test_segmented_national.py @@ -930,8 +930,8 @@ def test__typed_spm_error_spawns_nothing(self, monkeypatch): # Resolution failures are cheap only if they happen before 20 # children are spawned. def unavailable(params): - msg = "SPM_GEOGRAPHY_REQUIRED" - raise SPMInputError(msg, "County required") + error_code = "SPM_GEOGRAPHY_REQUIRED" + raise SPMInputError(error_code, "County required") monkeypatch.setattr(executor_spm, "normalize_runtime_spm", unavailable) monkeypatch.setattr( diff --git a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/endpoints.py b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/endpoints.py index 334356309..194b1af69 100644 --- a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/endpoints.py +++ b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/endpoints.py @@ -569,9 +569,9 @@ def _build_policyengine_bundle( try: capability = SPMCapability.model_validate(capability) except ValueError as exc: - msg = "SPM_CONFIGURATION_UNAVAILABLE" + error_code = "SPM_CONFIGURATION_UNAVAILABLE" raise SPMInputError( - msg, + error_code, "This worker bundle has invalid canonical SPM capability metadata", ) from exc return PolicyEngineBundle( @@ -729,7 +729,7 @@ def _bind_observability_id( if observability_id is not None: try: runtime.set_context(observability_id=observability_id) - except Exception: # noqa: BLE001, S110 - telemetry is non-fatal + except Exception: # noqa: BLE001, S110 # telemetry is non-fatal pass return observability_id From 5aedfd79c3cc5f922adfaebc148ac47d69841a9e Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:50:17 +0400 Subject: [PATCH 12/23] Register TRY, EM and BLE in the ruff-lint rule sim.quality.ruff-lint now lists the group 5 families, and the rendered rules page and AGENTS.md Rules block are regenerated. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- changelog_entry.yaml | 1 + docs/generated/rules.md | 8 ++++---- rules/registry.toml | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f4b120e32..d6c088c79 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -143,7 +143,7 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. - `sim.quality.every-project-tested`: Every project and library with unit tests runs them in the PR test matrix. (doc-only, not checked: follow it yourself) - `sim.quality.ruff-format-src`: Source code under projects/*/src and libs/*/src is formatted with ruff format. (blocking: ruff format) - `sim.quality.ruff-format-tests`: Test code is formatted with ruff format too; CI currently checks only src directories. (doc-only, not checked: follow it yourself) -- `sim.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in ruff.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (ANN, D, E4, E7, E9, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, 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, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, 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 c65b39a34..0ab453bca 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 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; error types and message text are unchanged - 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 - Enabled ruff's typing-only import rules (TC) and moved annotation-only imports under TYPE_CHECKING, keeping runtime imports for Pydantic models and FastAPI endpoints diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 34584216c..f2daf773d 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -249,7 +249,7 @@ flowchart LR | `sim.quality.every-project-tested` | Every project and library with unit tests runs them in the PR test matrix. | mechanical | doc-only | no | nothing | — | — | | `sim.quality.ruff-format-src` | Source code under projects/*/src and libs/*/src is formatted with ruff format. | mechanical | enforced | yes | ruff format | `pr.yml:lint` | — | | `sim.quality.ruff-format-tests` | Test code is formatted with ruff format too; CI currently checks only src directories. | mechanical | doc-only | no | nothing | — | — | -| `sim.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in ruff.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. | mechanical | enforced | yes | ruff check (ANN, D, E4, E7, E9, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, 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, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, 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` | — | @@ -263,7 +263,7 @@ flowchart LR 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__D__E4__E7__E9__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__UP__W_["ruff check (ANN, D, E4, E7, E9, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, UP, W)"] + 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)"] 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"] @@ -275,8 +275,8 @@ flowchart LR 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__D__E4__E7__E9__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__UP__W_ - e_ruff_check__ANN__D__E4__E7__E9__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__UP__W_ --> j_CI_pr_yml_lint + 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_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 1fa8ca420..17cc3f656 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -733,7 +733,7 @@ autofix = true [rule.enforcer] kind = "ruff" mode = "lint" -codes = ["ANN", "D", "E4", "E7", "E9", "F", "FLY", "FURB", "I", "PIE", "PYI", "RET", "RSE", "TC", "UP", "W"] +codes = ["ANN", "BLE", "D", "E4", "E7", "E9", "EM", "F", "FLY", "FURB", "I", "PIE", "PYI", "RET", "RSE", "TC", "TRY", "UP", "W"] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "lint" From 442b6754634a3315938b5e4ac93f0513357cac45 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:58:51 +0400 Subject: [PATCH 13/23] Name hoisted parity mismatch codes error_code, not msg ParityMismatch takes a code and a schema path, not a message, so the 26 variables the EM fix named msg are renamed error_code. Co-Authored-By: Claude Opus 5.5 --- .../stage12_parity.py | 60 +++++++++---------- .../stage12_qualification.py | 44 +++++++------- 2 files changed, 52 insertions(+), 52 deletions(-) diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_parity.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_parity.py index 6a4c8354d..57d7b7d15 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_parity.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_parity.py @@ -46,11 +46,11 @@ def canonicalize_simulation_frames( identifier = f"{entity}_id" frame = pd.DataFrame(frames[entity]).copy() if identifier not in frame.columns: - msg = "missing_row_identity" - raise ParityMismatch(msg, entity) + error_code = "missing_row_identity" + raise ParityMismatch(error_code, entity) if frame[identifier].duplicated().any(): - msg = "duplicate_row_identity" - raise ParityMismatch(msg, entity) + error_code = "duplicate_row_identity" + raise ParityMismatch(error_code, entity) canonical[entity] = ( frame.sort_values(identifier, kind="stable") .reset_index(drop=True) @@ -74,28 +74,28 @@ def compare_simulation_frames( expected = canonicalize_simulation_frames(incumbent) actual = canonicalize_simulation_frames(candidate) if expected.keys() != actual.keys(): - msg = "entity_membership_mismatch" - raise ParityMismatch(msg, "simulation") + error_code = "entity_membership_mismatch" + raise ParityMismatch(error_code, "simulation") allowed = tolerances or {} for entity, expected_frame in expected.items(): actual_frame = actual[entity] if tuple(expected_frame.columns) != tuple(actual_frame.columns): - msg = "column_membership_mismatch" - raise ParityMismatch(msg, entity) + error_code = "column_membership_mismatch" + raise ParityMismatch(error_code, entity) if len(expected_frame) != len(actual_frame): - msg = "row_membership_mismatch" - raise ParityMismatch(msg, entity) + error_code = "row_membership_mismatch" + raise ParityMismatch(error_code, entity) identifier = f"{entity}_id" if not expected_frame[identifier].equals(actual_frame[identifier]): - msg = "row_identity_mismatch" - raise ParityMismatch(msg, entity) + error_code = "row_identity_mismatch" + raise ParityMismatch(error_code, entity) for column in expected_frame.columns: path = f"{entity}.{column}" expected_values = expected_frame[column] actual_values = actual_frame[column] if str(expected_values.dtype) != str(actual_values.dtype): - msg = "dtype_mismatch" - raise ParityMismatch(msg, path) + error_code = "dtype_mismatch" + raise ParityMismatch(error_code, path) if pd.api.types.is_numeric_dtype(expected_values.dtype): tolerance = allowed.get(path, NumericalTolerance()) if not np.allclose( @@ -105,11 +105,11 @@ def compare_simulation_frames( atol=tolerance.absolute, equal_nan=True, ): - msg = "numeric_value_mismatch" - raise ParityMismatch(msg, path) + error_code = "numeric_value_mismatch" + raise ParityMismatch(error_code, path) elif not expected_values.equals(actual_values): - msg = "value_mismatch" - raise ParityMismatch(msg, path) + error_code = "value_mismatch" + raise ParityMismatch(error_code, path) def compare_aggregate_reports( @@ -129,8 +129,8 @@ def compare_aggregate_reports( def compare(expected: object, actual: object, path: str) -> None: if isinstance(expected, Mapping): if not isinstance(actual, Mapping) or expected.keys() != actual.keys(): - msg = "field_membership_mismatch" - raise ParityMismatch(msg, path or "report") + error_code = "field_membership_mismatch" + raise ParityMismatch(error_code, path or "report") for key in sorted(expected): compare(expected[key], actual[key], f"{path}.{key}".lstrip(".")) return @@ -140,11 +140,11 @@ def compare(expected: object, actual: object, path: str) -> None: if not isinstance(actual, Sequence) or isinstance( actual, (str, bytes, bytearray) ): - msg = "value_type_mismatch" - raise ParityMismatch(msg, path) + error_code = "value_type_mismatch" + raise ParityMismatch(error_code, path) if len(expected) != len(actual): - msg = "list_length_mismatch" - raise ParityMismatch(msg, path) + error_code = "list_length_mismatch" + raise ParityMismatch(error_code, path) for index, value in enumerate(expected): compare(value, actual[index], f"{path}[{index}]") return @@ -161,14 +161,14 @@ def compare(expected: object, actual: object, path: str) -> None: rel_tol=tolerance.relative, abs_tol=tolerance.absolute, ): - msg = "numeric_value_mismatch" - raise ParityMismatch(msg, path) + error_code = "numeric_value_mismatch" + raise ParityMismatch(error_code, path) return if type(expected) is not type(actual): - msg = "value_type_mismatch" - raise ParityMismatch(msg, path) + error_code = "value_type_mismatch" + raise ParityMismatch(error_code, path) if expected != actual: - msg = "value_mismatch" - raise ParityMismatch(msg, path) + error_code = "value_mismatch" + raise ParityMismatch(error_code, path) compare(incumbent, candidate, "") 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 292d5e356..0522f525c 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 @@ -131,19 +131,19 @@ def _validate_matching_input( if field == "spm" and expected_value is None and actual_value is None: continue if actual_value != expected_value: - msg = "normalized_input_mismatch" - raise ParityMismatch(msg, field) + error_code = "normalized_input_mismatch" + raise ParityMismatch(error_code, field) if existing_request.get("data") not in accepted_dataset_values: - msg = "dataset_provenance_mismatch" - raise ParityMismatch(msg, "data") + error_code = "dataset_provenance_mismatch" + raise ParityMismatch(error_code, "data") population = baseline.population if population.kind != "dataset": - msg = "population_kind_mismatch" - raise ParityMismatch(msg, "population.kind") + error_code = "population_kind_mismatch" + raise ParityMismatch(error_code, "population.kind") if population.artifact.uri != bundle.dataset.uri: - msg = "dataset_provenance_mismatch" + error_code = "dataset_provenance_mismatch" raise ParityMismatch( - msg, + error_code, "population.artifact.uri", ) @@ -154,27 +154,27 @@ def _existing_frames( side: Literal["baseline", "reform"], ) -> dict[str, pd.DataFrame]: if not isinstance(microdata, Mapping): - msg = "missing_existing_microdata" - raise ParityMismatch(msg, "report._microdata") + error_code = "missing_existing_microdata" + raise ParityMismatch(error_code, "report._microdata") raw_side = microdata.get(side) raw_dtypes = microdata.get("dtypes") side_dtypes = raw_dtypes.get(side) if isinstance(raw_dtypes, Mapping) else None if not isinstance(raw_side, Mapping) or not isinstance(side_dtypes, Mapping): - msg = "invalid_existing_microdata" - raise ParityMismatch(msg, f"report._microdata.{side}") + error_code = "invalid_existing_microdata" + raise ParityMismatch(error_code, f"report._microdata.{side}") frames: dict[str, pd.DataFrame] = {} for entity, columns in raw_side.items(): entity_dtypes = side_dtypes.get(entity) if not isinstance(entity, str) or not isinstance(columns, dict): - msg = "invalid_existing_microdata" + error_code = "invalid_existing_microdata" raise ParityMismatch( - msg, + error_code, f"report._microdata.{side}", ) if not isinstance(entity_dtypes, dict): - msg = "invalid_existing_dtypes" + error_code = "invalid_existing_dtypes" raise ParityMismatch( - msg, + error_code, f"report._microdata.dtypes.{side}.{entity}", ) frames[entity] = rebuild_entity_frame(columns, entity_dtypes) @@ -274,15 +274,15 @@ def run_existing(request: dict[str, Any]) -> Mapping[str, Any]: bundle = report.baseline.bundle if existing_result.get("model_version") != bundle.country_package_version: - msg = "country_package_provenance_mismatch" + error_code = "country_package_provenance_mismatch" raise ParityMismatch( - msg, + error_code, "report.model_version", ) if existing_result.get("data_version") != bundle.dataset.artifact_revision: - msg = "dataset_provenance_mismatch" + error_code = "dataset_provenance_mismatch" raise ParityMismatch( - msg, + error_code, "report.data_version", ) @@ -320,8 +320,8 @@ def run_existing(request: dict[str, Any]) -> Mapping[str, Any]: ) v2_result = v2_report.get("result") if not isinstance(v2_result, Mapping): - msg = "invalid_v2_aggregate" - raise ParityMismatch(msg, "report.result") + error_code = "invalid_v2_aggregate" + raise ParityMismatch(error_code, "report.result") compare_aggregate_reports( existing_result, v2_result, From fe0be04ebf623fd2509c8e746b53d2fa7eaa2c17 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 20:59:04 +0400 Subject: [PATCH 14/23] Fix the group 5 changelog line's YAML and wording The colon after "(TRY, EM, BLE)" made YAML read the item as a mapping rather than a string. The line also said error types were unchanged, but two type checks in qualify_stage12_parity.py now raise TypeError. Co-Authored-By: Claude Opus 5.5 --- changelog_entry.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog_entry.yaml b/changelog_entry.yaml index 0ab453bca..4d1de430b 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 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; error types and message text are unchanged + - 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 - Enabled ruff's typing-only import rules (TC) and moved annotation-only imports under TYPE_CHECKING, keeping runtime imports for Pydantic models and FastAPI endpoints From 180adfb729e0878cff9ee8f7e359dc6f6fbd290b Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 14 Oct 2026 20:39:19 +0400 Subject: [PATCH 15/23] Build the exception messages #743 and #744 added before raising #743 and #744 landed on main after this stack branched. EM and TRY003, which this PR enables, flag 34 raises in their sentinel, routing and package-update code that pass a literal message. Each message now moves into a msg variable before the raise, with the text unchanged. Co-Authored-By: Claude Opus 5.5 --- .../scripts/update_policyengine_package.py | 17 +++-- .../src/modal/utils/release_sentinel.py | 24 ++++--- .../modal/utils/update_version_registry.py | 65 ++++++++++++------- 3 files changed, 68 insertions(+), 38 deletions(-) diff --git a/.github/scripts/update_policyengine_package.py b/.github/scripts/update_policyengine_package.py index 5c8240404..f570b34ec 100644 --- a/.github/scripts/update_policyengine_package.py +++ b/.github/scripts/update_policyengine_package.py @@ -235,7 +235,8 @@ def dataset_sha256(country: str) -> str: value = reference.get("sha256") or "" digest = str(value).strip().lower() if digest and not re.fullmatch(r"[0-9a-f]{64}", digest): - raise SystemExit(f"Bundle records a malformed {country} dataset SHA-256") + msg = f"Bundle records a malformed {country} dataset SHA-256" + raise SystemExit(msg) return digest def certificate_sha256(country: str) -> str: @@ -305,10 +306,11 @@ def verify_dispatch() -> None: if payload.get("version") and payload["version"] != installed.get( "policyengine_version" ): - raise SystemExit( + msg = ( f"Dispatch is for policyengine {payload['version']}, but " f"{installed.get('policyengine_version')} is installed." ) + raise SystemExit(msg) expected = _mapping(_mapping(payload.get("bundle")).get("uk")) mismatches = [] compared = 0 @@ -339,17 +341,18 @@ def verify_dispatch() -> None: verdict = qualification.get("verdict") or "" receipt_sha256 = qualification.get("receipt_sha256") or "" if not isinstance(verdict, str) or not re.fullmatch(r"[a-z_]*", verdict): - raise SystemExit("The release dispatch carries a malformed verdict") + msg = "The release dispatch carries a malformed verdict" + raise SystemExit(msg) if not isinstance(receipt_sha256, str) or not re.fullmatch( r"(?:[0-9a-f]{64})?", receipt_sha256 ): - raise SystemExit("The release dispatch carries a malformed receipt digest") + msg = "The release dispatch carries a malformed receipt digest" + raise SystemExit(msg) if qualification.get("promotable") is False or ( verdict and verdict not in PROMOTABLE_VERDICTS ): - raise SystemExit( - f"The release dispatch says the UK pair was not promotable ({verdict})" - ) + 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}") 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 fc6bcf76c..ea776a9ad 100644 --- a/projects/policyengine-simulation-executor/src/modal/utils/release_sentinel.py +++ b/projects/policyengine-simulation-executor/src/modal/utils/release_sentinel.py @@ -94,12 +94,15 @@ def value_at(result: Mapping[str, Any], path: str) -> float: node: Any = result for key in path.split("."): if not isinstance(node, Mapping) or key not in node: - raise SentinelError(f"the result has no {path}") + msg = f"the result has no {path}" + raise SentinelError(msg) node = node[key] if isinstance(node, bool) or not isinstance(node, (int, float)): - raise SentinelError(f"{path} is {node!r}, not a number") + msg = f"{path} is {node!r}, not a number" + raise SentinelError(msg) if not math.isfinite(node): - raise SentinelError(f"{path} is {node}, not a finite number") + msg = f"{path} is {node}, not a finite number" + raise SentinelError(msg) return float(node) @@ -148,7 +151,8 @@ def compute_expected( runner = runner or _local_runner() identity = dict(identity or _bundle_identity()) if not identity["uk"].get("dataset_sha256"): - raise SentinelError("the installed bundle records no UK dataset SHA-256") + msg = "the installed bundle records no UK dataset SHA-256" + raise SentinelError(msg) recorded = [] for case in cases: started = time.monotonic() @@ -203,11 +207,13 @@ def access_token(environ: Mapping[str, str] = os.environ) -> str | None: present = [name for name in _AUTH_VARS if environ.get(name)] if not present: if _truthy(environ.get("SIMULATION_TEST_AUTH_REQUIRED")): - raise SentinelError("SIMULATION_TEST_AUTH_REQUIRED is set but no auth vars") + msg = "SIMULATION_TEST_AUTH_REQUIRED is set but no auth vars" + raise SentinelError(msg) return None if len(present) != len(_AUTH_VARS): missing = sorted(set(_AUTH_VARS) - set(present)) - raise SentinelError(f"partial auth configuration; missing {missing}") + msg = f"partial auth configuration; missing {missing}" + raise SentinelError(msg) status, _, payload = _http( "POST", environ["SIMULATION_TEST_AUTH_ISSUER"].rstrip("/") + "/oauth/token", @@ -220,7 +226,8 @@ def access_token(environ: Mapping[str, str] = os.environ) -> str | None: ) token = payload.get("access_token") if isinstance(payload, Mapping) else None if status != 200 or not token: - raise SentinelError(f"token request failed with HTTP {status}") + msg = f"token request failed with HTTP {status}" + raise SentinelError(msg) return str(token) @@ -247,7 +254,8 @@ def check_deployment( ) -> list[str]: """Submit each expected case to ``base_url`` and return what disagrees.""" if expected.get("schema") != SCHEMA: - raise SentinelError(f"expected file schema is {expected.get('schema')!r}") + msg = f"expected file schema is {expected.get('schema')!r}" + raise SentinelError(msg) http = http or _http base_url = base_url.rstrip("/") version = expected["policyengine_version"] 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 fcd045843..6fef5ef89 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 @@ -355,22 +355,25 @@ def build_restored_routing_state( state = _coerce_routing_state(current_state) current_latest = state["latest"].get("policyengine") if expected_latest is not None and current_latest != expected_latest: - raise ValueError( + msg = ( f"latest.policyengine is {current_latest!r}, not {expected_latest!r}; " "another release changed routing, so this rollback was not applied" ) + raise ValueError(msg) app_name = state["routes"]["policyengine"].get(policyengine_version) manifest = state["bundles"].get(policyengine_version) if not app_name or not isinstance(manifest, dict): - raise ValueError( + msg = ( f"policyengine {policyengine_version!r} has no published route and " "bundle manifest to restore" ) + raise ValueError(msg) if manifest.get("app_name") != app_name: - raise ValueError( + msg = ( f"policyengine {policyengine_version!r}'s manifest names app " f"{manifest.get('app_name')!r}, but its route is {app_name!r}" ) + raise ValueError(msg) restored_models = { country: manifest[country]["model_version"] for country in ("us", "uk") } @@ -463,12 +466,15 @@ def plan_routing_publication( def validate_routing_plan(plan: object) -> dict: """Return a plan whose digests match its states, or raise ``ValueError``.""" if not isinstance(plan, dict) or plan.get("schema") != ROUTING_PLAN_SCHEMA: - raise ValueError(f"not a {ROUTING_PLAN_SCHEMA} routing plan") + msg = f"not a {ROUTING_PLAN_SCHEMA} routing plan" + raise ValueError(msg) for side in ("before", "after"): if routing_state_digest(plan.get(side)) != plan.get(f"{side}_sha256"): - raise ValueError(f"the plan's {side} state does not match its digest") + msg = f"the plan's {side} state does not match its digest" + raise ValueError(msg) if not isinstance(plan["after"], dict): - raise ValueError("the plan publishes no routing state") # noqa: TRY004 + msg = "the plan publishes no routing state" + raise ValueError(msg) # noqa: TRY004 return plan @@ -524,13 +530,14 @@ def _require_serialized_routing_workflow() -> None: ): return workflows = ", ".join(sorted(SERIALIZED_ROUTING_WORKFLOW_FILES)) - raise RuntimeError( + msg = ( "Routing writes require GitHub Actions in " f"{CANONICAL_GITHUB_REPOSITORY} through {workflows}. " "Use those workflows, whose simulation-deploy-main concurrency group " "serializes routing writes. Read-only, --dry-run and --plan-out modes are available " "locally." ) + raise RuntimeError(msg) def _unfinished_publication(store: "modal.Dict | None") -> dict | None: @@ -542,9 +549,8 @@ def _unfinished_publication(store: "modal.Dict | None") -> dict | None: "completed", "restored", }: - raise RoutingConflict( - "invalid routing publication record; refusing routing writes" - ) + msg = "invalid routing publication record; refusing routing writes" + raise RoutingConflict(msg) validate_routing_plan(record.get("plan")) return record["plan"] if record["status"] == "pending" else None @@ -554,19 +560,21 @@ def _require_finished_publication( ) -> None: pending = _unfinished_publication(store) if pending is not None and not any(pending == plan for plan in allowed_plans): - raise RoutingConflict( + msg = ( f"Unfinished routing publication {pending['publication_id']}: " "refusing new routing writes. Inspect --print-pending-plan and restore " "the recorded plan through simulation-rollback with target and " "resolve_pending_publication=true before deploying again." ) + raise RoutingConflict(msg) def _write_publication_record(store: "modal.Dict", *, plan: dict, status: str) -> None: record = {"status": status, "plan": _json_copy(plan)} store[ROUTING_PUBLICATION_KEY] = record if store.get(ROUTING_PUBLICATION_KEY) != record: - raise RoutingConflict("the routing publication record reads back differently") + msg = "the routing publication record reads back differently" + raise RoutingConflict(msg) def record_routing_plan(*, environment: str, plan: dict) -> None: @@ -584,7 +592,8 @@ def record_routing_plan(*, environment: str, plan: dict) -> None: routing_state_digest(store.get(ROUTING_STATE_ACTIVE_KEY)) != plan["before_sha256"] ): - raise RoutingConflict("the routing state changed before recording the plan") + msg = "the routing state changed before recording the plan" + raise RoutingConflict(msg) _write_publication_record(store, plan=plan, status="pending") @@ -594,12 +603,14 @@ def complete_routing_plan(*, environment: str, plan: dict) -> None: validate_routing_plan(plan) store = _routing_store(environment, create_if_missing=False) if _unfinished_publication(store) != plan: - raise RoutingConflict("no matching unfinished routing publication to complete") + msg = "no matching unfinished routing publication to complete" + raise RoutingConflict(msg) if ( routing_state_digest(store.get(ROUTING_STATE_ACTIVE_KEY)) != plan["after_sha256"] ): - raise RoutingConflict("the completed release's routing state is not live") + msg = "the completed release's routing state is not live" + raise RoutingConflict(msg) _write_publication_record(store, plan=plan, status="completed") @@ -611,14 +622,16 @@ def apply_routing_plan(*, environment: str, plan: dict) -> None: _require_finished_publication(store, allowed_plans=[plan]) current = store.get(ROUTING_STATE_ACTIVE_KEY) if routing_state_digest(current) != plan["before_sha256"]: - raise RoutingConflict( + msg = ( "the routing state changed after this release was planned; " "nothing was published" ) + raise RoutingConflict(msg) store[ROUTING_STATE_ACTIVE_KEY] = plan["after"] written = store.get(ROUTING_STATE_ACTIVE_KEY) if routing_state_digest(written) != plan["after_sha256"]: - raise RoutingConflict("the published routing state reads back differently") + msg = "the published routing state reads back differently" + raise RoutingConflict(msg) def restore_routing_plans(*, environment: str, plans: list[dict]) -> str: @@ -634,14 +647,16 @@ def restore_routing_plans(*, environment: str, plans: list[dict]) -> str: for plan in plans: validate_routing_plan(plan) if not plans: - raise ValueError("no routing plan to restore") + msg = "no routing plan to restore" + raise ValueError(msg) store = _routing_store(environment, create_if_missing=False) _require_finished_publication(store, allowed_plans=plans) pending = _unfinished_publication(store) if store is None: if all(plan["before"] is None for plan in plans): return "unchanged" - raise RoutingConflict("there is no routing state to restore into") + msg = "there is no routing state to restore into" + raise RoutingConflict(msg) remaining = [ plan for plan in plans if plan["before_sha256"] != plan["after_sha256"] ] @@ -661,15 +676,17 @@ def restore_routing_plans(*, environment: str, plans: list[dict]) -> str: routing_state_digest(store.get(ROUTING_STATE_ACTIVE_KEY)) != plan["before_sha256"] ): - raise RoutingConflict("the restored routing state reads back differently") + msg = "the restored routing state reads back differently" + raise RoutingConflict(msg) outcome = "restored" if outcome == "unchanged" and not any( plan["before_sha256"] == digest for plan in plans ): - raise RoutingConflict( + msg = ( "the routing state is neither what this release replaced nor what " "it published; another change was made, so nothing was restored" ) + raise RoutingConflict(msg) if pending is not None: _write_publication_record(store, plan=pending, status="restored") return outcome @@ -716,9 +733,11 @@ def finalize_routing_publication( """ _require_serialized_routing_workflow() if release_environment not in {"beta", "prod"}: - raise ValueError("release_environment must be beta or prod") + msg = "release_environment must be beta or prod" + raise ValueError(msg) if publication_run is None and gates_passed: - raise ValueError("operator recovery requires gates_passed=false") + msg = "operator recovery requires gates_passed=false" + raise ValueError(msg) store = _routing_store(environment, create_if_missing=False) pending = _unfinished_publication(store) if pending is None: From 2647fa19a8eb26aaf58f16b9ca307e97d90fc185 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 14 Oct 2026 21:21:06 +0400 Subject: [PATCH 16/23] Build the exception messages #789's tests raise before raising PR enables, flag five raises in its new Stage 12 tests that pass a literal message; each message now moves into a msg variable first. TRY301 flags the raise inside the pooling test's transaction, which moves into a small helper so the same RuntimeError still ends that transaction. Co-Authored-By: Claude Opus 5.5 --- .../tests/test_store.py | 11 +++++++++-- .../tests/test_stage12_database_retry.py | 6 ++++-- .../tests/test_stage12_runtime.py | 3 ++- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/libs/policyengine-stage12-persistence/tests/test_store.py b/libs/policyengine-stage12-persistence/tests/test_store.py index 96dad3b0e..156e39d01 100644 --- a/libs/policyengine-stage12-persistence/tests/test_store.py +++ b/libs/policyengine-stage12-persistence/tests/test_store.py @@ -48,7 +48,7 @@ def local_engine(_url, **options): physical_connection = connection.connection.driver_connection assert connection.exec_driver_sql("SELECT 1").scalar_one() == 1 if fail_transaction: - raise RuntimeError("calculation failed") + _fail_the_calculation() except RuntimeError: assert fail_transaction @@ -63,6 +63,12 @@ def local_engine(_url, **options): engine.dispose() +def _fail_the_calculation() -> None: + """Raise the failure a calculation inside the transaction would.""" + msg = "calculation failed" + raise RuntimeError(msg) + + def _report() -> ComparisonReportRecord: return ComparisonReportRecord( evaluation_id=REPORT_ID, @@ -203,7 +209,8 @@ def _sqlalchemy_commit_failure(monkeypatch) -> OperationalError: def commit_then_lose_the_reply(dbapi_connection): dbapi_connection.commit() - raise sqlite3.OperationalError("server closed the connection unexpectedly") + msg = "server closed the connection unexpectedly" + raise sqlite3.OperationalError(msg) monkeypatch.setattr(engine.dialect, "do_commit", commit_then_lose_the_reply) try: diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_database_retry.py b/projects/policyengine-simulation-executor/tests/test_stage12_database_retry.py index ccfe6d40b..4c2cbbf74 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_database_retry.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_database_retry.py @@ -133,7 +133,8 @@ def test_errors_other_than_a_refused_connection_are_not_retried() -> None: def fail_with_invalid_connection_options() -> None: attempts.append(1) - raise ValueError("invalid connection option") + msg = "invalid connection option" + raise ValueError(msg) with pytest.raises(ValueError, match="invalid connection option"): open_connection_with_retry( @@ -173,7 +174,8 @@ def never_connect() -> None: def test_retry_telemetry_failure_does_not_stop_the_retry() -> None: class FailingRuntime: def log(self, *_args, **_kwargs): - raise RuntimeError("telemetry exporter unavailable") + msg = "telemetry exporter unavailable" + raise RuntimeError(msg) clock = ManualClock() opened_connection = OpenedConnection() diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py b/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py index fa7732040..e875ac0e1 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py @@ -1751,7 +1751,8 @@ def spawn(self, *, simulation, **_): raise self.dispatch_error def restore(self, invocation_id): - raise AssertionError(f"unexpected restored invocation {invocation_id}") + msg = f"unexpected restored invocation {invocation_id}" + raise AssertionError(msg) def test_coordinator_records_a_redacted_dispatch_failure_cause() -> None: From c39a522e0674280afe42951d5c2b4b261fc788fd Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 15 Oct 2026 02:23:32 +0400 Subject: [PATCH 17/23] Build the exception messages #795 added before raising #795 landed on main after this stack branched. EM and TRY003, which this PR enables, flag its two new RuntimeError raises (a child that timed out and a spent parent budget) and the cancel failure its test fake raises; each message now moves into a msg variable first, with the text unchanged. BLE001 flags _cancel_call, which now holds the best-effort cancel loop this PR suppressed; it keeps that suppression with its reason inline. Co-Authored-By: Claude Opus 5.5 --- .../src/modal/segmented_national.py | 10 ++++++---- .../tests/test_segmented_national.py | 3 ++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/projects/policyengine-simulation-executor/src/modal/segmented_national.py b/projects/policyengine-simulation-executor/src/modal/segmented_national.py index 5a16eae36..f01a0b577 100644 --- a/projects/policyengine-simulation-executor/src/modal/segmented_national.py +++ b/projects/policyengine-simulation-executor/src/modal/segmented_national.py @@ -443,10 +443,11 @@ def _respawn_overdue_child( }, ) self._cancel_all(children) - raise RuntimeError( + msg = ( "Segmented national child timed out for group " f"{overdue.group_label}: {reason}" - ) from None + ) + raise RuntimeError(msg) from None self._cancel_call(overdue.call) self.superseded_children.append(overdue) @@ -512,17 +513,18 @@ def _fail_if_budget_exhausted( }, ) self._cancel_all(children) - raise RuntimeError( + msg = ( "Segmented national run exceeded its " f"{self.parent_budget_seconds:.0f}s budget with groups still " f"running: {outstanding_groups}" ) + raise RuntimeError(msg) @staticmethod def _cancel_call(call: modal.FunctionCall) -> None: try: call.cancel() - except Exception: + except Exception: # noqa: BLE001 # 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: diff --git a/projects/policyengine-simulation-executor/tests/test_segmented_national.py b/projects/policyengine-simulation-executor/tests/test_segmented_national.py index 473ccbd68..f039ecfbb 100644 --- a/projects/policyengine-simulation-executor/tests/test_segmented_national.py +++ b/projects/policyengine-simulation-executor/tests/test_segmented_national.py @@ -180,7 +180,8 @@ def cancel(self): self.cancel_attempts += 1 if self.cancel_failures > 0: self.cancel_failures -= 1 - raise ConnectionError("cancel RPC failed") + msg = "cancel RPC failed" + raise ConnectionError(msg) self.cancelled = True From d035484efe4167a7f6970141919065e38bca3327 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 15 Oct 2026 02:53:23 +0400 Subject: [PATCH 18/23] Build the exception messages #791's tests raise before raising #791 landed on main after this stack branched. EM and TRY003, which this PR enables, flag ten raises in its new Stage 12 failure-cause and runtime tests that pass a literal message; each message now moves into a msg variable first, with the text unchanged. TRY301 flags the raise inside the try of the _raised helper, which now captures the raised error, with its traceback, through pytest.raises; that also drops the helper's blind except and its BLE001 suppression. Co-Authored-By: Claude Opus 5.5 --- .../tests/test_stage12_failure_causes.py | 17 +++++++++------- .../tests/test_stage12_runtime.py | 20 +++++++++++-------- 2 files changed, 22 insertions(+), 15 deletions(-) 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 2d79d6f52..b6636d9a5 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_failure_causes.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_failure_causes.py @@ -55,17 +55,17 @@ def record_exception(self, error, *, handled, status_code=None) -> None: def _raised(error: Exception) -> Exception: - try: + with pytest.raises(type(error)) as caught: raise error - except Exception as caught: # noqa: BLE001 - return caught + return caught.value def test_summary_names_type_stage_role_and_message() -> None: progress = StageProgress(role="reform") + msg = "Variable 'country' not found in household data" with pytest.raises(ValueError) as caught, progress.stage("stage12_calculation"): - raise ValueError("Variable 'country' not found in household data") + raise ValueError(msg) assert failure_summary(caught.value, progress=progress) == ( "ValueError in stage12_calculation (reform): " @@ -427,7 +427,8 @@ def test_summary_is_bounded_to_the_persisted_column_length() -> None: def test_unprintable_errors_still_produce_a_summary() -> None: class UnprintableError(Exception): def __str__(self) -> str: - raise RuntimeError("cannot render") + msg = "cannot render" + raise RuntimeError(msg) assert failure_summary(UnprintableError()) == "UnprintableError" @@ -450,7 +451,8 @@ def test_recorded_cause_is_redacted_and_keeps_the_original_stack() -> None: def test_recording_failures_never_escape() -> None: class FailingRuntime: def record_exception(self, *_args, **_kwargs) -> None: - raise RuntimeError("exporter unavailable") + msg = "exporter unavailable" + raise RuntimeError(msg) record_failure_cause(FailingRuntime(), ValueError("cause"), "ValueError: cause") record_failure_cause(None, ValueError("cause"), "ValueError: cause") @@ -458,6 +460,7 @@ def record_exception(self, *_args, **_kwargs) -> None: def _progress_in(stage_name: str) -> StageProgress: progress = StageProgress() + msg = "stage failed" with pytest.raises(RuntimeError), progress.stage(stage_name): - raise RuntimeError("stage failed") + raise RuntimeError(msg) return progress diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py b/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py index e875ac0e1..f76dbeda3 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_runtime.py @@ -748,9 +748,8 @@ def test_single_worker_persists_and_records_a_redacted_cause() -> None: runtime = RecordingRuntime() def fail(_simulation): - raise RuntimeError( - "could not reach postgresql://runtime:database-secret@db.example/postgres" - ) + msg = "could not reach postgresql://runtime:database-secret@db.example/postgres" + raise RuntimeError(msg) with pytest.raises(RuntimeError, match="Stage 12 simulation execution failed"): run_single_simulation( @@ -776,7 +775,8 @@ def fail(_simulation): def test_single_worker_names_the_failing_artifact_write_stage() -> None: class FailingSimulationArtifacts(FakeArtifacts): def write_simulation(self, **_kwargs): - raise OSError("bucket write rejected") + msg = "bucket write rejected" + raise OSError(msg) store = FakeStore() simulation = _planned_simulation(SimulationRole.BASELINE) @@ -1557,10 +1557,11 @@ class UnavailableProductionCall: object_id = "production-job-1" def get(self, *, timeout=None): - raise ConnectionError( + msg = ( "production result unavailable at " "https://storage.example.com/result.json?X-Goog-Signature=signed-value" ) + raise ConnectionError(msg) class UnavailableProductionInvoker(ConcurrentInvoker): @@ -1812,7 +1813,8 @@ def get(self, *, timeout=None): "completed_at": NOW, } ) - raise RuntimeError("Stage 12 simulation execution failed") + msg = "Stage 12 simulation execution failed" + raise RuntimeError(msg) class UnreachedCall: @@ -1820,7 +1822,8 @@ def __init__(self, object_id): self.object_id = object_id def get(self, *, timeout=None): - raise AssertionError("the coordinator waits for baseline first") + msg = "the coordinator waits for baseline first" + raise AssertionError(msg) class WorkerRecordedFailureInvoker: @@ -1840,7 +1843,8 @@ def spawn(self, *, simulation, **_): return UnreachedCall("call-reform") def restore(self, invocation_id): - raise AssertionError(f"unexpected restored invocation {invocation_id}") + msg = f"unexpected restored invocation {invocation_id}" + raise AssertionError(msg) def test_coordinator_keeps_the_cause_recorded_by_a_failed_worker() -> None: From 311a10e18dcdfefc1dcf6682565a8819a10345b1 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 15 Oct 2026 03:00:40 +0400 Subject: [PATCH 19/23] Write the BLE001 suppressions #791 added with an inline reason #791 landed on main after this stack branched. Its two new blind-except suppressions in the Stage 12 failure-cause module gave their reasons in the "- reason" form; like every other BLE001 suppression in this PR, they now use the " # reason" form, with the reasons unchanged. Co-Authored-By: Claude Opus 5.5 --- .../stage12_runtime/failure_causes.py | 4 ++-- 1 file 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 35e3cea1d..9f7baa350 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 @@ -227,7 +227,7 @@ def failure_summary( heading = f"{error_type}{_location(progress)}" message = redact_failure_message(_failure_message(error)) return _bounded(f"{heading}: {message}" if message else heading) - except Exception: # noqa: BLE001 - a summary must never mask the failure + except Exception: # noqa: BLE001 # a summary must never mask the failure return _bounded(error_type) @@ -242,7 +242,7 @@ def record_failure_cause( try: cause = Stage12FailureCause(summary).with_traceback(error.__traceback__) runtime.record_exception(cause, handled=True) - except Exception: # noqa: BLE001 - telemetry must not change the outcome + except Exception: # noqa: BLE001 # telemetry must not change the outcome logger.warning("stage12_failure_cause_not_recorded") From 9d14029901e3ac116343506667ad77d689aeecf7 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 00:59:03 +0400 Subject: [PATCH 20/23] Build the exception messages #758 added before raising #758 landed on main after this stack branched. EM and TRY003, which this PR enables, flag four raises with a literal message: the deployed permission_enforcement fixture's RuntimeError and three stand-ins in its permission tests. Each message now moves into a msg variable first, with the text unchanged. Co-Authored-By: Claude Opus 5.5 --- .../authenticated_tests/test_deployed_auth.py | 5 ++--- .../policyengine-simulation-entry/tests/test_auth.py | 9 ++++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/projects/policyengine-simulation-entry/authenticated_tests/test_deployed_auth.py b/projects/policyengine-simulation-entry/authenticated_tests/test_deployed_auth.py index 9f22b8c37..0551c1ed2 100644 --- a/projects/policyengine-simulation-entry/authenticated_tests/test_deployed_auth.py +++ b/projects/policyengine-simulation-entry/authenticated_tests/test_deployed_auth.py @@ -44,9 +44,8 @@ def permission_enforcement() -> None: or "0" ) if value not in {"0", "1"}: - raise RuntimeError( - "SIMULATION_ENTRYPOINT_TEST_ENFORCE_PERMISSIONS must be 0 or 1." - ) + msg = "SIMULATION_ENTRYPOINT_TEST_ENFORCE_PERMISSIONS must be 0 or 1." + raise RuntimeError(msg) if value == "0": pytest.skip("The target environment logs missing permissions only.") diff --git a/projects/policyengine-simulation-entry/tests/test_auth.py b/projects/policyengine-simulation-entry/tests/test_auth.py index b3e4549db..89358a854 100644 --- a/projects/policyengine-simulation-entry/tests/test_auth.py +++ b/projects/policyengine-simulation-entry/tests/test_auth.py @@ -532,7 +532,8 @@ def test_permission_telemetry_failure_does_not_change_response( def fail_permission_event(name, **kwargs): if name == "simulation_entry_permission_missing": - raise RuntimeError("telemetry unavailable") + msg = "telemetry unavailable" + raise RuntimeError(msg) return emit(name, **kwargs) failing_event = Mock(side_effect=fail_permission_event) @@ -563,7 +564,8 @@ def test_disabled_authentication_allows_enforced_routes( ): class Decoder: def __call__(self, token, runtime): - raise AssertionError("disabled authentication decoded a token") + msg = "disabled authentication decoded a token" + raise AssertionError(msg) monkeypatch.setattr(auth_module, "_decoder", lambda issuer, audience: Decoder()) app = create_app( @@ -587,7 +589,8 @@ def __call__(self, token, runtime): def test_public_routes_stay_public_when_permissions_are_enforced(monkeypatch): class Decoder: def __call__(self, token, runtime): - raise AssertionError("public route attempted authentication") + msg = "public route attempted authentication" + raise AssertionError(msg) monkeypatch.setattr(auth_module, "_decoder", lambda issuer, audience: Decoder()) app = create_app( From 3064edde0a42c8bedb5990d1b23282313e397e9a Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 00:59:03 +0400 Subject: [PATCH 21/23] Write the BLE001 suppression #758 added with an inline reason #758 landed on main after this stack branched. The blind except around its simulation_entry_permission_missing event gave its reason in the "- reason" form; like every other BLE001 suppression in this PR, it now uses the " # reason" form, with the reason unchanged. Co-Authored-By: Claude Opus 5.5 --- .../src/policyengine_simulation_entry/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/auth.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/auth.py index 261492acb..fa6d9c66f 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/auth.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/auth.py @@ -215,5 +215,5 @@ def _record_missing_permission( severity="WARNING", attributes=attributes, ) - except Exception: # noqa: BLE001, S110 - telemetry is non-fatal + except Exception: # noqa: BLE001, S110 # telemetry is non-fatal pass From 1a1495d23563ec2b85a584b1f60610daf0cda2fd Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 00:59:03 +0400 Subject: [PATCH 22/23] Build the exception messages #781 added before raising #781 landed on main after this stack branched. EM and TRY003, which this PR enables, flag nine raises with a literal message: the contract's segment-count validator and eight stand-ins in its executor and gateway job progress tests. Each message now moves into a msg variable first, with the text unchanged. Co-Authored-By: Claude Opus 5.5 --- .../job_progress.py | 3 ++- .../tests/test_job_progress.py | 21 ++++++++++++------- .../tests/test_job_progress.py | 3 ++- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/job_progress.py b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/job_progress.py index c23e8e0a2..ca823cb09 100644 --- a/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/job_progress.py +++ b/libs/policyengine-simulation-contract/src/policyengine_simulation_contract/job_progress.py @@ -71,7 +71,8 @@ class JobSegmentProgress(BaseModel): @model_validator(mode="after") def _completed_within_total(self) -> JobSegmentProgress: if self.completed > self.total: - raise ValueError("completed segments cannot exceed total segments") + msg = "completed segments cannot exceed total segments" + raise ValueError(msg) return self diff --git a/projects/policyengine-simulation-executor/tests/test_job_progress.py b/projects/policyengine-simulation-executor/tests/test_job_progress.py index 47140a503..a5e68fc97 100644 --- a/projects/policyengine-simulation-executor/tests/test_job_progress.py +++ b/projects/policyengine-simulation-executor/tests/test_job_progress.py @@ -236,7 +236,8 @@ def test__write_failures_are_logged_once_and_never_raised(self, caplog): def failing_write(job_id, record): calls.append(record.stage) - raise ConnectionError("modal unavailable") + msg = "modal unavailable" + raise ConnectionError(msg) reporter = StoredJobProgressReporter("fc-job-1", write=failing_write) @@ -453,7 +454,8 @@ def test__segmented_restart_is_held_until_simulating(self): def test__unreadable_earlier_record_starts_fresh(self, caplog): def failing_read(job_id): - raise ConnectionError("modal unavailable") + msg = "modal unavailable" + raise ConnectionError(msg) writer = _RecordingWriter() reporter = StoredJobProgressReporter( @@ -547,7 +549,8 @@ def test__failed_analysis_does_not_enter_outputs(self): progress = _RecordingReporter() def analysis(baseline, reform, include_cliff_impacts): - raise RuntimeError("model failed") + msg = "model failed" + raise RuntimeError(msg) builder = self._builder(analysis, progress) @@ -850,7 +853,8 @@ def dispatch(params, *, app_name, runtime, progress): def test__failed_job_still_records_where_it_stopped(self, modal_app, monkeypatch): def dispatch(params, *, app_name, runtime, progress): progress.enter(JobStage.BASELINE) - raise RuntimeError("model failed") + msg = "model failed" + raise RuntimeError(msg) events, writes = self._wire(modal_app, monkeypatch, dispatch) @@ -887,7 +891,8 @@ def test__preemption_before_the_calculation_stays_running( self, modal_app, monkeypatch ): def dispatch(params, *, app_name, runtime, progress): - raise AssertionError("dispatch should not run") + msg = "dispatch should not run" + raise AssertionError(msg) _, writes = self._wire( modal_app, monkeypatch, dispatch, earlier=_earlier_attempt() @@ -915,7 +920,8 @@ def dispatch(params, *, app_name, runtime, progress): self._wire(modal_app, monkeypatch, dispatch) def failing_write(job_id, record): - raise ConnectionError("modal unavailable") + msg = "modal unavailable" + raise ConnectionError(msg) monkeypatch.setattr(job_progress_state, "put_job_progress", failing_write) @@ -923,7 +929,8 @@ def failing_write(job_id, record): def test__no_call_id_means_no_reporting(self, modal_app, monkeypatch): def missing_call_id(): - raise RuntimeError("not inside a Modal function call") + msg = "not inside a Modal function call" + raise RuntimeError(msg) monkeypatch.setattr( modal_app.modal, "current_function_call_id", missing_call_id, raising=False diff --git a/projects/policyengine-simulation-gateway/tests/test_job_progress.py b/projects/policyengine-simulation-gateway/tests/test_job_progress.py index 8cc4586a0..d2954e50b 100644 --- a/projects/policyengine-simulation-gateway/tests/test_job_progress.py +++ b/projects/policyengine-simulation-gateway/tests/test_job_progress.py @@ -229,7 +229,8 @@ def test__unreadable_store_drops_stage_fields( stage_reporting_worker["func"].last_call.running = True def failing_read(job_id): - raise ConnectionError("modal unavailable") + msg = "modal unavailable" + raise ConnectionError(msg) monkeypatch.setattr(endpoints, "get_job_progress", failing_read) From c396a2545747cc715f3cb7ccb25e0f8c7beac17b Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 00:59:03 +0400 Subject: [PATCH 23/23] Write the BLE001 suppressions #781 added with an inline reason #781 landed on main after this stack branched. Its five new blind-except suppressions, in the Modal app's reporter and stage-duration helpers, the stored reporter's read and write, and the gateway's stage-field read, gave their reasons in the "- reason" form; like every other BLE001 suppression in this PR, they now use the " # reason" form, with the reasons unchanged. Co-Authored-By: Claude Opus 5.5 --- projects/policyengine-simulation-executor/src/modal/app.py | 4 ++-- .../src/policyengine_simulation_executor/job_progress.py | 4 ++-- .../src/policyengine_simulation_gateway/endpoints.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/projects/policyengine-simulation-executor/src/modal/app.py b/projects/policyengine-simulation-executor/src/modal/app.py index 6d2ff4426..bb15a0318 100644 --- a/projects/policyengine-simulation-executor/src/modal/app.py +++ b/projects/policyengine-simulation-executor/src/modal/app.py @@ -285,7 +285,7 @@ def _job_progress_reporter() -> "JobProgressReporter": try: job_id = modal.current_function_call_id() - except Exception: # noqa: BLE001 - stage reporting is non-fatal + except Exception: # noqa: BLE001 # stage reporting is non-fatal job_id = None if not job_id: return NULL_JOB_PROGRESS @@ -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 # telemetry is non-fatal pass diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/job_progress.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/job_progress.py index 9ac32475e..81f5c25f7 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/job_progress.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/job_progress.py @@ -280,7 +280,7 @@ def _earlier_attempt(self) -> JobProgressRecord | None: return None try: return self._read(self.job_id) - except Exception as exc: # noqa: BLE001 - stage reporting is non-fatal + except Exception as exc: # noqa: BLE001 # stage reporting is non-fatal if not self._read_failed: logger.warning( "Could not read job progress for %s: %s", @@ -317,7 +317,7 @@ def _publish(self) -> None: return try: self._write(self.job_id, self._published(self._record)) - except Exception as exc: # noqa: BLE001 - stage reporting is non-fatal + except Exception as exc: # noqa: BLE001 # stage reporting is non-fatal if not self._write_failed: logger.warning( "Could not record job progress for %s: %s", diff --git a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/endpoints.py b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/endpoints.py index 194b1af69..7919c0dda 100644 --- a/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/endpoints.py +++ b/projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/endpoints.py @@ -782,7 +782,7 @@ def _job_progress_payload( try: with runtime.span(ANNUAL_IMPACT_STAGES.name(Stage.MODAL_JOB_PROGRESS_READ)): record = get_job_progress(job_id) - except Exception as exc: # noqa: BLE001 - stage reporting is optional + except Exception as exc: # noqa: BLE001 # stage reporting is optional logger.warning( "Job progress unavailable for %s: %s", job_id, type(exc).__name__ )