From a7860f9fe97beb61258f64d770517db39c170257 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 15 Oct 2026 00:28:45 +0400 Subject: [PATCH 1/6] Build output-builder test frames through one weighted factory The output-builder tests built household frames as plain pd.DataFrames, which carry no weights, so they did not weight the way production's MicroDataFrames do. That is how #767 escaped. fixtures/weighted_frames.py now builds every builder test's entity frames as MicroDataFrames, and rejects a weight column that is the same on every row. The golden macro result keeps its budget: its households now weigh 0.5 and 1.5, with values chosen so that only a correctly weighted build reproduces it. The materialized-columns budget test now expects weighted totals, derived in the test from an explicit weighted formula; it expected unweighted sums before, which production never returned. Co-Authored-By: Claude Opus 5.5 --- .../fixtures/weighted_frames.py | 41 +++++++++++++ .../tests/test_simulation_output_budget.py | 5 +- .../tests/test_simulation_output_builder.py | 57 ++++++++++++------- .../test_simulation_output_geographic.py | 6 +- 4 files changed, 82 insertions(+), 27 deletions(-) create mode 100644 projects/policyengine-simulation-executor/fixtures/weighted_frames.py diff --git a/projects/policyengine-simulation-executor/fixtures/weighted_frames.py b/projects/policyengine-simulation-executor/fixtures/weighted_frames.py new file mode 100644 index 000000000..ae77ddbe2 --- /dev/null +++ b/projects/policyengine-simulation-executor/fixtures/weighted_frames.py @@ -0,0 +1,41 @@ +"""The one shared factory for entity frames in output-builder tests. + +policyengine holds each entity's output as a microdf ``MicroDataFrame`` weighted +by its ``_weight`` column, so ``.sum()`` on any column is already a +weighted sum. Builder tests build their entity frames here, so they weight +exactly as production does: #767 escaped because a builder test used plain +``pd.DataFrame``s, which carry no weights. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import pandas as pd +from microdf import MicroDataFrame + +if TYPE_CHECKING: + from collections.abc import Mapping, Sequence + + +def weighted_entity_frame( + columns: Mapping[str, Sequence[object]], *, entity: str = "household" +) -> MicroDataFrame: + """Return ``columns`` as a MicroDataFrame weighted by ``_weight``. + + Rows without a weight column get the weights 1, 2, 3 and so on. A given + weight column must differ between rows, so that a total which ignores the + weights can't match a weighted expectation by accident. + """ + weight_column = f"{entity}_weight" + frame = pd.DataFrame(dict(columns)) + if weight_column not in frame.columns: + frame[weight_column] = [float(row + 1) for row in range(len(frame))] + weights = frame[weight_column] + if len(frame) > 1 and (weights == weights.iloc[0]).all(): + msg = ( + f"{weight_column} is the same on every row; give the rows different " + "weights so that unweighted totals can't pass for weighted ones" + ) + raise ValueError(msg) + return MicroDataFrame(frame, weights=weight_column) diff --git a/projects/policyengine-simulation-executor/tests/test_simulation_output_budget.py b/projects/policyengine-simulation-executor/tests/test_simulation_output_budget.py index d828a1f42..175361ef4 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_output_budget.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_output_budget.py @@ -4,8 +4,7 @@ from types import SimpleNamespace -import pandas as pd -from microdf import MicroDataFrame +from fixtures.weighted_frames import weighted_entity_frame from policyengine_simulation_executor.simulation_output_budget import ( build_budgetary_impact, ) @@ -13,7 +12,7 @@ def _simulation(columns: dict[str, list[float]]): """Build a simulation whose household output is a policyengine MicroDataFrame.""" - household = MicroDataFrame(pd.DataFrame(columns), weights="household_weight") + household = weighted_entity_frame(columns) return SimpleNamespace( output_dataset=SimpleNamespace(data=SimpleNamespace(household=household)) ) diff --git a/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py b/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py index 647f30f89..9d95eab0b 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py @@ -8,7 +8,6 @@ from dataclasses import dataclass from types import SimpleNamespace -import pandas as pd import pytest from fixtures.macro_output import ( CURRENT_SINGLE_YEAR_MACRO_KEYS, @@ -29,6 +28,7 @@ FakeModelOutput, fake_analysis, ) +from fixtures.weighted_frames import weighted_entity_frame from policyengine_simulation_contract.uk_geography import ( UKLocalAuthorityBoundaryVersion, UKLocalAuthorityMetadata, @@ -175,25 +175,33 @@ def _with_observability_timings(callback): def _macro_baseline_reform(): + """Build households whose weighted totals are the golden macro result's budget. + + With weights 0.5 and 1.5, the weighted totals are a tax change of 100, a + benefit change of 30, a state tax change of 20, 2 households and a baseline + net income of 1000. Unweighted sums of the same rows would give 120, 40, 24 + and 1200, and summing the weights through the weighted frame (#767) would + give 2.5 households, so the golden result only matches a correct build. + """ baseline = _FakeSimulation( - pd.DataFrame( + weighted_entity_frame( { - "household_weight": [1.0, 1.0], - "household_net_income": [400.0, 600.0], - "household_tax": [50.0, 50.0], + "household_weight": [0.5, 1.5], + "household_net_income": [800.0, 400.0], + "household_tax": [60.0, 40.0], "household_benefits": [20.0, 30.0], "household_state_income_tax": [5.0, 5.0], } ) ) reform = _FakeSimulation( - pd.DataFrame( + weighted_entity_frame( { - "household_weight": [1.0, 1.0], - "household_net_income": [410.0, 620.0], - "household_tax": [100.0, 100.0], - "household_benefits": [35.0, 45.0], - "household_state_income_tax": [15.0, 15.0], + "household_weight": [0.5, 1.5], + "household_net_income": [810.0, 420.0], + "household_tax": [140.0, 80.0], + "household_benefits": [50.0, 40.0], + "household_state_income_tax": [21.0, 13.0], } ) ) @@ -1228,7 +1236,9 @@ def test_run_simulation_impl_core_passes_detected_uk_boundary_to_builder(monkeyp dataset = SimpleNamespace( data=SimpleNamespace( entity_data={ - "household": pd.DataFrame({"la_code_oa": ["E06000001", "E07000026"]}) + "household": weighted_entity_frame( + {"la_code_oa": ["E06000001", "E07000026"]} + ) } ) ) @@ -1699,7 +1709,7 @@ def economic_impact_analysis( def test_builder_budgetary_impact_uses_materialized_columns_and_uk_state_tax_zero(): baseline = _FakeSimulation( - pd.DataFrame( + weighted_entity_frame( { "household_weight": [1.0, 2.0], "household_net_income": [100.0, 200.0], @@ -1710,7 +1720,7 @@ def test_builder_budgetary_impact_uses_materialized_columns_and_uk_state_tax_zer ) ) reform = _FakeSimulation( - pd.DataFrame( + weighted_entity_frame( { "household_weight": [1.0, 2.0], "household_net_income": [120.0, 210.0], @@ -1728,14 +1738,19 @@ def test_builder_budgetary_impact_uses_materialized_columns_and_uk_state_tax_zer "uk", baseline, reform )._build_budgetary_impact() + # Changes are weighted sums, weight * (reform - baseline), over the + # weights 1 and 2; the household count is a plain sum of the weights. + tax_revenue_impact = 1.0 * (25.0 - 20.0) + 2.0 * (50.0 - 40.0) + state_tax_revenue_impact = 1.0 * (4.0 - 2.0) + 2.0 * (6.0 - 3.0) + benefit_spending_impact = 1.0 * (4.0 - 5.0) + 2.0 * (8.0 - 10.0) assert isinstance(us_budget, BudgetaryImpact) assert us_budget.model_dump(mode="json") == { - "tax_revenue_impact": 15.0, - "state_tax_revenue_impact": 5.0, - "benefit_spending_impact": -3.0, - "budgetary_impact": 18.0, - "households": 3.0, - "baseline_net_income": 300.0, + "tax_revenue_impact": tax_revenue_impact, + "state_tax_revenue_impact": state_tax_revenue_impact, + "benefit_spending_impact": benefit_spending_impact, + "budgetary_impact": tax_revenue_impact - benefit_spending_impact, + "households": 1.0 + 2.0, + "baseline_net_income": 1.0 * 100.0 + 2.0 * 200.0, } assert uk_budget.state_tax_revenue_impact == 0.0 @@ -1758,7 +1773,7 @@ def fail_change_output_variable(*_args, **_kwargs): def test_uk_constituency_impact_uses_policyengine_output_function(monkeypatch): def coded_simulation() -> SimpleNamespace: - household = pd.DataFrame({"constituency_code_oa": ["E14001063"]}) + household = weighted_entity_frame({"constituency_code_oa": ["E14001063"]}) return SimpleNamespace( output_dataset=SimpleNamespace(data=SimpleNamespace(household=household)) ) diff --git a/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py b/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py index 8891d1bdf..5e5e20cdf 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_output_geographic.py @@ -5,8 +5,8 @@ from contextlib import nullcontext from types import SimpleNamespace -import pandas as pd import pytest +from fixtures.weighted_frames import weighted_entity_frame from policyengine.data.uk_geography_assets import CONSTITUENCY_ASSET_SPEC from policyengine_simulation_contract.uk_geography import ( UKLocalAuthorityBoundaryVersion, @@ -33,7 +33,7 @@ def _uk_simulation( code: str, income: float, ) -> SimpleNamespace: - household = pd.DataFrame( + household = weighted_entity_frame( { "la_code_oa": [code], "household_net_income": [income], @@ -214,7 +214,7 @@ def test_uk_geographic_outputs_are_never_dropped_for_missing_area_codes( "_required_uk_geography_lookup_csv_path", lambda _: pytest.fail("a dataset without area codes needs no lookup"), ) - household = pd.DataFrame( + household = weighted_entity_frame( { "region": ["LONDON"], "household_net_income": [100.0], From b7f54fa857619bd9f79ac195464d263b62170b19 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 15 Oct 2026 00:31:59 +0400 Subject: [PATCH 2/6] Register the weighted-microdata rules, with guards and proofs The executor's entity frames are MicroDataFrames weighted by _weight, so any column's sum is already weighted. #767 fixed a weight column aggregated through one, which returned the sum of squared weights. Four rules now cover that class of mistake: - sim.microdata.weights-not-values (doc-only): weights, values, labels and counts each allow different operations. - sim.microdata.weights-not-aggregated-as-values: a weight is never the variable of a weighted aggregator or frame[] followed by a weighted reduction. Enforced, with no findings. - sim.microdata.weight-stripping-confined: raw values are read from a weighted frame only in simulation_output_common.py. Ratcheted: the UK geography builders' plain DataFrame handoff (_output_household) is baselined for a decision. - sim.microdata.builder-tests-use-weighted-frames: the output-builder tests build entity frames only through the shared weighted factory. Enforced, with no findings. The AST guards live in the executor's tests, so the executor leg of the Test job runs them. Each rule's proof uses the real pre-#767 budget line or the mistake it mirrors. The rationale is in docs/engineering/skills/microdata-weights.md. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 11 + changelog_entry.yaml | 1 + docs/engineering/skills/README.md | 2 + docs/engineering/skills/microdata-weights.md | 81 +++ docs/generated/rules.md | 31 +- .../tests/test_microdata_weight_boundaries.py | 517 ++++++++++++++++++ .../sim-microdata-weight-stripping.txt | 10 + .../bad_test_simulation_output_budget.py | 32 ++ .../good_test_simulation_output_budget.py | 18 + .../bad_simulation_output_budget.py | 20 + .../good_simulation_output_budget.py | 14 + .../bad_simulation_output_budget.py | 41 ++ .../good_simulation_output_budget.py | 40 ++ rules/registry.toml | 90 +++ 14 files changed, 906 insertions(+), 2 deletions(-) create mode 100644 docs/engineering/skills/microdata-weights.md create mode 100644 projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py create mode 100644 rules/baselines/sim-microdata-weight-stripping.txt create mode 100644 rules/fixtures/sim.microdata.builder-tests-use-weighted-frames/bad_test_simulation_output_budget.py create mode 100644 rules/fixtures/sim.microdata.builder-tests-use-weighted-frames/good_test_simulation_output_budget.py create mode 100644 rules/fixtures/sim.microdata.weight-stripping-confined/bad_simulation_output_budget.py create mode 100644 rules/fixtures/sim.microdata.weight-stripping-confined/good_simulation_output_budget.py create mode 100644 rules/fixtures/sim.microdata.weights-not-aggregated-as-values/bad_simulation_output_budget.py create mode 100644 rules/fixtures/sim.microdata.weights-not-aggregated-as-values/good_simulation_output_budget.py diff --git a/AGENTS.md b/AGENTS.md index e767f9889..2cbd0e79e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,6 +18,10 @@ Before changing correlation identifiers, telemetry transport, spans, stage names, logging, or observability failure handling, read `docs/engineering/skills/observability.md`. +When changing code that aggregates simulation microdata (the output builders, +the national reduce, Stage 12 aggregation, or anything that reads +`output_dataset.data`), read `docs/engineering/skills/microdata-weights.md`. + When adding or changing a GitHub Actions workflow step or a script under `.github/scripts/`, read `docs/engineering/skills/ci-workflows.md`. @@ -138,6 +142,13 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. - `sim.observability.id-is-diagnostic-only`: observability_id has diagnostic meaning only: never use it for idempotency, authorization, database identity, routing or business logic. (doc-only, not checked: follow it yourself) - `sim.observability.registered-stages-only`: Span names come from the stage plans in policyengine_simulation_observability.stages; runtime code never adds span-name string literals. (doc-only, not checked: follow it yourself) +### Weighted microdata + +- `sim.microdata.builder-tests-use-weighted-frames`: Output-builder tests build entity frames only through fixtures/weighted_frames.weighted_entity_frame, as MicroDataFrames with non-uniform weights, never as plain pd.DataFrames. (blocking: pytest `projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py`) +- `sim.microdata.weight-stripping-confined`: Raw values are read from a weighted frame or column (np.asarray, np.array, .values, .to_numpy(), .tolist(), pd.DataFrame or pd.Series) only in simulation_output_common.py, the module that holds _count_output_entities. (blocking: pytest `projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py`) +- `sim.microdata.weights-not-aggregated-as-values`: A weight column is never aggregated as a value: never the variable of a weighted aggregator (_sum_output_variable, _change_output_variable, Aggregate, ChangeAggregate) or frame[] followed by a weighted reduction; count entities with _count_output_entities. (blocking: pytest `projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py`) +- `sim.microdata.weights-not-values`: Microdata columns are weights, values, labels or counts: weights are only summed plainly to count entities or passed as weights=, values are aggregated with their own entity's weights, labels only group or filter, and counts derive from weights. (doc-only, not checked: follow it yourself) + ### Code quality and tests - `sim.quality.ci-cd-scripts-ruff-exemptions`: CI/CD scripts under .github may print their output (T201); every other ruff rule applies to them. (blocking: ruff per-file ignore of T201 for `.github/**`) diff --git a/changelog_entry.yaml b/changelog_entry.yaml index 6be24041c..b35390972 100644 --- a/changelog_entry.yaml +++ b/changelog_entry.yaml @@ -1,6 +1,7 @@ - bump: patch changes: added: + - Added weighted-microdata rules to the registry, with proof from the #767 household count; the executor's code never aggregates a weight column as a value, reads raw values from weighted frames only in simulation_output_common.py (the UK geography builders' plain DataFrame handoff is baselined for a decision), and its output-builder tests build entity frames through one shared factory with non-uniform weights, while a doc-only rule records that weights, values, labels and counts each allow different operations - Made CLAUDE.md and the Copilot instructions thin pointers to AGENTS.md, required workflow logic longer than one line to live in tested and shellchecked scripts, and registered the tracking-issue rule - Added a rules registry (rules/registry.toml) with a standard-library tool that validates it, checks every rule is enforced where it claims, and renders docs/generated/rules.md and the Rules block in AGENTS.md changed: diff --git a/docs/engineering/skills/README.md b/docs/engineering/skills/README.md index 33f2bd188..5a6061631 100644 --- a/docs/engineering/skills/README.md +++ b/docs/engineering/skills/README.md @@ -15,6 +15,8 @@ Current skills: verification, and title conventions. - `testing.md`: test placement, dependency boundaries, and expected validation commands. +- `microdata-weights.md`: weights, values, labels and counts in simulation + microdata, and the guards that keep weights from being aggregated as values. - `observability.md`: identifier ownership, HTTP and Modal transport, persistence, registered runtime stages, trace boundaries, and failure isolation. diff --git a/docs/engineering/skills/microdata-weights.md b/docs/engineering/skills/microdata-weights.md new file mode 100644 index 000000000..72feca192 --- /dev/null +++ b/docs/engineering/skills/microdata-weights.md @@ -0,0 +1,81 @@ +# Microdata Weights Skill + +Read this before changing code that aggregates simulation microdata: the +`simulation_output_*.py` builders, `segmented_national_reduce` and +`stage12_runtime/aggregation`, or anything else that reads +`output_dataset.data` or builds a `MicroDataFrame`. + +## Weights aren't values + +A microdata column is one of four kinds, and each allows different operations +(rule `sim.microdata.weights-not-values`): + +| Kind | Examples | Allowed | Never | +| --- | --- | --- | --- | +| Weight | `household_weight`, `person_weight` | A plain (unweighted) sum, to count entities; use as `weights=` | Aggregation as a value through a weighted frame or aggregator | +| Value | `household_net_income`, `household_tax` | Weighted aggregation, with its own entity's weights | Aggregation after the weights are stripped | +| Label | `household_income_decile`, `age`, `is_male`, `race` | Grouping and filtering | Summing or averaging | +| Count | `households`, people per decile | Derived from weights | | + +The executor's entity frames are microdf `MicroDataFrame`s weighted by +`_weight`: policyengine builds them that way, and so do +`segmented_national_reduce._us_dataset_from_frames` and +`stage12_runtime/aggregation._dataset_from_frames` when they rebuild a dataset. +So `.sum()`, `.mean()` and `.count()` on any column are already weighted. + +In #766 (fixed by #767), `build_budgetary_impact` passed `"household_weight"` +to `_sum_output_variable`, the helper it uses for ordinary values. That +weighted the weight column by itself, giving the sum of squared weights, and +`budget.households` came out about 10^4 times too high on every run. It +escaped because the builder test used plain `pd.DataFrame`s, which carry no +weights. Count entities with `_count_output_entities` instead. + +## Rules + +- `sim.microdata.weights-not-aggregated-as-values`: a weight is never the + variable of a weighted aggregator (`_sum_output_variable`, + `_change_output_variable`, policyengine's `Aggregate` and `ChangeAggregate`) + or `[]` followed by a weighted reduction (`sum`, `mean`, + `median`, `quantile`, `std`, `count`, `groupby`, `cumsum`). +- `sim.microdata.weight-stripping-confined`: raw values are read from a + weighted frame or column (`np.asarray`, `np.array`, `.values`, `.to_numpy()`, + `.tolist()`, `pd.DataFrame(...)`, `pd.Series(...)`) only in + `simulation_output_common.py`, which holds `_count_output_entities`. Summing + stripped values gives an unweighted sample total, the mirror image of #767. +- `sim.microdata.builder-tests-use-weighted-frames`: the output-builder tests + (`tests/test_simulation_output_*.py`) build entity frames only through + `fixtures/weighted_frames.weighted_entity_frame`, which returns a + `MicroDataFrame` with non-uniform weights, never as a plain `pd.DataFrame`. + +The guards live in +`projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py` +and run in the executor's `Test` job. + +## How the guard recognises a weight + +The guards read the source, so they recognise names, not types: + +- a weight is a string or f-string ending in `_weight` or `_weights` (including + `f"{entity}_weight"`), a name assigned from one, or a `.weights` attribute; +- a weighted frame is `.output_dataset.data.` (or + `getattr(.output_dataset.data, ...)`), `_entity_data(...)`, a + `MicroDataFrame` or `MicroSeries` built with `weights=`, a name assigned from + one, and anything indexed, filtered or computed from those. + +They don't follow data through function calls or containers. A weight passed +through a helper escapes them, and so do whole-frame readers that iterate +`output_dataset.data.entity_data` (`simulation_microdata`, the Stage 12 +simulation worker and the precompute determinism check). + +## Handling a finding + +Each finding needs a decision, recorded in the PR: + +- **Fix it.** Count entities with `_count_output_entities`, and aggregate values + through the weighted frame or the aggregators. +- **Move it.** A deliberate strip moves into `simulation_output_common.py` with + a comment saying why, as `_count_output_entities` does. + +Existing weight-stripping findings are listed in +`rules/baselines/sim-microdata-weight-stripping.txt`. That list may only +shrink. diff --git a/docs/generated/rules.md b/docs/generated/rules.md index a3e37bf0d..887314518 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -8,11 +8,11 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf | Tier | enforced | ratcheted | advisory | doc-only | Total | | --- | ---: | ---: | ---: | ---: | ---: | -| mechanical | 40 | 1 | 0 | 14 | 55 | +| mechanical | 42 | 2 | 0 | 15 | 59 | | metric | 0 | 0 | 0 | 0 | 0 | | judgment | 0 | 0 | 0 | 0 | 0 | -**Blocking:** 41 of 55 rules. +**Blocking:** 44 of 59 rules. ## Gaps @@ -22,6 +22,7 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf - `sim.boundaries.unit-tests-need-no-services`: Unit tests never require real network credentials, Modal, Hugging Face, GCP or deployed services; those seams are mocked. - `sim.contracts.regenerate-clients`: When public API schemas change, regenerate the clients and run the generated-client integration tests. - `sim.deploy.image-smoke-covers-every-image-lib`: The image smoke path filter includes every library mounted into a Modal image, so a change to any of them runs the smoke. +- `sim.microdata.weights-not-values`: Microdata columns are weights, values, labels or counts: weights are only summed plainly to count entities or passed as weights=, values are aggregated with their own entity's weights, labels only group or filter, and counts derive from weights. - `sim.observability.failures-never-break-requests`: Invalid observability configuration fails at build or deploy time, and telemetry failures at runtime never fail a request or computation. - `sim.observability.header-transport-only`: observability_id travels only in the X-PolicyEngine-Observability-Id header, never in a simulation JSON body, and polling never invents one. - `sim.observability.id-is-diagnostic-only`: observability_id has diagnostic meaning only: never use it for idempotency, authorization, database identity, routing or business logic. @@ -65,6 +66,7 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf ### Ratcheted (existing violations still tolerated) - `sim.ci.multiline-run-is-script`: 13 remaining in `rules/baselines/ci-multiline-run.txt` +- `sim.microdata.weight-stripping-confined`: 1 remaining in `rules/baselines/sim-microdata-weight-stripping.txt` ### Exceptions and review dates @@ -242,6 +244,31 @@ flowchart LR r_sim_observability_registered_stages_only -.-> gap_unenforced ``` +## Weighted microdata + +| Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | +| --- | --- | --- | --- | --- | --- | --- | --- | +| `sim.microdata.builder-tests-use-weighted-frames` | Output-builder tests build entity frames only through fixtures/weighted_frames.weighted_entity_frame, as MicroDataFrames with non-uniform weights, never as plain pd.DataFrames. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py` | `pr.yml:test` | yes | +| `sim.microdata.weight-stripping-confined` | Raw values are read from a weighted frame or column (np.asarray, np.array, .values, .to_numpy(), .tolist(), pd.DataFrame or pd.Series) only in simulation_output_common.py, the module that holds _count_output_entities. | mechanical | ratcheted | yes | pytest `projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py` | `pr.yml:test` | yes | +| `sim.microdata.weights-not-aggregated-as-values` | A weight column is never aggregated as a value: never the variable of a weighted aggregator (_sum_output_variable, _change_output_variable, Aggregate, ChangeAggregate) or frame[] followed by a weighted reduction; count entities with _count_output_entities. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py` | `pr.yml:test` | yes | +| `sim.microdata.weights-not-values` | Microdata columns are weights, values, labels or counts: weights are only summed plainly to count entities or passed as weights=, values are aggregated with their own entity's weights, labels only group or filter, and counts derive from weights. | mechanical | doc-only | no | nothing | — | — | + +```mermaid +flowchart LR + r_sim_microdata_builder_tests_use_weighted_frames("sim.microdata.builder-tests-use-weighted-frames (enforced)") + e_pytest_projects_policyengine_simulation_executor_tests_test_microdata_weight_boundaries_py["pytest projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py"] + j_CI_pr_yml_test["CI pr.yml:test"] + r_sim_microdata_weight_stripping_confined("sim.microdata.weight-stripping-confined (ratcheted)") + r_sim_microdata_weights_not_aggregated_as_values("sim.microdata.weights-not-aggregated-as-values (enforced)") + r_sim_microdata_weights_not_values("sim.microdata.weights-not-values (doc-only)") + gap_unenforced>"not enforced"] + r_sim_microdata_builder_tests_use_weighted_frames --> e_pytest_projects_policyengine_simulation_executor_tests_test_microdata_weight_boundaries_py + e_pytest_projects_policyengine_simulation_executor_tests_test_microdata_weight_boundaries_py --> j_CI_pr_yml_test + r_sim_microdata_weight_stripping_confined --> e_pytest_projects_policyengine_simulation_executor_tests_test_microdata_weight_boundaries_py + r_sim_microdata_weights_not_aggregated_as_values --> e_pytest_projects_policyengine_simulation_executor_tests_test_microdata_weight_boundaries_py + r_sim_microdata_weights_not_values -.-> gap_unenforced +``` + ## Code quality and tests | Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | diff --git a/projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py b/projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py new file mode 100644 index 000000000..d535c6aab --- /dev/null +++ b/projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py @@ -0,0 +1,517 @@ +"""Weighted-microdata guards: weights aren't values. + +Microdata columns come in four kinds (weights, values, labels and counts), and +each allows different operations; docs/engineering/skills/microdata-weights.md +sets them out. These checks read the source, so they recognise a weight by its +name: a string or f-string ending in ``_weight`` or ``_weights``, a name +assigned from one, or a ``.weights`` attribute. They don't follow data through +function calls or containers. +""" + +from __future__ import annotations + +import ast +from collections import Counter +from dataclasses import dataclass, field +from functools import cache +from pathlib import Path + +PROJECT_ROOT = Path(__file__).resolve().parents[1] +REPOSITORY_ROOT = PROJECT_ROOT.parents[1] +SOURCE_ROOT = PROJECT_ROOT / "src" +TESTS_ROOT = PROJECT_ROOT / "tests" +FIXTURES_ROOT = REPOSITORY_ROOT / "rules" / "fixtures" +WEIGHT_STRIPPING_BASELINE = ( + REPOSITORY_ROOT / "rules" / "baselines" / "sim-microdata-weight-stripping.txt" +) +# The one module allowed to read raw values from weighted frames: it holds +# _count_output_entities, the plain sum of an entity's weights from #767. +WEIGHT_STRIPPING_MODULE = ( + SOURCE_ROOT / "policyengine_simulation_executor" / "simulation_output_common.py" +) +# Output-builder tests, which build entity frames only through the shared +# factory in fixtures/weighted_frames.py. +BUILDER_TEST_PATTERN = "test_simulation_output_*.py" + +WEIGHT_SUFFIXES = ("_weight", "_weights") +# Reductions that a weighted MicroDataFrame column weights. +WEIGHTED_REDUCTIONS = frozenset( + {"sum", "mean", "median", "quantile", "std", "count", "groupby", "cumsum"} +) +# Weighted aggregators, with the position and keyword of the argument each one +# aggregates as a value. +WEIGHTED_AGGREGATORS: dict[str, tuple[int | None, str]] = { + "_sum_output_variable": (1, "variable"), + "_change_output_variable": (2, "variable"), + "Aggregate": (None, "variable"), + "ChangeAggregate": (None, "variable"), +} +# The executor's accessor for one entity's weighted output frame. +ENTITY_FRAME_ACCESSOR = "_entity_data" +# Calls and attributes that return a weighted column's or frame's raw values. +STRIPPING_FUNCTIONS = frozenset( + { + "np.array", + "np.asarray", + "numpy.array", + "numpy.asarray", + "pd.DataFrame", + "pd.Series", + "pandas.DataFrame", + "pandas.Series", + } +) +STRIPPING_METHODS = frozenset({"to_numpy", "tolist"}) +STRIPPING_ATTRIBUTES = frozenset({"values"}) +# Methods whose result is weighted exactly as their receiver is. +WEIGHTING_PRESERVING_METHODS = frozenset( + {"abs", "astype", "clip", "copy", "fillna", "round"} +) +# Entity frame constructors that builder tests reach only through the factory. +ENTITY_FRAME_CONSTRUCTORS = frozenset( + { + "DataFrame", + "MicroDataFrame", + "microdf.MicroDataFrame", + "pd.DataFrame", + "pandas.DataFrame", + } +) + +AGGREGATION = "weight aggregated as a value" +STRIPPING = "weights stripped" +UNWEIGHTED_TEST_FRAME = "builder test frame built outside the weighted factory" + + +@dataclass(frozen=True) +class Finding: + """One place a check flags, with a line-independent baseline entry.""" + + check: str + path: str + line: int + function: str + source: str + + @property + def entry(self) -> str: + """Return the baseline entry: path, enclosing function and expression.""" + return f"{self.path}::{self.function}::{self.source}" + + def __str__(self) -> str: + """Describe the finding with its line, for assertion messages.""" + return f"{self.path}:{self.line} ({self.function}): {self.source}" + + +@dataclass +class Scope: + """The names a module or function assigns, sorted by what they hold.""" + + weight_keys: set[str] = field(default_factory=set) + weight_columns: set[str] = field(default_factory=set) + weight_arrays: set[str] = field(default_factory=set) + weighted: set[str] = field(default_factory=set) + + def child(self) -> Scope: + """Return a nested function's scope, which starts with these names.""" + return Scope( + weight_keys=set(self.weight_keys), + weight_columns=set(self.weight_columns), + weight_arrays=set(self.weight_arrays), + weighted=set(self.weighted), + ) + + +def _dotted_name(node: ast.expr) -> str: + if isinstance(node, ast.Name): + return node.id + if isinstance(node, ast.Attribute): + return f"{_dotted_name(node.value)}.{node.attr}" + return "" + + +def _called_name(call: ast.Call) -> str: + if isinstance(call.func, ast.Name): + return call.func.id + if isinstance(call.func, ast.Attribute): + return call.func.attr + return "" + + +def _keyword(call: ast.Call, name: str) -> ast.expr | None: + for keyword in call.keywords: + if keyword.arg == name: + return keyword.value + return None + + +def _argument(call: ast.Call, position: int | None, name: str) -> ast.expr | None: + if position is not None and len(call.args) > position: + return call.args[position] + return _keyword(call, name) + + +def _names_a_weight(node: ast.expr) -> bool: + """Whether a string or f-string literal ends in ``_weight`` or ``_weights``.""" + if isinstance(node, ast.Constant) and isinstance(node.value, str): + return node.value.endswith(WEIGHT_SUFFIXES) + if isinstance(node, ast.JoinedStr) and node.values: + tail = node.values[-1] + return ( + isinstance(tail, ast.Constant) + and isinstance(tail.value, str) + and tail.value.endswith(WEIGHT_SUFFIXES) + ) + return False + + +def _is_weight_key(node: ast.expr, scope: Scope) -> bool: + if isinstance(node, ast.Name): + return node.id in scope.weight_keys + return _names_a_weight(node) + + +def _is_weight_column(node: ast.expr, scope: Scope) -> bool: + """Whether the expression is ``[]``, or a name assigned one.""" + if isinstance(node, ast.Name): + return node.id in scope.weight_columns + return isinstance(node, ast.Subscript) and _is_weight_key(node.slice, scope) + + +def _is_weight_array(node: ast.expr, scope: Scope) -> bool: + if isinstance(node, ast.Name): + return node.id in scope.weight_arrays + return isinstance(node, ast.Attribute) and node.attr == "weights" + + +def _is_weight(node: ast.expr, scope: Scope) -> bool: + return ( + _is_weight_key(node, scope) + or _is_weight_column(node, scope) + or _is_weight_array(node, scope) + ) + + +def _is_output_data(node: ast.expr) -> bool: + """Whether the expression is ``.output_dataset.data``.""" + return ( + isinstance(node, ast.Attribute) + and node.attr == "data" + and isinstance(node.value, ast.Attribute) + and node.value.attr == "output_dataset" + ) + + +def _is_weighted_call(call: ast.Call, scope: Scope) -> bool: + name = _called_name(call) + if name == ENTITY_FRAME_ACCESSOR: + return True + if name == "getattr" and call.args: + return _is_output_data(call.args[0]) + if name in {"MicroSeries", "MicroDataFrame"}: + weights = _keyword(call, "weights") + return weights is not None and not ( + isinstance(weights, ast.Constant) and weights.value is None + ) + return ( + isinstance(call.func, ast.Attribute) + and call.func.attr in WEIGHTING_PRESERVING_METHODS + and _is_weighted(call.func.value, scope) + ) + + +def _operands(node: ast.expr) -> list[ast.expr]: + """Return the operands whose weighting an operator's result keeps.""" + if isinstance(node, ast.BinOp): + return [node.left, node.right] + if isinstance(node, ast.UnaryOp): + return [node.operand] + if isinstance(node, ast.Compare): + return [node.left, *node.comparators] + if isinstance(node, ast.BoolOp): + return list(node.values) + if isinstance(node, ast.IfExp): + return [node.body, node.orelse] + return [] + + +def _is_weighted(node: ast.expr, scope: Scope) -> bool: + """Whether the expression holds a weighted frame or column. + + That is an entity's output frame (``.output_dataset.data.`` + or ``_entity_data(...)``), a MicroDataFrame or MicroSeries built with + weights, or anything indexed, filtered or computed from one. + """ + if isinstance(node, ast.Name): + return node.id in scope.weighted + if isinstance(node, ast.Attribute): + return _is_output_data(node.value) + if isinstance(node, ast.Call): + return _is_weighted_call(node, scope) + if isinstance(node, ast.Subscript): + receiver = node.value + if isinstance(receiver, ast.Attribute) and receiver.attr in {"loc", "iloc"}: + receiver = receiver.value + return _is_weighted(receiver, scope) + return any(_is_weighted(operand, scope) for operand in _operands(node)) + + +def _assignments(body: list[ast.stmt]) -> list[tuple[str, ast.expr]]: + """Return each ``name = value`` in a body, outside nested functions and classes.""" + assignments: list[tuple[str, ast.expr]] = [] + pending: list[ast.AST] = list(body) + while pending: + node = pending.pop() + if isinstance( + node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef, ast.Lambda) + ): + continue + if isinstance(node, ast.Assign): + assignments.extend( + (target.id, node.value) + for target in node.targets + if isinstance(target, ast.Name) + ) + elif ( + isinstance(node, (ast.AnnAssign, ast.NamedExpr)) + and isinstance(node.target, ast.Name) + and node.value is not None + ): + assignments.append((node.target.id, node.value)) + pending.extend(ast.iter_child_nodes(node)) + return assignments + + +def _with_assignments(scope: Scope, body: list[ast.stmt]) -> Scope: + """Sort the body's assigned names into the scope, until nothing changes.""" + assignments = _assignments(body) + changed = True + while changed: + changed = False + for name, value in assignments: + groups = ( + (scope.weight_keys, _is_weight_key(value, scope)), + (scope.weight_columns, _is_weight_column(value, scope)), + (scope.weight_arrays, _is_weight_array(value, scope)), + (scope.weighted, _is_weighted(value, scope)), + ) + for names, matches in groups: + if matches and name not in names: + names.add(name) + changed = True + return scope + + +class _MicrodataWalker: + """Walk a module with each function's scope and collect every finding.""" + + def __init__(self, relative_path: str) -> None: + self.relative_path = relative_path + self.findings: list[Finding] = [] + self._called_functions: set[int] = set() + + def walk(self, tree: ast.Module) -> None: + self._walk(tree, _with_assignments(Scope(), tree.body), "") + + def _walk(self, node: ast.AST, scope: Scope, function: str) -> None: + for child in ast.iter_child_nodes(node): + if isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + name = ( + child.name if function == "" else f"{function}.{child.name}" + ) + if isinstance(child, ast.ClassDef): + self._walk(child, scope, name) + else: + self._walk( + child, _with_assignments(scope.child(), child.body), name + ) + continue + self._check(child, scope, function) + self._walk(child, scope, function) + + def _report(self, check: str, node: ast.expr, function: str) -> None: + self.findings.append( + Finding( + check=check, + path=self.relative_path, + line=node.lineno, + function=function, + source=ast.unparse(node), + ) + ) + + def _check(self, node: ast.AST, scope: Scope, function: str) -> None: + if isinstance(node, ast.Call): + self._called_functions.add(id(node.func)) + self._check_aggregation(node, scope, function) + self._check_stripping_call(node, scope, function) + self._check_test_frame(node, function) + elif ( + isinstance(node, ast.Attribute) + and node.attr in STRIPPING_ATTRIBUTES + and id(node) not in self._called_functions + and _is_weighted(node.value, scope) + ): + self._report(STRIPPING, node, function) + + def _check_aggregation(self, call: ast.Call, scope: Scope, function: str) -> None: + name = _called_name(call) + aggregated: ast.expr | None = None + if name in WEIGHTED_AGGREGATORS: + aggregated = _argument(call, *WEIGHTED_AGGREGATORS[name]) + elif name == "MicroSeries" and _keyword(call, "weights") is not None: + aggregated = _argument(call, 0, "data") + if aggregated is not None and _is_weight(aggregated, scope): + self._report(AGGREGATION, call, function) + return + if ( + isinstance(call.func, ast.Attribute) + and call.func.attr in WEIGHTED_REDUCTIONS + and _is_weight_column(call.func.value, scope) + ): + self._report(AGGREGATION, call, function) + + def _check_stripping_call( + self, call: ast.Call, scope: Scope, function: str + ) -> None: + if _dotted_name(call.func) in STRIPPING_FUNCTIONS: + stripped = _argument(call, 0, "data") + elif ( + isinstance(call.func, ast.Attribute) and call.func.attr in STRIPPING_METHODS + ): + stripped = call.func.value + else: + return + if stripped is not None and _is_weighted(stripped, scope): + self._report(STRIPPING, call, function) + + def _check_test_frame(self, call: ast.Call, function: str) -> None: + name = _dotted_name(call.func) + constructor = ( + name.rsplit(".", 1)[0] + if name.endswith((".from_dict", ".from_records")) + else name + ) + if constructor in ENTITY_FRAME_CONSTRUCTORS: + self._report(UNWEIGHTED_TEST_FRAME, call, function) + + +@cache +def _module_findings(path: Path) -> tuple[Finding, ...]: + walker = _MicrodataWalker(path.relative_to(REPOSITORY_ROOT).as_posix()) + walker.walk(ast.parse(path.read_text(encoding="utf-8"))) + return tuple(walker.findings) + + +def _findings(paths: list[Path], check: str) -> list[Finding]: + assert paths, f"no modules to check for {check}" + return [ + finding + for path in paths + for finding in _module_findings(path) + if finding.check == check + ] + + +def _source_modules() -> list[Path]: + return sorted(SOURCE_ROOT.rglob("*.py")) + + +def _fixture_sources(rule_id: str, fixture: str, check: str) -> list[str]: + path = FIXTURES_ROOT / rule_id / fixture + return [ + finding.source for finding in _module_findings(path) if finding.check == check + ] + + +def _baseline_entries(path: Path) -> Counter[str]: + """Read a baseline file: one entry per line, skipping blanks and ``#`` comments.""" + return Counter( + line.strip() + for line in path.read_text(encoding="utf-8").splitlines() + if line.strip() and not line.lstrip().startswith("#") + ) + + +def _described(findings: list[Finding]) -> str: + return "\n".join(str(finding) for finding in findings) + + +def test_weights_are_never_aggregated_as_values() -> None: + findings = _findings(_source_modules(), AGGREGATION) + assert findings == [], ( + "a weight is aggregated as a value, which weights it by itself (the sum " + "of squared weights, as in #767); count entities with " + "_count_output_entities instead:\n" + _described(findings) + ) + + +def test_weight_stripping_stays_in_the_designated_module() -> None: + assert WEIGHT_STRIPPING_MODULE.is_file() + designated = WEIGHT_STRIPPING_MODULE.relative_to(REPOSITORY_ROOT).as_posix() + findings = [ + finding + for finding in _findings(_source_modules(), STRIPPING) + if finding.path != designated + ] + found = Counter(finding.entry for finding in findings) + baseline = _baseline_entries(WEIGHT_STRIPPING_BASELINE) + new = [finding for finding in findings if finding.entry in found - baseline] + assert new == [], ( + "raw values read from a weighted frame outside " + f"{WEIGHT_STRIPPING_MODULE.name}; an unweighted total of them is a sample " + "total, the mirror image of #767. Keep the weights, or decide with a " + "reviewer to move the read into the designated module:\n" + _described(new) + ) + fixed = sorted((baseline - found).elements()) + assert fixed == [], ( + "these entries no longer apply; remove them from " + f"{WEIGHT_STRIPPING_BASELINE.name} and lower the rule's baseline count " + "in rules/registry.toml:\n" + "\n".join(fixed) + ) + + +def test_builder_tests_build_entity_frames_with_weights() -> None: + findings = _findings( + sorted(TESTS_ROOT.glob(BUILDER_TEST_PATTERN)), UNWEIGHTED_TEST_FRAME + ) + assert findings == [], ( + "output-builder tests build entity frames only with " + "fixtures.weighted_frames.weighted_entity_frame, which weights them as " + "policyengine does; a plain DataFrame hid #767:\n" + _described(findings) + ) + + +def test_rule_proof_weights_not_aggregated_as_values() -> None: + """Proof for rule sim.microdata.weights-not-aggregated-as-values.""" + rule_id = "sim.microdata.weights-not-aggregated-as-values" + bad = "bad_simulation_output_budget.py" + good = "good_simulation_output_budget.py" + assert _fixture_sources(rule_id, bad, AGGREGATION) == [ + "_sum_output_variable(baseline, 'household_weight', entity='household')" + ] + assert _fixture_sources(rule_id, good, AGGREGATION) == [] + + +def test_rule_proof_weight_stripping_confined() -> None: + """Proof for rule sim.microdata.weight-stripping-confined.""" + rule_id = "sim.microdata.weight-stripping-confined" + bad = "bad_simulation_output_budget.py" + good = "good_simulation_output_budget.py" + assert _fixture_sources(rule_id, bad, STRIPPING) == [ + "np.asarray(household['household_net_income'])" + ] + assert _fixture_sources(rule_id, good, STRIPPING) == [] + + +def test_rule_proof_builder_tests_use_weighted_frames() -> None: + """Proof for rule sim.microdata.builder-tests-use-weighted-frames.""" + rule_id = "sim.microdata.builder-tests-use-weighted-frames" + bad = "bad_test_simulation_output_budget.py" + good = "good_test_simulation_output_budget.py" + assert _fixture_sources(rule_id, bad, UNWEIGHTED_TEST_FRAME) == [ + "pd.DataFrame(columns)", + "MicroDataFrame(pd.DataFrame(columns), weights='household_weight')", + "pd.DataFrame(columns)", + ] + assert _fixture_sources(rule_id, good, UNWEIGHTED_TEST_FRAME) == [] diff --git a/rules/baselines/sim-microdata-weight-stripping.txt b/rules/baselines/sim-microdata-weight-stripping.txt new file mode 100644 index 000000000..d687e2f43 --- /dev/null +++ b/rules/baselines/sim-microdata-weight-stripping.txt @@ -0,0 +1,10 @@ +# Tolerated existing violations. This list may only shrink. +# +# Rule sim.microdata.weight-stripping-confined. Each entry is +# ::::, and each needs a decision: keep the +# weights, or move the read into simulation_output_common.py with a reason. +# +# The UK geography builders hand policyengine's longwise geography impact a +# plain DataFrame of the household frame, which weights by its +# household_weight column itself. +projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_geographic.py::_output_household::pd.DataFrame(simulation.output_dataset.data.household) diff --git a/rules/fixtures/sim.microdata.builder-tests-use-weighted-frames/bad_test_simulation_output_budget.py b/rules/fixtures/sim.microdata.builder-tests-use-weighted-frames/bad_test_simulation_output_budget.py new file mode 100644 index 000000000..ca0a16323 --- /dev/null +++ b/rules/fixtures/sim.microdata.builder-tests-use-weighted-frames/bad_test_simulation_output_budget.py @@ -0,0 +1,32 @@ +"""Proof fixture: builder-test households built outside the weighted factory. + +The rule must reject it. ``plain_simulation`` is how the builder test that +missed #767 built its households: a plain ``pd.DataFrame`` carries no weights, +so summing ``household_weight`` through it counted households correctly while +production's weighted frames returned the sum of squared weights. +``hand_weighted_simulation`` weights its frame, but by hand rather than through +the one shared factory. +""" + +from __future__ import annotations + +from types import SimpleNamespace + +import pandas as pd +from microdf import MicroDataFrame + + +def plain_simulation(columns: dict[str, list[float]]) -> SimpleNamespace: + """Build a simulation whose household output is a plain DataFrame.""" + household = pd.DataFrame(columns) + return SimpleNamespace( + output_dataset=SimpleNamespace(data=SimpleNamespace(household=household)) + ) + + +def hand_weighted_simulation(columns: dict[str, list[float]]) -> SimpleNamespace: + """Build a simulation whose household output is weighted by hand.""" + household = MicroDataFrame(pd.DataFrame(columns), weights="household_weight") + return SimpleNamespace( + output_dataset=SimpleNamespace(data=SimpleNamespace(household=household)) + ) diff --git a/rules/fixtures/sim.microdata.builder-tests-use-weighted-frames/good_test_simulation_output_budget.py b/rules/fixtures/sim.microdata.builder-tests-use-weighted-frames/good_test_simulation_output_budget.py new file mode 100644 index 000000000..b5928b4ca --- /dev/null +++ b/rules/fixtures/sim.microdata.builder-tests-use-weighted-frames/good_test_simulation_output_budget.py @@ -0,0 +1,18 @@ +"""Proof fixture: builder-test households from the weighted factory. + +The rule must accept it. +""" + +from __future__ import annotations + +from types import SimpleNamespace + +from fixtures.weighted_frames import weighted_entity_frame + + +def weighted_simulation(columns: dict[str, list[float]]) -> SimpleNamespace: + """Build a simulation whose household output is weighted as policyengine's is.""" + household = weighted_entity_frame(columns) + return SimpleNamespace( + output_dataset=SimpleNamespace(data=SimpleNamespace(household=household)) + ) diff --git a/rules/fixtures/sim.microdata.weight-stripping-confined/bad_simulation_output_budget.py b/rules/fixtures/sim.microdata.weight-stripping-confined/bad_simulation_output_budget.py new file mode 100644 index 000000000..26b5158e4 --- /dev/null +++ b/rules/fixtures/sim.microdata.weight-stripping-confined/bad_simulation_output_budget.py @@ -0,0 +1,20 @@ +"""Proof fixture: weights stripped before a total. The rule must reject it. + +Summing the raw values of a weighted column gives an unweighted sample total, +the mirror image of #767. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import numpy as np + +if TYPE_CHECKING: + from policyengine.core import Simulation + + +def baseline_net_income(baseline: Simulation) -> float: + """Total household net income over the sample's records, ignoring weights.""" + household = baseline.output_dataset.data.household + return float(np.asarray(household["household_net_income"]).sum()) diff --git a/rules/fixtures/sim.microdata.weight-stripping-confined/good_simulation_output_budget.py b/rules/fixtures/sim.microdata.weight-stripping-confined/good_simulation_output_budget.py new file mode 100644 index 000000000..68a0c1c05 --- /dev/null +++ b/rules/fixtures/sim.microdata.weight-stripping-confined/good_simulation_output_budget.py @@ -0,0 +1,14 @@ +"""Proof fixture: a total that keeps its weights. The rule must accept it.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from policyengine.core import Simulation + + +def baseline_net_income(baseline: Simulation) -> float: + """Total household net income, weighted by the household frame's weights.""" + household = baseline.output_dataset.data.household + return float(household["household_net_income"].sum()) diff --git a/rules/fixtures/sim.microdata.weights-not-aggregated-as-values/bad_simulation_output_budget.py b/rules/fixtures/sim.microdata.weights-not-aggregated-as-values/bad_simulation_output_budget.py new file mode 100644 index 000000000..180c7b16e --- /dev/null +++ b/rules/fixtures/sim.microdata.weights-not-aggregated-as-values/bad_simulation_output_budget.py @@ -0,0 +1,41 @@ +"""Proof fixture: a weight aggregated as a value. The rule must reject it. + +This is the budget builder before #767. The household frame is a MicroDataFrame +weighted by ``household_weight``, so summing ``household_weight`` through it +returned the sum of squared weights, about 10^4 times the household count. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from policyengine_simulation_executor.simulation_macro_output import BudgetaryImpact +from policyengine_simulation_executor.simulation_output_common import ( + _change_output_variable, + _sum_output_variable, +) + +if TYPE_CHECKING: + from policyengine.core import Simulation + + +def build_budgetary_impact(baseline: Simulation, reform: Simulation) -> BudgetaryImpact: + """Compute headline budget figures as the builder did before #767.""" + tax_revenue_impact = _change_output_variable( + baseline, reform, "household_tax", entity="household" + ) + benefit_spending_impact = _change_output_variable( + baseline, reform, "household_benefits", entity="household" + ) + return BudgetaryImpact( + tax_revenue_impact=tax_revenue_impact, + state_tax_revenue_impact=0.0, + benefit_spending_impact=benefit_spending_impact, + budgetary_impact=tax_revenue_impact - benefit_spending_impact, + households=_sum_output_variable( + baseline, "household_weight", entity="household" + ), + baseline_net_income=_sum_output_variable( + baseline, "household_net_income", entity="household" + ), + ) diff --git a/rules/fixtures/sim.microdata.weights-not-aggregated-as-values/good_simulation_output_budget.py b/rules/fixtures/sim.microdata.weights-not-aggregated-as-values/good_simulation_output_budget.py new file mode 100644 index 000000000..9245f9f4a --- /dev/null +++ b/rules/fixtures/sim.microdata.weights-not-aggregated-as-values/good_simulation_output_budget.py @@ -0,0 +1,40 @@ +"""Proof fixture: households counted with a plain sum of their weights. + +The rule must accept it. This is the budget builder with #767's replacement. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from policyengine_simulation_executor.simulation_macro_output import BudgetaryImpact +from policyengine_simulation_executor.simulation_output_common import ( + _change_output_variable, + _count_output_entities, + _sum_output_variable, +) + +if TYPE_CHECKING: + from policyengine.core import Simulation + + +def build_budgetary_impact(baseline: Simulation, reform: Simulation) -> BudgetaryImpact: + """Compute headline budget figures as the builder does since #767.""" + tax_revenue_impact = _change_output_variable( + baseline, reform, "household_tax", entity="household" + ) + benefit_spending_impact = _change_output_variable( + baseline, reform, "household_benefits", entity="household" + ) + return BudgetaryImpact( + tax_revenue_impact=tax_revenue_impact, + state_tax_revenue_impact=0.0, + benefit_spending_impact=benefit_spending_impact, + budgetary_impact=tax_revenue_impact - benefit_spending_impact, + households=_count_output_entities( + baseline, entity="household", weight="household_weight" + ), + baseline_net_income=_sum_output_variable( + baseline, "household_net_income", entity="household" + ), + ) diff --git a/rules/registry.toml b/rules/registry.toml index 320d9236c..13d57c726 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -31,6 +31,10 @@ title = "API contracts and integration" id = "observability" title = "Observability" +[[category]] +id = "microdata" +title = "Weighted microdata" + [[category]] id = "quality" title = "Code quality and tests" @@ -692,6 +696,92 @@ autofix = false [rule.enforcer] kind = "none" +# --- Weighted microdata --------------------------------------------------- + +[[rule]] +id = "sim.microdata.weights-not-values" +category = "microdata" +statement = "Microdata columns are weights, values, labels or counts: weights are only summed plainly to count entities or passed as weights=, values are aggregated with their own entity's weights, labels only group or filter, and counts derive from weights." +rationale = "docs/engineering/skills/microdata-weights.md#weights-arent-values" +scope = ["projects/policyengine-simulation-executor/src/**/*.py"] +tier = "mechanical" +status = "doc-only" +autofix = false +[rule.enforcer] +kind = "none" + +[[rule]] +id = "sim.microdata.weights-not-aggregated-as-values" +category = "microdata" +statement = "A weight column is never aggregated as a value: never the variable of a weighted aggregator (_sum_output_variable, _change_output_variable, Aggregate, ChangeAggregate) or frame[] followed by a weighted reduction; count entities with _count_output_entities." +rationale = "docs/engineering/skills/microdata-weights.md#rules" +scope = ["projects/policyengine-simulation-executor/src/**/*.py"] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "pytest" +project = "projects/policyengine-simulation-executor" +nodes = ["tests/test_microdata_weight_boundaries.py::test_weights_are_never_aggregated_as_values"] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "test" +invocation = "uv run pytest tests/" +[rule.proof] +bad_fixture = "rules/fixtures/sim.microdata.weights-not-aggregated-as-values/bad_simulation_output_budget.py" +good_fixture = "rules/fixtures/sim.microdata.weights-not-aggregated-as-values/good_simulation_output_budget.py" +project = "projects/policyengine-simulation-executor" +test = "tests/test_microdata_weight_boundaries.py::test_rule_proof_weights_not_aggregated_as_values" + +[[rule]] +id = "sim.microdata.weight-stripping-confined" +category = "microdata" +statement = "Raw values are read from a weighted frame or column (np.asarray, np.array, .values, .to_numpy(), .tolist(), pd.DataFrame or pd.Series) only in simulation_output_common.py, the module that holds _count_output_entities." +rationale = "docs/engineering/skills/microdata-weights.md#handling-a-finding" +scope = ["projects/policyengine-simulation-executor/src/**/*.py"] +tier = "mechanical" +status = "ratcheted" +autofix = false +[rule.enforcer] +kind = "pytest" +project = "projects/policyengine-simulation-executor" +nodes = ["tests/test_microdata_weight_boundaries.py::test_weight_stripping_stays_in_the_designated_module"] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "test" +invocation = "uv run pytest tests/" +[rule.proof] +bad_fixture = "rules/fixtures/sim.microdata.weight-stripping-confined/bad_simulation_output_budget.py" +good_fixture = "rules/fixtures/sim.microdata.weight-stripping-confined/good_simulation_output_budget.py" +project = "projects/policyengine-simulation-executor" +test = "tests/test_microdata_weight_boundaries.py::test_rule_proof_weight_stripping_confined" +[rule.baseline] +file = "rules/baselines/sim-microdata-weight-stripping.txt" +count = 1 + +[[rule]] +id = "sim.microdata.builder-tests-use-weighted-frames" +category = "microdata" +statement = "Output-builder tests build entity frames only through fixtures/weighted_frames.weighted_entity_frame, as MicroDataFrames with non-uniform weights, never as plain pd.DataFrames." +rationale = "docs/engineering/skills/microdata-weights.md#rules" +scope = ["projects/policyengine-simulation-executor/tests/test_simulation_output_*.py"] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "pytest" +project = "projects/policyengine-simulation-executor" +nodes = ["tests/test_microdata_weight_boundaries.py::test_builder_tests_build_entity_frames_with_weights"] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "test" +invocation = "uv run pytest tests/" +[rule.proof] +bad_fixture = "rules/fixtures/sim.microdata.builder-tests-use-weighted-frames/bad_test_simulation_output_budget.py" +good_fixture = "rules/fixtures/sim.microdata.builder-tests-use-weighted-frames/good_test_simulation_output_budget.py" +project = "projects/policyengine-simulation-executor" +test = "tests/test_microdata_weight_boundaries.py::test_rule_proof_builder_tests_use_weighted_frames" + # --- Code quality and tests ------------------------------------------------ [[rule]] From 3ce71a938f9935d66767adf8211285ba71b9983a Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 15 Oct 2026 00:33:48 +0400 Subject: [PATCH 3/6] Warn when change totals are weighted differently on each side _change_output_variable sums reform minus baseline through the entity frames. MicroSeries arithmetic keeps the left operand's weights, so that change is weighted by the reform's weights alone, and a mismatch between the two sides would pass silently. It now logs a warning unless both frames carry the same weights, allowing only a float32 round trip (relative tolerance 1e-6). Frames without weights are not compared. The total is still published. Making the mismatch an error waits until logs show that no legitimate run produces one. Co-Authored-By: Claude Opus 5.5 --- changelog_entry.yaml | 1 + .../simulation_output_common.py | 40 ++++++++++++ .../tests/test_simulation_output_budget.py | 65 +++++++++++++++++++ 3 files changed, 106 insertions(+) diff --git a/changelog_entry.yaml b/changelog_entry.yaml index b35390972..ec9a49575 100644 --- a/changelog_entry.yaml +++ b/changelog_entry.yaml @@ -1,6 +1,7 @@ - bump: patch changes: added: + - Made the budget's reform-minus-baseline totals log a warning when the baseline and reform household frames carry different weights, since the change is then weighted by the reform's weights alone - Added weighted-microdata rules to the registry, with proof from the #767 household count; the executor's code never aggregates a weight column as a value, reads raw values from weighted frames only in simulation_output_common.py (the UK geography builders' plain DataFrame handoff is baselined for a decision), and its output-builder tests build entity frames through one shared factory with non-uniform weights, while a doc-only rule records that weights, values, labels and counts each allow different operations - Made CLAUDE.md and the Copilot instructions thin pointers to AGENTS.md, required workflow logic longer than one line to live in tested and shellchecked scripts, and registered the tracking-issue rule - Added a rules registry (rules/registry.toml) with a standard-library tool that validates it, checks every rule is enforced where it claims, and renders docs/generated/rules.md and the Rules block in AGENTS.md diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_common.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_common.py index 4531b9542..ae5c73308 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_common.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_common.py @@ -19,6 +19,10 @@ logger = logging.getLogger(__name__) +# Weights read back from a stored artifact may have passed through float32, so +# "identical" allows a float32 round trip and nothing more. +WEIGHT_RELATIVE_TOLERANCE = 1e-6 + def _number(value: ConvertibleToFloat | None, default: float = 0.0) -> float: if value is None: @@ -73,6 +77,41 @@ def _count_output_entities(simulation: Simulation, entity: str, weight: str) -> return float(np.asarray(data[weight], dtype=float).sum()) +def _warn_on_mismatched_weights( + baseline_data: pd.DataFrame, reform_data: pd.DataFrame, entity: str +) -> None: + """Warn when a reform-minus-baseline total's two sides are weighted differently. + + MicroSeries arithmetic keeps the left operand's weights, so reform minus + baseline is weighted by the reform's weights alone. That change total + only means something when both sides weight the same records identically. + Plain frames, which carry no weights, have nothing to compare. The total + is still published: this warns rather than refuses until logs show that + no legitimate run produces a mismatch. + """ + baseline_weights = getattr(baseline_data, "weights", None) + reform_weights = getattr(reform_data, "weights", None) + if baseline_weights is None and reform_weights is None: + return + if ( + baseline_weights is None + or reform_weights is None + or len(baseline_weights) != len(reform_weights) + or not np.allclose( + np.asarray(baseline_weights, dtype=float), + np.asarray(reform_weights, dtype=float), + rtol=WEIGHT_RELATIVE_TOLERANCE, + atol=0.0, + equal_nan=True, + ) + ): + logger.warning( + "Baseline and reform %s frames carry different weights, so their " + "change total is weighted by the reform's weights alone", + entity, + ) + + def _sum_output_variable(simulation: Simulation, variable: str, entity: str) -> float: data = _entity_data(simulation, entity) if variable in data.columns: @@ -98,6 +137,7 @@ def _change_output_variable( ) -> float: baseline_data = _entity_data(baseline, entity) reform_data = _entity_data(reform, entity) + _warn_on_mismatched_weights(baseline_data, reform_data, entity) if variable in baseline_data.columns and variable in reform_data.columns: return float((reform_data[variable] - baseline_data[variable]).sum()) diff --git a/projects/policyengine-simulation-executor/tests/test_simulation_output_budget.py b/projects/policyengine-simulation-executor/tests/test_simulation_output_budget.py index 175361ef4..c1c81378c 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_output_budget.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_output_budget.py @@ -2,13 +2,18 @@ from __future__ import annotations +import logging from types import SimpleNamespace +import numpy as np +import pytest from fixtures.weighted_frames import weighted_entity_frame from policyengine_simulation_executor.simulation_output_budget import ( build_budgetary_impact, ) +COMMON_LOGGER = "policyengine_simulation_executor.simulation_output_common" + def _simulation(columns: dict[str, list[float]]): """Build a simulation whose household output is a policyengine MicroDataFrame.""" @@ -18,6 +23,18 @@ def _simulation(columns: dict[str, list[float]]): ) +def _households(weights: list[float], tax: list[float]): + return _simulation( + { + "household_weight": weights, + "household_net_income": [100.0, 200.0], + "household_tax": tax, + "household_benefits": [5.0, 10.0], + "household_state_income_tax": [2.0, 3.0], + } + ) + + def test_households_counts_each_weight_once_on_weighted_frames(): baseline = _simulation( { @@ -46,3 +63,51 @@ def test_households_counts_each_weight_once_on_weighted_frames(): assert budget.baseline_net_income == 500.0 assert budget.tax_revenue_impact == 25.0 assert budget.state_tax_revenue_impact == 8.0 + + +def _mismatched_weight_warnings(caplog: pytest.LogCaptureFixture) -> list[str]: + return [ + record.getMessage() + for record in caplog.records + if record.name == COMMON_LOGGER + and record.levelno == logging.WARNING + and "different weights" in record.getMessage() + ] + + +def test_change_totals_warn_when_baseline_and_reform_are_weighted_differently( + caplog: pytest.LogCaptureFixture, +): + baseline = _households([1.0, 2.0], tax=[20.0, 40.0]) + reform = _households([2.0, 1.0], tax=[25.0, 50.0]) + + with caplog.at_level(logging.WARNING, logger=COMMON_LOGGER): + budget = build_budgetary_impact("us", baseline, reform) + + # reform - baseline is weighted by the reform's weights alone, and the + # total is still published. + expected_change = 2.0 * (25.0 - 20.0) + 1.0 * (50.0 - 40.0) + assert budget.tax_revenue_impact == pytest.approx(expected_change) + warnings = _mismatched_weight_warnings(caplog) + assert warnings + assert all("household frames" in warning for warning in warnings) + + +def test_change_totals_accept_weights_after_a_float32_round_trip( + caplog: pytest.LogCaptureFixture, +): + weights = [1.1, 2.3] + stored_weights = np.asarray(weights, dtype=np.float32).astype(float).tolist() + baseline = _households(weights, tax=[20.0, 40.0]) + reform = _households(stored_weights, tax=[25.0, 50.0]) + + with caplog.at_level(logging.WARNING, logger=COMMON_LOGGER): + budget = build_budgetary_impact("us", baseline, reform) + + assert _mismatched_weight_warnings(caplog) == [] + + # The change is weighted by the reform's (round-tripped) weights. + expected_change = stored_weights[0] * (25.0 - 20.0) + stored_weights[1] * ( + 50.0 - 40.0 + ) + assert budget.tax_revenue_impact == pytest.approx(expected_change) From 298903af8717ea5224649d098092a8be0c03b865 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 15 Oct 2026 00:35:53 +0400 Subject: [PATCH 4/6] Refuse an implausible published household count budget.households is a plain sum of the household weights. #767's bug published the sum of squared weights instead, about 10^4 times too high (135.6 billion for New York). build_budgetary_impact now raises ImplausibleEntityCountError for a count that is negative, not finite, or above its country's bound: 200 million US households (about 131 million, US Census Bureau Current Population Survey 2023) or 60 million UK households (about 28 million, ONS Families and households 2023). Other countries only need a finite, non-negative count. Co-Authored-By: Claude Opus 5.5 --- changelog_entry.yaml | 1 + .../simulation_output_budget.py | 41 +++++++++++++-- ...test_simulation_output_household_bounds.py | 52 +++++++++++++++++++ 3 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 projects/policyengine-simulation-executor/tests/test_simulation_output_household_bounds.py diff --git a/changelog_entry.yaml b/changelog_entry.yaml index ec9a49575..71428b4e4 100644 --- a/changelog_entry.yaml +++ b/changelog_entry.yaml @@ -5,6 +5,7 @@ - Added weighted-microdata rules to the registry, with proof from the #767 household count; the executor's code never aggregates a weight column as a value, reads raw values from weighted frames only in simulation_output_common.py (the UK geography builders' plain DataFrame handoff is baselined for a decision), and its output-builder tests build entity frames through one shared factory with non-uniform weights, while a doc-only rule records that weights, values, labels and counts each allow different operations - Made CLAUDE.md and the Copilot instructions thin pointers to AGENTS.md, required workflow logic longer than one line to live in tested and shellchecked scripts, and registered the tracking-issue rule - Added a rules registry (rules/registry.toml) with a standard-library tool that validates it, checks every rule is enforced where it claims, and renders docs/generated/rules.md and the Rules block in AGENTS.md + - Made the budget refuse a household count outside its country's plausible range (above 200 million in the US or 60 million in the UK, negative or not finite), raising ImplausibleEntityCountError instead of publishing a count like #767's changed: - Enabled every ruff rule (select ALL), ignoring only COM812 and ISC001, which conflict with ruff format, and the D203 and D213 halves of the mutually exclusive docstring pairs; complex functions, long parameter lists and magic numbers were split into smaller helpers and grouped parameters, each behind new characterization tests, across the contract and persistence validation, entry settings, app factory and Stage 12 adapter, gateway route resolution, Stage 12 coordination, workers, parity and qualification, the precompute and national reduce, the routing publisher and its command line, the release sentinel, the package update helper and the shared rules tool; Modal function, FastAPI route, Pydantic validator and command-line signatures and every OpenAPI document are unchanged, and three library APIs that other projects import keep a reasoned PLR0913 suppression - Enabled ruff's argument, idiom and pytest-style rules (ARG, ASYNC, B, C4, FBT, PD, PERF, PGH, PLW, PT, PTH, RUF, SIM); unused arguments are removed, underscored or explained, FastAPI auth dependencies use Annotated, re-raised errors keep their cause, and boolean options of in-repo helpers are keyword-only; OpenAPI documents are unchanged, and the shared rules tool's unittest tests are exempt from pytest-style rules; tests take the unused-argument rules (ARG001, ARG002, ARG005) as file-level exceptions, since their fakes and stubs mirror real signatures; best-effort telemetry and cleanup calls that swallow any error use contextlib.suppress, with the reason as a comment diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_budget.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_budget.py index 1a3b17c64..b188fa230 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_budget.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_budget.py @@ -2,6 +2,7 @@ from __future__ import annotations +import math from typing import TYPE_CHECKING from policyengine_simulation_executor.simulation_macro_output import ( @@ -20,6 +21,17 @@ if TYPE_CHECKING: from policyengine.core import Simulation +# Upper bounds on the published household count. Each is well above any real +# run and far below the sum of squared weights that #767 published (about 10^4 +# times the count). The US has about 131 million households (US Census Bureau, +# Current Population Survey 2023) and the UK about 28 million (ONS, Families and +# households in the UK 2023). +MAX_PLAUSIBLE_HOUSEHOLDS = {"us": 2e8, "uk": 6e7} + + +class ImplausibleEntityCountError(ValueError): + """A published entity count is negative, not finite or above its bound.""" + def build_detailed_budget(analysis: object) -> DetailedBudgetOutput: """Return per-program baseline, reform and difference totals from the analysis. @@ -45,6 +57,23 @@ def build_detailed_budget(analysis: object) -> DetailedBudgetOutput: return DetailedBudgetOutput(detailed_budget) +def _require_plausible_households(country: str, households: float) -> None: + """Refuse a household count no real run of the country could produce. + + A count is a plain sum of weights, so it is never negative, and one far + above the country's population means the weights were aggregated as + values. A country without a bound only needs a finite, non-negative count. + """ + maximum = MAX_PLAUSIBLE_HOUSEHOLDS.get(country.lower(), math.inf) + if not (math.isfinite(households) and 0 <= households <= maximum): + msg = ( + f"{country.upper()} household count {households:,.0f} is outside the " + f"plausible range 0 to {maximum:,.0f}; check that the household " + "weights are summed plainly, not aggregated as values" + ) + raise ImplausibleEntityCountError(msg) + + def build_budgetary_impact( country: str, baseline: Simulation, reform: Simulation ) -> BudgetaryImpact: @@ -52,7 +81,8 @@ def build_budgetary_impact( ``budgetary_impact`` is the tax revenue change minus the benefit spending change. State income tax is only modelled for the US; elsewhere its impact - is 0.0. + is 0.0. A household count outside the country's plausible range raises + ``ImplausibleEntityCountError``. """ tax_revenue_impact = _change_output_variable( baseline, reform, "household_tax", entity="household" @@ -71,14 +101,17 @@ def build_budgetary_impact( else 0.0 ) + households = _count_output_entities( + baseline, entity="household", weight="household_weight" + ) + _require_plausible_households(country, households) + return BudgetaryImpact( tax_revenue_impact=tax_revenue_impact, state_tax_revenue_impact=state_tax_revenue_impact, benefit_spending_impact=benefit_spending_impact, budgetary_impact=tax_revenue_impact - benefit_spending_impact, - households=_count_output_entities( - baseline, entity="household", weight="household_weight" - ), + households=households, baseline_net_income=_sum_output_variable( baseline, "household_net_income", entity="household" ), diff --git a/projects/policyengine-simulation-executor/tests/test_simulation_output_household_bounds.py b/projects/policyengine-simulation-executor/tests/test_simulation_output_household_bounds.py new file mode 100644 index 000000000..1e4c40b06 --- /dev/null +++ b/projects/policyengine-simulation-executor/tests/test_simulation_output_household_bounds.py @@ -0,0 +1,52 @@ +"""Plausibility bounds on the published household count.""" + +from __future__ import annotations + +from types import SimpleNamespace + +import pytest +from fixtures.weighted_frames import weighted_entity_frame +from policyengine_simulation_executor.simulation_output_budget import ( + ImplausibleEntityCountError, + build_budgetary_impact, +) + + +def _simulation(weights: list[float]): + household = weighted_entity_frame( + { + "household_weight": weights, + "household_net_income": [100.0, 200.0], + "household_tax": [20.0, 40.0], + "household_benefits": [5.0, 10.0], + "household_state_income_tax": [2.0, 3.0], + } + ) + return SimpleNamespace( + output_dataset=SimpleNamespace(data=SimpleNamespace(household=household)) + ) + + +def test_households_above_the_country_bound_are_refused(): + # 250 million households is more than the US bound of 200 million. + simulation = _simulation([1.5e8, 1e8]) + + with pytest.raises(ImplausibleEntityCountError, match="US household count"): + build_budgetary_impact("us", simulation, simulation) + + +def test_household_bounds_are_set_per_country(): + # 70 million households is plausible for the US but not for the UK, whose + # bound is 60 million. + simulation = _simulation([4e7, 3e7]) + + assert build_budgetary_impact("us", simulation, simulation).households == 7e7 + with pytest.raises(ImplausibleEntityCountError, match="UK household count"): + build_budgetary_impact("uk", simulation, simulation) + + +def test_negative_household_counts_are_refused(): + simulation = _simulation([-3.0, 1.0]) + + with pytest.raises(ImplausibleEntityCountError, match="household count -2"): + build_budgetary_impact("us", simulation, simulation) From fbfe2007c48a7672d02a26314b7892221eff76e9 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 15 Oct 2026 01:31:18 +0400 Subject: [PATCH 5/6] Record the UK local-authority handoff as a ratcheted stripping exception The UK local-authority breakdown hands policyengine's compute_longwise_uk_geography_impacts plain household frames, so that it can apply this service's boundary metadata. That function weights each area by the baseline's household_weight itself, so the strip is deliberate, but it is still a strip to remove. The constituency breakdown already hands policyengine the simulations, as the US congressional district breakdown does, and only needed the plain frame to check for its area-code column, which the weighted frame answers just as well. - The handoff moves into simulation_output_common.py as _unweighted_household_frame, with a docstring saying why it exists and what removes it. The constituency check reads the weighted frame. - sim.microdata.weight-stripping-confined now checks the designated module too: counting entities is always allowed there, and every other read is a recorded exception in the rule's baseline, which may only shrink. Its one entry is _unweighted_household_frame. A new test pins that sorting. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- changelog_entry.yaml | 2 +- docs/engineering/skills/microdata-weights.md | 15 ++- docs/generated/rules.md | 2 +- .../simulation_output_common.py | 19 +++- .../simulation_output_geographic.py | 13 +-- .../tests/test_microdata_weight_boundaries.py | 95 ++++++++++++++++--- .../sim-microdata-weight-stripping.txt | 15 +-- rules/registry.toml | 7 +- 9 files changed, 131 insertions(+), 39 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2cbd0e79e..b81495eaf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -145,7 +145,7 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. ### Weighted microdata - `sim.microdata.builder-tests-use-weighted-frames`: Output-builder tests build entity frames only through fixtures/weighted_frames.weighted_entity_frame, as MicroDataFrames with non-uniform weights, never as plain pd.DataFrames. (blocking: pytest `projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py`) -- `sim.microdata.weight-stripping-confined`: Raw values are read from a weighted frame or column (np.asarray, np.array, .values, .to_numpy(), .tolist(), pd.DataFrame or pd.Series) only in simulation_output_common.py, the module that holds _count_output_entities. (blocking: pytest `projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py`) +- `sim.microdata.weight-stripping-confined`: Raw values are read from a weighted frame or column (np.asarray, np.array, .values, .to_numpy(), .tolist(), pd.DataFrame or pd.Series) only in simulation_output_common.py; there, counting entities is always allowed and every other read is a recorded exception whose list may only shrink. (blocking: pytest `projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py`) - `sim.microdata.weights-not-aggregated-as-values`: A weight column is never aggregated as a value: never the variable of a weighted aggregator (_sum_output_variable, _change_output_variable, Aggregate, ChangeAggregate) or frame[] followed by a weighted reduction; count entities with _count_output_entities. (blocking: pytest `projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py`) - `sim.microdata.weights-not-values`: Microdata columns are weights, values, labels or counts: weights are only summed plainly to count entities or passed as weights=, values are aggregated with their own entity's weights, labels only group or filter, and counts derive from weights. (doc-only, not checked: follow it yourself) diff --git a/changelog_entry.yaml b/changelog_entry.yaml index 71428b4e4..a485ca4ce 100644 --- a/changelog_entry.yaml +++ b/changelog_entry.yaml @@ -2,7 +2,7 @@ changes: added: - Made the budget's reform-minus-baseline totals log a warning when the baseline and reform household frames carry different weights, since the change is then weighted by the reform's weights alone - - Added weighted-microdata rules to the registry, with proof from the #767 household count; the executor's code never aggregates a weight column as a value, reads raw values from weighted frames only in simulation_output_common.py (the UK geography builders' plain DataFrame handoff is baselined for a decision), and its output-builder tests build entity frames through one shared factory with non-uniform weights, while a doc-only rule records that weights, values, labels and counts each allow different operations + - Added weighted-microdata rules to the registry, with proof from the #767 household count; the executor's code never aggregates a weight column as a value, reads raw values from weighted frames only in simulation_output_common.py (the UK local-authority breakdown's plain household frame is its one recorded exception there, in a list that may only shrink), and its output-builder tests build entity frames through one shared factory with non-uniform weights, while a doc-only rule records that weights, values, labels and counts each allow different operations - Made CLAUDE.md and the Copilot instructions thin pointers to AGENTS.md, required workflow logic longer than one line to live in tested and shellchecked scripts, and registered the tracking-issue rule - Added a rules registry (rules/registry.toml) with a standard-library tool that validates it, checks every rule is enforced where it claims, and renders docs/generated/rules.md and the Rules block in AGENTS.md - Made the budget refuse a household count outside its country's plausible range (above 200 million in the US or 60 million in the UK, negative or not finite), raising ImplausibleEntityCountError instead of publishing a count like #767's diff --git a/docs/engineering/skills/microdata-weights.md b/docs/engineering/skills/microdata-weights.md index 72feca192..9a84a2d87 100644 --- a/docs/engineering/skills/microdata-weights.md +++ b/docs/engineering/skills/microdata-weights.md @@ -42,6 +42,8 @@ weights. Count entities with `_count_output_entities` instead. `.tolist()`, `pd.DataFrame(...)`, `pd.Series(...)`) only in `simulation_output_common.py`, which holds `_count_output_entities`. Summing stripped values gives an unweighted sample total, the mirror image of #767. + Within that module, counting entities is the one permanent read; every other + read is a recorded exception, and the list of exceptions may only shrink. - `sim.microdata.builder-tests-use-weighted-frames`: the output-builder tests (`tests/test_simulation_output_*.py`) build entity frames only through `fixtures/weighted_frames.weighted_entity_frame`, which returns a @@ -73,9 +75,14 @@ Each finding needs a decision, recorded in the PR: - **Fix it.** Count entities with `_count_output_entities`, and aggregate values through the weighted frame or the aggregators. -- **Move it.** A deliberate strip moves into `simulation_output_common.py` with - a comment saying why, as `_count_output_entities` does. +- **Move it, as a recorded exception.** A strip that can't be fixed yet moves + into `simulation_output_common.py` as a named helper, with a docstring saying + why and what would remove it. -Existing weight-stripping findings are listed in +Recorded exceptions are listed in `rules/baselines/sim-microdata-weight-stripping.txt`. That list may only -shrink. +shrink, so adding one is a deliberate change to the rule's baseline. Today it +holds one: `_unweighted_household_frame`, the plain household frame the UK +local-authority breakdown hands policyengine's longwise geography impact. It +goes once that breakdown hands policyengine the simulations, as the +constituency and US congressional district breakdowns already do. diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 887314518..37a03485b 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -249,7 +249,7 @@ flowchart LR | Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | | --- | --- | --- | --- | --- | --- | --- | --- | | `sim.microdata.builder-tests-use-weighted-frames` | Output-builder tests build entity frames only through fixtures/weighted_frames.weighted_entity_frame, as MicroDataFrames with non-uniform weights, never as plain pd.DataFrames. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py` | `pr.yml:test` | yes | -| `sim.microdata.weight-stripping-confined` | Raw values are read from a weighted frame or column (np.asarray, np.array, .values, .to_numpy(), .tolist(), pd.DataFrame or pd.Series) only in simulation_output_common.py, the module that holds _count_output_entities. | mechanical | ratcheted | yes | pytest `projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py` | `pr.yml:test` | yes | +| `sim.microdata.weight-stripping-confined` | Raw values are read from a weighted frame or column (np.asarray, np.array, .values, .to_numpy(), .tolist(), pd.DataFrame or pd.Series) only in simulation_output_common.py; there, counting entities is always allowed and every other read is a recorded exception whose list may only shrink. | mechanical | ratcheted | yes | pytest `projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py` | `pr.yml:test` | yes | | `sim.microdata.weights-not-aggregated-as-values` | A weight column is never aggregated as a value: never the variable of a weighted aggregator (_sum_output_variable, _change_output_variable, Aggregate, ChangeAggregate) or frame[] followed by a weighted reduction; count entities with _count_output_entities. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py` | `pr.yml:test` | yes | | `sim.microdata.weights-not-values` | Microdata columns are weights, values, labels or counts: weights are only summed plainly to count entities or passed as weights=, values are aggregated with their own entity's weights, labels only group or filter, and counts derive from weights. | mechanical | doc-only | no | nothing | — | — | diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_common.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_common.py index ae5c73308..4ce0d2feb 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_common.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_common.py @@ -8,10 +8,10 @@ from importlib import import_module from typing import TYPE_CHECKING, Any +import pandas as pd from policyengine_simulation_contract.spm import spm_error_detail if TYPE_CHECKING: - import pandas as pd from _typeshed import ConvertibleToFloat from policyengine.core import Simulation @@ -77,6 +77,23 @@ def _count_output_entities(simulation: Simulation, entity: str, weight: str) -> return float(np.asarray(data[weight], dtype=float).sum()) +def _unweighted_household_frame(simulation: Simulation) -> pd.DataFrame: + """Return the household output as a plain DataFrame, without its weights. + + policyengine's ``compute_longwise_uk_geography_impacts`` takes plain frames + and weights each area by the baseline's ``household_weight`` column itself. + The UK local-authority breakdown calls it directly, so that it can apply + this service's boundary metadata. Handing it the weighted frame would weight + twice if it ever reduced a column directly. + + This is a recorded exception to sim.microdata.weight-stripping-confined, + listed in rules/baselines/sim-microdata-weight-stripping.txt, which may only + shrink. It goes once the breakdown hands policyengine the simulations, as + the constituency and US congressional district breakdowns already do. + """ + return pd.DataFrame(simulation.output_dataset.data.household) + + def _warn_on_mismatched_weights( baseline_data: pd.DataFrame, reform_data: pd.DataFrame, entity: str ) -> None: diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_geographic.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_geographic.py index bc1588462..849c6c7a1 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_geographic.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_geographic.py @@ -6,7 +6,6 @@ from functools import lru_cache from typing import TYPE_CHECKING, Any -import pandas as pd from policyengine.data.uk_geography_assets import ( CONSTITUENCY_ASSET_SPEC, ) @@ -21,9 +20,11 @@ _output_model_dump, _output_module_function, _try_compute_output, + _unweighted_household_frame, ) if TYPE_CHECKING: + import pandas as pd from policyengine.core import Simulation from policyengine.data.uk_geography_assets import UKGeographyAssetSpec from policyengine_simulation_contract.uk_geography import UKLocalAuthorityMetadata @@ -235,10 +236,6 @@ def compute_and_format() -> CongressionalDistrictImpactOutput | None: return _try_compute_output("congressional district impacts", compute_and_format) -def _output_household(simulation: Simulation) -> pd.DataFrame: - return pd.DataFrame(simulation.output_dataset.data.household) - - def _require_uk_area_column( household: pd.DataFrame, column: str, breakdown: str ) -> None: @@ -268,7 +265,7 @@ def build_uk_constituency_impact( if country != "uk": return None _require_uk_area_column( - _output_household(baseline), "constituency_code_oa", "constituency" + baseline.output_dataset.data.household, "constituency_code_oa", "constituency" ) lookup_csv_path = _required_uk_geography_lookup_csv_path(CONSTITUENCY_ASSET_SPEC) @@ -302,7 +299,7 @@ def build_uk_local_authority_impact( if country != "uk": return None - baseline_household = _output_household(baseline) + baseline_household = _unweighted_household_frame(baseline) _require_uk_area_column(baseline_household, "la_code_oa", "local-authority") if uk_local_authority_metadata is None: msg = "UK local-authority boundary metadata is required" @@ -316,7 +313,7 @@ def build_uk_local_authority_impact( load_uk_local_authority_resources, ) - reform_household = _output_household(reform) + reform_household = _unweighted_household_frame(reform) numeric_records = compute_longwise_uk_geography_impacts( baseline_household=baseline_household, reform_household=reform_household, diff --git a/projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py b/projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py index d535c6aab..d60a4147c 100644 --- a/projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py +++ b/projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py @@ -29,6 +29,10 @@ WEIGHT_STRIPPING_MODULE = ( SOURCE_ROOT / "policyengine_simulation_executor" / "simulation_output_common.py" ) +# Counting entities is the one permanent read there: a plain sum of the +# weights, which weights-not-values allows. Every other read in the module is +# a recorded exception, listed in the baseline, and that list may only shrink. +ENTITY_COUNT_FUNCTION = "_count_output_entities" # Output-builder tests, which build entity frames only through the shared # factory in fixtures/weighted_frames.py. BUILDER_TEST_PATTERN = "test_simulation_output_*.py" @@ -446,29 +450,90 @@ def test_weights_are_never_aggregated_as_values() -> None: ) -def test_weight_stripping_stays_in_the_designated_module() -> None: - assert WEIGHT_STRIPPING_MODULE.is_file() - designated = WEIGHT_STRIPPING_MODULE.relative_to(REPOSITORY_ROOT).as_posix() - findings = [ +@dataclass(frozen=True) +class StrippingReview: + """Weight-stripping findings, sorted by what the rule allows.""" + + outside_designated_module: list[Finding] + unrecorded_exceptions: list[Finding] + stale_exceptions: list[str] + + +def _review_weight_stripping( + findings: list[Finding], *, designated: str, recorded: Counter[str] +) -> StrippingReview: + """Sort stripping findings against the designated module and its exceptions. + + Nothing outside the designated module may read raw values from weighted + frames. Inside it, counting entities is always allowed, and every other + read must be a recorded exception; the recorded list may only shrink. + """ + exceptions = [ finding - for finding in _findings(_source_modules(), STRIPPING) - if finding.path != designated + for finding in findings + if finding.path == designated and finding.function != ENTITY_COUNT_FUNCTION ] - found = Counter(finding.entry for finding in findings) - baseline = _baseline_entries(WEIGHT_STRIPPING_BASELINE) - new = [finding for finding in findings if finding.entry in found - baseline] - assert new == [], ( + found = Counter(finding.entry for finding in exceptions) + return StrippingReview( + outside_designated_module=[ + finding for finding in findings if finding.path != designated + ], + unrecorded_exceptions=[ + finding for finding in exceptions if finding.entry in found - recorded + ], + stale_exceptions=sorted((recorded - found).elements()), + ) + + +def test_weight_stripping_stays_in_the_designated_module() -> None: + assert WEIGHT_STRIPPING_MODULE.is_file() + review = _review_weight_stripping( + _findings(_source_modules(), STRIPPING), + designated=WEIGHT_STRIPPING_MODULE.relative_to(REPOSITORY_ROOT).as_posix(), + recorded=_baseline_entries(WEIGHT_STRIPPING_BASELINE), + ) + assert review.outside_designated_module == [], ( "raw values read from a weighted frame outside " f"{WEIGHT_STRIPPING_MODULE.name}; an unweighted total of them is a sample " "total, the mirror image of #767. Keep the weights, or decide with a " - "reviewer to move the read into the designated module:\n" + _described(new) + "reviewer to move the read into the designated module:\n" + + _described(review.outside_designated_module) ) - fixed = sorted((baseline - found).elements()) - assert fixed == [], ( - "these entries no longer apply; remove them from " + assert review.unrecorded_exceptions == [], ( + f"raw values read from a weighted frame in {WEIGHT_STRIPPING_MODULE.name} " + "for something other than counting entities. Each such read is a recorded " + f"exception in {WEIGHT_STRIPPING_BASELINE.name}, a list that may only " + "shrink; keep the weights instead:\n" + + _described(review.unrecorded_exceptions) + ) + assert review.stale_exceptions == [], ( + "these exceptions no longer apply; remove them from " f"{WEIGHT_STRIPPING_BASELINE.name} and lower the rule's baseline count " - "in rules/registry.toml:\n" + "\n".join(fixed) + "in rules/registry.toml:\n" + "\n".join(review.stale_exceptions) + ) + + +def test_weight_stripping_review_ratchets_the_designated_module() -> None: + designated = "src/simulation_output_common.py" + entity_count = Finding( + STRIPPING, designated, 1, ENTITY_COUNT_FUNCTION, "np.asarray(data[weight])" + ) + recorded_strip = Finding( + STRIPPING, designated, 2, "_unweighted_household_frame", "pd.DataFrame(frame)" ) + new_strip = Finding(STRIPPING, designated, 3, "_new_reader", "frame.values") + outside_strip = Finding(STRIPPING, "src/other.py", 4, "build", "frame.to_numpy()") + removed_entry = f"{designated}::_removed_reader::frame.tolist()" + + review = _review_weight_stripping( + [entity_count, recorded_strip, new_strip, outside_strip], + designated=designated, + recorded=Counter([recorded_strip.entry, removed_entry]), + ) + + assert review.outside_designated_module == [outside_strip] + assert review.unrecorded_exceptions == [new_strip] + assert review.stale_exceptions == [removed_entry] def test_builder_tests_build_entity_frames_with_weights() -> None: diff --git a/rules/baselines/sim-microdata-weight-stripping.txt b/rules/baselines/sim-microdata-weight-stripping.txt index d687e2f43..044b1e724 100644 --- a/rules/baselines/sim-microdata-weight-stripping.txt +++ b/rules/baselines/sim-microdata-weight-stripping.txt @@ -1,10 +1,13 @@ # Tolerated existing violations. This list may only shrink. # # Rule sim.microdata.weight-stripping-confined. Each entry is -# ::::, and each needs a decision: keep the -# weights, or move the read into simulation_output_common.py with a reason. +# ::::: a read of raw values from a weighted +# frame in simulation_output_common.py for something other than counting +# entities. Reads anywhere else fail outright. # -# The UK geography builders hand policyengine's longwise geography impact a -# plain DataFrame of the household frame, which weights by its -# household_weight column itself. -projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_geographic.py::_output_household::pd.DataFrame(simulation.output_dataset.data.household) +# The UK local-authority breakdown hands policyengine's longwise geography +# impact a plain DataFrame of the household frame, which weights by its +# household_weight column itself. It goes once the breakdown hands +# policyengine the simulations, as the constituency and US congressional +# district breakdowns already do. +projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_common.py::_unweighted_household_frame::pd.DataFrame(simulation.output_dataset.data.household) diff --git a/rules/registry.toml b/rules/registry.toml index 13d57c726..7a5af4721 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -736,7 +736,7 @@ test = "tests/test_microdata_weight_boundaries.py::test_rule_proof_weights_not_a [[rule]] id = "sim.microdata.weight-stripping-confined" category = "microdata" -statement = "Raw values are read from a weighted frame or column (np.asarray, np.array, .values, .to_numpy(), .tolist(), pd.DataFrame or pd.Series) only in simulation_output_common.py, the module that holds _count_output_entities." +statement = "Raw values are read from a weighted frame or column (np.asarray, np.array, .values, .to_numpy(), .tolist(), pd.DataFrame or pd.Series) only in simulation_output_common.py; there, counting entities is always allowed and every other read is a recorded exception whose list may only shrink." rationale = "docs/engineering/skills/microdata-weights.md#handling-a-finding" scope = ["projects/policyengine-simulation-executor/src/**/*.py"] tier = "mechanical" @@ -745,7 +745,10 @@ autofix = false [rule.enforcer] kind = "pytest" project = "projects/policyengine-simulation-executor" -nodes = ["tests/test_microdata_weight_boundaries.py::test_weight_stripping_stays_in_the_designated_module"] +nodes = [ + "tests/test_microdata_weight_boundaries.py::test_weight_stripping_stays_in_the_designated_module", + "tests/test_microdata_weight_boundaries.py::test_weight_stripping_review_ratchets_the_designated_module", +] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "test" From 063eceacc84d4030d9078f430e8f814561832893 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 03:20:31 +0400 Subject: [PATCH 6/6] Format the weight-boundaries guard's ratchet message ruff format joins the message's last string and the described findings onto one line. CI checks formatting only under src, so this test file missed it. Co-Authored-By: Claude Opus 5.5 --- .../tests/test_microdata_weight_boundaries.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py b/projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py index d60a4147c..40326b21d 100644 --- a/projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py +++ b/projects/policyengine-simulation-executor/tests/test_microdata_weight_boundaries.py @@ -503,8 +503,7 @@ def test_weight_stripping_stays_in_the_designated_module() -> None: f"raw values read from a weighted frame in {WEIGHT_STRIPPING_MODULE.name} " "for something other than counting entities. Each such read is a recorded " f"exception in {WEIGHT_STRIPPING_BASELINE.name}, a list that may only " - "shrink; keep the weights instead:\n" - + _described(review.unrecorded_exceptions) + "shrink; keep the weights instead:\n" + _described(review.unrecorded_exceptions) ) assert review.stale_exceptions == [], ( "these exceptions no longer apply; remove them from "