Skip to content

Itemise corporate_wealth's share-like components for the means tests - #1974

Open
MaxGhenis wants to merge 3 commits into
mainfrom
wealth-share-components
Open

MaxGhenis wants to merge 3 commits into
mainfrom
wealth-share-components

Conversation

@MaxGhenis

@MaxGhenis MaxGhenis commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Builds on #1791 (cash_isa, stocks_and_shares_isa), merged 2026-10-04.

What

corporate_wealth bundles assets the means tests value differently:

  • quoted shares are valued less 10% for the expenses of sale (ADM H1665; DMG 29671, 52671, 84763);
  • unit trusts are valued at the bid price with no costs of sale (ADM H1673-H1674; DMG 29680-29681);
  • ISAs are valued at what the holder would get by withdrawing (ADM H1656).

This PR itemises it, in four parts:

  1. Inputs. Two household STOCK inputs:
    • directly_held_shares: UK shares, listed or not, plus employee shares and options, held outside ISAs and pooled funds;
    • unit_and_investment_trusts: the Wealth and Assets Survey asks about both in one question.
  2. Uprating. Both inputs, plus Add cash ISA and stocks and shares ISA holdings variables #1791's cash_isa and stocks_and_shares_isa, are listed under per-capita GDP in uprating_indices.yaml, as corporate_wealth is. A dataset's identity corporate_wealth == directly_held_shares + unit_and_investment_trusts + stocks_and_shares_isa therefore survives projection. The growthfactors.md sentence now matches the YAML list exactly.
  3. Residual. unitemised_corporate_wealth (formula, STOCK) = max(0, corporate_wealth − the three components). It equals corporate_wealth on datasets without the components, including pre-split uk-data releases that still fold pension wealth in. It is about 0 on datasets that build corporate_wealth as the exact sum (Split pensions, shares, trusts, ISAs and secured debt out of WAS wealth policyengine-uk-data#501, microcosm).
  4. Capital lists. All six capital sources lists (UC, HB, IS, JSA, ESA, PC) name the three components plus unitemised_corporate_wealth instead of corporate_wealth.

Impact

None on any dataset, and that is tested. Without a valuation rule that tells the components apart, itemising leaves every total unchanged:

  • datasets without the components count corporate_wealth through the residual, as today;
  • split datasets count the same total, itemised;
  • Microcosm's current export (stocks_and_shares_isa alongside corporate_wealth) counts the ISA itemised and the rest as residual, again with the same total.

What changes is that a household situation given only component inputs now has them counted as capital. The follow-up stacked on #1969 then exempts unit trusts and ISAs from the 10% sale-expense deduction by listing only directly_held_shares and unitemised_corporate_wealth.

Invariants (tests)

  • unitemised_corporate_wealth == max(0, corporate_wealth − Σ components) for every input (Hypothesis).
  • Differential: for every programme, assessable capital with itemised holdings equals the capital of the same holdings entered as one corporate_wealth = max(corporate_wealth, Σ components) (Hypothesis, single-adult households).
  • No list names corporate_wealth together with its components, and every list names all four share-like sources.
  • Each component shares corporate_wealth's entity, quantity type, class uprating and uprating_indices.yaml index. cash_isa uprates with savings. total_wealth does not add the components on top of corporate_wealth.

Local runs

Run Result
New and touched Python tests 23 passed
tests/code_health 1,850 passed
YAML tests for ESA, HB, IS, JSA, UC, gov/dwp and household/wealth 809 passed
ruff format --check clean

Review

An independent review returned APPROVE WITH NITS at 8b86a54: (findings in the PR thread). Every nit is applied at 21412b6, except two:

axiom: n/a: model plumbing that changes no rule; the sale-expense rule itself is #1969's

🤖 Generated with Claude Code

corporate_wealth bundles three assets the means tests value differently:
quoted shares less 10% for the expenses of sale (ADM H1665), unit trusts at
the manager's withdrawal price with no deduction (H1673-H1674), and stocks
and shares ISAs at their withdrawal value (H1656). Add household inputs for
the two parts not yet modelled, directly_held_shares (UK shares and employee
shares and options) and unit_and_investment_trusts, and uprate them, cash_isa
and stocks_and_shares_isa with per-capita GDP like corporate_wealth, so a
dataset's identity corporate_wealth == sum of components survives
projection. The inputs have no consumers yet.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@juaristi22

Copy link
Copy Markdown
Collaborator

Reviewed commit 54f4c7da480f79963967dc6689af022e82673636. Needs changes; review coverage is partial. The additive wealth inputs passed their focused tests. The current-main integration now needs a documentation conflict resolved and the #1791 dependency handled before merge.

Confirmed integration conflict. A prospective merge of this exact head onto main 3c48247eb92b221312565bff3a1dbf0b87306e22, attempted on 2 October 2026, stopped with a content conflict in growthfactors.md. The attempt safely aborted without changing the PR. No integration tests ran on that prospective merged tree. GitHub's mergeability against the current add-isa-balance-variables base does not establish mergeability against main.

Preserve the component contract. test_wealth_components.py:26 verifies the share components use the same entity, stock semantics and uprating as corporate_wealth; test_wealth_components.py:42 checks they are not added again to total_wealth. These are useful inputs even before data builders or benefit formulas consume them. Their addition does not establish that every supported dataset has split pensions out of its corporate-wealth bundle, so it does not by itself resolve #1837.

Repair and acceptance steps:

  1. Land the ISA input parent Add cash ISA and stocks and shares ISA holdings variables #1791 first, then retarget this PR to main and rebase on its actual tip. If the parent is consolidated through another agreed route, preserve both ISA definitions and their changelog rather than dropping the dependency silently.
  2. Resolve growthfactors.md by retaining current-main descriptions and this PR's new component entries. Check the final prose against policyengine_uk/data/uprating_indices.yaml and variable metadata; do not select an entire outdated document side.
  3. Keep corporate_wealth as the aggregate holding its share/fund components, with cash_isa following the savings contract. Preserve the no-double-count test and the common uprating index, so a dataset that supplies the component identity retains it when projected.
  4. Run the focused suite on the integrated tree and obtain new-head hosted checks. Check documentation rendering if conflict resolution materially changes the page. Report any data migration separately, with its supported schema and consumers; no population-impact claim is needed for this input-only change.

Suggested checks after resolving the dependency and conflict:

uv run --no-sync pytest policyengine_uk/tests/test_wealth_components.py -q
uv run --no-sync ruff format --check .
uv run --no-sync ruff check .

Validation already completed on this exact PR snapshot: 3 tests passed in test_wealth_components.py, using an asserted snapshot import, core 3.32.9 and the existing environment without installing dependencies. That run preceded the attempted current-main integration. The combined tree, final documentation resolution, hosted CI after retargeting and any population-data consumers remain unverified.

Live posting check (2026-10-02T15:12:45.036767+00:00): same reviewed commit; GitHub reports MERGEABLE. No hosted checks reported. These checks do not replace the remaining validation listed above.

Base automatically changed from add-isa-balance-variables to main October 4, 2026 10:27
MaxGhenis and others added 2 commits October 4, 2026 06:28
# Conflicts:
#	docs/book/assumptions/growthfactors.md
The six capital sources lists (UC, HB, IS, JSA, ESA, PC) name
directly_held_shares, unit_and_investment_trusts and stocks_and_shares_isa
in place of corporate_wealth, plus a new formula variable,
unitemised_corporate_wealth = max(0, corporate_wealth - components), for
whatever a dataset does not itemise. Datasets without the components keep
counting corporate_wealth in full; datasets that build corporate_wealth as
the components' exact sum count the same total, now itemised, so each
component can carry its own valuation rule (the sale-expense deduction stacked
on #1969 exempts unit trusts and ISAs). No dataset's capital changes: a
differential property test checks that every programme's assessable capital
equals the same holdings entered as one corporate_wealth.

Review nits: the growth-factor sentence now matches uprating_indices.yaml
exactly; the variable docs cite the DMG for the legacy benefits, the expert
valuation of unquoted shares and the quoted-share rule for investment trusts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaxGhenis MaxGhenis changed the title Add share-like wealth component inputs Itemise corporate_wealth's share-like components for the means tests Oct 4, 2026
@MaxGhenis

Copy link
Copy Markdown
Collaborator Author

VERDICT: APPROVE WITH NITS

I couldn't run any commands here: there was no shell, so I didn't run gh pr view, git diff origin/main...HEAD, the merge-commit diff or the uv run pytest … command. I didn't read the two CLAUDE.md files either, because the search for them timed out. The tests have not been run, so I can't say whether they pass. The review below comes from reading the files at HEAD, the ADM H1 PDF and policyengine-uk-data PR #501. Run the pytest command before merging.

What checks out

  • ADM H1665: it falls under "Stocks and shares quoted on the London Stock Exchange" and says to "deduct 10% costs of sale". The docstring's "price less 10% for the expenses of sale" is accurate.

  • ADM H1673–H1674: value is the bid price × units, and "there are no costs of sale" because people withdraw through the manager. The docstring's paraphrase is accurate.

  • WAS mapping in Benefit unit rent fails to calculate from individual rent #501's docs/imputations.md:

    • directly_held_shares = DVFShUKVR8 + DVFESHARESR8 (UK shares plus employee shares and options).
    • unit_and_investment_trusts = DVFCollVR8 ("one survey question").
    • stocks_and_shares_isa = DVIISAVR8.
    • corporate_wealth is "the exact sum of its three components".
    • Overseas shares (DVFShOSVR8) are unmapped.

    This all matches the docstrings.

  • Uprating yaml (policyengine_uk/data/uprating_indices.yaml:92,94,116,118): each of the four variables appears exactly once, under yoy_growth.obr.per_capita.gdp, in alphabetical order. That matches corporate_wealth and savings, and the classes' uprating attribute matches corporate_wealth's. Being listed exactly once matters because apply_single_year_uprating (policyengine_uk/data/economic_assumptions.py:109-116) would apply the growth twice for a variable listed under two indices.

  • Merge commit: the main entries are still there (pension_credit_reported_capital, lifetime_isa_balance, household_lifetime_isa_balance) and there are no conflict markers under policyengine_uk/. That's all I could confirm without git.

  • Would the tests catch wrong uprating? Yes. test_components_match_corporate_wealth fails if a component is:

    • missing from the yaml ([] != [gdp]),
    • filed under a different index,
    • listed twice (a two-element list), or
    • given a class uprating that differs from corporate_wealth's.

    test_cash_isa_is_uprated_with_savings does the same for cash_isa. test_components_are_not_part_of_total_wealth guards against counting the components twice.

  • Naming: nothing collides with directly_held_shares or unit_and_investment_trusts. Putting them in variables/input/ alongside corporate_wealth fits, and the lowercase labels match corporate_wealth.

Findings

  1. docs/book/assumptions/growthfactors.md:63 (nit): the per-capita GDP sentence doesn't match the yaml list exactly. It is missing pension_credit_reported_capital (yaml line 109, from Tighten the Pension Credit reported-capital docs and properties (review of #2018) #2070). This probably came in from main rather than this PR, but this PR rewrites that sentence. Fix: add `pension_credit_reported_capital` between owned_land and pension_income.

  2. policyengine_uk/variables/input/directly_held_shares.py:10 and unit_and_investment_trusts.py:8 (nit): "The Enhanced FRS imputes it" is in the present tense, but uk-data Benefit unit rent fails to calculate from individual rent #501 is still a draft. Fix: say "is to impute (policyengine-uk-data Benefit unit rent fails to calculate from individual rent #501)", or merge Benefit unit rent fails to calculate from individual rent #501 first.

  3. directly_held_shares.py:12-14 (nit): "for the means tests" is too broad. ADM H1 covers Universal Credit only (H1001: "This Chapter gives guidance on capital and its effect on UC"); Pension Credit and legacy benefits are covered by the DMG. Also, the variable includes unlisted shares, which H1679 says need an expert valuation rather than the 10% rule. Fix: "Quoted shares are valued for Universal Credit at their price less 10% for the expenses of sale (ADM H1665); unquoted shares need an expert valuation (ADM H1679)."

  4. directly_held_shares.py:8 (nit): "shares in UK companies, listed or not" isn't supported by Benefit unit rent fails to calculate from individual rent #501's docs, which only say "UK shares". Fix: drop "listed or not", or cite the WAS questionnaire wording.

  5. unit_and_investment_trusts.py:10-13 (nit): the docstring explains only how unit trusts are valued. Investment trusts are listed companies, so they fall under H1665 (less 10%), not H1673–H1674. Also, H1673 refers to the "bid price". Fix: "Unit trusts are valued at the bid price with no deduction for costs of sale (ADM H1673-H1674); investment trusts are quoted shares, valued less 10% (ADM H1665)."

  6. policyengine_uk/variables/input/corporate_wealth.py:6-13 (nit): the new definition (three components, exact sum) describes datasets built with Benefit unit rent fails to calculate from individual rent #501. Datasets published now may also fold overseas shares, gilts, bonds and insurance products into corporate_wealth. Fix: add "Older datasets may also include overseas shares, gilts, bonds and insurance products."

  7. directly_held_shares.py:22 and unit_and_investment_trusts.py:21 (nit): default_value = 0 does nothing, since 0 is already the default for a float. It also differs from corporate_wealth, cash_isa and stocks_and_shares_isa, which leave it out, though some other inputs in this folder (e.g. private_pension_wealth, lifetime_isa_balance) do set it. Fix: remove it, or leave it; either works.

  8. changelog.d/wealth-share-components.added.md:1 (nit): unlike the other fragments (including add-isa-balance-variables.added.md), it doesn't start with "- ". Fix: add the leading dash.

  9. policyengine_uk/variables/household/wealth/{cash_isa,stocks_and_shares_isa}.py (nit, from Add cash ISA and stocks and shares ISA holdings variables #1791): these ISA variables live in household/wealth/, while the new components and corporate_wealth are in input/. Fix: if you want consistency, move them in a later PR; it doesn't need to block this one.

Out of scope, but worth a look: pension_credit_reported_capital uses default_value = -1 to mean "not supplied", and it is in the GDP uprating list. Projecting a dataset that stores that -1 would turn it into roughly -1.03 in later years, unless the consuming formula treats any negative value as unset.

@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.

Thanks Max. Reviewed at head 21412b6a.

What I checked:

  • Tests. test_capital_share_components.py and test_wealth_components.py: 16 passed. DWP, family and household YAML: 1,040 passed. CI is green and it merges cleanly into current main.
  • The residual. unitemised_corporate_wealth = max(0, cw − Σ components). On an unsplit household the capital totals match main exactly, so "no impact on any dataset" is true for this PR on its own.
  • Uprating. Each component is listed once under per-capita GDP, the same as corporate_wealth, and the growthfactors.md sentence now matches the YAML list.
  • No other readers. Nothing else reads corporate_wealth in a means test: only total_wealth, corporate_sector_wealth and the residual do.

Findings:

  1. Blocking (merge order). This PR must not end up on main alongside #1969 without #2131.

    • #1969's sale_expenses.sources names corporate_wealth, which this PR removes from every capital.sources list, so the 10% on shares silently disappears.
    • On a #1969 + #1974 tree, savings £5,000 + corporate_wealth £12,000 gives £17,000 again for UC, HB and IS, instead of £15,800. Seven of #1969's YAML cases fail.
    • The "Impact: none on any dataset" claim holds only while #1969 is unmerged. Please say so in the body.
    • Safe orders: merge this PR before #1969 and then land #1969 and #2131 back to back; or skip merging it separately and let #2131 (which contains it) carry it after #1969. If you take the second route, close this PR as superseded.
  2. Nit. variables/input/directly_held_shares.py:8-9. "listed or not" includes unquoted shares. They need an expert valuation (ADM H1679-H1681), not the H1665 price formula. The docstring already says so. It's worth a line noting that, under #2131, they still take the 10%, because selling them needs a solicitor or accountant (H1605). That reading is also the one #1965's owner-manager holding relies on.

This branch has not been deployed

No deployments
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.

3 participants