From fcc035f4b83fafde5d46255539464954840eaca4 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Thu, 15 Oct 2026 14:23:45 -0400 Subject: [PATCH 1/9] Fix silent UC health reform routes --- changelog.d/2426.fixed.md | 1 + docs/book/usage/scenarios.md | 1 + .../elements/disabled/amount.yaml | 2 +- policyengine_uk/scenarios/__init__.py | 5 +- policyengine_uk/scenarios/uc_reform.py | 67 +++- policyengine_uk/simulation.py | 12 + .../tests/test_from_reform_modifier.py | 360 ++++++++++++++++++ policyengine_uk/tests/test_uc_health_draw.py | 86 +++++ .../tests/test_uc_health_warnings.py | 252 ++++++++++++ policyengine_uk/utils/dated_reforms.py | 125 ++++++ policyengine_uk/utils/scenario.py | 73 ++-- 11 files changed, 957 insertions(+), 27 deletions(-) create mode 100644 changelog.d/2426.fixed.md create mode 100644 policyengine_uk/tests/test_from_reform_modifier.py create mode 100644 policyengine_uk/tests/test_uc_health_draw.py create mode 100644 policyengine_uk/tests/test_uc_health_warnings.py diff --git a/changelog.d/2426.fixed.md b/changelog.d/2426.fixed.md new file mode 100644 index 0000000000..b0fd5ea87c --- /dev/null +++ b/changelog.d/2426.fixed.md @@ -0,0 +1 @@ +- Fixed standalone dated-reform modifiers silently dropping parameter changes, warned when UC rebalancing supersedes an LCWRA amount reform, and restored new-claimant draws for data-built core simulations. diff --git a/docs/book/usage/scenarios.md b/docs/book/usage/scenarios.md index 5050ae1b6b..f2065430d4 100644 --- a/docs/book/usage/scenarios.md +++ b/docs/book/usage/scenarios.md @@ -418,6 +418,7 @@ Processing also builds some parameters from others: the triple lock rate (`gov.e A dataset's later years are projected from its first by the economic assumptions as the simulation is built. Dated changes that change what that projection reads (a growth rate, say, through `parameter_changes`, `"start.stop"` or bare-year keys, or a 1 January key on a calendar-year series) are written before the data is built, so incomes, rents and council tax follow them, as with `applied_before_data_load=True`. Other dated changes come just after the data is built, before the steps that read parameters while the simulation is built: the Universal Credit rebalancing awards and the structural reforms that parameters switch on. Other single-day keys and a scenario's own simulation modifier run after those steps, as they always have, and leave the data as built. Datasets loaded from `hf://` or `gs://` are cached once projected, under current law only: a simulation whose projection inputs differ builds its own, so a scenario never reaches its baseline or a later simulation. A sum with a part applied before data load applies all its parameter changes before the data is built (see "Combining scenarios" above). All of this applies when the scenario or reform is passed to `Simulation(...)`. `Scenario.apply(simulation)` on a simulation that is already built edits its annualised values instead, as it always has: a `"2027-04-06.2028-04-05"` key there overrides those dates of the fiscal-year values, so it does not set fiscal year 2027, and the model logs a warning saying so. Pass the scenario when constructing the simulation for dated measures. +The modifier returned by `Scenario.from_reform` behaves the same way on a built simulation for changes not already written onto its statutory timeline, and raises if a key contains no 1 January annual value. ### Creating complex scenarios with simulation modifiers diff --git a/policyengine_uk/parameters/gov/dwp/universal_credit/elements/disabled/amount.yaml b/policyengine_uk/parameters/gov/dwp/universal_credit/elements/disabled/amount.yaml index 3319c90ac8..18c4266db3 100644 --- a/policyengine_uk/parameters/gov/dwp/universal_credit/elements/disabled/amount.yaml +++ b/policyengine_uk/parameters/gov/dwp/universal_credit/elements/disabled/amount.yaml @@ -1,4 +1,4 @@ -description: Limited capability for work-related activity element amount under the Universal Credit. +description: Limited capability for work-related activity element amount under the Universal Credit. In years when the rebalancing modifier runs, the rebalancing protected_health_element and new_claimant_health_element parameters set the amount paid, so a health element reform for those years changes those parameters. values: 2016-04-10: 315.6 2017-04-10: 318.76 diff --git a/policyengine_uk/scenarios/__init__.py b/policyengine_uk/scenarios/__init__.py index 023c243f16..4be5973b03 100644 --- a/policyengine_uk/scenarios/__init__.py +++ b/policyengine_uk/scenarios/__init__.py @@ -3,4 +3,7 @@ from .pip_reform import reform_pip_phase_in from .reindex_benefit_cap import reindex_benefit_cap from .repeal_two_child_limit import repeal_two_child_limit -from .uc_reform import universal_credit_july_2025_reform +from .uc_reform import ( + UCHealthElementOverrideWarning, + universal_credit_july_2025_reform, +) diff --git a/policyengine_uk/scenarios/uc_reform.py b/policyengine_uk/scenarios/uc_reform.py index de6d0233d6..bac5daed80 100644 --- a/policyengine_uk/scenarios/uc_reform.py +++ b/policyengine_uk/scenarios/uc_reform.py @@ -1,7 +1,10 @@ from policyengine_uk.model_api import Scenario from policyengine_uk import Microsimulation +from policyengine_uk.utils.data_source import built_from_data from policyengine_uk.utils.supplied_inputs import set_model_value import numpy as np +import warnings +from policyengine_core.periods import instant # The standard allowance amounts the protected LCWRA amount is paired with in @@ -97,6 +100,63 @@ def _protected_existing_health_element_monthly( NEW_CLAIMANT_STATUS = "uc_receives_new_claimant_health_element" +UC_REBALANCING_YEARS = range(2026, 2030) +LCWRA_AMOUNT = "gov.dwp.universal_credit.elements.disabled.amount" + + +class UCHealthElementOverrideWarning(UserWarning): + """An LCWRA amount reform is superseded by the rebalancing inputs.""" + + +def warn_shadowed_lcwra_reform(sim) -> None: + """Warn once when a reformed amount is read in an active rebalancing year. + + Statutory changes are sampled when annualising the LCWRA parameter; + overrides of the built parameters are read at 1 January. Construction + and Scenario.apply defer this check until all their changes have run, + so a reform that also switches rebalancing off does not warn. + """ + from policyengine_uk.utils.dated_reforms import legacy_override_bounds + + if getattr(sim, "_defer_uc_health_warning", False) or getattr( + sim, "_uc_health_amount_warning_emitted", False + ): + return + system = sim.tax_benefit_system + assignments = [ + change + for change in getattr(system, "dated_assignments", ()) + if change.path == LCWRA_AMOUNT + ] + override_windows = [ + legacy_override_bounds(change, system.parameters) + for change in getattr(system, "legacy_overrides", ()) + if change.path == LCWRA_AMOUNT + ] + if not assignments and not override_windows: + return + rebalancing = system.parameters.gov.dwp.universal_credit.rebalancing + years = [] + for year in UC_REBALANCING_YEARS: + if not rebalancing.active(year): + continue + statutory_sample = instant(f"{year}-{FISCAL_YEAR_SAMPLE_DATE}") + annual_sample = instant(f"{year}-01-01") + if any( + change.start <= statutory_sample <= change.stop for change in assignments + ) or any(start <= annual_sample <= stop for start, stop in override_windows): + years.append(year) + if years: + warnings.warn( + f"A reform to {LCWRA_AMOUNT} is superseded in " + f"{', '.join(map(str, years))} by Universal Credit rebalancing inputs. " + "For a health element reform in these years change " + "gov.dwp.universal_credit.rebalancing.protected_health_element and " + "gov.dwp.universal_credit.rebalancing.new_claimant_health_element.", + UCHealthElementOverrideWarning, + stacklevel=2, + ) + sim._uc_health_amount_warning_emitted = True def _new_claimant_health_element_status( @@ -137,12 +197,12 @@ def add_universal_credit_reform(sim: Microsimulation): 2029: 0.22, } # WPI Economics for Trussell Trust based on admin PIP data, 2025 new_claimant_health_element = rebalancing.new_claimant_health_element - built_from_data = getattr(sim, "built_from_dataset", False) - for year in range(2026, 2030): + is_data = built_from_data(sim) + for year in UC_REBALANCING_YEARS: if not rebalancing.active(year): continue is_post_2025_claimant = _new_claimant_health_element_status( - sim, year, uc_seed < post_2025_claimant_share[year], built_from_data + sim, year, uc_seed < post_2025_claimant_share[year], is_data ) # Copy: calculate returns the simulation's cached array, and the writes # below must reach the cache only through set_input. @@ -164,6 +224,7 @@ def add_universal_credit_reform(sim: Microsimulation): # Standard allowance uplift is handled in the formula itself so user # reforms to the base amount are applied before the uplift. + warn_shadowed_lcwra_reform(sim) universal_credit_july_2025_reform = Scenario( diff --git a/policyengine_uk/simulation.py b/policyengine_uk/simulation.py index 3f769e8950..385be1f50b 100644 --- a/policyengine_uk/simulation.py +++ b/policyengine_uk/simulation.py @@ -234,6 +234,7 @@ def __init__( capital_gains_projection: Optional common baseline gains projection, shared with the reform's baseline. Omit to retain GDP per capita. """ + self._defer_uc_health_warning = True self._cgt_input_provenance = {} self._cgt_defaulted_periods = set() self._cgt_calculation_depth = 0 @@ -253,6 +254,10 @@ def __init__( if reform is not None: scenario = Scenario.from_reform(reform) + # Only a construction-time reference: preserve the existing reading + # of this scenario's single-day leftovers while rejecting unreadable + # overrides from a user modifier's separate from_reform call. + self._building_dated_reforms = scenario.dated_reforms if scenario else None self.branch_name = "default" self.invalidated_caches = set() @@ -369,6 +374,8 @@ def __init__( else: self.baseline = self.clone() self.baseline._cgt_warning_enabled = False + self.baseline._defer_uc_health_warning = False + self.baseline._building_dated_reforms = None self.calculated_periods = [] @@ -378,6 +385,11 @@ def __init__( self._refresh_capital_gains_projection() self._capture_cgt_supplied_inputs() self._cgt_warning_enabled = True + from policyengine_uk.scenarios.uc_reform import warn_shadowed_lcwra_reform + + self._defer_uc_health_warning = False + self._building_dated_reforms = None + warn_shadowed_lcwra_reform(self) def _refresh_capital_gains_projection(self) -> None: projection = self.tax_benefit_system.capital_gains_projection diff --git a/policyengine_uk/tests/test_from_reform_modifier.py b/policyengine_uk/tests/test_from_reform_modifier.py new file mode 100644 index 0000000000..ed7cfb9eef --- /dev/null +++ b/policyengine_uk/tests/test_from_reform_modifier.py @@ -0,0 +1,360 @@ +"""A dated reform's returned modifier works on an already-built simulation. + +Construction still writes statutory dates before annualisation. A modifier +used separately instead follows Scenario.apply's annualised override reading. +""" + +import datetime +import logging + +import numpy as np +import pytest +from hypothesis import HealthCheck, given, settings +from hypothesis import strategies as st +from policyengine_core.reforms import Reform + +from policyengine_uk import Simulation +from policyengine_uk.scenarios.uc_reform import add_universal_credit_reform +from policyengine_uk.utils.dated_reforms import ReformPeriodWarning +from policyengine_uk.utils.scenario import Scenario + + +BASIC_RATE = "gov.hmrc.income_tax.rates.uk[0].rate" +PERSONAL_ALLOWANCE = "gov.hmrc.income_tax.allowances.personal_allowance.amount" +VAT_RATE = "gov.hmrc.vat.standard_rate" +NEW_CLAIMANT_AMOUNT = "gov.dwp.universal_credit.rebalancing.new_claimant_health_element" +YEARS = (2026, 2027) +SITUATION = { + "people": { + "claimant": { + "age": {year: 35 for year in YEARS}, + "uc_limited_capability_for_WRA": {year: True for year in YEARS}, + } + }, + "benunits": { + "benunit": { + "members": ["claimant"], + "would_claim_uc": {year: True for year in YEARS}, + "uc_receives_new_claimant_health_element": {year: True for year in YEARS}, + } + }, + "households": {"household": {"members": ["claimant"]}}, +} +HEALTH_REFORM = { + NEW_CLAIMANT_AMOUNT: { + "2026-01-01.2026-12-31": 438.95, + "2027-01-01.2027-12-31": 455.0, + } +} + + +def parameter(simulation, path, date): + return simulation.tax_benefit_system.parameters.get_child(path)(date) + + +@pytest.fixture(scope="module") +def baseline(): + return Simulation(situation=SITUATION) + + +@pytest.mark.parametrize("reform_class", [False, True]) +def test_scorecard_modifier_changes_parameters_and_lcwra(baseline, reform_class): + simulation = baseline.clone() + original = [simulation.calculate("uc_LCWRA_element", y).copy() for y in YEARS] + reform = Reform.from_dict(HEALTH_REFORM) if reform_class else HEALTH_REFORM + + Scenario.from_reform(reform).simulation_modifier(simulation) + simulation.tax_benefit_system.reset_parameter_caches() + add_universal_credit_reform(simulation) + + for year, monthly, previous in zip(YEARS, (438.95, 455.0), original): + assert parameter(simulation, NEW_CLAIMANT_AMOUNT, str(year)) == monthly + amount = simulation.calculate("uc_LCWRA_element", year) + np.testing.assert_allclose(amount, monthly * 12) + assert np.all(amount > previous) + + +@pytest.mark.parametrize("before_data_load", [False, True]) +def test_scorecard_modifier_wrapped_in_user_scenario(before_data_load): + def modifier(simulation): + Scenario.from_reform(HEALTH_REFORM).simulation_modifier(simulation) + simulation.tax_benefit_system.reset_parameter_caches() + if getattr(simulation, "populations", None) is not None: + add_universal_credit_reform(simulation) + + simulation = Simulation( + situation=SITUATION, + scenario=Scenario( + simulation_modifier=modifier, + applied_before_data_load=before_data_load, + ), + ) + for year, monthly in zip(YEARS, (438.95, 455.0)): + assert parameter(simulation, NEW_CLAIMANT_AMOUNT, str(year)) == monthly + np.testing.assert_allclose( + simulation.calculate("uc_LCWRA_element", year), monthly * 12 + ) + + +@st.composite +def annualised_changes(draw): + path = draw( + st.sampled_from([BASIC_RATE, PERSONAL_ALLOWANCE, VAT_RATE, NEW_CLAIMANT_AMOUNT]) + ) + start = draw(st.dates(datetime.date(2023, 1, 1), datetime.date(2032, 12, 31))) + single_day = draw(st.booleans()) + stop = start if single_day else draw(st.dates(start, datetime.date(2032, 12, 31))) + key = str(start) if single_day else f"{start}.{stop}" + value = draw(st.integers(1, 100_000)) / 100 + contains_new_year = any( + start <= datetime.date(year, 1, 1) <= stop + for year in range(start.year, stop.year + 1) + ) + return path, key, value, contains_new_year + + +@settings( + max_examples=24, + deadline=None, + derandomize=True, + suppress_health_check=[HealthCheck.too_slow], +) +@given(change=annualised_changes()) +def test_modifier_matches_scenario_apply_on_built_parameters(baseline, change): + path, key, value, contains_new_year = change + scenario = Scenario.from_reform({path: {key: value}}) + direct, applied = baseline.clone(), baseline.clone() + if not contains_new_year: + for simulation, operation in ( + (direct, scenario.simulation_modifier), + (applied, scenario.apply), + ): + with pytest.raises(ValueError) as error: + operation(simulation) + assert path in str(error.value) + assert key in str(error.value) + return + + scenario.simulation_modifier(direct) + scenario.apply(applied) + instants = [ + f"{year}-{month_day}" + for year in range(2023, 2033) + for month_day in ("01-01", "04-30", "07-01", "12-31") + ] + for date in instants: + assert parameter(direct, path, date) == parameter(applied, path, date) + + +@pytest.mark.parametrize("operation", ["simulation_modifier", "apply"]) +def test_built_override_warns_about_annualisation(baseline, caplog, operation): + simulation = baseline.clone() + scenario = Scenario.from_reform({BASIC_RATE: {"2026-01-01.2026-12-31": 0.31}}) + with caplog.at_level(logging.WARNING, logger="policyengine_uk.utils.scenario"): + getattr(scenario, operation)(simulation) + assert "dated reform keys are not annualised again" in caplog.text + assert parameter(simulation, BASIC_RATE, "2026-01-01") == 0.31 + + +@pytest.mark.parametrize("operation", ["simulation_modifier", "apply"]) +@pytest.mark.parametrize("key", ["2026-04-06", "2026-04-06.2026-12-31"]) +def test_built_override_rejects_key_that_changes_no_fiscal_year( + baseline, operation, key +): + simulation = baseline.clone() + scenario = Scenario.from_reform({BASIC_RATE: {key: 0.31}}) + with pytest.raises(ValueError) as error: + getattr(scenario, operation)(simulation) + assert BASIC_RATE in str(error.value) + assert key in str(error.value) + + +@pytest.mark.parametrize("before_data_load", [False, True]) +@pytest.mark.parametrize("key", ["2026-04-06", "2026-04-06.2026-12-31"]) +def test_external_modifier_wrapped_at_build_rejects_unreadable_window( + before_data_load, key +): + def modifier(simulation): + Scenario.from_reform({BASIC_RATE: {key: 0.31}}).simulation_modifier(simulation) + + with pytest.raises(ValueError) as error: + Simulation( + situation=SITUATION, + scenario=Scenario( + simulation_modifier=modifier, + applied_before_data_load=before_data_load, + ), + ) + assert BASIC_RATE in str(error.value) + assert key in str(error.value) + + +@pytest.mark.parametrize("before_data_load", [False, True]) +def test_own_single_day_modifier_at_build_retains_the_legacy_reading(before_data_load): + scenario = Scenario.from_reform({BASIC_RATE: {"2026-04-06": 0.31}}) + scenario.applied_before_data_load = before_data_load + with pytest.warns(ReformPeriodWarning): + simulation = Simulation(situation=SITUATION, scenario=scenario) + assert parameter(simulation, BASIC_RATE, "2026-01-01") == 0.20 + assert parameter(simulation, BASIC_RATE, "2026-04-06") == 0.31 + + +@pytest.mark.parametrize("operation", ["simulation_modifier", "apply"]) +def test_invalid_override_reports_every_path_without_partial_changes( + baseline, operation +): + simulation = baseline.clone() + missing = ( + "gov.dwp.universal_credit.no_such_parameter", + "gov.hmrc.no_such_parameter", + ) + before = parameter(simulation, BASIC_RATE, "2026-01-01") + changes = {BASIC_RATE: {"2026-01-01.2026-12-31": 0.31}} + changes.update({path: {"2026-01-01.2026-12-31": 1} for path in missing}) + with pytest.raises(ValueError) as error: + getattr(Scenario.from_reform(changes), operation)(simulation) + for path in missing: + assert path in str(error.value) + assert parameter(simulation, BASIC_RATE, "2026-01-01") == before + + +@pytest.mark.parametrize("operation", ["simulation_modifier", "apply"]) +def test_malformed_keys_report_every_path_without_partial_changes(baseline, operation): + simulation = baseline.clone() + changes = {BASIC_RATE: {"not-a-period": 0.31}, VAT_RATE: {"bad": 0.27}} + before = [parameter(simulation, path, "2026-01-01") for path in changes] + with pytest.raises(ValueError) as error: + getattr(Scenario.from_reform(changes), operation)(simulation) + for path, values in changes.items(): + assert path in str(error.value) + assert next(iter(values)) in str(error.value) + assert [parameter(simulation, path, "2026-01-01") for path in changes] == before + + +def test_modifier_only_overrides_changes_missing_from_the_system(): + written = {BASIC_RATE: {"2026-04-06.2027-04-05": 0.31}} + simulation = Simulation( + situation=SITUATION, + reform=({VAT_RATE: {"2026": 0.22}}, written), + ) + changes = {**written, NEW_CLAIMANT_AMOUNT: HEALTH_REFORM[NEW_CLAIMANT_AMOUNT]} + Scenario.from_reform(changes).simulation_modifier(simulation) + assert parameter(simulation, BASIC_RATE, "2026-01-01") == 0.31 + assert parameter(simulation, BASIC_RATE, "2027-01-01") == 0.20 + assert parameter(simulation, NEW_CLAIMANT_AMOUNT, "2026-01-01") == 438.95 + assert parameter(simulation, NEW_CLAIMANT_AMOUNT, "2027-01-01") == 455.0 + assert {item.path for item in simulation.tax_benefit_system.legacy_overrides} == { + NEW_CLAIMANT_AMOUNT + } + + +def test_reform_from_dict_modifier_preserves_its_onwards_reading(baseline): + simulation = baseline.clone() + scenario = Scenario.from_reform(Reform.from_dict({BASIC_RATE: {"2026": 0.31}})) + scenario.simulation_modifier(simulation) + assert parameter(simulation, BASIC_RATE, "2025-01-01") == 0.20 + for year in range(2026, 2033): + assert parameter(simulation, BASIC_RATE, str(year)) == 0.31 + + +@pytest.mark.parametrize( + "spec", + [ + pytest.param({"ETERNITY": 0.31}, id="eternity"), + pytest.param(0.31, id="scalar"), + pytest.param({"2026": 0.31}, id="bare_year"), + pytest.param({"2026-01": 0.31}, id="january_month"), + pytest.param({"2026-04-06": 0.31}, id="day_onwards"), + ], +) +def test_reform_from_dict_modifier_matches_apply_for_core_period_readings( + baseline, spec +): + scenario = Scenario.from_reform(Reform.from_dict({BASIC_RATE: spec})) + direct, applied = baseline.clone(), baseline.clone() + scenario.simulation_modifier(direct) + scenario.apply(applied) + for year in range(2023, 2033): + for month_day in ("01-01", "01-31", "02-01", "04-06", "12-31"): + date = f"{year}-{month_day}" + assert parameter(direct, BASIC_RATE, date) == parameter( + applied, BASIC_RATE, date + ) + assert ( + parameter( + direct, + BASIC_RATE, + "2026-01-01" + if isinstance(spec, dict) and "2026-01" in spec + else "2027-01-01", + ) + == 0.31 + ) + + +@pytest.mark.parametrize("before_data_load", [False, True]) +@pytest.mark.parametrize("representation", ["dict", "class", "repeated_tuple", "sum"]) +def test_construction_retains_statutory_fiscal_years(before_data_load, representation): + changes = {BASIC_RATE: {"2026-04-06.2027-04-05": 0.31}} + if representation == "class": + scenario = Scenario.from_reform(Reform.from_dict(changes)) + elif representation == "repeated_tuple": + scenario = Scenario.from_reform((changes, changes)) + elif representation == "sum": + scenario = Scenario.from_reform(changes) + Scenario.from_reform(changes) + else: + scenario = Scenario.from_reform(changes) + scenario.applied_before_data_load = before_data_load + simulation = Simulation(situation=SITUATION, scenario=scenario) + assert parameter(simulation, BASIC_RATE, "2025-01-01") == 0.20 + assert parameter(simulation, BASIC_RATE, "2026-01-01") == 0.31 + assert parameter(simulation, BASIC_RATE, "2027-01-01") == 0.20 + assert simulation.tax_benefit_system.legacy_overrides == [] + + +@pytest.mark.parametrize("before_first", [False, True]) +def test_sum_keeps_each_parts_timing_and_copied_dated_reforms(before_first): + seen = [] + before = Scenario.from_reform({BASIC_RATE: {"2026-04-06.2027-04-05": 0.31}}) + before.applied_before_data_load = True + after = Scenario.from_reform({VAT_RATE: {"2027-04-06.2028-04-05": 0.27}}) + before = before + Scenario( + simulation_modifier=lambda sim: seen.append( + getattr(sim, "populations", None) is None + ), + applied_before_data_load=True, + ) + after = after + Scenario( + simulation_modifier=lambda sim: seen.append( + getattr(sim, "populations", None) is None + ) + ) + scenario = before + after if before_first else after + before + simulation = Simulation(situation=SITUATION, scenario=scenario) + assert seen == [True, False] + assert parameter(simulation, BASIC_RATE, "2026-01-01") == 0.31 + assert parameter(simulation, BASIC_RATE, "2027-01-01") == 0.20 + assert parameter(simulation, VAT_RATE, "2027-01-01") == 0.27 + assert simulation.tax_benefit_system.legacy_overrides == [] + + +@pytest.mark.parametrize("copy_kind", ["clone", "branch"]) +def test_modifier_on_copy_of_simulation_built_with_reform_does_not_apply_twice( + copy_kind, +): + changes = {BASIC_RATE: {"2026-04-06.2027-04-05": 0.31}} + original = Simulation(situation=SITUATION, reform=(changes, changes)) + simulation = ( + original.clone() if copy_kind == "clone" else original.get_branch("copy") + ) + dates = [f"{year}-01-01" for year in range(2023, 2033)] + before = [parameter(simulation, BASIC_RATE, date) for date in dates] + assignments = list(simulation.tax_benefit_system.dated_assignments) + + # A fresh Scenario owns a different DatedReform object; the assignment + # written onto the system, including either repeated occurrence, matters. + Scenario.from_reform(changes).simulation_modifier(simulation) + + assert [parameter(simulation, BASIC_RATE, date) for date in dates] == before + assert simulation.tax_benefit_system.dated_assignments == assignments + assert simulation.tax_benefit_system.legacy_overrides == [] diff --git a/policyengine_uk/tests/test_uc_health_draw.py b/policyengine_uk/tests/test_uc_health_draw.py new file mode 100644 index 0000000000..2238955944 --- /dev/null +++ b/policyengine_uk/tests/test_uc_health_draw.py @@ -0,0 +1,86 @@ +"""The UC new-claimant draw follows the data source on MOCK households.""" + +import numpy as np +import pandas as pd +import pytest +from policyengine_core.simulations import Simulation as CoreSimulation + +from policyengine_uk import Microsimulation, Simulation +import policyengine_uk.simulation as uk_simulation +from policyengine_uk.scenarios.uc_reform import add_universal_credit_reform + +pytestmark = pytest.mark.usefixtures("cloned_uk_tax_benefit_system") + +YEARS = range(2026, 2030) +SHARES = (0.11, 0.13, 0.16, 0.22) +COUNT = 1_000 + + +def _mock_frame(): + """One synthetic adult per benefit unit, with all years supplied.""" + ids = np.arange(COUNT) + columns = {} + for year in YEARS: + for variable in ( + "person_id", + "person_benunit_id", + "person_household_id", + "benunit_id", + "household_id", + ): + columns[f"{variable}__{year}"] = ids + columns[f"age__{year}"] = np.full(COUNT, 30) + columns[f"uc_limited_capability_for_WRA__{year}"] = np.ones(COUNT, dtype=bool) + columns[f"household_weight__{year}"] = np.ones(COUNT) + return pd.DataFrame(columns) + + +@pytest.mark.parametrize( + "simulation_class", + [CoreSimulation, Simulation, Microsimulation], + ids=["core", "uk", "uk_micro"], +) +def test_data_built_simulations_keep_the_seed_43_new_claimant_draw(simulation_class): + kwargs = {"dataset": _mock_frame()} + if simulation_class is CoreSimulation: + kwargs["tax_benefit_system"] = uk_simulation.CountryTaxBenefitSystem() + kwargs["default_calculation_period"] = 2026 + sim = simulation_class(**kwargs) + add_universal_credit_reform(sim) + + draws = np.random.default_rng(43).random(COUNT) + for year, share in zip(YEARS, SHARES): + status = np.asarray( + sim.calculate("uc_receives_new_claimant_health_element", year) + ) + np.testing.assert_array_equal(status, draws < share) + assert status.mean() == pytest.approx(share, abs=0.03) + + +@pytest.mark.parametrize( + "simulation_class", + [CoreSimulation, Simulation, Microsimulation], + ids=["core", "uk", "uk_micro"], +) +def test_situations_keep_every_claimant_protected(simulation_class): + situation = { + "people": { + "adult": { + "age": {year: 30 for year in YEARS}, + "uc_limited_capability_for_WRA": {year: True for year in YEARS}, + } + }, + "benunits": {"benunit": {"members": ["adult"]}}, + "households": {"household": {"members": ["adult"]}}, + } + kwargs = {"situation": situation} + if simulation_class is CoreSimulation: + kwargs["tax_benefit_system"] = uk_simulation.CountryTaxBenefitSystem() + kwargs["default_calculation_period"] = 2026 + sim = simulation_class(**kwargs) + add_universal_credit_reform(sim) + + for year in YEARS: + assert not np.asarray( + sim.calculate("uc_receives_new_claimant_health_element", year) + ).any() diff --git a/policyengine_uk/tests/test_uc_health_warnings.py b/policyengine_uk/tests/test_uc_health_warnings.py new file mode 100644 index 0000000000..5d83cd7e38 --- /dev/null +++ b/policyengine_uk/tests/test_uc_health_warnings.py @@ -0,0 +1,252 @@ +"""LCWRA amount changes warn when rebalancing supplies the amount paid.""" + +import warnings +from datetime import date, timedelta + +import numpy as np +import pytest +from hypothesis import HealthCheck, example, given, settings +from hypothesis import strategies as st +from policyengine_core.reforms import Reform + +from policyengine_uk import Scenario, Simulation +import policyengine_uk.scenarios.uc_reform as uc_reform + +pytestmark = pytest.mark.usefixtures("cloned_uk_tax_benefit_system") + +AMOUNT = "gov.dwp.universal_credit.elements.disabled.amount" +ACTIVE = "gov.dwp.universal_credit.rebalancing.active" +PROTECTED = "gov.dwp.universal_credit.rebalancing.protected_health_element" +NEW_CLAIMANT = "gov.dwp.universal_credit.rebalancing.new_claimant_health_element" +YEARS = range(2025, 2033) +SITUATION = { + "people": { + "adult": { + "age": {year: 30 for year in YEARS}, + "uc_limited_capability_for_WRA": {year: True for year in YEARS}, + } + }, + "benunits": {"benunit": {"members": ["adult"]}}, + "households": {"household": {"members": ["adult"]}}, +} + + +def _warning_type(): + # On main the regression must fail because no warning is emitted. + return getattr(uc_reform, "UCHealthElementOverrideWarning", UserWarning) + + +def _via_route(route, changes): + if route == "reform_dict": + return Simulation(situation=SITUATION, reform=changes) + if route == "reform_class": + return Simulation(situation=SITUATION, reform=Reform.from_dict(changes)) + if route == "parameter_changes": + return Simulation( + situation=SITUATION, scenario=Scenario(parameter_changes=changes) + ) + if route == "tuple": + return Simulation(situation=SITUATION, reform=(changes, changes)) + scenario = Scenario.from_reform(changes) + if route == "sum": + scenario = scenario + Scenario.from_reform(changes) + if route == "before_load": + scenario.applied_before_data_load = True + if route in ("apply", "modifier"): + sim = Simulation(situation=SITUATION) + if route == "apply": + scenario.apply(sim) + else: + scenario.simulation_modifier(sim) + return sim + return Simulation(situation=SITUATION, scenario=scenario) + + +ROUTES = ( + "reform_dict", + "scenario", + "reform_class", + "parameter_changes", + "before_load", + "apply", + "modifier", + "tuple", + "sum", +) + + +@pytest.mark.parametrize("route", ROUTES) +def test_amount_reform_warns_once_on_every_route(route): + changes = {AMOUNT: {"2026-01-01.2028-12-31": 600}} + with pytest.warns(_warning_type(), match="protected_health_element") as caught: + _via_route(route, changes) + + health_warnings = [w for w in caught if issubclass(w.category, _warning_type())] + assert len(health_warnings) == 1 + message = str(health_warnings[0].message) + assert PROTECTED in message + assert NEW_CLAIMANT in message + assert all(str(year) in message for year in (2026, 2027, 2028)) + assert "2029" not in message + + +@pytest.mark.parametrize( + "changes", + [ + None, + {NEW_CLAIMANT: {"2027-01-01.2027-12-31": 500}}, + {AMOUNT: {"2025-01-01.2025-12-31": 600}}, + {AMOUNT: {"2030-01-01.2030-12-31": 600}}, + {AMOUNT: {"2027-01-01.2027-12-31": 600}, ACTIVE: False}, + ], +) +def test_baseline_unrelated_outside_and_disabled_reforms_do_not_warn(changes): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + Simulation(situation=SITUATION, reform=changes) + assert not [w for w in caught if issubclass(w.category, _warning_type())] + + +@pytest.mark.parametrize("route", ROUTES) +def test_active_false_in_the_same_reform_suppresses_the_warning(route): + changes = {AMOUNT: {"2027-01-01.2027-12-31": 600}, ACTIVE: False} + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + _via_route(route, changes) + assert not [w for w in caught if issubclass(w.category, _warning_type())] + + +def test_parameter_changes_apply_warns_on_the_built_simulation(): + sim = Simulation(situation=SITUATION) + with pytest.warns(_warning_type(), match="2026, 2027, 2028, 2029"): + Scenario(parameter_changes={AMOUNT: 600}).apply(sim) + + +def test_baseline_clone_can_warn_when_a_reform_is_applied(): + baseline = Simulation(situation=SITUATION).baseline + with pytest.warns(_warning_type(), match="2027"): + Scenario.from_reform({AMOUNT: {"2027-01-01.2027-12-31": 600}}).apply(baseline) + + +def test_the_warning_does_not_change_values(): + baseline = Simulation(situation=SITUATION) + with pytest.warns(_warning_type(), match="protected_health_element"): + reformed = Simulation( + situation=SITUATION, + reform={AMOUNT: {"2026-01-01.2029-12-31": 600}}, + ) + for year in range(2026, 2030): + for variable in ("uc_LCWRA_element", "universal_credit"): + np.testing.assert_array_equal( + reformed.calculate(variable, year), baseline.calculate(variable, year) + ) + + +def test_warning_can_be_escalated_to_an_error(): + with warnings.catch_warnings(): + warnings.simplefilter("error", _warning_type()) + with pytest.raises(_warning_type(), match="new_claimant_health_element"): + Simulation( + situation=SITUATION, + reform={AMOUNT: {"2027-01-01.2027-12-31": 600}}, + ) + + +def test_warning_follows_the_years_used_by_the_rebalancing_loop(monkeypatch): + monkeypatch.setattr(uc_reform, "UC_REBALANCING_YEARS", (2025,), raising=False) + with pytest.warns(_warning_type(), match="2025"): + Simulation( + situation=SITUATION, + reform={AMOUNT: {"2025-01-01.2025-12-31": 600}}, + ) + + +def test_warning_also_covers_an_extended_projection_horizon(monkeypatch): + sim = Simulation( + situation=SITUATION, + reform={AMOUNT: {"2030-01-01.2030-12-31": 600}}, + ) + monkeypatch.setattr(uc_reform, "UC_REBALANCING_YEARS", (2030,), raising=False) + with pytest.warns(_warning_type(), match="2030"): + warn = getattr( + uc_reform, + "warn_shadowed_lcwra_reform", + uc_reform.add_universal_credit_reform, + ) + warn(sim) + + +@pytest.mark.parametrize("key", ["2027", "2027-01-01", "2027-04-06.2028-04-05"]) +def test_warning_uses_the_route_reading_of_period_keys(key): + with pytest.warns(_warning_type(), match="2027") as caught: + Simulation(situation=SITUATION, reform={AMOUNT: {key: 600}}) + message = str(caught[0].message) + assert "2026" not in message + assert "2028" not in message + + +def test_warning_names_only_the_years_still_active(): + with pytest.warns(_warning_type(), match="2026, 2028") as caught: + Simulation( + situation=SITUATION, + reform={ + AMOUNT: {"2026-01-01.2028-12-31": 600}, + ACTIVE: {"2027-01-01.2027-12-31": False}, + }, + ) + assert "2027" not in str(caught[0].message) + + +def test_tuple_and_sum_defer_warning_until_active_override_runs(): + # Single-day changes run as overrides after the construction-time UC + # modifier. They must be checked after the whole tuple or sum has run. + amount = {AMOUNT: {"2027-01-01": 600}} + disabled = {ACTIVE: {"2027-01-01": False}} + for scenario in ( + Scenario.from_reform((amount, disabled)), + Scenario.from_reform(amount) + Scenario.from_reform(disabled), + ): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + Simulation(situation=SITUATION, scenario=scenario) + assert not [w for w in caught if issubclass(w.category, _warning_type())] + + +@settings( + max_examples=25, + deadline=None, + derandomize=True, + suppress_health_check=[HealthCheck.too_slow], +) +@example(start=date(2027, 4, 30), length=0, active=[True] * 4) +@example(start=date(2027, 4, 29), length=0, active=[True] * 4) +@given( + start=st.dates(min_value=date(2023, 1, 1), max_value=date(2032, 12, 31)), + length=st.integers(0, 1_100), + active=st.lists(st.booleans(), min_size=4, max_size=4), +) +def test_warning_iff_a_reformed_window_reaches_an_active_rebalancing_year( + start, length, active +): + stop = start + timedelta(days=length) + changes = { + AMOUNT: {f"{start}.{stop}": 600}, + ACTIVE: { + f"{year}-01-01.{year}-12-31": enabled + for year, enabled in zip(range(2026, 2030), active) + }, + } + expected_years = [ + year + for year, enabled in zip(range(2026, 2030), active) + if enabled and start <= date(year, 4, 30) <= stop + ] + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + Simulation(situation=SITUATION, reform=changes) + health_warnings = [w for w in caught if issubclass(w.category, _warning_type())] + assert bool(health_warnings) == bool(expected_years) + if expected_years: + assert len(health_warnings) == 1 + message = str(health_warnings[0].message) + assert all(str(year) in message for year in expected_years) diff --git a/policyengine_uk/utils/dated_reforms.py b/policyengine_uk/utils/dated_reforms.py index 4b4e97c3c9..b21ac506c9 100644 --- a/policyengine_uk/utils/dated_reforms.py +++ b/policyengine_uk/utils/dated_reforms.py @@ -483,6 +483,131 @@ def apply_legacy_overrides( ) +def legacy_override_update_kwargs(override: LegacyOverride, parameters) -> dict: + """The existing annualised-override reading, without changing the tree.""" + from policyengine_core.reforms.reform import _period_key_update_kwargs + + from policyengine_uk.utils.parameters import uk_fiscal_year_period + + if override.reading == ALL_PERIODS: + return {"period": None, "value": override.value} + if override.reading == REFORM_FROM_DICT: + if override.key is None: + return {"period": CORE_SCALAR_PERIOD, "value": override.value} + return _period_key_update_kwargs(override.key, override.value) + if override.key is None: + return {"start": REFORM_DICT_SCALAR_START, "value": override.value} + if "." in override.key: + start, stop = interval_bounds(override.key) + return {"start": start, "stop": stop, "value": override.value} + target = parameters.get_child(override.path) + return { + "period": ( + uk_fiscal_year_period(override.key) + if target.metadata.get("preserve_calendar_dates") + else period(override.key) + ), + "value": override.value, + } + + +def legacy_override_bounds(override: LegacyOverride, parameters): + """Inclusive dates the annualised override edits (not statutory dates).""" + kwargs = legacy_override_update_kwargs(override, parameters) + if "period" in kwargs: + if kwargs["period"] is None: + return EARLIEST, FLAT_STOP + parsed = period(kwargs["period"]) + return parsed.start, parsed.stop + return instant(kwargs.get("start") or EARLIEST), instant( + kwargs.get("stop") or FLAT_STOP + ) + + +def validate_annualised_overrides( + parameters, overrides, require_annual_value=True +) -> None: + """Reject every invalid or annually unreadable override before editing.""" + errors = [] + for override in overrides: + try: + target = parameters.get_child(override.path) + start, stop = legacy_override_bounds(override, parameters) + first_year = start.year + ((start.month, start.day) > (1, 1)) + if require_annual_value and instant(f"{first_year:04d}-01-01") > stop: + raise ValueError("the key contains no 1 January annual value") + probe = target.clone() + probe.parent = ParameterNode("override_validation", data={}) + probe.update(**legacy_override_update_kwargs(override, parameters)) + except Exception as error: + errors.append(f"{override.path} (key {override.key!r}): {error}") + if errors: + raise ValueError("Cannot apply annualised overrides: " + "; ".join(errors)) + + +def read_override_reform(changes, reading, defines, calendar_years=None, warn=False): + """Read a modifier's reform, naming every path with an invalid key.""" + + def read(spec): + if reading == REFORM_DICT: + return read_reform_dict( + spec, defines, warn=warn, calendar_years=calendar_years + ) + return read_reform_from_dict(spec, defines) + + try: + return read(changes) + except (ValueError, TypeError, KeyError) as original: + errors = [] + for path, spec in changes.items(): + entries = spec.items() if isinstance(spec, Mapping) else [(None, spec)] + for key, value in entries: + try: + read({path: {key: value} if isinstance(spec, Mapping) else value}) + except (ValueError, TypeError, KeyError) as error: + errors.append(f"{path} (key {key!r}): {error}") + raise ValueError( + "Cannot read reform overrides: " + "; ".join(errors) + ) from original + + +def unwritten_reform_overrides(changes, reading, defines, written, calendar_years): + """Keep legacy changes and changes absent from the statutory write log. + + Compare assignment contents, ignoring order: scenario splitting copies + dated reforms, repeated tuple entries have different orders, and cloned + systems retain their assignments. Generated parameter paths use the same + ``defines`` predicate as the construction route. + """ + + def read(spec, predicate, warn=False): + return read_override_reform(spec, reading, predicate, calendar_years, warn) + + assignments, legacy = read(changes, defines, warn=True) + + def was_written(assignment): + return any( + (assignment.path, assignment.start, assignment.stop, assignment.value) + == (entry.path, entry.start, entry.stop, entry.value) + for entry in written + ) + + if all(was_written(assignment) for assignment in assignments): + return legacy, False + # Select each original key rather than converting assignments back to + # keys: annualised overrides retain the route's own bare-year reading. + overrides = [] + for path, spec in changes.items(): + entries = spec.items() if isinstance(spec, Mapping) else [(None, spec)] + for key, value in entries: + change = {path: {key: value} if isinstance(spec, Mapping) else value} + selected, _ = read(change, defines) + if not selected or not all(was_written(item) for item in selected): + _, pending = read(change, lambda path: False) + overrides.extend(pending) + return overrides, True + + Timeline = List[ParameterAtInstant] diff --git a/policyengine_uk/utils/scenario.py b/policyengine_uk/utils/scenario.py index 310dc94275..6dcab246c5 100644 --- a/policyengine_uk/utils/scenario.py +++ b/policyengine_uk/utils/scenario.py @@ -14,6 +14,9 @@ apply_legacy_overrides, read_reform_dict, read_reform_from_dict, + read_override_reform, + unwritten_reform_overrides, + validate_annualised_overrides, ) from policyengine_uk.utils.parameters import check_parameter_not_removed @@ -69,13 +72,7 @@ def _is_reform_from_dict(reform: Type[Reform]) -> bool: def _legacy_override_modifier(dated: DatedReform) -> Callable[[Simulation], None]: - """Apply what a dated reform leaves to the annualised parameters. - - That is single-day or single-month reform-dict keys and paths the - statutory tree does not define, such as a parameter a structural reform - in the same tuple adds. The simulation applies the rest from the - statutory timeline before any modifier runs. - """ + """Override changes the simulation has not written on its timeline.""" def modifier(simulation: Simulation) -> None: from policyengine_uk.tax_benefit_system import ( @@ -86,19 +83,50 @@ def modifier(simulation: Simulation) -> None: # Scenario.apply edits an already-built simulation's annualised # parameters, so there every change of the reform is an override. every_change = getattr(simulation, APPLYING_TO_BUILT_SIMULATION, False) - defines = (lambda path: False) if every_change else reform_parameter_defines() - _, legacy = dated.read( - defines, - warn=not every_change, - calendar_years=parameter_reads_calendar_years(), - ) - if not legacy: - return - _add_overrides(simulation, legacy) + if every_change: + _, legacy = read_override_reform( + dated.changes, dated.reading, lambda path: False + ) + overriding = True + else: + legacy, overriding = unwritten_reform_overrides( + dated.changes, + dated.reading, + reform_parameter_defines(), + getattr(simulation.tax_benefit_system, "dated_assignments", ()), + parameter_reads_calendar_years(), + ) + overriding = overriding or ( + bool(legacy) + and dated + not in (getattr(simulation, "_building_dated_reforms", None) or ()) + ) + if legacy: + if overriding: + if not every_change: + _warn_annualised_overrides() + validate_annualised_overrides( + simulation.tax_benefit_system.parameters, + legacy, + require_annual_value=overriding, + ) + _add_overrides(simulation, legacy) + from policyengine_uk.scenarios.uc_reform import warn_shadowed_lcwra_reform + + warn_shadowed_lcwra_reform(simulation) return modifier +def _warn_annualised_overrides() -> None: + logger.warning( + "Scenario.apply edits the built simulation's annualised " + "parameters, so its dated reform keys are not annualised " + "again. Pass the scenario when constructing the Simulation to " + "apply them on the statutory timeline." + ) + + def _add_overrides(simulation: Simulation, overrides: List[LegacyOverride]) -> None: system = simulation.tax_benefit_system if hasattr(system, "add_legacy_overrides"): @@ -483,12 +511,7 @@ def apply(self, simulation: Simulation) -> None: simulation: The simulation to modify """ if self.dated_reforms: - logger.warning( - "Scenario.apply edits the built simulation's annualised " - "parameters, so its dated reform keys are not annualised " - "again. Pass the scenario when constructing the Simulation to " - "apply them on the statutory timeline." - ) + _warn_annualised_overrides() # Apply parameter changes first overrides: List[LegacyOverride] = [] @@ -525,11 +548,14 @@ def apply(self, simulation: Simulation) -> None: # Then apply simulation modifier if self.simulation_modifier: + deferred = getattr(simulation, "_defer_uc_health_warning", False) + simulation._defer_uc_health_warning = True setattr(simulation, APPLYING_TO_BUILT_SIMULATION, True) try: self.simulation_modifier(simulation) finally: delattr(simulation, APPLYING_TO_BUILT_SIMULATION) + simulation._defer_uc_health_warning = deferred # UK simulations derive paired CGT series after modifiers run. # Keep Scenario compatible with Core and other country simulations. refresh_projection = getattr( @@ -537,6 +563,9 @@ def apply(self, simulation: Simulation) -> None: ) if callable(refresh_projection): refresh_projection() + from policyengine_uk.scenarios.uc_reform import warn_shadowed_lcwra_reform + + warn_shadowed_lcwra_reform(simulation) def __repr__(self) -> str: """String representation of the Scenario.""" From c2e1d5873950248b6f2bcfa557c335c22158671d Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Thu, 15 Oct 2026 16:26:27 -0400 Subject: [PATCH 2/9] Preserve atomic rollback for malformed Core reform keys --- .../tests/test_from_reform_modifier.py | 17 +++++++++++++++++ policyengine_uk/utils/dated_reforms.py | 14 +++++++++++--- policyengine_uk/utils/scenario.py | 11 ++++++++++- 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/policyengine_uk/tests/test_from_reform_modifier.py b/policyengine_uk/tests/test_from_reform_modifier.py index ed7cfb9eef..199eabc535 100644 --- a/policyengine_uk/tests/test_from_reform_modifier.py +++ b/policyengine_uk/tests/test_from_reform_modifier.py @@ -231,6 +231,23 @@ def test_malformed_keys_report_every_path_without_partial_changes(baseline, oper assert [parameter(simulation, path, "2026-01-01") for path in changes] == before +@pytest.mark.parametrize("operation", ["simulation_modifier", "apply"]) +def test_malformed_core_keys_roll_back_and_report_every_path(baseline, operation): + simulation = baseline.clone() + malformed = {BASIC_RATE: {"not-a-period": 0.31}, VAT_RATE: {"bad": 0.27}} + changes = {PERSONAL_ALLOWANCE: {"2026": 13_000}, **malformed} + before = [parameter(simulation, path, "2026-01-01") for path in changes] + overrides = list(simulation.tax_benefit_system.legacy_overrides) + scenario = Scenario.from_reform(Reform.from_dict(changes)) + with pytest.raises(ValueError) as error: + getattr(scenario, operation)(simulation) + for path, values in malformed.items(): + assert path in str(error.value) + assert next(iter(values)) in str(error.value) + assert [parameter(simulation, path, "2026-01-01") for path in changes] == before + assert simulation.tax_benefit_system.legacy_overrides == overrides + + def test_modifier_only_overrides_changes_missing_from_the_system(): written = {BASIC_RATE: {"2026-04-06.2027-04-05": 0.31}} simulation = Simulation( diff --git a/policyengine_uk/utils/dated_reforms.py b/policyengine_uk/utils/dated_reforms.py index b21ac506c9..687630442e 100644 --- a/policyengine_uk/utils/dated_reforms.py +++ b/policyengine_uk/utils/dated_reforms.py @@ -525,14 +525,22 @@ def legacy_override_bounds(override: LegacyOverride, parameters): def validate_annualised_overrides( - parameters, overrides, require_annual_value=True + parameters, overrides, require_annual_value=True, defer_invalid_keys=False ) -> None: - """Reject every invalid or annually unreadable override before editing.""" + """Validate overrides, optionally deferring Core key errors to rollback.""" errors = [] for override in overrides: try: target = parameters.get_child(override.path) - start, stop = legacy_override_bounds(override, parameters) + try: + start, stop = legacy_override_bounds(override, parameters) + except (ValueError, TypeError, KeyError): + # Core leaves from_dict keys unparsed until the atomic + # override batch. Keep that rollback path, then report all + # bad keys after the batch restores the system. + if defer_invalid_keys and override.reading == REFORM_FROM_DICT: + continue + raise first_year = start.year + ((start.month, start.day) > (1, 1)) if require_annual_value and instant(f"{first_year:04d}-01-01") > stop: raise ValueError("the key contains no 1 January annual value") diff --git a/policyengine_uk/utils/scenario.py b/policyengine_uk/utils/scenario.py index 6dcab246c5..cf423b981b 100644 --- a/policyengine_uk/utils/scenario.py +++ b/policyengine_uk/utils/scenario.py @@ -109,8 +109,17 @@ def modifier(simulation: Simulation) -> None: simulation.tax_benefit_system.parameters, legacy, require_annual_value=overriding, + defer_invalid_keys=True, ) - _add_overrides(simulation, legacy) + try: + _add_overrides(simulation, legacy) + except (ValueError, TypeError, KeyError): + validate_annualised_overrides( + simulation.tax_benefit_system.parameters, + legacy, + require_annual_value=overriding, + ) + raise from policyengine_uk.scenarios.uc_reform import warn_shadowed_lcwra_reform warn_shadowed_lcwra_reform(simulation) From 5d1faea8bfec5a9c9468a09681e0cc9384ffb1fa Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Thu, 15 Oct 2026 16:33:01 -0400 Subject: [PATCH 3/9] Share the UC rebalancing years name with the horizon fix --- policyengine_uk/scenarios/uc_reform.py | 6 +++--- policyengine_uk/tests/test_uc_health_warnings.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/policyengine_uk/scenarios/uc_reform.py b/policyengine_uk/scenarios/uc_reform.py index bac5daed80..9aa3da7be3 100644 --- a/policyengine_uk/scenarios/uc_reform.py +++ b/policyengine_uk/scenarios/uc_reform.py @@ -100,7 +100,7 @@ def _protected_existing_health_element_monthly( NEW_CLAIMANT_STATUS = "uc_receives_new_claimant_health_element" -UC_REBALANCING_YEARS = range(2026, 2030) +REBALANCING_YEARS = range(2026, 2030) LCWRA_AMOUNT = "gov.dwp.universal_credit.elements.disabled.amount" @@ -137,7 +137,7 @@ def warn_shadowed_lcwra_reform(sim) -> None: return rebalancing = system.parameters.gov.dwp.universal_credit.rebalancing years = [] - for year in UC_REBALANCING_YEARS: + for year in REBALANCING_YEARS: if not rebalancing.active(year): continue statutory_sample = instant(f"{year}-{FISCAL_YEAR_SAMPLE_DATE}") @@ -198,7 +198,7 @@ def add_universal_credit_reform(sim: Microsimulation): } # WPI Economics for Trussell Trust based on admin PIP data, 2025 new_claimant_health_element = rebalancing.new_claimant_health_element is_data = built_from_data(sim) - for year in UC_REBALANCING_YEARS: + for year in REBALANCING_YEARS: if not rebalancing.active(year): continue is_post_2025_claimant = _new_claimant_health_element_status( diff --git a/policyengine_uk/tests/test_uc_health_warnings.py b/policyengine_uk/tests/test_uc_health_warnings.py index 5d83cd7e38..16e992aff3 100644 --- a/policyengine_uk/tests/test_uc_health_warnings.py +++ b/policyengine_uk/tests/test_uc_health_warnings.py @@ -153,7 +153,7 @@ def test_warning_can_be_escalated_to_an_error(): def test_warning_follows_the_years_used_by_the_rebalancing_loop(monkeypatch): - monkeypatch.setattr(uc_reform, "UC_REBALANCING_YEARS", (2025,), raising=False) + monkeypatch.setattr(uc_reform, "REBALANCING_YEARS", (2025,), raising=False) with pytest.warns(_warning_type(), match="2025"): Simulation( situation=SITUATION, @@ -166,7 +166,7 @@ def test_warning_also_covers_an_extended_projection_horizon(monkeypatch): situation=SITUATION, reform={AMOUNT: {"2030-01-01.2030-12-31": 600}}, ) - monkeypatch.setattr(uc_reform, "UC_REBALANCING_YEARS", (2030,), raising=False) + monkeypatch.setattr(uc_reform, "REBALANCING_YEARS", (2030,), raising=False) with pytest.warns(_warning_type(), match="2030"): warn = getattr( uc_reform, From 4785c9918586cf666c4d2bb9d41b0739cb69b065 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Thu, 15 Oct 2026 16:39:06 -0400 Subject: [PATCH 4/9] Keep the shared UC horizon declaration beside module imports --- policyengine_uk/scenarios/uc_reform.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/policyengine_uk/scenarios/uc_reform.py b/policyengine_uk/scenarios/uc_reform.py index 9aa3da7be3..9f1332c365 100644 --- a/policyengine_uk/scenarios/uc_reform.py +++ b/policyengine_uk/scenarios/uc_reform.py @@ -7,6 +7,9 @@ from policyengine_core.periods import instant +REBALANCING_YEARS = range(2026, 2030) + + # The standard allowance amounts the protected LCWRA amount is paired with in # the Universal Credit Act 2025 s. 4(2) duty: every amount of the allowance. STANDARD_ALLOWANCE_TYPES = ( @@ -100,7 +103,6 @@ def _protected_existing_health_element_monthly( NEW_CLAIMANT_STATUS = "uc_receives_new_claimant_health_element" -REBALANCING_YEARS = range(2026, 2030) LCWRA_AMOUNT = "gov.dwp.universal_credit.elements.disabled.amount" From 8ca68869f943262f54f6ca3a4057a558fd3f70a3 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Fri, 16 Oct 2026 03:34:43 -0400 Subject: [PATCH 5/9] Preserve effective order in standalone dated-reform modifiers --- changelog.d/2426.fixed.md | 1 + docs/book/usage/scenarios.md | 3 +- .../tests/test_from_reform_modifier.py | 251 +++++++++++++++++- policyengine_uk/utils/dated_reforms.py | 209 +++++++++++++-- policyengine_uk/utils/scenario.py | 34 ++- 5 files changed, 468 insertions(+), 30 deletions(-) diff --git a/changelog.d/2426.fixed.md b/changelog.d/2426.fixed.md index b0fd5ea87c..4c4b39b063 100644 --- a/changelog.d/2426.fixed.md +++ b/changelog.d/2426.fixed.md @@ -1 +1,2 @@ - Fixed standalone dated-reform modifiers silently dropping parameter changes, warned when UC rebalancing supersedes an LCWRA amount reform, and restored new-claimant draws for data-built core simulations. +- Preserved standalone modifier order across repeated and reordered reforms and retained no-op behavior for already-effective reforms on copies. diff --git a/docs/book/usage/scenarios.md b/docs/book/usage/scenarios.md index f2065430d4..3900098463 100644 --- a/docs/book/usage/scenarios.md +++ b/docs/book/usage/scenarios.md @@ -418,7 +418,8 @@ Processing also builds some parameters from others: the triple lock rate (`gov.e A dataset's later years are projected from its first by the economic assumptions as the simulation is built. Dated changes that change what that projection reads (a growth rate, say, through `parameter_changes`, `"start.stop"` or bare-year keys, or a 1 January key on a calendar-year series) are written before the data is built, so incomes, rents and council tax follow them, as with `applied_before_data_load=True`. Other dated changes come just after the data is built, before the steps that read parameters while the simulation is built: the Universal Credit rebalancing awards and the structural reforms that parameters switch on. Other single-day keys and a scenario's own simulation modifier run after those steps, as they always have, and leave the data as built. Datasets loaded from `hf://` or `gs://` are cached once projected, under current law only: a simulation whose projection inputs differ builds its own, so a scenario never reaches its baseline or a later simulation. A sum with a part applied before data load applies all its parameter changes before the data is built (see "Combining scenarios" above). All of this applies when the scenario or reform is passed to `Simulation(...)`. `Scenario.apply(simulation)` on a simulation that is already built edits its annualised values instead, as it always has: a `"2027-04-06.2028-04-05"` key there overrides those dates of the fiscal-year values, so it does not set fiscal year 2027, and the model logs a warning saying so. Pass the scenario when constructing the simulation for dated measures. -The modifier returned by `Scenario.from_reform` behaves the same way on a built simulation for changes not already written onto its statutory timeline, and raises if a key contains no 1 January annual value. +The modifier returned by `Scenario.from_reform` behaves the same way on a built simulation for changes that are no longer effective, and raises if an external override key contains no 1 January annual value. +An already-effective reform is a no-op on a copy, clone or branch, including legacy single-day keys; composed reforms compare final writes in their own order and account for later overrides. ### Creating complex scenarios with simulation modifiers diff --git a/policyengine_uk/tests/test_from_reform_modifier.py b/policyengine_uk/tests/test_from_reform_modifier.py index 199eabc535..4ed1581e2a 100644 --- a/policyengine_uk/tests/test_from_reform_modifier.py +++ b/policyengine_uk/tests/test_from_reform_modifier.py @@ -19,6 +19,8 @@ from policyengine_uk.utils.scenario import Scenario +pytestmark = pytest.mark.usefixtures("cloned_uk_tax_benefit_system") + BASIC_RATE = "gov.hmrc.income_tax.rates.uk[0].rate" PERSONAL_ALLOWANCE = "gov.hmrc.income_tax.allowances.personal_allowance.amount" VAT_RATE = "gov.hmrc.vat.standard_rate" @@ -53,10 +55,19 @@ def parameter(simulation, path, date): @pytest.fixture(scope="module") -def baseline(): +def baseline(cloned_uk_tax_benefit_system): return Simulation(situation=SITUATION) +def _sequence_scenario(changes, representation="tuple"): + if representation == "tuple": + return Scenario.from_reform(tuple(changes)) + scenario = Scenario() + for change in changes: + scenario = scenario + Scenario.from_reform(change) + return scenario + + @pytest.mark.parametrize("reform_class", [False, True]) def test_scorecard_modifier_changes_parameters_and_lcwra(baseline, reform_class): simulation = baseline.clone() @@ -375,3 +386,241 @@ def test_modifier_on_copy_of_simulation_built_with_reform_does_not_apply_twice( assert [parameter(simulation, BASIC_RATE, date) for date in dates] == before assert simulation.tax_benefit_system.dated_assignments == assignments assert simulation.tax_benefit_system.legacy_overrides == [] + + +@pytest.mark.parametrize("representation", ["tuple", "sum"]) +@pytest.mark.parametrize("case", ["reordered", "repeated_after_override"]) +def test_composite_modifier_respects_order_of_historical_changes(case, representation): + first = {BASIC_RATE: {"2027-01-01.2027-12-31": 0.31}} + second = {BASIC_RATE: {"2027-01-01.2027-12-31": 0.35}} + constructed, incoming = ( + ((first, second), (second, first)) + if case == "reordered" + else ((first,), (first, second, first)) + ) + original = Simulation(situation=SITUATION, reform=constructed) + direct, applied = original.clone(), original.clone() + scenario = _sequence_scenario(incoming, representation) + + scenario.simulation_modifier(direct) + scenario.apply(applied) + + for date in ("2026-12-31", "2027-01-01", "2027-07-01", "2027-12-31", "2028-01-01"): + assert parameter(direct, BASIC_RATE, date) == parameter( + applied, BASIC_RATE, date + ) + assert parameter(direct, BASIC_RATE, "2027-01-01") == 0.31 + + +def test_modifier_reapplies_historical_change_after_a_later_legacy_override(): + first = {BASIC_RATE: {"2027-01-01.2027-12-31": 0.31}} + second = {BASIC_RATE: {"2027-01-01.2027-12-31": 0.35}} + original = Simulation(situation=SITUATION, reform=first) + Scenario.from_reform(second).apply(original) + assert parameter(original, BASIC_RATE, "2027-01-01") == 0.35 + direct, applied = original.clone(), original.clone() + scenario = Scenario.from_reform(first) + + scenario.simulation_modifier(direct) + scenario.apply(applied) + + for date in ("2026-12-31", "2027-01-01", "2027-12-31", "2028-01-01"): + assert parameter(direct, BASIC_RATE, date) == parameter( + applied, BASIC_RATE, date + ) + assert parameter(direct, BASIC_RATE, "2027-01-01") == 0.31 + + +def test_modifier_reapplies_historical_change_after_a_direct_parameter_update(): + changes = {BASIC_RATE: {"2027-01-01.2027-12-31": 0.31}} + original = Simulation(situation=SITUATION, reform=changes) + original.tax_benefit_system.parameters.get_child(BASIC_RATE).update( + period="2027", value=0.35 + ) + original.tax_benefit_system.reset_parameter_caches() + assert parameter(original, BASIC_RATE, "2027-01-01") == 0.35 + direct, applied = original.clone(), original.clone() + scenario = Scenario.from_reform(changes) + + scenario.simulation_modifier(direct) + scenario.apply(applied) + + for date in ("2026-12-31", "2027-01-01", "2027-07-01", "2027-12-31", "2028-01-01"): + assert parameter(direct, BASIC_RATE, date) == parameter( + applied, BASIC_RATE, date + ) + assert parameter(direct, BASIC_RATE, "2027-01-01") == 0.31 + + +def test_modifier_replays_later_keys_when_an_earlier_window_needs_reapplication(): + changes = { + BASIC_RATE: { + "2026-01-01.2027-12-31": 0.31, + "2027-01-01.2027-12-31": 0.35, + } + } + original = Simulation(situation=SITUATION, reform=changes) + original.tax_benefit_system.parameters.get_child(BASIC_RATE).update( + period="2026", value=0.25 + ) + original.tax_benefit_system.reset_parameter_caches() + assert parameter(original, BASIC_RATE, "2026-01-01") == 0.25 + direct, applied = original.clone(), original.clone() + scenario = Scenario.from_reform(changes) + + scenario.simulation_modifier(direct) + scenario.apply(applied) + + for date in ( + "2025-12-31", + "2026-01-01", + "2026-07-01", + "2026-12-31", + "2027-01-01", + "2027-07-01", + "2027-12-31", + "2028-01-01", + ): + assert parameter(direct, BASIC_RATE, date) == parameter( + applied, BASIC_RATE, date + ) + assert parameter(direct, BASIC_RATE, "2026-01-01") == 0.31 + assert parameter(direct, BASIC_RATE, "2027-01-01") == 0.35 + + +@pytest.mark.parametrize( + "later_window", + ["2027-01-01.2027-12-31", "2027-07-01.2028-06-30"], +) +def test_modifier_reapplies_historical_change_with_a_partially_overridden_window( + later_window, +): + first = {BASIC_RATE: {"2026-01-01.2028-12-31": 0.31}} + later = {BASIC_RATE: {later_window: 0.35}} + original = Simulation(situation=SITUATION, reform=(first, later)) + direct, applied = original.clone(), original.clone() + scenario = Scenario.from_reform(first) + + scenario.simulation_modifier(direct) + scenario.apply(applied) + + for year in range(2025, 2030): + for month_day in ("01-01", "04-06", "06-30", "07-01", "12-31"): + date = f"{year}-{month_day}" + assert parameter(direct, BASIC_RATE, date) == parameter( + applied, BASIC_RATE, date + ) + if 2026 <= year <= 2028: + assert parameter(direct, BASIC_RATE, date) == 0.31 + + +@pytest.mark.parametrize("copy_kind", ["clone", "branch"]) +def test_own_legacy_single_day_modifier_on_copy_is_a_noop(copy_kind, caplog): + changes = {BASIC_RATE: {"2027-04-06": 0.31}} + with pytest.warns(ReformPeriodWarning): + original = Simulation(situation=SITUATION, reform=changes) + simulation = ( + original.clone() if copy_kind == "clone" else original.get_branch("copy") + ) + dates = ("2027-01-01", "2027-04-05", "2027-04-06", "2027-04-07", "2028-01-01") + before = [parameter(simulation, BASIC_RATE, date) for date in dates] + overrides = list(simulation.tax_benefit_system.legacy_overrides) + + with caplog.at_level(logging.WARNING, logger="policyengine_uk.utils.scenario"): + Scenario.from_reform(changes).simulation_modifier(simulation) + + assert [parameter(simulation, BASIC_RATE, date) for date in dates] == before + assert simulation.tax_benefit_system.legacy_overrides == overrides + assert "dated reform keys are not annualised again" not in caplog.text + + +@pytest.mark.parametrize("copy_kind", ["clone", "branch"]) +@pytest.mark.parametrize("representation", ["tuple", "sum"]) +@pytest.mark.parametrize("key", ["2027-01-01.2027-12-31", "2027-04-06.2028-04-05"]) +def test_own_composite_modifier_on_copy_preserves_order_without_reapplying( + copy_kind, representation, key, caplog +): + changes = ({BASIC_RATE: {key: 0.31}}, {BASIC_RATE: {key: 0.35}}) + original = Simulation( + situation=SITUATION, + scenario=_sequence_scenario(changes, representation), + ) + simulation = ( + original.clone() if copy_kind == "clone" else original.get_branch("copy") + ) + dates = [ + f"{year}-{month_day}" + for year in range(2026, 2030) + for month_day in ("01-01", "04-05", "04-06", "12-31") + ] + before = [parameter(simulation, BASIC_RATE, date) for date in dates] + assignments = list(simulation.tax_benefit_system.dated_assignments) + overrides = list(simulation.tax_benefit_system.legacy_overrides) + + with caplog.at_level(logging.WARNING, logger="policyengine_uk.utils.scenario"): + _sequence_scenario(changes, representation).simulation_modifier(simulation) + + assert [parameter(simulation, BASIC_RATE, date) for date in dates] == before + assert simulation.tax_benefit_system.dated_assignments == assignments + assert simulation.tax_benefit_system.legacy_overrides == overrides == [] + assert "dated reform keys are not annualised again" not in caplog.text + + +@st.composite +def overlapping_reform_sequences(draw): + # A small pool makes shared paths/windows, repeated entries and different + # orders common, including writes that were present only earlier in history. + entries = draw( + st.lists( + st.tuples( + st.sampled_from([BASIC_RATE, VAT_RATE]), + st.sampled_from( + [ + "2026-01-01.2026-12-31", + "2027-01-01.2027-12-31", + "2026-01-01.2027-12-31", + ] + ), + st.sampled_from([0.25, 0.31, 0.35]), + ), + min_size=2, + max_size=4, + unique=True, + ) + ) + index = st.integers(0, len(entries) - 1) + constructed = draw(st.lists(index, min_size=1, max_size=5)) + incoming = draw(st.lists(index, min_size=2, max_size=6)) + + def changes(order): + return tuple({entries[i][0]: {entries[i][1]: entries[i][2]}} for i in order) + + return changes(constructed), changes(incoming) + + +@pytest.mark.parametrize("prebuilt", [False, True]) +@settings( + max_examples=32, + deadline=None, + derandomize=True, + suppress_health_check=[HealthCheck.too_slow], +) +@given(sequences=overlapping_reform_sequences()) +def test_sequence_modifier_matches_apply_with_repeated_and_reordered_writes( + baseline, prebuilt, sequences +): + constructed, incoming = sequences + original = ( + Simulation(situation=SITUATION, reform=constructed) if prebuilt else baseline + ) + direct, applied = original.clone(), original.clone() + scenario = _sequence_scenario(incoming) + + scenario.simulation_modifier(direct) + scenario.apply(applied) + + for path in (BASIC_RATE, VAT_RATE): + for year in range(2025, 2030): + for month_day in ("01-01", "04-05", "04-06", "06-30", "07-01", "12-31"): + date = f"{year}-{month_day}" + assert parameter(direct, path, date) == parameter(applied, path, date) diff --git a/policyengine_uk/utils/dated_reforms.py b/policyengine_uk/utils/dated_reforms.py index 687630442e..ff63ce0676 100644 --- a/policyengine_uk/utils/dated_reforms.py +++ b/policyengine_uk/utils/dated_reforms.py @@ -579,41 +579,196 @@ def read(spec): ) from original -def unwritten_reform_overrides(changes, reading, defines, written, calendar_years): - """Keep legacy changes and changes absent from the statutory write log. +def _assignment_boundaries(assignments): + """Instants at which the effective value of a write sequence can change.""" + return { + boundary + for entry in assignments + for boundary in (entry.start, entry.stop.offset(1, "day")) + } - Compare assignment contents, ignoring order: scenario splitting copies - dated reforms, repeated tuple entries have different orders, and cloned - systems retain their assignments. Generated parameter paths use the same - ``defines`` predicate as the construction route. - """ - def read(spec, predicate, warn=False): - return read_override_reform(spec, reading, predicate, calendar_years, warn) +def _effective_assignments_match(expected, written): + """Every span written by expected has the same final write in written.""" + for path in {entry.path for entry in expected}: + wanted = [entry for entry in expected if entry.path == path] + history = sorted( + (entry for entry in written if entry.path == path), + key=lambda entry: entry.order, + ) + for point in _assignment_boundaries(wanted + history): + change = next( + ( + entry + for entry in reversed(wanted) + if entry.start <= point <= entry.stop + ), + None, + ) + if change is None: + continue + effective = next( + ( + entry + for entry in reversed(history) + if entry.start <= point <= entry.stop + ), + None, + ) + if effective is None or effective.value != change.value: + return False + return True - assignments, legacy = read(changes, defines, warn=True) - def was_written(assignment): - return any( - (assignment.path, assignment.start, assignment.stop, assignment.value) - == (entry.path, entry.start, entry.stop, entry.value) - for entry in written +def _overrides_are_effective(overrides, parameters): + """Check final values at every boundary, including intervening writes.""" + assignments = [ + ParameterAssignment( + change.path, *legacy_override_bounds(change, parameters), change.value ) + for change in overrides + ] + for path in {entry.path for entry in assignments}: + target = parameters.get_child(path) + wanted = [entry for entry in assignments if entry.path == path] + points = _assignment_boundaries(wanted) | { + instant(entry.instant_str) for entry in target.values_list + } + for point in points: + change = next( + ( + entry + for entry in reversed(wanted) + if entry.start <= point <= entry.stop + ), + None, + ) + if change is not None and target(point) != change.value: + return False + return True + + +def _statutory_values_are_current(assignments, legacy, system): + """Detect later edits without rebuilding the simulation or its system. + + Annualise only the affected leaf's stored projected timeline, using the + construction rule. This also detects edits made directly by a modifier, + which need not have an entry in the system's override log. + """ + from policyengine_uk.utils.parameters import convert_to_fiscal_year_parameters + + timelines = getattr(system, "_projected_timelines", None) or {} + for path in {entry.path for entry in assignments}: + if path not in timelines: + return False + target = system.parameters.get_child(path) + expected = target.clone() + expected.values_list = list(timelines[path]) + probe = ParameterNode("effective_reform_probe", data={}) + probe.add_child("value", expected) + annualised = path.startswith("gov.") and not expected.metadata.get( + "preserve_calendar_dates", False + ) + if annualised: + convert_to_fiscal_year_parameters(probe) + for change in legacy: + if change.path == path: + expected.update( + **legacy_override_update_kwargs(change, system.parameters) + ) + wanted = [entry for entry in assignments if entry.path == path] + windows = [(entry.start, entry.stop) for entry in wanted] + if annualised: + for year in range(2015, 2041): + sample = instant(f"{year}-04-30") + start, stop = uk_fiscal_year_bounds(year) + if any( + (entry.start <= stop and start <= entry.stop) + if expected.metadata.get("fiscal_year_blend", False) + else entry.start <= sample <= entry.stop + for entry in wanted + ): + windows.append(calendar_year_bounds(year)) + points = _assignment_boundaries(wanted) | { + instant(entry.instant_str) + for entry in target.values_list + expected.values_list + } + if any( + target(point) != expected(point) + for point in points + if any(start <= point <= stop for start, stop in windows) + ): + return False + return True - if all(was_written(assignment) for assignment in assignments): - return legacy, False - # Select each original key rather than converting assignments back to - # keys: annualised overrides retain the route's own bare-year reading. + +def _reform_writes_are_effective(assignments, legacy, system, written=None): + if written is None: + written = getattr(system, "dated_assignments", ()) + if not _effective_assignments_match(assignments, written): + return False + if any(change not in getattr(system, "legacy_overrides", ()) for change in legacy): + return False + return _statutory_values_are_current(assignments, legacy, system) and ( + _overrides_are_effective(legacy, system.parameters) + ) + + +def reform_modifiers_are_effective(dated_reforms, system): + """Whether a whole ordered composite can retain its construction reading. + + Compare final writes on every overlapping span, rather than looking for + unordered historical membership. Later annualised overrides must also + agree with the requested result. Legacy leftovers may be no-ops on copies + only when the log contains them and their current values still agree. + """ + from policyengine_uk.tax_benefit_system import ( + parameter_reads_calendar_years, + reform_parameter_defines, + ) + + assignments, legacy = [], [] + try: + for dated in dated_reforms: + selected, leftovers = read_override_reform( + dated.changes, + dated.reading, + reform_parameter_defines(), + parameter_reads_calendar_years(), + ) + assignments.extend(selected) + legacy.extend(leftovers) + return _reform_writes_are_effective(assignments, legacy, system) + except (ValueError, TypeError, KeyError, AttributeError): + # Validation on the applying route reports all invalid paths/keys. + return False + + +def unwritten_reform_overrides( + changes, reading, defines, written, calendar_years, system +): + """Keep legacy changes and statutory writes that are no longer effective. + + Construction handles its own statutory writes before running modifiers. + On a built simulation only effective final writes count, including later + annualised overrides. Comparing contents survives copied scenario parts. + """ + + def read(spec, predicate, warn=False): + return read_override_reform(spec, reading, predicate, calendar_years, warn) + + read(changes, defines, warn=True) overrides = [] for path, spec in changes.items(): - entries = spec.items() if isinstance(spec, Mapping) else [(None, spec)] - for key, value in entries: - change = {path: {key: value} if isinstance(spec, Mapping) else value} - selected, _ = read(change, defines) - if not selected or not all(was_written(item) for item in selected): - _, pending = read(change, lambda path: False) - overrides.extend(pending) - return overrides, True + change = {path: spec} + assignments, legacy = read(change, defines) + if _reform_writes_are_effective(assignments, legacy, system, written): + continue + # Replay this path's keys together in their own order: an earlier + # outstanding write may supersede a later key that is effective now. + _, pending = read(change, lambda path: False) + overrides.extend(pending) + return overrides, bool(overrides) Timeline = List[ParameterAtInstant] diff --git a/policyengine_uk/utils/scenario.py b/policyengine_uk/utils/scenario.py index cf423b981b..f49f15d34a 100644 --- a/policyengine_uk/utils/scenario.py +++ b/policyengine_uk/utils/scenario.py @@ -15,6 +15,7 @@ read_reform_dict, read_reform_from_dict, read_override_reform, + reform_modifiers_are_effective, unwritten_reform_overrides, validate_annualised_overrides, ) @@ -72,7 +73,12 @@ def _is_reform_from_dict(reform: Type[Reform]) -> bool: def _legacy_override_modifier(dated: DatedReform) -> Callable[[Simulation], None]: - """Override changes the simulation has not written on its timeline.""" + """Override changes that are no longer effective on the simulation. + + Construction retains its statutory and legacy readings. On copies, + already-effective reforms (including legacy single-day keys) are no-ops; + external overrides still require a key containing a 1 January value. + """ def modifier(simulation: Simulation) -> None: from policyengine_uk.tax_benefit_system import ( @@ -88,6 +94,15 @@ def modifier(simulation: Simulation) -> None: dated.changes, dated.reading, lambda path: False ) overriding = True + elif dated in (getattr(simulation, "_building_dated_reforms", None) or ()): + _, legacy = dated.read( + reform_parameter_defines(), + warn=True, + calendar_years=parameter_reads_calendar_years(), + ) + overriding = False + elif reform_modifiers_are_effective((dated,), simulation.tax_benefit_system): + return else: legacy, overriding = unwritten_reform_overrides( dated.changes, @@ -95,6 +110,7 @@ def modifier(simulation: Simulation) -> None: reform_parameter_defines(), getattr(simulation.tax_benefit_system, "dated_assignments", ()), parameter_reads_calendar_years(), + simulation.tax_benefit_system, ) overriding = overriding or ( bool(legacy) @@ -124,6 +140,7 @@ def modifier(simulation: Simulation) -> None: warn_shadowed_lcwra_reform(simulation) + modifier._dated_reforms = (dated,) return modifier @@ -176,11 +193,26 @@ def _chain( ) -> Optional[Callable[[Simulation], None]]: """A modifier that runs ``first`` and then ``second``, either of them None.""" if first and second: + first_dated = getattr(first, "_dated_reforms", None) + second_dated = getattr(second, "_dated_reforms", None) + dated = ( + first_dated + second_dated + if first_dated is not None and second_dated is not None + else None + ) def chained(simulation: Simulation) -> None: + if ( + dated is not None + and not getattr(simulation, APPLYING_TO_BUILT_SIMULATION, False) + and not getattr(simulation, "_building_dated_reforms", None) + and reform_modifiers_are_effective(dated, simulation.tax_benefit_system) + ): + return first(simulation) second(simulation) + chained._dated_reforms = dated return chained return first or second From 3e8699ac9283a5e9b6110aabba663d0fca4217d1 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Fri, 16 Oct 2026 03:35:03 -0400 Subject: [PATCH 6/9] Defer standalone composite UC warnings until all children finish --- changelog.d/2426.fixed.md | 1 + .../tests/test_uc_health_warnings.py | 54 +++++++++++++++++++ policyengine_uk/utils/scenario.py | 48 +++++++++++++---- 3 files changed, 93 insertions(+), 10 deletions(-) diff --git a/changelog.d/2426.fixed.md b/changelog.d/2426.fixed.md index 4c4b39b063..dde52e93a1 100644 --- a/changelog.d/2426.fixed.md +++ b/changelog.d/2426.fixed.md @@ -1,2 +1,3 @@ - Fixed standalone dated-reform modifiers silently dropping parameter changes, warned when UC rebalancing supersedes an LCWRA amount reform, and restored new-claimant draws for data-built core simulations. - Preserved standalone modifier order across repeated and reordered reforms and retained no-op behavior for already-effective reforms on copies. +- Deferred LCWRA override warnings until standalone tuple and sum modifiers finish, with one warning per amount-reform application. diff --git a/policyengine_uk/tests/test_uc_health_warnings.py b/policyengine_uk/tests/test_uc_health_warnings.py index 16e992aff3..72d04f4e99 100644 --- a/policyengine_uk/tests/test_uc_health_warnings.py +++ b/policyengine_uk/tests/test_uc_health_warnings.py @@ -212,6 +212,60 @@ def test_tuple_and_sum_defer_warning_until_active_override_runs(): assert not [w for w in caught if issubclass(w.category, _warning_type())] +@pytest.mark.parametrize("route", ["tuple", "sum"]) +@pytest.mark.parametrize("escalated", [False, True]) +def test_standalone_composite_defers_warning_until_active_override_runs( + route, escalated +): + amount = {AMOUNT: {"2027-01-01.2027-12-31": 600}} + disabled = {ACTIVE: {"2027-01-01.2027-12-31": False}} + scenario = ( + Scenario.from_reform((amount, disabled)) + if route == "tuple" + else Scenario.from_reform(amount) + Scenario.from_reform(disabled) + ) + sim = Simulation(situation=SITUATION) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("error" if escalated else "always", _warning_type()) + scenario.simulation_modifier(sim) + + assert not [w for w in caught if issubclass(w.category, _warning_type())] + parameters = sim.tax_benefit_system.parameters + assert parameters.get_child(AMOUNT)("2027-01-01") == 600 + assert not parameters.get_child(ACTIVE)("2027-01-01") + + +@pytest.mark.parametrize("route", ["tuple", "sum"]) +def test_single_child_standalone_composite_warns_once_per_application(route): + amount = {AMOUNT: {"2027-01-01.2027-12-31": 600}} + scenario = ( + Scenario.from_reform((amount,)) + if route == "tuple" + else Scenario() + Scenario.from_reform(amount) + ) + sim = Simulation(situation=SITUATION) + + for _ in range(2): + with pytest.warns(_warning_type(), match="2027") as caught: + scenario.simulation_modifier(sim) + assert len([w for w in caught if issubclass(w.category, _warning_type())]) == 1 + + +@pytest.mark.parametrize("operation", ["simulation_modifier", "apply"]) +def test_unrelated_application_does_not_repeat_an_earlier_amount_warning(operation): + with pytest.warns(_warning_type(), match="2027"): + sim = Simulation( + situation=SITUATION, + reform={AMOUNT: {"2027-01-01.2027-12-31": 600}}, + ) + unrelated = Scenario.from_reform(({NEW_CLAIMANT: {"2027-01-01.2027-12-31": 500}},)) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always", _warning_type()) + getattr(unrelated, operation)(sim) + assert not [w for w in caught if issubclass(w.category, _warning_type())] + + @settings( max_examples=25, deadline=None, diff --git a/policyengine_uk/utils/scenario.py b/policyengine_uk/utils/scenario.py index f49f15d34a..982115052f 100644 --- a/policyengine_uk/utils/scenario.py +++ b/policyengine_uk/utils/scenario.py @@ -81,6 +81,7 @@ def _legacy_override_modifier(dated: DatedReform) -> Callable[[Simulation], None """ def modifier(simulation: Simulation) -> None: + _reset_uc_health_warning(simulation, (dated,)) from policyengine_uk.tax_benefit_system import ( parameter_reads_calendar_years, reform_parameter_defines, @@ -102,7 +103,7 @@ def modifier(simulation: Simulation) -> None: ) overriding = False elif reform_modifiers_are_effective((dated,), simulation.tax_benefit_system): - return + legacy, overriding = [], False else: legacy, overriding = unwritten_reform_overrides( dated.changes, @@ -144,6 +145,16 @@ def modifier(simulation: Simulation) -> None: return modifier +def _reset_uc_health_warning(simulation, dated_reforms, parameter_changes=None): + """An amount reform gets one diagnostic for this application.""" + from policyengine_uk.scenarios.uc_reform import LCWRA_AMOUNT + + if LCWRA_AMOUNT in (parameter_changes or {}) or any( + LCWRA_AMOUNT in dated.changes for dated in dated_reforms + ): + simulation._uc_health_amount_warning_emitted = False + + def _warn_annualised_overrides() -> None: logger.warning( "Scenario.apply edits the built simulation's annualised " @@ -202,15 +213,29 @@ def _chain( ) def chained(simulation: Simulation) -> None: - if ( - dated is not None - and not getattr(simulation, APPLYING_TO_BUILT_SIMULATION, False) - and not getattr(simulation, "_building_dated_reforms", None) - and reform_modifiers_are_effective(dated, simulation.tax_benefit_system) - ): - return - first(simulation) - second(simulation) + deferred = getattr(simulation, "_defer_uc_health_warning", False) + _reset_uc_health_warning(simulation, dated or ()) + simulation._defer_uc_health_warning = True + try: + already_effective = ( + dated is not None + and not getattr(simulation, APPLYING_TO_BUILT_SIMULATION, False) + and not getattr(simulation, "_building_dated_reforms", None) + and reform_modifiers_are_effective( + dated, simulation.tax_benefit_system + ) + ) + if not already_effective: + first(simulation) + second(simulation) + finally: + simulation._defer_uc_health_warning = deferred + if not deferred: + from policyengine_uk.scenarios.uc_reform import ( + warn_shadowed_lcwra_reform, + ) + + warn_shadowed_lcwra_reform(simulation) chained._dated_reforms = dated return chained @@ -551,6 +576,9 @@ def apply(self, simulation: Simulation) -> None: Args: simulation: The simulation to modify """ + _reset_uc_health_warning( + simulation, self.dated_reforms or (), self.parameter_changes + ) if self.dated_reforms: _warn_annualised_overrides() From 05231cf33cc00897b45138b84eea445bc736515d Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Fri, 16 Oct 2026 03:40:25 -0400 Subject: [PATCH 7/9] Preserve callback-only scenario compositions --- policyengine_uk/utils/scenario.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/policyengine_uk/utils/scenario.py b/policyengine_uk/utils/scenario.py index 982115052f..ee12446244 100644 --- a/policyengine_uk/utils/scenario.py +++ b/policyengine_uk/utils/scenario.py @@ -213,6 +213,10 @@ def _chain( ) def chained(simulation: Simulation) -> None: + if simulation is None: + first(simulation) + second(simulation) + return deferred = getattr(simulation, "_defer_uc_health_warning", False) _reset_uc_health_warning(simulation, dated or ()) simulation._defer_uc_health_warning = True From 636796c89edd0ac7419ffa1105f3c2b1d5a455d3 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Fri, 16 Oct 2026 06:29:35 -0400 Subject: [PATCH 8/9] Keep UC warnings optional for callback-only simulations --- changelog.d/2426.fixed.md | 1 + policyengine_uk/scenarios/uc_reform.py | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/changelog.d/2426.fixed.md b/changelog.d/2426.fixed.md index dde52e93a1..c7d0ee40f2 100644 --- a/changelog.d/2426.fixed.md +++ b/changelog.d/2426.fixed.md @@ -1,3 +1,4 @@ - Fixed standalone dated-reform modifiers silently dropping parameter changes, warned when UC rebalancing supersedes an LCWRA amount reform, and restored new-claimant draws for data-built core simulations. - Preserved standalone modifier order across repeated and reordered reforms and retained no-op behavior for already-effective reforms on copies. - Deferred LCWRA override warnings until standalone tuple and sum modifiers finish, with one warning per amount-reform application. +- Preserved callback-only Scenario.apply compatibility for lightweight simulation objects without a tax-benefit system. diff --git a/policyengine_uk/scenarios/uc_reform.py b/policyengine_uk/scenarios/uc_reform.py index f3284dd423..41967b34b3 100644 --- a/policyengine_uk/scenarios/uc_reform.py +++ b/policyengine_uk/scenarios/uc_reform.py @@ -151,7 +151,9 @@ def warn_shadowed_lcwra_reform(sim) -> None: sim, "_uc_health_amount_warning_emitted", False ): return - system = sim.tax_benefit_system + system = getattr(sim, "tax_benefit_system", None) + if system is None: + return assignments = [ change for change in getattr(system, "dated_assignments", ()) From 330392565f80903cff753f979442e4237314806c Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Fri, 16 Oct 2026 15:58:09 -0400 Subject: [PATCH 9/9] Replay mixed statutory and legacy reform modifiers in order --- changelog.d/2426.fixed.md | 1 + docs/book/usage/scenarios.md | 2 +- .../tests/test_from_reform_modifier.py | 90 +++++++++++++++++- policyengine_uk/utils/dated_reforms.py | 95 +++++++++---------- policyengine_uk/utils/scenario.py | 25 ++++- 5 files changed, 156 insertions(+), 57 deletions(-) diff --git a/changelog.d/2426.fixed.md b/changelog.d/2426.fixed.md index c7d0ee40f2..6611110c79 100644 --- a/changelog.d/2426.fixed.md +++ b/changelog.d/2426.fixed.md @@ -2,3 +2,4 @@ - Preserved standalone modifier order across repeated and reordered reforms and retained no-op behavior for already-effective reforms on copies. - Deferred LCWRA override warnings until standalone tuple and sum modifiers finish, with one warning per amount-reform application. - Preserved callback-only Scenario.apply compatibility for lightweight simulation objects without a tax-benefit system. +- Replayed mixed statutory and legacy modifiers in their requested order, restricting composite no-ops to exact latest write sequences with current values. diff --git a/docs/book/usage/scenarios.md b/docs/book/usage/scenarios.md index 3900098463..75fbb0c47c 100644 --- a/docs/book/usage/scenarios.md +++ b/docs/book/usage/scenarios.md @@ -419,7 +419,7 @@ A dataset's later years are projected from its first by the economic assumptions All of this applies when the scenario or reform is passed to `Simulation(...)`. `Scenario.apply(simulation)` on a simulation that is already built edits its annualised values instead, as it always has: a `"2027-04-06.2028-04-05"` key there overrides those dates of the fiscal-year values, so it does not set fiscal year 2027, and the model logs a warning saying so. Pass the scenario when constructing the simulation for dated measures. The modifier returned by `Scenario.from_reform` behaves the same way on a built simulation for changes that are no longer effective, and raises if an external override key contains no 1 January annual value. -An already-effective reform is a no-op on a copy, clone or branch, including legacy single-day keys; composed reforms compare final writes in their own order and account for later overrides. +A pure statutory or pure legacy reform can be a no-op on a copy, clone or branch only when its complete write sequence matches the latest recorded writes on its paths, the other log has no writes on those paths, and its values remain current. Mixed statutory and legacy composites always replay in their requested order; this can append duplicate override-log entries. An external key that contains no 1 January annual value raises with its path and key. ### Creating complex scenarios with simulation modifiers diff --git a/policyengine_uk/tests/test_from_reform_modifier.py b/policyengine_uk/tests/test_from_reform_modifier.py index 4ed1581e2a..7bf3b76175 100644 --- a/policyengine_uk/tests/test_from_reform_modifier.py +++ b/policyengine_uk/tests/test_from_reform_modifier.py @@ -9,7 +9,7 @@ import numpy as np import pytest -from hypothesis import HealthCheck, given, settings +from hypothesis import HealthCheck, example, given, settings from hypothesis import strategies as st from policyengine_core.reforms import Reform @@ -412,6 +412,76 @@ def test_composite_modifier_respects_order_of_historical_changes(case, represent assert parameter(direct, BASIC_RATE, "2027-01-01") == 0.31 +@pytest.mark.parametrize("copy_kind", ["clone", "branch"]) +@pytest.mark.parametrize("representation", ["tuple", "sum"]) +def test_mixed_composite_modifier_replays_after_external_legacy_override( + copy_kind, representation +): + statutory = {BASIC_RATE: {"2027-01-01.2027-12-31": 0.31}} + legacy = {BASIC_RATE: {"2027-01-01": 0.35}} + original = Simulation(situation=SITUATION, reform=statutory) + Scenario.from_reform(legacy).apply(original) + assert parameter(original, BASIC_RATE, "2027-01-01") == 0.35 + direct, applied = ( + (original.clone(), original.clone()) + if copy_kind == "clone" + else ( + original.get_branch("mixed-direct", clone_system=True), + original.get_branch("mixed-applied", clone_system=True), + ) + ) + scenario = _sequence_scenario((legacy, statutory), representation) + + scenario.simulation_modifier(direct) + scenario.apply(applied) + + for date in ("2026-12-31", "2027-01-01", "2027-01-02", "2027-12-31", "2028-01-01"): + assert parameter(direct, BASIC_RATE, date) == parameter( + applied, BASIC_RATE, date + ) + assert parameter(direct, BASIC_RATE, "2027-01-01") == 0.31 + assert parameter(original, BASIC_RATE, "2027-01-01") == 0.35 + + +@pytest.mark.parametrize("copy_kind", ["clone", "branch"]) +@pytest.mark.parametrize("representation", ["tuple", "sum", "dict"]) +def test_mixed_composite_modifier_replays_reversed_statutory_and_legacy_order( + copy_kind, representation +): + key = "2027-01-01.2027-12-31" + statutory = {BASIC_RATE: {key: 0.31}} + legacy = {BASIC_RATE: {"2027-01-01": 0.35}} + if representation == "dict": + with pytest.warns(ReformPeriodWarning, match="mixes single-day keys"): + original = Simulation( + situation=SITUATION, + reform={BASIC_RATE: {key: 0.31, "2027-01-01": 0.35}}, + ) + scenario = Scenario.from_reform({BASIC_RATE: {"2027-01-01": 0.35, key: 0.31}}) + else: + original = Simulation(situation=SITUATION, reform=(statutory, legacy)) + scenario = _sequence_scenario((legacy, statutory), representation) + assert parameter(original, BASIC_RATE, "2027-01-01") == 0.35 + direct, applied = ( + (original.clone(), original.clone()) + if copy_kind == "clone" + else ( + original.get_branch("reordered-direct", clone_system=True), + original.get_branch("reordered-applied", clone_system=True), + ) + ) + + scenario.simulation_modifier(direct) + scenario.apply(applied) + + for date in ("2026-12-31", "2027-01-01", "2027-01-02", "2027-12-31", "2028-01-01"): + assert parameter(direct, BASIC_RATE, date) == parameter( + applied, BASIC_RATE, date + ) + assert parameter(direct, BASIC_RATE, "2027-01-01") == 0.31 + assert parameter(original, BASIC_RATE, "2027-01-01") == 0.35 + + def test_modifier_reapplies_historical_change_after_a_later_legacy_override(): first = {BASIC_RATE: {"2027-01-01.2027-12-31": 0.31}} second = {BASIC_RATE: {"2027-01-01.2027-12-31": 0.35}} @@ -568,14 +638,16 @@ def test_own_composite_modifier_on_copy_preserves_order_without_reapplying( @st.composite def overlapping_reform_sequences(draw): - # A small pool makes shared paths/windows, repeated entries and different - # orders common, including writes that were present only earlier in history. + # A small pool makes shared paths, mixed legacy/statutory keys, repeated + # entries and different orders common, including superseded writes. entries = draw( st.lists( st.tuples( st.sampled_from([BASIC_RATE, VAT_RATE]), st.sampled_from( [ + "2026-01-01", + "2027-01-01", "2026-01-01.2026-12-31", "2027-01-01.2027-12-31", "2026-01-01.2027-12-31", @@ -606,6 +678,18 @@ def changes(order): suppress_health_check=[HealthCheck.too_slow], ) @given(sequences=overlapping_reform_sequences()) +@example( + sequences=( + ( + {BASIC_RATE: {"2027-01-01.2027-12-31": 0.31}}, + {BASIC_RATE: {"2027-01-01": 0.35}}, + ), + ( + {BASIC_RATE: {"2027-01-01": 0.35}}, + {BASIC_RATE: {"2027-01-01.2027-12-31": 0.31}}, + ), + ) +) def test_sequence_modifier_matches_apply_with_repeated_and_reordered_writes( baseline, prebuilt, sequences ): diff --git a/policyengine_uk/utils/dated_reforms.py b/policyengine_uk/utils/dated_reforms.py index ff63ce0676..6f7357c04c 100644 --- a/policyengine_uk/utils/dated_reforms.py +++ b/policyengine_uk/utils/dated_reforms.py @@ -588,36 +588,20 @@ def _assignment_boundaries(assignments): } -def _effective_assignments_match(expected, written): - """Every span written by expected has the same final write in written.""" - for path in {entry.path for entry in expected}: - wanted = [entry for entry in expected if entry.path == path] - history = sorted( - (entry for entry in written if entry.path == path), - key=lambda entry: entry.order, - ) - for point in _assignment_boundaries(wanted + history): - change = next( - ( - entry - for entry in reversed(wanted) - if entry.start <= point <= entry.stop - ), - None, - ) - if change is None: - continue - effective = next( - ( - entry - for entry in reversed(history) - if entry.start <= point <= entry.stop - ), - None, - ) - if effective is None or effective.value != change.value: - return False - return True +def _assignment_sequence_matches(expected, written): + """The latest writes on the affected paths are this exact sequence.""" + paths = {entry.path for entry in expected} + history = sorted( + (entry for entry in written if entry.path in paths), + key=lambda entry: entry.order, + ) + + def contents(entries): + return [(entry.path, entry.start, entry.stop, entry.value) for entry in entries] + + return len(history) >= len(expected) and contents(history[-len(expected) :]) == ( + contents(expected) + ) def _overrides_are_effective(overrides, parameters): @@ -648,7 +632,7 @@ def _overrides_are_effective(overrides, parameters): return True -def _statutory_values_are_current(assignments, legacy, system): +def _statutory_values_are_current(assignments, system): """Detect later edits without rebuilding the simulation or its system. Annualise only the affected leaf's stored projected timeline, using the @@ -671,11 +655,6 @@ def _statutory_values_are_current(assignments, legacy, system): ) if annualised: convert_to_fiscal_year_parameters(probe) - for change in legacy: - if change.path == path: - expected.update( - **legacy_override_update_kwargs(change, system.parameters) - ) wanted = [entry for entry in assignments if entry.path == path] windows = [(entry.start, entry.stop) for entry in wanted] if annualised: @@ -703,24 +682,39 @@ def _statutory_values_are_current(assignments, legacy, system): def _reform_writes_are_effective(assignments, legacy, system, written=None): + # The two logs do not retain their joint write order. Never infer a + # mixed reform's result by replaying one log after the other. + if assignments and legacy: + return False if written is None: written = getattr(system, "dated_assignments", ()) - if not _effective_assignments_match(assignments, written): - return False - if any(change not in getattr(system, "legacy_overrides", ()) for change in legacy): - return False - return _statutory_values_are_current(assignments, legacy, system) and ( - _overrides_are_effective(legacy, system.parameters) - ) + overrides = getattr(system, "legacy_overrides", ()) + if assignments: + paths = {entry.path for entry in assignments} + return ( + not any(entry.path in paths for entry in overrides) + and _assignment_sequence_matches(assignments, written) + and _statutory_values_are_current(assignments, system) + ) + if legacy: + paths = {entry.path for entry in legacy} + history = [entry for entry in overrides if entry.path in paths] + return ( + not any(entry.path in paths for entry in written) + and len(history) >= len(legacy) + and history[-len(legacy) :] == legacy + and _overrides_are_effective(legacy, system.parameters) + ) + return True def reform_modifiers_are_effective(dated_reforms, system): - """Whether a whole ordered composite can retain its construction reading. + """Skip only an exact latest sequence from one write log, still current. - Compare final writes on every overlapping span, rather than looking for - unordered historical membership. Later annualised overrides must also - agree with the requested result. Legacy leftovers may be no-ops on copies - only when the log contains them and their current values still agree. + Mixed statutory/legacy reforms always replay: separate logs cannot prove + their joint order. A pure sequence must be the suffix of its log on the + affected paths, have no writes in the other log on those paths, and pass + the current-value check (including direct parameter edits). """ from policyengine_uk.tax_benefit_system import ( parameter_reads_calendar_years, @@ -757,7 +751,10 @@ def unwritten_reform_overrides( def read(spec, predicate, warn=False): return read_override_reform(spec, reading, predicate, calendar_years, warn) - read(changes, defines, warn=True) + assignments, legacy = read(changes, defines, warn=True) + if assignments and legacy: + _, pending = read(changes, lambda path: False) + return pending, bool(pending) overrides = [] for path, spec in changes.items(): change = {path: spec} diff --git a/policyengine_uk/utils/scenario.py b/policyengine_uk/utils/scenario.py index ee12446244..021e070b1f 100644 --- a/policyengine_uk/utils/scenario.py +++ b/policyengine_uk/utils/scenario.py @@ -76,8 +76,9 @@ def _legacy_override_modifier(dated: DatedReform) -> Callable[[Simulation], None """Override changes that are no longer effective on the simulation. Construction retains its statutory and legacy readings. On copies, - already-effective reforms (including legacy single-day keys) are no-ops; - external overrides still require a key containing a 1 January value. + exact latest sequences from a single write log may be no-ops, including + legacy single-day keys. Mixed statutory/legacy sequences always replay; + external overrides require a key containing a 1 January value. """ def modifier(simulation: Simulation) -> None: @@ -230,8 +231,24 @@ def chained(simulation: Simulation) -> None: ) ) if not already_effective: - first(simulation) - second(simulation) + # An external composite whose complete sequence cannot + # be skipped must replay every child as Scenario.apply + # does, without child-level no-op shortcuts. + applying = getattr(simulation, APPLYING_TO_BUILT_SIMULATION, False) + replay = ( + dated is not None + and not applying + and not getattr(simulation, "_building_dated_reforms", None) + ) + if replay: + _warn_annualised_overrides() + setattr(simulation, APPLYING_TO_BUILT_SIMULATION, True) + try: + first(simulation) + second(simulation) + finally: + if replay: + delattr(simulation, APPLYING_TO_BUILT_SIMULATION) finally: simulation._defer_uc_health_warning = deferred if not deferred: