Repository navigation
Keep the pension salary-sacrifice cap within National Insurance - #2462
Conversation
bcdfe20 to
e8a06dc
Compare
|
Rebased onto main All 282 distinct targeted cases passed across the qualified bounded runs. Independent policy/source and code/test reviews completed with no findings or material gaps; the code reviewer passed 144 committed-snapshot tests. Formatting, lint, diff and Fix-evidence checks passed. The existing review request and |
vahid-ahmadi
left a comment
There was a problem hiding this comment.
Fine once the linked issue and the #2363 clash are sorted. The law change looks right to me.
@juaristi22, the core change matches the statute. The NICs (Employer Pensions Contributions) Act 2026 s.1 inserts SSCBA 1992 s.4(6A), which treats sacrifice above the cap as earnings for Part 1 (contributions) only. ITEPA 2003 s.308 still exempts the employer contribution from income tax. So the excess belongs in ni_class_1_income and nowhere else, which is what this PR does. The salary_sacrifice_pension_ni_employee and _employer measures are not added to any NI total, so the excess is not charged twice.
-
should: link #2173 as well (
changelog.d/2454.fixed.md, PR body). #2173 reports the same defect from the adjusted net income side: sacrifice above £2,000 raised ANI from 2029-30 and switched off 30 hours and Tax-Free Childcare. I ran its example on this head (£97,000 pay after a £10,000 sacrifice, one two-year-old). In 2029 ANI is now £96,844.80 (only the broad-base haircut),ni_class_1_incomeis £104,844.80, and both childcare income tests pass. Adding "Fixes #2173" would close it with this PR. -
should: this changes the meaning of an input that open #2363 relies on. #2363 rebuilds the two NI measures as Class 1 on earnings less
salary_sacrifice_returned_to_income. After this PR that variable defaults to zero, so #2363's measures would come out as zero unless it is moved ontosalary_sacrifice_pension_ni_excess. #2490 also edits both measure files. The PR body says the #2363 limitations remain, but the two PRs need a merge order and a follow-up edit on whichever lands second. -
should: keep the earnings base in step with #2464 (
policyengine_uk/utils/pensions.py:40).annual_allowance_net_incometakes relevant earnings asemployment_incomeplus positive self-employment income. #2464 addsemployment_benefits(SSP, SMP and so on) to the same base inpension_contributions_relief. If both merge as they stand, the AA net-pay ordering and the relief itself would use different earnings for someone on statutory pay. Whichever lands second should align the helper. -
nit: the microsimulation test module still describes the old method (
policyengine_uk/tests/microsimulation/test_salary_sacrifice_cap_reform.py:7"matching blog",:11"redirected excess").test_excess_redirected_to_pension(:151) andtest_full_excess_redirected(:214) now test NIC-only excess, so the names mislead. The blog method no longer matches the model: it redirected the excess into member contributions. -
nit:
salary_sacrifice_returned_to_incomekeeps a name that no longer says what it is. It is now an optional input for an actual return to pay. That is fine for compatibility. A line in the docstring saying the model never computes it would help readers who know the old behaviour.
No overlap with #1820, #1855, #1864, #2184, #2187, #2189, #2193, #2382, #2386 or #2433.
What I checked
- Read #2454 and #2173, the full PR body and the full diff, against main
62a0117f. The head contains main. - Read the NICs (Employer Pensions Contributions) Act 2026 s.1 on legislation.gov.uk. Checked the ITEPA 2003 s.308 reference.
- Followed every reader of
salary_sacrifice_returned_to_income,ni_class_1_incomeand the two NI measures, to check for double counting. - Hand-checked the AA YAML case. Pay of £175,000 less a 0.16% haircut gives £174,720. Threshold income is £174,720 + £30,000 = £204,720. Adjusted income is £174,720 + £130,000 = £304,720. The tapered allowance is £60,000 − £44,720/2 = £37,640, and the charge is (£130,000 − £37,640) × 45% = £41,562. All match.
policyengine-core testontests/policy/baseline/gov/hmrc/national_insuranceand.../pensions: 257 passed.- pytest on
test_salary_sacrifice_ni_only.pyandtest_salary_sacrifice_needs_pay_properties.py: 31 passed. - Restored main's
salary_sacrifice_returned_to_income.pyandni_class_1_income.py. The reproducertest_nic_cap_preserves_ordinary_tax_pay_member_payments_and_allowancesthen fails for 2029 and 2030, and 3 cases insalary_sacrifice_returned_to_income.yamlfail. ruff checkandruff format --checkon the 15 changed Python files: clean.- CI on this head is green, including Fix evidence. The changelog type is
fixed, and the Reproducer: and axiom: lines are present. - Not checked: the microsimulation revenue test (about £3.3bn). It needs private data, and I couldn't confirm that CI runs it.
🤖 Generated with Claude Code
e8a06dc to
3531d11
Compare
|
Addressed review 5563241802 on the stacked candidate
Local validation: 3,589 YAML passes; one full native Python run gives 6,542 passes, 95 skips and one expected failure. The original source audit rejects Darwin's glibc-only skip; a separate exact one-node local exception accepts the unchanged outcomes. No Linux/glibc assertion or private benchmark pass is claimed. Hosted new-head CI and human review remain separate; the stack is not merge-ready while its parent is open. |
vahid-ahmadi
left a comment
There was a problem hiding this comment.
@juaristi22 round 2, at 3531d110b. This is now stacked on #2464 (base fix-2371-statutory-pay-pension-relief), so I reviewed this PR's own diff against #2464's head 4d4787e65.
Verdict: items 1 and 3 to 5 are fixed. Item 2 (#2363) is still open, and nothing has run CI on this head, so I'm not approving yet. The law change was already right in round 1 and hasn't moved.
Round-1 points
- should: link #2173: fixed. The body has "Fixes #2173" and the changelog mentions it. The new case
NI-only sacrifice excess preserves both childcare income tests, 2173insalary_sacrifice_returned_to_income.yamlis the issue's own household. In 2029, ANI is £96,844.80, NI earnings £104,844.80, and both childcare income tests pass. - should: #2363 relies on
salary_sacrifice_returned_to_income: not fixed. The body now says the two need landing coordination, but there is no merge order or plan.git merge-treeagainst #2363's head385c547b5still conflicts in bothsalary_sacrifice_pension_ni_*.pyfiles and insalary_sacrifice_pension_ni_employer.yaml. If #2363 rebases onto this as it stands, its measures will come out as zero. #2363 is Max's draft, so I'd land this first and leave a comment on #2363 saying it needs to readsalary_sacrifice_pension_ni_excess. Once that comment is up and the order is in this PR's body, I'll treat this point as closed. - should: same earnings base as #2464: fixed.
annual_allowance_net_incomenow uses #2464'srelevant_uk_earningshelper, so it includesemployment_benefits. The supplied-ANI case with SSP and an actual return covers this. - nit: microsimulation test names and docstring: fixed. These are now
test_excess_included_in_ni_earningsandtest_full_excess_included_in_ni_earnings, and the module docstring describes the NI-only method. - nit:
salary_sacrifice_returned_to_incomedocstring: fixed. It now says this is an optional supplied input that the model never computes.
New findings
- should: no CI on this head.
gh pr checksreports no checks, because the workflows don't run for a PR whose base isn't main. Once #2464 merges, please retarget this to main so that CI and the aggregate-impact report run on the combined change before merge.
What I checked
- Read the three commits and both replies, and the diff against
4d4787e65. - Fresh worktree and uv venv at the head.
policyengine-core testontests/policy/baseline/gov/hmrc/pensionsand.../national_insurance: 271 passed. pytest ontest_salary_sacrifice_ni_only.pyandtest_salary_sacrifice_needs_pay_properties.py: 31 passed.ruff checkandruff format --checkon the 15 changed Python files: clean. git merge-treeagainst current main (clean), #2490 (clean) and #2363 (conflicts, as above).- Not checked: the private-data revenue test (about £3.3bn), and hosted CI, which hasn't run.
🤖 Generated with Claude Code
|
Documented the landing order #2464 → #2462 → #2363 and linked your existing #2363 coordination handoff, so no duplicate comment is needed. That handoff names |
Aggregate impact on certified data: passBase Checks: 0 broken by this change, 1 failing on both sides, 0 fixed. Moves flagged for review: 59 (thresholds in Notes
Moves flagged for review
19 more flags are in the report artifact. Receipts by tax head
Spending by benefit and service
Caseloads
Government balance, each component counted once
Model totals
Household income totals
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
Household net income by decileMean change in household net income per household, and as a share of the decile's base income; households stay in the base run's deciles.
No change in any year: components the model counts twice; population. 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
|
|
#2464 has merged, and #2462 now targets main. Integration commit Validation qualifies 116 unique Python cases: 46 independently recorded cases plus targeted runs of 47 and 23 cases, with named outcomes and JUnit evidence. The original six-file command timed out at 600 seconds and remains recorded as incomplete; it is not counted as a pass. The complete pensions and NI YAML directories passed 271 cases. Consumer acceptance/migration for #2363, #2490 API/owner/landing gates, #2386 documentation reconciliation, and existing Axiom follow-ups remain open. This does not certify those separate branches or parity. |
vahid-ahmadi
left a comment
There was a problem hiding this comment.
@juaristi22 round 3, at 893dcff6b. Since round 2 (3531d110b), #2464 has merged and this PR targets main through the integration merge.
Verdict: both round-2 points are fixed and CI is green. One should is left, on the axiom: line. I missed it in rounds 1 and 2, sorry. I'll approve once it is changed. The law change hasn't moved since round 1.
Round-2 points
- should: landing order with #2363: fixed. The body now gives #2464, then #2462, then #2363, and links my handoff comment on #2363, which names
salary_sacrifice_pension_ni_excessas the input #2363 should read. Nothing more is needed from this PR.git merge-treeagainst #2363's head385c547b5still conflicts in bothsalary_sacrifice_pension_ni_*.pyfiles andsalary_sacrifice_pension_ni_employer.yaml, as expected. That work belongs to #2363. - should: no CI on the stacked head: fixed. All 16 checks pass on
893dcff6b, including Test / Python 1/4 to 4/4, Test / Result, Test / YAML, Fix evidence, Lint, the docs builds and Aggregate impact. That covers the full Python suite, so the six-file local command that timed out no longer matters.
Round-1 items 1 and 3 to 5 were already fixed in round 2 and are unchanged.
The integration keeps #2464
893dcff6bhas parents3531d110b(the round-2 head) andb9b567144(main after #2464's merge73d8889aa).- The PR's own diff in round 2 (
4d4787e65..3531d110b) and its diff againstb9b567144cover the same 22 files, so the merge dropped none of them. Each of them has the same blob as at3531d110b. In each of them, main's blob equals #2464's stacked head4d4787e65. So main had not changed any of these files after #2464, and taking this PR's side dropped nothing. - #2464's other files (
pension_contributions_relief.py,statutory_pay_pension_relief.yaml, the README) are at main's version. Its two files that this PR also edits (utils/pensions.py,pension_annual_allowance_threshold_income.py) still use the sharedrelevant_uk_earningshelper andpension_contributions_relief. statutory_pay_pension_relief.yamlpasses (it is in the pensions run below).
New findings
- should: PR body:
axiom: needed. The template anddocs/engineering/skills/github-prs.mdkeepneededfor external contributors who can't do the Axiom work, so that a maintainer can follow up. On a maintainer's PR it leaves the parity step with no owner. Please use one of the standard options. rulespec-ukmainhas no module for SSCBA 1992 s.4 or the 2026 Act. It hasuk/statutes/ukpga/1992/4/for other sections, and the Class 1 pilot pipelines underuk/statutes/social_security/workers/don't mention salary sacrifice. I found no open issue there for the cap. Soencoded-correctisn't open to you. That leaves<rulespec-uk PR> encoded, or<rulespec-uk issue> queuedwith the blocker recorded and thepe-paritylabel, as CONTRIBUTING.md asks. For companion cases, use values from the Act rather than ones the model computes: the £2,000 cap, and a plain case such as a £10,000 sacrifice giving £8,000 of Class 1 earnings. The #2173 household's figures include the model's 0.16% haircut, so they don't fit. - nit:
policyengine_uk/data/fiscal_partition.yaml:484. The note onsalary_sacrifice_pension_ni_employeestill says "The excess sacrifice is added back to employment_income, so Class 1 already charges it." After this PR it is added toni_class_1_incomeonly. Themeasure_withinclassification is still right, so only the note's wording is out of date.
Overlaps
- Current main
47642d1ee: none of the 10 PRs merged sinceb9b567144touches the 22 files, andgit merge-treeis clean. - My draft #2505 (employer NI zero rate for under-21s and apprentices): it changes
ni_class_1_employer.py, not the salary-sacrifice measure files, and merges cleanly with this PR. On the combined tree the NI YAML directory passes (152) and so dotest_ni_class_1_age_properties.pyandtest_salary_sacrifice_ni_only.py(30). The actual employer liability handles the excess correctly. For a 19-year-old on £40,000 with a £6,000 sacrifice in 2029,ni_class_1_incomeincludes the £4,000 excess andni_class_1_employeris £0. The flat-ratesalary_sacrifice_pension_ni_employermeasure still reports £600. That measure was already a flat-rate approximation before this PR, and #2363 replaces it with the change in Class 1 liability. So no change is needed here, but whichever of #2505 and #2363 lands second should check that case. - #2490: clean on the files here. Its only conflict with this PR is in
carers_allowance_earnings.py, which this PR doesn't touch. #2386: no shared files.
What I checked
- Read the integration commit, your 19 and 20 Oct comments and the updated body, against round 2 at
3531d110b. - Fresh worktree and uv venv at
893dcff6b.policyengine-core testontests/policy/baseline/gov/hmrc/pensionsand.../national_insurance: 271 passed. pytest ontest_salary_sacrifice_ni_only.pyandtest_salary_sacrifice_needs_pay_properties.py: 31 passed. - Aggregate impact on certified data: 0 checks broken. Market income falls by about £14.5bn a year from 2029-30, because main used to put the excess into
employment_income. NI receipts barely move, and income tax falls by £0.3m. That is what the NI-only cap should do. - Not checked: the private-data revenue test.
🤖 Generated with Claude Code
|
Created the owned, |
vahid-ahmadi
left a comment
There was a problem hiding this comment.
@juaristi22 round 4, at 3c7f6bcb7.
Approving. Both round-3 points are fixed and CI is green. Two optional nits on rulespec-uk#477 below.
Round-3 points
-
should,
axiom:line: fixed. The body now readsaxiom: TheAxiomFoundation/rulespec-uk#477 queued. Fix evidence ran after your last body edit (09:03:44Z) and passed at3c7f6bcb7. #477 is open, labelledpe-parity, and is ready to dispatch:- Module paths and corpus: proposed paths for ss.1 to 3 of the 2026 Act and for s.4 of both 1992 Acts, with the pinned corpus and its hash.
- Law verbatim: I fetched the enacted 2026 Act from legislation.gov.uk. The XML hash matches the one in the issue. A word-level diff of your ss.1, 2 and 3 quotes against it differs only in one space before a comma in each of ss.1 and 2. That includes the NI text's "in respect of benefit", which is how the source reads.
- Outputs and companion cases: the £2,000 limit, jurisdiction, effective tax year and added Class 1 earnings. The five cases (£0, £1,999.99, £2,000, £2,000.01, £10,000) are worked out from the Act's figures, not the model's, and match the cap in the PE parameter.
- Blocker: recorded as no approved billed signed-encoder run, the same pattern as the other queued issues.
The conditional companion cases are fine. New s.4(6A) only says regulations may treat the excess as earnings. Section 1(4) fixes £2,000 for the first regulations, and s.4(6B) requires amounts up to that limit to stay untreated. I found no regulations under the Act on legislation.gov.uk (made or draft). So no case can be unconditional until regulations exist. PolicyEngine's 2029 baseline assumes the same thing. Saying so in the issue, with s.4(6C) and (6D) kept as unencoded boundaries, is the honest framing.
-
nit,
fiscal_partition.yamlnote: fixed.3c7f6bcb7has a single parent,893dcff6b, and changes only line 484, fromemployment_incometoni_class_1_income. The classification and every other file are unchanged.
New nits (optional)
- nit, #477 opening line. It still says "Proposed label:
pe-parity. This is a nonexecuting local draft; no issue has been created." That was true of the draft, but now it's wrong. It also names an owner by handle, which the other queued issues avoid. A few spaces were also lost in the prose, though not in the quotes: "or2026Act", "Class1", "5April2029", "up to£1". Worth a quick edit so the encoder isn't confused. - nit, later and outside this PR.
salary_sacrifice_pension_cap.yamlstill cites only the GOV.UK policy paper and Budget 2025. Now that the Act is law, a later PR could addukpga/2026/15/section/1as a reference.
What I checked
- CI at
3c7f6bcb7: all 16 checks pass, including Test / Python 1/4 to 4/4, Test / Result, Test / YAML, Fix evidence and Aggregate impact. - Merges cleanly with current main (
42c01db91), and nothing merged since round 3 touches the PR's 23 files. Round 3's notes on #2363 and #2505 still stand.
🤖 Generated with Claude Code
Fixes #2454
Fixes #2173
Apply the modeled pension salary-sacrifice cap only to Class 1 NI earnings. Ordinary employment income and member pension payments continue to represent actual payments.
salary_sacrifice_pension_ni_excessmeasures the NI-only excess;salary_sacrifice_returned_to_incomeremains a supplied actual cash/member-payment input, with default zero and no computed excess formula.This PR was originally stacked on #2464 at
4d4787e654e7e3bc0a48500fb2c8beb8c78ea306. #2464 merged as73d8889aae11099bd00299eace658a1916b10480. The PR now targetsmain; integration commit893dcff6bb5babdca2794aea71fdcbbbfc27163dcombines the previous PR head with captured mainb9b567144d45add85bc58ad7908dddd12e0be14d. Its 22 feature-file blobs and modes remain unchanged, and all other files match that captured main. It retains the parent's common relevant-earnings helper, coherent pension-relief/annual-allowance callers and all 12 statutory-pay fixtures, including supplied-ANI, net-pay/RAS, trading-loss, age and direct-input behavior. The two private-test names/docstrings now describe NI excess accurately; their executable assertions, fixtures and numbers are unchanged.The nine return-input policy cases comprise seven retained cases and two additions: a natural #2173 childcare-income regression and a separately qualified supplied-ANI/SSP/actual-return control. In the natural 2029 case, £97,000 pay before the existing labour-supply response and £10,000 employer sacrifice give ordinary pay/ANI £96,844.80, NI-only excess £8,000 and NI earnings £104,844.80. Both childcare income tests remain satisfied without forcing intermediate eligibility or income variables. This checks the income conditions, not complete childcare awards.
The 2026 Act and Treasury guidance establish the NI-only cap; the employer-contribution exemption remains under ITEPA 2003 s308.
Reproducer:
policyengine_uk/tests/policy/baseline/gov/hmrc/pensions/salary_sacrifice_returned_to_income.yaml::NI-only sacrifice excess preserves both childcare income tests, 2173Historical validation at
3531d110b5a821b32f10087e02720af1757cdf81:main. Fresh required and applicable hosted checks, including documentation and aggregate-impact checks, remain necessary for the integrated head.Current integration validation at
893dcff6bb5babdca2794aea71fdcbbbfc27163dqualifies 116 unique Python cases: 46 from the independent CODE run and 70 from two separately recorded targeted runs of 47 and 23 cases. Named call outcomes and JUnit results cover the exact collected set with no failed or skipped cases. The original six-file command timed out at 600 seconds and is retained as an incomplete run; it is not counted as a pass. The complete pensions and National Insurance YAML directories passed 271 cases. Formatting, lint, Towncrier, diff and trusted actual-base Fix-evidence checks passed on the unchanged captured source/runtime. These local results do not replace required and applicable hosted CI on the combined head.Private population benchmarks were not executed. Annual-age, cash-proxy and future payroll-interval limitations remain. Parent #2464 has merged; its issue #2371 remains partial and SPP/#2368 remains open. The current-main integration preserves the previously reviewed feature files and requires fresh combined-head hosted CI. It does not certify separate consumer branches or their landing.
Own #2490 and external #2363 require separate NI-excess-input/curve landing coordination; external #2386 requires separate pension-doc reconciliation. This PR does not repair those branches or certify their combined compatibility. Existing #2493/#2452 and Axiom follow-ups remain.
Landing coordination
Landing and consumer plan: #2464, then #2462, then #2363. #2464 has merged and #2462 now targets main with the captured current-main integration described above. Required hosted CI and aggregate-impact checks on the combined head remain outstanding before #2462 merges. #2363 then needs to read
salary_sacrifice_pension_ni_excessfor its NI-excess measures while preservingsalary_sacrifice_returned_to_incomeas an actual-pay/member-payment input. Vahid's existing coordination handoff records that consumer change and proposed order. The draft author's acceptance and migration remain outstanding; this handoff does not certify the combined tree.axiom: TheAxiomFoundation/rulespec-uk#477 queued
Owned parity handoff: @juaristi22. The signed encoder is blocked because its separately billed run has not been approved. The underlying encoding/parity follow-up remains needed and OPEN; this queued tracking reference does not certify encoding or parity.