Stop sending x and y on UK area results

changelog.d/918.changed.md

@@ -0,0 +1,3 @@
+The executor no longer sends `x` or `y` on UK constituency and local-authority records. It no longer reads the `x` and `y` columns of policyengine.py's constituency lookup or attaches its packaged local-authority positions, and it no longer refuses a published area without a map position. Results it returns from now on carry neither key, which the contract already allows (#984). The beta integration check of a local-area breakdown now requires records without them.
+
+The executor no longer ships `coordinates_lad22.csv` and `coordinates_lad23.csv`. Each boundary version's local-authority codes now come from `codes_lad22.csv` and `codes_lad23.csv`, which list exactly the codes the coordinate files did, so detecting LAD22 or LAD23 codes, and its checks, are unchanged. The Stage 12 static runtime file smoke test reports `lad22_codes` and `lad23_codes` instead of `lad22_coordinates` and `lad23_coordinates`.

docs/migration/stage-12-modal-worker-foundation.md

@@ -255,31 +255,39 @@ configuration, an unknown code, or a dataset without a distinguishing code
 fails the Stage 12 run. Dataset names and release labels do not select a
 boundary version, and there is no fallback to the newest known configuration.
 
-The executor temporarily packages three display-only resources under its
-expandable `static_runtime_files/uk_local_authorities/` directory. The v2 Modal
-image copies that directory explicitly because Modal's Python-source layer does
-not include non-Python files by default:
-
-- one common `code,name` file containing the union of supported authority
-  codes;
-- one `code,x,y` file for the 374-authority LAD22 configuration; and
-- one `code,x,y` file for the 361-authority LAD23 configuration.
+The executor temporarily packages three resources under its expandable
+`static_runtime_files/uk_local_authorities/` directory. The v2 Modal image
+copies that directory explicitly because Modal's Python-source layer does not
+include non-Python files by default:
+
+- one common `code,name` file, `names.csv`, containing the union of supported
+  authority codes;
+- one `code` file, `codes_lad22.csv`, listing the 374-authority LAD22
+  configuration; and
+- one `code` file, `codes_lad23.csv`, listing the 361-authority LAD23
+  configuration.
+
+The two code lists replaced `coordinates_lad22.csv` and
+`coordinates_lad23.csv`, which also gave each authority an integer map
+position (`x` and `y`). Results no longer carry a map position, so the
+executor no longer ships one; the code lists keep exactly the codes those
+files listed.
 
 The Enhanced FRS geography is LAD22: its 2021 output areas are crosswalked to
 2022 local-authority boundaries. The output-area vintage does not make the
-local-authority boundary version LAD21. Names and coordinates are attached only
-after numeric local-authority impacts have been calculated; these resources
-never assign a household to an authority.
+local-authority boundary version LAD21. Names are attached only after numeric
+local-authority impacts have been calculated; these resources never assign a
+household to an authority.
 
 The detected boundary version is stored as typed metadata in both child Parquet
 artifacts. The coordinator requires baseline and reform to agree before it
 enriches the aggregate output. Existing non-Stage-12 calculations continue to
 use the existing GCS lookup and are not changed by this path.
 
 This resource ownership is temporary. A future boundary version must add its
-codes to the common names file, add a distinct coordinate file, and register a
-typed detector definition. Longer term, display-coordinate rendering should
-move to the front end, and Stage 14 or later work should remove these temporary
+codes to the common names file, add a distinct code file, and register a typed
+detector definition. Results carry no map positions, which the app draws from
+its own layout files, and Stage 14 or later work should remove these temporary
 executor resources when the authoritative v2 report architecture supersedes
 Stage 12.
 

projects/policyengine-apis-integ/tests/simulation/test_calculate.py

@@ -263,7 +263,8 @@ def assert_uk_local_area_breakdown(breakdown_result: object) -> None:
     """Check a local-area breakdown came from the local-area dataset.
 
     Both area levels must publish at least one area, and every published area
-    must carry a disclosure tier.
+    must carry a disclosure tier and no x or y, which the executor no longer
+    sends.
     """
     breakdown = _as_mapping(breakdown_result, label="local-area breakdown")
     assert breakdown.get("kind") == "local_areas", (
@@ -283,6 +284,10 @@ def assert_uk_local_area_breakdown(breakdown_result: object) -> None:
             assert record.get("disclosure_tier") in _PUBLISHED_DISCLOSURE_TIERS, (
                 f"Expected a published disclosure_tier in {key}, got {record}"
             )
+            map_position = {"x", "y"} & set(record)
+            assert not map_position, (
+                f"Expected {key} records without x or y, got {record}"
+            )
 
 
 class _GeneratedClientObject:
@@ -326,8 +331,6 @@ def test_result_assertions_accept_generated_client_objects() -> None:
 _BREAKDOWN_CONSTITUENCY = {
     "constituency_code": "E14001063",
     "constituency_name": "Aldershot",
-    "x": 56,
-    "y": -40,
     "average_household_income_change": -150.0,
     "relative_household_income_change": -0.0026,
     "population": 40530.0,
@@ -340,8 +343,6 @@ def test_result_assertions_accept_generated_client_objects() -> None:
 _BREAKDOWN_LOCAL_AUTHORITY = {
     "local_authority_code": "E06000063",
     "local_authority_name": "Cumberland",
-    "x": 4,
-    "y": 18,
     "average_household_income_change": -120.0,
     "relative_household_income_change": -0.002,
     "population": 120000.0,
@@ -389,6 +390,21 @@ def test_generated_client_reads_area_records_without_x_and_y() -> None:
     assert status.result.to_dict() == breakdown
 
 
+def test_generated_client_reads_area_records_with_x_and_y() -> None:
+    # Breakdowns that started before the executor stopped sending x and y
+    # still carry them, until the contract removes the two fields.
+    breakdown = {
+        **_BREAKDOWN_RESULT,
+        "constituency_impact": [{**_BREAKDOWN_CONSTITUENCY, "x": 56, "y": -40}],
+        "local_authority_impact": [{**_BREAKDOWN_LOCAL_AUTHORITY, "x": 4, "y": 18}],
+    }
+
+    status = JobStatusResponse.from_dict({"status": "complete", "result": breakdown})
+
+    assert isinstance(status.result, LocalAreaBreakdownOutput)
+    assert status.result.to_dict() == breakdown
+
+
 @pytest.mark.parametrize(
     ("change", "message"),
     [
@@ -403,6 +419,18 @@ def test_generated_client_reads_area_records_without_x_and_y() -> None:
             },
             "disclosure_tier",
         ),
+        (
+            {"constituency_impact": [{**_BREAKDOWN_CONSTITUENCY, "x": 56, "y": -40}]},
+            "x or y",
+        ),
+        (
+            {
+                "local_authority_impact": [
+                    {**_BREAKDOWN_LOCAL_AUTHORITY, "x": 4, "y": 18}
+                ]
+            },
+            "x or y",
+        ),
     ],
 )
 def test_breakdown_assertions_reject_an_incomplete_breakdown(change, message) -> None:

projects/policyengine-simulation-executor/src/modal/v2_static_runtime_files_smoke.py

@@ -44,12 +44,8 @@ def smoke_stage12_static_runtime_files() -> dict[str, int | str]:
     resources = load_uk_local_authority_resources()
     return {
         "names": len(resources.names),
-        "lad22_coordinates": len(
-            resources.coordinates[UKLocalAuthorityBoundaryVersion.LAD22]
-        ),
-        "lad23_coordinates": len(
-            resources.coordinates[UKLocalAuthorityBoundaryVersion.LAD23]
-        ),
+        "lad22_codes": len(resources.codes[UKLocalAuthorityBoundaryVersion.LAD22]),
+        "lad23_codes": len(resources.codes[UKLocalAuthorityBoundaryVersion.LAD23]),
         "installed_dataset_boundary_version": (
             validate_installed_uk_local_authority_dataset()
         ),
@@ -62,8 +58,8 @@ def main() -> None:
     observed = smoke_stage12_static_runtime_files.remote()
     expected_counts = {
         "names": 378,
-        "lad22_coordinates": 374,
-        "lad23_coordinates": 361,
+        "lad22_codes": 374,
+        "lad23_codes": 361,
     }
     if {key: observed.get(key) for key in expected_counts} != expected_counts:
         msg = (

projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_geographic.py

@@ -3,7 +3,6 @@
 from __future__ import annotations
 
 import logging
-import math
 from collections.abc import Mapping
 from dataclasses import dataclass, field
 from functools import lru_cache
@@ -82,7 +81,7 @@ def _complete_uk_geography_output(
     code_field: str,
     name_field: str,
 ) -> GeographicImpactOutput:
-    """Require every UK geography record to contain lookup-owned metadata.
+    """Require every UK geography record to carry its code and lookup name.
 
     An empty list is valid: disclosure control may withhold every area.
     """
@@ -100,9 +99,6 @@ def _complete_uk_geography_output(
         if not isinstance(name, str) or not name.strip() or name == code:
             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:
-            msg = f"UK geography {code!r} is missing lookup coordinates"
-            raise ValueError(msg)
     return output
 
 
@@ -646,39 +642,26 @@ def compute_uk_geography_disclosure_output(
     )
 
 
-def _uk_constituency_lookup(
-    lookup_csv_path: str,
-) -> dict[str, tuple[str, int | None, int | None]]:
-    """Return each constituency's name and map coordinates, in lookup order."""
+def _uk_constituency_lookup(lookup_csv_path: str) -> dict[str, str]:
+    """Return each constituency's name, in lookup order.
+
+    The lookup's other columns, such as its x and y map positions, are not
+    read: sim-api results no longer carry an area's map position.
+    """
     lookup = pd.read_csv(lookup_csv_path)
     if "code" not in lookup.columns:
         msg = (
             "UK constituency lookup CSV must contain a 'code' column: "
             f"{lookup_csv_path}"
         )
         raise ValueError(msg)
-    entries: dict[str, tuple[str, int | None, int | None]] = {}
+    names: dict[str, str] = {}
     for row in lookup.to_dict("records"):
         code = _normalised_area_code(row.get("code"))
         if not code:
             continue
-        name = _normalised_area_code(row.get("name")) or code
-        entries[code] = (
-            name,
-            _lookup_coordinate(row.get("x")),
-            _lookup_coordinate(row.get("y")),
-        )
-    return entries
-
-
-def _lookup_coordinate(value: object) -> int | None:
-    if isinstance(value, bool) or not isinstance(value, int | float | str):
-        return None
-    try:
-        number = float(value)
-    except ValueError:
-        return None
-    return int(number) if math.isfinite(number) else None
+        names[code] = _normalised_area_code(row.get("name")) or code
+    return names
 
 
 def _require_uk_disclosure_output(
@@ -699,7 +682,7 @@ def build_uk_constituency_impact(
 
     Areas that disclosure control withholds are left out. Downloads the
     constituency lookup if needed and raises when a published area lacks a
-    lookup name or coordinates.
+    lookup name.
     """
     if country != "uk":
         return None
@@ -712,18 +695,14 @@ def build_uk_constituency_impact(
     ordered = [code for code in lookup if code in changes] + sorted(
         set(changes) - set(lookup)
     )
-    records: list[dict[str, object]] = []
-    for code in ordered:
-        name, x, y = lookup.get(code, (code, None, None))
-        records.append(
-            {
-                "constituency_code": code,
-                "constituency_name": name,
-                "x": x,
-                "y": y,
-                **changes[code].record_fields(),
-            }
-        )
+    records: list[dict[str, object]] = [
+        {
+            "constituency_code": code,
+            "constituency_name": lookup.get(code, code),
+            **changes[code].record_fields(),
+        }
+        for code in ordered
+    ]
     return _complete_uk_geography_output(
         records,
         code_field="constituency_code",
@@ -737,7 +716,7 @@ def build_uk_local_authority_impact(
     *,
     uk_local_authority_metadata: UKLocalAuthorityMetadata | None = None,
 ) -> GeographicImpactOutput | None:
-    """Return the published UK local authorities, named and placed for the boundaries.
+    """Return the published UK local authorities, named for the boundary version.
 
     None for other countries. For the UK, ``uk_local_authority_metadata`` is
     required, and a code outside its boundary version raises ``ValueError``.
@@ -757,13 +736,11 @@ def build_uk_local_authority_impact(
     )
 
     boundary_version = uk_local_authority_metadata.boundary_version
-    display_metadata = load_uk_local_authority_resources().metadata_for(
-        boundary_version
-    )
+    names = load_uk_local_authority_resources().names_for(boundary_version)
     records: list[dict[str, object]] = []
     for code in sorted(changes):
-        metadata = display_metadata.get(code)
-        if metadata is None:
+        name = names.get(code)
+        if name is None:
             msg = (
                 f"UK local-authority code {code!r} is not part of the detected "
                 f"{boundary_version.value.upper()} boundary version"
@@ -772,9 +749,7 @@ def build_uk_local_authority_impact(
         records.append(
             {
                 "local_authority_code": code,
-                "local_authority_name": metadata.name,
-                "x": metadata.x,
-                "y": metadata.y,
+                "local_authority_name": name,
                 **changes[code].record_fields(),
             }
         )

projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_worker_validation.py

@@ -122,8 +122,8 @@ def _check_uk_local_authority_resources() -> None:
     )
 
     resources = load_uk_local_authority_resources()
-    for boundary_version in resources.coordinates:
-        resources.metadata_for(boundary_version)
+    for boundary_version in resources.codes:
+        resources.names_for(boundary_version)
 
 
 def _check_uk_local_authority_dataset(dataset_path: str) -> None: