Repository navigation
Follow-ups from #2434's review: construction-wrapper own-dict boundary, docs and changelog #2480
Description
Follow-up from the UK hub's round 4 review of #2434 (approved with nits at 2ffa1b13e). None of these blocks #2434, and none is a regression from main.
1. A construction-time wrapper call equal to the scenario's own dict keeps a stale value (P2)
policyengine_uk/utils/scenario.py:99treats anyDatedReformequal to one of the scenario's own as "own", and takes the construction reading without an effectiveness proof.scenario.py:228and:241disable the composite proof and the full replay while_building_dated_reformsis set (simulation.py:260).
Reproducer. Let RATE = gov.hmrc.income_tax.rates.uk[0].rate, A = {RATE: {"2027-01-01.2027-12-31": .31}} and L = {RATE: {"2027-01-01": .35}}.
- Build
Simulation(scenario=Scenario.from_reform(A) + Scenario(simulation_modifier=wrapper)). - Inside
wrapper, callScenario.from_reform(L).apply(sim), thenScenario.from_reform(A).simulation_modifier(sim), or the composite(L, A).
The modifier leaves .35 on 1 January 2027, where .apply gives .31. A direct edit followed by a request for A leaves a stale .40. No error or warning is raised. On #2434's head, 4 of 12 such cases mismatch; on main, all 12 do.
Possible fix: treat an equal dict as "own" only while no foreign override or assignment has been logged on its paths since construction applied its parameters.
Care is needed: equality-based ownership is load-bearing for policyengine.py's composer (provenance/receipt.py, scenario_composition/v1), which runs each member's from_reform modifier inside its own wrapper. A naive identity or value check would also change behaviour for deep-copied scenarios and for LCWRA amounts that rebalancing overwrites.
Add a test pinning whichever boundary is chosen.
2. Docs and changelog
docs/book/usage/scenarios.md:421–422: state finding 1's boundary until it is fixed. Also say that mixed composites always replay, which rewrites a copy's own construction values. For example,{RATE: {"2026-06-01.2027-03-31": .31}, VAT: {"2027-01-01": .25}}on a clone of a simulation built with it moves the 2027 rate from .20 to .31.- Dated measures on indexed thresholds need
Simulation(scenario=...). On a baseline-built simulation, the modifier andapplyfollow the annualised reading and can't express Resume indexation from the changed amount after a dated change to an indexed threshold #2423's indexation resume. For example, a PA freeze to 5 April 2031 returns to 13,090 in 2032. changelog.d/2426.fixed.md: add a bullet.Scenario.applynow raises for keys covering no 1 January ("2027-04-06","2027-04-06.2027-12-31","2027-02-01.2027-12-31"), where main silently applied them from that date.
3. A test for relative reforms on copies
The review reply says "relative-reform clone/branch controls" passed, but no such test exists. The reviewer's probes passed 12 of 12: a Reform class, a modifier scaling the PA by 1.1, and a CPI growth-rate dict. Add them as a test.
Evidence: the UK hub's review files, reviews/uk2434/review-r4.md (local to the hub).
Activity