Run each part's modifier at its own time in a sum of scenarios

[MaxGhenis]

Fixes #2406

Problem (reproduced on main 714b143, MOCK two-household dataset, fiscal year 2025)

• Every sum was applied after data load. Scenario.__add__ dropped applied_before_data_load.
• So no_economic_assumptions + reform froze the parameters but not the data. remove_economic_assumptions zeroed the growth in the parameters only after Simulation.__init__ had uprated the dataset's later years with it, and disable_economic_assumptions was set too late for build_from_multi_year_dataset to reset them.
• Other before-load scenarios were affected too: the documented macro route (Scenario(parameter_changes={...yoy_growth...}, applied_before_data_load=True)) plus a reform, and even the sum of two before-load scenarios.

MOCK data	AE growth 2027	employment income 2025 / 26 / 27	income tax 2027 (PA £20,000)
no_economic_assumptions	0	30,000 / 30,000 / 30,000
no_economic_assumptions + reform, main	0	30,000 / 31,020 / 31,764	£2,352.90
no_economic_assumptions + reform, this branch	0	30,000 / 30,000 / 30,000	£2,000

Setting one flag for the whole sum would be wrong. A modifier that edits the data (one that calls sim.calculate) would then run before the populations exist, as test_flagging_the_whole_sum_would_run_the_data_edit_without_data shows.

Fix

• Each part's modifier runs at the part's own time. A sum whose operands are applied at different times keeps the modifiers of the before-load parts and of the after-load parts, each chained in the order added. Simulation.__init__ runs the before-load ones before it builds the data, and the after-load ones at the end, where a scenario's modifier always ran. Data-editing modifiers therefore still see the data.
• All of a mixed sum's parameter changes come first. They are applied as + merged them, which is the precedence Scenario.apply gives them, before every modifier and before the data is built. Changing the parameters rebuilds them, so applying an after-load part's changes after a before-load modifier would discard that modifier's direct edits, for example a structural Reform class applied before load (review r1). A deliberate consequence: in a mixed sum, an after-load part's change to a growth input also re-projects the data. Key the URL dataset cache on the uprating inputs, and let parameter changes that reach them re-uprate the data #2405 does the same for any scenario.
• Scenario.parts_by_data_load() returns (before, after), which is what Simulation applies at each time. before carries the sum's parameter changes, copied, and the before-load modifiers; after carries the after-load modifiers only.
• Public fields are unchanged. parameter_changes, dated_reforms and the chained simulation_modifier hold the whole sum, as the previous + made them. Scenario.apply and code that reads the fields see the whole sum. Sums whose parts share one timing take no new path. That covers every Scenario.from_reform tuple and policyengine.py's modifier + reform sums.
• applied_before_data_load is True for a sum when every part that changes something is applied before data load, so no_economic_assumptions + macro now runs before data load. A mixed sum reads False. Scenario() has no timing of its own.
• A sum is fixed when made.
  • Its chained modifier binds its operands' modifiers when the sum is made (it used to look them up when called).
  • A sum whose flag or modifier is reassigned afterwards is applied as one scenario, as its fields say.
  • In-place edits to its parameter changes are honoured.
  • __deepcopy__ shares one memo between fields and private attributes, so model_copy(deep=True) keeps the split with callable-object modifiers.
• URL dataset cache. A simulation with a part applied before data load neither reads nor writes the hf:// and gs:// dataset cache. On main, no_economic_assumptions alone (and so the sum, once fixed) cached its frozen projection under the plain URL, and the baseline and every later simulation in the process picked it up (review r1).

Invariants (tested)

1. Partition and order: every modifier runs exactly once, at its part's time. A before-load modifier sees no populations and an after-load one sees them. Within a time, modifiers run in the order added, however the sum is bracketed. Tested by test_a_simulation_runs_each_part_once_at_its_own_time (a property over a real Simulation) and test_a_sum_splits_its_modifiers_by_timing.
2. Parameter changes before modifiers: every modifier sees every part's parameter change, with the later operand's holding (same property). A before-load part's direct edits and Reform classes survive an after-load reform, and the parameters and income tax match the all-after-load sum (test_a_before_load_parts_edits_survive_an_after_load_reform). Timing does not change which change holds (test_a_sums_parameter_changes_keep_the_precedence_of_plus).
3. Backward compatibility, tested against a copy of the previous __add__: a sum's fields and chained modifier equal those the previous __add__ made, for any bracketing. Uniform-timing sums are unchanged apart from the flag. Scenario() adds no part.
4. Freeze, tested against no_economic_assumptions alone: no_economic_assumptions + reform, in either order, has the data (every table, every year) and the economic assumption parameters of no_economic_assumptions alone, plus the reform's change. Tested on a single-year dataset, a multi-year dataset (input not mutated), a situation, and hf:// and gs:// URLs in both cache orders, with the baseline and later simulations kept at current law's projection.
5. Before-load parameter changes reach the data: MACRO + reform, in either order, projects the data as MACRO alone does: 20% growth into 2027, where main gives 2.4%.
6. Data-editing modifiers applied after data load still see the data in a sum with no_economic_assumptions, in either order.
7. A sum is fixed when made: later edits to an operand don't reach it. Reassigning the flag or modifier applies it whole. Returned parts are copies. Copies keep the split.

Reproducer: policyengine_uk/tests/test_scenario_sum_timing.py::test_no_economic_assumptions_plus_reform_freezes_the_data

On origin/main (dd64849), 24 of the 35 new tests fail, including the reproducer in both orders and every URL-cache case. The 11 that pass check behaviour main already had: the parameter freeze, a situation, the precedence of +, before-load direct edits on a situation (main applied every sum after load), Scenario.apply, and that one flag for the whole sum fails.

Reviews

• r1 (Opus 5.5, at cbd909d, REQUEST_CHANGES) found the URL cache leak, the discarded direct edits, deep copies of callable-object modifiers, parts returned by reference, and doc claims. All are addressed above. The earlier operand-keeping split, which also had to follow the sum's bracketing because the merge of parameter_changes isn't associative, is gone.
• r2 (Opus 5.5, at 0d21113): APPROVE. It confirmed every r1 finding fixed, with mutation runs. Removing the cache skip fails all 5 URL tests. The split property is not vacuous: 153 mixed sums had an after part. Its follow-ups are applied in ec25db7: the Enum cache test, the calculated-input assertions under the freeze, and docs wording. Re-reviewed at ec25db7: APPROVE.
• Sol r2: cancelled. Two attempts failed on lane transport errors, and no Codex lane was free. The Opus review is the independent review for the gate.
• 05a28f3 merges origin/main, which includes Read calendar-year series' bare years as calendar years, and make generated-series reforms take effect #2404's canonicalised merge in Scenario.__add__. scenario.py and simulation.py merged cleanly. The only conflict was the after-load paragraph in scenarios.md: I kept Read calendar-year series' bare years as calendar years, and make generated-series reforms take effect #2404's text and added this PR's one sentence.

Follow-up, filed by the UK hub as #2417: under no_economic_assumptions, Simulation.dataset holds the projected, un-reset dataset. build_from_single_year_dataset, build_from_dataset and build_from_file overwrite the reset copy that build_from_multi_year_dataset sets. So sim.dataset can disagree with the values the model calculates. That happens on main through the URL cache, and with this PR it also happens for no_economic_assumptions plus a growth change. The hub is building the fix.

Open PRs this touches

• Key the URL dataset cache on the uprating inputs, and let parameter changes that reach them re-uprate the data #2405 (uprating inputs reach the data) changes the same lines of Simulation.__init__ and _dataset_cache_key. Rebase plan:
  • When there is a before part, every parameter change is already before the build, so Key the URL dataset cache on the uprating inputs, and let parameter changes that reach them re-uprate the data #2405's probe isn't needed.
  • When there is no before part, apply Key the URL dataset cache on the uprating inputs, and let parameter changes that reach them re-uprate the data #2405's condition (since de78bb8: _projects_data(situation, dataset) and self._scenario_changes_uprating_inputs(...)) to after_data_load.
  • The cache: keep both checks in _dataset_cache_key: _scenario_before_data_load first, then Key the URL dataset cache on the uprating inputs, and let parameter changes that reach them re-uprate the data #2405's fingerprint. A before-load structural reform that changes no parameter still poisons the cache through _pre_encode_enum_columns if only the fingerprint is checked (test_a_before_load_structural_reform_keeps_out_of_the_url_dataset_cache, review r2).
• Read calendar-year series' bare years as calendar years, and make generated-series reforms take effect #2404 canonicalises each operand's parameter paths in __add__'s merge. The parts reuse the sum's merged fields, so it composes; whichever PR lands second keeps those lines.

Checks run

• ruff format --check ., ruff check ., and ruff check --select F811,F821 policyengine_uk: clean.
• pytest -n 4 on the merged head (0d21113): 487 passed, 0 failed. That covered the new file (35 tests) plus test_reform_representations, test_dated_reform_routes, test_no_economic_assumptions, test_uc_rebalancing, test_triple_lock_statutory_inputs, test_capital_gains_projection, test_threshold_indexation, test_domestic_rates_uprating, test_dataframe_dataset, test_abolish_benefit_cap_scenario and test_simulation_dataset_sources. On ec25db7 the changed tests pass, and the Enum cache test fails with the flag disabled.

axiom: n/a: infra

🤖 Generated with Claude Code

[MaxGhenis]

Hand-off to the UK hub.

• Head: 05a28f3 (merge of origin/main, including Read calendar-year series' bare years as calendar years, and make generated-series reforms take effect #2404, into ec25db7).
• Review: independent Opus 5.5 review, APPROVE at 05a28f3, after r1 REQUEST_CHANGES at cbd909d and r2 APPROVE at 0d21113 and ec25db7. The record is in ~/reviews/pe-uk-2407-scenario-sum-timing/REVIEWS.md, with reproduction scripts in opus-review/. Sol review attempts were cancelled after lane transport failures, and the Codex lanes were closed.
• Passed: Changes, Lint, Fix evidence, documentation builds, and the smoke imports on 3.11–3.14. All checks passed on ec25db7.
• Locally: 487 passed on 0d21113. On origin/main, 24 of the 35 new tests fail, including the reproducer.
• Pending: the Test / Python 1–4 and YAML shards on 05a28f3. A local re-run on the merged tree is paused by Fleet ops (CPU freeze until 21:30Z); CI covers it.
• Merge: the merge queue (merge method MERGE). No approving review is required. No conflicts; MERGEABLE.
• Depends on: nothing. Interacts with Key the URL dataset cache on the uprating inputs, and let parameter changes that reach them re-uprate the data #2405: whichever lands second rebases. The plan is in the PR body: keep _scenario_before_data_load and Key the URL dataset cache on the uprating inputs, and let parameter changes that reach them re-uprate the data #2405's fingerprint check together in _dataset_cache_key.
• Follow-up: Under no_economic_assumptions, Simulation.dataset can hold the uprated dataset while calculations use the frozen one #2417, the sim.dataset reset under no_economic_assumptions, owned by the UK hub.
• Holds and decisions: none.
• Cleanup after merge: worktree ~/PolicyEngine/worktrees/pe-uk-scenario-sum-timing (branch scenario-sum-keeps-timing). Its paused pytest belongs to Fleet ops' freeze and finishes on its own after 21:30Z.

🤖 Generated with Claude Code

[MaxGhenis]

Merged by the merge queue at the handed-off head 05a28f3ae. Landed by the UK hub on the gates:

• An independent Opus 5.5 review approved every head:
  • r1 REQUEST_CHANGES at cbd909d, fixed in b20ac13;
  • r2 APPROVE at 0d21113 and ec25db7, run with tests;
  • the merge with main (Read calendar-year series' bare years as calendar years, and make generated-series reforms take effect #2404) approved at 05a28f3 by reading.
    The record is in the hand-off comment and ~/reviews/pe-uk-2407-scenario-sum-timing/REVIEWS.md. The Codex lanes were closed, so Opus was the reviewer.
• Every check passed on 05a28f3, MERGEABLE, no CHANGES_REQUESTED.
• Impact (MOCK, in the body): no_economic_assumptions + reform now keeps the data frozen. Sums whose parts share one timing are unchanged.
• Follow-ups:
  • Key the URL dataset cache on the uprating inputs, and let parameter changes that reach them re-uprate the data #2405 rebases on this, keeping _scenario_before_data_load and Key the URL dataset cache on the uprating inputs, and let parameter changes that reach them re-uprate the data #2405's fingerprint check together in _dataset_cache_key;
  • Under no_economic_assumptions, Simulation.dataset can hold the uprated dataset while calculations use the frozen one #2417 (sim.dataset under no_economic_assumptions) is owned by the UK hub.