diff --git a/changelog.d/2480.fixed.md b/changelog.d/2480.fixed.md new file mode 100644 index 000000000..6aa350961 --- /dev/null +++ b/changelog.d/2480.fixed.md @@ -0,0 +1 @@ +Fix stale construction-time reform wrappers after foreign writes or direct edits while preserving copied scenarios and constructor readings; document mixed-composite replay, indexed-threshold construction and the existing rejection of built overrides whose keys contain no 1 January annual value. diff --git a/docs/book/usage/scenarios.md b/docs/book/usage/scenarios.md index 20b40689b..706772f6d 100644 --- a/docs/book/usage/scenarios.md +++ b/docs/book/usage/scenarios.md @@ -424,7 +424,12 @@ 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. -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. + +During construction, the already-written statutory changes of an original dated reform keep the constructor's reading while their parameter paths remain untouched by foreign writes or direct parameter edits. This includes a value-equal reform called by a wrapper or a deep-copied scenario. The original modifiers still apply their single-day leftovers in order. If a user modifier edits those statutory paths and then requests the same reform, that request instead overrides the built, annualised values. An external composite replays its complete requested sequence; its children cannot skip a write merely because they equal an original constructor reform. Edits before data load remain foreign when a later modifier runs after data load. This boundary concerns parameter writes, not the different award values that Universal Credit rebalancing intentionally stores. + +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 on a built simulation always replay in their requested order; this can append duplicate override-log entries and change the copy's original construction reading. For example, a dict with a basic-rate window `"2026-06-01.2027-03-31": 0.31` and a VAT single-day key `"2027-01-01": 0.25` gives the 2027 basic rate its unchanged 20% construction value. Replaying its modifier on a clone overrides 1 January 2027 with 31%; the original simulation stays unchanged. Relative `Reform` classes and custom modifiers still run when called on a copy; a modifier that multiplies a parameter by 1.1 applies that multiplication to the copy's current value. When changing policy parameters on a branch, request `get_branch("policy", clone_system=True)`; the default branch shares its tax-benefit system with its parent. + +Dated measures on indexed thresholds also need `Simulation(scenario=...)` or `reform=...` for the statutory projection and indexation resume described above. Applying them to a baseline-built simulation overrides only its already annualised values; it does not recalculate the indexed path after the override ends. An external key that contains no 1 January annual value raises with its path and key, including `"2027-04-06"` and `"2027-04-06.2027-12-31"`. ### Creating complex scenarios with simulation modifiers diff --git a/policyengine_uk/simulation.py b/policyengine_uk/simulation.py index 385be1f50..b2cdd10e8 100644 --- a/policyengine_uk/simulation.py +++ b/policyengine_uk/simulation.py @@ -30,7 +30,7 @@ UKMultiYearDataset, UKSingleYearDataset, ) -from policyengine_uk.utils.scenario import Scenario +from policyengine_uk.utils.scenario import ConstructionReformState, Scenario from policyengine_uk.data.economic_assumptions import ( extend_single_year_dataset, reset_growthfactor_uprating, @@ -258,6 +258,7 @@ def __init__( # 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._construction_reform_state = None self.branch_name = "default" self.invalidated_caches = set() @@ -295,8 +296,10 @@ def __init__( ) if before_data_load is not None: self.apply_scenario_parameters(before_data_load) + self._construction_reform_state = ConstructionReformState(self) if before_data_load.simulation_modifier is not None: before_data_load.simulation_modifier(self) + self._construction_reform_state.observe_user_edits(self) elif after_parameters_before_data: self.apply_scenario_parameters(after_data_load) self._refresh_capital_gains_projection() @@ -325,6 +328,8 @@ def __init__( # (including single-day reform-dict keys) leave the data as built. if after_data_load is not None and not after_parameters_before_data: self.apply_scenario_parameters(after_data_load) + if self._construction_reform_state is None and self._building_dated_reforms: + self._construction_reform_state = ConstructionReformState(self) # Universal Credit reform (July 2025). Needs closer integration in the baseline, # but adding here for ease of toggling on/off via the 'active' parameter. @@ -376,10 +381,13 @@ def __init__( self.baseline._cgt_warning_enabled = False self.baseline._defer_uc_health_warning = False self.baseline._building_dated_reforms = None + self.baseline._construction_reform_state = None self.calculated_periods = [] if after_data_load is not None: + if self._construction_reform_state is not None: + self._construction_reform_state.refresh_framework(self) if after_data_load.simulation_modifier is not None: after_data_load.simulation_modifier(self) self._refresh_capital_gains_projection() @@ -389,6 +397,7 @@ def __init__( self._defer_uc_health_warning = False self._building_dated_reforms = None + self._construction_reform_state = None warn_shadowed_lcwra_reform(self) def _refresh_capital_gains_projection(self) -> None: @@ -450,6 +459,9 @@ def set_input(self, variable_name: str, period: Period, value) -> None: def clone(self, *args, **kwargs) -> "Simulation": clone = super().clone(*args, **kwargs) + clone._construction_reform_state = copy.deepcopy( + getattr(self, "_construction_reform_state", None) + ) copy_model_values(self, clone) clone._cgt_input_provenance = copy.deepcopy( getattr(self, "_cgt_input_provenance", {}) diff --git a/policyengine_uk/tests/test_construction_reform_wrappers.py b/policyengine_uk/tests/test_construction_reform_wrappers.py new file mode 100644 index 000000000..04a29ea6a --- /dev/null +++ b/policyengine_uk/tests/test_construction_reform_wrappers.py @@ -0,0 +1,427 @@ +"""Construction-owned reform wrappers retain their reading until edited.""" + +import copy +from decimal import Decimal, ROUND_CEILING + +import numpy as np +import pytest +from policyengine_core.reforms import Reform + +from policyengine_uk import Simulation +from policyengine_uk.scenarios.uc_reform import UCHealthElementOverrideWarning +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" +YEARS = (2026, 2027, 2028) +SITUATION = { + "people": {"person1": {"age": {year: 40 for year in YEARS}}}, + "benunits": {"benunit": {"members": ["person1"]}}, + "households": {"household": {"members": ["person1"]}}, +} +A = {BASIC_RATE: {"2027-01-01.2027-12-31": 0.31}} +L = {BASIC_RATE: {"2027-01-01": 0.35}} +VAT = "gov.hmrc.vat.standard_rate" +PA = "gov.hmrc.income_tax.allowances.personal_allowance.amount" +CPI = "gov.economic_assumptions.yoy_growth.obr.consumer_price_index" +STATUTORY_CPI = "gov.economic_assumptions.yoy_growth.september_cpi_uprating" +FISCAL_A = {BASIC_RATE: {"2026-04-06.2027-04-05": 0.31}} + + +def parameter(simulation, path, date): + return simulation.tax_benefit_system.parameters.get_child(path)(date) + + +@pytest.mark.parametrize("before_data_load", [False, True]) +@pytest.mark.parametrize("edit", ["legacy", "direct"]) +@pytest.mark.parametrize("composite", [False, True]) +def test_equal_constructor_wrapper_after_edit_matches_apply( + before_data_load, edit, composite +): + def build(use_apply): + def wrapper(simulation): + if edit == "legacy": + Scenario.from_reform(L).apply(simulation) + else: + simulation.tax_benefit_system.parameters.get_child(BASIC_RATE).update( + period="2027", value=0.40 + ) + simulation.tax_benefit_system.reset_parameter_caches() + requested = Scenario.from_reform((L, A) if composite else A) + if use_apply: + requested.apply(simulation) + else: + requested.simulation_modifier(simulation) + + own = Scenario.from_reform(A) + own.applied_before_data_load = before_data_load + return Simulation( + situation=SITUATION, + scenario=own + + Scenario( + simulation_modifier=wrapper, + applied_before_data_load=before_data_load, + ), + ) + + direct, applied = build(False), build(True) + assert parameter(applied, BASIC_RATE, "2027-01-01") == 0.31 + assert parameter(direct, BASIC_RATE, "2027-01-01") == 0.31 + 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 + ) + + +@pytest.mark.parametrize("before_data_load", [False, True]) +@pytest.mark.parametrize("deepcopy", [False, True]) +@pytest.mark.parametrize("reform_class", [False, True]) +def test_disjoint_composer_member_wrappers_keep_constructor_reading( + before_data_load, deepcopy, reform_class +): + # The acquired scenario_composition/v1 contract: all dated changes first, + # followed by every member's modifier, including fresh equal member calls. + reforms = (FISCAL_A, {VAT: {"2027-01-01.2027-12-31": 0.27}}) + members = [Scenario.from_reform(r) for r in reforms] + + def wrapper(simulation): + for reform in reforms: + request = Reform.from_dict(reform) if reform_class else reform + Scenario.from_reform(request).simulation_modifier(simulation) + + if reform_class: + members = [Scenario.from_reform(Reform.from_dict(r)) for r in reforms] + scenario = Scenario( + dated_reforms=[d for member in members for d in member.dated_reforms], + simulation_modifier=wrapper, + applied_before_data_load=before_data_load, + ) + if deepcopy: + scenario = copy.deepcopy(scenario) + simulation = Simulation(situation=SITUATION, scenario=scenario) + assert parameter(simulation, BASIC_RATE, "2026-01-01") == 0.31 + assert parameter(simulation, BASIC_RATE, "2027-01-01") == 0.20 + assert parameter(simulation, VAT, "2027-01-01") == 0.27 + assert simulation.tax_benefit_system.legacy_overrides == [] + assert simulation._construction_reform_state is None + assert simulation.baseline._construction_reform_state is None + + +@pytest.mark.parametrize("before_data_load", [False, True]) +@pytest.mark.parametrize("deepcopy", [False, True]) +def test_pristine_mixed_own_wrapper_keeps_legacy_and_statutory_readings( + before_data_load, deepcopy +): + changes = {**FISCAL_A, VAT: {"2027-01-01": 0.27}} + + def wrapper(simulation): + Scenario.from_reform(changes).simulation_modifier(simulation) + + scenario = Scenario.from_reform(changes) + scenario.applied_before_data_load = before_data_load + scenario += Scenario( + simulation_modifier=wrapper, applied_before_data_load=before_data_load + ) + if deepcopy: + scenario = copy.deepcopy(scenario) + simulation = Simulation(situation=SITUATION, scenario=scenario) + assert parameter(simulation, BASIC_RATE, "2026-01-01") == 0.31 + assert parameter(simulation, BASIC_RATE, "2027-01-01") == 0.20 + assert parameter(simulation, VAT, "2027-01-01") == 0.27 + assert parameter(simulation, VAT, "2027-01-02") == 0.20 + + +@pytest.mark.parametrize("before_data_load", [False, True]) +def test_unrelated_direct_edit_does_not_replay_own_fiscal_change(before_data_load): + def wrapper(simulation): + simulation.tax_benefit_system.parameters.get_child(VAT).update( + period="2027", value=0.40 + ) + simulation.tax_benefit_system.reset_parameter_caches() + Scenario.from_reform(FISCAL_A).simulation_modifier(simulation) + + scenario = Scenario.from_reform(FISCAL_A) + scenario.applied_before_data_load = before_data_load + simulation = Simulation( + situation=SITUATION, + scenario=scenario + + Scenario( + simulation_modifier=wrapper, applied_before_data_load=before_data_load + ), + ) + assert parameter(simulation, BASIC_RATE, "2026-01-01") == 0.31 + assert parameter(simulation, BASIC_RATE, "2027-01-01") == 0.20 + assert parameter(simulation, VAT, "2027-01-01") == 0.40 + assert simulation.tax_benefit_system.legacy_overrides == [] + + +@pytest.mark.parametrize("edit", ["legacy", "direct"]) +def test_foreign_edit_survives_phase_boundary_as_foreign(edit): + def before(simulation): + if edit == "legacy": + Scenario.from_reform(L).apply(simulation) + else: + simulation.tax_benefit_system.parameters.get_child(BASIC_RATE).update( + period="2027", value=0.40 + ) + simulation.tax_benefit_system.reset_parameter_caches() + + def after(simulation): + Scenario.from_reform(A).simulation_modifier(simulation) + + scenario = Scenario.from_reform(A) + scenario.applied_before_data_load = True + scenario += Scenario(simulation_modifier=before, applied_before_data_load=True) + scenario += Scenario(simulation_modifier=after) + simulation = Simulation(situation=SITUATION, scenario=scenario) + assert parameter(simulation, BASIC_RATE, "2027-01-01") == 0.31 + + +@pytest.mark.parametrize("before_data_load", [False, True]) +def test_direct_edit_outside_window_still_disables_construction_privilege( + before_data_load, +): + def wrapper(simulation): + simulation.tax_benefit_system.parameters.get_child(BASIC_RATE).update( + period="2030", value=0.40 + ) + simulation.tax_benefit_system.reset_parameter_caches() + Scenario.from_reform(FISCAL_A).simulation_modifier(simulation) + + scenario = Scenario.from_reform(FISCAL_A) + scenario.applied_before_data_load = before_data_load + simulation = Simulation( + situation=SITUATION, + scenario=scenario + + Scenario( + simulation_modifier=wrapper, applied_before_data_load=before_data_load + ), + ) + # A direct edit has no write-log record. Even an edit outside A's dates + # ends its path's construction privilege: A now takes the built reading. + assert parameter(simulation, BASIC_RATE, "2027-01-01") == 0.31 + assert parameter(simulation, BASIC_RATE, "2030-01-01") == 0.40 + + +@pytest.mark.parametrize("before_data_load", [False, True]) +@pytest.mark.parametrize("deepcopy", [False, True]) +def test_pristine_equal_member_keeps_constructor_later_member_order( + before_data_load, deepcopy +): + later = {BASIC_RATE: {"2027-01-01.2027-12-31": 0.35}} + + def wrapper(simulation): + Scenario.from_reform(A).simulation_modifier(simulation) + + scenario = Scenario.from_reform((A, later)) + scenario.applied_before_data_load = before_data_load + scenario += Scenario( + simulation_modifier=wrapper, applied_before_data_load=before_data_load + ) + if deepcopy: + scenario = copy.deepcopy(scenario) + simulation = Simulation(situation=SITUATION, scenario=scenario) + assert parameter(simulation, BASIC_RATE, "2027-01-01") == 0.35 + assert simulation.tax_benefit_system.legacy_overrides == [] + + +@pytest.mark.parametrize("before_data_load", [False, True]) +def test_original_lcwra_shadow_is_not_a_foreign_parameter_edit(before_data_load): + situation = copy.deepcopy(SITUATION) + situation["people"]["person1"]["uc_limited_capability_for_WRA"] = { + year: True for year in YEARS + } + situation["benunits"]["benunit"].update( + would_claim_uc={year: True for year in YEARS}, + uc_receives_new_claimant_health_element={year: False for year in YEARS}, + ) + path = "gov.dwp.universal_credit.elements.disabled.amount" + changes = {path: {"2026-01-01.2028-12-31": 999.0}} + own = Scenario.from_reform(changes) + own.applied_before_data_load = before_data_load + with pytest.warns(UCHealthElementOverrideWarning): + control = Simulation(situation=situation, scenario=own) + + def wrapper(simulation): + Scenario.from_reform(changes).simulation_modifier(simulation) + + with pytest.warns(UCHealthElementOverrideWarning) as recorded: + simulation = Simulation( + situation=situation, + scenario=own + + Scenario( + simulation_modifier=wrapper, + applied_before_data_load=before_data_load, + ), + ) + assert len(recorded) == 1 + assert parameter(simulation, path, "2027-01-01") == 999.0 + expected = control.calculate("uc_LCWRA_element", 2027) + assert np.all((expected > 0) & (expected < 999.0 * 12)) + np.testing.assert_array_equal( + simulation.calculate("uc_LCWRA_element", 2027), expected + ) + assert simulation.tax_benefit_system.legacy_overrides == [] + + +class ScaleAllowance(Reform): + def apply(self): + target = self.parameters.get_child(PA) + target.update(period="2027", value=target("2027") * 1.1) + + +def scale_allowance(simulation): + target = simulation.tax_benefit_system.parameters.get_child(PA) + target.update(period="2027", value=target("2027") * 1.1) + simulation.tax_benefit_system.reset_parameter_caches() + + +@pytest.mark.parametrize("copy_kind", ["clone", "branch"]) +@pytest.mark.parametrize("representation", ["class", "modifier", "cpi_dict"]) +def test_relative_reforms_on_copies_match_explicit_apply(copy_kind, representation): + if representation == "class": + scenario = Scenario.from_reform(ScaleAllowance) + elif representation == "modifier": + scenario = Scenario(simulation_modifier=scale_allowance) + else: + scenario = Scenario.from_reform({CPI: {"2027": 0.08}}) + original = Simulation(situation=SITUATION) + original_allowance = parameter(original, PA, "2027") + original_cpi = parameter(original, CPI, "2027") + direct = ( + original.clone() + if copy_kind == "clone" + else original.get_branch("copy", clone_system=True) + ) + applied = original.clone() + scenario.simulation_modifier(direct) + scenario.apply(applied) + for path in (PA, CPI): + for year in YEARS: + assert parameter(direct, path, str(year)) == parameter( + applied, path, str(year) + ) + if representation == "cpi_dict": + assert parameter(direct, CPI, "2027") == 0.08 + else: + assert parameter(direct, PA, "2027") == pytest.approx(original_allowance * 1.1) + assert parameter(original, PA, "2027") == original_allowance + assert parameter(original, CPI, "2027") == original_cpi + + +@pytest.mark.parametrize("copy_kind", ["clone", "branch"]) +def test_mixed_replay_on_copy_changes_its_own_construction_reading(copy_kind): + changes = { + BASIC_RATE: {"2026-06-01.2027-03-31": 0.31}, + VAT: {"2027-01-01": 0.25}, + } + original = Simulation(situation=SITUATION, reform=changes) + assert parameter(original, BASIC_RATE, "2027-01-01") == 0.20 + simulation = ( + original.clone() + if copy_kind == "clone" + else original.get_branch("copy", clone_system=True) + ) + Scenario.from_reform(changes).simulation_modifier(simulation) + assert parameter(simulation, BASIC_RATE, "2027-01-01") == 0.31 + assert parameter(simulation, VAT, "2027-01-01") == 0.25 + assert parameter(original, BASIC_RATE, "2027-01-01") == 0.20 + + +def test_indexed_freeze_needs_construction_for_indexation_resume(): + changes = {PA: {"2026-04-06.2033-04-05": 12_570}} + constructed = Simulation(situation=SITUATION, reform=changes) + built = Simulation(situation=SITUATION) + baseline_2034 = parameter(built, PA, "2034") + Scenario.from_reform(changes).apply(built) + increase = Decimal(str(parameter(constructed, STATUTORY_CPI, "2033"))) + expected = float( + (Decimal(12_570) * (1 + increase) / 10).to_integral_value( + rounding=ROUND_CEILING + ) + * 10 + ) + assert parameter(constructed, PA, "2033") == expected + assert parameter(built, PA, "2033") == 12_570 + assert parameter(built, PA, "2034") == baseline_2034 + + +@pytest.mark.parametrize("copy_kind", ["clone", "branch"]) +def test_isolated_construction_copy_preserves_parent_reading_and_tax(copy_kind): + situation = copy.deepcopy(SITUATION) + situation["people"]["person1"]["employment_income"] = { + year: 30_000 for year in YEARS + } + observations = {} + + def signature(simulation): + system = simulation.tax_benefit_system + target = system.parameters.get_child(BASIC_RATE) + return copy.deepcopy( + ( + [(entry.instant_str, entry.value) for entry in target.values_list], + system.dated_assignments, + system.legacy_overrides, + ) + ) + + def wrapper(parent): + before = signature(parent) + parent_state = parent._construction_reform_state + checkpoint_before = copy.deepcopy(parent_state.__dict__) + child = ( + parent.clone(clone_tax_benefit_system=True) + if copy_kind == "clone" + else parent.get_branch("construction_copy", clone_system=True) + ) + assert child.tax_benefit_system is not parent.tax_benefit_system + child_state = child._construction_reform_state + observations["separate_checkpoint_fields"] = ( + child_state is not parent_state + and all( + getattr(child_state, field) is not getattr(parent_state, field) + for field in ("owned", "paths", "dirty", "expected") + ) + and child_state.expected[BASIC_RATE] + is not parent_state.expected[BASIC_RATE] + ) + assert child_state.owned == parent_state.owned + child.tax_benefit_system.parameters.get_child(BASIC_RATE).update( + period="2027", value=0.40 + ) + child.tax_benefit_system.reset_parameter_caches() + Scenario.from_reform(FISCAL_A).simulation_modifier(child) + assert parameter(child, BASIC_RATE, "2027-01-01") == 0.31 + assert signature(parent) == before + assert parameter(parent, BASIC_RATE, "2027-01-01") == 0.20 + observations["parent_checkpoint_unchanged"] = ( + parent_state.__dict__ == checkpoint_before + ) + Scenario.from_reform(FISCAL_A).simulation_modifier(parent) + observations["parent_timeline_and_logs_unchanged"] = signature(parent) == before + + own = Scenario.from_reform(FISCAL_A) + control = Simulation(situation=situation, scenario=own) + simulation = Simulation( + situation=situation, + scenario=own + Scenario(simulation_modifier=wrapper), + ) + np.testing.assert_array_equal(control.calculate("income_tax", 2027), [3_486.0]) + np.testing.assert_array_equal( + simulation.calculate("employment_income", 2027), [30_000] + ) + np.testing.assert_array_equal( + simulation.calculate("personal_allowance", 2027), [12_570] + ) + np.testing.assert_array_equal( + simulation.calculate("income_tax", 2027), control.calculate("income_tax", 2027) + ) + assert parameter(simulation, BASIC_RATE, "2026-01-01") == 0.31 + assert parameter(simulation, BASIC_RATE, "2027-01-01") == 0.20 + assert observations == { + "separate_checkpoint_fields": True, + "parent_checkpoint_unchanged": True, + "parent_timeline_and_logs_unchanged": True, + } diff --git a/policyengine_uk/utils/scenario.py b/policyengine_uk/utils/scenario.py index 021e070b1..3871b0948 100644 --- a/policyengine_uk/utils/scenario.py +++ b/policyengine_uk/utils/scenario.py @@ -72,6 +72,78 @@ def _is_reform_from_dict(reform: Type[Reform]) -> bool: ) +class ConstructionReformState: + """The constructor's writes, until user code edits their parameter paths. + + Ownership compares contents so copied scenarios and member wrappers keep + the construction reading. Current values alone cannot identify writes: + a foreign override can have the same value but a different date reading. + """ + + def __init__(self, simulation): + from policyengine_uk.tax_benefit_system import ( + parameter_reads_calendar_years, + reform_parameter_defines, + ) + + self.owned = copy.deepcopy(tuple(simulation._building_dated_reforms or ())) + self.paths = set() + for dated in self.owned: + assignments, _ = dated.read( + reform_parameter_defines(), + calendar_years=parameter_reads_calendar_years(), + ) + self.paths.update(entry.path for entry in assignments) + self.dirty = set() + self.expected = {} + self.refresh_framework(simulation) + + @staticmethod + def _current(simulation, path): + system = simulation.tax_benefit_system + target = system.parameters.get_child(path) + return copy.deepcopy( + ( + [(entry.instant_str, entry.value) for entry in target.values_list], + [entry for entry in system.dated_assignments if entry.path == path], + [entry for entry in system.legacy_overrides if entry.path == path], + ) + ) + + def _check_paths(self, simulation, paths): + for path in paths: + if self.expected[path] != self._current(simulation, path): + self.dirty.add(path) + return not self.dirty.intersection(paths) + + def pristine(self, simulation, dated_reforms): + if any(dated not in self.owned for dated in dated_reforms): + return False + paths = self.paths.intersection( + path for dated in dated_reforms for path in dated.changes + ) + return self._check_paths(simulation, paths) + + def accept_owned(self, simulation, dated): + # Only called after a pristine own modifier writes its leftovers. + for path in self.paths.intersection(dated.changes): + self.expected[path] = self._current(simulation, path) + + def observe_user_edits(self, simulation): + self._check_paths(simulation, self.paths) + + def refresh_framework(self, simulation): + # The framework may change clean paths between the two user phases. + # A user edit observed before that boundary must not become owned. + for path in self.paths - self.dirty: + self.expected[path] = self._current(simulation, path) + + +def _construction_modifiers_are_pristine(dated_reforms, simulation): + state = getattr(simulation, "_construction_reform_state", None) + return state is not None and state.pristine(simulation, dated_reforms) + + def _legacy_override_modifier(dated: DatedReform) -> Callable[[Simulation], None]: """Override changes that are no longer effective on the simulation. @@ -90,13 +162,26 @@ 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) + applying = getattr(simulation, APPLYING_TO_BUILT_SIMULATION, False) + construction_owned = not applying and _construction_modifiers_are_pristine( + (dated,), simulation + ) + state = getattr(simulation, "_construction_reform_state", None) + construction_replay = ( + not applying + and state is not None + and dated in state.owned + and not construction_owned + ) + every_change = applying or construction_replay + if construction_replay: + _warn_annualised_overrides() if every_change: _, legacy = read_override_reform( dated.changes, dated.reading, lambda path: False ) overriding = True - elif dated in (getattr(simulation, "_building_dated_reforms", None) or ()): + elif construction_owned: _, legacy = dated.read( reform_parameter_defines(), warn=True, @@ -114,11 +199,7 @@ def modifier(simulation: Simulation) -> None: parameter_reads_calendar_years(), simulation.tax_benefit_system, ) - overriding = overriding or ( - bool(legacy) - and dated - not in (getattr(simulation, "_building_dated_reforms", None) or ()) - ) + overriding = overriding or (bool(legacy) and not construction_owned) if legacy: if overriding: if not every_change: @@ -138,6 +219,8 @@ def modifier(simulation: Simulation) -> None: require_annual_value=overriding, ) raise + if construction_owned: + simulation._construction_reform_state.accept_owned(simulation, dated) from policyengine_uk.scenarios.uc_reform import warn_shadowed_lcwra_reform warn_shadowed_lcwra_reform(simulation) @@ -238,7 +321,7 @@ def chained(simulation: Simulation) -> None: replay = ( dated is not None and not applying - and not getattr(simulation, "_building_dated_reforms", None) + and not _construction_modifiers_are_pristine(dated, simulation) ) if replay: _warn_annualised_overrides()