Repository navigation
Remove x and y from the contract's UK area records - #924
Merged
Merged
Conversation
ConstituencyImpactRecord and LocalAuthorityImpactRecord no longer have x or y fields. Both records forbid extra fields, so a record carrying either, even as null, is refused, and the gateway reports a job whose result has one as failed. Apps deployed for older policyengine.py versions still send both, so this must not merge until no such app is routed. Nothing uses pydantic's exclude_if any more, so the contract's pydantic floor returns to 2.0, and the five locks that record it return to their state on main. Regenerate the gateway's golden OpenAPI, which drops both properties and the description's paragraph about them. Written red-green: the contract tests failed first with "DID NOT RAISE" for records and job results carrying x or y, and because the records still declared both fields; the gateway returned 200 instead of 500 for a result with x and y; and the generated client's record models still took x and y. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ields Brings in #919's merge of main and resolves two conflicts with it: - changelog_entry.yaml: main replaced it with changelog fragments. This branch's entry becomes changelog.d/923.changed.md. - The gateway's test_endpoints.py: main split it. This branch drops x and y from the shared breakdown result, now in fixtures/gateway_responses.py, and its changes to the x-and-y tests, with its new test of an older app's records, go to test_simulation_job_status.py, where #919's tests now are. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Nov 2, 2026
Brings in #985 (x and y optional in the contract, with reworded comments and test names) and #919 (the executor stops sending them). In each of the six conflicts this branch's side wins, because this branch removes the text main reworded: - macro_output.py: the record docstrings lose the x and y paragraph; the fields, _is_none and the pydantic 2.12 floor stay removed. - The contract tests read records without a map position and refuse records that carry x or y, even as null, instead of keeping them. - apis-integ's generated-client test checks the client's record models have no x or y, replacing the tests that read records with and without them. - The gateway's job-status tests use the shared breakdown fixture, which has no x or y, and the golden OpenAPI equals the document the merged code generates. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Requests never reach apps built for older policyengine.py versions, so the tests and changelog no longer explain the refusal of x and y by them: - The contract tests' comments say records carrying x or y are refused. - The gateway test is test__given_records_with_x_and_y__then_the_job_fails, and its comment says the change waits until no breakdown started before the executor stopped sending x and y is still running. - The changelog fragment drops its sentence about older apps. - apis-integ's _without_x_and_y, which nothing calls after the merge, is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #923
Same-Repository Draft PR Check
PolicyEngine/policyengine-sim-api, not a personal fork.Summary
#985 made
xandyoptional in the contract and #919 stopped the executor sending them; both are onmain. This PR removes them from the contract's area records altogether:ConstituencyImpactRecordandLocalAuthorityImpactRecordhave noxory. A record that carries either, even asnull, is refused (extra="forbid").exclude_ifany more, so the contract's pydantic floor returns to 2.0, one specifier line in each of the five lock files that record it. Accept optional withheld-area code lists on UK local-area breakdown results #976 raises it to 2.12 again for its own fields.xory.Older apps
Requests never reach apps built for .py versions before #919, so this PR doesn't keep them working. The removal takes three deploys (#985, #919, this PR) only so that no deploy leaves the executor and the gateway disagreeing.
Testing
Written red-green: the tests were written first and seen failing for the expected reason, then the code was written and both were committed together.
test_records_with_x_or_y_are_refused(both, either one, or both null, on each record) andtest_a_job_status_refuses_area_records_with_x_and_y(breakdown and national results) failed withDID NOT RAISE;test_records_validate_and_dump_without_x_or_yfailed because the records still declared both fields.test__given_records_with_x_and_y__then_the_job_fails(first named..._from_an_older_app_...) got 200 instead of 500.test_generated_client_area_records_have_no_x_or_yfailed because the record models still tookxandy.Commands run (focused, locally):
uv run --frozen pyteston the contract'stest_macro_output_uk_geography.py,test_local_area_breakdown.pyandtest_gateway_models.py; the gateway'stest_endpoints.pyandtest_openapi_golden.py; the entry'stest_openapi.py; and the executor'stest_simulation_output_geographic.py,test_simulation_output_builder.py,test_uk_local_authority_metadata.pyandtest_stage12_runtime.py.python -m policyengine_simulation_gateway.generate_openapi;./scripts/generate-clients.sh, thenuv run --frozen pytest tests/ -m "not requires_gcp and not beta_only"inpolicyengine-apis-integ: everything passed excepttest_ping, which needs a gateway running on localhost.make check,python scripts/run_quality_guards.pywithGITHUB_BASE_REF=main,python -m scripts.rules render(no changes) and.github/scripts/check-uv-locks.sh.main(a7f11195, with Accept UK area records with or without x and y #985 and Stop sending x and y on UK area results #919; in each of the six conflicts this PR's removal wins) and dropping the older-app wording (d3a195d9): the 8 changed contract tests (18 cases), the three job-status tests andtest_openapi_golden.py(gateway), andtest_generated_client_area_records_have_no_x_or_yagainst a regenerated client pass; the golden OpenAPI equals the generated document;uv lock --checkpasses in the six affected projects, and ruff and.github/scripts/check-changelog-fragment.shpass.mainand its full CI runs.🤖 Generated with Claude Code