Skip to content

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

Merged
anth-volk merged 3 commits into
masterfrom
chore/v2-area-impacts-without-xy
Nov 3, 2026
Merged

anth-volk merged 3 commits into
masterfrom
chore/v2-area-impacts-without-xy

Conversation

@anth-volk

@anth-volk anth-volk commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #4102

Warning

Deploying this PR changes the staging and production v2 database schemas. The release's v2 migration job drops four columns, and their values can't be recovered. Anthony must approve the merge.

Summary

API v2's constituency_impacts and local_authority_impacts tables stored each area's hex-map position in required x and y integer columns. Nothing reads or writes either table, and the app draws its maps from its own layout files, so the positions are unused.

  • ConstituencyImpact and LocalAuthorityImpact no longer define x and y.
  • v2 revision e86f19a6c4da (parent f5c1d9b4685b, catalog format 2 metadata) was autogenerated from that metadata with uv run alembic -c alembic-v2.ini revision --autogenerate --rev-id e86f19a6c4da, against a disposable Postgres database at the previous head. After master gained fb1a3e40d3d6 on the same parent, it was generated again on top of it in the merge commit 75c797384, and again on top of f5c1d9b4685b in the merge commit be0e090fd, each time with the same operations. It has no post-generation corrections. Its upgrade drops the four columns; its downgrade adds them back as required integers without a server default, which works only while both tables are empty. V2_ALEMBIC_HEAD_REVISION is now e86f19a6c4da.
  • Compatibility with the previous Public API revision. The Stage 12A revision kept every column the previous Public API revision maps, so that revision keeps working during the deploy window and after an application rollback. This revision drops four columns that the previous revision maps. That is safe because the previous revision can't read or write either table: no managed database role (policyengine_v2_runtime, policyengine_v2_catalog_writer, policyengine_stage12_<environment>) holds any privilege on them in policyengine_api/data/v2/privileges.py, and no Public API code refers to them outside the models.
  • test_stage_12a_schema_serves_the_previous_revision and master's test_countries_schema_serves_the_previous_revision upgraded to head. Each now upgrades to the revision it was written for (fbb7488ebe8a and fb1a3e40d3d6), because head now drops columns that their parent revisions map. A new lifecycle test, test_area_impact_coordinate_removal_changes_nothing_else, checks that the four columns are the only difference between f5c1d9b4685b and head.
  • The Alembic guide (docs/engineering/skills/alembic-migrations.md) documents the revision, and the Stage 12A guide names its own revision where it said head.

No route, request or response changes: nothing serves these tables. No CI workflow, Docker image or deployment code changes; the existing v2 seed workflow applies the revision before each Cloud Run deploy. No inline ruff suppressions are added.

Testing

The work was done red-green: the tests were written first and seen failing, then the code was written and both were committed together in b03f88e55.

Seen failing before the change (5 failed, 64 passed):

  • tests/unit/v2/test_models.py::test_uk_area_impacts_store_no_map_coordinates: x and y were on both tables.
  • tests/unit/v2/test_alembic_v2.py::test_v2_revision_chain_has_generated_policy_and_user_identity_changes: the chain had no revision after the previous head.
  • tests/unit/v2/test_alembic_v2.py::test_area_impact_coordinate_removal_is_generated_and_reversible: the revision file didn't exist.
  • tests/integration/test_alembic_v2_lifecycle.py::test_empty_upgrade_check_base_downgrade_and_reupgrade: head still had the columns.
  • tests/integration/test_alembic_v2_lifecycle.py::test_area_impact_coordinate_removal_changes_nothing_else: no columns were dropped.

After the change, against a disposable postgres:17 database named policyengine_v2_alembic_test, set up as the alembic-v2-check.yml service container is:

  • FLASK_DEBUG=1 uv run --frozen pytest tests/unit/v2/test_models.py tests/unit/v2/test_alembic_v2.py tests/integration/test_alembic_v2_lifecycle.py tests/unit/v2/test_catalog_publication.py tests/unit/v2/test_user_association_migration_qualification.py tests/unit/v2/test_privilege_manifest.py -q: 122 passed.
  • tests/integration/test_v2_privilege_reconciliation.py (DEPLOYMENT_ENVIRONMENT=test): 3 passed.
  • tests/unit/test_alembic_skill.py, tests/unit/test_alembic_workflows.py, tests/unit/data/test_database_access_boundaries.py: 26 passed.
  • alembic -c alembic-v2.ini check before generating: exactly four remove_column operations, one for each column. After upgrading: No new upgrade operations detected.
  • alembic -c alembic-v2.ini downgrade -1, then upgrade head, then current --check-heads: e86f19a6c4da (head).
  • make format, ruff check ., uv run --frozen --extra dev mypy, python scripts/run_quality_guards.py and python scripts/export_migration_contracts.py (no generated changes): all pass.

After merging master on 3 Nov (be0e090fd), on a disposable postgres:17 database:

  • alembic -c alembic-v2.ini heads: e86f19a6c4da (head); check: No new upgrade operations detected.
  • The revision-chain, coordinate-removal and countries tests in tests/unit/v2/test_alembic_v2.py: passed.
  • test_empty_upgrade_check_base_downgrade_and_reupgrade, test_stage_12a_schema_serves_the_previous_revision, test_countries_schema_serves_the_previous_revision and test_area_impact_coordinate_removal_changes_nothing_else: 4 passed.
  • ruff check and ruff format --check on the changed Python files, and uv run --frozen --extra dev mypy: pass.

The full suite runs in CI.

🤖 Generated with Claude Code

`constituency_impacts` and `local_authority_impacts` stored each area's
hex-map position in required `x` and `y` integer columns. Nothing reads
or writes either table, and the app draws its maps from its own layout
files, so the positions are unused.

- `ConstituencyImpact` and `LocalAuthorityImpact` no longer define `x`
  and `y`.
- v2 revision `e86f19a6c4da` was autogenerated from that metadata with
  `alembic -c alembic-v2.ini revision --autogenerate --rev-id
  e86f19a6c4da`, with no post-generation corrections. Its upgrade drops
  the four columns; its downgrade adds them back as required integers,
  which works only while both tables are empty. It is the new
  `V2_ALEMBIC_HEAD_REVISION`.
- The previous Public API revision still maps these columns, but no
  managed database role holds any privilege on either table, so it can't
  read or write them during the deploy window or after a rollback. A new
  lifecycle test checks that the four columns are the only difference
  from the Stage 12A schema. The Stage 12A previous-revision test now
  upgrades to the Stage 12A revision, which it was written for, instead
  of head.
- The Alembic guide documents the revision, and the Stage 12A guide
  names its own revision where it said head.

Tests were written first and seen failing: the model test found `x` and
`y` on both tables, the revision chain and revision-text tests found no
`e86f19a6c4da`, and the lifecycle tests found the columns still present
at head.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.22%. Comparing base (eb96f3a) to head (be0e090).
⚠️ Report is 43 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4103   +/-   ##
=======================================
  Coverage   88.22%   88.22%           
=======================================
  Files         285      285           
  Lines       16847    16856    +9     
  Branches     2741     2744    +3     
=======================================
+ Hits        14863    14871    +8     
  Misses       1302     1302           
- Partials      682      683    +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

master added v2 revision fb1a3e40d3d6 (the countries table) on
fbb7488ebe8a, the parent of this branch's e86f19a6c4da, so the v2
history had two heads. Resolved:

- e86f19a6c4da is generated again on top of fb1a3e40d3d6: the old file
  was removed, a disposable Postgres was upgraded to fb1a3e40d3d6, and
  `alembic -c alembic-v2.ini revision --autogenerate --rev-id
  e86f19a6c4da` produced the same four drop_column operations and
  their downgrade, formatted with ruff. Only `Revises`,
  `down_revision` and the creation date differ, and `alembic check`
  finds nothing left to generate.
- V2_ALEMBIC_HEAD_REVISION stays e86f19a6c4da, and the revision-chain
  unit test lists fb1a3e40d3d6 between it and fbb7488ebe8a. This
  branch's revision test expects the new parent, and master's
  countries tests find their revision by ID instead of as the head.
- master turned the Stage 12A serving test into
  _assert_head_serves_previous_revision, used for Stage 12A and for
  the countries table; this branch made the Stage 12A check upgrade to
  its own revision, because head now drops columns the previous
  release maps. The helper is now
  _assert_revision_serves_previous_revision, and each check upgrades
  to the revision it was written for. The coordinate test compares
  head with the countries revision, and the downgrade walk steps back
  to the countries revision before the countries downgrade.
- The Alembic guide keeps both sections, in revision order.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
master added v2 revision f5c1d9b4685b (catalog format 2 metadata) on
fb1a3e40d3d6, the parent of this branch's e86f19a6c4da, so the v2
history had two heads. Resolved:

- e86f19a6c4da is generated again on top of f5c1d9b4685b: the old file
  was removed, a disposable Postgres was upgraded to f5c1d9b4685b, and
  `alembic -c alembic-v2.ini revision --autogenerate --rev-id
  e86f19a6c4da` produced the same four drop_column operations and
  their downgrade, in a different order, formatted with ruff. Only
  `Revises`, `down_revision`, the creation date and that order differ,
  and `alembic check` finds nothing left to generate.
- V2_ALEMBIC_HEAD_REVISION stays e86f19a6c4da. The revision-chain unit
  test lists f5c1d9b4685b between it and the countries revision, and
  this branch's revision test expects the new parent.
- The lifecycle test's coordinate checks compare head with, and
  downgrade to, f5c1d9b4685b instead of the countries revision, since
  head's parent now adds the catalog format 2 columns.
- The Alembic guide names the new parent, and the Stage 12A guide takes
  master's wording of the same sentence.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@anth-volk
anth-volk marked this pull request as ready for review November 3, 2026 20:19
@anth-volk
anth-volk merged commit 42697cb into master Nov 3, 2026
22 checks passed
@anth-volk
anth-volk deleted the chore/v2-area-impacts-without-xy branch November 3, 2026 20:19

This branch was previously deployed

1 inactive deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant