Require fix pull requests to change a test and name their reproducer

[MaxGhenis]

Part of workstream W2 of the Autumn Budget CI plan (deliverable 4: a fix-PR inclusion check).

Why

The model keeps changing up to Budget day (28 October). Every fix that lands should carry a test that reproduces the bug, so a reviewer can run it and the bug can't come back silently. Nothing checks that today. The audit found merged fix #1915, whose old YAML test sat outside the collected test tree, so it never protected anything.

What

A Fix evidence check (.github/workflows/pr_fix_evidence.yaml, .github/check_fix_pr.py).

Which PRs are fix PRs: those with the bug or bug-fix label, or that add, modify or type-change a fragment directly in changelog.d/ with fixed in any dot-separated part of its name after the first, as towncrier reads it. Other PRs pass, with a one-line note.

A fix PR must:

1. Add, modify or type-change an executable test. That is a test_*.py or *_test.py module under policyengine_uk/tests/, or a YAML policy test (.yaml or .yml) under policyengine_uk/tests/policy/. Deleted files don't count.
2. Name its reproducer on a Reproducer: line in the description. Several ids can be separated by commas or semicolons, and backticks are optional. Lines inside HTML comments, such as the template's guidance, are ignored. The id must be one of:
   • A pytest node id in a changed file: file.py::test or file.py::TestClass::test, ignoring any [param] suffix. It must name a test pytest collects and runs:
     • a module-level function, or a method of a module-level Test* class;
     • the class derives only from object or unittest.TestCase (also as a bare TestCase whose last binding before the class is from unittest import TestCase) and has no __init__ or __new__;
     • no __test__ other than True, and no fixture;
     • no skip or xfail mark, or unittest skip or expectedFailure, on the test, its class or its module. skipif is allowed, because its condition is usually a platform check that holds false in CI. A test that skips itself at run time (pytest.skip) can't be seen statically; the docs say not to.
   • A changed YAML test, optionally file.yaml::case name. The named entry, or any entry for a bare file, has a string name and a non-empty output mapping.
   • n/a: <reason>, with a non-empty reason. It's accepted only when no file under policyengine_uk/ outside the tests is added, modified, type-changed or deleted, and neither pyproject.toml nor uv.lock changes.

Reporting: every failed requirement is reported, with the accepted formats, in the log and the job summary. The CLI exits 0 for pass, 1 for fail and 2 for unusable input.

Workflow safety: it runs on pull_request_target, so the checker is always the trusted base-branch copy. The job never executes, installs or imports PR code:

• It checks out the base branch with no ref and persist-credentials: false.
• The PR title, body and labels reach the script only through environment variables. They are written to a fresh directory under RUNNER_TEMP.
• It fetches the PR head as git objects and diffs it against the merge base with -z --no-renames.
• It runs the base checker with uv run --no-project --with pyyaml==6.0.2.
• It reads test files from git objects at the head SHA and rejects symlinks and gitlinks.
• YAML validation fails closed without PyYAML.

Merge queue: the check also runs on merge_group and passes there, so a branch rule can require it under a merge queue. Each PR's evidence was checked before it joined the queue.

Docs: the PR template gains a Reproducer section, and docs/engineering/skills/github-prs.md describes the rules.

Out of scope for now, per the plan: running the reproducer against the old code to show it failed before the fix.

Invariants

Tested in policyengine_uk/tests/code_health/test_check_fix_pr.py: 167 tests, about 7 s. The check reads tests statically, so it catches mistakes, not deliberate evasion; reviewers still read the test.

1. Exactly an added, modified or type-changed fixed fragment, or the bug/bug-fix label, classifies a PR as a fix (property).
2. Deleted tests never count as changed tests or reproducers (property).
3. Any list of ids, with any supported separators, spacing and backticks, round-trips through the Reproducer: parser (property).
4. A pytest reproducer is accepted exactly when pytest collects the named test from the changed file. This is checked differentially against real pytest collection, including falsy __test__, __new__, inherited constructors and name.__test__. Collected tests marked to skip or expected to fail are rejected, an intended difference.
5. n/a is accepted exactly when the reason is non-empty and no model source outside the tests or dependency file changed (property).
6. A non-fix PR always passes, whatever its body (property).
7. The CLI returns 0, 1 or 2 for pass, fail or unusable input, and appends to the job summary without replacing it.
8. NUL-delimited git diffs keep literal paths. Git-object reads check committed content without following symlinks or running PR code, and the standalone script follows the same rules.
9. The workflow runs the trusted base checker on pull_request_target, keeps its files under RUNNER_TEMP, keeps no token in the checkout, and reports on merge groups, with no path filter.
10. YAML reproducers need parsed list entries with a string name and a non-empty output mapping. A missing PyYAML fails closed.
11. Reproducer lines inside HTML comments never count as evidence.

Verified

• Over the repository's own tests, the checker accepts 1,008 of the 1,010 pytest tests, rejecting only the two strict xfails (Marriage Allowance is booked on the recipient as the transferor's unused allowance, not the fixed s.55B transfer #1947, Benefit cap, targeted childcare and local CTR use the wrong UC earned income figure #1986). It accepts all 340 YAML policy test files, whose 3,007 entries all have a non-empty output mapping.
• The new collection shapes match real pytest 8.4.1 collection, and the collection tests also pass under pytest 9.1.1.
• ruff and actionlint are clean.

Reviews

All in ~/reviews/budget-ci-w2/, by GPT-6.1 Sol:

• 2353d17: review-2325-2353d1710.md, REQUEST_CHANGES with 6 blocking findings, fixed in 9e378d9.
• 4ddff4c: review-2325-4ddff4c59.md, APPROVE with 4 non-blocking findings, closed in 11059ed.
• 11059ed: review-2325c-out.md, APPROVE with 3 non-blocking findings; 5 and 6 closed in 2a6fc1b (item 7, the conservative edge cases, fail closed and none occur in the repository).
• 2a6fc1b: Opus 5.5, review-2325d-2a6fc1b48.md, APPROVE; its non-blocking items 2, 5, 7 and 8 closed in 354a1a7 (a relative or shadowed TestCase, pytestmark arguments, a unittest class in the pytest oracle, docs).
• 354a1a7: Opus 5.5, review-2325e-354a1a7ed.md, APPROVE. Its finding 2 was a gap 354a1a7 opened (an aliased mark such as m.skip in pytestmark). 3de2f81 adopts its prototype fix for that and for the TestCase binding edges, with seven regression cases.
• 3de2f81: Opus 5.5 confirmation, review-2325f-3de2f81a0.md, APPROVE. The checker matches the prototype, the seven cases fail on 354a1a7 and pass here, and the sweep is unchanged. Its remaining notes are contrived edges (calls inside pytestmark, a local TestCase in a helper, match captures) that need deliberate effort, which the docs now exclude.

Effect on open PRs

pull_request_target always runs the base branch's workflow, so this check doesn't run on this PR, and it starts reporting only after merge. PRs already open get it on their next push, edit or label change. Open fix PRs without a Reproducer: line will then show it red. It is not a required check until the coordinating session adds it to the branch rules (decision d1368). Those PRs should be re-triggered first, or they'd sit at "Expected — waiting".

Reproducer: n/a: this PR adds a check, it does not fix a bug

axiom: n/a: CI tooling

🤖 Generated with Claude Code

[MaxGhenis]

Hand-off to the UK hub (W2, Autumn Budget CI plan)

• Head: 3de2f81.
• Reviews: all in ~/reviews/budget-ci-w2/.
  • GPT-6.1 Sol: review-2325-2353d1710.md requested changes, fixed in 9e378d9. review-2325-4ddff4c59.md and review-2325c-out.md (at 11059ed) both approve.
  • Opus 5.5: review-2325d-2a6fc1b48.md and review-2325e-354a1a7ed.md approve, and review-2325f-3de2f81a0.md confirms the current head.
• CI: all seven checks pass at 3de2f81.
• Depends on: nothing.
• After merge: the check runs on pull_request_target, so it can't run on this PR. It starts reporting only after merge, and PRs already open get it on their next push, edit or label change.
  • Re-trigger the open fix PRs before Fix evidence becomes a required check. It is one of the checks in d1368's list.
  • Open fix PRs without a Reproducer: line will show it red until they add one.

🤖 Generated with Claude Code

[MaxGhenis]

UK hub landing audit for #2325, head 3de2f81a01611b829a2f87fbce5d868fa884e082.

• Reviews:
  • GPT-6.1 Sol approved at 4ddff4c59 and 11059edbc, after requesting changes at 2353d1710, which were fixed.
  • Opus 5.5 approved later deltas and confirmed this head (~/reviews/budget-ci-w2/review-2325f-3de2f81a0.md).
• Gates:
  • gh pr checks exits 0, with all 7 checks passing, including tests/code_health/test_check_fix_pr.py.
  • MERGEABLE / CLEAN after Publish only the policyengine-uk release files whose checks passed #2306 merged. The only file it shares with Publish only the policyengine-uk release files whose checks passed #2306 is a docs page.
  • Merge commit, with --match-head-commit.
• After merge:
  • Fix evidence runs on pull_request_target, so it starts on open PRs at their next push, edit or label change.
  • It is not a required check. Making it one is part of d1368 (Max). Before then, re-trigger the open fix PRs, because ones without a Reproducer: line will show red until they add one.