Repository navigation
Compare a pull request with its merge commit's first parent in the Aggregate impact report - #2522
Conversation
For pull_request runs, the Aggregate impact workflow compared GitHub's merge commit with github.event.pull_request.base.sha. That SHA lags the base branch once the branch moves on, so the report counted main's newer merges as the pull request's impact (#2516's report took in 26 commits from main, #2518's 10). The head is now checked out with its parents (fetch-depth 2), and a new "Choose the base" step reads the merge commit's first parent from it, as pr_code_changes.yaml's Changes job already does. The step fails the job if the head does not have exactly two parents. A workflow_dispatch run passes its base through unchanged. The tests run the workflow's own step scripts on real git histories, with the head fetched at the workflow's fetch depth: the base is the first parent for any history (Hypothesis), a head without two parents fails, no step reads pull_request.base.sha, and a dispatch is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Hand-off to the UK hub (build job
|
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; components the model counts twice; 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 84 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
Components the model counts twice: 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
|
|
Merged by the UK hub (session local_8e19f9d5, Opus 5.5) on its merge gates. One merge-queue enqueue, pinned to head
🤖 Generated with Claude Code |
Defect
For
pull_requestruns,.github/workflows/aggregate-impact.yamlmeasured GitHub's merge commit ($GITHUB_SHA) againstgithub.event.pull_request.base.sha. Its comment said the merge commit's first parent is the base, butpull_request.base.shais not that parent. It lagsmainoncemainmoves on. The report then countedmain's newer merges as the pull request's own impact.685963c2d207aae39f(run 39364560222)815f89cb77e8682590f0732e38bb9b567144815f89cb7815f89cb7(Parents from
gh api repos/PolicyEngine/policyengine-uk/commits/<merge>. On the source of #2516's moves: its previous run, 39356846876, already mixed in #2457, #2458, #2460, #2464 and #2466 and flagged nothing. The 55 flags arrived with #2468, #2469, #2470, #2472 and #2475.)Fix
fetch-depth: 2).git -C head rev-list --parents -n 1 HEAD. This is the same approach aspr_code_changes.yaml's Changes job (fetch-depth: 2,base=HEAD^1), so there is no API call and no token.steps.base.outputs.sha).workflow_dispatch: unchanged. "Choose the commits" still sets its base from the input (defaultmain), and "Choose the base" passes it through.base_name: still the base ref (main). The report header already prints each side's checked-out SHA (git rev-parse HEADin each checkout), so it now shows the first parent with no change to the tool.pull_request.base.shaany more. The comments now say what the base is and why.Invariant: for every
pull_requestrun, base equals the merge commit's first parent, so the report is exactly what merging would change. Equivalently, no commit of the base branch is counted as the pull request's: the commits the comparison attributes (base..merge) are the pull request's own commits and the merge commit.Tests
policyengine_uk/tests/code_health/test_aggregate_impact_workflow.pynow runs the workflow's own step scripts withbash -eo pipefail. Actions runs these steps withbash -e {0}; neither script has a pipe, so the two behave the same here. They run on real git histories, with the head fetched by SHA at the depth that "Check out the head" sets.test_a_pull_request_is_compared_with_its_merge_commits_first_parent: main moves on after the PR forks. The base is main's tip, and only the PR's commit and the merge are attributed. Against the fork point (whatpull_request.base.shacan still name), main's two later merges were attributed too.test_no_commit_of_the_base_branch_counts_as_the_pull_requests(Hypothesis, 25 derandomized examples): any mix of main commits, PR commits and "Update branch" merges. The base is always the first parent, and the attributed commits are exactly the PR's own plus the merge.test_a_head_that_does_not_merge_two_commits_has_no_base[0, 1, 3]: a root commit, a plain commit and an octopus merge each fail the job with the error and set no base.test_the_events_base_sha_is_not_the_comparison_base: no${{ }}expression readspull_request.base.sha. The steps run in order (choose the commits, check out the head, choose the base, check out the base), and the base checkout usessteps.base.outputs.sha.test_a_pull_request_head_is_its_merge_commit, andtest_a_dispatch_compares_the_commits_it_namesfor both default and given inputs: dispatch outputs are as before.Results:
uvx ruff format --check .passes, andruff checkon the test module is clean.main's workflow, the reproducer fails (no step reads the first parent), and so does the expression test (it findsgithub.event.pull_request.base.sha).fetch-depth: 2: 4;pull_request.base.shaagain: 1;steps.commits.outputs.base: 1.Audit of past reports
Scope: every PR merged since 2026-10-18 23:00Z (#2335 onwards), and every open PR with an aggregate-impact comment. Every version of each report comment was checked, using GitHub's edit history, with the GitHub API only.
pull_requestrun of this workflow.maincommits as the PR's impact.815f89cb7, no model file)62a0117f9ahas nowar_widows_pension, so the base measurement crashed. The one "broken" check (sign:vat) fails on both sides in clean reportsEvery other PR with a report is clean.
Owed after this merges: a fresh run on #2518, #1913, #2516, #2189 and #1661, by a push or by merging
main. Then #2518's status line and #1913's sentence can rest on clean reports. This PR edits none of them.Impact
CI only.
fixedfragment calls for a patch release (.github/bump_version.py,infer_bump).pull_requestrun takes the workflow file from the merge commit. Report each pull request's effect on national aggregates on the certified dataset #2335's reports show this: it posted them for a week before the workflow reachedmain. With no model change, this PR's report should show no change.Reproducer: policyengine_uk/tests/code_health/test_aggregate_impact_workflow.py::test_a_pull_request_is_compared_with_its_merge_commits_first_parent
axiom: n/a: CI tooling
🤖 Generated with Claude Code