Skip to content

Make Hypothesis draw the same examples from the source tree and the wheel - #2400

Merged
MaxGhenis merged 1 commit into
mainfrom
fix-benefit-cap-earnings-statutory-pay
Oct 22, 2026
Merged

MaxGhenis merged 1 commit into
mainfrom
fix-benefit-cap-earnings-statutory-pay

Conversation

@anth-volk

@anth-volk anth-volk commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Follow-up to #2397, rebased onto main after #2399.

What and why

#2399 fixed the benefit cap earnings exemption and unblocked releases. This PR is now only the test-environment half of #2397: why pull request checks passed a commit that Release test failed.

Hypothesis 6.169 biases its draws with literal constants harvested from local modules (hypothesis/internal/conjecture/providers.py: _get_local_constants), and never counts a path under /site-packages/ as local (constants_ast.py: is_local_module_file). From the source tree it collects 37 integers, 51 floats and 1,301 strings from policyengine_uk, including thresholds such as 10_152; from the installed wheel, none. With the same derandomize seed, the two environments generate different examples, so a property-test failure can show up only at release. Main still has this gap: #2399 pinned the one family that failed, but nothing stops the same divergence on any other property test.

  • policyengine_uk/tests/conftest.py: pytest_configure now empties Hypothesis's local-constant pool (_ignore_hypothesis_local_constants), and raises if the private function it replaces is gone.
  • policyengine_uk/tests/test_hypothesis_environment.py: reads the pool through HypothesisProvider._local_constants, as Hypothesis does, and fails if a Hypothesis upgrade brings the local constants back.

The earlier version of this branch also carried the model fix, the @example pin, the #1986 xfail removal and benefit_cap_earnings.yaml; all of that is dropped, as #2399 covers it.

Trade-off (from review): with no local constants, Hypothesis no longer aims draws at the model's own thresholds, which is what found this bug. That can be added back deliberately later, for example with @example cases each side of the cap earnings threshold.

Test plan

  • Locally, source tree (uv sync --frozen --extra dev, Python 3.13) at 5d670bd: the guard test, test_uc_statutory_pay_properties.py and test_uc_income_from_capital_properties.py (15 passed).
  • The guard test fails on main without the conftest change, and passes with it.
  • ruff format --check clean on the changed files.
  • Changelog fragment in changelog.d/
  • Full suite in CI.

Reproducer

Reproducer: policyengine_uk/tests/test_hypothesis_environment.py::test_hypothesis_draws_no_constants_from_local_modules

axiom: n/a: test configuration only; no policy change.

🤖 Generated with Claude Code

@anth-volk
anth-volk marked this pull request as ready for review October 13, 2026 17:20

@vahid-ahmadi vahid-ahmadi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine once rebased down to the Hypothesis fix. Main already has the rest.

@anth-volk, #2399 merged to main on 14 Oct and shipped in 2.130.1. It makes the same change to is_benefit_cap_exempt_earnings.py (summing uc_individual_earned_income_before_mif), removes the #1986 strict xfail in test_uc_income_from_capital_properties.py, and pins the sick-pay family with @example in test_uc_statutory_pay_properties.py. That is why this branch now conflicts.

  1. should: rebase down to the part main doesn't have: _ignore_hypothesis_local_constants in policyengine_uk/tests/conftest.py, the guard test policyengine_uk/tests/test_hypothesis_environment.py, and the second line of the changelog fragment. On main, conftest.py has no Hypothesis change and the guard test doesn't exist, so source-tree checks and Release test can still draw different examples. That was the cause of #2397. #2399 closed the issue, but the cause is still on main. benefit_cap_earnings.yaml can go: #2399 adds benefit_cap_earnings_exception_earned_income.yaml, which covers statutory sick, maternity and paternity pay, tax on a private pension and the MIF.
  2. nit, non-blocking: emptying the local-constant pool makes the two environments agree, but it also stops Hypothesis biasing draws towards the model's own thresholds (such as 10_152), and that bias is what found this bug. A later change could add those thresholds back on purpose, for example @example cases each side of the cap earnings threshold.

What I checked

  • The PR head is fd9a0eee9, with no rebase since 13 Oct. GitHub reports it as conflicting with main.
  • On origin/main: is_benefit_cap_exempt_earnings.py uses uc_individual_earned_income_before_mif, the #1986 xfail is gone, and test_uc_statutory_pay_properties.py has the @example pin. conftest.py has no local-constants change, and test_hypothesis_environment.py doesn't exist. Hypothesis is still locked at 6.169.0.
  • I did not rerun the tests on this head; this note is about scope only.

🤖 Generated with Claude Code

Rebased onto main after #2399, which made the benefit cap earnings fix.
Only the test-environment change remains: conftest stops Hypothesis
drawing literal constants from policyengine_uk's own modules, and a
guard test fails if a Hypothesis upgrade bypasses that.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@anth-volk
anth-volk force-pushed the fix-benefit-cap-earnings-statutory-pay branch from fd9a0ee to 5d670bd Compare October 15, 2026 20:37
@anth-volk anth-volk changed the title Test the benefit cap earnings exemption on UC earned income Make Hypothesis draw the same examples from the source tree and the wheel Oct 15, 2026

@juaristi22 juaristi22 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 5d670bd778927e68c8e9bfc6651510253de03ea9. Approved the narrowed test-environment fix: the diff contains the Hypothesis local-constant hook, a provider-level guard and changelog; the earlier benefit-cap changes are covered by #2399.

The provider reads the replaced getter and its cache is cleared before property tests run. Independent controls verify that the exact guard fails without the hook (1,506 source literals), passes with it, stays empty after later imports, and raises clearly if the private getter disappears. Source/site-packages classification and the CI source/wheel workflow were checked. The documented loss of automatic threshold-literal bias is an explicit trade-off; the relevant tests retain deliberate boundary samples and examples.

Validation: 15 focused tests and 11 controls passed. Local execution used Hypothesis 6.168.2/Core 3.32.27; the lock's Hypothesis 6.169.0 is covered by exact-head hosted CI. A built-wheel execution was not repeated locally. Reproducer and Axiom n/a records are appropriate. No findings or material evidence gaps.

Review status: COMPLETE. Review severity: APPROVE.

@vahid-ahmadi vahid-ahmadi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anth-volk round 2, at 5d670bd77. I looked at what changed since fd9a0eee9, which here means the whole narrowed diff.

Verdict: the should is fixed. Approving.

Round-1 points

  1. Rebase down to the Hypothesis fix: fixed. Against main the branch is now three files and 45 lines: _ignore_hypothesis_local_constants in tests/conftest.py, the guard tests/test_hypothesis_environment.py, and a one-line changelog fragment. The model change, the @example pin, the #1986 xfail removal and benefit_cap_earnings.yaml are all gone, as #2399 covers them. It now merges cleanly with current main (8b12bce94).
  2. nit, loss of threshold bias: recorded. The description now sets this out as the trade-off, with deliberate @example boundary cases as the way back. That's fine.

New findings

None. One cosmetic point: the fragment is still named fix-benefit-cap-earnings-statutory-pay.fixed.md. Only its text reaches the changelog, so it can stay.

What I checked

  • Built a merge of this head into current main and ran it in a fresh uv venv, with Hypothesis 6.169.0 as locked on main.
  • The guard plus test_uc_statutory_pay_properties.py and test_uc_income_from_capital_properties.py: 15 passed.
  • Control: with the pytest_configure call removed, the guard fails with 1,520 local constants on current main.
  • CI at this head ran against main as it was on 15 Oct, and main is now 266 commits further on. The hook changes the examples every property test draws, so I ran all 80 test_*propert*.py files on the merge: 573 passed, 1 xfailed.
  • No other open PR touches tests/conftest.py.
  • CI is green at this head.
  • Not checked: I didn't build a wheel and compare its draws side by side with the source tree. I'm relying on Hypothesis never counting site-packages as local, and on the guard showing the source-tree pool is empty.

🤖 Generated with Claude Code

@MaxGhenis
MaxGhenis added this pull request to the merge queue Oct 22, 2026
Merged via the queue into main with commit 9e040fa Oct 22, 2026
14 of 15 checks passed
@MaxGhenis
MaxGhenis deleted the fix-benefit-cap-earnings-statutory-pay branch October 22, 2026 18:26
@MaxGhenis

Copy link
Copy Markdown
Collaborator

Merged by the UK hub (Claude session 8e19f9d5) at head 5d670bd77, through the merge queue with the head pinned.

  • Review: @juaristi22 (16 Oct) and @vahid-ahmadi (19 Oct) approved at this exact head.
  • CI: every check on gh pr checks passed at this head.
  • Why now: Max asked (22 Oct) to clear the policyengine-uk PR backlog before the Autumn Budget on 28 October, without waiting on each other.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants