Drop the x and y columns from API v2's UK area impact tables

changelog.d/v2-area-impacts-without-xy.removed.md

@@ -0,0 +1 @@
+API v2's `constituency_impacts` and `local_authority_impacts` tables no longer have the `x` and `y` hex-map position columns; v2 Alembic revision `e86f19a6c4da` drops them. Nothing read or wrote these tables, and the app draws its maps from its own layout files.

docs/engineering/skills/alembic-migrations.md

@@ -422,3 +422,31 @@ the new release, so instances of the previous release that start afterwards,
 and a rollback to it, refuse to start until the revision is downgraded. The
 module docstring of `supported_countries.py` lists the other checks a new
 country must widen.
+
+### API v2 UK area impact map coordinate removal
+
+Revision `e86f19a6c4da` was autogenerated, with `--rev-id e86f19a6c4da`, after
+the `x` and `y` fields were removed from the `ConstituencyImpact` and
+`LocalAuthorityImpact` models, and generated again on top of each revision
+that reached `master` first: the countries revision `fb1a3e40d3d6`, then the
+catalog format 2 revision `f5c1d9b4685b`. It has no post-generation
+corrections. Its
+upgrade drops `x` and `y` from `constituency_impacts` and
+`local_authority_impacts`; its downgrade adds them back as required integers
+without a server default, as the baseline created them.
+
+The columns held hex-map positions, which nothing needs: the app draws its
+maps from its own layout files. The Stage 12A revision kept every column that
+the previous Public API revision maps, so that revision keeps working during
+the deploy window and after an application rollback
+(`docs/migration/stage-12a-user-login.md`). This revision drops four columns
+that the previous revision maps, which is safe because that revision can't
+read or write either table: no managed database role holds any privilege on
+them (`policyengine_api/data/v2/privileges.py`), and no Public API code reads
+or writes them. The lifecycle test
+`test_area_impact_coordinate_removal_changes_nothing_else` checks that these
+four columns are the only difference from the catalog format 2 revision's
+schema.
+
+The dropped values are not recoverable. Adding a required column fails on a
+table that has rows, so the downgrade works only while both tables are empty.

docs/migration/stage-12a-user-login.md

@@ -402,23 +402,23 @@ row with a NULL `primary_country` does not fail there.
 
 The lifecycle test `test_stage_12a_schema_serves_the_previous_revision`
 exercises this rather than only comparing columns. It reflects the schema at
-the parent revision with SQLAlchemy and upgrades to head. Then, through the
-reflected parent tables, it does the following for every table the parent
-revision maps:
+the parent revision with SQLAlchemy and upgrades to `fbb7488ebe8a`. Then,
+through the reflected parent tables, it does the following for every table the
+parent revision maps:
 
 - inserts a row that supplies only the parent's required columns (plus the
   nullable values that the parent's own check constraints demand);
 - reads the row back with every parent column;
 - rewrites every non-key column;
 - deletes the row.
 
-Rows of the two dormant tables are inserted through head's tables, since only
-new code writes them, and are then read, rewritten, and deleted through the
-parent's. The test also deletes, through the parent's `users` table, a user who
-owns rows in all four association tables and a `user_identities` row, and it
-checks that every one of them cascades away. It fails if head drops a parent
-column, or adds a required column without a server default to a table the
-parent revision writes.
+Rows of the two dormant tables are inserted through the Stage 12A tables,
+since only new code writes them, and are then read, rewritten, and deleted
+through the parent's. The test also deletes, through the parent's `users`
+table, a user who owns rows in all four association tables and a
+`user_identities` row, and it checks that every one of them cascades away. It
+fails if Stage 12A drops a parent column, or adds a required column without a
+server default to a table the parent revision writes.
 
 ## Settings
 
@@ -571,8 +571,8 @@ The disposable-Postgres suite proves the following:
 - `MATCH SIMPLE` composite keys accept NULL v2 references;
 - account deletion cascades after removing legacy mappings;
 - NaN and Infinity never reach JSONB;
-- the parent revision's table usage works against head (see "Schema
-  compatibility");
+- the parent revision's table usage works against the Stage 12A schema (see
+  "Schema compatibility");
 - the qualification transaction is read-only.
 
 ## Deferred

migrations/v2/versions/e86f19a6c4da_drop_uk_area_impact_map_coordinates.py

@@ -0,0 +1,49 @@
+"""drop UK area impact map coordinates
+
+Revision ID: e86f19a6c4da
+Revises: f5c1d9b4685b
+Create Date: 2026-11-03 23:42:47.925451
+Generation: uv run alembic -c alembic-v2.ini revision --autogenerate
+"""
+
+from typing import Sequence, Union
+
+from alembic import op
+import sqlalchemy as sa
+import sqlmodel
+
+
+revision: str = "e86f19a6c4da"
+down_revision: Union[str, None] = "f5c1d9b4685b"
+branch_labels: Union[str, Sequence[str], None] = None
+depends_on: Union[str, Sequence[str], None] = None
+
+
+def upgrade() -> None:
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.drop_column("constituency_impacts", "y")
+    op.drop_column("constituency_impacts", "x")
+    op.drop_column("local_authority_impacts", "y")
+    op.drop_column("local_authority_impacts", "x")
+    # ### end Alembic commands ###
+
+
+def downgrade() -> None:
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.add_column(
+        "local_authority_impacts",
+        sa.Column("x", sa.INTEGER(), autoincrement=False, nullable=False),
+    )
+    op.add_column(
+        "local_authority_impacts",
+        sa.Column("y", sa.INTEGER(), autoincrement=False, nullable=False),
+    )
+    op.add_column(
+        "constituency_impacts",
+        sa.Column("x", sa.INTEGER(), autoincrement=False, nullable=False),
+    )
+    op.add_column(
+        "constituency_impacts",
+        sa.Column("y", sa.INTEGER(), autoincrement=False, nullable=False),
+    )
+    # ### end Alembic commands ###

policyengine_api/data/v2/migration_target.py

@@ -24,7 +24,7 @@
     from collections.abc import Mapping
 
 V2_ALEMBIC_DISPOSABLE_TEST = "V2_ALEMBIC_DISPOSABLE_TEST"
-V2_ALEMBIC_HEAD_REVISION = "f5c1d9b4685b"
+V2_ALEMBIC_HEAD_REVISION = "e86f19a6c4da"
 DISPOSABLE_DATABASE_NAME = "policyengine_v2_alembic_test"
 DISPOSABLE_HOSTS = frozenset({"localhost", "127.0.0.1", "::1", "postgres"})
 MIGRATION_ROLE = "policyengine_v2_migrator"

policyengine_api/data/v2/models/reports.py

@@ -606,7 +606,7 @@ class CongressionalDistrictImpact(GeographicImpactBase, table=True):
 
 
 class ConstituencyImpact(GeographicImpactBase, table=True):
-    """Household income change in one UK constituency, with its map coordinates.
+    """Household income change in one UK constituency.
 
     One row per run and constituency code.
     """
@@ -622,14 +622,12 @@ class ConstituencyImpact(GeographicImpactBase, table=True):
 
     constituency_code: str = Field(max_length=64)
     constituency_name: str = Field(max_length=255)
-    x: int
-    y: int
 
     report_run: ReportRun = Relationship(back_populates="constituency_impacts")
 
 
 class LocalAuthorityImpact(GeographicImpactBase, table=True):
-    """Household income change in one UK local authority, with its map coordinates.
+    """Household income change in one UK local authority.
 
     One row per run and local authority code.
     """
@@ -645,7 +643,5 @@ class LocalAuthorityImpact(GeographicImpactBase, table=True):
 
     local_authority_code: str = Field(max_length=64)
     local_authority_name: str = Field(max_length=255)
-    x: int
-    y: int
 
     report_run: ReportRun = Relationship(back_populates="local_authority_impacts")

tests/integration/test_alembic_v2_lifecycle.py

@@ -31,8 +31,14 @@
 STAGE_12_PREVIOUS_REVISION = "724b1b11a33e"
 STAGE_12_COMPARISON_PREVIOUS_REVISION = "439303be14fe"
 STAGE_12A_PREVIOUS_REVISION = "91af4fc8d8c1"
-COUNTRIES_PREVIOUS_REVISION = "fbb7488ebe8a"
+STAGE_12A_REVISION = "fbb7488ebe8a"
+COUNTRIES_PREVIOUS_REVISION = STAGE_12A_REVISION
+COUNTRIES_REVISION = "fb1a3e40d3d6"
+# The catalog format 2 revision, the parent of the area impact coordinate removal.
+CATALOG_FORMAT_2_REVISION = "f5c1d9b4685b"
 V2_TABLE_NAMES = frozenset(table.name for table in V2_METADATA.tables.values())
+AREA_IMPACT_TABLES = ("constituency_impacts", "local_authority_impacts")
+MAP_COORDINATE_COLUMNS = ("x", "y")
 
 
 def _disposable_url() -> str:
@@ -100,6 +106,8 @@ def _assert_head(engine) -> None:
         "legacy_user_mappings",
         "legacy_user_policy_mappings",
     } <= set(inspect(engine).get_table_names(schema="public"))
+    for table_name in AREA_IMPACT_TABLES:
+        assert set(MAP_COORDINATE_COLUMNS).isdisjoint(_columns(engine, table_name))
 
 
 def _columns(engine, table_name: str) -> dict[str, dict]:
@@ -109,6 +117,17 @@ def _columns(engine, table_name: str) -> dict[str, dict]:
     }
 
 
+def _assert_area_impact_coordinates_restored(engine: Engine) -> None:
+    """Check the catalog format 2 revision's map coordinates are required integers."""
+    _assert_revision(engine, CATALOG_FORMAT_2_REVISION)
+    for table_name in AREA_IMPACT_TABLES:
+        columns = _columns(engine, table_name)
+        for column_name in MAP_COORDINATE_COLUMNS:
+            assert isinstance(columns[column_name]["type"], sa.Integer)
+            assert not columns[column_name]["nullable"]
+            assert columns[column_name]["default"] is None
+
+
 def _assert_pre_stage_12a_association_shape(engine) -> None:
     assert "user_identities" not in inspect(engine).get_table_names(schema="public")
     for table_name, reference in (
@@ -436,6 +455,10 @@ def test_empty_upgrade_check_base_downgrade_and_reupgrade() -> None:
         _upgrade_and_assert_head(config, engine)
         _assert_countries_shape(engine)
 
+        command.downgrade(config, CATALOG_FORMAT_2_REVISION)
+        _assert_area_impact_coordinates_restored(engine)
+        _upgrade_and_assert_head(config, engine)
+
         command.downgrade(config, COUNTRIES_PREVIOUS_REVISION)
         _assert_revision(engine, COUNTRIES_PREVIOUS_REVISION)
         _assert_pre_countries_shape(engine)
@@ -636,18 +659,18 @@ def _rewrite_mapped_columns(connection, table: sa.Table, row: dict) -> None:
 
 @dataclass(frozen=True)
 class PreviousRevisionSchemas:
-    """A previous revision's reflected schema and head's, after the upgrade.
+    """A previous revision's reflected schema and the one it was upgraded to.
 
     ``read_only_tables`` are the previous revision's tables that only new code
-    writes, so their test rows are inserted through head's table.
+    writes, so their test rows are inserted through the upgraded table.
     """
 
     parent: sa.MetaData
     head: sa.MetaData
     read_only_tables: frozenset[str]
 
     def insert_table(self, table_name: str) -> sa.Table | None:
-        """Head's table for a read-only table, otherwise ``None``."""
+        """Return the upgraded table for a read-only table, otherwise ``None``."""
         if table_name in self.read_only_tables:
             return self.head.tables[table_name]
         return None
@@ -698,19 +721,21 @@ def _assert_user_deletion_cascades(
         assert remaining == 0, table_name
 
 
-def _assert_head_serves_previous_revision(
+def _assert_revision_serves_previous_revision(
     previous_revision: str,
+    revision: str,
     read_only_tables: frozenset[str],
 ) -> None:
-    """Run a previous Public API revision's table usage against head.
+    """Run a previous Public API revision's table usage against ``revision``.
 
-    The parent revision's schema is reflected and then used, after upgrading,
+    The parent revision's schema is reflected and then used, after upgrading
+    to ``revision``, the revision it ran beside during that deploy,
     to insert a row into every table it maps (supplying only what it
     requires), read the row back with every mapped column, rewrite those
     columns, delete it, and delete a user who owns association rows so the
-    cascades run. Head may not give any table the parent revision writes a
-    new required column without a server default; ``read_only_tables`` are the
-    parent's tables that only new code writes.
+    cascades run. ``revision`` may not give any table the parent revision
+    writes a new required column without a server default; ``read_only_tables``
+    are the parent's tables that only new code writes.
     """
     database_url = _disposable_url()
     config = _config()
@@ -720,7 +745,7 @@ def _assert_head_serves_previous_revision(
         command.downgrade(config, previous_revision)
         parent = sa.MetaData()
         parent.reflect(engine)
-        command.upgrade(config, "head")
+        command.upgrade(config, revision)
         head = sa.MetaData()
         head.reflect(engine)
         parent_tables = [
@@ -781,20 +806,67 @@ def _assert_head_serves_previous_revision(
 
 
 def test_stage_12a_schema_serves_the_previous_revision() -> None:
-    _assert_head_serves_previous_revision(
+    _assert_revision_serves_previous_revision(
         STAGE_12A_PREVIOUS_REVISION,
+        STAGE_12A_REVISION,
         STAGE_12A_PARENT_READ_ONLY_TABLES,
     )
 
 
 def test_countries_schema_serves_the_previous_revision() -> None:
     """The release before the countries table keeps working during a deploy."""
-    _assert_head_serves_previous_revision(
+    _assert_revision_serves_previous_revision(
         COUNTRIES_PREVIOUS_REVISION,
+        COUNTRIES_REVISION,
         frozenset(),
     )
 
 
+def test_area_impact_coordinate_removal_changes_nothing_else() -> None:
+    """Check that head drops the map coordinates and changes nothing else.
+
+    Head is compared with its parent, the catalog format 2 revision. The
+    previous Public API revision still maps those four columns, but it can't
+    read or write either table: no managed database role holds any privilege
+    on them (``policyengine_api/data/v2/privileges.py``). Every table it can
+    use keeps every column.
+    """
+    database_url = _disposable_url()
+    config = _config()
+    engine = create_engine(database_url)
+
+    try:
+        command.downgrade(config, CATALOG_FORMAT_2_REVISION)
+        parent = sa.MetaData()
+        parent.reflect(engine)
+        command.upgrade(config, "head")
+        head = sa.MetaData()
+        head.reflect(engine)
+
+        assert set(head.tables) == set(parent.tables)
+        dropped_columns = sorted(
+            f"{table.name}.{column.name}"
+            for table in parent.tables.values()
+            for column in table.columns
+            if column.name not in head.tables[table.name].columns
+        )
+        added_columns = sorted(
+            f"{table.name}.{column.name}"
+            for table in head.tables.values()
+            for column in table.columns
+            if column.name not in parent.tables[table.name].columns
+        )
+        assert dropped_columns == sorted(
+            f"{table_name}.{column_name}"
+            for table_name in AREA_IMPACT_TABLES
+            for column_name in MAP_COORDINATE_COLUMNS
+        )
+        assert added_columns == []
+    finally:
+        command.upgrade(config, "head")
+        engine.dispose()
+
+
 def test_upgrade_to_head_validates_the_resulting_schema_against_metadata() -> None:
     database_url = _disposable_url()
     config = _config()