Repository navigation
Conversation
Port policyengine-us's scanner for engine-time randomness and hash draws as a code-health test. EXEMPT lists the package's SHA-256 digests, which fingerprint, key caches or check content; RATCHET lists today's draws: the splitmix64 helper, the formulas and uprating that call it, and the seeded numpy draws in participation and two scenarios. Cases that need newer syntax are filtered at collection, so no Python reports a skip. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwQq6H1FBndAKeuXdSGuft
|
Ready for hub review Head: Pre-flight
Comparison with the reference (after formatting)
The scanner from the constants down to Lists, reconciledMy run on I read every function behind the digests. All are SHA-256 used for provenance, a cache key or a content check, and none decides who gets something or meets a threshold. The two that the code compares are:
The ratchet draws I read:
One possible difference from the brief: without Observations, not acted on:
Star imports in a possible cycleThe scanner's own graph shows 9 of 1,432 star imports (1,418 of them from package modules) in a possible cycle:
Tests (all on
|
| Python | Collected | Passed | Skipped | Wall time | Peak RSS |
|---|---|---|---|---|---|
| 3.11.18 | 369 | 369 | 0 | 18.8 s | 56 MB |
| 3.12.3 | 380 | 380 | 0 | 22.7 s | 59 MB |
| 3.13.18 | 382 | 382 | 0 | 20.3 s | 59 MB |
| 3.14.6 | 382 | 382 | 0 | 18.8 s | 65 MB |
3.11 leaves out the 13 cases that need 3.12 or 3.13 syntax; 3.12 leaves out the 2 that need 3.13.
Caveat: the brief's bare uvx … pytest <file> --noconftest gives collection errors on every Python. pytest 9 sets up the policyengine_uk Package node above the file, which imports policyengine_uk/__init__.py, and that needs policyengine-core. I ran instead through a scratch plugin (-p skip_package_init) that turns _pytest.python.Package.setup into a no-op. The plugin is not committed. The guard itself imports only the standard library and pytest.
The guard file in the locked environment, serial, with conftest: 28.8 s wall, 316 MB peak RSS. Most of that memory comes from the conftest importing the model. The longest single call is the first package_scan (about 7 s; the scan alone takes about 7.8 s for 1,534 modules on 3.13).
The wheel: uv build --wheel (sha256 48570c87…), locked environment with --no-install-project, then uv pip install --no-deps the wheel, run from an empty directory against the installed tests directory: 382 passed. The scan reads the same 1,534 modules with identical counts and lines.
uv run pytest policyengine_uk/tests/code_health -n 4: 3,544 passed. Before I added the fixture, it gave 1 failed (test_every_test_module_cache_is_released) and 3,543 passed.
ruff format --check ., ruff check ., ruff check --select F811,F821 policyengine_uk (all ruff 0.17.2) and git diff --check pass.
Mutation checks (scratch copies; all killed)
import randomadded towarm_home_discount_random_draw.py: killed.stochastic import (1 found, 0 allowed).- A second
splitmix64_uniformcall inwarm_home_discount_random_draw.formula: killed.draw helper call (2 found, 1 allowed). - A new
new_drawsindata/economic_assumptions.pythat callssplitmix64_uniform: killed.new_draws: draw helper call (1 found, 0 allowed). - One ratchet occurrence removed (the module-level
Scenarioinpip_reform.pyreplaced): killed as stale bytest_allowlist_entries_still_match. - The
xorshiftentry raised from 3 to 4, which takesRATCHETto 22 against a ceiling of 21: killed bytest_ratchet_only_shrinksand as stale. - The
_fingerprintEXEMPTentry removed: killed. The hash use and 7 draw helper calls spread throughcapital_gains_projection.py,simulation.pyandtax_benefit_system.py.
Open work, checked
I ran the guard over each PR's head:
- Break wealth-decile ties by a fixed hash of household_id #2618 (
47ee0515b):_fixed_hashreports 3hash constantand 3xorshift, plus adraw helper callinhousehold_wealth_decile.formula, as the brief says. It also reports thewarm_home_discount_random_drawentry stale, because its base (3bcacaaa2) predates that file. - Give UK simulations a working subsample #2265 (
e4c54d7fa) and Count only supplied inputs that are still stored in their tier #2308 (5b4dc2143):utils/subsample.pyreports_random_state(hash use,random source), a module-levelrandom source, anddraw_household_counts(sampling callanddraw helper call).subsample_simulationandSimulation.subsamplealso report draw helper calls. Both also report findings and stale entries from their older bases (for example arandom sourceinuprate_student_loan_plans; and in Count only supplied inputs that are still stored in their tier #2308, 3random sourceinapply_participation_responsesand draw helper calls inSimulation.build_from_*). These clear when the PRs mergemain. - Reuse prepared current-law baselines for local H5 scenarios #2862 (
3886de2d1):_core_fingerprintreports a hash use and spreads to_identity,prepare_current_law_baseline,reuse_prepared_baseline,validate_prepared_baseline_request,Simulation.__init__andSimulation.prepare_baseline.
None has merged.
Judgment calls
- No changelog fragment.
check_changelog_fragments.pyonly warns when a PR adds none, a fragment would trigger a release for a tests-only change, and recent tests-only commits onmain(abab81f8b,c78f88649,8199bc05e,c9397c23e) add none.github-prs.mdsays every PR needs one, so the hub may wantchangelog.d/deterministic-engine-guard.changed.md. - I added the cache-release fixture (hunk 4 above) to meet pe-uk's own code-health rule.
- The PR body has no
Fixesline, because no issue exists for this work.
CI on 0835944 (checked 05:57Z)
All seven Test / Python shards, Test / YAML and Test / Result pass, so the strict outcome check found no new skip, xfail or XPASS. Changelog fragments, Changes, Documentation build, Fix evidence, Lint, the four install and smoke-import jobs and Release build also pass. Aggregate impact was still running at 05:57Z, after 73 minutes. No push was needed.
Skipped
Nothing needs private data. I didn't run the full suite; CI covers it.
Generated by Claude Code
Aggregate impact on certified data: passBase Checks: 0 broken by this change, 1 failing on both sides, 0 fixed. Moves flagged for review: 0 (thresholds in Notes
Moves flagged for reviewNone. Spending by benefit and service
Caseloads
Household net income by decileNo household's net income changes in any year. No change in any year: receipts by tax head; government balance, each component counted once; model totals; household income totals; population; poverty rates; people in poverty; inequality; mean household net income in each side's own deciles; standard reform costings (change in gov_balance); standard reforms: change in each reform's target. Checks failing on both base and head (1)
Counted-once checkEach of 92 components was raised by £1 per unit in 2026-27, and every head of its totals was read household by household: its own head must rise by exactly £1, no other tax head may move, no other benefit may rise, and the model's own total must move by what its heads moved. A head that breaks a rule is measured again twice: with every other head held at its baseline value, and with every other head held at its value after the first £1 while the component rises again. It fails where either still shows the response. 0 failed; 0 were skipped.
Provenance
Dataset: Receipts by tax head: head levels
Spending by benefit and service: head levels
Caseloads: head levels
Government balance, each component counted once: head levels
Model totals: head levels
Household income totals: head levels
Population: head levels
Poverty rates: head levels
People in poverty: head levels
Inequality: head levels
Mean household net income in each side's own deciles: head levels
Standard reform costings (change in gov_balance): head levels
Standard reforms: change in each reform's target: head levels
|
|
Ready for hub review Verdict: REQUEST_CHANGES at The port is faithful, every test passes on every Python, and the lists match the scan entry by entry. Three things need to change before merge: a model output that differs between runs and that neither the lists nor the stated limits cover, a missing changelog fragment whose cited precedent doesn't hold, and advice to four open PRs that the ratchet's own ceiling rules out. None is P1: CI is green and no model output changes. All runs below used a detached scratch worktree of the head, with FindingsP2-1. Formulas read the clock, and neither the lists nor the stated limits say so
A mutation confirms the guard can't see this. A new formula returning Replacement text:
P2-2. The PR needs a changelog fragment, and the hand-off's precedent says the opposite of what it shows
Add P2-3. Three of the four open PRs can't "carry their own entry"The body says: "The guard fails each of these PRs until it carries its own entry or changes." I merged the guard's head into each spot-checked head (
Replace the section's opening sentence and the #2862 bullet with: and add to the #2618 bullet: "Its hash orders tied households and so decides their decile; #2618 must order ties without a hash, for example by P3-1. The "82" sentence says the spread reaches 82 occurrences; it reaches 40The scan without Body replacement: "Without P3-2. "So
|
The guard's docstring now says it looks for draws and not for the clock (#2875). The comments on the splitmix64 and Universal Credit entries say which constants the scan recognises, which entries reach the hash and how Simulation.__init__ applies the modifier. Two test rows follow PolicyEngine/policyengine-us#10447 at 221f2fe44, and a changelog fragment records the guard. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
juaristi22
left a comment
There was a problem hiding this comment.
Review at 6f4d3313060987611bd00da46d3f392ffda1013e — COMPLETE; no confirmed findings or material evidence gaps.
All 389 focused checks passed (384 randomness-guard checks and five cache-release checks). The manual review covered alias/import resolution, scope handling, exclusions, model inventory and ratchet behavior, and compared the earlier-head author review with the fixes adopted in this head. Conclusions apply to the guard’s documented static-analysis scope and exemptions.
The PR remains draft; this is a review comment, not an approval.
juaristi22
left a comment
There was a problem hiding this comment.
Review status: COMPLETE — approved at 6f4d3313060987611bd00da46d3f392ffda1013e. No findings or material evidence gaps remain.
The head and description are unchanged from the completed review. I retained the verified 389 prior test passes (384 randomness-guard cases and five module-cache checks), together with the completed resolver/import-graph and author-feedback assessment; no code review or tests were repeated for the readiness change.
The PR is now ready for review. Fresh checks pass at this exact head, including the required Lint and Test / Result contexts. Historical benchmarks and the guard's documented limits retain their original qualifications.
Addresses #2874
axiom: n/a: code-health test only
Why
policyengine-core bans randomness in variable formulas, so the data build has to draw each stochastic input and store it (PolicyEngine/policyengine-core#500). Core's own check looks for numpy and stdlib randomness in formulas, but it misses the splitmix64 hash of entity ids in
policyengine_uk/utils/stochastic.pyand the seeded numpy draws outside formulas. This PR addspolicyengine_uk/tests/code_health/test_no_engine_time_randomness.py, which fails on any new draw and lists today's draws as a ratchet. Later PRs move each draw into the data build and lower the ratchet. This PR changes no model code and no output.Where the guard comes from
The file ports the scanner of PolicyEngine/policyengine-us#10447 at
221f2fe44ff265b8911810bcc440b3586fa6fc51(policyengine_us/tests/code_health/test_no_engine_time_randomness.py, git blob15b6826c0ebecbec84c6b51f128e5f76226e0e47). The scanner, from the constants down topackage_scan, and every generic detector test stay as they were. The port changes only:policyengine_ustopolicyengine_uk;RATCHET,RATCHET_CEILING,EXEMPTandOFFLINE_MODULES(empty: the scan finds no offline tool that needs one, sotest_model_never_imports_offline_modulespasses trivially until a module joins it);test_model_api_passes_on_cores_numpy_and_its_own_helpersnow checks a pe-uk variable module andadd_for_members, whichpolicyengine_uk/model_api.pypasses on;test_splitmix64_hash_and_its_uprating_caller_are_draw_helpersreplaces the pe-us start-month test; the offline-import tests check an example name of their own,policyengine_uk.tools.offline, since pe-uk has notoolspackage;package_scan's cache when the module's tests finish, astest_test_module_caches_released.pyrequires of every test module;pytest.skipcalls for cases that need newer syntax._on_this_pythonfilters those cases as pytest collects them, and two tests whose cases all need Python 3.12 sit underif sys.version_info >= (3, 12), so no Python collects a case it cannot parse and no run reports a skip. Every case stays.The lists
EXEMPTholds 21 digest occurrences in 9 files. Each computes a SHA-256 for provenance, cache keys, content checks or an equality check, and none decides who gets something or meets a threshold:build_metadata.pyget_data_build_fingerprintdata/capital_gains_projection.py_fingerprintdata/dataset_sources.py_cached_dataset_pathdata/economic_assumptions.pyuprating_input_fingerprintutils/aggregate_impact/__main__.py_sha256utils/aggregate_impact/dataset.pysha256_fileutils/aggregate_impact/measure.py_digest_tool_files,definitions_digest,model_fingerprint,runtime_identityutils/aggregate_impact/partition.pyload_partitionutils/build_lha_published_rates.pyfetcheach with its module's
hashlibimport.RATCHETholds 21 occurrences in 10 files, soRATCHET_CEILING = 21:utils/stochastic.py:splitmix64_uniform, the three splitmix64 constants and three xorshift steps;uc_deduction_random_draw.py,uc_deduction_type_random_draw.py,warm_home_discount_random_draw.pyandmonths_since_last_birthday.py: one formula call each tosplitmix64_uniform;data/economic_assumptions.py:uprate_student_loan_planshashesperson_idand compares the draw with the take-up rate;apply_single_year_uprating,apply_uprating,extend_single_year_datasetandSimulation._extend_dataset_with_cgt_provenanceinsimulation.pyreach it;dynamics/participation.py: a seeded numpyRandomStateinapply_participation_responses;scenarios/uc_reform.py: a seeded numpyGeneratorinadd_universal_credit_reform, and the module-levelScenariothat passes it on;scenarios/pip_reform.py:np.random.seedandnp.random.randominmodify_simulation, and the module-levelScenariothat passes it on.The scan over
mainat47a2817e2d57e6acbc08b76e09fde89fc27b81fareads 1,534 modules, cuts no lookup at the depth limit and finds no other draw pattern. WithoutEXEMPT, the scan reports 82 occurrences in 21 files: the 21 inRATCHET, the 21 digests, and 40 draw helper calls that reach the digests.Invariants
test_package_draws_nothing_outside_the_allowlists, including a second call in a scope the lists already name and a new function in a listed file.test_allowlist_entries_still_match, andtest_ratchet_only_shrinksfails whenRATCHETexceedsRATCHET_CEILING. SoRATCHETcan't grow unless a change also raisesRATCHET_CEILING, which the file says never to do.The guard reports recognised patterns, so it works as a ratchet, not a proof that no hand-written draw exists. It inherits every limit the docstring lists: it does not follow
evalorexec, imports of a computed name, values reached through objects or containers, or methods as draw helpers. For example,Simulationcallsuniversal_credit_july_2025_reform.simulation_modifier(self)through an object, which the scan does not follow. The guard does not scan policyengine-core, so it cannot show that nothing downstream reads numpy's global stream. It also looks for draws and hashes, not reads of the clock: four real income variables readinflation_adjustment, which prices to the yeardatetime.datetime.now()returns (#2875).Interaction with open work
The file says never to raise
RATCHET_CEILING, so none of these PRs can add aRATCHETentry. The guard fails each of them until the PR changes or, for #2862, adds anEXEMPTentry:_fixed_hashinvariables/household/wealth/household_wealth_decile.py, a copy of the splitmix64 mix that orders tied households and so decides which decile each falls in: threehash constant, threexorshift, and adraw helper callfrom the formula. It needs a tie-break without a hash, or a stored draw.utils/subsample.py, where_random_statereports ahash useand arandom sourceanddraw_household_countsasampling call, plus the calls that reach them, includingSimulation.subsample._core_fingerprintinutils/prepared_baseline.py, which identifies policyengine-core's code and decides no household's outcome. AnEXEMPTentry forutils/prepared_baseline.py,("<module>", "hash library"): 1and("_core_fingerprint", "hash use"): 1, clears all nine occurrences the guard reports.Impact
Impact: none; a code-health test only.
Tests
On commit
0835944512f8632f878168bbe1d2eceba951500d:uv run pytest policyengine_uk/tests/code_health/test_no_engine_time_randomness.py -n 4(Python 3.13.18, locked environment): 382 passed.--noconftest, and a plugin that makes_pytest.python.Package.setupa no-op so pytest doesn't importpolicyengine_uk/__init__.py: Python 3.11.18, 369 passed; 3.12.3, 380 passed; 3.13.18, 382 passed; 3.14.6, 382 passed. None skipped..github/actions/test-environmentinstalls it (Python 3.13.18): 382 passed; the scan reads the same 1,534 modules with the same counts.uv run pytest policyengine_uk/tests/code_health -n 4(Python 3.13.18): 3,544 passed.ruff format --check,ruff check,ruff check --select F811,F821 policyengine_ukandgit diff --checkpass.This head differs from that commit only in comments, the docstring, two test rows from the pe-us guard and the changelog fragment, and CI runs the whole guard on it.
🤖 Generated with Claude Code