From b9a81b8784b59d8bc74464b9883f59272b1cb38f Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 31 Oct 2026 05:13:33 -0400 Subject: [PATCH 1/9] Record why the aggregate report withholds a reform year withheld_reasons tests every condition of reform withholding and gives one reason per condition and component or variable it holds for: suppressed_component_changed, small_component_left, small_balance_change, small_target_change and no_baseline. Each names the condition and the variable, and the published metric id of a partition component, never a count or an amount. reform_withheld keeps its verdict: any reason. Both reform figures keep the reasons under withheld_because, the long-format report gives them on each reforms and reform_targets row, the comparison carries them per side, and the comment lists them under the reform costings. A failing responsiveness detail in a withheld year names them too. Fixes #2654. Co-Authored-By: Claude Opus 5.5 --- .../reform_withheld_main_reference.py | 85 +++ .../test_aggregate_impact_withheld_reasons.py | 584 ++++++++++++++++++ policyengine_uk/utils/aggregate_impact/api.py | 62 +- .../utils/aggregate_impact/compare.py | 10 + .../utils/aggregate_impact/measure.py | 129 +++- .../utils/aggregate_impact/privacy.py | 56 +- .../utils/aggregate_impact/render.py | 56 ++ 7 files changed, 930 insertions(+), 52 deletions(-) create mode 100644 policyengine_uk/tests/aggregate_impact/reform_withheld_main_reference.py create mode 100644 policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py diff --git a/policyengine_uk/tests/aggregate_impact/reform_withheld_main_reference.py b/policyengine_uk/tests/aggregate_impact/reform_withheld_main_reference.py new file mode 100644 index 0000000000..5e5452a1b8 --- /dev/null +++ b/policyengine_uk/tests/aggregate_impact/reform_withheld_main_reference.py @@ -0,0 +1,85 @@ +"""Frozen reform withholding from main (da31e8698), with commentary omitted. + +Keep this independent of the production tool: it is the differential oracle +for #2654, which records why a reform year is withheld and must not change +which years are withheld. +""" + +from typing import Mapping, Optional + +import numpy as np + +MIN_CELL = 10 +CHANGE_GBP = 0.005 + + +def small(count: int) -> bool: + return 0 < count < MIN_CELL + + +def array(sim, variable: str, period, map_to: Optional[str] = None) -> np.ndarray: + try: + result = sim.calculate(variable, period, map_to=map_to, unweighted=True) + except TypeError: + result = sim.calculate(variable, period, map_to=map_to) + return np.asarray(getattr(result, "values", result)) + + +def entity_of(sim, variable: str) -> str: + return sim.tax_benefit_system.variables[variable].entity.key + + +def to_household(sim, values: np.ndarray, entity: str) -> np.ndarray: + if entity == "household": + return np.asarray(values, dtype=np.float64) + return np.asarray( + sim.map_result(np.asarray(values, dtype=np.float64), entity, "household"), + dtype=np.float64, + ) + + +def _weighted_records(mask, weight) -> int: + return int(np.count_nonzero(np.asarray(mask) & (np.asarray(weight) > 0))) + + +def reform_withheld( + reformed, year: int, baseline: Optional[Mapping], components +) -> bool: + if baseline is None: + return True + for variable, before in baseline["hidden"].items(): + if not np.array_equal( + array(reformed, variable, year).astype(np.float64), before + ): + return True + weights = {} + + def weight(variable): + entity = entity_of(reformed, variable) + if entity not in weights: + weights[entity] = array(reformed, f"{entity}_weight", year).astype( + np.float64 + ) + return weights[entity] + + for variable in components: + if variable in baseline["hidden"]: + continue + values = array(reformed, variable, year).astype(np.float64) + if small(_weighted_records(values != 0, weight(variable))): + return True + balance = to_household( + reformed, + array(reformed, "gov_balance", year).astype(np.float64), + entity_of(reformed, "gov_balance"), + ) + changed = np.abs(balance - baseline["balance"]) > CHANGE_GBP + if small(_weighted_records(changed, baseline["household_weight"])): + return True + for variable, before in baseline["targets"].items(): + after = array(reformed, variable, year).astype(np.float64) + if small( + _weighted_records(np.abs(after - before) > CHANGE_GBP, weight(variable)) + ): + return True + return False diff --git a/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py b/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py new file mode 100644 index 0000000000..6528143282 --- /dev/null +++ b/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py @@ -0,0 +1,584 @@ +"""Why a reform year is withheld (#2654): one reason per condition that holds. + +``measure.withheld_reasons`` names each condition of reform withholding that +holds, and the component or variable it holds for. These tests pin one case +per condition, two conditions together, and, for all small worlds, that a +year is withheld exactly when it has a reason, that the reasons match a +direct computation, and that the years withheld and the figures published +match main's ``reform_withheld`` (the frozen oracle in +``reform_withheld_main_reference.py``). +""" + +import re +from types import SimpleNamespace + +import numpy as np +import pytest +from hypothesis import given, settings +from hypothesis import strategies as st + +from policyengine_uk.tests.aggregate_impact import ( + reform_withheld_main_reference as main_reference, +) +from policyengine_uk.utils.aggregate_impact.api import to_report +from policyengine_uk.utils.aggregate_impact.compare import compare +from policyengine_uk.utils.aggregate_impact.measure import ( + Recorder, + _weighted_records, + baseline_records, + component_metric_ids, + measure_reforms, + reform_withheld, + weighted_total, + withheld_reasons, +) +from policyengine_uk.utils.aggregate_impact.partition import parse_partition +from policyengine_uk.utils.aggregate_impact.privacy import ( + MIN_CELL, + NO_BASELINE, + SMALL_BALANCE_CHANGE, + SMALL_COMPONENT_LEFT, + SMALL_TARGET_CHANGE, + SUPPRESSED_COMPONENT_CHANGED, + WITHHELD_CONDITIONS, + withheld_text, +) +from policyengine_uk.utils.aggregate_impact.reforms import parse_reforms +from policyengine_uk.utils.aggregate_impact.render import _withheld_notes, render +from policyengine_uk.utils.aggregate_impact.spec import load_spec + +SPEC = load_spec() +N = 20 +YEAR = 2026 +HEADS = ("income_tax", "dividend_tax", "state_pension") +METRIC_IDS = { + "income_tax": "receipts.income_tax", + "dividend_tax": "receipts.dividend_tax", + "state_pension": "spending.state_pension", +} + + +class WorldSim: + """Household arrays by year and variable, with household weights. + + gov_balance is income tax plus dividend tax less the state pension. + """ + + def __init__(self, values_by_year, weight): + self.values = values_by_year + self.weight = np.asarray(weight, dtype=float) + entity = SimpleNamespace(key="household") + names = {v for values in values_by_year.values() for v in values} + self.tax_benefit_system = SimpleNamespace( + variables={ + v: SimpleNamespace(entity=entity) + for v in names | {"gov_balance", "household_weight"} + } + ) + + def calculate(self, variable, period, map_to=None, unweighted=True): + if variable == "household_weight": + return self.weight + values = self.values[int(period)] + if variable == "gov_balance": + return _balance(values) + return np.asarray(values.get(variable, np.zeros(N)), dtype=float) + + +def _balance(values) -> np.ndarray: + zero = np.zeros(N) + return ( + np.asarray(values.get("income_tax", zero), dtype=float) + + np.asarray(values.get("dividend_tax", zero), dtype=float) + - np.asarray(values.get("state_pension", zero), dtype=float) + ) + + +def _partition(): + def head(i, totals): + return {"id": i, "label": i, "variable": i, "totals": totals} + + return parse_partition( + { + "schema_version": 1, + "totals": {"gov_tax": {}, "gov_spending": {}}, + "tax_heads": [ + head("income_tax", ["gov_tax"]), + head("dividend_tax", ["gov_tax"]), + ], + "spending_heads": [head("state_pension", ["gov_spending"])], + } + ) + + +def _reform(target="income_tax", name="Change it"): + return parse_reforms( + { + "reforms": [ + { + "name": name, + "parameters": {"a": 1}, + "responds": { + "variable": target, + "direction": "up", + "minimum": 1e12, + }, + } + ] + } + ) + + +def _measure(base, reformed, weight=None, target="income_tax", years=(YEAR,)): + """Measure one reform as measure() does: record the baseline's heads, + with their suppression, then cost the reform against the baseline. + + ``base`` and ``reformed`` map variable to household values, or year to + such a mapping.""" + if not isinstance(next(iter(base)), int): + base = {year: base for year in years} + reformed = {year: reformed for year in years} + weight = np.ones(N) if weight is None else np.asarray(weight, dtype=float) + partition = _partition() + base_sim = WorldSim(base, weight) + rec = Recorder() + for year in years: + for head in partition.heads: + section = "receipts" if head.kind == "tax" else "spending" + values = np.asarray(base[year].get(head.variable, np.zeros(N))) + rec.metric( + f"{section}.{head.id}", + head.label, + section, + "gbp", + year, + float(np.sum(weight * values)), + records=_weighted_records(values != 0, weight), + ) + reform = _reform(target) + measure_reforms( + lambda reform=None: WorldSim(reformed if reform else base, weight), + None, + {y: weighted_total(base_sim, "gov_balance", y) for y in years}, + {y: {target: weighted_total(base_sim, target, y)} for y in years}, + reform, + list(years), + rec, + baseline={ + y: baseline_records(base_sim, partition, rec, y, [target]) for y in years + }, + components=partition.component_variables(), + ) + return rec + + +def _reasons(rec, year=YEAR, metric="reforms.change_it"): + return rec.metrics[metric]["withheld_because"].get(str(year), []) + + +def _conditions(reasons): + return [(r["condition"], r["variable"], r["metric_id"]) for r in reasons] + + +def _world(income_tax, state_pension, dividend_tax=None, **others): + values = { + "income_tax": np.asarray(income_tax, dtype=float), + "state_pension": np.asarray(state_pension, dtype=float), + "dividend_tax": np.full(N, 50.0) + if dividend_tax is None + else np.asarray(dividend_tax, dtype=float), + } + values.update({k: np.asarray(v, dtype=float) for k, v in others.items()}) + return values + + +def _first(n, value, otherwise=0.0): + return np.r_[np.full(n, value), np.full(N - n, otherwise)] + + +# One case per condition. Each world fires exactly one condition. + + +def test_a_reform_that_changes_a_suppressed_component_names_it(): + """Two pensions (suppressed); the reform raises one and every income tax + bill, so 20 balances and 20 target records change.""" + base = _world(np.full(N, 100.0), _first(2, 1_000.0)) + reformed = _world(np.full(N, 110.0), np.r_[1_100.0, 1_000.0, np.zeros(N - 2)]) + rec = _measure(base, reformed) + assert _conditions(_reasons(rec)) == [ + (SUPPRESSED_COMPONENT_CHANGED, "state_pension", "spending.state_pension") + ] + assert rec.metrics["reforms.change_it"]["values"][str(YEAR)] is None + + +def test_a_reform_that_leaves_a_small_component_names_it(): + """Dividend tax falls from 20 payers to 5: a new small cell.""" + base = _world(np.full(N, 100.0), np.full(N, 100.0)) + reformed = _world(np.full(N, 110.0), np.full(N, 100.0), _first(5, 50.0)) + rec = _measure(base, reformed) + assert _conditions(_reasons(rec)) == [ + (SMALL_COMPONENT_LEFT, "dividend_tax", "receipts.dividend_tax") + ] + + +def test_a_reform_that_changes_few_balances_says_so(): + """Three households' income tax rises; the target, the state pension, + doesn't move.""" + base = _world(np.full(N, 100.0), np.full(N, 100.0)) + reformed = _world(_first(3, 110.0, 100.0), np.full(N, 100.0)) + rec = _measure(base, reformed, target="state_pension") + assert _conditions(_reasons(rec)) == [(SMALL_BALANCE_CHANGE, "gov_balance", None)] + + +def test_a_reform_that_changes_few_target_records_names_the_target(): + """Pension Credit, outside gov_balance here, rises for three households; + every household's income tax rises too.""" + base = _world(np.full(N, 100.0), np.full(N, 100.0), pension_credit=np.zeros(N)) + reformed = _world( + np.full(N, 110.0), np.full(N, 100.0), pension_credit=_first(3, 25.0) + ) + rec = _measure(base, reformed, target="pension_credit") + assert _conditions(_reasons(rec)) == [(SMALL_TARGET_CHANGE, "pension_credit", None)] + target = rec.metrics["reform_targets.change_it"] + assert target["values"][str(YEAR)] is None + assert ( + target["withheld_because"] + == rec.metrics["reforms.change_it"]["withheld_because"] + ) + + +def test_a_year_without_baseline_records_says_so(): + rec = Recorder() + reform = parse_reforms({"reforms": [{"name": "Any", "parameters": {"a": 1}}]}) + world = {y: _world(np.full(N, 1.0), np.full(N, 1.0)) for y in (2025, 2026)} + measure_reforms( + lambda reform=None: WorldSim(world, np.ones(N)), + None, + {2025: 0.0, 2026: 0.0}, + {}, + reform, + [2025, 2026], + rec, + ) + assert rec.metrics["reforms.any"]["withheld_because"] == { + "2025": [{"condition": NO_BASELINE, "variable": None, "metric_id": None}], + "2026": [{"condition": NO_BASELINE, "variable": None, "metric_id": None}], + } + + +def test_two_conditions_together_are_both_recorded(): + """The reform raises one suppressed pension and leaves dividend tax with + five payers; every income tax bill changes, so neither the balance nor + the target is a small change. Main stopped at the first condition.""" + base = _world(np.full(N, 100.0), _first(2, 1_000.0)) + reformed = _world( + np.full(N, 110.0), + np.r_[1_100.0, 1_000.0, np.zeros(N - 2)], + _first(5, 50.0), + ) + rec = _measure(base, reformed) + assert _conditions(_reasons(rec)) == [ + (SUPPRESSED_COMPONENT_CHANGED, "state_pension", "spending.state_pension"), + (SMALL_COMPONENT_LEFT, "dividend_tax", "receipts.dividend_tax"), + ] + detail = rec.checks["responsive:change_it"]["failing"][str(YEAR)] + assert ( + "the reform changes a record of spending.state_pension, which the " + "baseline suppresses; the reform leaves receipts.dividend_tax with " + "fewer than 10 records" in detail + ) + + +def test_every_condition_at_once(): + """A 10% rise in one suppressed £4 pension, and dividend tax falls from + 12 payers to 5: eight balances and one target record change.""" + base = _world(np.full(N, 100.0), _first(1, 4.0), _first(12, 50.0)) + reformed = _world(np.full(N, 100.0), _first(1, 4.4), _first(5, 50.0)) + rec = _measure(base, reformed, target="state_pension") + assert [c for c, _, _ in _conditions(_reasons(rec))] == [ + SUPPRESSED_COMPONENT_CHANGED, + SMALL_COMPONENT_LEFT, + SMALL_BALANCE_CHANGE, + SMALL_TARGET_CHANGE, + ] + + +def test_a_published_year_records_no_reason(): + base = _world(np.full(N, 100.0), np.full(N, 100.0)) + reformed = _world(np.full(N, 110.0), np.full(N, 100.0)) + rec = _measure(base, reformed) + record = rec.metrics["reforms.change_it"] + assert record["values"][str(YEAR)] == pytest.approx(200.0) + assert record["withheld_because"] == {} and "suppressed" not in record + + +def test_component_metric_ids_name_heads_and_double_counts(): + partition = SimpleNamespace( + heads=( + SimpleNamespace(variable="income_tax", id="income_tax", kind="tax"), + SimpleNamespace(variable="dla", id="disability", kind="spending"), + ), + double_counts=(SimpleNamespace(variable="sdlt_corporate"),), + ) + assert component_metric_ids(partition) == { + "income_tax": "receipts.income_tax", + "dla": "spending.disability", + "sdlt_corporate": "double_counts.sdlt_corporate", + } + + +# The issue's example: two heads suppressed in every year, a reform withheld +# in 2027-28 to 2030-31 only. + +ISSUE_YEARS = tuple(range(2025, 2032)) +WITHHELD_YEARS = (2027, 2028, 2029, 2030) + + +def _issue_example(): + base = {y: _world(np.full(N, 100.0), _first(2, 1_000.0)) for y in ISSUE_YEARS} + reformed = { + y: _world( + np.full(N, 103.0), + _first(2, 1_000.0), + _first(4, 50.0) if y in WITHHELD_YEARS else np.full(N, 50.0), + ) + for y in ISSUE_YEARS + } + return _measure(base, reformed, years=ISSUE_YEARS) + + +def test_the_issue_example_says_why_only_some_years_are_withheld(): + rec = _issue_example() + record = rec.metrics["reforms.change_it"] + assert record["suppressed"] == [str(y) for y in WITHHELD_YEARS] + assert set(record["withheld_because"]) == set(record["suppressed"]) + for year in WITHHELD_YEARS: + assert _conditions(_reasons(rec, year)) == [ + (SMALL_COMPONENT_LEFT, "dividend_tax", "receipts.dividend_tax") + ] + for year in set(ISSUE_YEARS) - set(WITHHELD_YEARS): + assert record["values"][str(year)] == pytest.approx(60.0) + + +def test_the_long_format_report_carries_the_reasons_on_reform_rows_only(): + rec = _issue_example() + report = to_report( + { + "status": "ok", + "provenance": {}, + "partition": {"sha256": "p"}, + "years": list(ISSUE_YEARS), + "metrics": list(rec.metrics.values()), + "checks": list(rec.checks.values()), + } + ) + rows = report["metrics"] + reform_rows = [r for r in rows if r["metric_id"].startswith("reform")] + assert {r["metric_id"] for r in reform_rows} == { + "reforms.change_it", + "reform_targets.change_it", + } + for row in reform_rows: + assert row["suppressed"] == bool(row["withheld_because"]) + assert row["suppressed"] == (row["year"] in WITHHELD_YEARS) + assert all("withheld_because" not in r for r in rows if r not in reform_rows) + target_2027 = next( + r + for r in reform_rows + if r["metric_id"] == "reform_targets.change_it" and r["year"] == 2027 + ) + assert target_2027["value"] is None + assert target_2027["withheld_because"] == [ + { + "condition": SMALL_COMPONENT_LEFT, + "variable": "dividend_tax", + "metric_id": "receipts.dividend_tax", + } + ] + + +def _side(rec): + from policyengine_uk.tests.aggregate_impact.test_aggregate_impact_compare import ( + result, + ) + + side = result({}) + side["metrics"] = list(rec.metrics.values()) + return side + + +def test_the_comment_says_why_each_reform_year_is_suppressed(): + rec = _issue_example() + summary = compare(_side(rec), _side(rec), SPEC) + assert summary["status"] == "pass", summary["failures"] + row = next(r for r in summary["rows"] if r["id"] == "reforms.change_it") + reason = [ + { + "condition": SMALL_COMPONENT_LEFT, + "variable": "dividend_tax", + "metric_id": "receipts.dividend_tax", + } + ] + assert row["values"]["2027"]["withheld_because"] == { + "base": reason, + "head": reason, + } + assert "withheld_because" not in row["values"]["2025"] + text = render(summary, SPEC) + line = ( + "- Change it, 2027-28 to 2030-31 (base and head): the reform leaves " + "receipts.dividend_tax with fewer than 10 records." + ) + assert line in text + # Once, under the costings, though the targets are withheld too. + assert text.count("receipts.dividend_tax with fewer than 10") == 1 + assert text.index(line) > text.index("### Standard reform costings") + + +def test_the_comment_tells_the_sides_apart(): + base_rec = _issue_example() + world = {y: _world(np.full(N, 100.0), _first(2, 1_000.0)) for y in ISSUE_YEARS} + head_rec = _measure( + world, + { + y: _world( + np.full(N, 103.0), + np.r_[1_001.0, 1_000.0, np.zeros(N - 2)] + if y == 2026 + else w["state_pension"], + ) + for y, w in world.items() + }, + years=ISSUE_YEARS, + ) + summary = compare(_side(base_rec), _side(head_rec), SPEC) + notes = _withheld_notes( + [r for r in summary["rows"] if r["section"] == "reforms"], ISSUE_YEARS + ) + assert notes == [ + "- Change it, 2027-28 to 2030-31 (base): the reform leaves " + "receipts.dividend_tax with fewer than 10 records.", + "- Change it, 2026-27 (head): the reform changes a record of " + "spending.state_pension, which the baseline suppresses.", + ] + + +# For all small worlds. + +values = st.floats(1.0, 1e4, allow_nan=False) +deltas = st.sampled_from([0.0, 0.004, 0.006, 1.0, -250.0, 1e3]) + + +@st.composite +def worlds(draw): + """A baseline and a reformed world of N households, with each variable's + number of non-zero records and of changed records drawn from 0 to N, so + small cells come up often; some households carry no weight.""" + weight = np.asarray( + draw(st.lists(st.sampled_from([0.0, 1.0, 2.5]), min_size=N, max_size=N)) + ) + base, reformed = {}, {} + for variable in HEADS + ("pension_credit",): + support = draw(st.integers(0, N)) + level = np.asarray(draw(st.lists(values, min_size=N, max_size=N))) + before = np.where(np.arange(N) < support, level, 0.0) + start = draw(st.integers(0, N - 1)) + changed = draw(st.integers(0, N)) + delta = draw(deltas) + moved = (np.arange(N) - start) % N < changed + after = np.where(moved, before + delta, before) + if draw(st.booleans()): + # Some reforms take a component away from most of its records. + after = np.where(np.arange(N) < draw(st.integers(0, N)), after, 0.0) + base[variable], reformed[variable] = before, after + target = draw(st.sampled_from(HEADS + ("pension_credit",))) + return base, reformed, weight, target + + +def _direct_reasons(base, reformed, weight, target): + """The conditions computed straight from the arrays: a second + implementation of the rule.""" + + def records(mask): + return int(np.count_nonzero(mask & (weight > 0))) + + def small(count): + return 0 < count < MIN_CELL + + hidden = [v for v in HEADS if small(records(base[v] != 0))] + out = [ + (SUPPRESSED_COMPONENT_CHANGED, v, METRIC_IDS[v]) + for v in hidden + if not np.array_equal(reformed[v], base[v]) + ] + out += [ + (SMALL_COMPONENT_LEFT, v, METRIC_IDS[v]) + for v in HEADS + if v not in hidden and small(records(reformed[v] != 0)) + ] + if small(records(np.abs(_balance(reformed) - _balance(base)) > 0.005)): + out.append((SMALL_BALANCE_CHANGE, "gov_balance", None)) + if small(records(np.abs(reformed[target] - base[target]) > 0.005)): + out.append((SMALL_TARGET_CHANGE, target, None)) + return out + + +@settings(max_examples=300, deadline=None) +@given(world=worlds()) +def test_a_year_is_withheld_exactly_when_it_has_a_reason(world): + """For every world: the figures are withheld exactly when a reason is + recorded; the reasons are the conditions computed directly; the years + withheld match main's reform_withheld; and the published figures are + the direct weighted changes.""" + base, reformed, weight, target = world + rec = _measure(base, reformed, weight, target) + reasons = _reasons(rec) + for metric_id in ("reforms.change_it", "reform_targets.change_it"): + record = rec.metrics[metric_id] + withheld = record["values"][str(YEAR)] is None + assert withheld == bool(reasons) + assert set(record.get("suppressed", [])) == set(record["withheld_because"]) + assert record["withheld_because"].get(str(YEAR), []) == reasons + assert _conditions(reasons) == _direct_reasons(base, reformed, weight, target) + assert all(r["condition"] in WITHHELD_CONDITIONS for r in reasons) + + partition = _partition() + base_sim, reformed_sim = ( + WorldSim({YEAR: base}, weight), + WorldSim({YEAR: reformed}, weight), + ) + records = baseline_records(base_sim, partition, rec, YEAR, [target]) + components = partition.component_variables() + on_main = main_reference.reform_withheld(reformed_sim, YEAR, records, components) + assert on_main == bool(reasons) + assert reform_withheld(reformed_sim, YEAR, records, components) == on_main + assert withheld_reasons(reformed_sim, YEAR, records, components) == reasons + + costing = float(np.sum(weight * (_balance(reformed) - _balance(base)))) + change = float(np.sum(weight * (reformed[target] - base[target]))) + published = rec.metrics["reforms.change_it"]["values"][str(YEAR)] + target_change = rec.metrics["reform_targets.change_it"]["values"][str(YEAR)] + if on_main: + assert published is None and target_change is None + else: + assert published == pytest.approx(costing, abs=1e-6) + assert target_change == pytest.approx(change, abs=1e-6) + + +@settings(max_examples=300, deadline=None) +@given(world=worlds()) +def test_a_reason_never_gives_a_count_or_an_amount(world): + """A reason holds only names; in words it says "fewer than 10" at most, + whatever the counts and amounts behind it.""" + base, reformed, weight, target = world + rec = _measure(base, reformed, weight, target) + reasons = _reasons(rec) + for reason in reasons: + assert set(reason) == {"condition", "variable", "metric_id"} + assert all(value is None or isinstance(value, str) for value in reason.values()) + text = withheld_text(reasons) + assert not re.search(r"\d", text.replace(f"fewer than {MIN_CELL}", "")) + assert "£" not in text + detail = rec.checks["responsive:change_it"]["failing"].get(str(YEAR), "") + if reasons: + assert text in detail and "£" not in detail diff --git a/policyengine_uk/utils/aggregate_impact/api.py b/policyengine_uk/utils/aggregate_impact/api.py index f9c0bcc575..9f239a4ffd 100644 --- a/policyengine_uk/utils/aggregate_impact/api.py +++ b/policyengine_uk/utils/aggregate_impact/api.py @@ -55,7 +55,13 @@ def _spec_for_years(spec: Spec, years: Optional[Sequence[int]]) -> Spec: def to_report(result: dict) -> dict: - """Flatten a measurement into the long-format qualification report.""" + """Flatten a measurement into the long-format qualification report. + + Rows of the ``reforms.`` and ``reform_targets.`` figures also + carry ``withheld_because``: for a year the tool withholds, the conditions + that withheld it, each as ``{"condition", "variable", "metric_id"}`` + (``privacy.WITHHELD_CONDITIONS``); for a published year, an empty list. + """ provenance = result.get("provenance") or {} report = { "schema": TOOL_SCHEMA + "/report", @@ -75,29 +81,37 @@ def to_report(result: dict) -> dict: metric["id"].split(".", 1)[1] if section in PARTITIONED_SECTIONS else None ) for year, value in metric["values"].items(): - report["metrics"].append( - { - "metric_id": metric["id"], - "year": int(year), - # None when the figure comes from fewer than 10 records. - "value": value, - "suppressed": year in metric.get("suppressed", ()), - # The suppressed components this total is rebuilt without - # that year (measure.rebuild_protected); compare totals at - # equal exclusion. - "excludes": sorted((metric.get("excludes") or {}).get(year) or {}), - # The sign each enters the total with (+1 or -1), so two - # additive totals with different exclusions can be lined - # up from published figures (compare._aligned). - "exclude_signs": dict( - sorted(((metric.get("excludes") or {}).get(year) or {}).items()) - ), - "unit": metric["kind"], - "kind": KIND_BY_SECTION.get(section, section), - "partition": head_id, - "label": metric["label"], - } - ) + row = { + "metric_id": metric["id"], + "year": int(year), + # None when the figure comes from fewer than 10 records. + "value": value, + "suppressed": year in metric.get("suppressed", ()), + # The suppressed components this total is rebuilt without + # that year (measure.rebuild_protected); compare totals at + # equal exclusion. + "excludes": sorted((metric.get("excludes") or {}).get(year) or {}), + # The sign each enters the total with (+1 or -1), so two + # additive totals with different exclusions can be lined + # up from published figures (compare._aligned). + "exclude_signs": dict( + sorted(((metric.get("excludes") or {}).get(year) or {}).items()) + ), + "unit": metric["kind"], + "kind": KIND_BY_SECTION.get(section, section), + "partition": head_id, + "label": metric["label"], + } + if "withheld_because" in metric: + # A reform figure: why its year is withheld, one reason per + # condition and the component or variable it holds for + # (measure.withheld_reasons); empty exactly when the year is + # published. + row["withheld_because"] = [ + dict(reason) + for reason in (metric["withheld_because"].get(year) or []) + ] + report["metrics"].append(row) for check in result["checks"]: report["identities"].append( { diff --git a/policyengine_uk/utils/aggregate_impact/compare.py b/policyengine_uk/utils/aggregate_impact/compare.py index dc1a161bcf..f24087122b 100644 --- a/policyengine_uk/utils/aggregate_impact/compare.py +++ b/policyengine_uk/utils/aggregate_impact/compare.py @@ -193,6 +193,16 @@ def compare_metrics(base: Mapping, head: Mapping, spec: Spec, years) -> tuple: and bool(base_metric and head_metric) and (b_hidden or h_hidden or (_finite(b) and _finite(h))), } + reasons = { + side: list( + ((metric or {}).get("withheld_because") or {}).get(key) or [] + ) + for side, metric in (("base", base_metric), ("head", head_metric)) + } + if any(reasons.values()): + # Why each side withholds a reform figure this year + # (measure.withheld_reasons). + row["values"][key]["withheld_because"] = reasons if (delta is not None and delta != 0) or row["values"][key]["withheld"]: row["changed"] = True if ( diff --git a/policyengine_uk/utils/aggregate_impact/measure.py b/policyengine_uk/utils/aggregate_impact/measure.py index 192a512422..ea0a644054 100644 --- a/policyengine_uk/utils/aggregate_impact/measure.py +++ b/policyengine_uk/utils/aggregate_impact/measure.py @@ -16,7 +16,18 @@ import numpy as np -from .privacy import MIN_CELL, count_text, small +from .privacy import ( + MIN_CELL, + NO_BASELINE, + SMALL_BALANCE_CHANGE, + SMALL_COMPONENT_LEFT, + SMALL_TARGET_CHANGE, + SUPPRESSED_COMPONENT_CHANGED, + count_text, + small, + withheld_reason, + withheld_text, +) from . import TOOL_SCHEMA from .guard import counted_once, snapshot_inputs from .partition import Partition, closure_problems, group_label, read_model_lists @@ -1233,14 +1244,33 @@ def weighted_total(sim, variable: str, year: int) -> float: CHANGE_GBP = 0.005 +def component_metric_ids(partition: Partition) -> dict: + """``{variable: published metric id}`` for each head and declared double + count.""" + out = {} + for head in partition.heads: + section = "receipts" if head.kind == "tax" else "spending" + out.setdefault(head.variable, f"{section}.{head.id}") + for dc in partition.double_counts: + out.setdefault(dc.variable, f"double_counts.{dc.variable}") + return out + + def baseline_records(sim, partition: Partition, rec: "Recorder", year: int, targets): """What the reform checks need from the baseline run in ``year``: each suppressed component's values, each household's gov_balance and each - target variable's values. Kept in memory only, never written.""" + target variable's values, and each component's published metric id, so + that a reason for withholding can name it. Kept in memory only, never + written.""" + hidden = suppressed_components(rec, partition, year) return { "hidden": { variable: array(sim, variable, year).astype(np.float64).copy() - for variable in suppressed_components(rec, partition, year) + for variable in hidden + }, + "metric_ids": { + **component_metric_ids(partition), + **{variable: metric_id for variable, (metric_id, _) in hidden.items()}, }, "household_weight": array(sim, "household_weight", year) .astype(np.float64) @@ -1257,10 +1287,11 @@ def baseline_records(sim, partition: Partition, rec: "Recorder", year: int, targ } -def reform_withheld( +def withheld_reasons( reformed, year: int, baseline: Optional[Mapping], components -) -> bool: - """Whether a reform's costing and target change for ``year`` are withheld. +) -> list: + """Why a reform's costing and target change for ``year`` are withheld: + empty when they are published. They would be built from a small cell (Sol re-review 7 of #2335) when the reform: @@ -1270,22 +1301,33 @@ def reform_withheld( figures (the tax on one recipient's pension, say). An unchanged record compares equal, because a rebuilt baseline reproduces the first run exactly (``determinism:rebuilt_baseline``); noise could only withhold - more; + more (``suppressed_component_changed``); - leaves any other component with 1 to 9 records (``privacy.small``), - whose change would give that new small cell back; - - changes gov_balance in 1 to 9 households, or a target variable in 1 to - 9 records, by more than ``CHANGE_GBP``: the change is theirs alone, as - for a decile row. - - Withheld too when there is no baseline record for the year. + whose change would give that new small cell back + (``small_component_left``); + - changes gov_balance in 1 to 9 households (``small_balance_change``), or + a target variable in 1 to 9 records (``small_target_change``), by more + than ``CHANGE_GBP``: the change is theirs alone, as for a decile row. + + Withheld too when there is no baseline record for the year + (``no_baseline``). It tests every condition and gives one reason for + each component or variable a condition holds for + (``privacy.withheld_reason``): the reason names it and gives no count or + amount (#2654). """ if baseline is None: - return True + return [withheld_reason(NO_BASELINE)] + metric_ids = baseline.get("metric_ids") or {} + reasons = [] for variable, before in baseline["hidden"].items(): if not np.array_equal( array(reformed, variable, year).astype(np.float64), before ): - return True + reasons.append( + withheld_reason( + SUPPRESSED_COMPONENT_CHANGED, variable, metric_ids.get(variable) + ) + ) weights = {} def weight(variable): @@ -1301,7 +1343,11 @@ def weight(variable): continue values = array(reformed, variable, year).astype(np.float64) if small(_weighted_records(values != 0, weight(variable))): - return True + reasons.append( + withheld_reason( + SMALL_COMPONENT_LEFT, variable, metric_ids.get(variable) + ) + ) balance = to_household( reformed, array(reformed, "gov_balance", year).astype(np.float64), @@ -1309,14 +1355,36 @@ def weight(variable): ) changed = np.abs(balance - baseline["balance"]) > CHANGE_GBP if small(_weighted_records(changed, baseline["household_weight"])): - return True + reasons.append(withheld_reason(SMALL_BALANCE_CHANGE, "gov_balance")) for variable, before in baseline["targets"].items(): after = array(reformed, variable, year).astype(np.float64) if small( _weighted_records(np.abs(after - before) > CHANGE_GBP, weight(variable)) ): - return True - return False + reasons.append(withheld_reason(SMALL_TARGET_CHANGE, variable)) + return reasons + + +def reform_withheld( + reformed, year: int, baseline: Optional[Mapping], components +) -> bool: + """Whether a reform's costing and target change for ``year`` are withheld: + whether any condition of ``withheld_reasons`` holds.""" + return bool(withheld_reasons(reformed, year, baseline, components)) + + +def _record_reasons(record: dict, year: int, reasons: list) -> None: + """Keep a withheld year's reasons on a reform figure's record. + + Every reform figure carries ``withheld_because``; it lists exactly the + years its ``suppressed`` lists, so a year is withheld exactly when it + has a reason. + """ + # Rebound rather than written through, as for _exclude. + record["withheld_because"] = { + **(record.get("withheld_because") or {}), + **({str(year): list(reasons)} if reasons else {}), + } def as_standard_reforms(reforms) -> list: @@ -1350,7 +1418,9 @@ def measure_reforms( (``reform_withheld``) publishes neither the costing nor the target change, and its responsiveness detail gives no amount: a 10% uplift's cost of £100 would give back a suppressed £1,000 (Sol re-reviews 6 and 7 - of #2335). Without ``baseline``, every year is withheld. + of #2335). Without ``baseline``, every year is withheld. Both figures + keep each withheld year's reasons (``withheld_reasons``) under + ``withheld_because``, and a failing responsiveness detail gives them. First rebuilds an unreformed simulation the same way, which must reproduce the baseline exactly: otherwise reform costings would include @@ -1391,19 +1461,20 @@ def measure_reforms( "error": None, } target_changes = {} - hidden_years = set() + # The reasons for each withheld year (withheld_reasons). + hidden_years = {} try: parameters = reform_parameters(reform, baseline_parameters, years) outcome["parameters"] = parameters reformed = factory(reform=parameters) for year in years: - withheld = reform_withheld( + reasons = withheld_reasons( reformed, year, (baseline or {}).get(year), components ) - if withheld: - hidden_years.add(year) + if reasons: + hidden_years[year] = reasons # records=1 publishes the figure as suppressed (privacy.py). - records = 1 if withheld else None + records = 1 if reasons else None rec.metric( f"reforms.{reform.id}", reform.name, @@ -1414,6 +1485,7 @@ def measure_reforms( - baseline_balance[year], records=records, ) + _record_reasons(rec.metrics[f"reforms.{reform.id}"], year, reasons) if reform.responds is not None: variable = reform.responds.variable change = ( @@ -1430,6 +1502,9 @@ def measure_reforms( change, records=records, ) + _record_reasons( + rec.metrics[f"reform_targets.{reform.id}"], year, reasons + ) del reformed except Exception as error: # noqa: BLE001 - one reform's failure is reported, not fatal outcome["error"] = f"{type(error).__name__}: {error}" @@ -1464,8 +1539,8 @@ def measure_reforms( if year in hidden_years and not ok: detail = ( f"{fiscal_year_label(year)}: {reform.responds.variable} did " - "not move enough (the amount is withheld: it comes from " - f"fewer than {MIN_CELL} records)" + "not move enough (the amount is withheld: " + f"{withheld_text(hidden_years[year])})" ) rec.check( f"responsive:{reform.id}", diff --git a/policyengine_uk/utils/aggregate_impact/privacy.py b/policyengine_uk/utils/aggregate_impact/privacy.py index 80a6c32e75..304795a326 100644 --- a/policyengine_uk/utils/aggregate_impact/privacy.py +++ b/policyengine_uk/utils/aggregate_impact/privacy.py @@ -13,7 +13,11 @@ (identities, the HBAI reproduction, overlap guards) or aren't run that year (the sign check of the suppressed head). A reform's costing and target change are withheld in a year they would come from a small cell -(``measure.reform_withheld``). +(``measure.reform_withheld``). The report records which conditions withheld +them (``measure.withheld_reasons``): each reason names the condition and the +component or variable it holds for, never a count, an amount or a share. A +reason that a cell is small says "fewer than 10", as a suppressed figure +already does. Deliberate exceptions: @@ -28,6 +32,14 @@ - Reform withholding counts households, not amounts: one household changing by £10,000 alongside ten changing by a penny is published. That is the same counting rule as for levels; no dominance rule is applied. +- A withheld year's ``suppressed_component_changed`` reason says that the + reform changes some record of a component the baseline suppresses, and + names it: a yes or no that can turn on that component's few records, as + the counted-once verdicts above do. The withheld figure already says that + one of the conditions holds; the reason adds which, and which suppressed + component, so a reader can tell a reform whose own change is too small to + publish from one that only touches another small cell. A published year + records no reason, so it says nothing new. Threat model. The report defends against disclosure through any combination of the figures it publishes, across any number of reports, from honest @@ -56,3 +68,45 @@ def small(count: int) -> bool: def count_text(count: int, what: str) -> str: """``"N "``, or ``"fewer than 10 "`` for a small cell.""" return f"fewer than {MIN_CELL} {what}" if small(count) else f"{count:,} {what}" + + +# Why a reform's figures for a year are withheld (measure.withheld_reasons). +NO_BASELINE = "no_baseline" +SUPPRESSED_COMPONENT_CHANGED = "suppressed_component_changed" +SMALL_COMPONENT_LEFT = "small_component_left" +SMALL_BALANCE_CHANGE = "small_balance_change" +SMALL_TARGET_CHANGE = "small_target_change" +WITHHELD_CONDITIONS = ( + NO_BASELINE, + SUPPRESSED_COMPONENT_CHANGED, + SMALL_COMPONENT_LEFT, + SMALL_BALANCE_CHANGE, + SMALL_TARGET_CHANGE, +) + + +def withheld_reason(condition: str, variable=None, metric_id=None) -> dict: + """One reason a reform year is withheld: the condition and what it holds + for. ``metric_id`` is the published id of a partition component.""" + return {"condition": condition, "variable": variable, "metric_id": metric_id} + + +def withheld_text(reasons) -> str: + """Reasons in words, with no count or amount: "fewer than 10" at most.""" + return "; ".join(_reason_text(reason) for reason in reasons) + + +def _reason_text(reason) -> str: + condition = reason.get("condition") + what = reason.get("metric_id") or reason.get("variable") + if condition == NO_BASELINE: + return "the baseline run recorded nothing for the year" + if condition == SUPPRESSED_COMPONENT_CHANGED: + return f"the reform changes a record of {what}, which the baseline suppresses" + if condition == SMALL_COMPONENT_LEFT: + return f"the reform leaves {what} with fewer than {MIN_CELL} records" + if condition == SMALL_BALANCE_CHANGE: + return f"the reform changes {what} in fewer than {MIN_CELL} households" + if condition == SMALL_TARGET_CHANGE: + return f"the reform changes {what} in fewer than {MIN_CELL} records" + return f"unrecognised condition {condition}" diff --git a/policyengine_uk/utils/aggregate_impact/render.py b/policyengine_uk/utils/aggregate_impact/render.py index 7c711dd760..88a9992366 100644 --- a/policyengine_uk/utils/aggregate_impact/render.py +++ b/policyengine_uk/utils/aggregate_impact/render.py @@ -6,6 +6,7 @@ from typing import Mapping, Optional from . import COMMENT_MARKER +from .privacy import withheld_text from .spec import Spec, fiscal_year_label SECTIONS = ( @@ -126,6 +127,43 @@ def _change_cell(row, year) -> str: return fmt_change(cell["delta"], row["kind"]) + _mark(cell) +def _spans(years) -> str: + """Years as runs of consecutive years: "2025-26, 2027-28 to 2030-31".""" + runs = [] + for year in sorted(years): + if runs and year == runs[-1][-1] + 1: + runs[-1].append(year) + else: + runs.append([year]) + return ", ".join( + fiscal_year_label(run[0]) + if len(run) == 1 + else f"{fiscal_year_label(run[0])} to {fiscal_year_label(run[-1])}" + for run in runs + ) + + +def _withheld_notes(rows, years) -> list: + """Why each side withholds a reform figure, one line per figure, reasons + and run of years (measure.withheld_reasons).""" + grouped = {} + for row in rows: + for side in ("base", "head"): + by_reasons = {} + for year in years: + cell = row["values"].get(str(year)) or {} + reasons = (cell.get("withheld_because") or {}).get(side) + if reasons: + by_reasons.setdefault(withheld_text(reasons), []).append(int(year)) + for text, withheld_years in by_reasons.items(): + key = (row["label"], text, tuple(withheld_years)) + grouped.setdefault(key, []).append(side) + return [ + f"- {label}, {_spans(withheld_years)} ({' and '.join(sides)}): {text}." + for (label, text, withheld_years), sides in grouped.items() + ] + + # A figure from fewer than 10 records is not published (privacy.py). _SUPPRESSED_CELL = "suppressed" # The sides leave out different suppressed figures, so no change is shown. @@ -474,6 +512,7 @@ def described(prov): parts += ["", "### Moves flagged for review", "", _flags(summary)] unchanged = [] + explained = False for section, title in SECTIONS: rows = _section_rows(summary, section) if not rows: @@ -482,6 +521,23 @@ def described(prov): unchanged.append(title[:1].lower() + title[1:]) continue parts += ["", f"### {title}", "", _changes_table(rows, years)] + notes = ( + _withheld_notes(rows, years) + if section in ("reforms", "reform_targets") and not explained + else [] + ) + if notes: + # Once, under the first reform table shown: a reform's costing + # and target change are withheld in the same years, for the same + # reasons. + explained = True + parts += [ + "", + "The report suppresses a reform's costing and target change for " + "a year when they would come from a small cell (`privacy.py`):", + "", + *notes, + ] parts += ["", "### Household net income by decile", "", _deciles(summary, years)] if unchanged: parts += ["", "**No change in any year:** " + "; ".join(unchanged) + "."] From 4dcc1c68b5e1940dc5c4af6248141dc75d2dfed5 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 31 Oct 2026 05:51:47 -0400 Subject: [PATCH 2/9] Add the #2654 changelog fragment and README note Co-Authored-By: Claude Opus 5.5 --- changelog.d/2654.added.md | 1 + policyengine_uk/utils/aggregate_impact/README.md | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 changelog.d/2654.added.md diff --git a/changelog.d/2654.added.md b/changelog.d/2654.added.md new file mode 100644 index 0000000000..4585a9e1f4 --- /dev/null +++ b/changelog.d/2654.added.md @@ -0,0 +1 @@ +The aggregate impact report now records why it withholds a standard reform's costing and target change in a year. For each withheld year, `withheld_because` names every condition that holds (`suppressed_component_changed`, `small_component_left`, `small_balance_change`, `small_target_change` or `no_baseline`) and the component or variable it holds for, in `result.json` and in the long-format report that qualification reads. The pull request comment lists the reasons under the reform costings. No reason gives a count or an amount. diff --git a/policyengine_uk/utils/aggregate_impact/README.md b/policyengine_uk/utils/aggregate_impact/README.md index 700fd0fe35..942bc464c9 100644 --- a/policyengine_uk/utils/aggregate_impact/README.md +++ b/policyengine_uk/utils/aggregate_impact/README.md @@ -33,7 +33,9 @@ Small cells (`privacy.py`): - A total that holds one is rebuilt from its other components and marked †. - Poverty, inequality and deciles are withheld while HBAI income holds one. - A reform's figures for a year are withheld when they would come from a - small cell. + small cell. `withheld_because` names each condition that withheld them and + the component or variable it holds for, never a count, and the comment + lists them under the reform costings. `privacy.py` also states the threat model. From 55466bfb968f76a7ef7fed8608449196dc2257ce Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 31 Oct 2026 06:11:15 -0400 Subject: [PATCH 3/9] Say that small_target_change can name another reform's target measure() checks the target of every reform it costs, as on main, so a reason names the variable; a test pins the case. Co-Authored-By: Claude Opus 5.5 --- .../test_aggregate_impact_withheld_reasons.py | 28 +++++++++++++++++++ policyengine_uk/utils/aggregate_impact/api.py | 2 ++ .../utils/aggregate_impact/measure.py | 2 ++ 3 files changed, 32 insertions(+) diff --git a/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py b/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py index 6528143282..de399cc5b4 100644 --- a/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py +++ b/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py @@ -247,6 +247,34 @@ def test_a_reform_that_changes_few_target_records_names_the_target(): ) +def test_a_small_change_in_another_reforms_target_names_that_target(): + """measure() checks every costed reform's target: a reform aimed at + income tax is withheld when it moves Pension Credit, another reform's + target, in three records.""" + world = _world(np.full(N, 100.0), np.full(N, 100.0), pension_credit=np.zeros(N)) + reformed = _world( + np.full(N, 110.0), np.full(N, 100.0), pension_credit=_first(3, 25.0) + ) + base_sim = WorldSim({YEAR: world}, np.ones(N)) + targets = ["income_tax", "pension_credit"] + rec = Recorder() + measure_reforms( + lambda reform=None: WorldSim({YEAR: reformed if reform else world}, np.ones(N)), + None, + {YEAR: weighted_total(base_sim, "gov_balance", YEAR)}, + {YEAR: {t: weighted_total(base_sim, t, YEAR) for t in targets}}, + _reform("income_tax", name="Raise income tax"), + [YEAR], + rec, + baseline={YEAR: baseline_records(base_sim, _partition(), rec, YEAR, targets)}, + components=_partition().component_variables(), + ) + reasons = rec.metrics["reforms.raise_income_tax"]["withheld_because"] + assert _conditions(reasons[str(YEAR)]) == [ + (SMALL_TARGET_CHANGE, "pension_credit", None) + ] + + def test_a_year_without_baseline_records_says_so(): rec = Recorder() reform = parse_reforms({"reforms": [{"name": "Any", "parameters": {"a": 1}}]}) diff --git a/policyengine_uk/utils/aggregate_impact/api.py b/policyengine_uk/utils/aggregate_impact/api.py index 9f239a4ffd..5ba884e21f 100644 --- a/policyengine_uk/utils/aggregate_impact/api.py +++ b/policyengine_uk/utils/aggregate_impact/api.py @@ -61,6 +61,8 @@ def to_report(result: dict) -> dict: carry ``withheld_because``: for a year the tool withholds, the conditions that withheld it, each as ``{"condition", "variable", "metric_id"}`` (``privacy.WITHHELD_CONDITIONS``); for a published year, an empty list. + A ``small_target_change`` names the variable, which can be another + reform's target: the tool checks the target of every reform it costs. """ provenance = result.get("provenance") or {} report = { diff --git a/policyengine_uk/utils/aggregate_impact/measure.py b/policyengine_uk/utils/aggregate_impact/measure.py index ea0a644054..f87c84fa70 100644 --- a/policyengine_uk/utils/aggregate_impact/measure.py +++ b/policyengine_uk/utils/aggregate_impact/measure.py @@ -1308,6 +1308,8 @@ def withheld_reasons( - changes gov_balance in 1 to 9 households (``small_balance_change``), or a target variable in 1 to 9 records (``small_target_change``), by more than ``CHANGE_GBP``: the change is theirs alone, as for a decile row. + ``measure`` checks the target of every reform it costs, so the reason + names the variable: this reform's own target or another's. Withheld too when there is no baseline record for the year (``no_baseline``). It tests every condition and gives one reason for From a9d62ce1f48c5c356d68e4033a94e077c7d74c3f Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 31 Oct 2026 06:31:06 -0400 Subject: [PATCH 4/9] State the reasons' disclosure limits and cap the comment's notes privacy.py now describes reasons about groups the reform defines, and adds a deliberate exception: an own-target small_target_change beside the published responsive verdict bounds that small cell's total. withheld_reasons says that evaluating every condition can fail a reform in a year main withheld early. The comment lists at most 40 reason lines and points to the artifact for the rest. Co-Authored-By: Claude Opus 5.5 --- .../test_aggregate_impact_withheld_reasons.py | 47 +++++++++++++++++-- policyengine_uk/utils/aggregate_impact/api.py | 8 ++-- .../utils/aggregate_impact/measure.py | 18 ++++--- .../utils/aggregate_impact/privacy.py | 28 +++++++---- .../utils/aggregate_impact/render.py | 12 +++-- 5 files changed, 85 insertions(+), 28 deletions(-) diff --git a/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py b/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py index de399cc5b4..ede4ea317c 100644 --- a/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py +++ b/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py @@ -1,11 +1,11 @@ -"""Why a reform year is withheld (#2654): one reason per condition that holds. +"""Why the tool withholds a reform year (#2654): one reason per condition. ``measure.withheld_reasons`` names each condition of reform withholding that holds, and the component or variable it holds for. These tests pin one case -per condition, two conditions together, and, for all small worlds, that a -year is withheld exactly when it has a reason, that the reasons match a -direct computation, and that the years withheld and the figures published -match main's ``reform_withheld`` (the frozen oracle in +per condition, two conditions together, and, for all small worlds, that the +tool withholds a year exactly when it records a reason, that the reasons +match a direct computation, and that the years withheld and the figures +published match main's ``reform_withheld`` (the frozen oracle in ``reform_withheld_main_reference.py``). """ @@ -491,6 +491,43 @@ def test_the_comment_tells_the_sides_apart(): ] +def test_the_comment_caps_the_reasons_it_lists(): + """25 reforms, each withheld in a different year on each side, give 50 + lines: the comment shows 40 and points to the artifact for the rest.""" + from policyengine_uk.utils.aggregate_impact.render import MAX_WITHHELD_ROWS + + def side(year, variable): + reason = [ + { + "condition": SMALL_COMPONENT_LEFT, + "variable": variable, + "metric_id": f"receipts.{variable}", + } + ] + measured = _side(Recorder()) + measured["metrics"] = [ + { + "id": f"reforms.reform_{i}", + "label": f"Reform {i}", + "section": "reforms", + "group": None, + "kind": "gbp", + "unit": None, + "values": {str(y): None if y == year else 1.0 for y in SPEC.years}, + "suppressed": [str(year)], + "withheld_because": {str(year): reason}, + } + for i in range(25) + ] + return measured + + summary = compare(side(2025, "income_tax"), side(2026, "dividend_tax"), SPEC) + text = render(summary, SPEC) + listed = [line for line in text.splitlines() if line.startswith("- Reform ")] + assert len(listed) == MAX_WITHHELD_ROWS + assert "- 10 more lines are in the report artifact." in text + + # For all small worlds. values = st.floats(1.0, 1e4, allow_nan=False) diff --git a/policyengine_uk/utils/aggregate_impact/api.py b/policyengine_uk/utils/aggregate_impact/api.py index 5ba884e21f..f0ed650c64 100644 --- a/policyengine_uk/utils/aggregate_impact/api.py +++ b/policyengine_uk/utils/aggregate_impact/api.py @@ -105,10 +105,10 @@ def to_report(result: dict) -> dict: "label": metric["label"], } if "withheld_because" in metric: - # A reform figure: why its year is withheld, one reason per - # condition and the component or variable it holds for - # (measure.withheld_reasons); empty exactly when the year is - # published. + # A reform figure: why the tool withholds its year, one + # reason per condition and the component or variable it + # holds for (measure.withheld_reasons); empty exactly when + # the tool publishes the year. row["withheld_because"] = [ dict(reason) for reason in (metric["withheld_because"].get(year) or []) diff --git a/policyengine_uk/utils/aggregate_impact/measure.py b/policyengine_uk/utils/aggregate_impact/measure.py index f87c84fa70..c2c339fe41 100644 --- a/policyengine_uk/utils/aggregate_impact/measure.py +++ b/policyengine_uk/utils/aggregate_impact/measure.py @@ -1290,10 +1290,11 @@ def baseline_records(sim, partition: Partition, rec: "Recorder", year: int, targ def withheld_reasons( reformed, year: int, baseline: Optional[Mapping], components ) -> list: - """Why a reform's costing and target change for ``year`` are withheld: - empty when they are published. + """Why the tool withholds a reform's costing and target change for + ``year``: one reason per condition that holds, none when it publishes + them. - They would be built from a small cell (Sol re-review 7 of #2335) when the + They would come from a small cell (Sol re-review 7 of #2335) when the reform: - changes any record of a component suppressed in the baseline, by any @@ -1315,7 +1316,10 @@ def withheld_reasons( (``no_baseline``). It tests every condition and gives one reason for each component or variable a condition holds for (``privacy.withheld_reason``): the reason names it and gives no count or - amount (#2654). + amount (#2654). Main's ``reform_withheld`` stopped at the first condition, + so a variable that fails to compute under the reform now fails the reform + in a year an earlier condition withholds, as it already did in every + other year. """ if baseline is None: return [withheld_reason(NO_BASELINE)] @@ -1379,10 +1383,10 @@ def _record_reasons(record: dict, year: int, reasons: list) -> None: """Keep a withheld year's reasons on a reform figure's record. Every reform figure carries ``withheld_because``; it lists exactly the - years its ``suppressed`` lists, so a year is withheld exactly when it - has a reason. + years its ``suppressed`` lists, so the tool withholds a year exactly when + it records a reason. """ - # Rebound rather than written through, as for _exclude. + # Rebinds rather than writes through, as _exclude does. record["withheld_because"] = { **(record.get("withheld_because") or {}), **({str(year): list(reasons)} if reasons else {}), diff --git a/policyengine_uk/utils/aggregate_impact/privacy.py b/policyengine_uk/utils/aggregate_impact/privacy.py index 304795a326..7c5bc6c36e 100644 --- a/policyengine_uk/utils/aggregate_impact/privacy.py +++ b/policyengine_uk/utils/aggregate_impact/privacy.py @@ -11,13 +11,15 @@ household incomes for deciles, so no published total includes it. Checks whose verdict would depend on its values leave out the households holding it (identities, the HBAI reproduction, overlap guards) or aren't run that year -(the sign check of the suppressed head). A reform's costing and target change -are withheld in a year they would come from a small cell -(``measure.reform_withheld``). The report records which conditions withheld -them (``measure.withheld_reasons``): each reason names the condition and the +(the sign check of the suppressed head). The tool withholds a reform's +costing and target change in a year they would come from a small cell +(``measure.reform_withheld``), and records which conditions withheld them +(``measure.withheld_reasons``): each reason names the condition and the component or variable it holds for, never a count, an amount or a share. A -reason that a cell is small says "fewer than 10", as a suppressed figure -already does. +reason about a group the reform defines (a component's records under the +reform, or the households or records whose balance or target it changes) +says only that the group has "fewer than 10", as a decile row's "fewer than +10 households change" does. Deliberate exceptions: @@ -40,6 +42,13 @@ component, so a reader can tell a reform whose own change is too small to publish from one that only touches another small cell. A published year records no reason, so it says nothing new. +- A withheld year's ``small_target_change`` that names the reform's own + target, beside the ``responsive:`` verdict the report publishes for + that year, says whether fewer than 10 records together moved the target by + at least the reform's minimum: a bound on that cell's total, never its + value. A ``small_balance_change`` says much the same when the target is + part of gov_balance and nothing offsets it there. The counting rule above + already publishes more (one household's £10,000 beside ten pennies). Threat model. The report defends against disclosure through any combination of the figures it publishes, across any number of reports, from honest @@ -70,7 +79,8 @@ def count_text(count: int, what: str) -> str: return f"fewer than {MIN_CELL} {what}" if small(count) else f"{count:,} {what}" -# Why a reform's figures for a year are withheld (measure.withheld_reasons). +# The conditions under which the tool withholds a reform's figures for a +# year (measure.withheld_reasons). NO_BASELINE = "no_baseline" SUPPRESSED_COMPONENT_CHANGED = "suppressed_component_changed" SMALL_COMPONENT_LEFT = "small_component_left" @@ -86,8 +96,8 @@ def count_text(count: int, what: str) -> str: def withheld_reason(condition: str, variable=None, metric_id=None) -> dict: - """One reason a reform year is withheld: the condition and what it holds - for. ``metric_id`` is the published id of a partition component.""" + """One reason the tool withholds a reform year: the condition and what it + holds for. ``metric_id`` is the published id of a partition component.""" return {"condition": condition, "variable": variable, "metric_id": metric_id} diff --git a/policyengine_uk/utils/aggregate_impact/render.py b/policyengine_uk/utils/aggregate_impact/render.py index 88a9992366..6a1f15cc42 100644 --- a/policyengine_uk/utils/aggregate_impact/render.py +++ b/policyengine_uk/utils/aggregate_impact/render.py @@ -26,6 +26,7 @@ ("reform_targets", "Standard reforms: change in each reform's target"), ) MAX_FLAG_ROWS = 40 +MAX_WITHHELD_ROWS = 40 def _finite(value) -> bool: @@ -527,10 +528,15 @@ def described(prov): else [] ) if notes: - # Once, under the first reform table shown: a reform's costing - # and target change are withheld in the same years, for the same - # reasons. + # Once, under the first reform table shown: the tool withholds a + # reform's costing and target change in the same years, for the + # same reasons. explained = True + if len(notes) > MAX_WITHHELD_ROWS: + notes = notes[:MAX_WITHHELD_ROWS] + [ + f"- {len(notes) - MAX_WITHHELD_ROWS} more lines are in the " + "report artifact." + ] parts += [ "", "The report suppresses a reform's costing and target change for " From 64c98a3da58a9669688248015a14684ad9133912 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 31 Oct 2026 06:47:59 -0400 Subject: [PATCH 5/9] Name who withholds in the new docstrings and comments Co-Authored-By: Claude Opus 5.5 --- .../reform_withheld_main_reference.py | 4 ++-- .../test_aggregate_impact_withheld_reasons.py | 16 ++++++++-------- .../utils/aggregate_impact/measure.py | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/policyengine_uk/tests/aggregate_impact/reform_withheld_main_reference.py b/policyengine_uk/tests/aggregate_impact/reform_withheld_main_reference.py index 5e5452a1b8..4892be37d5 100644 --- a/policyengine_uk/tests/aggregate_impact/reform_withheld_main_reference.py +++ b/policyengine_uk/tests/aggregate_impact/reform_withheld_main_reference.py @@ -1,8 +1,8 @@ """Frozen reform withholding from main (da31e8698), with commentary omitted. Keep this independent of the production tool: it is the differential oracle -for #2654, which records why a reform year is withheld and must not change -which years are withheld. +for #2654, which records why the tool withholds a reform year and must not +change which years it withholds. """ from typing import Mapping, Optional diff --git a/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py b/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py index ede4ea317c..552590056a 100644 --- a/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py +++ b/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py @@ -248,9 +248,9 @@ def test_a_reform_that_changes_few_target_records_names_the_target(): def test_a_small_change_in_another_reforms_target_names_that_target(): - """measure() checks every costed reform's target: a reform aimed at - income tax is withheld when it moves Pension Credit, another reform's - target, in three records.""" + """measure() checks every costed reform's target: the tool withholds a + reform aimed at income tax when it moves Pension Credit, another + reform's target, in three records.""" world = _world(np.full(N, 100.0), np.full(N, 100.0), pension_credit=np.zeros(N)) reformed = _world( np.full(N, 110.0), np.full(N, 100.0), pension_credit=_first(3, 25.0) @@ -458,7 +458,7 @@ def test_the_comment_says_why_each_reform_year_is_suppressed(): "receipts.dividend_tax with fewer than 10 records." ) assert line in text - # Once, under the costings, though the targets are withheld too. + # Once, under the costings, though the tool withholds the targets too. assert text.count("receipts.dividend_tax with fewer than 10") == 1 assert text.index(line) > text.index("### Standard reform costings") @@ -591,10 +591,10 @@ def small(count): @settings(max_examples=300, deadline=None) @given(world=worlds()) def test_a_year_is_withheld_exactly_when_it_has_a_reason(world): - """For every world: the figures are withheld exactly when a reason is - recorded; the reasons are the conditions computed directly; the years - withheld match main's reform_withheld; and the published figures are - the direct weighted changes.""" + """For every world: the tool withholds the figures exactly when it + records a reason; the reasons match the conditions computed directly; + the years withheld match main's reform_withheld; and the published + figures equal the direct weighted changes.""" base, reformed, weight, target = world rec = _measure(base, reformed, weight, target) reasons = _reasons(rec) diff --git a/policyengine_uk/utils/aggregate_impact/measure.py b/policyengine_uk/utils/aggregate_impact/measure.py index c2c339fe41..b722e9ca09 100644 --- a/policyengine_uk/utils/aggregate_impact/measure.py +++ b/policyengine_uk/utils/aggregate_impact/measure.py @@ -1374,8 +1374,8 @@ def weight(variable): def reform_withheld( reformed, year: int, baseline: Optional[Mapping], components ) -> bool: - """Whether a reform's costing and target change for ``year`` are withheld: - whether any condition of ``withheld_reasons`` holds.""" + """Whether the tool withholds a reform's costing and target change for + ``year``: whether any condition of ``withheld_reasons`` holds.""" return bool(withheld_reasons(reformed, year, baseline, components)) From 7e4e6fc7e6baaeb9013a07bc46c1ec1dc5b21da1 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 31 Oct 2026 07:50:20 -0400 Subject: [PATCH 6/9] Pin the reasons' edge cases and say what no_baseline can't tell - Test two small targets at once, a change of exactly CHANGE_GBP, a newly small double count and a target on benefit units, and add CHANGE_GBP to the Hypothesis deltas. - Qualify the numeric-text property: names such as bursary_fund_16_to_19 hold digits that count nothing; check the words around them. - Say in privacy.py that reasons across reform rows can bound a cell and that the reform list decides which rows exist, and pin that list. - Say that a no_baseline reason says nothing about the reform's records, and describe withheld_reasons as it behaves. - Name the comment's overflow count and the file and field that hold every reason. Co-Authored-By: Claude Opus 5.5 --- .../reform_withheld_main_reference.py | 6 +- .../test_aggregate_impact_withheld_reasons.py | 210 ++++++++++++++++-- policyengine_uk/utils/aggregate_impact/api.py | 3 + .../utils/aggregate_impact/measure.py | 12 +- .../utils/aggregate_impact/privacy.py | 4 + .../utils/aggregate_impact/render.py | 5 +- 6 files changed, 206 insertions(+), 34 deletions(-) diff --git a/policyengine_uk/tests/aggregate_impact/reform_withheld_main_reference.py b/policyengine_uk/tests/aggregate_impact/reform_withheld_main_reference.py index 4892be37d5..9847bd0014 100644 --- a/policyengine_uk/tests/aggregate_impact/reform_withheld_main_reference.py +++ b/policyengine_uk/tests/aggregate_impact/reform_withheld_main_reference.py @@ -1,8 +1,8 @@ -"""Frozen reform withholding from main (da31e8698), with commentary omitted. +"""Reform withholding as a yes or no, frozen at da31e8698 without commentary. Keep this independent of the production tool: it is the differential oracle -for #2654, which records why the tool withholds a reform year and must not -change which years it withholds. +for ``measure.withheld_reasons`` (#2654), which must withhold exactly the +years this rule withholds. """ from typing import Mapping, Optional diff --git a/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py b/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py index 552590056a..a1c0165209 100644 --- a/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py +++ b/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py @@ -5,7 +5,7 @@ per condition, two conditions together, and, for all small worlds, that the tool withholds a year exactly when it records a reason, that the reasons match a direct computation, and that the years withheld and the figures -published match main's ``reform_withheld`` (the frozen oracle in +published match the yes-or-no rule frozen at da31e8698 (the oracle in ``reform_withheld_main_reference.py``). """ @@ -23,6 +23,7 @@ from policyengine_uk.utils.aggregate_impact.api import to_report from policyengine_uk.utils.aggregate_impact.compare import compare from policyengine_uk.utils.aggregate_impact.measure import ( + CHANGE_GBP, Recorder, _weighted_records, baseline_records, @@ -32,7 +33,11 @@ weighted_total, withheld_reasons, ) -from policyengine_uk.utils.aggregate_impact.partition import parse_partition +from policyengine_uk.utils.aggregate_impact.partition import ( + default_partition_path, + load_partition, + parse_partition, +) from policyengine_uk.utils.aggregate_impact.privacy import ( MIN_CELL, NO_BASELINE, @@ -41,9 +46,10 @@ SMALL_TARGET_CHANGE, SUPPRESSED_COMPONENT_CHANGED, WITHHELD_CONDITIONS, + withheld_reason, withheld_text, ) -from policyengine_uk.utils.aggregate_impact.reforms import parse_reforms +from policyengine_uk.utils.aggregate_impact.reforms import load_reforms, parse_reforms from policyengine_uk.utils.aggregate_impact.render import _withheld_notes, render from policyengine_uk.utils.aggregate_impact.spec import load_spec @@ -85,6 +91,28 @@ def calculate(self, variable, period, map_to=None, unweighted=True): return np.asarray(values.get(variable, np.zeros(N)), dtype=float) +class BenunitWorldSim(WorldSim): + """A WorldSim whose Child Benefit falls on 40 benefit units, two in each + household, each weighted as its household.""" + + HOUSEHOLD = np.repeat(np.arange(N), 2) + + def __init__(self, values_by_year, weight): + super().__init__(values_by_year, weight) + self.tax_benefit_system.variables["child_benefit"] = SimpleNamespace( + entity=SimpleNamespace(key="benunit") + ) + + def calculate(self, variable, period, map_to=None, unweighted=True): + if variable == "benunit_weight": + return self.weight[self.HOUSEHOLD] + return super().calculate(variable, period, map_to, unweighted) + + def map_result(self, values, source, target): + assert (source, target) == ("benunit", "household") + return np.bincount(self.HOUSEHOLD, weights=values, minlength=N) + + def _balance(values) -> np.ndarray: zero = np.zeros(N) return ( @@ -94,7 +122,10 @@ def _balance(values) -> np.ndarray: ) -def _partition(): +def _partition(double_counts=()): + """Three heads, and any double counts named, each inside the state + pension.""" + def head(i, totals): return {"id": i, "label": i, "variable": i, "totals": totals} @@ -107,6 +138,14 @@ def head(i, totals): head("dividend_tax", ["gov_tax"]), ], "spending_heads": [head("state_pension", ["gov_spending"])], + "model_double_counts": [ + { + "variable": variable, + "contained_in": "state_pension", + "totals": ["gov_spending"], + } + for variable in double_counts + ], } ) @@ -129,18 +168,29 @@ def _reform(target="income_tax", name="Change it"): ) -def _measure(base, reformed, weight=None, target="income_tax", years=(YEAR,)): +def _measure( + base, + reformed, + weight=None, + target="income_tax", + years=(YEAR,), + others=(), + double_counts=(), + sim=WorldSim, +): """Measure one reform as measure() does: record the baseline's heads, with their suppression, then cost the reform against the baseline. ``base`` and ``reformed`` map variable to household values, or year to - such a mapping.""" + such a mapping. ``others`` are other reforms' targets, which measure() + checks too.""" if not isinstance(next(iter(base)), int): base = {year: base for year in years} reformed = {year: reformed for year in years} weight = np.ones(N) if weight is None else np.asarray(weight, dtype=float) - partition = _partition() - base_sim = WorldSim(base, weight) + partition = _partition(double_counts) + targets = [target, *others] + base_sim = sim(base, weight) rec = Recorder() for year in years: for head in partition.heads: @@ -157,15 +207,15 @@ def _measure(base, reformed, weight=None, target="income_tax", years=(YEAR,)): ) reform = _reform(target) measure_reforms( - lambda reform=None: WorldSim(reformed if reform else base, weight), + lambda reform=None: sim(reformed if reform else base, weight), None, {y: weighted_total(base_sim, "gov_balance", y) for y in years}, - {y: {target: weighted_total(base_sim, target, y)} for y in years}, + {y: {t: weighted_total(base_sim, t, y) for t in targets} for y in years}, reform, list(years), rec, baseline={ - y: baseline_records(base_sim, partition, rec, y, [target]) for y in years + y: baseline_records(base_sim, partition, rec, y, targets) for y in years }, components=partition.component_variables(), ) @@ -297,7 +347,7 @@ def test_a_year_without_baseline_records_says_so(): def test_two_conditions_together_are_both_recorded(): """The reform raises one suppressed pension and leaves dividend tax with five payers; every income tax bill changes, so neither the balance nor - the target is a small change. Main stopped at the first condition.""" + the target is a small change. The tool records both conditions.""" base = _world(np.full(N, 100.0), _first(2, 1_000.0)) reformed = _world( np.full(N, 110.0), @@ -355,6 +405,115 @@ def test_component_metric_ids_name_heads_and_double_counts(): } +def test_two_small_targets_are_both_named(): + """The reform moves Pension Credit and Child Benefit, two other reforms' + targets, in three records each; every income tax bill rises too.""" + zero = np.zeros(N) + base = _world( + np.full(N, 100.0), np.full(N, 100.0), pension_credit=zero, child_benefit=zero + ) + reformed = _world( + np.full(N, 110.0), + np.full(N, 100.0), + pension_credit=_first(3, 25.0), + child_benefit=_first(3, 10.0), + ) + rec = _measure(base, reformed, others=("pension_credit", "child_benefit")) + assert _conditions(_reasons(rec)) == [ + (SMALL_TARGET_CHANGE, "pension_credit", None), + (SMALL_TARGET_CHANGE, "child_benefit", None), + ] + + +def test_a_change_of_exactly_half_a_penny_changes_no_record(): + """Three households hold nothing until the reform pays each of them a + state pension of exactly CHANGE_GBP, which moves neither their balance + nor the target by more than CHANGE_GBP; a penny does.""" + base = _world(_first(3, 0.0, 100.0), _first(3, 0.0, 100.0), _first(3, 0.0, 50.0)) + reformed = dict(base, state_pension=_first(3, CHANGE_GBP, 100.0)) + rec = _measure(base, reformed, target="state_pension") + assert _reasons(rec) == [] + target = rec.metrics["reform_targets.change_it"]["values"][str(YEAR)] + assert target == pytest.approx(3 * CHANGE_GBP) + penny = dict(base, state_pension=_first(3, 0.01, 100.0)) + assert _conditions(_reasons(_measure(base, penny, target="state_pension"))) == [ + (SMALL_BALANCE_CHANGE, "gov_balance", None), + (SMALL_TARGET_CHANGE, "state_pension", None), + ] + + +def test_a_reform_that_leaves_a_small_double_count_names_it(): + """A double count, counted inside the state pension, falls from 20 + records to 4: a new small cell, named by its published id.""" + base = _world(np.full(N, 100.0), np.full(N, 100.0), pension_top_up=np.full(N, 5.0)) + reformed = _world( + np.full(N, 110.0), np.full(N, 100.0), pension_top_up=_first(4, 5.0) + ) + rec = _measure(base, reformed, double_counts=("pension_top_up",)) + assert _conditions(_reasons(rec)) == [ + (SMALL_COMPONENT_LEFT, "pension_top_up", "double_counts.pension_top_up") + ] + + +@pytest.mark.parametrize( + "households, withheld", + [ + # Ten weighted benefit units in five households: not a small cell. + (6, False), + # Four weighted benefit units in two households. + (3, True), + ], +) +def test_a_benefit_unit_target_counts_benefit_units(households, withheld): + """Child Benefit falls on 40 benefit units in 20 households, and the + first household carries no weight. The reform raises Child Benefit for + both benefit units in the first few households and every income tax + bill, so the tool counts the target's records with benefit unit + weights.""" + weight = _first(1, 0.0, 1.0) + base = _world(np.full(N, 100.0), np.full(N, 100.0), child_benefit=np.zeros(2 * N)) + reformed = _world( + np.full(N, 110.0), + np.full(N, 100.0), + child_benefit=np.r_[ + np.full(2 * households, 10.0), np.zeros(2 * (N - households)) + ], + ) + rec = _measure(base, reformed, weight, "child_benefit", sim=BenunitWorldSim) + reasons = _conditions(_reasons(rec)) + target = rec.metrics["reform_targets.change_it"]["values"][str(YEAR)] + if withheld: + assert reasons == [(SMALL_TARGET_CHANGE, "child_benefit", None)] + assert target is None + else: + assert reasons == [] + assert target == pytest.approx(10.0 * 2 * (households - 1)) + + +def test_reason_words_add_no_digit_to_the_names_they_quote(): + """Some component names hold digits, such as bursary_fund_16_to_19's age + range; a name counts no records. Around the names, a reason's words add + no digit but the 10 in "fewer than 10".""" + ids = component_metric_ids(load_partition(default_partition_path())) + assert "bursary_fund_16_to_19" in ids + for name in {*ids, *ids.values()}: + for condition in WITHHELD_CONDITIONS: + text = withheld_text([withheld_reason(condition, name)]) + words = text.replace(name, "").replace(f"fewer than {MIN_CELL}", "") + assert not re.search(r"\d", words), text + + +def test_privacy_py_covers_the_stock_reform_rows(): + """privacy.py's exception for reasons across reform rows rests on the + reform list in the repository: a change to that list revisits it.""" + assert " ".join(r.id for r in load_reforms()) == ( + "basic_rate_plus_1pp higher_rate_plus_1pp personal_allowance_13000 " + "child_benefit_additional_25 uc_taper_20pc employee_nics_main_10pc " + "vat_standard_plus_2pp additional_rate_plus_3pp " + "uc_standard_allowance_plus_10pc state_pension_plus_10pc" + ) + + # The issue's example: two heads suppressed in every year, a reform withheld # in 2027-28 to 2030-31 only. @@ -525,13 +684,16 @@ def side(year, variable): text = render(summary, SPEC) listed = [line for line in text.splitlines() if line.startswith("- Reform ")] assert len(listed) == MAX_WITHHELD_ROWS - assert "- 10 more lines are in the report artifact." in text + assert ( + "- 10 more left out here; the report artifact's `summary.json` gives " + "every reason under `withheld_because`." in text + ) # For all small worlds. values = st.floats(1.0, 1e4, allow_nan=False) -deltas = st.sampled_from([0.0, 0.004, 0.006, 1.0, -250.0, 1e3]) +deltas = st.sampled_from([0.0, 0.004, CHANGE_GBP, 0.006, 1.0, -250.0, 1e3]) @st.composite @@ -593,7 +755,7 @@ def small(count): def test_a_year_is_withheld_exactly_when_it_has_a_reason(world): """For every world: the tool withholds the figures exactly when it records a reason; the reasons match the conditions computed directly; - the years withheld match main's reform_withheld; and the published + the years withheld match the frozen rule; and the published figures equal the direct weighted changes.""" base, reformed, weight, target = world rec = _measure(base, reformed, weight, target) @@ -614,16 +776,16 @@ def test_a_year_is_withheld_exactly_when_it_has_a_reason(world): ) records = baseline_records(base_sim, partition, rec, YEAR, [target]) components = partition.component_variables() - on_main = main_reference.reform_withheld(reformed_sim, YEAR, records, components) - assert on_main == bool(reasons) - assert reform_withheld(reformed_sim, YEAR, records, components) == on_main + frozen = main_reference.reform_withheld(reformed_sim, YEAR, records, components) + assert frozen == bool(reasons) + assert reform_withheld(reformed_sim, YEAR, records, components) == frozen assert withheld_reasons(reformed_sim, YEAR, records, components) == reasons costing = float(np.sum(weight * (_balance(reformed) - _balance(base)))) change = float(np.sum(weight * (reformed[target] - base[target]))) published = rec.metrics["reforms.change_it"]["values"][str(YEAR)] target_change = rec.metrics["reform_targets.change_it"]["values"][str(YEAR)] - if on_main: + if frozen: assert published is None and target_change is None else: assert published == pytest.approx(costing, abs=1e-6) @@ -633,16 +795,18 @@ def test_a_year_is_withheld_exactly_when_it_has_a_reason(world): @settings(max_examples=300, deadline=None) @given(world=worlds()) def test_a_reason_never_gives_a_count_or_an_amount(world): - """A reason holds only names; in words it says "fewer than 10" at most, - whatever the counts and amounts behind it.""" + """A reason holds only names; around them, its words say "fewer than + 10" at most, whatever the counts and amounts behind it.""" base, reformed, weight, target = world rec = _measure(base, reformed, weight, target) reasons = _reasons(rec) + text = withheld_text(reasons) + words = text.replace(f"fewer than {MIN_CELL}", "") for reason in reasons: assert set(reason) == {"condition", "variable", "metric_id"} assert all(value is None or isinstance(value, str) for value in reason.values()) - text = withheld_text(reasons) - assert not re.search(r"\d", text.replace(f"fewer than {MIN_CELL}", "")) + words = words.replace(reason["metric_id"] or reason["variable"] or "", "") + assert not re.search(r"\d", words) assert "£" not in text detail = rec.checks["responsive:change_it"]["failing"].get(str(YEAR), "") if reasons: diff --git a/policyengine_uk/utils/aggregate_impact/api.py b/policyengine_uk/utils/aggregate_impact/api.py index f0ed650c64..26b5d7691a 100644 --- a/policyengine_uk/utils/aggregate_impact/api.py +++ b/policyengine_uk/utils/aggregate_impact/api.py @@ -63,6 +63,9 @@ def to_report(result: dict) -> dict: (``privacy.WITHHELD_CONDITIONS``); for a published year, an empty list. A ``small_target_change`` names the variable, which can be another reform's target: the tool checks the target of every reform it costs. + A ``no_baseline`` reason stands alone: without a baseline record the + tool tests no other condition, so the reason says nothing about the + reform's records. """ provenance = result.get("provenance") or {} report = { diff --git a/policyengine_uk/utils/aggregate_impact/measure.py b/policyengine_uk/utils/aggregate_impact/measure.py index b722e9ca09..ab2f11ee17 100644 --- a/policyengine_uk/utils/aggregate_impact/measure.py +++ b/policyengine_uk/utils/aggregate_impact/measure.py @@ -1313,13 +1313,13 @@ def withheld_reasons( names the variable: this reform's own target or another's. Withheld too when there is no baseline record for the year - (``no_baseline``). It tests every condition and gives one reason for - each component or variable a condition holds for + (``no_baseline``). With nothing to compare against, the function tests + no other condition, so that reason says nothing about how many records + the reform changes. Otherwise the function tests every condition and + gives one reason for each component or variable a condition holds for (``privacy.withheld_reason``): the reason names it and gives no count or - amount (#2654). Main's ``reform_withheld`` stopped at the first condition, - so a variable that fails to compute under the reform now fails the reform - in a year an earlier condition withholds, as it already did in every - other year. + amount (#2654). A variable that fails to compute under the reform + therefore fails the reform in every year, whichever conditions hold. """ if baseline is None: return [withheld_reason(NO_BASELINE)] diff --git a/policyengine_uk/utils/aggregate_impact/privacy.py b/policyengine_uk/utils/aggregate_impact/privacy.py index 7c5bc6c36e..d47726aa0e 100644 --- a/policyengine_uk/utils/aggregate_impact/privacy.py +++ b/policyengine_uk/utils/aggregate_impact/privacy.py @@ -49,6 +49,10 @@ value. A ``small_balance_change`` says much the same when the target is part of gov_balance and nothing offsets it there. The counting rule above already publishes more (one household's £10,000 beside ten pennies). + Reasons from several reform rows can together bound a cell, down to an + exact count when reforms change disjoint groups of a component's records + that together cover it, and the repository's reform list + (``standard_reforms.yaml``) decides which rows exist. Threat model. The report defends against disclosure through any combination of the figures it publishes, across any number of reports, from honest diff --git a/policyengine_uk/utils/aggregate_impact/render.py b/policyengine_uk/utils/aggregate_impact/render.py index 6a1f15cc42..478c38e935 100644 --- a/policyengine_uk/utils/aggregate_impact/render.py +++ b/policyengine_uk/utils/aggregate_impact/render.py @@ -534,8 +534,9 @@ def described(prov): explained = True if len(notes) > MAX_WITHHELD_ROWS: notes = notes[:MAX_WITHHELD_ROWS] + [ - f"- {len(notes) - MAX_WITHHELD_ROWS} more lines are in the " - "report artifact." + f"- {len(notes) - MAX_WITHHELD_ROWS} more left out here; the " + "report artifact's `summary.json` gives every reason under " + "`withheld_because`." ] parts += [ "", From 4c9b70c7bbbf43617d64f47cc97f90fec831af69 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 31 Oct 2026 07:55:28 -0400 Subject: [PATCH 7/9] Show that no_baseline alone says nothing about the reform's records The no-baseline test now changes one household's income tax and still records only no_baseline, and withheld_text's docstring says that names can hold digits. Co-Authored-By: Claude Opus 5.5 --- .../test_aggregate_impact_withheld_reasons.py | 6 +++++- policyengine_uk/utils/aggregate_impact/privacy.py | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py b/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py index a1c0165209..8fc3edc348 100644 --- a/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py +++ b/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py @@ -326,11 +326,15 @@ def test_a_small_change_in_another_reforms_target_names_that_target(): def test_a_year_without_baseline_records_says_so(): + """The reform changes one household's income tax, a small change, but + with no baseline record the tool tests nothing else: no_baseline alone + says nothing about how many records the reform changes.""" rec = Recorder() reform = parse_reforms({"reforms": [{"name": "Any", "parameters": {"a": 1}}]}) world = {y: _world(np.full(N, 1.0), np.full(N, 1.0)) for y in (2025, 2026)} + reformed = {y: dict(w, income_tax=_first(1, 2.0, 1.0)) for y, w in world.items()} measure_reforms( - lambda reform=None: WorldSim(world, np.ones(N)), + lambda reform=None: WorldSim(reformed if reform else world, np.ones(N)), None, {2025: 0.0, 2026: 0.0}, {}, diff --git a/policyengine_uk/utils/aggregate_impact/privacy.py b/policyengine_uk/utils/aggregate_impact/privacy.py index d47726aa0e..44068d46c6 100644 --- a/policyengine_uk/utils/aggregate_impact/privacy.py +++ b/policyengine_uk/utils/aggregate_impact/privacy.py @@ -106,7 +106,9 @@ def withheld_reason(condition: str, variable=None, metric_id=None) -> dict: def withheld_text(reasons) -> str: - """Reasons in words, with no count or amount: "fewer than 10" at most.""" + """Reasons in words, with no count or amount: around the names they + quote, which can hold digits (``bursary_fund_16_to_19``) that count + nothing, "fewer than 10" at most.""" return "; ".join(_reason_text(reason) for reason in reasons) From d1882db3439e3feaf81500aef5c3d7b6b616a10a Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 31 Oct 2026 08:33:49 -0400 Subject: [PATCH 8/9] Count benefit units by their weights, and state the exact-count bound - Add a case that weighted and unweighted benefit-unit counts tell apart: eight weighted of ten changed. - privacy.py: reasons across reform rows can pin an exact count when as many separate groups as a published count's records each change; in CI the head's standard_reforms.yaml decides which rows exist. - A variable that fails in any year fails the reform; the comment's overflow line names who gives the rest. Co-Authored-By: Claude Opus 5.5 --- .../test_aggregate_impact_withheld_reasons.py | 10 ++++++---- policyengine_uk/utils/aggregate_impact/measure.py | 5 +++-- policyengine_uk/utils/aggregate_impact/privacy.py | 9 +++++---- policyengine_uk/utils/aggregate_impact/render.py | 4 ++-- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py b/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py index 8fc3edc348..d3f83929d8 100644 --- a/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py +++ b/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py @@ -462,9 +462,11 @@ def test_a_reform_that_leaves_a_small_double_count_names_it(): @pytest.mark.parametrize( "households, withheld", [ - # Ten weighted benefit units in five households: not a small cell. + # Ten weighted benefit units of 12, in five households: not small. (6, False), - # Four weighted benefit units in two households. + # Eight weighted benefit units of 10, in four households. + (5, True), + # Four weighted benefit units of six, in two households. (3, True), ], ) @@ -689,8 +691,8 @@ def side(year, variable): listed = [line for line in text.splitlines() if line.startswith("- Reform ")] assert len(listed) == MAX_WITHHELD_ROWS assert ( - "- 10 more left out here; the report artifact's `summary.json` gives " - "every reason under `withheld_because`." in text + "- The report artifact's `summary.json` gives these and 10 more under " + "`withheld_because`." in text ) diff --git a/policyengine_uk/utils/aggregate_impact/measure.py b/policyengine_uk/utils/aggregate_impact/measure.py index ab2f11ee17..f2e32e5ae6 100644 --- a/policyengine_uk/utils/aggregate_impact/measure.py +++ b/policyengine_uk/utils/aggregate_impact/measure.py @@ -1318,8 +1318,9 @@ def withheld_reasons( the reform changes. Otherwise the function tests every condition and gives one reason for each component or variable a condition holds for (``privacy.withheld_reason``): the reason names it and gives no count or - amount (#2654). A variable that fails to compute under the reform - therefore fails the reform in every year, whichever conditions hold. + amount (#2654). A variable that fails to compute under the reform in + any year therefore fails the reform, whichever conditions hold that + year. """ if baseline is None: return [withheld_reason(NO_BASELINE)] diff --git a/policyengine_uk/utils/aggregate_impact/privacy.py b/policyengine_uk/utils/aggregate_impact/privacy.py index 44068d46c6..85484f8ad6 100644 --- a/policyengine_uk/utils/aggregate_impact/privacy.py +++ b/policyengine_uk/utils/aggregate_impact/privacy.py @@ -49,10 +49,11 @@ value. A ``small_balance_change`` says much the same when the target is part of gov_balance and nothing offsets it there. The counting rule above already publishes more (one household's £10,000 beside ten pennies). - Reasons from several reform rows can together bound a cell, down to an - exact count when reforms change disjoint groups of a component's records - that together cover it, and the repository's reform list - (``standard_reforms.yaml``) decides which rows exist. + Reasons from several reform rows can together bound a cell, even to an + exact count (ten reforms that each change a separate group of the ten + records behind a published count of 10 show one record in each group), + and the reform list, in CI the head's ``standard_reforms.yaml``, decides + which rows exist. Threat model. The report defends against disclosure through any combination of the figures it publishes, across any number of reports, from honest diff --git a/policyengine_uk/utils/aggregate_impact/render.py b/policyengine_uk/utils/aggregate_impact/render.py index 478c38e935..8754881725 100644 --- a/policyengine_uk/utils/aggregate_impact/render.py +++ b/policyengine_uk/utils/aggregate_impact/render.py @@ -534,8 +534,8 @@ def described(prov): explained = True if len(notes) > MAX_WITHHELD_ROWS: notes = notes[:MAX_WITHHELD_ROWS] + [ - f"- {len(notes) - MAX_WITHHELD_ROWS} more left out here; the " - "report artifact's `summary.json` gives every reason under " + "- The report artifact's `summary.json` gives these and " + f"{len(notes) - MAX_WITHHELD_ROWS} more under " "`withheld_because`." ] parts += [ From f04147d87c5e9bfb4a0774ec09d00a1a4ba02b5f Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 31 Oct 2026 12:56:12 -0400 Subject: [PATCH 9/9] Introduce the report's withheld notes as reasons, not small cells A no_baseline note has no small cell behind it. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/utils/aggregate_impact/render.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/policyengine_uk/utils/aggregate_impact/render.py b/policyengine_uk/utils/aggregate_impact/render.py index 8754881725..5ab81b23fa 100644 --- a/policyengine_uk/utils/aggregate_impact/render.py +++ b/policyengine_uk/utils/aggregate_impact/render.py @@ -540,8 +540,8 @@ def described(prov): ] parts += [ "", - "The report suppresses a reform's costing and target change for " - "a year when they would come from a small cell (`privacy.py`):", + "The report suppresses a reform's costing and target change in " + "these years, for the reasons below (`privacy.py`):", "", *notes, ]