Repository navigation
Conversation
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>
|
Reviewed commit Confirmed integration conflict. A prospective merge of this exact head onto main Preserve the component contract. test_wealth_components.py:26 verifies the share components use the same entity, stock semantics and uprating as Repair and acceptance steps:
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 Live posting check (2026-10-02T15:12:45.036767+00:00): same reviewed commit; GitHub reports |
# 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>
|
VERDICT: APPROVE WITH NITS I couldn't run any commands here: there was no shell, so I didn't run What checks out
Findings
Out of scope, but worth a look: |
vahid-ahmadi
left a comment
There was a problem hiding this comment.
Thanks Max. Reviewed at head 21412b6a.
What I checked:
- Tests.
test_capital_share_components.pyandtest_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 thegrowthfactors.mdsentence now matches the YAML list. - No other readers. Nothing else reads
corporate_wealthin a means test: onlytotal_wealth,corporate_sector_wealthand the residual do.
Findings:
-
Blocking (merge order). This PR must not end up on main alongside #1969 without #2131.
- #1969's
sale_expenses.sourcesnamescorporate_wealth, which this PR removes from everycapital.sourceslist, 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.
- #1969's
-
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.
Builds on #1791 (
cash_isa,stocks_and_shares_isa), merged 2026-10-04.What
corporate_wealthbundles assets the means tests value differently:This PR itemises it, in four parts:
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.cash_isaandstocks_and_shares_isa, are listed under per-capita GDP inuprating_indices.yaml, ascorporate_wealthis. A dataset's identitycorporate_wealth == directly_held_shares + unit_and_investment_trusts + stocks_and_shares_isatherefore survives projection. Thegrowthfactors.mdsentence now matches the YAML list exactly.unitemised_corporate_wealth(formula, STOCK) = max(0,corporate_wealth− the three components). It equalscorporate_wealthon datasets without the components, including pre-split uk-data releases that still fold pension wealth in. It is about 0 on datasets that buildcorporate_wealthas the exact sum (Split pensions, shares, trusts, ISAs and secured debt out of WAS wealth policyengine-uk-data#501, microcosm).unitemised_corporate_wealthinstead ofcorporate_wealth.Impact
None on any dataset, and that is tested. Without a valuation rule that tells the components apart, itemising leaves every total unchanged:
corporate_wealththrough the residual, as today;stocks_and_shares_isaalongsidecorporate_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_sharesandunitemised_corporate_wealth.Invariants (tests)
unitemised_corporate_wealth == max(0, corporate_wealth − Σ components)for every input (Hypothesis).corporate_wealth = max(corporate_wealth, Σ components)(Hypothesis, single-adult households).corporate_wealthtogether with its components, and every list names all four share-like sources.corporate_wealth's entity, quantity type, class uprating anduprating_indices.yamlindex.cash_isauprates withsavings.total_wealthdoes not add the components on top ofcorporate_wealth.Local runs
tests/code_healthgov/dwpandhousehold/wealthruff format --checkReview
An independent review returned APPROVE WITH NITS at 8b86a54: (findings in the PR thread). Every nit is applied at 21412b6, except two:
variables/input/is left to a later PR;corporate_wealthnever included them.axiom: n/a: model plumbing that changes no rule; the sale-expense rule itself is #1969's
🤖 Generated with Claude Code