Repository navigation
Conversation
Nationwide reports in the app now take congressional districts from a separate run of each state and DC on that state's dataset, so a district shows the same figures in a nationwide report as in its state's report. National runs no longer need to compute them. _should_build_us_congressional_district_impact now allows only state/* regions, in any case. National runs (no region, or us) and region-group runs return congressional_district_impact null. Segmented national children run as region groups, and the segmented national reduce and Stage 12 aggregation build their output with the national region, so all of them follow the same check. District and place runs still compute none. The contract field is already nullable. Tests, seen failing before the change: - test_region_group_resolution.py: state/ca and STATE/CA are allowed; None, us and region_group/... are refused - test_simulation_output_builder.py: the output builder skips the district calculation for None, us and a region group, and still runs it for state/ut Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
National runs now return congressional_district_impact null, so the beta check in test_calculate_default_model would fail against them. A new helper, assert_no_congressional_district_results, checks that the field is present and null, and test_calculate_default_model uses it in place of the at-large district check. The state checks still expect UT- districts. _AT_LARGE_DISTRICT_IDS and assert_congressional_district_results' expected_district_ids option served only the national check, so they go. Tests, seen failing before the helper existed: local tests that the helper accepts a null field, in a dict or a generated client object, and refuses an empty district list, a listed district, a generated client object with districts, and a result without the field. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The docstring of _should_build_us_congressional_district_impact and the changelog item said Stage 12 aggregation computes no districts. Stage 12 aggregation passes on the report's own region, so it computes none for a national report, but a report for one state still computes that state's districts. Both now say so. Documentation only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
main added two changelog items at the top of the changed section, where this branch also added one, so the two sides conflicted. Both are kept, with this branch's item first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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 #954
Same-Repository Draft PR Check
PolicyEngine/policyengine-sim-api, not a personal fork.Do not merge yet
Merging to
maindeploys staging and then production. Until PolicyEngine/policyengine-app-v2#1439 is released, the app reads congressional districts from nationwide results, so this may merge only after that release has been in production for at least a day.That app release in turn waits for the Public API's PolicyEngine/policyengine-api#4064 and this repository's #940 to deploy.
Summary
Why:
What changes:
_should_build_us_congressional_district_impact(simulation_output_geographic.py) now returns true only for astate/*region, in any case.congressional_district_impact: null:us;simulation_output_geographic.pyandsegmented_national_reduce.py, and a changelog entry.assert_no_congressional_district_results, checks that the field is present and null;test_calculate_default_model, a US national run, uses it in place of the at-large district check;UT-districts._AT_LARGE_DISTRICT_IDSandassert_congressional_district_results'expected_district_idsoption served only the national check, so they are removed.Older model versions: a deployment rebuilds only the Modal app for the current policyengine.py version. Apps already deployed for older versions are not rebuilt and keep their code, so a request pinned to an older model version can still return nationwide districts.
Testing
Each commit is red-green: the tests were written first and run before the code change.
TestCongressionalDistrictRegions::test__rejects_national_and_group(None,us,region_group/…);test_builder_skips_districts_for_national_and_region_group_runsforNone,usand a region group, because the output builder still ran the district calculation.test__allows_states(state/ca,STATE/CA);test__rejects_district_and_place;test_builder_builds_districts_for_state_runs(state/ut).Run locally:
uv run --frozen pytest tests/inprojects/policyengine-simulation-executor: 1881 passed, 22 skipped.uv run --frozen pytest tests/simulation/test_calculate.py -m "not beta_only"inprojects/policyengine-apis-integ: 23 passed.make check: Ruff format and lint, and Pyright for every package, passed.GITHUB_BASE_REF=main python scripts/run_quality_guards.py: every guard passed.Not run locally: the
beta_onlyintegration tests, which run against the beta deployment. After this deploys to beta, a US national result from the current version's app should havecongressional_district_impact: null, and astate/utresult should still listUT-districts. The null check applies only to the current version's app, for the reason under "Older model versions".🤖 Generated with Claude Code