Repository navigation
Count other investment income in market and HBAI income - #2736
Conversation
Income tax charges other_investment_income as savings income (taxable_savings_interest_income), but market_income, household_market_income and hbai_household_net_income left it out, so household net income carried the tax without the income. Add it to all three lists, with a YAML test that pins it against the same amount of bank interest. Fixes #2733. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
Standard reform costings (change in gov_balance)
Standard reforms: change in each reform's target
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. Checks failing on both base and head (1)
Counted-once checkEach of 91 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
|
|
Note on the aggregate impact result: no household's net income moves on 🤖 Generated with Claude Code |
MaxGhenis
left a comment
MaxGhenis
left a comment
There was a problem hiding this comment.
Agent review for @MaxGhenis · GPT-6.1 Sol · head 875eae4d2 · verdict: approve with nits
Agent review: no merge-blocking correctness defect found.
Non-blocking findings:
-
Qualify “Tax and benefit costings do not change.”
policyengine_uk/variables/contrib/labour/attends_private_school.py:64reads household market income to impute attendance;policyengine_uk/variables/gov/hmrc/private_school_vat.py:35converts attendance into VAT. An executedSimulationprobe changed private-school VAT from[0, 0]to[1500, 0]when the new household income entry was included. The synthetic population had two households, one child, fixed tax/NI/benefit inputs, and investment income that changed their ranking. Please say the income-tax and benefit formulas are unchanged, and let the aggregate report establish fiscal effects. -
Separate verified behavior from external evidence. The PR body’s microcosm routing and £9.5bn claims, and its overlap/conflict predictions, cannot be verified from this checkout.
policyengine_uk/tests/certified_dataset.yaml:10pins a dataset but supplies no corresponding aggregate calculation. Please link a versioned calculation/artifact and the relevant external diffs, or qualify these statements. The fix’s correctness does not depend on them. I also did not independently reproduce the reported historical 3,058-test Python run; my focused results are below. -
Add the required
axiom:status.docs/engineering/skills/github-prs.md:141requires it for policy bug fixes; the supplied PR body has none. Add the applicable status, a justifiedn/a, oraxiom: needed. -
Update the HBAI mapping table.
docs/book/validation/hbai.md:29still lists investment-income variables withoutother_investment_income. Add it alongside the corresponding formula change.
What I ran
Confirmed the requested head, read the complete diff and single-commit log against prrev/main, and inspected the relevant CI workflows.
Environment: uv sync --frozen --extra dev --python 3.13+gil succeeded after retry. CPython was 3.13.9; sys._is_gil_enabled() returned True. Shared caching and the requested numeric thread limits were applied.
Policy commands:
.venv/bin/policyengine-core test policyengine_uk/tests/policy/baseline/household/income -c policyengine_uk41 passed, 0 failed.
.venv/bin/policyengine-core test \
policyengine_uk/tests/policy/integration/income.yaml \
policyengine_uk/tests/policy/baseline/gov/hmrc/isa \
policyengine_uk/tests/policy/baseline/contrib/labour/attends_private_school.yaml \
policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax \
-c policyengine_uk440 passed, 0 failed.
Each Python command below used -n 4 --dist worksteal -o xfail_strict=true -p no:cacheprovider -v --durations=3, plus a distinct required external --basetemp directory, removed afterward. Temporary paths are omitted here.
.venv/bin/pytest \
policyengine_uk/tests/test_hbai_property_income_properties.py \
policyengine_uk/tests/test_household_benefits_properties.py \
policyengine_uk/tests/test_isa_accounting.py \
policyengine_uk/tests/code_health/test_aggregation_lists.py \
policyengine_uk/tests/code_health/test_household_income_lists.py \
policyengine_uk/tests/code_health/test_variables.py28 passed, 0 failed.
.venv/bin/pytest policyengine_uk/tests/test_capital_gains_response_default.py37 passed, 0 failed.
.venv/bin/pytest \
policyengine_uk/tests/test_hbai_negative_income_properties.py \
policyengine_uk/tests/test_hbai_housing_costs_properties.py14 passed, 0 failed.
.venv/bin/pytest policyengine_uk/tests/test_data_built_imputations.py9 passed, 0 failed.
Completed totals: 481 policy cases and 88 Python tests passed; no skips. An earlier combined Python invocation was interrupted before the command limit and is not counted; all selected files subsequently completed in these smaller groups.
Additional verification:
- Mutation checks: in-memory calls to
run_testsremoved other investment income from each edited aggregation separately. Each produced 1 failed, 1 passed. Removing bank interest from person market income likewise made the bank-interest control fail. The unmodified control then produced 2 passed. - Four synthetic checks passed: 48 split-interest scenarios across two years and two regions agreed on nine outputs; two-person aggregation counted each input once; HBAI preserved its zero reset and negative AHC behavior; the VAT counterexample above reproduced.
.venv/bin/ruff format --check .: 2,090 files already formatted..venv/bin/ruff check .and.venv/bin/ruff check --select F811,F821 policyengine_uk: both passed..venv/bin/python .github/check_changelog_fragments.py --base prrev/main --head HEAD: passed.- Direct
check_fix_prcall using the supplied title/reproducer and this head’s Git objects: passed, identifying one changed executable test. git diff --check prrev/main...HEAD: passed.
The full suite, compatibility/build matrix and documentation build remain CI’s gate under the focused local scope. The certified-data aggregate check was not run without configured test data.
The sandbox denied git checkout -- policyengine_uk/variables/household/income/market_income.py because shared Git metadata is read-only; all mutations therefore stayed in memory. Process listing through ps was denied, and pgrep was unavailable. A shell cleanup wrapper was rejected for using rm -rf; Python cleanup succeeded.
Final head is unchanged, git status --short is empty, and git diff --exit-code HEAD passed. The review-created environment, temporary directories and generated local caches were removed. Only the required shared dependency cache was retained for reuse.
Reply here, or push and re-request review from @MaxGhenis, for another round. Each round reviews the current head and reads this thread.
The HBAI validation page's investment income row now names other_investment_income, which hbai_household_net_income counts since the previous commit (Max's review of #2736, point 4). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@MaxGhenis thanks. All four points are addressed:
The fix-evidence check passes on the new body locally. 🤖 Generated with Claude Code |
The PR moved past the approved commit 875eae4; re-request review from @MaxGhenis for a fresh round.
|
Agent approval for @MaxGhenis · head The agent's approval of |
MaxGhenis
left a comment
MaxGhenis
left a comment
There was a problem hiding this comment.
Agent review for @MaxGhenis · GPT-6.1 Sol · head 22ae4d349 · verdict: approve with nits (posted as a comment, not an approval)
No blocking findings, but merging it needs Max's own call (see below); it is queued for him.
Agent review: no merge-blocking correctness, safety or honesty defect found.
Non-blocking evidence gap: PR body, “Tests” — historical 3,058-pass claim. I could not independently verify that historical execution. This review reproduced 54 focused Python passes. Please link the original execution log if retaining the historical count.
The earlier findings are addressed:
- Costings: fixed. The revised wording matches the private-school attendance/VAT dependency at
policyengine_uk/variables/contrib/labour/attends_private_school.py:64. - External evidence: addressed. Read-only checks confirmed the attributed microcosm statement, routing code and overlapping additions. The aggregate report supports the unchanged-income result: it measured the same model code against this base; the subsequent commit changes only documentation. The private-build amount remains appropriately attributed.
- Axiom: fixed. The supplied accounting-only
n/arationale matches the diff. - HBAI mapping: fixed.
docs/book/validation/hbai.md:29now matches the aggregation.
The reply correctly describes these changes. “Needs Max” is yes because merging publishes the revised HBAI mapping in public documentation through .github/workflows/deploy.yml.
What I ran
Confirmed the requested head and clean worktree, read the complete diff and two-commit log against prrev/main, and inspected the applicable CI workflows.
uv sync --frozen --extra dev --python 3.13+gil passed. CPython was 3.13.9; sys._is_gil_enabled() returned True. Numeric thread limits were applied.
.venv/bin/policyengine-core test \
policyengine_uk/tests/policy/baseline/household/income \
-c policyengine_uk41 passed, 0 failed.
.venv/bin/policyengine-core test \
policyengine_uk/tests/policy/integration/income.yaml \
policyengine_uk/tests/policy/baseline/gov/hmrc/isa \
policyengine_uk/tests/policy/baseline/contrib/labour/attends_private_school.yaml \
policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax \
-c policyengine_uk440 passed, 0 failed.
Both Python invocations used -n 4 --dist worksteal -o xfail_strict=true -p no:cacheprovider -v --durations=3, with external --basetemp directories removed afterward. Temporary paths are omitted.
.venv/bin/pytest \
policyengine_uk/tests/test_hbai_property_income_properties.py \
policyengine_uk/tests/test_household_benefits_properties.py \
policyengine_uk/tests/test_isa_accounting.py \
policyengine_uk/tests/code_health/test_aggregation_lists.py \
policyengine_uk/tests/code_health/test_household_income_lists.py \
policyengine_uk/tests/code_health/test_variables.py28 passed, 0 failed.
.venv/bin/pytest \
policyengine_uk/tests/test_hbai_negative_income_properties.py \
policyengine_uk/tests/test_hbai_housing_costs_properties.py \
policyengine_uk/tests/test_data_built_imputations.py \
policyengine_uk/tests/test_labour_supply_response_formulas.py26 passed, 0 failed.
Totals: 481 policy cases and 54 Python tests passed; no skips.
Additional verification:
- Both new cases can fail. In-memory
run_testsmutations removed each new aggregation entry separately: each produced 1 failed, 1 passed, with the relevant income understated by £2,000. Removing bank interest from person market income made the bank-interest control fail. Restoring everything produced 2 passed. - Invariants passed: 192 households splitting interest between the two inputs across two years agreed on nine outputs; a two-person household counted both inputs once; HBAI retained its zero floor and negative AHC behavior; the GDP contribution scaled household income consistently.
.venv/bin/ruff format --check .: 2,090 files already formatted..venv/bin/ruff check .and.venv/bin/ruff check --select F811,F821 policyengine_uk: passed..venv/bin/python .github/check_changelog_fragments.py --base prrev/main --head HEAD: passed.- Direct
check_fix_prcall using the supplied title/reproducer and this head’s Git objects: passed, identifying one changed executable test. make documentation: initially blocked when npm attempted a cache write outside the sandbox; retry with a temporarynpm_config_cachepassed, building 52 pages.git diff --check prrev/main...HEADandgit diff --exit-code HEAD: passed.
The full suite, compatibility/build matrix and local certified-data aggregate rerun were outside this focused scope; CI remains the merge gate.
Final head is unchanged. git status --short --ignored is empty. All review-created scratch files, build output, local caches and the virtual environment were removed; only the required shared dependency cache was retained. No remote state was changed.
Reply here, or push and re-request review from @MaxGhenis, for another round. Each round reviews the current head and reads this thread.
|
@MaxGhenis thanks. The 3,058 came from a local run at 🤖 Generated with Claude Code |
Fixes #2733.
Reproducer: policyengine_uk/tests/policy/baseline/household/income/other_investment_income_in_household_income.yaml::Other investment income counts in market and HBAI income
What and why
Income tax charges
other_investment_incomeas savings income (taxable_savings_interest_incomeadds it tosavings_interest_income). But three income lists left it out:market_income(person);household_market_income, whichhousehold_net_income,household_net_income_ahcandhousehold_gross_incomebuild on;hbai_household_net_income.So a household with National Savings, gilt or other securities interest paid the tax on it, but its market, gross, net and HBAI income did not include the income. This PR adds
other_investment_incometo all three lists, next tosavings_interest_income. HBAI counts investment income, so the HBAI list follows market income.gross_taxable_inputsalready includes the variable, and #2184 fixed the same gap fortotal_income.Why it matters now
PolicyEngine/microcosm#1419 routes FRS NS&I and gilt interest into
other_investment_income. Its author reports £9.5bn of it in the calibrated national cut for 2024 (comment); that figure comes from a private build and is not reproduced here. On such data, household net income, deciles and poverty leave the income out without this fix.The income tax and benefit formulas do not change. One fiscal path does read market income:
attends_private_school(variables/contrib/labour/attends_private_school.py:64) useshousehold_market_incometo impute attendance, andprivate_school_vatfollows from attendance. So private school VAT can move where other investment income changes a household's rank. The aggregate impact check onenhanced_frs_2024_25.h5shows no change in any year, presumably because that dataset holds no other investment income.Overlaps
From their diffs on 29 October 2026: #2002 (
sublet_income), #2719 (household_energy_payment) and #2726 (energy_bills_rebate) add other lines to the same lists. Each adds a different line, so a conflict, if any, should be small. Not checked by a trial merge.Tests
household/income/other_investment_income_in_household_income.yaml(2 cases, 2026-27): £40,000 employment plus £2,000 other investment income gives market and household market income £42,000, income tax £5,686 ((40,000 − 12,570) × 20% + (2,000 − 1,000 PSA) × 20%), NI £2,194.40 and HBAI net income £34,119.60. The same £2,000 as bank interest gives the same values.policyengine-core testonhousehold/income(41 passed);attends_private_school.yamland the income tax folder (440 passed);pytestontest_hbai_property_income_properties.py,test_household_benefits_properties.py,test_isa_accounting.py,test_capital_gains_response_default.pyandcode_health/(3,058 passed, on a local run at875eae4d2; no log was kept, so CI's Python shards are the record);ruff format --checkandruff checkonvariables/household/income.axiom: n/a: this changes which inputs the household income aggregates (market income, HBAI net income) count; no statutory provision changes, and income tax already charged the income.
🤖 Generated with Claude Code