diff --git a/changelog.d/uk-breakdown-withheld-codes.added.md b/changelog.d/uk-breakdown-withheld-codes.added.md new file mode 100644 index 000000000..6e75643fe --- /dev/null +++ b/changelog.d/uk-breakdown-withheld-codes.added.md @@ -0,0 +1 @@ +The OpenAPI description of `GET /{country_id}/economy/{policy_id}/over/{baseline_policy_id}` now declares `constituency_withheld_codes` and `local_authority_withheld_codes` in a `breakdown=local_areas` result: the sorted ONS codes of each level's areas that the run estimated but disclosure control withheld, `[]` when none were, and null or absent when the result doesn't report them. The Public API returns them unchanged from the simulation API. diff --git a/changelog.d/uk-breakdown-withheld-codes.fixed.md b/changelog.d/uk-breakdown-withheld-codes.fixed.md new file mode 100644 index 000000000..50d33fa24 --- /dev/null +++ b/changelog.d/uk-breakdown-withheld-codes.fixed.md @@ -0,0 +1 @@ +UK `breakdown=local_areas` results cached before the simulation API listed each level's withheld area codes (`constituency_withheld_codes` and `local_authority_withheld_codes`) are no longer served: a breakdown request's options hash now includes `breakdown_schema=2`, so the next request computes the breakdown again. Hashes and cached results of requests without a breakdown are unchanged. diff --git a/docs/canonical-spm.md b/docs/canonical-spm.md index 30608e013..5fb6387de 100644 --- a/docs/canonical-spm.md +++ b/docs/canonical-spm.md @@ -335,7 +335,19 @@ accepted only for the UK, with `region` exactly `uk` or one UK country requests, including other spellings of those regions, return HTTP 400 before anything is read from the cache or submitted. The API sends `"breakdown": "local_areas"` to the simulation API, which computes the impacts in a run separate from the national result, and -returns its result (`kind: "local_areas"`) like any other economy result. If +returns its result (`kind: "local_areas"`) like any other economy result. +Besides `constituency_impact` and `local_authority_impact`, the result has +`constituency_withheld_codes` and `local_authority_withheld_codes`: the ONS +codes, sorted, of each level's areas that the run estimated but disclosure +control withheld, which the impact lists leave out. `[]` means none were +withheld. `null` or an absent field means the result doesn't report them, as +in results computed before the simulation API reported them. An area the run +never estimated, such as one with no households in the dataset, is not listed. +The API stores and returns these fields unchanged. A breakdown's options hash +also includes `breakdown_schema=2`, the version of the breakdown result, so a +breakdown cached before the result reported withheld codes is not returned and +the next request computes it again; hashes of requests without `breakdown` +don't include it. If the simulation API refuses the request with HTTP 400 or 422, because the deployed simulation API or the .py release it routes to cannot run breakdowns, the route returns HTTP 502 with the simulation API's reason and stores diff --git a/policyengine_api/openapi_spec.yaml b/policyengine_api/openapi_spec.yaml index 31940fd8d..6da4a1d7e 100644 --- a/policyengine_api/openapi_spec.yaml +++ b/policyengine_api/openapi_spec.yaml @@ -884,7 +884,7 @@ paths: # Query parameters are generated from policyengine_api.query_parameters. responses: 200: - description: Annual economic impact, progress, or persisted error state. Canonical completed results include actual baseline/reform receipts containing the selected SPM configuration. While computing, the optional stage fields come from the simulation entrypoint and appear only when its worker reports stages. With breakdown=local_areas, a completed result is the simulation API's local-area breakdown instead, with kind local_areas, the constituency_impact and local_authority_impact lists, and the dataset and version fields. + description: Annual economic impact, progress, or persisted error state. Canonical completed results include actual baseline/reform receipts containing the selected SPM configuration. While computing, the optional stage fields come from the simulation entrypoint and appear only when its worker reports stages. With breakdown=local_areas, a completed result is the simulation API's local-area breakdown instead, with kind local_areas, the constituency_impact and local_authority_impact lists, constituency_withheld_codes and local_authority_withheld_codes, and the dataset and version fields. A breakdown result cached before the simulation API reported withheld codes is not returned; the next request computes the breakdown again. content: application/json: schema: @@ -902,6 +902,18 @@ paths: properties: spm_provenance: $ref: '#/components/schemas/SPMComparisonProvenance' + constituency_withheld_codes: + type: array + nullable: true + items: + type: string + description: breakdown=local_areas only. The ONS codes, sorted, of the constituencies the run estimated but disclosure control withheld, which constituency_impact leaves out. An empty list means none were withheld. Null or an absent field means the result doesn't report them, as in results computed before the simulation API reported them, so a constituency missing from constituency_impact may or may not have been withheld. An area the run never estimated, such as one with no households in the dataset, is not listed. + local_authority_withheld_codes: + type: array + nullable: true + items: + type: string + description: breakdown=local_areas only. The ONS codes, sorted, of the local authorities the run estimated but disclosure control withheld, which local_authority_impact leaves out. An empty list means none were withheld. Null or an absent field means the result doesn't report them, as in results computed before the simulation API reported them, so a local authority missing from local_authority_impact may or may not have been withheld. An area the run never estimated, such as one with no households in the dataset, is not listed. stage: type: string description: Current stage of a computing run, such as queued, loading_data, baseline, reform, simulating or outputs. Later versions may add stages; clients must accept a value they do not know. diff --git a/policyengine_api/services/economy_impact_types.py b/policyengine_api/services/economy_impact_types.py index 5ddcc48db..ad8be874f 100644 --- a/policyengine_api/services/economy_impact_types.py +++ b/policyengine_api/services/economy_impact_types.py @@ -53,6 +53,17 @@ class ImpactStatus(Enum): # telemetry envelope forbids extra fields, so the run is marked here rather # than by a new field; geography_type and geography_code still name its region. LOCAL_AREA_BREAKDOWN_SIMULATION_KIND = "local_area_breakdown" +# The version of the local-area breakdown result that a breakdown request's +# options hash names, as breakdown_schema. A breakdown result cached under an +# earlier version, or before the hash named one, is not served, and the next +# request computes it again. Requests without a breakdown have no such field, +# so their hashes and cache entries don't change. Raise it when the simulation +# API's local-area result gains or changes fields, since a simulation API +# deploy that keeps the same .py version doesn't change any other part of the +# cache identity. +# 2: each level lists the ONS codes of the areas the run estimated but +# disclosure control withheld (PolicyEngine/policyengine-sim-api#968). +LOCAL_AREA_BREAKDOWN_RESULT_SCHEMA_VERSION = 2 BUDGET_WINDOW_CLAIM_ATTEMPTS = 3 REFORM_IMPACT_START_CLAIM_ATTEMPTS = 3 # A budget-window progress message names this many computing years and counts diff --git a/policyengine_api/services/economy_stored_impacts.py b/policyengine_api/services/economy_stored_impacts.py index 0ec81fb75..5473d7629 100644 --- a/policyengine_api/services/economy_stored_impacts.py +++ b/policyengine_api/services/economy_stored_impacts.py @@ -13,6 +13,7 @@ ReformImpactStartClaim, ) from policyengine_api.services.economy_impact_types import ( + LOCAL_AREA_BREAKDOWN_RESULT_SCHEMA_VERSION, EconomicImpactSetupOptions, ImpactAction, ImpactStatus, @@ -234,6 +235,10 @@ def _build_options_hash( bundle_parts.append(f"policyengine_version={bundle.policyengine_version}") if options.get("spm") is not None: bundle_parts.append("spm_provenance_schema=canonical-spm-comparison-v2") + if options.get("breakdown") is not None: + bundle_parts.append( + f"breakdown_schema={LOCAL_AREA_BREAKDOWN_RESULT_SCHEMA_VERSION}" + ) if bundle.runtime_app_name: bundle_parts.append(f"runtime_app_name={bundle.runtime_app_name}") return "[" + "&".join([option_pairs, *bundle_parts]).strip("&") + "]" diff --git a/tests/contract/test_simulation_gateway_contract.py b/tests/contract/test_simulation_gateway_contract.py index 217037170..8126e5902 100644 --- a/tests/contract/test_simulation_gateway_contract.py +++ b/tests/contract/test_simulation_gateway_contract.py @@ -16,6 +16,7 @@ MOCK_BATCH_SUBMIT_RESPONSE_SUCCESS, MOCK_HEALTH_RESPONSE, MOCK_LOCAL_AREA_BREAKDOWN_RESULT, + MOCK_LOCAL_AREA_BREAKDOWN_RESULT_WITH_WITHHELD_CODES, MOCK_MODAL_JOB_ID, MOCK_POLL_RESPONSE_COMPLETE, MOCK_RESOLVED_APP_NAME, @@ -203,9 +204,14 @@ def _uk_submission(options: dict) -> dict: "local_areas", MOCK_LOCAL_AREA_BREAKDOWN_RESULT, ), + ( + {"breakdown": "local_areas"}, + "local_areas", + MOCK_LOCAL_AREA_BREAKDOWN_RESULT_WITH_WITHHELD_CODES, + ), ({}, None, MOCK_POLL_RESPONSE_COMPLETE["result"]), ], - ids=["local-area-breakdown", "national"], + ids=["local-area-breakdown", "local-area-breakdown-withheld-codes", "national"], ) def test_gateway_uk_comparison_sends_breakdown_only_when_requested( monkeypatch, options, expected_breakdown, result diff --git a/tests/fixtures/libs/simulation_entrypoint.py b/tests/fixtures/libs/simulation_entrypoint.py index 11a6c4d50..fa1d8cf21 100644 --- a/tests/fixtures/libs/simulation_entrypoint.py +++ b/tests/fixtures/libs/simulation_entrypoint.py @@ -83,6 +83,20 @@ } ], } +# The same result from a simulation API that lists, for each level, the ONS +# codes of the areas the run estimated but disclosure control withheld, sorted +# (PolicyEngine/policyengine-sim-api#968). An empty list means none were. +MOCK_LOCAL_AREA_BREAKDOWN_RESULT_WITH_WITHHELD_CODES = { + **MOCK_LOCAL_AREA_BREAKDOWN_RESULT, + "constituency_withheld_codes": ["E14001172", "S14000021"], + "local_authority_withheld_codes": [], +} +# A result that doesn't report its withheld areas, with null for each level. +MOCK_LOCAL_AREA_BREAKDOWN_RESULT_WITH_NULL_WITHHELD_CODES = { + **MOCK_LOCAL_AREA_BREAKDOWN_RESULT, + "constituency_withheld_codes": None, + "local_authority_withheld_codes": None, +} MOCK_POLICYENGINE_BUNDLE = { "model_version": "1.459.0", "policyengine_version": "3.4.0", diff --git a/tests/unit/routes/test_economy_spm_query.py b/tests/unit/routes/test_economy_spm_query.py index c871237d7..3df5cd765 100644 --- a/tests/unit/routes/test_economy_spm_query.py +++ b/tests/unit/routes/test_economy_spm_query.py @@ -17,7 +17,11 @@ from policyengine_api.services.budget_window_cache import BudgetWindowCache from policyengine_api.services.economy_service import EconomyService from policyengine_api.services.reform_impacts_service import ReformImpactsService -from tests.fixtures.libs.simulation_entrypoint import MOCK_LOCAL_AREA_BREAKDOWN_RESULT +from tests.fixtures.libs.simulation_entrypoint import ( + MOCK_LOCAL_AREA_BREAKDOWN_RESULT, + MOCK_LOCAL_AREA_BREAKDOWN_RESULT_WITH_NULL_WITHHELD_CODES, + MOCK_LOCAL_AREA_BREAKDOWN_RESULT_WITH_WITHHELD_CODES, +) @pytest.fixture(params=[False, True], ids=["annual", "budget-window"]) @@ -542,20 +546,32 @@ def test_annual_route_returns_502_when_the_simulation_api_refuses_a_breakdown( assert gateway.run.call_args.args[0]["breakdown"] == "local_areas" +@pytest.mark.parametrize( + "simulation_result", + [ + MOCK_LOCAL_AREA_BREAKDOWN_RESULT_WITH_WITHHELD_CODES, + MOCK_LOCAL_AREA_BREAKDOWN_RESULT_WITH_NULL_WITHHELD_CODES, + MOCK_LOCAL_AREA_BREAKDOWN_RESULT, + ], + ids=["withheld-codes", "null-withheld-codes", "no-withheld-codes"], +) def test_annual_route_stores_and_returns_a_completed_breakdown_unchanged( - economy_service_http, + economy_service_http, simulation_result ): """A finished breakdown job is polled once, stored, and then read back. The second request polls the job (``_handle_execution_state``); the third reads the stored record (``_handle_completed_impact``) without polling. + The withheld area codes come back as the simulation API sent them, and a + result without them, from a simulation API that doesn't report them, + comes back without them. """ client, gateway = economy_service_http breakdown_query = [*UK_ANNUAL_QUERY, ("breakdown", "local_areas")] gateway.get_execution_by_id.return_value = ModalSimulationExecution( job_id="submitted-job", status="complete", - result=MOCK_LOCAL_AREA_BREAKDOWN_RESULT, + result=simulation_result, resolved_app_name="policyengine-simulation-test", ) gateway.get_execution_status.side_effect = lambda execution: execution.status @@ -571,13 +587,11 @@ def test_annual_route_stores_and_returns_a_completed_breakdown_unchanged( assert response.status_code == 200, response.get_json() body = response.get_json() assert body["status"] == "ok" - result = body["result"] + returned = body["result"] # The breakdown's own fields come back unchanged; the service adds # only the bundle identity it adds to every economy result. - assert { - key: result[key] for key in MOCK_LOCAL_AREA_BREAKDOWN_RESULT - } == MOCK_LOCAL_AREA_BREAKDOWN_RESULT - assert set(result) - set(MOCK_LOCAL_AREA_BREAKDOWN_RESULT) == { + assert {key: returned[key] for key in simulation_result} == simulation_result + assert set(returned) - set(simulation_result) == { "policyengine_bundle", "resolved_app_name", } diff --git a/tests/unit/services/test_economy_service_request_plumbing.py b/tests/unit/services/test_economy_service_request_plumbing.py index 0f95d8093..44b593050 100644 --- a/tests/unit/services/test_economy_service_request_plumbing.py +++ b/tests/unit/services/test_economy_service_request_plumbing.py @@ -7,7 +7,7 @@ import json from dataclasses import replace -from datetime import timedelta +from datetime import datetime, timedelta from unittest.mock import patch import pytest @@ -32,7 +32,10 @@ ImpactStatus, RuntimeBundleIdentity, ) -from policyengine_api.services.reform_impacts_service import ReformImpactsService +from policyengine_api.services.reform_impacts_service import ( + ReformImpactExecution, + ReformImpactsService, +) from tests.fixtures.libs.simulation_entrypoint import MOCK_LOCAL_AREA_BREAKDOWN_RESULT from tests.fixtures.services.economy_service import ( MOCK_API_VERSION, @@ -412,7 +415,16 @@ def test_budget_window_poll_reports_running_years( f"&policyengine_version={MOCK_POLICYENGINE_VERSION}" ) UK_NATIONAL_LOOKUP_HASH = f"[{UK_BUNDLE_HASH_FIELDS}]" -UK_BREAKDOWN_LOOKUP_HASH = f"[breakdown=local_areas&{UK_BUNDLE_HASH_FIELDS}]" +# A breakdown's hash names the version of the breakdown result it holds, so +# breakdowns cached before the result listed its withheld area codes are not +# served. Hashes without a breakdown have no such field and keep their values. +UK_BREAKDOWN_LOOKUP_HASH = ( + f"[breakdown=local_areas&{UK_BUNDLE_HASH_FIELDS}&breakdown_schema=2]" +) +# The hash a breakdown had before that version was added. +UK_BREAKDOWN_LOOKUP_HASH_BEFORE_WITHHELD_CODES = ( + f"[breakdown=local_areas&{UK_BUNDLE_HASH_FIELDS}]" +) def _stored_options_hash(lookup_hash: str) -> str: @@ -561,6 +573,139 @@ def test_national_and_breakdown_hashes_never_match_each_others_lookup( assert not _like_matches(_stored_options_hash(other_hash), pattern) +# The hashes a request without a breakdown stored before breakdowns had a +# result version, written out so that any change to them shows here. +UK_STORED_HASH_WITHOUT_BREAKDOWN = ( + "[dataset=default&model_version=1.2.3&policyengine_version=3.4.0" + "&runtime_app_name=policyengine-simulation-us1-2-3-uk2-7-8]" +) + + +@pytest.mark.usefixtures("uk_package_versions") +@pytest.mark.parametrize( + ("country_id", "region", "options", "stored_hash"), + [ + ("uk", "uk", {}, UK_STORED_HASH_WITHOUT_BREAKDOWN), + ("uk", "country/scotland", {}, UK_STORED_HASH_WITHOUT_BREAKDOWN), + ("uk", "country/ni", {}, UK_STORED_HASH_WITHOUT_BREAKDOWN), + ("us", "us", MOCK_OPTIONS, MOCK_OPTIONS_HASH), + ("us", "ca", MOCK_OPTIONS, MOCK_OPTIONS_HASH), + ], + ids=[ + "uk-national", + "uk-nation-scotland", + "uk-nation-ni", + "us-national", + "us-state", + ], +) +def test_requests_without_a_breakdown_keep_their_stored_hashes( + economy_service, country_id, region, options, stored_hash +): + economy_service.get_economic_impact( + EconomicImpactRequest( + country_id=country_id, + policy_id=MOCK_POLICY_ID, + baseline_policy_id=MOCK_BASELINE_POLICY_ID, + region=region, + dataset="default", + options=options, + api_version=MOCK_API_VERSION, + ), + time_period="2025", + ) + + stored = economy_service._reform_impacts.get_recent_reform_impacts(5) + assert [impact.options_hash for impact in stored] == [stored_hash] + + +def _store_completed_uk_impact( + service: EconomyService, + *, + options: dict, + options_hash: str, + job_id: str, + result: dict, +) -> None: + """Store a finished impact for the UK in 2026 under this options hash.""" + service._reform_impacts.set_reform_impact( + identity=ReformImpactIdentity( + scope=ReformImpactScope( + country_id="uk", + reform_policy_id=MOCK_POLICY_ID, + baseline_policy_id=MOCK_BASELINE_POLICY_ID, + region="uk", + dataset="default", + time_period="2026", + ), + api_version=CACHE_VERSION, + options_hash=options_hash, + ), + execution=ReformImpactExecution( + execution_id=job_id, + start_time=datetime(2026, 10, 1, 12, 0), + observability_id=MOCK_OBSERVABILITY_ID, + ), + options=options, + status="computing", + reform_impact_json={}, + ) + service._reform_impacts.set_complete_reform_impact( + execution_id=job_id, + reform_impact_json={**result, "resolved_app_name": MOCK_RESOLVED_APP_NAME}, + ) + + +UK_NATIONAL_RESULT = {"budget": {"budgetary_impact": 1.0}} + + +@pytest.mark.usefixtures("uk_package_versions") +def test_a_breakdown_cached_before_the_withheld_codes_is_recomputed_but_not_national( + economy_service, mock_simulation_entrypoint +): + _store_completed_uk_impact( + economy_service, + options={"breakdown": "local_areas"}, + options_hash=_stored_options_hash( + UK_BREAKDOWN_LOOKUP_HASH_BEFORE_WITHHELD_CODES + ), + job_id="breakdown-job-before-withheld-codes", + result=MOCK_LOCAL_AREA_BREAKDOWN_RESULT, + ) + _store_completed_uk_impact( + economy_service, + options={}, + options_hash=_stored_options_hash(UK_NATIONAL_LOOKUP_HASH), + job_id="national-job", + result=UK_NATIONAL_RESULT, + ) + + national = economy_service.get_economic_impact(_uk_request({}), time_period="2026") + breakdown = economy_service.get_economic_impact( + _uk_request({"breakdown": "local_areas"}), time_period="2026" + ) + + # The national result is still read from its cache entry. + assert national.status == ImpactStatus.OK + assert national.data is not None + assert national.data["budget"] == UK_NATIONAL_RESULT["budget"] + # The breakdown stored without the withheld codes is not: a new job runs. + assert breakdown == EconomicImpactResult.computing() + submitted = [ + call.args[0].get("breakdown") + for call in mock_simulation_entrypoint.run.call_args_list + ] + assert submitted == ["local_areas"] + stored = economy_service._reform_impacts.get_recent_reform_impacts(5) + assert {impact.execution_id: impact.options_hash for impact in stored} == { + "breakdown-job-before-withheld-codes": _stored_options_hash( + UK_BREAKDOWN_LOOKUP_HASH_BEFORE_WITHHELD_CODES + ), + "national-job": _stored_options_hash(UK_NATIONAL_LOOKUP_HASH), + MOCK_MODAL_JOB_ID: _stored_options_hash(UK_BREAKDOWN_LOOKUP_HASH), + } + + @pytest.mark.usefixtures("uk_package_versions") def test_a_breakdown_request_never_reuses_the_national_job( economy_service, mock_simulation_entrypoint @@ -878,6 +1023,20 @@ def test_options_hash_marks_spm_provenance_schema(): ) +def test_options_hash_marks_the_breakdown_result_schema(): + options_hash = EconomyService()._build_options_hash( + options={"breakdown": "local_areas"}, + bundle=RuntimeBundleIdentity( + dataset="default", model_version="1", runtime_app_name="app" + ), + ) + + assert options_hash == ( + "[breakdown=local_areas&dataset=default&model_version=1" + "&breakdown_schema=2&runtime_app_name=app]" + ) + + def test_empty_options_hash_drops_the_leading_separator(): options_hash = EconomyService()._build_options_hash( options={}, bundle=RuntimeBundleIdentity(dataset="default", model_version="1") diff --git a/tests/unit/test_openapi_spm.py b/tests/unit/test_openapi_spm.py index 6f5b8942a..600557422 100644 --- a/tests/unit/test_openapi_spm.py +++ b/tests/unit/test_openapi_spm.py @@ -90,6 +90,27 @@ def test_economy_query_and_receipt_schemas_match_public_http_contract(): } +def test_annual_economy_result_declares_the_breakdown_withheld_area_codes(): + """A local-area breakdown result lists each level's withheld ONS codes.""" + operation = load_specification()["paths"][ + "/{country_id}/economy/{policy_id}/over/{baseline_policy_id}" + ]["get"] + completed = operation["responses"]["200"] + result = completed["content"]["application/json"]["schema"]["properties"]["result"][ + "properties" + ] + for field_name in ( + "constituency_withheld_codes", + "local_authority_withheld_codes", + ): + assert field_name in completed["description"] + field = result[field_name] + assert field["type"] == "array" + assert field["items"] == {"type": "string"} + assert field["nullable"] is True + assert field["description"].startswith("breakdown=local_areas only.") + + def test_metadata_discovery_and_selection_constraints_are_published(): spec = load_specification() metadata = spec["paths"]["/{country_id}/metadata"]["get"]["responses"]["200"][