[MaxGhenis]

Fixes #2426

Part of #2427 (items 1 and 3; item 2 stays open).

Current head: 2ffa1b13ede68cd8170262e078319fcd60840c6a. Merged main: e7610ac52570b4f96d389aef98db9bb994bc3542.

A downstream Budget replay called Scenario.from_reform(d).simulation_modifier(sim) on an existing simulation and silently retained the old UC health-element amounts. The modifier now applies outstanding changes as annualised overrides and reports paths and keys when an override cannot affect an annual value. LCWRA amount reforms warn when UC rebalancing supersedes them, and Core simulations built from data receive the existing seeded claimant draw.

Round 4: strict no-op boundary and mixed write order

The previous guard considered statutory assignments and legacy overrides separately. That could accept a mixed composite after checking the legacy writes after the statutory timeline, even when the requested order ended with a statutory write. For example, build with a 2027 window setting the basic rate to .31, apply a legacy "2027-01-01" override of .35, then request (legacy, window): the modifier kept .35 where Scenario.apply gives .31.

The shortcut now requires a conservative proof of the requested sequence. It skips a standalone dated composite only when:

• Its writes are entirely statutory or entirely legacy.
• Their exact joint order and values match a suffix of the corresponding applied history, filtered to the affected paths.
• The other history contains no writes to those paths.
• The current parameter values still match those applied writes.

Every standalone mixed statutory/legacy composite on a built simulation replays its full requested order using the same override flag as Scenario.apply. A failed composite no-op proof replays all its children, retaining their joint order. A single pure reform can retain independently proven paths while applying its outstanding paths. Duplicate override-log entries are accepted, as on the existing legacy route. Overrides that cannot affect an annual value still raise with their paths and keys.

Actual construction keeps its existing statutory and legacy readings. A proven pure legacy construction reform remains a no-op on a clone or branch, including a single-day 6 April key; an external 6 April-only override still raises because it contains no 1 January. Actual construction still runs each modifier once; the passing copy controls retain the original construction values without reapplying the proven sequence.

The new regressions cover the external legacy override, reversed mixed tuple/sum order, reversed key order within one dict, clones, and branches with clone_system=True. The sequence property now samples legacy January keys alongside interval keys on shared paths and includes an explicit mixed counterexample.

Mechanism and invariants

utils/scenario.py preserves each dict or Core Reform.from_dict reading. utils/dated_reforms.py:591,684,711 restricts the no-op shortcut to the exact suffix/current-value checks; utils/scenario.py:202 replays an unproved dated composite through the apply flag. utils/dated_reforms.py validates annualised overrides and reports malformed paths/keys together; malformed Core keys retain the existing atomic per-child rollback route. The modifier does not rebuild parameters or rerun build-time UC inputs and structural reforms. The durable route is Simulation(scenario=...).

The outermost standalone composite defers UCHealthElementOverrideWarning until all children finish. An amount change followed by active=False completes without that warning, including under warning escalation; a single amount application still warns once. Warning years come from the shared production uc_reform.REBALANCING_YEARS, used by the overwrite loop and derived from the projection horizon. The data-source gate uses utils.data_source.built_from_data, restoring the seeded draw for Core dataset simulations.

• I1: outstanding modifier changes equal Scenario.apply, or raise naming paths; proven already-applied construction changes retain their construction reading.
• I2: construction outputs are unchanged, including windowed, bare-year, 6 April, single-day, month and scalar keys; dicts/classes; tuples/sums; and both data-load timings.
• I3: the warning fires iff an LCWRA amount reform reaches an active rebalancing year, following the overwrite loop's shared horizon.
• I4: the warning changes no value.
• I5: UK Simulation/Microsimulation retain their existing draw status: datasets draw; situations draw nobody.

The behavior changes are standalone modifiers applying outstanding writes or rejecting unreadable overrides, the diagnostic warning, and the Core dataset draw. #2427 item 2, the interpretation of active=False, remains open.

Ownership and integration

Round 4 changes the new guard code in dated_reforms.py and adds approximately 20 lines of routing in W1-owned utils/scenario.py. It adds no changes to simulation.py or tax_benefit_system.py. Earlier minimal simulation build hooks remain. W1's write_assignment, requires_projection, _entry and the dated-reforms module docstring remain untouched. The amount YAML keeps main's values, references and uprating, with the earlier description clarification.

The PR already integrated #2284's LCWRA freeze, #2241's SDLT changes, the Scottish top-rate correction and #2428's horizon-based rebalancing years, protected amounts, index reform and claimant-share parameter. Round 4 main merge result: Merged at 2ffa1b1. Current main is newer than the reviewed a94c65d base; one standard-allowance comment overlapped the diagnostic call in uc_reform.py. Kept main's uprating-index comment plus warn_shadowed_lcwra_reform(sim), retaining every executable main change.

Inherited round-three merge-tree controls: #2423 (08d177662), #2354 (11b806732) and merged #2428 (a66d1adbc) were clean against the then-head and main. #2421 (8e42bbdd5) had the same untouched dataset-source test conflict as its main control; simulation.py merged cleanly. Any refreshed integration results: Final-head merge-tree controls remain clean for #2423 and #2354; #2421 retains the same dataset-source-test conflict as current main. Raw outputs are in .job/artifacts/final-merge-*.

Modifier call sites

Calls under policyengine_uk/	Behavior
simulation.py, own before/after-load modifiers	Construction keeps its readings. An external reform called by a wrapper applies as an annualised override, except when the wrapper's call equals one of the scenario's own dated reforms: during construction that call keeps the construction reading and does not re-apply after another write to its paths (inherited from #2426's spec; the hub review's round 4 finding 1, follow-up issue #2480).
simulation.py, UC reform modifier; tests/test_uc_rebalancing.py	Existing UK awards and supplied claimant status retain their behavior; Core data simulations receive the seeded draw. The diagnostic waits for the complete build.
utils/scenario.py, Scenario.apply	Explicit annualised overrides retain per-child malformed-key rollback and one final warning check.
Removed OBR-elasticity and state-pension-age parameter tests	Removed-path rejection occurs before returning a modifier.
tests/test_scenario_sum_timing.py	Callback order and timing remain unchanged, including callback-only chains invoked with None.
Modifier, UC warning and draw regression files	Cover standalone and wrapped modifiers, apply equivalence, key errors, historical writes, copies, final active years, escalation and seeded draws.

Validation

Inherited round-three evidence at 636796c89: 112 targeted tests passed; independent revalidation confirmed 146 tests on the merged tree, all 54 previously pending nodes, 78 ordering probes, warning deferral and legacy-copy behavior against main. The 37 construction routes and both MOCK baselines were byte-identical. Full CI passed at that head. The round-four review identified the additional mixed-order cases described above.

Round-four results:

• New regression failures against 636796c89: All ten new fixed regression cases and the widened prebuilt sequence-property node fail for the reported behavioral reason (.35 != .31): 11 failures, no import/setup failures.
• New regressions at the fix: All ten mixed fixed cases and 18 existing order/copy controls passed in the early focused selection. That 29-node command was deliberately interrupted (exit 130) after 28 named passes to prioritize publication; its unnamed XML placeholder is excluded and the command is not reported as successful. The deferred prebuilt property subsequently passed, along with both widened sequence-property nodes, in the complete 231-test suite below.
• Three new test files, dated-reform routes, UC freeze and after-freeze suites: 231 passed, no failures/errors/skips; pytest and foreground launcher both exited 0 at exact pushed head 2ffa1b13e (-n4, numerical threads=1, MOCK only). Includes all ten new mixed cases and both widened sequence properties. Evidence: .job/artifacts/required-targeted-results.json and JUnit XML.
• Current-main/head differential, all 37 routes and both MOCK baselines: Passed: direct main e7610ac52, cloned main and cloned head 2ffa1b13e are byte-identical (1,054,315 bytes each; SHA256 3d6d8ecd36fa3444b29c4e110650a28a4fbb1e9b580dbcafaf528381c35ca3ef). All differential processes and the supplied summarizer exited 0; exact revisions and 37-route/two-baseline assertions are in .job/artifacts/differential-summary.json.
• Reviewer scripts p1/order_probes.py, p2/warning_probe.py and scripts/legacy_and_rollback.py: All four supplied-script invocations exited 0: 78 order comparisons, 53 warning checks and ten legacy/rollback records on each source. Against accepted round-three payloads, all order/warning/main records and eight of ten head legacy records are unchanged. The only differences are the two diagnostic prefixes described below; all parameter values and override counts agree.
• make format and uvx ruff format --check .: Passed before both commits and before the single successful push (1730 files already formatted on the merged tree).
• CI for the round-four head: All hosted checks for 2ffa1b13e succeeded, including all four Python shards, YAML, Test / Result, Python 3.11–3.14 smoke imports, lint, fix evidence and documentation. Workflow. Captured independently from local validation in .job/artifacts/ci-final.json.

The auxiliary round-three payload comparator exits 1 to flag exactly two differences: malformed tuple_classes and sum_classes standalone modifiers now raise ValueError: Cannot apply annualised overrides: rather than ValueError: Cannot read reform overrides:. The affected path (gov.hmrc.vat.standard_rate), invalid key (bad-vat-key), remaining error detail, before/after values (.20/.31) and override count (1) are identical. This is 149/151 unchanged payloads, with no value or log-count differences. Evidence: .job/artifacts/r3-probe-comparison.json and r3-diagnostic-changes.json.

All local model validation uses MOCK data, foreground processes, at most four pytest workers and BLAS/OpenMP threads capped at four. Documentation build: not run locally for the small prose correction; both hosted documentation checks passed at the pushed head. Independent hub review and W4 coordination remain open.

Reproducer: policyengine_uk/tests/test_from_reform_modifier.py::test_mixed_composite_modifier_replays_after_external_legacy_override, policyengine_uk/tests/test_from_reform_modifier.py::test_mixed_composite_modifier_replays_reversed_statutory_and_legacy_order, policyengine_uk/tests/test_from_reform_modifier.py::test_scorecard_modifier_changes_parameters_and_lcwra, policyengine_uk/tests/test_uc_health_warnings.py::test_standalone_composite_defers_warning_until_active_override_runs

axiom: n/a: reform routing diagnostics and simulation data-source plumbing do not change statutory policy rules

Hub note (10/17): branch updated with main 1bdc7a0ab (merge 546e351ae, tree 8e9f00af). Main's changes since the review's base b4dbaf57a are #2061's Pension Credit files only, with no file in common with this PR. Hub review round 4 (APPROVE WITH NITS at 2ffa1b13e) ran its six test files, probes and differential on 2ffa1b13e merged with b4dbaf57a. Its docs, changelog and test nits are in #2480.

[MaxGhenis]

Continuation handoff at 4785c9918586cf666c4d2bb9d41b0739cb69b065 (includes main 72fc91559).

• Targeted modifier/warning/draw suite: 83 passed; rollback-focused rerun: 3 passed, including the previous existing-suite failure. Existing suites: PENDING.
• make format and uvx ruff format --check . passed. Locked Python3.13.9/core3.33.2, <=4 workers/threads, foreground, MOCK only. No real-data microsimulation or downloads.
• Inherited differential at original guard fcc035f4 vs original main 1a31ef1fd: 37 construction routes plus MOCK baselines byte-identical, SHA256 57cb3015e3cf424da336b05c3743978387091ab378abb04ab1c4be34628af1b1. Copied artifacts verified; no construction differential rerun because continuation changes malformed-key handling and horizon naming/location, not valid construction behavior. Merged main includes its own freeze/SDLT changes.
• Merge-tree: Resume indexation from the changed amount after a dated change to an indexed threshold #2423 and Give a simulation its identifiers for a year before its data #2354 clean. Keep frozen simulation datasets aligned with loaded inputs #2421 has the identical pre-existing dataset-source test conflict against main; simulation.py clean. Keep the protected UC LCWRA amount from 2030-31 #2428 conflicts in uc_reform.py; retain its horizon/share logic with our helper draw gate/warning and remove the redundant earlier fixed-year declaration. Exact heads and resolution instructions are in the PR body and .job artifacts.
• Freeze the UC LCWRA element for 2026-27 to 2029-30 (UC Act 2025 s. 3) #2284 description-only conflict resolved by combining descriptions, retaining its values/uprating. Minimal W1 hooks, no refactor. UC health element reforms can silently score £0: LCWRA amount unreachable in 2026-29, 'rebalancing off' is not pre-Act law, draw gate misses core sims #2427 item2 remains open. Hub owns independent review and release/W4 certification. No self-review, in-job reviewer or PR merge.

Evidence and handoff are in .job/ within the assigned workspace, including HANDOFF.md, one handoffs.jsonl row, logs/XML, merge-tree outputs, prior differential artifacts, and a verified rescue bundle.

Published by the UK hub from the continuation job's bundle (head 4785c991) after both build jobs reached Subfleet's 6 h wall; the job's own pushes failed on GitHub TLS. Evidence: hub folder jobs/fix-2426-uc-guard/rescue-r2/. Existing suites at the continuation: 328 tests, 0 failures; targeted: 83, 0 failures.

[MaxGhenis]

Addressed both round-three defects at pushed head 636796c89edd0ac7419ffa1105f3c2b1d5a455d3 (six forward commits; main 36d2feb57 merged).

P1 checks effective ordered writes, including later recorded/direct overrides. Both reported ordering examples now produce .31, matching Scenario.apply. P2 defers warnings until the outermost standalone tuple or sum finishes: amount-then-active=False warns zero times and succeeds under escalation; a single child still warns once per application. Already-effective construction reforms remain no-ops on copies, clones and branches, including legacy single-day 6 April keys. Unreadable external overrides name their paths and keys. Warning years come from the shared uc_reform.REBALANCING_YEARS definition.

Final-head validation: 112 targeted passed, 45 scoped integration passed, the unchanged callback reproducer 1 passed, and 98 reviewer checks across 20 records passed. All 37 construction routes plus 2 MOCK baselines are byte-identical to merged main (SHA256 b54cf0301f77182e00def0200072c70c937d640ec99ce108f0985a263201ac35). Full final-head CI passed, including all four Python shards and Result. All six production hashes match committed HEAD.

The six-hour wall limited the required local bulk run: 275 passed, 3 skipped, no observed failures, then interrupted with exit -2; 54/332 cases remained in that original command. Its workers loaded parent b8a1911a0, before the absent-system diagnostic guard; they did not load the final guard. A separate required case passed at the final head, giving 279/332 unique accepted results and 53 pending IDs. The two-case additional run was canceled with no completed results or assertion failures; it contributes no coverage. Exact pending IDs, interruption records and both source manifests are in .job/final-results.json. These partial local runs are not reported as successful commands; full final-head CI is independently green.

#2423, #2354 and merged #2428 have clean head/main merge controls. #2421 retains the identical untouched dataset-source test conflict as main; W1 functions/docstring remain unchanged. The hub owns independent re-review and W4 coordination; #2427 item 2 remains open. All model processes are stopped and generated .hypothesis data is deleted. No local real-data build, self-review or PR merge.

[juaristi22]

Program review

Base repository: PolicyEngine/policyengine-uk
PR number: 2434
Reviewed head SHA: 636796c
Merge base SHA: 36d2feb
Mode: full
Scope: All changed reform-modifier replay/order/no-op/annualisation, construction compatibility, warning deferral and restored Core dataset claimant draw through finalUC.
Source manifest: /private/tmp/policyengine-command-runs/202cefbfced3/pr-2434-review-sources.json
Review status: COMPLETE

Source Documents

No source documents registered; see scope and validation.

Critical

None.

Should Address

None.

Suggestions

None.

Evidence Gaps

No material gaps reported.

Notes

• 112changedfile +10directrepositorytests PASS; two validfinalUCcontrols verify Coreseed43/suppliedstatus andstandaloneoverride+explicitrebalancing throughUC/HBAI.
• Rawcombinedcommand123PASS/1FAILED exit1: solefailure invalidreviewerperiodfixture, preservedasrawfailure. Correctedexternalcaseonly reran1PASS; noPRtestfailure orchangedsettings.124validselectedcases, notcombinedexit0.
• No statutoryvalue/reference/forecast/seed/share changes; amountYAMLdescriptiononly. Rootperformedpolicy/sourceapplicabilityrole; validemptyv1manifest,0newsourceacquisition, no legalcertification orencoder.
• Authorparent-local332casebulk remainsincomplete, mixed279/332coverage distinct; fullcapturedheadhostedCIallSUCCESS is separateevidence. No privatepopulation/replay/fulllocalmodelsuite.
• LocalCore3.32.27/Hypothesis6.168.2 differsfromlockedCore3.33.2/Hypothesis6.169. Sixproductionhashesmatchbody/capturedcommit; snapshotimports/cleanlinessverified.
• Implementationreviewclear; bodyW4coordinationremainsopen. FreshrequiredCI/activity/conflict/stackgates stillapply, no merge-readyclaim. Rootpolicyworkinterleaved/notseparatelytimed.

Validation Summary

124validselectedcases PASS (122repository+2finalUCcontrols). Initialcombined123pass/1invalidreviewerfixturefailure,312.34spytest/317.51sprocess; correctedonecase1PASS1.26s/4.11s. ExactheadallhostedCIchecksSUCCESS inclLint/TestResult/YAML/4Python/Fixevidence/docs. Allsixproductionhashesmatch, unchangedamountvalues/source/helper/pipeline.0sourceoriginals/renders;0findings/gaps. Originalparentbulk completionnotclaimed.

Timing

elapsed seconds: 877.43s; setup seconds: 2.08s; code role seconds: 686.07s; source acquisition seconds: 0.00s; adjudication seconds: 0.00s; consolidation cleanup seconds: 146.16s

Review Severity

APPROVE. Open findings: 0 critical, 0 should address, 0 suggestions.

[MaxGhenis]

Addressed the round-four mixed-order finding at 2ffa1b1 (fix 330392565, merge of main e7610ac52). The single push succeeded on its first attempt; history was preserved.

A standalone mixed statutory/legacy composite now replays its complete requested order through the same override flag as Scenario.apply. A pure dated composite skips only when its exact joint write sequence matches the latest path-filtered suffix of one applied history, the other history has no writes to those paths, and current values still agree. An unproved composite replays every child in order. Construction retains its reading; duplicate override-log entries are accepted. The PR body and usage documentation state this boundary.

Validation at the pushed head:

• All ten new mixed fixed cases and the widened prebuilt sequence property fail behaviorally at 636796c89 (.35 != .31): 11 failures, no import/setup failures.
• The six requested files passed: 231 tests, no failures/errors/skips, pytest and launcher exit 0, -n4, numerical threads=1, MOCK only. This includes all ten new fixed cases, both widened sequence properties and the relative-reform clone/branch controls. The earlier focused command had been interrupted after 28 passes (exit 130); its deferred property is now resolved by this complete run.
• Fresh supplied differential and summarizer passed: 37 construction routes and both MOCK baselines byte-identical across direct main e7610ac52, cloned main and pushed head. Each output is 1,054,315 bytes, SHA256 3d6d8ecd36fa3444b29c4e110650a28a4fbb1e9b580dbcafaf528381c35ca3ef.
• All supplied R3 probe invocations exited 0: 78 ordering comparisons, 53 warning checks, and ten legacy/rollback records each for head/main. The auxiliary payload comparator exits 1 because exactly two malformed head tuple/sum class modifier records changed error prefix from Cannot read reform overrides: to Cannot apply annualised overrides:. The path, invalid key, remaining detail, before/after values and override count are identical. All other 149/151 payloads are unchanged; no value or log-count changes.
• Formatting/lint and the pre-push format check passed. All hosted CI checks passed at this exact head, including all four Python shards and Test / Result.

Current main's only merge conflict was a standard-allowance comment overlapping the warning call; both were retained. Final #2423/#2354 merge-tree controls are clean; #2421 retains the same untouched dataset-test conflict as main. All 2,753 source hashes stayed unchanged through validation. Generated .hypothesis was deleted; all local validation processes exited.

The work is pushed and .job/HANDOFF.md plus the single .job/handoffs.jsonl row contain the evidence handoff. Independent hub review and W4 coordination remain open; #2427 item 2 stays open. No PR merge or self-review.

[juaristi22]

Rereviewed 2ffa1b13ede68cd8170262e078319fcd60840c6a against the previous reviewed ancestor 636796c89edd0ac7419ffa1105f3c2b1d5a455d3. Independent code/test and policy-applicability reviews are COMPLETE, with no actionable findings or material evidence gaps. The conservative no-op proof and full replay of an unproved mixed composite preserve the requested statutory/legacy write order, construction readings and per-child rollback.

Accepted coverage is 54 head cases and 10 separate cases on the clean integration with actual main 14999dc03502a2a269f0c955685593574da014d6 (tree 7f31ad2fa6ad88cb22d67596c7ecf4986ddd9993). This includes all ten new fixed cases, both original 32-example sequence properties and the unchanged 25-example warning property. The initial command retained 60 repository passes and four failures in two reviewer lifecycle premises repeated across both trees. The modifier does not rebuild stored UC inputs; correcting only those probes to use explicit rebalancing or a statutory construction disable passed all four separately. No passing repository cases were rerun, no PR code changed, and no single all-pass initial session is claimed. The final hypothetical UC controls give £8,698.80 and £12,298.80 through the documented routes.

Applicability checks independently confirmed unchanged statutory amounts, references, uprating, eligibility, seed and claimant shares; the amount YAML changes only its description. axiom: n/a is supported for this routing/data-plumbing scope. No external legal certification, private population replay, author 231-test count or rerun of all 124 prior-review cases is claimed. Local Core 3.32.27/Hypothesis 6.168.2 differs from locked 3.33.2/6.169.0.

W4 coordination, #2427 item 2 and the recorded #2421 integration issue remain separate merge conditions. This approval covers the implementation at the captured head and does not clear those conditions.

[juaristi22]

Incremental re-review of 546e351aeaeb: COMPLETE, no new findings.

Independent policy/source and code/test reviews checked the main integration: 101 of 103 incoming paths match captured main, while the dated-reform changes preserve the previously reviewed routing/replay logic. All 38 focused cases passed, including copy/order/no-op, indexation, rollback and final-UC lifecycle controls. Earlier test totals remain historical. This is an engineering review; no new statutory policy or encoded parity is certified.

Required hosted CI is still pending. The inherited constructor boundary in #2480, W4 coordination, #2427 item 2's active=False interpretation and data-source integration qualifications remain; this approval does not establish merge readiness.

Local validation used Core 3.32.27 rather than locked 3.33.2. Tests ran against captured main 1bdc7a0; main subsequently advanced to 3f8d4bd with release packaging/version-only changes, separately checked as outside this review scope. No tests on the new main combination are claimed.

[MaxGhenis]

Landed by the UK hub (merge queue, pinned to head 546e351aeaeb47e8ea0a4454f2d693d4230c1656). Fixes #2426, and #2427 items 1 and 3.

• The head: 546e351ae is the reviewed 2ffa1b13e merged with main 1bdc7a0ab by the hub (gh pr update-branch), giving tree 8e9f00af, so CI runs on the integrated tree as the review asked.
  • Main's changes since the review's base b4dbaf57a are Pay no Pension Credit child amount to a family with a tax credit award #2061's Pension Credit files only, and none is in this PR's 11 files.
  • The review ran its six test files (268 passed), probes and differential on 2ffa1b13e merged with b4dbaf57a.
• Gates at this head: gh pr checks exits 0, mergeable, not a draft, no CHANGES_REQUESTED.
• Review chain: the hub's independent GPT-6.1 Sol reviews gave REQUEST_CHANGES at 4785c991 and 636796c89, then APPROVE WITH NITS at 2ffa1b13e (Subfleet 20261016-190358-uk2434-review-r4).
  • All round 3 P1 probes pass (4/4, 6/6, 78/78), as do 100 of 100 new attacks and a 526-case random differential.
  • All 37 construction routes and both MOCK baselines are byte-identical to main.
  • It is semantically integrated with Keep prior law before a dated change that starts on a parameter's first stated date #2436 (the first-date case keeps £217.26 for 2024 and 2025 on every route) and with Resume indexation from the changed amount after a dated change to an indexed threshold #2423 (the indexation resume on construction and own-copy routes).
• Impact: none on current law. The 37-route differential is byte-identical. The fix makes Scenario.from_reform(d).simulation_modifier(sim) on a built simulation equal Scenario.apply, or raise naming the paths. LCWRA reforms warn when UC rebalancing supersedes them, and Core simulations built from data get the seeded claimant draw.
• Follow-ups (Follow-ups from #2434's review: construction-wrapper own-dict boundary, docs and changelog #2480):
  • the construction-wrapper own-dict boundary (finding 1; the body's call-site row is corrected);
  • docs on mixed-composite replay on copies and indexed thresholds;
  • a Scenario.apply changelog bullet;
  • a relative-reform copy test.