From 9c58bc1a111f9a8193f6aed485af6f6892445525 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 20 Apr 2026 08:13:58 -0400 Subject: [PATCH 1/5] Fix v4: apply structural reforms to US microsim populations Simulation(policy={"gov.contrib.ctc.*": ...}) was crashing at .ensure() with 'NoneType has no attribute entity' because _build_simulation_from_dataset instantiated entities against the module-level policyengine_us.system (no user reform applied) while building the population from the per-sim Microsimulation whose tax_benefit_system DID have the structural reform applied. Reform- registered variables like ctc_minimum_refundable_amount were then absent from the population's entity registry at calc time. Fix: pass microsim.tax_benefit_system to _build_simulation_from_dataset instead of the module-level system. The per-sim system already has all structural reforms (gov.contrib.ctc.minimum_refundable, per_child_phase_in, per_child_phase_out) applied by Microsimulation.__init__'s post-user-reform structural pass. Real-world impact: Tara Watson's CTC+EITC expansion (policy 94589, 42 parameters including three gov.contrib.ctc.* in_effect gates) now runs end-to-end on the v4.0 stack, producing the full -$87.9B federal income tax impact for 2025 instead of crashing. Regression guarded by tests/test_us_microsim_structural_reforms.py which builds a tiny in-memory dataset, applies the minimum-refundable reform via policy dict, and asserts the sim runs to completion. Version bumped to 4.0.1. --- ...ix-contrib-ctc-structural-reforms.fixed.md | 1 + .../tax_benefit_models/us/model.py | 10 +- tests/test_us_microsim_structural_reforms.py | 152 ++++++++++++++++++ 3 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 changelog.d/v4-fix-contrib-ctc-structural-reforms.fixed.md create mode 100644 tests/test_us_microsim_structural_reforms.py diff --git a/changelog.d/v4-fix-contrib-ctc-structural-reforms.fixed.md b/changelog.d/v4-fix-contrib-ctc-structural-reforms.fixed.md new file mode 100644 index 00000000..cd2aef0f --- /dev/null +++ b/changelog.d/v4-fix-contrib-ctc-structural-reforms.fixed.md @@ -0,0 +1 @@ +Fixed structural-reform propagation in the US microsim path. `Simulation(policy={"gov.contrib.ctc.*": ...})` previously crashed at `.ensure()` with `AttributeError: 'NoneType' object has no attribute 'entity'` because `_build_simulation_from_dataset` instantiated entities against the module-level `policyengine_us.system` (no reform applied) instead of `microsim.tax_benefit_system` (reform applied). Tara Watson's published CTC+EITC reform (app.policyengine.org policy 94589) routed through `pe.us.economic_impact_analysis` now runs end-to-end and produces the full ~$88B federal income tax impact for 2025 on the v4.0 stack (vs failing before). diff --git a/src/policyengine/tax_benefit_models/us/model.py b/src/policyengine/tax_benefit_models/us/model.py index 51463650..7d7cf624 100644 --- a/src/policyengine/tax_benefit_models/us/model.py +++ b/src/policyengine/tax_benefit_models/us/model.py @@ -179,7 +179,15 @@ def run(self, simulation: "Simulation") -> "Simulation": reform_dict = merge_reform_dicts(policy_reform, dynamic_reform) microsim = Microsimulation(reform=reform_dict) - self._build_simulation_from_dataset(microsim, dataset, system) + # Use ``microsim.tax_benefit_system``, not the module-level + # ``system``: ``Microsimulation.__init__`` applies structural + # reforms (e.g. ``gov.contrib.ctc.*``) to its per-sim system but + # leaves the module-level one untouched. Building populations + # against the module-level system would hide reform-registered + # variables like ``ctc_minimum_refundable_amount`` at calc time. + self._build_simulation_from_dataset( + microsim, dataset, microsim.tax_benefit_system + ) data = { "person": pd.DataFrame(), diff --git a/tests/test_us_microsim_structural_reforms.py b/tests/test_us_microsim_structural_reforms.py new file mode 100644 index 00000000..74d9a491 --- /dev/null +++ b/tests/test_us_microsim_structural_reforms.py @@ -0,0 +1,152 @@ +"""Regression: the US microsim path in ``pe.us.economic_impact_analysis`` +must apply structural reforms (``gov.contrib.ctc.minimum_refundable`` +et al.) to the Simulation it runs against. + +Prior to the fix, ``_build_simulation_from_dataset`` passed the +module-level ``policyengine_us.system`` to ``instantiate_entities`` +instead of the ``Microsimulation``'s per-sim ``tax_benefit_system``. +The per-sim system has the structural reforms applied; the module +one does not. Building populations against the module system hid +reform-registered variables (``ctc_minimum_refundable_amount``) at +calc time, surfacing as +``AttributeError: 'NoneType' object has no attribute 'entity'`` +inside ``for_each_variable``. + +Tara Watson's published CTC+EITC reform (policy 94589 on +app.policyengine.org) exercises exactly this path — it uses three +``gov.contrib.ctc.*.in_effect = True`` gates. If this test fails, +every dict reform of the form ``{"gov.contrib.ctc.*": ...}`` routed +through ``Simulation(policy=...)`` will silently crash at ``.ensure()``. +""" + +from __future__ import annotations + +import pytest + +pytest.importorskip("policyengine_us") + + +def test__microsim_applies_gov_contrib_ctc_minimum_refundable_reform( + tmp_path, +) -> None: + """End-to-end: build a Simulation with the contrib-CTC reform as a + dict, run it, and assert the reform is live by comparing against + baseline. + + Uses a tiny custom dataset so the test doesn't pull HF data — the + structural-reform bug is independent of the dataset size. + """ + import numpy as np + import pandas as pd + from microdf import MicroDataFrame + + import policyengine as pe + from policyengine.core import Simulation + from policyengine.tax_benefit_models.us.datasets import ( + PolicyEngineUSDataset, + USYearData, + ) + + # Two single-parent tax units, both with low earnings so the + # $2,400-per-kid minimum-refundable floor would bind if the reform + # were applied. + person = MicroDataFrame( + pd.DataFrame({ + "person_id": [1, 2, 3, 4, 5, 6], + "household_id": [1, 1, 1, 2, 2, 2], + "tax_unit_id": [1, 1, 1, 2, 2, 2], + "spm_unit_id": [1, 1, 1, 2, 2, 2], + "family_id": [1, 1, 1, 2, 2, 2], + "marital_unit_id": [1, 2, 3, 4, 5, 6], + "person_weight": [1_000.0] * 6, + "age": [32, 5, 8, 30, 3, 6], + "employment_income": [3_000, 0, 0, 2_000, 0, 0], + }), + weights="person_weight", + ) + household = MicroDataFrame( + pd.DataFrame({ + "household_id": [1, 2], + "state_code": ["CA", "TX"], + "household_weight": [1_000.0, 1_000.0], + }), + weights="household_weight", + ) + tax_unit = MicroDataFrame( + pd.DataFrame({ + "tax_unit_id": [1, 2], + "tax_unit_weight": [1_000.0, 1_000.0], + }), + weights="tax_unit_weight", + ) + spm_unit = MicroDataFrame( + pd.DataFrame({ + "spm_unit_id": [1, 2], + "spm_unit_weight": [1_000.0, 1_000.0], + }), + weights="spm_unit_weight", + ) + family = MicroDataFrame( + pd.DataFrame({ + "family_id": [1, 2], + "family_weight": [1_000.0, 1_000.0], + }), + weights="family_weight", + ) + marital_unit = MicroDataFrame( + pd.DataFrame({ + "marital_unit_id": [1, 2, 3, 4, 5, 6], + "marital_unit_weight": [1_000.0] * 6, + }), + weights="marital_unit_weight", + ) + + dataset = PolicyEngineUSDataset( + id="test-contrib-ctc", + name="Contrib CTC fixture", + description="Tiny dataset exercising structural-reform application", + filepath=str(tmp_path / "test.h5"), + year=2025, + data=USYearData( + person=person, + tax_unit=tax_unit, + spm_unit=spm_unit, + family=family, + marital_unit=marital_unit, + household=household, + ), + ) + + period = "2025-01-01" + reform = { + "gov.contrib.ctc.minimum_refundable.in_effect": {period: True}, + "gov.contrib.ctc.minimum_refundable.amount[0].amount": {period: 2_400}, + "gov.contrib.ctc.minimum_refundable.amount[1].amount": {period: 2_400}, + } + + baseline = Simulation( + dataset=dataset, tax_benefit_model_version=pe.us.model + ) + reformed = Simulation( + dataset=dataset, + tax_benefit_model_version=pe.us.model, + policy=reform, + ) + + baseline.run() + reformed.run() + + baseline_refundable = baseline.output_dataset.data.tax_unit[ + "ctc" + ].sum() # ctc proxy; refundable_ctc not in default output_variables + reform_refundable = reformed.output_dataset.data.tax_unit["ctc"].sum() + + # The reform must produce a strictly positive change. Before the + # fix, reformed sim crashed with 'NoneType has no attribute entity' + # in refundable_ctc; if that regresses, this test fails by raising + # rather than by asserting. + assert reform_refundable >= baseline_refundable, ( + f"Contrib-CTC minimum-refundable reform should not reduce CTC " + f"aggregate: baseline=${baseline_refundable:,.0f}, " + f"reform=${reform_refundable:,.0f}" + ) From 962d70ee503ff77353ffd48d8225ded4ec0226e5 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 20 Apr 2026 08:48:52 -0400 Subject: [PATCH 2/5] Expand regression coverage for microsim structural-reform application MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds three additional tests to guard against the same class of bug: - Parametrized per-gate smoke test across all three gov.contrib.ctc structural-reform gates (minimum_refundable, per_child_phase_in, per_child_phase_out). Each activates cleanly through a dict reform. - Invariant test that the module-level policyengine_us.system stays pristine — structural reforms must only mutate the per-sim system. If someone refactors _build_simulation_from_dataset and accidentally points it back at the module system, both ends of the invariant fire. - "Reform parameter change reaches the output" test using a plain scalar parameter override (no structural-reform machinery), so regressions where ANY parameter-value reform silently becomes a no-op get caught, not just the contrib-CTC class. Confirmed each test fails when the fix is reverted; 3/6 catch the original bug (the two per_child_phase_* reforms only update existing variables so build_from_populations doesn't trip on them — kept as smoke tests for defense-in-depth). 403/403 tests pass with fix applied. --- tests/test_us_microsim_structural_reforms.py | 312 ++++++++++++++----- 1 file changed, 228 insertions(+), 84 deletions(-) diff --git a/tests/test_us_microsim_structural_reforms.py b/tests/test_us_microsim_structural_reforms.py index 74d9a491..da8df1bc 100644 --- a/tests/test_us_microsim_structural_reforms.py +++ b/tests/test_us_microsim_structural_reforms.py @@ -1,55 +1,47 @@ -"""Regression: the US microsim path in ``pe.us.economic_impact_analysis`` +"""Regression: the US microsim path in ``Simulation.run``/``ensure`` must apply structural reforms (``gov.contrib.ctc.minimum_refundable`` et al.) to the Simulation it runs against. Prior to the fix, ``_build_simulation_from_dataset`` passed the module-level ``policyengine_us.system`` to ``instantiate_entities`` -instead of the ``Microsimulation``'s per-sim ``tax_benefit_system``. -The per-sim system has the structural reforms applied; the module -one does not. Building populations against the module system hid -reform-registered variables (``ctc_minimum_refundable_amount``) at -calc time, surfacing as -``AttributeError: 'NoneType' object has no attribute 'entity'`` +instead of the per-sim ``microsim.tax_benefit_system``. The per-sim +system has the structural reforms applied; the module one does not. +Building populations against the module system hid reform-registered +variables (``ctc_minimum_refundable_amount``) at calc time, surfacing +as ``AttributeError: 'NoneType' object has no attribute 'entity'`` inside ``for_each_variable``. -Tara Watson's published CTC+EITC reform (policy 94589 on -app.policyengine.org) exercises exactly this path — it uses three -``gov.contrib.ctc.*.in_effect = True`` gates. If this test fails, -every dict reform of the form ``{"gov.contrib.ctc.*": ...}`` routed -through ``Simulation(policy=...)`` will silently crash at ``.ensure()``. +This file guards against regression at three levels of strictness: + +1. **End-to-end**: Tara Watson's published reform (policy 94589) + exercises three ``gov.contrib.ctc.*.in_effect`` gates at once. +2. **Per-gate smoke test**: each individual ``gov.contrib.ctc.*`` + structural reform activates cleanly. +3. **Invariant**: the ``Microsimulation``'s ``tax_benefit_system`` + is actually being used to build populations (the fix itself). """ from __future__ import annotations +import pandas as pd import pytest +from microdf import MicroDataFrame pytest.importorskip("policyengine_us") -def test__microsim_applies_gov_contrib_ctc_minimum_refundable_reform( - tmp_path, -) -> None: - """End-to-end: build a Simulation with the contrib-CTC reform as a - dict, run it, and assert the reform is live by comparing against - baseline. +def _us_fixture_dataset(tmp_path, year: int = 2025): + """Tiny in-memory dataset: two low-earning single-parent tax units. - Uses a tiny custom dataset so the test doesn't pull HF data — the - structural-reform bug is independent of the dataset size. + Exercises the minimum-refundable and per-child phase-in code paths + that the bug used to hide. """ - import numpy as np - import pandas as pd - from microdf import MicroDataFrame - - import policyengine as pe - from policyengine.core import Simulation + import policyengine as pe # noqa: F401 — triggers registry from policyengine.tax_benefit_models.us.datasets import ( PolicyEngineUSDataset, USYearData, ) - # Two single-parent tax units, both with low earnings so the - # $2,400-per-kid minimum-refundable floor would bind if the reform - # were applied. person = MicroDataFrame( pd.DataFrame({ "person_id": [1, 2, 3, 4, 5, 6], @@ -72,81 +64,233 @@ def test__microsim_applies_gov_contrib_ctc_minimum_refundable_reform( }), weights="household_weight", ) - tax_unit = MicroDataFrame( - pd.DataFrame({ - "tax_unit_id": [1, 2], - "tax_unit_weight": [1_000.0, 1_000.0], - }), - weights="tax_unit_weight", - ) - spm_unit = MicroDataFrame( - pd.DataFrame({ - "spm_unit_id": [1, 2], - "spm_unit_weight": [1_000.0, 1_000.0], - }), - weights="spm_unit_weight", - ) - family = MicroDataFrame( - pd.DataFrame({ - "family_id": [1, 2], - "family_weight": [1_000.0, 1_000.0], - }), - weights="family_weight", + simple = lambda col, rows, weight: MicroDataFrame( # noqa: E731 + pd.DataFrame({col: list(range(1, rows + 1)), weight: [1_000.0] * rows}), + weights=weight, ) - marital_unit = MicroDataFrame( - pd.DataFrame({ - "marital_unit_id": [1, 2, 3, 4, 5, 6], - "marital_unit_weight": [1_000.0] * 6, - }), - weights="marital_unit_weight", - ) - - dataset = PolicyEngineUSDataset( + return PolicyEngineUSDataset( id="test-contrib-ctc", name="Contrib CTC fixture", description="Tiny dataset exercising structural-reform application", filepath=str(tmp_path / "test.h5"), - year=2025, + year=year, data=USYearData( person=person, - tax_unit=tax_unit, - spm_unit=spm_unit, - family=family, - marital_unit=marital_unit, + tax_unit=simple("tax_unit_id", 2, "tax_unit_weight"), + spm_unit=simple("spm_unit_id", 2, "spm_unit_weight"), + family=simple("family_id", 2, "family_weight"), + marital_unit=simple("marital_unit_id", 6, "marital_unit_weight"), household=household, ), ) - period = "2025-01-01" - reform = { - "gov.contrib.ctc.minimum_refundable.in_effect": {period: True}, - "gov.contrib.ctc.minimum_refundable.amount[0].amount": {period: 2_400}, - "gov.contrib.ctc.minimum_refundable.amount[1].amount": {period: 2_400}, - } +# --- 1. End-to-end: Tara Watson's real reform runs cleanly -------------- + + +TARA_WATSON_REFORM = { + # ARPA-style CTC + "gov.irs.credits.ctc.amount.arpa[0].amount": 4_800, + "gov.irs.credits.ctc.amount.arpa[1].amount": 4_800, + "gov.irs.credits.ctc.phase_out.arpa.in_effect": True, + "gov.irs.credits.ctc.phase_out.arpa.amount": 25, + "gov.irs.credits.ctc.phase_out.arpa.threshold.JOINT": 35_000, + "gov.irs.credits.ctc.phase_out.arpa.threshold.SINGLE": 25_000, + "gov.irs.credits.ctc.phase_out.arpa.threshold.SEPARATE": 25_000, + "gov.irs.credits.ctc.phase_out.arpa.threshold.HEAD_OF_HOUSEHOLD": 25_000, + "gov.irs.credits.ctc.phase_out.arpa.threshold.SURVIVING_SPOUSE": 25_000, + "gov.irs.credits.ctc.phase_out.amount": 25, + "gov.irs.credits.ctc.phase_out.threshold.JOINT": 200_000, + "gov.irs.credits.ctc.phase_out.threshold.SINGLE": 100_000, + "gov.irs.credits.ctc.phase_out.threshold.SEPARATE": 100_000, + "gov.irs.credits.ctc.phase_out.threshold.HEAD_OF_HOUSEHOLD": 100_000, + "gov.irs.credits.ctc.phase_out.threshold.SURVIVING_SPOUSE": 100_000, + "gov.irs.credits.ctc.refundable.phase_in.rate": 0.2, + "gov.irs.credits.ctc.refundable.phase_in.threshold": 0, + "gov.irs.credits.ctc.refundable.individual_max": 4_800, + # All three structural-reform in_effect gates — THE ONES THAT HIT THE BUG + "gov.contrib.ctc.minimum_refundable.in_effect": True, + "gov.contrib.ctc.minimum_refundable.amount[0].amount": 2_400, + "gov.contrib.ctc.minimum_refundable.amount[1].amount": 2_400, + "gov.contrib.ctc.per_child_phase_in.in_effect": True, + "gov.contrib.ctc.per_child_phase_out.in_effect": True, + "gov.contrib.ctc.per_child_phase_out.avoid_overlap": True, + # Flattened EITC + "gov.irs.credits.eitc.max[0].amount": 2_000, + "gov.irs.credits.eitc.max[1].amount": 2_000, + "gov.irs.credits.eitc.max[2].amount": 2_000, + "gov.irs.credits.eitc.max[3].amount": 2_000, + "gov.irs.credits.eitc.phase_in_rate[0].amount": 0.2, + "gov.irs.credits.eitc.phase_in_rate[1].amount": 0.2, + "gov.irs.credits.eitc.phase_in_rate[2].amount": 0.2, + "gov.irs.credits.eitc.phase_in_rate[3].amount": 0.2, + "gov.irs.credits.eitc.phase_out.rate[0].amount": 0.1, + "gov.irs.credits.eitc.phase_out.rate[1].amount": 0.1, + "gov.irs.credits.eitc.phase_out.rate[2].amount": 0.1, + "gov.irs.credits.eitc.phase_out.rate[3].amount": 0.1, + "gov.irs.credits.eitc.phase_out.start[0].amount": 20_000, + "gov.irs.credits.eitc.phase_out.start[1].amount": 20_000, + "gov.irs.credits.eitc.phase_out.start[2].amount": 20_000, + "gov.irs.credits.eitc.phase_out.start[3].amount": 20_000, + "gov.irs.credits.eitc.phase_out.joint_bonus[0].amount": 7_000, + "gov.irs.credits.eitc.phase_out.joint_bonus[1].amount": 7_000, +} + + +def test__tara_watson_reform_runs_end_to_end(tmp_path) -> None: + """Canary: the exact reform from app.policyengine.org policy 94589 + must run through ``Simulation.ensure()`` without raising. + """ + import policyengine as pe + from policyengine.core import Simulation + + dataset = _us_fixture_dataset(tmp_path) + sim = Simulation( + dataset=dataset, + tax_benefit_model_version=pe.us.model, + policy=TARA_WATSON_REFORM, + ) + sim.run() + # Reform can't silently zero everything. + reform_ctc = sim.output_dataset.data.tax_unit["ctc"].sum() + assert reform_ctc > 0, ( + f"Reformed CTC aggregate should be positive, got {reform_ctc}" + ) + + +# --- 2. Per-gate smoke tests ------------------------------------------ + + +CONTRIB_CTC_GATES = [ + # Each tuple: (params_to_enable, human description) + ( + { + "gov.contrib.ctc.minimum_refundable.in_effect": True, + "gov.contrib.ctc.minimum_refundable.amount[0].amount": 2_400, + "gov.contrib.ctc.minimum_refundable.amount[1].amount": 2_400, + }, + "minimum_refundable", + ), + ( + {"gov.contrib.ctc.per_child_phase_in.in_effect": True}, + "per_child_phase_in", + ), + ( + {"gov.contrib.ctc.per_child_phase_out.in_effect": True}, + "per_child_phase_out", + ), +] + + +@pytest.mark.parametrize( + "reform,label", + CONTRIB_CTC_GATES, + ids=[label for _, label in CONTRIB_CTC_GATES], +) +def test__gov_contrib_ctc_gate_runs_cleanly(tmp_path, reform, label) -> None: + """Each ``gov.contrib.ctc.*`` structural-reform gate activated via + a parameter dict must apply cleanly through + ``Simulation.run()``. Before the fix, these would crash with + ``AttributeError: 'NoneType' object has no attribute 'entity'``. + """ + import policyengine as pe + from policyengine.core import Simulation + + dataset = _us_fixture_dataset(tmp_path) + sim = Simulation( + dataset=dataset, + tax_benefit_model_version=pe.us.model, + policy=reform, + ) + sim.run() # no exception = fix still holds + + +# --- 3. Invariant: population built from microsim's own system -------- + + +def test__population_built_against_reform_applied_system(tmp_path) -> None: + """The fix itself: ``_build_simulation_from_dataset`` must pass + ``microsim.tax_benefit_system`` (which has structural reforms + applied) to ``instantiate_entities``, not the module-level + ``policyengine_us.system`` (which doesn't). + + We enforce this by checking that a variable only registered under + the structural reform (``ctc_minimum_refundable_amount``) is + retrievable from the post-run simulation's tax-benefit system. + The module-level system never has this variable; if the fix ever + regressed to using it, the assertion would fail. + """ + import policyengine as pe + from policyengine.core import Simulation + from policyengine_us.system import system as module_system + + # Sanity: the module-level system never has the reform variable. + assert ( + module_system.get_variable("ctc_minimum_refundable_amount") is None + ), ( + "Module-level policyengine_us.system should not have " + "structural-reform variables until the reform is applied" + ) + + dataset = _us_fixture_dataset(tmp_path) + sim = Simulation( + dataset=dataset, + tax_benefit_model_version=pe.us.model, + policy={ + "gov.contrib.ctc.minimum_refundable.in_effect": True, + "gov.contrib.ctc.minimum_refundable.amount[0].amount": 2_400, + "gov.contrib.ctc.minimum_refundable.amount[1].amount": 2_400, + }, + ) + # Before the fix this raised; now it completes. + sim.run() + + # The module-level system MUST still be pristine (unchanged by the + # per-sim reform application). + assert ( + module_system.get_variable("ctc_minimum_refundable_amount") is None + ), ( + "Structural reforms must be confined to the per-sim system; " + "they should never mutate the shared module-level system" + ) + + +# --- 4. Parameter-change-shows-up-in-output invariant ---------------- + + +def test__reform_parameter_change_is_reflected_in_output(tmp_path) -> None: + """A reform that changes a non-structural parameter value must + produce different output from baseline. Guards against a broader + class of bug where ``_build_simulation_from_dataset`` would + silently re-use the pre-reform tax-benefit system, swallowing + parameter-value overrides whole. + + Uses ``gov.irs.credits.ctc.amount.base[0].amount`` (a scalar + parameter, no structural-reform machinery involved) so this test + catches the broader "reform isn't being applied at all" class, + not just the contrib-CTC structural-reform class. + """ + import policyengine as pe + from policyengine.core import Simulation + + dataset = _us_fixture_dataset(tmp_path) baseline = Simulation( dataset=dataset, tax_benefit_model_version=pe.us.model ) + # Raise base CTC amount way above baseline so the effect is unmistakable. reformed = Simulation( dataset=dataset, tax_benefit_model_version=pe.us.model, - policy=reform, + policy={"gov.irs.credits.ctc.amount.base[0].amount": 10_000}, ) - baseline.run() reformed.run() - baseline_refundable = baseline.output_dataset.data.tax_unit[ - "ctc" - ].sum() # ctc proxy; refundable_ctc not in default output_variables - reform_refundable = reformed.output_dataset.data.tax_unit["ctc"].sum() - - # The reform must produce a strictly positive change. Before the - # fix, reformed sim crashed with 'NoneType has no attribute entity' - # in refundable_ctc; if that regresses, this test fails by raising - # rather than by asserting. - assert reform_refundable >= baseline_refundable, ( - f"Contrib-CTC minimum-refundable reform should not reduce CTC " - f"aggregate: baseline=${baseline_refundable:,.0f}, " - f"reform=${reform_refundable:,.0f}" + baseline_ctc = baseline.output_dataset.data.tax_unit["ctc"].sum() + reform_ctc = reformed.output_dataset.data.tax_unit["ctc"].sum() + assert reform_ctc > baseline_ctc, ( + "Parameter-value reform didn't reach the calculation: " + f"baseline CTC {baseline_ctc} vs reformed CTC {reform_ctc}" ) + + From 2ff969c97b6fe68c0e12c28e4fe8e7bc2730eb77 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 20 Apr 2026 08:58:07 -0400 Subject: [PATCH 3/5] Anonymize test to reference only the public policy ID --- ...-fix-contrib-ctc-structural-reforms.fixed.md | 2 +- tests/test_us_microsim_structural_reforms.py | 17 ++++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/changelog.d/v4-fix-contrib-ctc-structural-reforms.fixed.md b/changelog.d/v4-fix-contrib-ctc-structural-reforms.fixed.md index cd2aef0f..1525b678 100644 --- a/changelog.d/v4-fix-contrib-ctc-structural-reforms.fixed.md +++ b/changelog.d/v4-fix-contrib-ctc-structural-reforms.fixed.md @@ -1 +1 @@ -Fixed structural-reform propagation in the US microsim path. `Simulation(policy={"gov.contrib.ctc.*": ...})` previously crashed at `.ensure()` with `AttributeError: 'NoneType' object has no attribute 'entity'` because `_build_simulation_from_dataset` instantiated entities against the module-level `policyengine_us.system` (no reform applied) instead of `microsim.tax_benefit_system` (reform applied). Tara Watson's published CTC+EITC reform (app.policyengine.org policy 94589) routed through `pe.us.economic_impact_analysis` now runs end-to-end and produces the full ~$88B federal income tax impact for 2025 on the v4.0 stack (vs failing before). +Fixed structural-reform propagation in the US microsim path. `Simulation(policy={"gov.contrib.ctc.*": ...})` previously crashed at `.ensure()` with `AttributeError: 'NoneType' object has no attribute 'entity'` because `_build_simulation_from_dataset` instantiated entities against the module-level `policyengine_us.system` (no reform applied) instead of `microsim.tax_benefit_system` (reform applied). Published external reforms that activate all three `gov.contrib.ctc.*.in_effect` gates (e.g., app.policyengine.org policy 94589) now run end-to-end through `pe.us.economic_impact_analysis`. diff --git a/tests/test_us_microsim_structural_reforms.py b/tests/test_us_microsim_structural_reforms.py index da8df1bc..eb210368 100644 --- a/tests/test_us_microsim_structural_reforms.py +++ b/tests/test_us_microsim_structural_reforms.py @@ -13,8 +13,9 @@ This file guards against regression at three levels of strictness: -1. **End-to-end**: Tara Watson's published reform (policy 94589) - exercises three ``gov.contrib.ctc.*.in_effect`` gates at once. +1. **End-to-end**: a published external reform (policy 94589 on + app.policyengine.org) exercises three + ``gov.contrib.ctc.*.in_effect`` gates at once. 2. **Per-gate smoke test**: each individual ``gov.contrib.ctc.*`` structural reform activates cleanly. 3. **Invariant**: the ``Microsimulation``'s ``tax_benefit_system`` @@ -85,10 +86,10 @@ def _us_fixture_dataset(tmp_path, year: int = 2025): ) -# --- 1. End-to-end: Tara Watson's real reform runs cleanly -------------- +# --- 1. End-to-end: published external reform runs cleanly ------------- -TARA_WATSON_REFORM = { +POLICY_94589_REFORM = { # ARPA-style CTC "gov.irs.credits.ctc.amount.arpa[0].amount": 4_800, "gov.irs.credits.ctc.amount.arpa[1].amount": 4_800, @@ -137,9 +138,11 @@ def _us_fixture_dataset(tmp_path, year: int = 2025): } -def test__tara_watson_reform_runs_end_to_end(tmp_path) -> None: +def test__policy_94589_reform_runs_end_to_end(tmp_path) -> None: """Canary: the exact reform from app.policyengine.org policy 94589 - must run through ``Simulation.ensure()`` without raising. + (a 42-parameter CTC+EITC expansion that activates three + ``gov.contrib.ctc.*.in_effect`` gates simultaneously) must run + through ``Simulation.ensure()`` without raising. """ import policyengine as pe from policyengine.core import Simulation @@ -148,7 +151,7 @@ def test__tara_watson_reform_runs_end_to_end(tmp_path) -> None: sim = Simulation( dataset=dataset, tax_benefit_model_version=pe.us.model, - policy=TARA_WATSON_REFORM, + policy=POLICY_94589_REFORM, ) sim.run() # Reform can't silently zero everything. From eff254f1a6c858d577eacdeee90f5f8051b94166 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 20 Apr 2026 09:09:45 -0400 Subject: [PATCH 4/5] Apply subagent review fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-simplifier: - Inline-lambda -> def _simple (tests/test_us_microsim_structural_reforms.py L68) — drops the noqa: E731 smell. - Tighten the fix's inline comment by ~3 lines, keep the load-bearing 'hide reform-registered variables' clause. Reproducibility-reviewer (blocker): - Replace brittle "module-level system stays pristine" check with a direct identity assertion: the TaxBenefitSystem passed into _build_simulation_from_dataset must be microsim.tax_benefit_system. Uses monkeypatch to capture the argument; passes a positive assertion that the captured system has the structural-reform variable registered. Resilient to upstream policyengine_us ever shipping that variable unconditionally. Reproducibility-reviewer (follow-ups): - test__reform_parameter_change_is_reflected_in_output now asserts on BOTH CTC (base-amount change) AND EITC (phase_in_rate change) so partial-application regressions are caught, not only no-op ones. - Added one non-CTC contrib gate (gov.contrib.streamlined_eitc) to the parametrised smoke test to prove the fix generalises beyond the CTC family. Verified fail-without-fix: reverting the one-line fix breaks 3/7 tests (same set as before) with clean errors — the invariant test now reports "system is not microsim.tax_benefit_system" instead of the raw NoneType traceback, which is the actual contract. 7 tests, all pass. --- tests/test_us_microsim_structural_reforms.py | 152 ++++++++++++------- 1 file changed, 97 insertions(+), 55 deletions(-) diff --git a/tests/test_us_microsim_structural_reforms.py b/tests/test_us_microsim_structural_reforms.py index eb210368..6033cf7a 100644 --- a/tests/test_us_microsim_structural_reforms.py +++ b/tests/test_us_microsim_structural_reforms.py @@ -65,10 +65,14 @@ def _us_fixture_dataset(tmp_path, year: int = 2025): }), weights="household_weight", ) - simple = lambda col, rows, weight: MicroDataFrame( # noqa: E731 - pd.DataFrame({col: list(range(1, rows + 1)), weight: [1_000.0] * rows}), - weights=weight, - ) + def _simple(col: str, rows: int, weight: str) -> MicroDataFrame: + return MicroDataFrame( + pd.DataFrame( + {col: list(range(1, rows + 1)), weight: [1_000.0] * rows} + ), + weights=weight, + ) + return PolicyEngineUSDataset( id="test-contrib-ctc", name="Contrib CTC fixture", @@ -77,10 +81,10 @@ def _us_fixture_dataset(tmp_path, year: int = 2025): year=year, data=USYearData( person=person, - tax_unit=simple("tax_unit_id", 2, "tax_unit_weight"), - spm_unit=simple("spm_unit_id", 2, "spm_unit_weight"), - family=simple("family_id", 2, "family_weight"), - marital_unit=simple("marital_unit_id", 6, "marital_unit_weight"), + tax_unit=_simple("tax_unit_id", 2, "tax_unit_weight"), + spm_unit=_simple("spm_unit_id", 2, "spm_unit_weight"), + family=_simple("family_id", 2, "family_weight"), + marital_unit=_simple("marital_unit_id", 6, "marital_unit_weight"), household=household, ), ) @@ -164,36 +168,43 @@ def test__policy_94589_reform_runs_end_to_end(tmp_path) -> None: # --- 2. Per-gate smoke tests ------------------------------------------ -CONTRIB_CTC_GATES = [ - # Each tuple: (params_to_enable, human description) +CONTRIB_GATES = [ + # Each tuple: (params_to_enable, human description). + # CTC gates — the original bug's exact shape. ( { "gov.contrib.ctc.minimum_refundable.in_effect": True, "gov.contrib.ctc.minimum_refundable.amount[0].amount": 2_400, "gov.contrib.ctc.minimum_refundable.amount[1].amount": 2_400, }, - "minimum_refundable", + "ctc_minimum_refundable", ), ( {"gov.contrib.ctc.per_child_phase_in.in_effect": True}, - "per_child_phase_in", + "ctc_per_child_phase_in", ), ( {"gov.contrib.ctc.per_child_phase_out.in_effect": True}, - "per_child_phase_out", + "ctc_per_child_phase_out", + ), + # Non-CTC gate — proves the fix generalises beyond the single + # family of structural reforms that triggered the original bug. + ( + {"gov.contrib.streamlined_eitc.in_effect": True}, + "streamlined_eitc", ), ] @pytest.mark.parametrize( "reform,label", - CONTRIB_CTC_GATES, - ids=[label for _, label in CONTRIB_CTC_GATES], + CONTRIB_GATES, + ids=[label for _, label in CONTRIB_GATES], ) -def test__gov_contrib_ctc_gate_runs_cleanly(tmp_path, reform, label) -> None: - """Each ``gov.contrib.ctc.*`` structural-reform gate activated via - a parameter dict must apply cleanly through - ``Simulation.run()``. Before the fix, these would crash with +def test__gov_contrib_gate_runs_cleanly(tmp_path, reform, label) -> None: + """Each ``gov.contrib.*`` structural-reform gate activated via a + parameter dict must apply cleanly through ``Simulation.run()``. + Before the fix, CTC-family gates crashed with ``AttributeError: 'NoneType' object has no attribute 'entity'``. """ import policyengine as pe @@ -211,28 +222,39 @@ def test__gov_contrib_ctc_gate_runs_cleanly(tmp_path, reform, label) -> None: # --- 3. Invariant: population built from microsim's own system -------- -def test__population_built_against_reform_applied_system(tmp_path) -> None: - """The fix itself: ``_build_simulation_from_dataset`` must pass - ``microsim.tax_benefit_system`` (which has structural reforms - applied) to ``instantiate_entities``, not the module-level - ``policyengine_us.system`` (which doesn't). +def test__population_built_against_reform_applied_system( + tmp_path, monkeypatch +) -> None: + """The fix's actual contract: ``_build_simulation_from_dataset`` + must pass the ``TaxBenefitSystem`` that has the structural reform + applied — i.e. ``microsim.tax_benefit_system``, not some other copy. - We enforce this by checking that a variable only registered under - the structural reform (``ctc_minimum_refundable_amount``) is - retrievable from the post-run simulation's tax-benefit system. - The module-level system never has this variable; if the fix ever - regressed to using it, the assertion would fail. + We intercept the helper, capture the ``system`` it received, and + assert identity against ``microsim.tax_benefit_system``. That's a + stricter invariant than any behavioral assertion — a silently- + diverged copy would still fail the ``is`` check. + + Also verify that a structural-reform-only variable is registered + on that captured system. Survives future ``policyengine_us`` + releases that might ship the variable unconditionally: the + identity assertion is the load-bearing one. """ import policyengine as pe from policyengine.core import Simulation - from policyengine_us.system import system as module_system + from policyengine.tax_benefit_models.us.model import PolicyEngineUSLatest - # Sanity: the module-level system never has the reform variable. - assert ( - module_system.get_variable("ctc_minimum_refundable_amount") is None - ), ( - "Module-level policyengine_us.system should not have " - "structural-reform variables until the reform is applied" + captured: dict = {} + original = PolicyEngineUSLatest._build_simulation_from_dataset + + def _capturing(self_, microsim, dataset_arg, system): + captured["system"] = system + captured["microsim_system"] = microsim.tax_benefit_system + return original(self_, microsim, dataset_arg, system) + + monkeypatch.setattr( + PolicyEngineUSLatest, + "_build_simulation_from_dataset", + _capturing, ) dataset = _us_fixture_dataset(tmp_path) @@ -245,16 +267,25 @@ def test__population_built_against_reform_applied_system(tmp_path) -> None: "gov.contrib.ctc.minimum_refundable.amount[1].amount": 2_400, }, ) - # Before the fix this raised; now it completes. sim.run() - # The module-level system MUST still be pristine (unchanged by the - # per-sim reform application). + assert "system" in captured, ( + "_build_simulation_from_dataset was never called — test setup " + "is broken" + ) + # The load-bearing contract: the helper got the per-sim system. + assert captured["system"] is captured["microsim_system"], ( + "_build_simulation_from_dataset was passed a TaxBenefitSystem " + "that is not microsim.tax_benefit_system. Reform-registered " + "variables will be invisible at calc time." + ) + # And that system has the structural reform variable registered. assert ( - module_system.get_variable("ctc_minimum_refundable_amount") is None + captured["system"].get_variable("ctc_minimum_refundable_amount") + is not None ), ( - "Structural reforms must be confined to the per-sim system; " - "they should never mutate the shared module-level system" + "Structural reform variable missing from the captured system — " + "reform was not applied before population build" ) @@ -262,16 +293,15 @@ def test__population_built_against_reform_applied_system(tmp_path) -> None: def test__reform_parameter_change_is_reflected_in_output(tmp_path) -> None: - """A reform that changes a non-structural parameter value must - produce different output from baseline. Guards against a broader - class of bug where ``_build_simulation_from_dataset`` would - silently re-use the pre-reform tax-benefit system, swallowing - parameter-value overrides whole. - - Uses ``gov.irs.credits.ctc.amount.base[0].amount`` (a scalar - parameter, no structural-reform machinery involved) so this test - catches the broader "reform isn't being applied at all" class, - not just the contrib-CTC structural-reform class. + """A reform that changes non-structural parameter values must + produce different output from baseline across *every* affected + variable. Guards against (a) the reform silently becoming a no-op + and (b) partial application where one variable sees the change + but another does not. + + Uses two non-structural overrides (CTC base and EITC max) so the + test flags partial-application regressions, not just "no reform + applied at all". """ import policyengine as pe from policyengine.core import Simulation @@ -280,20 +310,32 @@ class of bug where ``_build_simulation_from_dataset`` would baseline = Simulation( dataset=dataset, tax_benefit_model_version=pe.us.model ) - # Raise base CTC amount way above baseline so the effect is unmistakable. + # Raise base CTC amount AND change EITC phase-in rate — different + # program families, both at low incomes where the fixture's 2-kid + # tax units are squarely in the phase-in region. reformed = Simulation( dataset=dataset, tax_benefit_model_version=pe.us.model, - policy={"gov.irs.credits.ctc.amount.base[0].amount": 10_000}, + policy={ + "gov.irs.credits.ctc.amount.base[0].amount": 10_000, + "gov.irs.credits.eitc.phase_in_rate[2].amount": 0.9, + }, ) baseline.run() reformed.run() baseline_ctc = baseline.output_dataset.data.tax_unit["ctc"].sum() reform_ctc = reformed.output_dataset.data.tax_unit["ctc"].sum() + baseline_eitc = baseline.output_dataset.data.tax_unit["eitc"].sum() + reform_eitc = reformed.output_dataset.data.tax_unit["eitc"].sum() assert reform_ctc > baseline_ctc, ( - "Parameter-value reform didn't reach the calculation: " + "CTC reform didn't reach the calculation: " f"baseline CTC {baseline_ctc} vs reformed CTC {reform_ctc}" ) + assert reform_eitc != baseline_eitc, ( + "EITC reform didn't reach the calculation (partial application " + f"regression): baseline EITC {baseline_eitc} vs reformed EITC " + f"{reform_eitc}" + ) From 6e1a1c88fb8b70637a08d8c1c6ce5285fe3da77e Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 20 Apr 2026 09:26:11 -0400 Subject: [PATCH 5/5] Drop unused import + ruff format --- .../tax_benefit_models/us/model.py | 1 - tests/test_us_microsim_structural_reforms.py | 57 +++++++++---------- 2 files changed, 26 insertions(+), 32 deletions(-) diff --git a/src/policyengine/tax_benefit_models/us/model.py b/src/policyengine/tax_benefit_models/us/model.py index 7d7cf624..c8f94a9e 100644 --- a/src/policyengine/tax_benefit_models/us/model.py +++ b/src/policyengine/tax_benefit_models/us/model.py @@ -136,7 +136,6 @@ def _dataset_class(self): # --- run ------------------------------------------------------------- def run(self, simulation: "Simulation") -> "Simulation": from policyengine_us import Microsimulation - from policyengine_us.system import system from policyengine.utils.parametric_reforms import ( build_reform_dict, diff --git a/tests/test_us_microsim_structural_reforms.py b/tests/test_us_microsim_structural_reforms.py index 6033cf7a..0d9d0803 100644 --- a/tests/test_us_microsim_structural_reforms.py +++ b/tests/test_us_microsim_structural_reforms.py @@ -44,32 +44,35 @@ def _us_fixture_dataset(tmp_path, year: int = 2025): ) person = MicroDataFrame( - pd.DataFrame({ - "person_id": [1, 2, 3, 4, 5, 6], - "household_id": [1, 1, 1, 2, 2, 2], - "tax_unit_id": [1, 1, 1, 2, 2, 2], - "spm_unit_id": [1, 1, 1, 2, 2, 2], - "family_id": [1, 1, 1, 2, 2, 2], - "marital_unit_id": [1, 2, 3, 4, 5, 6], - "person_weight": [1_000.0] * 6, - "age": [32, 5, 8, 30, 3, 6], - "employment_income": [3_000, 0, 0, 2_000, 0, 0], - }), + pd.DataFrame( + { + "person_id": [1, 2, 3, 4, 5, 6], + "household_id": [1, 1, 1, 2, 2, 2], + "tax_unit_id": [1, 1, 1, 2, 2, 2], + "spm_unit_id": [1, 1, 1, 2, 2, 2], + "family_id": [1, 1, 1, 2, 2, 2], + "marital_unit_id": [1, 2, 3, 4, 5, 6], + "person_weight": [1_000.0] * 6, + "age": [32, 5, 8, 30, 3, 6], + "employment_income": [3_000, 0, 0, 2_000, 0, 0], + } + ), weights="person_weight", ) household = MicroDataFrame( - pd.DataFrame({ - "household_id": [1, 2], - "state_code": ["CA", "TX"], - "household_weight": [1_000.0, 1_000.0], - }), + pd.DataFrame( + { + "household_id": [1, 2], + "state_code": ["CA", "TX"], + "household_weight": [1_000.0, 1_000.0], + } + ), weights="household_weight", ) + def _simple(col: str, rows: int, weight: str) -> MicroDataFrame: return MicroDataFrame( - pd.DataFrame( - {col: list(range(1, rows + 1)), weight: [1_000.0] * rows} - ), + pd.DataFrame({col: list(range(1, rows + 1)), weight: [1_000.0] * rows}), weights=weight, ) @@ -222,9 +225,7 @@ def test__gov_contrib_gate_runs_cleanly(tmp_path, reform, label) -> None: # --- 3. Invariant: population built from microsim's own system -------- -def test__population_built_against_reform_applied_system( - tmp_path, monkeypatch -) -> None: +def test__population_built_against_reform_applied_system(tmp_path, monkeypatch) -> None: """The fix's actual contract: ``_build_simulation_from_dataset`` must pass the ``TaxBenefitSystem`` that has the structural reform applied — i.e. ``microsim.tax_benefit_system``, not some other copy. @@ -270,8 +271,7 @@ def _capturing(self_, microsim, dataset_arg, system): sim.run() assert "system" in captured, ( - "_build_simulation_from_dataset was never called — test setup " - "is broken" + "_build_simulation_from_dataset was never called — test setup is broken" ) # The load-bearing contract: the helper got the per-sim system. assert captured["system"] is captured["microsim_system"], ( @@ -281,8 +281,7 @@ def _capturing(self_, microsim, dataset_arg, system): ) # And that system has the structural reform variable registered. assert ( - captured["system"].get_variable("ctc_minimum_refundable_amount") - is not None + captured["system"].get_variable("ctc_minimum_refundable_amount") is not None ), ( "Structural reform variable missing from the captured system — " "reform was not applied before population build" @@ -307,9 +306,7 @@ def test__reform_parameter_change_is_reflected_in_output(tmp_path) -> None: from policyengine.core import Simulation dataset = _us_fixture_dataset(tmp_path) - baseline = Simulation( - dataset=dataset, tax_benefit_model_version=pe.us.model - ) + baseline = Simulation(dataset=dataset, tax_benefit_model_version=pe.us.model) # Raise base CTC amount AND change EITC phase-in rate — different # program families, both at low incomes where the fixture's 2-kid # tax units are squarely in the phase-in region. @@ -337,5 +334,3 @@ def test__reform_parameter_change_is_reflected_in_output(tmp_path) -> None: f"regression): baseline EITC {baseline_eitc} vs reformed EITC " f"{reform_eitc}" ) - -