[MaxGhenis]

The six items deferred from the 2026-09-30 adversarial review of #6, batched into one engine change so the job cache is invalidated once and the results are rebuilt once. Rebased onto main (after #6, #11 and #12).

What changes

1. Cache key hashes what the engine computes, not its bytes. engine.job_key now uses engine_semantics(): each engine file's syntax tree (ast.dump, no line numbers) with comments, docstrings and other bare string statements removed, and pyproject.toml parsed. An edit to prose alone keeps every cached job; any change to code, names or values reruns them. The results file's provenance still records raw file hashes (engine_hashes, source_hashes), so test_not_stale still fails on any edit, and the rebuild after a prose edit only reassembles from the cache. households.key uses the same semantics.
2. One rounding. rules.round_rate (numpy's) is the only rounding of statutory inputs and rates: in the rules, rate_sources, the additional pension's September CPI (september_cpi), the Pension Credit guarantee (guarantee_growth, pension_credit_levels) and the guarantee check in run_path. Python's round() disagrees with it on 84 of the 151 half-grid values from -3% to 12% (for example round(0.0355, 3) is 0.035 and np.round gives 0.036). One round() stays on purpose: engine.model_triple_lock_rate reproduces policyengine-uk's add_triple_lock to check the model's own triple lock, so it rounds as upstream does. A differential test runs the upstream builder on half-grid inputs.
   • The existing Hypothesis "half-grid" strategy in tests/test_rules.py drew (k + 0.5) / 2000. Those are quarter-grid values, so it never produced a tie. It now draws true half-grid values (k + 0.5) / 1000, plus values on the 0.1-point grid so inputs tie with each other and with the floor.
3. Tie labels as in the history table. rules.triple_lock_source names what set a triple-lock rise. It is "floor" whenever neither input exceeds 2.5% (a tie with the floor included); otherwise it is the larger input, and CPI when the two inputs tie. engine.rate_sources and trajectories.triple_lock_history both use it. The Burnham plan's non-top-up years are "cpi" only above 2.5%. A differential test feeds the 2011-2026 published inputs through rate_sources and gets the history table's labels in every year, including April 2017 (earnings exactly 2.5%: the floor) and April 2022 (earnings suspended, tied with CPI: CPI). Before this change rate_sources said "earnings" in both.
4. engine.py docstring no longer says the Pension Credit guarantee follows CPI. It says the guarantee follows May-July earnings, and the check list includes it.
5. Pension types read back from the model. engine.held_pension_types reads state_pension_type from the simulation after pin(), in every year. It fails the run if any person's type in the model differs from the held one (an input the model ignored, recomputed or reordered), if the array lengths differ, or if a type is unknown. fixed_inputs.held_pension_type_records (unweighted) and held_pension_type_people (weighted) now come from the model, for every horizon year. The check also runs in run_history and run_coverage.
6. Job isolation.
   • run_jobs reports every failed job, including those that fail while the running jobs finish after the first failure. Previously only the failures seen at the first wake-up were reported. Queued jobs are cancelled; running jobs finish and stay cached. The error says how many failed and how many did not run.
   • Jobs start in their own session (run_child, start_new_session=True) and are registered. Ctrl-C, SIGTERM or SIGHUP (terminate_on_signals, used by run_jobs and cli.main) stops every running job's process group: SIGTERM, then SIGKILL after 10 s. An already-ignored SIGHUP (nohup) stays ignored.
   • A job whose build is SIGKILLed notices it has been re-parented (watch_parent, polling every 2 s; macOS has no parent-death signal) and kills its own group.
   • slot_lock replaces the blocking flock. It polls with LOCK_NB, writes the holder's pid into the lock file, logs who holds the directory while waiting, gives up with LockTimeout after an hour, and gives up at once with Aborted when the build is stopping.
   • The household examples use the same helpers. Job input and output files are removed on failure too; about 30 stale ones were left in each worker directory.
   • Also: config.py's comment says DWP "describes" the plan, as the README and dashboard do.

Invariants (each property-tested or differential-tested)

• Cache key (tests/test_cache_key.py, Hypothesis over the real engine files):
  • Random comments added anywhere, and every docstring rewritten, keep source_semantics and move the raw provenance hash.
  • Changing any code constant, renaming any name, or changing any binary operator moves it. So does a dependency pin in pyproject.toml.
  • Every module a job imports (transitively) is in ENGINE_FILES, and none of them reads __doc__.
• Rounding (tests/test_engine_pure.py, tests/test_rules.py):
  • For all inputs, half-grid included, the September CPI the additional pension uses and the rise the guarantee uses equal the inputs the State Pension rules use.
  • round_rate is np.round for scalars and arrays.
  • model_triple_lock_rate equals policyengine-uk's add_triple_lock on random and half-grid inputs.
• Labels:
  • For all inputs, the named input equals the rate paid.
  • "floor" appears exactly when neither input exceeds 2.5%, and a tie between the inputs is CPI's.
  • rate_sources equals triple_lock_history on 2011-2026.
  • test_results recomputes every run's rate_sources from its recorded inputs.
• Pension types:
  • The model's types equal the held ones person by person in every year of every run.
  • In the file, BASIC and NEW records never rise, are constant from 2028-29 (the age rise is the only change), and every year has the same number of people.
• Jobs (tests/test_jobs.py):
  • For any outcomes, failed + did not run + finished = every job, exactly the failed jobs are named, and every finished job is cached.
  • A job that fails after the first failure is reported.
  • No job process or grandchild outlives a build stopped by SIGTERM, SIGHUP, Ctrl-C or SIGKILL.
  • slot_lock names the holder, times out, waits for a holder that lets go, and gives up when stopped.

Mutation check before opening this: I reintroduced each old behaviour or likely regression one at a time. The tests caught all 12:

• Python round in september_cpi or in the guarantee;
• the old triple-lock or Burnham tie labels;
• round_rate in the model check;
• the type check disabled;
• raw bytes in the key;
• only the first failures reported;
• no new session;
• no kill on abort;
• no parent watch;
• a lock wait that never times out.

Results

Rebuilt in full (every job reran, because the cache key changed), then reassembled from the cache after the rebase onto main. Compared with main's data/results.json across central, expected_value, trajectories, coverage and benchmarks:

• No number moves. There are 0 numeric differences. The expected saving in 2039-40 is still £8.411bn gross and £5.392bn net, and the central path £0.659bn / £0.428bn.
• One label changes, as expected: the random path's April 2035 triple-lock rise is now "floor", not "earnings". Earnings of 2.465% publish as 2.5%, tying the floor.
• New keys only: the pension-type counts the model reads back each year (fixed_inputs.held_pension_type_records and held_pension_type_people). These are aggregate counts.

Checks

• Real-model smoke run before the rebuild: the central path and 2026-27 coverage on the Enhanced FRS.
  • The model reads back the held types with no mismatch. BASIC is 18,437 records in every year; NEW falls from 9,871 to 8,278 only at 2028-29, when the age rises.
  • The central path's 2034-35 and 2039-40 gross and net savings are identical to the committed file, bit for bit.
• After the rebuild and rebase: pytest 176 passed; dashboard vitest 63 passed; eslint 0 errors (2 old warnings); next build succeeds.
• On the real build, SIGTERM stopped the running Microcosm coverage job ("Stopping: killed 1 running job(s)") and left no job processes.

Downstream

• Working paper: Public Pension Indexation Under Uncertainty: Fiscal and Distributional Effects (/paper route) #8 (paper). paper/paperdata.py reads each path's rate_sources, so the paper needs a re-render on the new results for the random path's April 2035 label.
• The policyengine-uk bump (removing the Housing Benefit workaround, bumping to 2.102.5) is a separate change: it moves every number and waits on a policyengine.py release. It will rebase onto this one and rebuild once.

axiom: n/a: pipeline infrastructure (cache key, rounding helper, labels, run checks, job isolation); no policy encoded or changed

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
uk-triple-lock	Ready	Preview	Oct 1, 2026 1:21pm UTC

[vahid-ahmadi]

Independent review of adac499, with the later commits up to f1751cb checked too (docstring wording and test robustness; the findings below still apply there). Verdict: approve with nits. Nothing blocks, and the six changes do what the description says.

I reproduced the results diff against main: 0 numeric differences, one label change (random path, April 2035: earnings → floor), and 60 new aggregate pension-type count keys.

On 3.13/macOS the full suite gives 175 passed, 1 failed. The failure is test_moment_tilt_hits_every_target_whatever_the_scale (seed=283, unit=0.6875). It also fails on main (43e5102) and is outside this diff, so it predates this PR; I'll file it separately. test_jobs and test_cache_key passed 3× with no leaked processes.

Should fix

1. The tie rule isn't a convention (rules.round_rate docstring, METHOD.md). np.round is half-to-even on the binary float, so a tie's outcome depends on how the value was computed.
   • 17 of the 151 half-grid values round differently when built as (k+.5)/1000 vs pct/100 (e.g. 9.95% → 10.0% vs 9.9%), and round_rate(0.0345) is 3.4%.
   • The results are unaffected: none of the 6,151 statutory floats in results.json, nor the OBR line, sits on the half-grid.
   • But "the precision ONS publishes" shouldn't imply an ONS tie rule. Say "numpy's float rounding; immaterial for continuous draws", or round Decimal(repr(x)) with an explicit rule.
2. The first-failure log line (engine.py, "A job failed: cancelling…") names neither the job nor the error, and the build may now wait hours for running Microcosm jobs. Please include kind, key prefix and exception.

Nits
3. The except BaseException cleanup in run_jobs runs after terminate_on_signals has restored the handlers. A second SIGTERM or Ctrl-C during the 10 s grace period skips the SIGKILL sweep, leaving watch_parent as the only backstop. Suggest pthread_sigmask around kill_children.
4. __init__.py runs in every job but isn't in ENGINE_FILES. The coverage test only follows relative imports, so it can't see it or absolute triple_lock.* imports.
5. Consider sys.version_info[:2] in the key. Today it only moves because ast.dump differs between Python versions.
6. Python round() still appears at trajectories.py:400 (actual_rise, 3 dp) and model_horizon.py:125 (5 dp). Route the first through round_rate or say why not; note whether the second mirrors upstream.
7. held_pension_types reads the input back right after set_input. It catches the enum fallback, length and unknown names, but not a downstream formula that ignores state_pension_type. "Ignored, recomputed or reordered" still overstates that.
8. signal.SIGHUP as a default argument makes engine.py fail to import on Windows. Fine if Windows is unsupported; worth saying.
9. september_cpi and guarantee_growth now follow rate_decimals, so a None spec loses the old 0.1-point rounding. No current spec is affected; worth a line in the body.

Checked and fine

• Cache key: no job module reads data files at runtime (data comes through hashed args), and neither these modules nor policyengine-core reads __doc__.
• Signals and locks: handlers are restored, nohup's SIGHUP stays ignored, the stop flag is checked under the lock that registers each child, and the slot_lock holder line is correct.
• The dashboard's describeSource uses the same keys, the docs match the file, and only aggregate pension-type counts are written.

scenario/obr-premium rebases onto this with two small conflicts (cli.main, rate_sources). I'll open it once this merges.

[juaristi22]

Program review

Base repository: PolicyEngine/uk-triple-lock
PR number: 10
Reviewed head SHA: f1751cb
Merge base SHA: 43e5102
Mode: full
Scope: changed engine behavior and affected dependencies: semantic cache key, statutory rounding, source labels, pension-type readback, job/process/lock isolation, committed outputs and tests
Source manifest: /private/tmp/policyengine-command-runs/23429b7be472/pr-10-review-20261001-review-sources.json
Review status: PARTIAL

Source Documents

• obr-errors.xlsx — cached original.
• obr-central.xlsx — cached original.
• obr-lted-pr-copy.xlsx — cached original.
• pr-8-main-full-20260930-dwp-state-pension-uprating.txt — cached original.
• pr-8-main-full-20260930-dwp-tables.xlsx — cached original.

Critical

None.

Should Address

A1 — Apply the fiscal rounding rule in the policy-gap backtests (OPEN)

Location: src/triple_lock/ts_backtest.py:110-114, called by src/triple_lock/expected_value.py:313-320; this PR's new claim is docs/METHOD.md:7. Trigger: A simulated CPI or earnings input lies off the 0.1 percentage-point grid (the monthly model produces continuous values). Expected: The backtest used to judge the reform gap and choose the primary calibration should apply the same 3-decimal statutory-input rule as full fiscal paths (engine.spec_rates and expected_value.rule_levels). Observed: gap_pct calls rules.rates_matrix without decimals; its default is None, so it leaves the inputs and resulting rates unrounded. ev_backtest uses that function for every forecasted and realised four-year gap, while this PR's rules.round_rate and docs/METHOD.md say statutory inputs are rounded to 0.1 point. Recomputing the March 2018 EFO point-forecast origin directly from data/obr_forecast_errors.csv reproduces its committed unrounded gap 0.0652% (to four decimals); using fiscal 3-decimal rounding gives 0.0973%, a 0.0321-point change. For a larger illustrative model-generated case, the committed random path's 2029–2032 statutory inputs yield a four-uprating gap of about 0.5263% unrounded versus 0.3846% rounded (pure arithmetic from committed inputs; no population model needed). Impact: Historical gap bias and interval/backtest diagnostics validate a different policy arithmetic than the fiscal estimate; the effect on the 12-origin calibration ranking has not been recomputed. Pass the fiscal precision into gap_pct when it is used to validate the policy impact, then recompute and report whether the backtest choice changes. If unrounded gap scoring is intentionally a separate macro diagnostic, distinguish it explicitly from the fiscal-rule validation and narrow the new “one rounding” claim. The unrounded gap_pct call is inherited unchanged from the base, but this PR's new cross-component rounding assertion makes the inconsistency a directly affected claim.

A2 — A job that exits can leave a same-group descendant beyond cleanup (SHOULD ADDRESS) (OPEN)

Location: src/triple_lock/engine.py:925-940,943-960; affects _run_isolated at :1070-1084 and households.run at src/triple_lock/households.py:170-181.

Trigger: A model job starts a subprocess in its process group that closes inherited stdout/stderr, then the direct job exits normally or with a failure. Expected: The new session/child registry mechanism should stop that descendant before releasing the worker directory, consistent with run_child/kill_children documentation and the new process-isolation tests. Observed: run_child removes the direct Popen from _children after communicate() returns, without checking whether the process group has remaining members. Subsequent kill_children() sees no registered job and sends no signal. In a controlled reproduction using the exact extracted functions from this head, the child exited 0, a same-group grandchild wrote a heartbeat 0.3 seconds later, and kill_children reported zero processes. See diagnostic script and output. The diagnostic kills its grandchild in finally.

Impact and limits: If a PolicyEngine loader or other job code leaves such a subprocess, it can continue working in the worker directory after the lock is released; a later build can acquire that directory while it remains active. The in-repo job bodies do not directly launch such a descendant, and I did not establish one in a real PolicyEngine run, so this is a confirmed helper contract gap with contingent production impact, not a demonstrated wrong estimate or critical failure. The existing tests cover killing descendants while the direct child is still registered, but not after its exit. The SIGKILL parent watcher checks the original PPID of a still-running direct worker (engine.py:963-989), so it cannot close this gap after that worker exits. A focused test should cover a direct child exiting while a descendant remains.

Suggestions

None.

Evidence Gaps

• The changed local Pytest suite and a full PolicyEngine population build were NOT RUN independently: the local snapshot lacks numpy/hypothesis/PolicyEngine and licensed survey data. GitHub CI succeeded at this SHA; its success does not independently regenerate private population outputs.
• The revised 12-origin calibration ranking and gap diagnostics under fiscal 3-decimal rounding were not recomputed, so the size of that finding's effect on model selection is unknown.
• The process cleanup counterexample was reproduced with the exact helper functions; whether a production PolicyEngine worker leaves a same-group descendant after its direct process exits is unverified.

Notes

• The committed 2039 gross/net results and every pre-existing fiscal/household numeric output are unchanged. New pension-type diagnostic counts/weights are numeric additions, and one 2035 random-path source label correctly changes from earnings to floor.
• The one-rounding inconsistency in gap_pct predates PR Engine: semantic cache key, one rounding, history tie labels, model-read pension types, job isolation #10, but this PR states the cross-component rounding rule and adds differential tests, making the backtest discrepancy an affected claim.
• Five cached source originals passed local integrity checks. The DWP policy page was checked live; the NumPy half-tie rule is a model convention, not a statutory quotation.

Validation Summary

At exact head f1751cb, GitHub Dashboard lint/test/build, Pipeline tests and committed-output checks, and Vercel: SUCCESS. Read-only checks: AST import closure/parse PASS, exact-function process-group reproduction confirms escaped descendant, base/head JSON comparison confirms no pre-existing numeric impact differences, annual pension-type vectors present in committed results, March 2018 EFO gap arithmetic 0.0652% unrounded versus 0.0973% with fiscal 3dp rounding. Local Pytest and full population rerun NOT RUN; five cached source files passed SHA-256 integrity.

Timing

Not measured.

Review Severity

COMMENT. Open findings: 0 critical, 2 should address, 0 suggestions.

[vahid-ahmadi]

@juaristi22 thanks for the review. Both points are fixed in #16, with tests:

• A1: gap_pct now uses the fiscal 0.1-point rounding. Your example reproduces: 0.3846% rounded vs 0.5263% unrounded.
• A2: run_child kills the job's process group once the job exits.

Neither changes a published number; A1 only moves the backtest diagnostics. Since engine.py is in the cache key, #16's results land with the model-v2 rebuild (#14) instead of a separate multi-hour rebuild now. #10 is merged, with its results unchanged.