From c5d8abfb44ac000c4c73a2a120a2b9b734aef0b0 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 17:24:14 -0400 Subject: [PATCH 01/21] Allow explicit SPM calculator post-release consumers --- changelog.d/spm-calculator-postrelease-compat.fixed.md | 1 + pyproject.toml | 2 +- uv.lock | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 changelog.d/spm-calculator-postrelease-compat.fixed.md diff --git a/changelog.d/spm-calculator-postrelease-compat.fixed.md b/changelog.d/spm-calculator-postrelease-compat.fixed.md new file mode 100644 index 00000000000..3cb21112c23 --- /dev/null +++ b/changelog.d/spm-calculator-postrelease-compat.fixed.md @@ -0,0 +1 @@ +Allow spm-calculator 1.0.0.post1 alongside 1.0.0 so explicitly pinned development consumers can resolve either version; retain the default release lock at 1.0.0. diff --git a/pyproject.toml b/pyproject.toml index 9d8803a0fe4..445483baaf3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,7 +24,7 @@ dependencies = [ # extension and is available on every supported Python version. "pandas>=3.0", "policyengine-core>=3.30.1", - "spm-calculator==1.0.0", + "spm-calculator>=1.0.0,<=1.0.0.post1", "tables>=3.9", "tqdm>=4.67.1", ] diff --git a/uv.lock b/uv.lock index 5d86fcc7c44..33b973e6ab0 100644 --- a/uv.lock +++ b/uv.lock @@ -1763,7 +1763,7 @@ requires-dist = [ { name = "policyengine-core", specifier = ">=3.30.1" }, { name = "ruff", marker = "extra == 'dev'", specifier = ">=0.9.0" }, { name = "setuptools", marker = "extra == 'dev'", specifier = ">=80.9.0" }, - { name = "spm-calculator", specifier = "==1.0.0" }, + { name = "spm-calculator", specifier = ">=1.0.0,<=1.0.0.post1" }, { name = "tables", specifier = ">=3.9" }, { name = "towncrier", marker = "extra == 'dev'", specifier = ">=24.8.0" }, { name = "tqdm", specifier = ">=4.67.1" }, From 437394de07b1aec965a0c23e6f981d6dfae9de44 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 19:14:10 -0400 Subject: [PATCH 02/21] Start unified SPM candidate progress log Co-Authored-By: Claude Fable 5.1 --- PROGRESS.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 PROGRESS.md diff --git a/PROGRESS.md b/PROGRESS.md new file mode 100644 index 00000000000..fbaf2ba9684 --- /dev/null +++ b/PROGRESS.md @@ -0,0 +1,29 @@ +# PROGRESS — unified SPM candidate (max/spm-unified-candidate-20260914) + +## State +Base: `upstream/main` @ `2cc6f045ab1f389bc781ffd9f5e874055b3c0a3d` (version 2.1.0). +Worktree: `/Users/maxghenis/spm-rebuild-20260908/worktrees/policyengine-us-spm-unified-20260914`. + +Verified PR heads (2026-09-14): +| PR | head | base merge-base | +|---|---|---| +| #9466 | `9f9510b735b3b9b2e375854d66fd315202e31d35` | 2cc6f045ab | +| #9463 | `d8cbd4b7a0b292beafad37cfc72813e1d710fc3e` (lane 20260914-144927 still RUNNING — re-verify) | 2cc6f045ab | +| #9448 | `8f74a5152d1e52c33bd7a7cba6340346bd7fe004` | 2ba7daa30e | +| #9464 | `cafcd9443df7f638a5ad547a92c6e0bcfd784fbb` | 4d34e3d1ab | +| #9462 | `47e94fa491bbf65ae4b6bd5667f054445b26de36` | deliberately excluded | + +## Done +- [x] Read reconciliation memo, FINAL-REPORT, EXPANDED-REVIEW, FINDINGS-RECOVERY +- [x] Worktree + branch from `upstream/main` +- [x] Step 2a: #9466 cherry-picked as-is (`c5d8abfb44`) + +## Next +- [ ] Poll lane 20260914-144927-spm-fix-isolation-202 until not RUNNING; re-verify #9463 head +- [ ] Step 2b: #9463 isolation mechanism + county FIPS + docs + 16 regression tests (drop PROGRESS.md, lane-bookkeeping pinned_tbs.py) +- [ ] Step 2c: #9448 reduced (CPUC exclusion, general/CBO/Tlaib housing switch, release tooling, policy-family + source-input contract tests) +- [ ] Step 2d: #9464 reduced (alias guard, dataset digest, batch exit shim, citations) +- [ ] Step 3: EXPANDED-REVIEW R1/R2 fixes; R8 FERA note +- [ ] Step 4: verification suites + exit codes +- [ ] Step 5: push, draft PR, CI +- [ ] REMOVE PROGRESS.md before final push From a6f1e8da146744c78c663468ce2a761b62d99f90 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 19:29:05 -0400 Subject: [PATCH 03/21] Isolate shared policy from later reforms and repair clone routing Carries PR #9463 (head d8cbd4b7a0b292beafad37cfc72813e1d710fc3e) as-is: detach_parameters, _rebind_method_aliases, bind_private_entities and isolate_parameter_tracing in policyengine_us/spm.py, the county FIPS text contract, the microsimulation documentation correction, and all 28 regression tests (21 isolation plus 7 county). The tools/pinned_tbs.py cache key gains the parameter tree because a shared-policy system now keeps its identity while detaching its tree; test_pinned_systems_are_rebuilt_when_their_source_tree_detaches covers it and the New York EITC and CTC variables consume it, so it is production code rather than lane bookkeeping. PR #9463's PROGRESS.md is not carried. Co-Authored-By: Claude Fable 5.1 --- changelog.d/spm-runtime-isolation.fixed.md | 1 + docs/usage/microsimulation.md | 25 +- policyengine_us/spm.py | 598 +++++++++++++++++- .../core/test_spm_simulation_isolation.py | 488 ++++++++++++++ policyengine_us/tests/core/test_spm_system.py | 141 ++++- .../test_extend_single_year_dataset.py | 2 + .../tests/microsimulation/test_microsim.py | 17 +- policyengine_us/tools/pinned_tbs.py | 18 +- 8 files changed, 1237 insertions(+), 53 deletions(-) create mode 100644 changelog.d/spm-runtime-isolation.fixed.md create mode 100644 policyengine_us/tests/core/test_spm_simulation_isolation.py diff --git a/changelog.d/spm-runtime-isolation.fixed.md b/changelog.d/spm-runtime-isolation.fixed.md new file mode 100644 index 00000000000..2b07dcf1c74 --- /dev/null +++ b/changelog.d/spm-runtime-isolation.fixed.md @@ -0,0 +1 @@ +Isolate one simulation's reform, clone and trace from every other simulation: a parameter reform applied after construction no longer changes unrelated simulations, a clone's `calc` and `df` aliases calculate on the clone rather than the original, `set_input` finds a variable a reform added, each traced request records its own parameter accesses, and a `county_fips` input that is not a five-digit string is rejected however it is spelled. diff --git a/docs/usage/microsimulation.md b/docs/usage/microsimulation.md index 95db177874a..c472d8e8fdd 100644 --- a/docs/usage/microsimulation.md +++ b/docs/usage/microsimulation.md @@ -110,12 +110,16 @@ formula-owned SPM outputs such as `spm_unit_spm_threshold`. Any observed Census measurement is retained under a separate report-only name. The legacy files under `hf://policyengine/policyengine-us-data/` predate that -contract, so resource and poverty outputs are not available over them: +contract, so SPM measurements are not available over them: - `cps_2023.h5` stores `spm_unit_spm_threshold`, so the loader rejects it. - `enhanced_cps_2024.h5` loads and computes tax variables, but carries no SPM - independence roles, so SPM units consisting of one 15-to-17-year-old classify - no measurement adult and every resource output over the file fails closed. + independence roles, so 18 of its SPM units - each a lone 15-to-17-year-old - + classify no measurement adult, and `spm_unit_spm_threshold` raises + `SPM_COMPOSITION_REQUIRED` over the file. That is what + `test_legacy_enhanced_cps_lacks_source_backed_spm_independence_roles` + checks. Outputs that no longer reach the threshold, household net income + and benefits among them, do compute over the file. A household simulation that has no county input can select an SPM area explicitly instead: @@ -205,12 +209,13 @@ When running microsimulations, verify that weights produce sensible population t ```python sim = Microsimulation() -# Check population -person_weight = sim.calc("person_weight", map_to="person") +# Check population. The weights are the values here, so read them +# unweighted: a weighted sum would square them. +person_weight = sim.calc("person_weight", map_to="person", use_weights=False) print(f"Total population: {person_weight.sum():,.0f}") # Check household count -household_weight = sim.calc("household_weight") +household_weight = sim.calc("household_weight", use_weights=False) print(f"Total households: {household_weight.sum():,.0f}") # Verify key aggregates against published statistics @@ -221,6 +226,14 @@ total_snap = sim.calc("snap").sum() print(f"Total SNAP benefits: ${total_snap / 1e9:.1f}B") ``` +`calc` returns a weighted series whose `sum()` multiplies each value by its +weight, which is what makes the aggregates above population totals. Summing a +weight variable that way squares the weights: with household weights of 2 and +3, `sim.calc("household_weight").sum()` is 13 rather than 5. Either opt out +with `use_weights=False`, as above, or use `count()`, which sums the weights +themselves - `sim.calc("age", map_to="person").count()` is the same population +total. + Compare these totals against official statistics to validate your analysis: - US population: ~330 million - US households: ~130 million diff --git a/policyengine_us/spm.py b/policyengine_us/spm.py index bedc086632e..8a276d7b062 100644 --- a/policyengine_us/spm.py +++ b/policyengine_us/spm.py @@ -7,10 +7,12 @@ import re from collections.abc import Mapping +from contextlib import contextmanager from copy import copy, deepcopy from functools import lru_cache -from inspect import signature +from inspect import ismethod, signature +from policyengine_core import periods as periods_ from policyengine_core.simulations import Simulation as CoreSimulation from policyengine_core.taxbenefitsystems import TaxBenefitSystem from spm_calculator.errors import SPMInputError @@ -41,19 +43,28 @@ def is_county_fips(value): - """Accept only a five-digit county FIPS code. - - Integers, pandas missing values and truncated codes all arrive here as - strings, because ``county_fips`` is a string variable and the model casts - every input to its own dtype. A within-state CPS code such as ``5`` and a - missing value such as ``nan`` are not county FIPS codes, so they must fail - as an absent county rather than as an unrecognised one. + """Accept only a five-digit county FIPS code, supplied as text. + + The advertised contract is a five-digit *string*. Stringifying whatever + arrived enforced the digit count alone, so an integer whose decimal form + happens to be five digits (``36061``) passed, while the same mistake for a + state whose code carries a leading zero (``6037``, meaning Los Angeles + County ``06037``) failed - one silently accepted, the other reported as an + absent county. Reject every non-text value here instead, so an integer + county code, a pandas missing value and a truncated code all fail the same + way and name the same fix. + + ``numpy.str_`` and ``numpy.bytes_`` subclass ``str`` and ``bytes``, so a + county read back out of the model - which stores ``county_fips`` as text - + is still accepted. A within-state CPS code such as ``5`` and a missing + value such as ``nan`` fail as an absent county rather than an unrecognised + one, whichever type they arrive as. """ - if value is None: - return False if isinstance(value, bytes): value = value.decode() - return COUNTY_FIPS_PATTERN.fullmatch(str(value)) is not None + if not isinstance(value, str): + return False + return COUNTY_FIPS_PATTERN.fullmatch(value) is not None class CountyRequiringSPMProvider(PolicyEngineSPMProvider): @@ -65,15 +76,85 @@ class CountyRequiringSPMProvider(PolicyEngineSPMProvider): request sending an integer or a missing value, would otherwise be told its county assignment is unavailable, which points at the artifact instead of at the input. + + This provider also remembers which counties the model was handed as + something other than text. ``county_fips`` declares ``value_type = str``, + but core maps ``str`` to the numpy ``object`` dtype, so the model stores + whatever it is given and an integer column stays integers - and both + readers of that column stringify before calling a provider. An integer + county code for a state without a leading zero therefore arrived here + indistinguishable from the same code sent correctly as a string, and was + silently accepted, while the same mistake for California (``6037`` for Los + Angeles County ``06037``) was reported as an absent county. Recording the + input's type at the one point that still sees it, and rejecting it when a + county measurement actually needs that county, makes the two fail the same + way without demanding SPM geography from a caller who never asks for it. """ - def calculate_unit(self, *, year, adults, children, tenure, county_fips=None): - if self.geography_kind == "county" and not is_county_fips(county_fips): + def __post_init__(self): + super().__post_init__() + # The base is a frozen dataclass, so this receipt of input types is + # attached rather than declared. It maps (year, county text) to the + # repr of what was actually supplied, and ``record_county_input_types`` + # replaces a year's entries outright, so correcting an input clears it. + object.__setattr__(self, "_untyped_counties", {}) + + def snapshot(self, *, copy_receipts=False): + snapshot = super().snapshot(copy_receipts=copy_receipts) + if copy_receipts: + # A new simulation re-reads its own inputs; only a clone, which + # keeps this simulation's holders, inherits what they held. + snapshot._untyped_counties.update(self._untyped_counties) + return snapshot + + def record_county_input_types(self, year, values): + """Replace this year's record of counties not supplied as text. + + Replacing rather than accumulating is what lets a caller correct the + input: re-setting ``county_fips`` for a period re-derives the record + for that period from what the model now holds. + """ + if self.geography_kind != "county": + # Nothing will ask this provider for a county. + return + untyped = self._untyped_counties + for key in [key for key in untyped if key[0] == year]: + del untyped[key] + for value in values: + if not isinstance(value, (str, bytes)): + untyped[(year, str(value))] = repr(value) + + def require_county_input(self, year, county_fips): + """Reject a county this provider cannot honour as a five-digit string.""" + if self.geography_kind != "county": + return + if is_county_fips(county_fips): + supplied = self._untyped_counties.get((year, county_fips)) + if supplied is None: + return + # The county reaching a provider has already been stringified by + # the variable that read the column, so this is as precise as the + # rejection gets: every request for this county in this year + # fails, not only the household whose input was mistyped. raise SPMInputError( "SPM_GEOGRAPHY_REQUIRED", - f"County selection has no county FIPS input ({county_fips!r}): " - f"{COUNTY_INPUT_FIX}", + f"County {county_fips} was supplied for {year} as {supplied}, " + f"not as text: {COUNTY_INPUT_FIX}", ) + raise SPMInputError( + "SPM_GEOGRAPHY_REQUIRED", + f"County selection has no county FIPS input ({county_fips!r}): " + f"{COUNTY_INPUT_FIX}", + ) + + def _amounts(self, year, adults, children, tenure, county): + # Check before the memo, so a cache entry filled by a correctly typed + # row cannot let an identically spelled untyped row through. + self.require_county_input(year, county) + return super()._amounts(year, adults, children, tenure, county) + + def calculate_unit(self, *, year, adults, children, tenure, county_fips=None): + self.require_county_input(year, county_fips) return super().calculate_unit( year=year, adults=adults, @@ -181,20 +262,305 @@ def spm_config(provider): } +SHARED_TREE_MARK = "_spm_tree_is_shared" + + +def mark_tree_shared(tree): + """Record that more than one system can now read this parameter tree. + + Ownership is tracked on the tree rather than on the systems reading it, + because a system's own history says nothing about who else holds the tree + it is pointing at: a system that took a private copy, and then lent it to a + second simulation, must stop writing to that copy in place. The mark is + one-way. Dropping it would need to know when the last other reader went + away, and a tree wrongly believed private is silently shared state, while a + tree wrongly believed shared only costs one clone. + """ + if tree is not None: + setattr(tree, SHARED_TREE_MARK, True) + return tree + + +def unmark_tree_shared(tree): + """Record that this tree has exactly one reader. + + Core's ``ParameterNode.clone`` copies the node's instance dictionary, so a + clone taken from a shared tree arrives carrying the mark even though it is + brand new and private. + """ + if tree is not None: + setattr(tree, SHARED_TREE_MARK, False) + return tree + + +def tree_is_shared(tree): + return tree is not None and getattr(tree, SHARED_TREE_MARK, False) + + +SHARED_POLICY_BASES = {} + + +def shared_policy_class(base): + """Return the shared-parameter subclass of ``base``, creating it once. + + ``share_spm_policy`` hands back a system whose parameter tree belongs to + someone else, so the tree needs a read barrier that a plain attribute + cannot provide. Subclassing keeps that barrier off every other system: an + independently built ``CountryTaxBenefitSystem`` is untouched. + """ + if getattr(base, "shared_policy_base", None) is not None: + # Already a shared-policy class: sharing a shared system is idempotent. + return base + created = SHARED_POLICY_BASES.get(base) + if created is None: + name = f"SharedParameter{base.__name__}" + if name in globals(): + # Two bases sharing a class name would otherwise publish one class + # under the other's name, and pickle would restore the wrong one. + name = f"{name}{len(SHARED_POLICY_BASES) + 1}" + created = type( + name, (SharedParameterPolicy, base), {"shared_policy_base": base} + ) + # Publish it under its own name so a system built from it still pickles: + # a class only reachable from a dictionary has no import path. + created.__module__ = __name__ + created.__qualname__ = name + globals()[name] = created + SHARED_POLICY_BASES[base] = created + return created + + +class SharedParameterPolicy: + """Policy state that borrows another system's parameter tree. + + Reading parameters through this system costs nothing: the tree and its warm + at-instant caches are the lending system's own. Writing to it must not + happen at all, because core applies a reform to whatever tree the system + hands it - ``Simulation.apply_reform`` calls ``reform.apply(system)`` + directly, bypassing ``Reform.__init__``'s defensive clone - and a mutated + shared tree changes unrelated simulations, including ones already built. + + So the tree is copy-on-write. Reform application arms the read barrier; + the first read of ``parameters`` inside that window takes a private clone + through core, leaving the lending system's tree and warm caches untouched. + Cloning rebuilds every node of a 130,000-parameter tree and takes seconds, + which is why it is deferred to the reforms that actually reach the tree: + the structural reform that every simulation re-applies at its own start + instant only rebinds variables, so it never reads ``parameters`` and never + pays for a clone. + """ + + shared_policy_base = None + + @property + def parameters(self): + if self.__dict__.get("detaching_shared_parameters", False): + self.detach_parameters() + return self.__dict__["shared_parameters"] + + @parameters.setter + def parameters(self, value): + self.__dict__["shared_parameters"] = value + + @property + def shares_parameters(self): + """Whether anything but this system can see writes to this tree.""" + return tree_is_shared(self.__dict__["shared_parameters"]) + + @property + def detaching_shared_parameters(self): + """Whether a reform is being applied to this system right now.""" + return self.__dict__.get("detaching_shared_parameters", False) + + def detach_parameters(self): + """Take a private copy of the shared tree; report whether one was made.""" + tree = self.__dict__["shared_parameters"] + if tree is None or not tree_is_shared(tree): + return False + # Clear the barrier first: cloning reads the tree through core, and + # that read must not re-enter this method. + self.__dict__["detaching_shared_parameters"] = False + self.__dict__["shared_parameters"] = unmark_tree_shared(tree.clone()) + # The lender keeps its warm at-instant cache; this system starts cold + # because its parameter values are about to differ. + self._parameters_at_instant_cache = {} + return True + + @contextmanager + def detaching_parameters(self): + """Arm the copy-on-write barrier for the duration of a reform.""" + previous = self.__dict__.get("detaching_shared_parameters", False) + self.__dict__["detaching_shared_parameters"] = True + try: + yield + finally: + self.__dict__["detaching_shared_parameters"] = previous + + def apply_reform_set(self, reform): + # Core applies a reform to this system's own tree, so detach first + # however the reform arrives. + with self.detaching_parameters(): + return super().apply_reform_set(reform) + + def modify_parameters(self, modifier_function): + # The documented way for a reform to edit parameters, and core hands + # the live tree to the modifier rather than a copy. + self.detach_parameters() + return super().modify_parameters(modifier_function) + + def load_extension(self, extension): + # Core merges the extension's parameters into this tree in place. + self.detach_parameters() + return super().load_extension(extension) + + def add_abolition_parameters(self): + # Core adds an abolition child per variable, in place. + self.detach_parameters() + return super().add_abolition_parameters() + + def clone(self): + # Core's clone writes the cloned tree into the new instance's + # dictionary, where this class's property shadows it, so hand it an + # ordinary instance and let the base decide how to clone. + return type(self).shared_policy_base.clone(self.unshared_copy()) + + def unshared_copy(self): + """A plain shallow copy holding the tree as an ordinary attribute. + + Core's ``TaxBenefitSystem.clone`` writes the cloned tree straight into + the new instance's ``__dict__`` under ``parameters``, which this class's + property would shadow, so anything that clones through core starts from + an ordinary instance instead. + """ + policy = copy(self) + tree = policy.__dict__.pop("shared_parameters", None) + policy.__dict__.pop("shares_parameters_with_lender", None) + policy.__dict__.pop("detaching_shared_parameters", None) + policy.__class__ = type(self).shared_policy_base + policy.parameters = tree + return policy + + +def plain_policy_copy(system): + """Shallow-copy any system into one that holds its tree as an attribute.""" + if isinstance(system, SharedParameterPolicy): + return system.unshared_copy() + return copy(system) + + +def with_parameter_barrier(system): + """Give ``system`` a copy-on-write parameter tree, in place. + + Every simulation's own system carries the barrier, not only the ones that + borrow the shipped policy, because the tree a simulation starts out owning + can acquire other readers later - a branch copies the system, and another + simulation can be built on it - and from then on a reform must clone + before it writes. Whether anyone else is reading is recorded on the tree + (see :func:`mark_tree_shared`), so installing the barrier costs nothing + and changes nothing on its own. + """ + if isinstance(system, SharedParameterPolicy): + return system + system.__class__ = shared_policy_class(type(system)) + system.__dict__["shared_parameters"] = system.__dict__.pop("parameters", None) + system.__dict__["detaching_shared_parameters"] = False + return system + + +def bind_private_entities(policy): + """Give ``policy`` its own Entity objects, bound to its own registry. + + An Entity resolves variable names through the system it is bound to + (``Entity.get_variable``), and a population asks its entity before reading + or creating a holder. Sharing the lending system's entities therefore sent + every holder lookup to the lending system's registry, so a variable that a + reform added to this system's private registry was invisible to + ``set_input``, which raised ``VariableNotFoundError`` for a variable the + system plainly had. + + Core's own constructor and ``clone`` copy entities for the same reason. + Unlike ``clone``, keep ``person_entity`` and ``group_entities`` pointing at + the very objects in ``entities``, so there is exactly one entity object per + key to rebind. + """ + entities = [copy(entity) for entity in policy.entities] + by_key = {entity.key: entity for entity in entities} + policy.entities = entities + policy.person_entity = by_key[policy.person_entity.key] + policy.group_entities = [by_key[entity.key] for entity in policy.group_entities] + policy.group_entity_keys = [entity.key for entity in policy.group_entities] + for entity in entities: + entity.set_tax_benefit_system(policy) + return policy + + +def isolate_parameter_tracing(system, tracer, branch_name): + """Give ``system`` its own root parameter node, primed for ``tracer``. + + Core marks a traced request by writing that request's tracer, trace flag + and branch name onto the root node of the parameter tree, and the root then + caches the resulting ``TracingParameterNodeAtInstant`` in its own + at-instant cache. On a shared tree the second request reuses the first + request's cached tracing node, so its parameter accesses are recorded + against the first request's tracer - or, once that request's trace frame + has closed, recorded nowhere at all - and the shared tree is left holding a + finished request's tracer for every simulation that reads it afterwards. + + Only the root carries those fields: children are wrapped on the fly from + the root's tracing node and cache plain at-instant nodes. A shallow copy of + the root - the same children, its own at-instant cache and its own trace + fields - therefore isolates a request's parameter receipts completely, and + costs microseconds rather than the seconds a full tree clone takes. The + children stay shared, so a later reform still detaches the whole tree. + + Prime the copy as traced rather than waiting for core to mark it. Core sets + those fields in ``_run_formula``, but ``_calculate`` reads + ``parameters(period).gov.abolitions`` first, so the at-instant node for + each period is built and cached before the tree is ever marked as traced + and no parameter access is recorded at all. + """ + root = system.parameters + if root is None: + return None + if tree_is_shared(root): + private = copy(root) + private._at_instant_cache = {} + # The copy shares the original's children, and each child's parent + # still points at the original root, so an edit made through a child + # would clear the original's at-instant cache and leave this one + # stale. Mark the copy shared: any write detaches a real clone first. + mark_tree_shared(private) + system.parameters = private + system._parameters_at_instant_cache = {} + root = private + else: + # Nobody else reads this tree, so it can be marked traced where it + # stands - and leaving it in place keeps each parameter's parent + # pointing at the root whose at-instant cache an edit has to clear. + root._at_instant_cache = {} + system._parameters_at_instant_cache = {} + root.trace = True + root.tracer = tracer + root.branch_name = branch_name + return root + + def share_spm_policy(system): """Isolate receipts and variable registration without rebuilding policy. - An ordinary simulation applies no user reform, so it needs private receipts - and a private variable registry, not a private copy of the policy itself. - It is also new rather than cloned, so no previous receipt belongs to it. - Core's TaxBenefitSystem.clone() rebuilds the whole parameter tree node by - node and empties both at-instant caches, and this country then deep-copies - every variable object on top; doing that per household simulation throws - away the shared instance's warm parameter caches and lands on household API - request latency. - - Share the parameter tree and its at-instant caches, and reuse the variable - objects. Every core operation that a reform performs on a variable + An ordinary simulation applies no user reform, so it needs private + receipts, a private variable registry and private entities to resolve that + registry - not a private copy of the policy itself. It is also new rather + than cloned, so no previous receipt belongs to it. + + Core's ``TaxBenefitSystem.clone()`` rebuilds the whole parameter tree node + by node and empties both at-instant caches, and this country then + deep-copies every variable object on top; doing that per household + simulation throws away the shared instance's warm parameter caches and + lands on household API request latency. + + So share the parameter tree and its at-instant caches, and reuse the + variable objects. Every core operation that a reform performs on a variable (add_variable, replace_variable, update_variable, neutralize_variable, annualize_variable) rebinds ``variables[name]`` to a newly constructed object rather than mutating the registered one, so a private dict is enough @@ -202,9 +568,18 @@ def share_spm_policy(system): re-applied at its own start instant - out of the shared instance. ``test_ordinary_simulation_shares_default_policy_state`` enforces that invariant against the shared instance itself. + + A reform that reaches the parameter tree has no such protection, so the + returned system takes a private copy of the tree the first time a reform + reads it: see :class:`SharedParameterPolicy`. """ policy = copy(system) policy.variables = dict(system.variables) + bind_private_entities(policy) + with_parameter_barrier(policy) + # The lender is now one reader among several, so neither side may write to + # this tree in place - including a lender that had taken a private copy. + mark_tree_shared(policy.__dict__["shared_parameters"]) policy.spm_forecast_provider = system.spm_forecast_provider.snapshot( copy_receipts=False ) @@ -218,10 +593,19 @@ def clone_spm_system(system, *, copy_receipts=True): losing instance changes such as neutralization and inherited reform fields. Let core clone parameters/entities, then copy the actual variable state. """ - policy = copy(system) + policy = plain_policy_copy(system) policy.variables = {} cloned = TaxBenefitSystem.clone(policy) + unmark_tree_shared(cloned.parameters) + # Core copies ``entities``, ``person_entity`` and ``group_entities`` + # separately, leaving two objects per group entity key; keep one, so a + # rebind reaches every reader of that entity. by_key = {entity.key: entity for entity in cloned.entities} + cloned.person_entity = by_key[cloned.person_entity.key] + cloned.group_entities = [by_key[entity.key] for entity in cloned.group_entities] + cloned.group_entity_keys = [entity.key for entity in cloned.group_entities] + for entity in cloned.entities: + entity.set_tax_benefit_system(cloned) memo = {id(system): cloned, id(system.parameters): cloned.parameters} for entity in system.entities: memo[id(entity)] = by_key[entity.key] @@ -241,6 +625,40 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) # Core switches the baseline system after cloning its populations. self._rebind_holders() + # Core sets ``trace`` before this simulation owns its policy state. + self._isolate_parameter_tracing() + # Every input path - situation builder, dataset loader, direct + # set_input - has landed in the holder by now. + self._record_county_input_types() + + @property + def trace(self): + return CoreSimulation.trace.fget(self) + + @trace.setter + def trace(self, trace): + CoreSimulation.trace.fset(self, trace) + self._isolate_parameter_tracing() + + def _isolate_parameter_tracing(self): + """Stop a traced request writing its tracer onto shared parameters.""" + if not self.trace: + return + policy = getattr(self, "tax_benefit_system", None) + # Core sets ``trace`` before a new simulation owns its policy state, + # both while constructing one and while cloning one, so leave the + # original's system alone until this simulation has claimed its own. + if policy is not None and getattr(policy, "simulation", None) is self: + isolate_parameter_tracing(policy, self.tracer, self.branch_name) + for branch in getattr(self, "branches", {}).values(): + branch._isolate_parameter_tracing() + + def get_branch(self, name="branch", clone_system=False): + branch = super().get_branch(name, clone_system) + # Core names the branch and hands it this simulation's tracer after + # cloning, so re-prime the branch's root with what it ended up holding. + branch._isolate_parameter_tracing() + return branch def _rebind_holders(self): """Bind populations and cached holders to their branch's policy state.""" @@ -267,9 +685,61 @@ def _rebind_holders(self): branch._rebind_holders() def apply_reform(self, reform): - super().apply_reform(reform) + policy = self.tax_benefit_system + detaching = getattr(policy, "detaching_parameters", None) + if detaching is None or policy.detaching_shared_parameters: + # Either an ordinary system, or core recursing through a tuple of + # reforms into this override again. Reading ``parameters`` inside + # the armed window would itself trip the barrier and clone the + # tree, which is exactly what a variable-only reform must not pay. + super().apply_reform(reform) + self._rebind_holders() + return + previous_children = getattr(policy.parameters, "children", None) + with detaching(): + super().apply_reform(reform) + self._adopt_detached_parameters(previous_children) + # A detached tree is a fresh root, carrying none of the trace state + # core wrote onto the one it replaced. + self._isolate_parameter_tracing() self._rebind_holders() + def _adopt_detached_parameters(self, previous_children): + """Move branches off the pre-reform tree onto the detached copy. + + A branch created with ``clone_system=False`` shares its parent's policy + state deliberately, so a reform the parent applies is a reform the + branch runs under. Detaching would otherwise strand the branch on the + unreformed tree. + + Branches are recognised by the children of their root node rather than + by the root itself, because a traced branch holds a shallow copy of the + root with those same children (see ``isolate_parameter_tracing``) and + would never match on identity. An adopting branch keeps sharing - the + tree now has more than one reader, so its own reform must detach rather + than rewrite its parent's policy. + """ + detached = self.tax_benefit_system.parameters + if detached is None or detached.children is previous_children: + return + cache = self.tax_benefit_system._parameters_at_instant_cache + for name, branch in self.branches.items(): + if name == "baseline": + # The baseline branch holds unreformed policy by construction. + continue + policy = branch.tax_benefit_system + tree = getattr(policy, "parameters", None) + if tree is not None and tree.children is previous_children: + # The detached tree now has a second reader. + mark_tree_shared(detached) + policy.parameters = detached + policy._parameters_at_instant_cache = cache + # A traced branch needs its own root back, with its own + # at-instant cache, or its parameter receipts rejoin the + # parent's. + branch._isolate_parameter_tracing() + branch._adopt_detached_parameters(previous_children) + @property def spm_config(self): return spm_config(self.tax_benefit_system.spm_forecast_provider) @@ -287,8 +757,10 @@ def _prepare_spm_system(self, args, kwargs, config, start_instant): reform = arguments.arguments.get("reform") if supplied is None: if reform is not None: - chosen = self.default_tax_benefit_system( - reform=reform, spm=config, start_instant=start_instant + chosen = with_parameter_barrier( + self.default_tax_benefit_system( + reform=reform, spm=config, start_instant=start_instant + ) ) else: # No reform and no supplied system: nothing distinguishes this @@ -298,7 +770,9 @@ def _prepare_spm_system(self, args, kwargs, config, start_instant): elif reform is not None: # Core applies the reform set to whatever system it is handed, so # the caller's own system must not be the one it reforms. - chosen = clone_spm_system(supplied, copy_receipts=False) + chosen = with_parameter_barrier( + clone_spm_system(supplied, copy_receipts=False) + ) else: # A caller that builds one system and runs many households through # it - the household API among them - keeps that system's warm @@ -332,14 +806,71 @@ def clone(self, debug=False, trace=False, clone_tax_benefit_system=True): # Core branches may share policy state, but each calculation's # receipt belongs to that simulation's provider. cloned.tax_benefit_system = copy(self.tax_benefit_system) + # Two systems now read this tree, so neither may write to it in + # place: a branch's reform must not rewrite its parent's policy. + mark_tree_shared(getattr(cloned.tax_benefit_system, "parameters", None)) cloned.tax_benefit_system.spm_forecast_provider = ( self.tax_benefit_system.spm_forecast_provider.snapshot( copy_receipts=True ) ) cloned._rebind_holders() + self._rebind_method_aliases(cloned) + cloned._isolate_parameter_tracing() return cloned + def _rebind_method_aliases(self, cloned): + """Point copied bound-method aliases at the clone, not the original. + + Core keeps backwards-compatibility aliases as bound methods on the + instance (``self.calc = self.calculate``, ``self.df = + self.calculate_dataframe``) and its ``clone`` copies the instance + dictionary verbatim, so every alias on the clone still called the + original simulation. ``clone.calc(...)`` therefore returned the + original's values under the original's policy, and recorded the + original's SPM receipts, while ``clone.calculate(...)`` - documented as + the same call - returned the clone's. Rebind by method name so an alias + core adds later is repaired too. + """ + for name, value in list(cloned.__dict__.items()): + if ismethod(value) and value.__self__ is self: + cloned.__dict__[name] = getattr(cloned, value.__func__.__name__) + + def _record_county_input_types(self, period=None): + """Tell this simulation's providers which counties were not text. + + The county a provider is asked for has already been stringified by the + variable that reads the column, so the type has to be captured here, + where the stored input is still whatever the caller supplied. Record it + on this simulation's own provider and on its branches', because a + reform simulation's baseline arm is handed a separate provider that + never sees an input of its own. + """ + holder = self.get_holder("county_fips") + periods = ( + holder.get_known_periods() if period is None else [periods_.period(period)] + ) + for known_period in periods: + array = holder.get_array(known_period) + if array is None or array.dtype.kind in "SU": + # A real text dtype cannot be hiding an integer. + continue + year = int(known_period.start.year) + for simulation in self._simulation_family(): + record = getattr( + simulation.tax_benefit_system.spm_forecast_provider, + "record_county_input_types", + None, + ) + if record is not None: + record(year, array) + + def _simulation_family(self): + """This simulation and every branch that reads the same inputs.""" + yield self + for branch in getattr(self, "branches", {}).values(): + yield from branch._simulation_family() + def set_input(self, variable_name, period, value): # Core's loader calls set_input for every dataset format. Reject saved # measurement/resource outputs here without loading the population twice. @@ -353,4 +884,7 @@ def set_input(self, variable_name, period, value): f"Dataset supplies formula-owned SPM output {variable_name}. " "Use primitive inputs and retain observed outputs under report-only names." ) - return super().set_input(variable_name, period, value) + result = super().set_input(variable_name, period, value) + if variable_name == "county_fips": + self._record_county_input_types(period) + return result diff --git a/policyengine_us/tests/core/test_spm_simulation_isolation.py b/policyengine_us/tests/core/test_spm_simulation_isolation.py new file mode 100644 index 00000000000..1c88396d872 --- /dev/null +++ b/policyengine_us/tests/core/test_spm_simulation_isolation.py @@ -0,0 +1,488 @@ +"""One simulation's reform, clone or trace must not reach another's. + +Every test here reproduces a defect an external review found on the shipped +2.0.1 runtime, where ordinary simulations share the default policy state. +""" + +import hashlib +from copy import copy + +import pytest +from policyengine_core.periods import YEAR +from policyengine_core.reforms import Reform +from policyengine_core.taxbenefitsystems import TaxBenefitSystem +from policyengine_core.variables import Variable + +from policyengine_us import Simulation +from policyengine_us.entities import Person +from policyengine_us.spm import clone_spm_system, share_spm_policy +from policyengine_us.system import system +from policyengine_us.tools.pinned_tbs import get_pre_arpa_eitc_tbs + +SINGLE_STANDARD_DEDUCTION = "gov.irs.deductions.standard.amount.SINGLE" +BASELINE_INCOME_TAX = 4_016 +REFORMED_INCOME_TAX = 3_718 + + +def earner_situation(): + return { + "people": {"person": {"age": {2024: 40}, "employment_income": {2024: 50_000}}}, + "households": { + "household": {"members": ["person"], "county_fips": {2024: "06037"}} + }, + } + + +def parameter_fingerprint(policy): + """Digest every authored parameter value, so a shared-tree edit is visible.""" + digest = hashlib.sha256() + for parameter in policy.parameters.get_descendants(): + values = getattr(parameter, "values_list", None) + if values is None: + continue + digest.update(parameter.name.encode()) + for value_at_instant in values: + digest.update( + f"|{value_at_instant.instant_str}={value_at_instant.value}".encode() + ) + return digest.hexdigest() + + +def parameter_accesses(tracer): + """Every parameter name this tracer recorded, at any depth.""" + recorded = [] + + def visit(node): + recorded.extend(parameter.name for parameter in node.parameters) + for child in node.children: + visit(child) + + for tree in tracer.trees: + visit(tree) + return recorded + + +class NeutralizeIncomeTax(Reform): + def apply(self): + self.neutralize_variable("income_tax") + + +class clone_only_income(Variable): + value_type = float + entity = Person + definition_period = YEAR + label = "Income input added only by the test reform" + + +class AddedInputReform(Reform): + def apply(self): + self.update_variable(clone_only_income) + + +def test_parameter_reform_leaves_every_other_simulation_unreformed(): + """A reform applied to one simulation must not change the shipped policy. + + Core's ``Simulation.apply_reform`` calls ``reform.apply(system)`` directly + rather than constructing the reform, so it bypasses the defensive parameter + clone in ``Reform.__init__`` and edits whatever tree the system hands it. + On a shared tree that silently re-based every other simulation - including + ones already built and calculated - onto the reformed parameter. + """ + fingerprint_before = parameter_fingerprint(system) + reformed = Simulation(situation=earner_situation()) + unrelated = Simulation(situation=earner_situation()) + assert reformed.calculate("income_tax", 2024)[0] == BASELINE_INCOME_TAX + assert unrelated.calculate("income_tax", 2024)[0] == BASELINE_INCOME_TAX + + reformed.apply_reform({SINGLE_STANDARD_DEDUCTION: {"2024": 100_000}}) + + assert reformed.calculate("income_tax", 2024)[0] == REFORMED_INCOME_TAX + # An existing simulation, recomputed from its own inputs. Use the same + # purge apply_reform performs: deleting one variable's arrays leaves the + # cached intermediates that a contaminated tree would have to flow through. + unrelated._invalidate_all_caches() + assert unrelated.calculate("income_tax", 2024)[0] == BASELINE_INCOME_TAX + assert unrelated.calculate("standard_deduction", 2024)[0] < 100_000 + # And one built after the reform. + later = Simulation(situation=earner_situation()) + assert later.calculate("income_tax", 2024)[0] == BASELINE_INCOME_TAX + # Nothing reached the shared instance itself. + assert parameter_fingerprint(system) == fingerprint_before + + +def test_parameter_reform_detaches_the_shared_tree_and_its_warm_caches(): + simulation = Simulation(situation=earner_situation()) + policy = simulation.tax_benefit_system + assert policy.parameters is system.parameters + assert policy._parameters_at_instant_cache is system._parameters_at_instant_cache + + simulation.apply_reform({SINGLE_STANDARD_DEDUCTION: {"2024": 100_000}}) + + assert policy.parameters is not system.parameters + assert policy._parameters_at_instant_cache is not ( + system._parameters_at_instant_cache + ) + assert not policy.shares_parameters + + +def test_variable_only_reform_keeps_sharing_the_parameter_tree(): + """Detaching rebuilds 130,000 parameter nodes, so it must stay lazy. + + Every simulation re-applies the structural reform at its own start instant, + and that reform only rebinds variables. Detaching for it would put a full + tree clone on every household API request. + """ + simulation = Simulation(situation=earner_situation()) + policy = simulation.tax_benefit_system + + simulation.apply_reform(NeutralizeIncomeTax) + + assert simulation.calculate("income_tax", 2024)[0] == 0 + assert policy.parameters is system.parameters + assert policy.shares_parameters + + +def test_shared_policy_branch_follows_its_parents_parameter_reform(): + """A branch cloned without its own system shares policy deliberately.""" + simulation = Simulation(situation=earner_situation()) + branch = simulation.get_branch("shared_policy") + + simulation.apply_reform({SINGLE_STANDARD_DEDUCTION: {"2024": 100_000}}) + + assert branch.tax_benefit_system.parameters is ( + simulation.tax_benefit_system.parameters + ) + assert branch.calculate("income_tax", 2024)[0] == REFORMED_INCOME_TAX + isolated = simulation.get_branch("separate_policy", clone_system=True) + assert isolated.calculate("income_tax", 2024)[0] == REFORMED_INCOME_TAX + + +def test_clone_method_aliases_calculate_on_the_clone(): + """``calc`` is documented as ``calculate``; core copies it as a bound method. + + Core keeps ``self.calc = self.calculate`` and + ``self.df = self.calculate_dataframe`` in the instance dictionary and its + ``clone`` copies that dictionary verbatim, so every alias on the clone + still called the original simulation, under the original's policy. + """ + original = Simulation(situation=earner_situation()) + assert original.calculate("income_tax", 2024)[0] == BASELINE_INCOME_TAX + clone = original.clone() + clone.apply_reform(NeutralizeIncomeTax) + + assert clone.calculate("income_tax", 2024)[0] == 0 + assert clone.calc("income_tax", 2024)[0] == 0 + assert clone.calc.__self__ is clone + assert clone.df.__self__ is clone + # The original keeps its own policy and its own answer. + assert original.calculate("income_tax", 2024)[0] == BASELINE_INCOME_TAX + + +def test_clone_method_alias_records_receipts_on_the_clone(): + original = Simulation(situation=earner_situation()) + clone = original.clone() + + clone.calc("spm_unit_spm_threshold", 2025) + + assert set(clone.spm_provenance()["years"]) == {"2025"} + assert original.spm_provenance()["years"] == {} + + +def test_reform_added_variable_accepts_input_on_an_ordinary_simulation(): + """A population asks its entity for a variable before it builds a holder. + + The entity resolves the name through the system it is bound to, so a shared + system whose entities still belonged to the shared instance raised + ``VariableNotFoundError`` for a variable its own registry plainly held. + """ + simulation = Simulation(situation=earner_situation()) + simulation.apply_reform(AddedInputReform) + + simulation.set_input("clone_only_income", 2024, [123]) + + assert simulation.calculate("clone_only_income", 2024)[0] == 123 + assert "clone_only_income" not in system.variables + + +@pytest.mark.parametrize("period", [2033, 2034]) +def test_traced_simulations_record_their_own_parameter_accesses(period): + """Core marks the parameter tree with the requesting simulation's tracer. + + On a shared tree the second traced request reused the first request's + cached tracing node, so its parameter accesses were recorded against the + first request's tracer - or, with that request's trace frame closed, + recorded nowhere - and the shared tree kept a finished request's tracer for + every simulation that read it afterwards. + """ + first = Simulation(situation=earner_situation()) + first.trace = True + first.calculate("spm_unit_fpg", period) + first_accesses = parameter_accesses(first.tracer) + + second = Simulation(situation=earner_situation()) + second.trace = True + second.calculate("spm_unit_fpg", period) + + expected = { + "gov.abolitions.spm_unit_fpg", + "gov.abolitions.spm_unit_size", + "gov.hhs.fpg.additional_person", + "gov.hhs.fpg.first_person", + } + assert set(first_accesses) == expected + assert set(parameter_accesses(second.tracer)) == expected + # The first simulation's trace did not grow when the second one calculated. + assert parameter_accesses(first.tracer) == first_accesses + # Each traced request owns the root node core writes its tracer onto. + first_root = first.tax_benefit_system.parameters + second_root = second.tax_benefit_system.parameters + assert first_root is not second_root + assert first_root._at_instant_cache is not second_root._at_instant_cache + assert first_root.tracer is first.tracer + assert second_root.tracer is second.tracer + + +def test_tracing_leaves_the_shared_parameter_tree_untraced(): + traced = Simulation(situation=earner_situation()) + traced.trace = True + traced.calculate("spm_unit_fpg", 2035) + + assert traced.tax_benefit_system.parameters is not system.parameters + assert system.parameters.trace is False + assert system.parameters.tracer is None + assert system.parameters.branch_name is None + # Tracing changes no answer. + untraced = Simulation(situation=earner_situation()) + assert ( + untraced.calculate("spm_unit_fpg", 2035)[0] + == (traced.calculate("spm_unit_fpg", 2035)[0]) + ) + + +def test_cloning_a_shared_policy_system_stops_sharing_its_tree(): + """Core's clone writes the cloned tree into the instance dictionary. + + A shared system resolves ``parameters`` through a property, which would + shadow that write and hand back a "clone" still sharing the lender's tree, + with nothing raised. + """ + shared = Simulation(situation=earner_situation()).tax_benefit_system + assert shared.parameters is system.parameters + + cloned = shared.clone() + + assert cloned.parameters is not system.parameters + assert not getattr(cloned, "shares_parameters", False) + assert "shared_parameters" not in cloned.__dict__ + assert parameter_fingerprint(cloned) == parameter_fingerprint(system) + + +def test_shared_policy_class_has_an_import_path(): + """A class only reachable from a dictionary cannot be pickled or named. + + Sharing policy puts every simulation's system on a class this module + creates at run time, so that class has to be published under its own name - + otherwise anything that pickles a system (a worker pool, joblib, a notebook + checkpoint) fails on the class itself rather than on its contents. + """ + import pickle + + import policyengine_us.spm as spm + + shared_class = type(Simulation(situation=earner_situation()).tax_benefit_system) + + assert shared_class.__module__ == spm.__name__ + assert getattr(spm, shared_class.__qualname__) is shared_class + assert pickle.loads(pickle.dumps(shared_class)) is shared_class + + +def test_parameter_reform_on_a_shared_branch_leaves_its_parent_alone(): + """A branch that reforms policy owns the tree it reformed.""" + simulation = Simulation(situation=earner_situation()) + branch = simulation.get_branch("reformed_branch") + + branch.apply_reform({SINGLE_STANDARD_DEDUCTION: {"2024": 100_000}}) + + assert branch.calculate("income_tax", 2024)[0] == REFORMED_INCOME_TAX + assert simulation.calculate("income_tax", 2024)[0] == BASELINE_INCOME_TAX + assert simulation.tax_benefit_system.parameters is system.parameters + + +@pytest.mark.parametrize( + "mutate", + [ + lambda policy: policy.modify_parameters( + {SINGLE_STANDARD_DEDUCTION: {"year:2024:1": 100_000}} + ), + lambda policy: policy.add_abolition_parameters(), + # Core's load_extension ends in ``self.parameters.merge(...)``. Stub + # the base out: a real extension is not needed to pin that this + # country override detaches before core reaches the tree. + lambda policy: policy.load_extension("any_extension"), + ], + ids=["modify_parameters", "add_abolition_parameters", "load_extension"], +) +def test_in_place_parameter_edits_detach_before_they_write(mutate, monkeypatch): + """Core edits these into the live tree rather than a copy of it.""" + monkeypatch.setattr( + TaxBenefitSystem, "load_extension", lambda self, extension: None + ) + fingerprint_before = parameter_fingerprint(system) + policy = Simulation(situation=earner_situation()).tax_benefit_system + + mutate(policy) + + assert policy.parameters is not system.parameters + assert parameter_fingerprint(system) == fingerprint_before + + +def test_shared_policy_over_a_core_cloning_base_clones_without_sharing(): + """A base that clones through core must not defeat the property. + + Core's ``TaxBenefitSystem.clone`` writes the cloned tree straight into the + new instance's dictionary, where ``SharedParameterPolicy.parameters`` + shadows it, so a system built on a base that uses core's clone would hand + back a "clone" still reading the lender's tree, with nothing raised. + """ + base = copy(system) + base.__class__ = type( + "CoreCloningCountrySystem", + (type(system),), + {"clone": TaxBenefitSystem.clone}, + ) + shared = share_spm_policy(base) + assert shared.parameters is system.parameters + + cloned = shared.clone() + + assert cloned.parameters is not system.parameters + assert parameter_fingerprint(cloned) == parameter_fingerprint(system) + + +def test_cloned_systems_hold_one_entity_object_per_key(): + """Core's clone leaves two objects per group-entity key, both bound. + + Rebinding through ``entities`` would then miss the copy that + ``instantiate_entities`` and core's simulation builder read. + """ + cloned = clone_spm_system(system) + + by_key = {entity.key: entity for entity in cloned.entities} + assert len(by_key) == len(cloned.entities) + assert cloned.person_entity is by_key[cloned.person_entity.key] + assert all(entity is by_key[entity.key] for entity in cloned.group_entities) + for entity in cloned.entities: + assert entity._tax_benefit_system is cloned + + +def test_pinned_systems_are_rebuilt_when_their_source_tree_detaches(): + """A shared system keeps its identity while swapping its tree. + + The NY EITC and CTC formulas reuse a cached clone pinned to an earlier + federal vintage. Keyed on the system alone, that clone would survive a + reform the system detached for, and NY's decoupled credits would silently + go on reading pre-reform federal parameters. + """ + simulation = Simulation(situation=earner_situation()) + policy = simulation.tax_benefit_system + pinned = get_pre_arpa_eitc_tbs(policy) + assert get_pre_arpa_eitc_tbs(policy) is pinned + + simulation.apply_reform({SINGLE_STANDARD_DEDUCTION: {"2024": 100_000}}) + + assert policy.parameters is not system.parameters + assert get_pre_arpa_eitc_tbs(policy) is not pinned + + +@pytest.mark.parametrize("traced", [False, True]) +def test_shared_branch_follows_its_parent_whether_or_not_it_is_traced(traced): + """Tracing gives a simulation its own root node, not its own policy. + + A branch cloned without its own system shares its parent's policy, so the + detached tree has to reach it however the two roots are spelled. Matching + on root identity missed every traced branch, and the model's own formulas + branch this way - itemizing against not itemizing, the state EITC + refundability branches, marginal tax rates - so a traced simulation + reported pre-reform numbers under a reform. + """ + simulation = Simulation(situation=earner_situation()) + simulation.trace = traced + branch = simulation.get_branch("shared_policy") + + simulation.apply_reform({SINGLE_STANDARD_DEDUCTION: {"2024": 100_000}}) + + assert simulation.calculate("income_tax", 2024)[0] == REFORMED_INCOME_TAX + assert branch.calculate("income_tax", 2024)[0] == REFORMED_INCOME_TAX + assert branch.tax_benefit_system.parameters.children is ( + simulation.tax_benefit_system.parameters.children + ) + + +def test_traced_simulation_sees_a_later_parameter_reform(): + """A traced root must not outlive the tree its children belong to. + + A shallow root copy keeps the original's children, and each child's parent + still points at the original root, so a parameter edit clears that root's + at-instant cache and leaves the traced copy serving pre-reform values. + """ + simulation = Simulation( + situation=earner_situation(), + reform=Reform.from_dict({SINGLE_STANDARD_DEDUCTION: {"2024": 20_000}}), + ) + simulation.trace = True + assert simulation.calculate("standard_deduction", 2024)[0] == 20_000 + + simulation.apply_reform({SINGLE_STANDARD_DEDUCTION: {"2024": 100_000}}) + + assert simulation.calculate("standard_deduction", 2024)[0] == 100_000 + + +def test_lending_a_detached_tree_stops_the_lender_writing_to_it(): + """Owning a tree once is not owning it forever. + + A simulation that detached a private tree, and then lent it to a second + simulation, has to clone again before its next reform: the borrower can + see everything it writes. + """ + lender = Simulation(situation=earner_situation()) + lender.apply_reform({SINGLE_STANDARD_DEDUCTION: {"2024": 100_000}}) + borrower = Simulation( + tax_benefit_system=lender.tax_benefit_system, situation=earner_situation() + ) + assert borrower.calculate("standard_deduction", 2024)[0] == 100_000 + + lender.apply_reform({SINGLE_STANDARD_DEDUCTION: {"2024": 7_777}}) + + assert lender.calculate("standard_deduction", 2024)[0] == 7_777 + borrower._invalidate_all_caches() + assert borrower.calculate("standard_deduction", 2024)[0] == 100_000 + + +def test_a_branch_reform_does_not_rewrite_its_parents_policy(): + simulation = Simulation(situation=earner_situation()) + branch = simulation.get_branch("reforming_branch") + simulation.apply_reform({SINGLE_STANDARD_DEDUCTION: {"2024": 50_000}}) + + branch.apply_reform({SINGLE_STANDARD_DEDUCTION: {"2024": 100_000}}) + + assert branch.calculate("standard_deduction", 2024)[0] == 100_000 + assert simulation.calculate("standard_deduction", 2024)[0] == 50_000 + + +def test_a_tuple_of_variable_only_reforms_keeps_sharing_the_tree(): + """Core recurses into this override for each member of a reform tuple. + + Reading ``parameters`` inside the outer call's armed window would itself + trip the barrier, so every tuple reform - including the purely structural + ones - would have paid for a full tree clone. + """ + simulation = Simulation(situation=earner_situation()) + policy = simulation.tax_benefit_system + + simulation.apply_reform((NeutralizeIncomeTax, AddedInputReform)) + + assert simulation.calculate("income_tax", 2024)[0] == 0 + simulation.set_input("clone_only_income", 2024, [123]) + assert simulation.calculate("clone_only_income", 2024)[0] == 123 + assert policy.parameters is system.parameters + assert policy.shares_parameters diff --git a/policyengine_us/tests/core/test_spm_system.py b/policyengine_us/tests/core/test_spm_system.py index 2b60bcd08ed..9177cbe660d 100644 --- a/policyengine_us/tests/core/test_spm_system.py +++ b/policyengine_us/tests/core/test_spm_system.py @@ -15,7 +15,7 @@ from policyengine_us import Microsimulation, Simulation from policyengine_us.data.dataset_schema import USMultiYearDataset, USSingleYearDataset from policyengine_us.entities import Person -from policyengine_us.spm import create_spm_provider +from policyengine_us.spm import create_spm_provider, is_county_fips from policyengine_us.system import DEFAULT_DATASET, _resolve_dataset_path, system @@ -152,8 +152,22 @@ def test_ordinary_simulation_shares_default_policy_state(): assert policy is not system assert policy.parameters is system.parameters assert policy._parameters_at_instant_cache is system._parameters_at_instant_cache - # Entities stay bound to the shared instance, exactly as before this change. - assert policy.entities is system.entities + # Entities are private and bound to this simulation's own registry: an + # entity resolves variable names through the system it is bound to, so + # sharing the shared instance's entities would send every holder lookup to + # the shared registry. + assert policy.entities is not system.entities + assert {entity.key for entity in policy.entities} == { + entity.key for entity in system.entities + } + for entity in policy.entities: + assert entity._tax_benefit_system is policy + for entity in system.entities: + assert entity._tax_benefit_system is system + # One object per key, so rebinding one reaches every reader of it. + by_key = {entity.key: entity for entity in policy.entities} + assert policy.person_entity is by_key[policy.person_entity.key] + assert all(entity is by_key[entity.key] for entity in policy.group_entities) # A private registry, holding the shared instance's own variable objects. assert policy.variables is not system.variables assert ( @@ -381,23 +395,134 @@ def missing(*args, **kwargs): @pytest.mark.parametrize("simulation_type", [Simulation, Microsimulation]) -def test_integer_county_column_requires_county_fips_instead_of_reporting_unavailable( - simulation_type, +@pytest.mark.parametrize( + "counties", + [ + # A legacy population file storing the CPS within-state code. + [5, 1], + # A county code whose decimal form is five digits, so only its type + # distinguishes it from the documented input. + [36_061, 36_061], + # The same mistake for a state whose code carries a leading zero. + [6_037, 6_037], + [float("nan"), float("nan")], + ], + ids=["within_state", "five_digit_integer", "dropped_leading_zero", "missing"], +) +def test_non_text_county_column_requires_county_fips_however_it_is_spelled( + simulation_type, counties ): - """Legacy population files store the CPS within-state code as an integer.""" + """``county_fips`` is documented as a five-digit *string*. + + Core maps a ``str`` variable onto the numpy ``object`` dtype, so the model + stores an integer column as integers, and every reader stringifies before + asking the forecast provider. An integer county code for a state without a + leading zero was therefore accepted silently, while the identical mistake + for California was reported as an absent county. + """ source = small_dataset() - source.household["county_fips"] = [5, 1] + source.household["county_fips"] = counties simulation = simulation_type(dataset=source) with pytest.raises(SPMInputError) as error: simulation.calculate("spm_unit_spm_threshold", 2024) assert error.value.code == "SPM_GEOGRAPHY_REQUIRED" assert "five-digit string" in str(error.value) assert 'geography_kind="national"' in str(error.value) - # The same population computes once an SPM area is selected explicitly. + # The same population computes once an SPM area is selected explicitly: + # a caller who never asks for a county is never asked for one. national = simulation_type(dataset=source, spm={"geography_kind": "national"}) assert np.all(national.calculate("spm_unit_spm_threshold", 2024) > 0) +def test_correcting_a_county_input_clears_its_rejection(): + """The record has to follow the input, not outlive it.""" + situation = single_person_situation() + situation["households"]["household"]["county_fips"] = {2024: 36_061} + simulation = Simulation(situation=situation) + with pytest.raises(SPMInputError): + simulation.calculate("spm_unit_spm_threshold", 2024) + + simulation.set_input("county_fips", 2024, ["36061"]) + + assert simulation.calculate("spm_unit_spm_threshold", 2024)[0] > 0 + + +def test_a_county_rejection_does_not_follow_the_system_to_a_new_simulation(): + """A new simulation reads its own inputs; only a clone keeps these.""" + situation = single_person_situation() + situation["households"]["household"]["county_fips"] = {2024: 36_061} + mistyped = Simulation(situation=situation) + with pytest.raises(SPMInputError): + mistyped.calculate("spm_unit_spm_threshold", 2024) + + # The same county, sent correctly: the lender's record of its own mistake + # must not reject it. + situation = single_person_situation() + situation["households"]["household"]["county_fips"] = {2024: "36061"} + correct = Simulation( + tax_benefit_system=mistyped.tax_benefit_system, situation=situation + ) + assert correct.calculate("spm_unit_spm_threshold", 2024)[0] > 0 + with pytest.raises(SPMInputError): + mistyped.clone().calculate("spm_unit_spm_threshold", 2024) + + +def test_a_reform_simulations_baseline_arm_rejects_the_same_county_input(): + """Core hands the baseline arm a provider that never sees an input.""" + situation = single_person_situation() + situation["households"]["household"]["county_fips"] = {2024: 36_061} + simulation = Simulation( + situation=situation, + reform=Reform.from_dict( + {"gov.irs.credits.ctc.amount.base[0].amount": {"2024": 0}} + ), + ) + for arm in (simulation, simulation.baseline): + with pytest.raises(SPMInputError) as error: + arm.calculate("spm_unit_spm_threshold", 2024) + assert error.value.code == "SPM_GEOGRAPHY_REQUIRED" + + +def test_a_national_selection_records_no_county_input_types(): + """Nothing will ask that provider for a county, so nothing is scanned.""" + source = small_dataset() + source.household["county_fips"] = [36_061, 36_061] + national = Microsimulation(dataset=source, spm={"geography_kind": "national"}) + provider = national.tax_benefit_system.spm_forecast_provider + assert provider._untyped_counties == {} + assert np.all(national.calculate("spm_unit_spm_threshold", 2024) > 0) + + +def test_text_county_column_still_resolves_its_county(): + source = small_dataset() + source.household["county_fips"] = ["06037", "36061"] + simulation = Microsimulation(dataset=source) + assert np.all(simulation.calculate("spm_unit_spm_threshold", 2024) > 0) + assert len(simulation.spm_provenance()["geographies"]) == 2 + + +@pytest.mark.parametrize( + ("value", "accepted"), + [ + ("06037", True), + (b"06037", True), + (np.str_("36061"), True), + (np.bytes_(b"36061"), True), + ("6037", False), + ("", False), + ("360610", False), + (36_061, False), + (np.int64(36_061), False), + (36_061.0, False), + (float("nan"), False), + (None, False), + (True, False), + ], +) +def test_county_fips_accepts_five_digit_text_only(value, accepted): + assert is_county_fips(value) is accepted + + def small_dataset(): groups = ("household", "tax_unit", "spm_unit", "family", "marital_unit") return USSingleYearDataset( diff --git a/policyengine_us/tests/microsimulation/data/fixtures/test_extend_single_year_dataset.py b/policyengine_us/tests/microsimulation/data/fixtures/test_extend_single_year_dataset.py index 0439c6ab2be..29d0e786128 100644 --- a/policyengine_us/tests/microsimulation/data/fixtures/test_extend_single_year_dataset.py +++ b/policyengine_us/tests/microsimulation/data/fixtures/test_extend_single_year_dataset.py @@ -267,6 +267,8 @@ def mock_super_init(self, *args, **kwargs): for population in self.populations.values(): population.simulation = self self._user_input_keys = set() + # Core's __init__ sets these before the country's __init__ resumes. + self.trace = False self.branches = {} self.baseline = None self.is_over_dataset = True diff --git a/policyengine_us/tests/microsimulation/test_microsim.py b/policyengine_us/tests/microsimulation/test_microsim.py index 7fa111a3754..25f764eee7e 100644 --- a/policyengine_us/tests/microsimulation/test_microsim.py +++ b/policyengine_us/tests/microsimulation/test_microsim.py @@ -24,10 +24,16 @@ def test_legacy_enhanced_cps_lacks_source_backed_spm_independence_roles(): Its county column does hold five-digit FIPS codes, so geography is not what fails. 18 of its 43,134 SPM units are a lone 15-to-17-year-old carrying no source-backed SPM independence role, so those units classify no measurement - adult, and every output that reaches the threshold - household net income, - benefits, poverty and marginal tax rates included - fails closed over the - file. Assert on the threshold itself: it raises straight off the + adult and every output that reaches the threshold fails closed over the + file. Only the threshold is asserted here: it raises straight off the composition, without building the whole resource chain over 43,134 units. + + Resource outputs are no longer among the outputs that reach it. The + capped housing subsidy asks for the SPM housing portion only for units + receiving housing assistance, and none of these 18 units does - which is + asserted below - so household net income and benefits compute over the + file. Computing them here would add two minutes to this suite, so they + are left to the resource tests. """ import numpy as np from spm_calculator.errors import SPMInputError @@ -41,6 +47,11 @@ def test_legacy_enhanced_cps_lacks_source_backed_spm_independence_roles(): adults = np.asarray(simulation.calculate("spm_measurement_adults", 2024)) assert (adults < 1).sum() == 18 + assisted = np.asarray( + simulation.calculate("receives_housing_assistance", 2024, map_to="spm_unit") + ) + assert assisted.sum() == 870 + assert not assisted[adults < 1].any() with pytest.raises(SPMInputError) as error: simulation.calculate("spm_unit_spm_threshold", 2024) assert error.value.code == "SPM_COMPOSITION_REQUIRED" diff --git a/policyengine_us/tools/pinned_tbs.py b/policyengine_us/tools/pinned_tbs.py index 2e9c1318836..8f2ce50f787 100644 --- a/policyengine_us/tools/pinned_tbs.py +++ b/policyengine_us/tools/pinned_tbs.py @@ -14,22 +14,32 @@ from policyengine_core.parameters import Parameter from policyengine_core.periods import instant -# pin name -> (weakref to base TBS, pinned clone). +# pin name -> (weakref to base TBS, weakref to its parameter tree, pinned clone). # The clone is held strongly so it survives across simulations; the # weakref to the base is used to detect that the base system changed # (e.g. a reformed system in another test), in which case the entry is # rebuilt. Using a weakref (rather than id() alone) also guards against # id reuse after the base system is garbage-collected. +# +# The parameter tree is tracked separately because a shared-policy system +# keeps its identity while swapping its tree: a reform detaches a private +# copy in place (see policyengine_us.spm.SharedParameterPolicy), so a pin +# built before that reform would otherwise be reused after it. _PINNED_TBS_CACHE = {} def _get_pinned_tbs(base_tbs, pin_name, pin_fn): entry = _PINNED_TBS_CACHE.get(pin_name) - if entry is not None and entry[0]() is base_tbs: - return entry[1] + parameters = base_tbs.parameters + if entry is not None and entry[0]() is base_tbs and entry[1]() is parameters: + return entry[2] pinned = base_tbs.clone() pin_fn(pinned) - _PINNED_TBS_CACHE[pin_name] = (weakref.ref(base_tbs), pinned) + _PINNED_TBS_CACHE[pin_name] = ( + weakref.ref(base_tbs), + weakref.ref(parameters), + pinned, + ) return pinned From 17cf3f1812533a035e3f5582e35e7aaa27a086d8 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 19:46:26 -0400 Subject: [PATCH 04/21] Record verified PR heads and unified-candidate progress Co-Authored-By: Claude Fable 5.1 --- PROGRESS.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/PROGRESS.md b/PROGRESS.md index fbaf2ba9684..fd850781548 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -4,26 +4,26 @@ Base: `upstream/main` @ `2cc6f045ab1f389bc781ffd9f5e874055b3c0a3d` (version 2.1.0). Worktree: `/Users/maxghenis/spm-rebuild-20260908/worktrees/policyengine-us-spm-unified-20260914`. -Verified PR heads (2026-09-14): -| PR | head | base merge-base | +Verified PR heads (`gh pr view --json headRefOid`, 2026-09-14): +| PR | head | merge-base with main | |---|---|---| | #9466 | `9f9510b735b3b9b2e375854d66fd315202e31d35` | 2cc6f045ab | -| #9463 | `d8cbd4b7a0b292beafad37cfc72813e1d710fc3e` (lane 20260914-144927 still RUNNING — re-verify) | 2cc6f045ab | +| #9463 | `d8cbd4b7a0b292beafad37cfc72813e1d710fc3e` (lane 20260914-144927 still RUNNING — re-verify before push) | 2cc6f045ab | | #9448 | `8f74a5152d1e52c33bd7a7cba6340346bd7fe004` | 2ba7daa30e | | #9464 | `cafcd9443df7f638a5ad547a92c6e0bcfd784fbb` | 4d34e3d1ab | -| #9462 | `47e94fa491bbf65ae4b6bd5667f054445b26de36` | deliberately excluded | +| #9462 | `47e94fa491bbf65ae4b6bd5667f054445b26de36` | deliberately excluded (Microcosm #925) | ## Done - [x] Read reconciliation memo, FINAL-REPORT, EXPANDED-REVIEW, FINDINGS-RECOVERY - [x] Worktree + branch from `upstream/main` - [x] Step 2a: #9466 cherry-picked as-is (`c5d8abfb44`) +- [x] Step 2b: #9463 applied (`a6f1e8da14`) — isolation mechanism, county FIPS, docs, 16 regression + tests, pinned_tbs.py cache key; dropped its PROGRESS.md ## Next -- [ ] Poll lane 20260914-144927-spm-fix-isolation-202 until not RUNNING; re-verify #9463 head -- [ ] Step 2b: #9463 isolation mechanism + county FIPS + docs + 16 regression tests (drop PROGRESS.md, lane-bookkeeping pinned_tbs.py) -- [ ] Step 2c: #9448 reduced (CPUC exclusion, general/CBO/Tlaib housing switch, release tooling, policy-family + source-input contract tests) -- [ ] Step 2d: #9464 reduced (alias guard, dataset digest, batch exit shim, citations) -- [ ] Step 3: EXPANDED-REVIEW R1/R2 fixes; R8 FERA note +- [ ] Step 2c: #9448 reduced +- [ ] Step 2d: #9464 reduced +- [ ] Step 3: EXPANDED-REVIEW R1/R2 fixes; R8 FERA note in CPUC parameter - [ ] Step 4: verification suites + exit codes - [ ] Step 5: push, draft PR, CI - [ ] REMOVE PROGRESS.md before final push From d1251c47cb9669f7878cb4332933a88fddd5ca70 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 20:04:50 -0400 Subject: [PATCH 05/21] Keep SPM valuation out of program benefits and CARE/FERA income From #9448, reduced. General household benefits, CBO means-tested transfers and the three Tlaib household-benefit overrides now add actual `housing_assistance` instead of the Census SPM's capped housing subsidy, so a change to an SPM threshold no longer moves a non-SPM result. SPM resources keep the capped valuation, now allocated across SPM units by member share. CARE/FERA countable income excludes housing subsidies from the August 14, 2014 parameter date of CPUC D.14-08-030; annual values resolve at January 1, so calendar 2014 retains the earlier modelled inclusion (using the actual benefit) and calendar 2015 first applies the exclusion. Also carries #9448's release-candidate wheel tooling and the isolated build backend pin. Dropped from #9448: its `spm.py` and `system.py` isolation hunks (#9463 owns the mechanism), its default-dataset hash and batch-runner hunks (#9464 owns those), its `docs/usage/microsimulation.md` edit (#9463's is more specific), its `reforms/reforms.py` change (no consumer) and its `tests/core/test_spm_system.py` edits (they invert #9463's contract). Co-Authored-By: Claude Fable 5.1 --- .github/release-lock.md | 25 +- .github/release_build.py | 161 ++++++++++ .github/tests/test_release_build.py | 209 +++++++++++++ .github/workflows/pr.yaml | 48 ++- .github/workflows/push.yaml | 8 +- changelog.d/care-housing-income.fixed.md | 1 + changelog.d/release-wheel-candidate.fixed.md | 4 + changelog.d/spm-housing-allocation.fixed.md | 1 + docs/spm.md | 64 +++- .../household/cbo_means_tested_transfers.yaml | 5 +- .../gov/household/household_benefits.yaml | 4 +- .../gov/states/ca/cpuc/income_sources.yaml | 82 ++++- .../boost/boost_middle_class_tax_credit.py | 6 +- .../edaa_end_child_poverty_act.py | 4 +- .../congress/tlaib/end_child_poverty_act.py | 6 +- .../core/test_spm_source_input_contract.py | 42 +++ .../ca/cpuc/ca_cpuc_countable_income.yaml | 72 +++++ .../gov/states/ca/cpuc/integration.yaml | 72 +++++ .../household/cbo_household_income.yaml | 3 +- .../income/household/household_benefits.yaml | 29 +- .../household/household_net_income.yaml | 8 +- .../boost/boost_middle_class_tax_credit.yaml | 9 +- .../edaa_end_child_poverty_act.yaml | 9 +- .../end_child_poverty_act/integration.yaml | 9 +- .../ecpa_head_start_benefits_composition.yaml | 2 +- .../tests/unit/test_cpuc_spm_independence.py | 80 +++++ .../tests/unit/test_spm_housing_allocation.py | 296 ++++++++++++++++++ .../unit/test_spm_measurement_boundary.py | 104 ++++++ .../income/household/household_benefits.py | 4 +- .../spm_unit_allocated_housing_subsidy.py | 20 ++ .../spm_unit_allocated_tenant_payment.py | 24 ++ .../spm_unit_capped_housing_subsidy.py | 10 +- pyproject.toml | 11 +- 33 files changed, 1367 insertions(+), 65 deletions(-) create mode 100644 .github/release_build.py create mode 100644 .github/tests/test_release_build.py create mode 100644 changelog.d/care-housing-income.fixed.md create mode 100644 changelog.d/release-wheel-candidate.fixed.md create mode 100644 changelog.d/spm-housing-allocation.fixed.md create mode 100644 policyengine_us/tests/core/test_spm_source_input_contract.py create mode 100644 policyengine_us/tests/unit/test_cpuc_spm_independence.py create mode 100644 policyengine_us/tests/unit/test_spm_housing_allocation.py create mode 100644 policyengine_us/tests/unit/test_spm_measurement_boundary.py create mode 100644 policyengine_us/variables/household/income/spm_unit/spm_unit_allocated_housing_subsidy.py create mode 100644 policyengine_us/variables/household/income/spm_unit/spm_unit_allocated_tenant_payment.py diff --git a/.github/release-lock.md b/.github/release-lock.md index 6a862f89c0e..4dda3650af8 100644 --- a/.github/release-lock.md +++ b/.github/release-lock.md @@ -41,13 +41,34 @@ the same prerequisite, and `Publish` checks its own committed checkout again before installation or build. Model environments use locked synchronization; subsequent commands use that environment without synchronizing again. +The PR `CandidateWheel` matrix starts from the reviewed head in two separate +clean checkouts. Both run the actual release version-bump helper; the `rc1` +entry appends `rc1` to that computed next version before both perform the +root-only lock refresh. Each uses the same Ubuntu version, Python, uv, locked +development environment, and `make` build as `Publish`. Each uploads its +unpublished wheel and `release-build/receipt.json`; the job has read-only +repository access and no publication step. The receipt rejects changed or +untracked policy source, checks the wheel's version and backend, and records source, lock, tool, and +wheel identities. Download the `release-wheel-rc1-` and +`release-wheel-final-` artifacts to qualify their actual bytes. +A change to the head or changelog fragments requires a new candidate; the +candidate does not reserve a registry version. +GitHub updates its hosted Ubuntu images even within `ubuntu-24.04`, so receipts +also record `ImageOS` and `ImageVersion` for release-environment comparison. + +`pyproject.toml` pins Hatchling and its complete non-optional dependency closure +because `python -m build` creates an isolated backend environment outside the +runtime lock. Both builds use those same pins and the frontend version in +`uv.lock`. `Publish` verifies its own receipt before publishing. Registry wheel +comparison and downstream qualification remain separate release checks. + `ReleaseLockTests` has no dependency on a model job or the country lock check. It uses only the Python standard library and uv, so it can test the guard while the country dependency registry is incomplete: ```sh -python -m unittest discover -s .github/tests -p test_release_lock.py -v -RELEASE_LOCK_REAL_UV=1 python -m unittest discover -s .github/tests -p test_release_lock.py -v +python -m unittest discover -s .github/tests -p 'test_release_*.py' -v +RELEASE_LOCK_REAL_UV=1 python -m unittest discover -s .github/tests -p 'test_release_*.py' -v ``` The opt-in probe creates an isolated, minimal project with a standard PyPI diff --git a/.github/release_build.py b/.github/release_build.py new file mode 100644 index 00000000000..6508fc32d1e --- /dev/null +++ b/.github/release_build.py @@ -0,0 +1,161 @@ +"""Verify clean policy source and record a locally built release wheel. + +The PR matrix can prepare either candidate using the actual release bump helper. +Both candidates and Publish record a receipt after the same make command. +This script never resolves dependencies or publishes. +""" + +from __future__ import annotations + +import hashlib +from importlib.metadata import version +import json +import os +from pathlib import Path +import platform +import re +import subprocess +import sys +import tomllib +from zipfile import ZipFile + + +ROOT = Path(__file__).resolve().parents[1] + + +def prepare_candidate(root: Path, kind: str) -> None: + """Use the actual release helper, optionally preparing its rc1 counterpart.""" + if kind not in {"rc1", "final"}: + raise ValueError("Candidate kind must be rc1 or final") + subprocess.run( + [sys.executable, str(root / ".github/bump_version.py")], cwd=root, check=True + ) + if kind == "rc1": + path = root / "pyproject.toml" + project = path.read_text() + final_version = tomllib.loads(project)["project"]["version"] + prepared, count = re.subn( + rf'^(version\s*=\s*"){re.escape(final_version)}("\s*)$', + rf"\g<1>{final_version}rc1\g<2>", + project, + flags=re.MULTILINE, + ) + if count != 1: + raise ValueError("Expected exactly one release version assignment") + path.write_text(prepared) + + +def _sha256(path: Path) -> str: + return hashlib.sha256(path.read_bytes()).hexdigest() + + +def checked_build_requirements(project: dict) -> dict[str, str]: + """Require exact pins for every declared isolated-build requirement.""" + if project["build-system"]["build-backend"] != "hatchling.build": + raise ValueError("Release backend must match the reviewed Hatchling build") + requirements = {} + for requirement in project["build-system"]["requires"]: + match = re.fullmatch(r"([A-Za-z0-9_.-]+)==([0-9][A-Za-z0-9_.]*)", requirement) + if match is None or match[1] in requirements: + raise ValueError("Release build requirements must be unique exact pins") + requirements[match[1]] = match[2] + if "hatchling" not in requirements: + raise ValueError("Release build requires an exact Hatchling pin") + return requirements + + +def wheel_identity( + path: Path, *, expected_version: str, hatchling_version: str +) -> dict: + """Bind wheel bytes to the requested release and its declared backend.""" + with ZipFile(path) as wheel: + bad_member = wheel.testzip() + if bad_member is not None: + raise ValueError(f"Invalid wheel member: {bad_member}") + infos = [name for name in wheel.namelist() if name.endswith(".dist-info/WHEEL")] + if len(infos) != 1: + raise ValueError("Expected exactly one wheel metadata directory") + prefix = infos[0].removesuffix("WHEEL") + metadata = wheel.read(prefix + "METADATA").decode() + wheel_metadata = wheel.read(infos[0]).decode() + if re.findall(r"^Version: (.+)$", metadata, re.MULTILINE) != [expected_version]: + raise ValueError("Wheel version differs from the prepared release") + if re.findall(r"^Generator: (.+)$", wheel_metadata, re.MULTILINE) != [ + f"hatchling {hatchling_version}" + ]: + raise ValueError("Wheel generator differs from the pinned backend") + return { + "filename": path.name, + "sha256": _sha256(path), + "size_bytes": path.stat().st_size, + } + + +def write_build_receipt(root: Path = ROOT) -> Path: + """Fail if tests changed package inputs, then retain only public build facts.""" + subprocess.run( + ["git", "diff", "--exit-code", "HEAD", "--", "policyengine_us"], + cwd=root, + check=True, + ) + untracked = subprocess.check_output( + ["git", "ls-files", "--others", "--exclude-standard", "--", "policyengine_us"], + cwd=root, + ) + if untracked.strip(): + raise ValueError("Untracked policy source would enter the release wheel") + project = tomllib.loads((root / "pyproject.toml").read_text()) + lock = tomllib.loads((root / "uv.lock").read_text()) + requirements = checked_build_requirements(project) + frontend = version("build") + locked_frontends = [ + item["version"] for item in lock["package"] if item["name"] == "build" + ] + if locked_frontends != [frontend]: + raise ValueError("Installed build frontend differs from the registry lock") + wheels = sorted((root / "dist").glob("*.whl")) + if len(wheels) != 1: + raise ValueError("Expected exactly one freshly built release wheel") + receipt = { + "source_head": subprocess.check_output( + ["git", "rev-parse", "HEAD"], cwd=root, text=True + ).strip(), + "policy_tree": subprocess.check_output( + ["git", "rev-parse", "HEAD:policyengine_us"], cwd=root, text=True + ).strip(), + "policy_source_matches_head": True, + "prepared_version": project["project"]["version"], + "pyproject_sha256": _sha256(root / "pyproject.toml"), + "uv_lock_sha256": _sha256(root / "uv.lock"), + "python": platform.python_version(), + "platform": platform.system(), + "machine": platform.machine(), + "runner_image": { + "os": os.environ.get("ImageOS"), + "version": os.environ.get("ImageVersion"), + }, + "uv": subprocess.check_output(["uv", "--version"], text=True).strip(), + "frontend": {"name": "build", "version": frontend}, + "isolated_build_requirements": requirements, + "wheel": wheel_identity( + wheels[0], + expected_version=project["project"]["version"], + hatchling_version=requirements["hatchling"], + ), + } + output = root / "release-build" / "receipt.json" + output.parent.mkdir(exist_ok=True) + output.write_text(json.dumps(receipt, indent=2, sort_keys=True) + "\n") + return output + + +if __name__ == "__main__": + import argparse + + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--prepare", choices=("rc1", "final")) + args = parser.parse_args() + if args.prepare: + prepare_candidate(ROOT, args.prepare) + else: + print(write_build_receipt().read_text(), end="") diff --git a/.github/tests/test_release_build.py b/.github/tests/test_release_build.py new file mode 100644 index 00000000000..2eb27d5416b --- /dev/null +++ b/.github/tests/test_release_build.py @@ -0,0 +1,209 @@ +"""Release artifact contracts without importing the country model.""" + +import importlib.util +import json +from pathlib import Path +import re +import subprocess +import tempfile +import tomllib +import unittest +from unittest.mock import patch +from zipfile import ZipFile + + +ROOT = Path(__file__).resolve().parents[2] +SPEC = importlib.util.spec_from_file_location( + "release_build", ROOT / ".github/release_build.py" +) +release_build = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(release_build) + + +def job(text, name): + return re.search( + rf"^ {name}:\n(.*?)(?=^ [A-Za-z]|\Z)", text, re.MULTILINE | re.DOTALL + )[1] + + +class WorkflowTests(unittest.TestCase): + def test_candidate_build_matches_publish_without_publication_or_policy_mutations( + self, + ): + pr = (ROOT / ".github/workflows/pr.yaml").read_text() + candidate = job(pr, "CandidateWheel") + publish = job((ROOT / ".github/workflows/push.yaml").read_text(), "Publish") + commands = ( + "python .github/release_lock.py --committed", + 'python .github/release_build.py --prepare "${{ matrix.kind }}"', + "python .github/release_lock.py --refresh", + "uv sync --locked --extra dev", + "uv run --no-sync make", + "uv run --no-sync python .github/release_build.py", + "actions/upload-artifact@", + ) + self.assertEqual( + [candidate.index(value) for value in commands], + sorted(candidate.index(value) for value in commands), + ) + self.assertIn("github.event.pull_request.head.sha", candidate) + self.assertIn("kind: [rc1, final]", candidate) + self.assertIn("release-wheel-${{ matrix.kind }}-", candidate) + for expected in ( + "runs-on: ubuntu-24.04", + 'python-version: "3.14.7"', + 'version: "0.12.13"', + "uv sync --locked --extra dev", + "uv run --no-sync make", + "python .github/release_build.py", + ): + self.assertIn(expected, candidate) + self.assertIn(expected, publish) + for prohibited in ( + "update_itemization", + "pypi-publish", + "secrets.", + "git push", + "contents: write", + ): + self.assertNotIn(prohibited, candidate) + self.assertIn("dist/*.whl", candidate) + self.assertIn("release-build/receipt.json", candidate) + self.assertNotIn("run: uv build", job(pr, "Quick-Feedback")) + self.assertLess( + publish.index("python .github/release_build.py"), + publish.index("pypa/gh-action-pypi-publish"), + ) + + def test_isolated_backend_closure_is_exactly_pinned(self): + project = tomllib.loads((ROOT / "pyproject.toml").read_text()) + pins = release_build.checked_build_requirements(project) + self.assertEqual( + set(pins), + { + "hatchling", + "packaging", + "pathspec", + "pluggy", + "tomlkit", + "trove-classifiers", + }, + ) + project["build-system"]["requires"][0] = "hatchling>=1.32" + with self.assertRaises(ValueError): + release_build.checked_build_requirements(project) + + +class PreparationTests(unittest.TestCase): + def test_each_candidate_uses_actual_fragment_version_helper(self): + for category, next_version in ( + ("fixed", "4.2.4"), + ("added", "4.3.0"), + ("breaking", "5.0.0"), + ): + for kind in ("rc1", "final"): + with self.subTest(category=category, kind=kind): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + (root / ".github").mkdir() + (root / ".github/bump_version.py").write_bytes( + (ROOT / ".github/bump_version.py").read_bytes() + ) + (root / "changelog.d").mkdir() + (root / f"changelog.d/test.{category}.md").write_text( + "Change\n" + ) + project = root / "pyproject.toml" + project.write_text('[project]\nversion = "4.2.3"\n') + release_build.prepare_candidate(root, kind) + expected = next_version + ("rc1" if kind == "rc1" else "") + self.assertEqual( + tomllib.loads(project.read_text())["project"]["version"], + expected, + ) + + +class ReceiptTests(unittest.TestCase): + def setUp(self): + temporary = tempfile.TemporaryDirectory(prefix="release-artifact-test-") + self.addCleanup(temporary.cleanup) + self.root = Path(temporary.name) + self.policy = self.root / "policyengine_us" / "parameter.yaml" + self.policy.parent.mkdir() + self.policy.write_text("branching: true\n") + (self.root / "pyproject.toml").write_text( + '[project]\nversion = "1.0.0"\n[build-system]\nrequires = ["hatchling==1.32.0"]\nbuild-backend = "hatchling.build"\n' + ) + (self.root / "uv.lock").write_text( + '[[package]]\nname = "build"\nversion = "1.4.0"\n' + ) + for args in ( + ("init", "-q"), + ("add", "."), + ( + "-c", + "user.name=Fixture", + "-c", + "user.email=fixture@example.invalid", + "commit", + "-qm", + "fixture", + ), + ): + subprocess.run( + ["git", *args], cwd=self.root, check=True, capture_output=True + ) + (self.root / "dist").mkdir() + self.wheel = self.root / "dist" / "policyengine_us-1.0.0-py3-none-any.whl" + self.write_wheel() + + def write_wheel(self, version="1.0.0", generator="hatchling 1.32.0"): + with ZipFile(self.wheel, "w") as wheel: + wheel.writestr("policyengine_us/parameter.yaml", self.policy.read_bytes()) + wheel.writestr( + "policyengine_us-1.0.0.dist-info/METADATA", + f"Name: policyengine-us\nVersion: {version}\n", + ) + wheel.writestr( + "policyengine_us-1.0.0.dist-info/WHEEL", + f"Wheel-Version: 1.0\nGenerator: {generator}\n", + ) + + def test_receipt_binds_actual_wheel_and_permits_only_root_version_preparation(self): + project_path = self.root / "pyproject.toml" + project_path.write_text(project_path.read_text().replace('"1.0.0"', '"1.0.1"')) + self.write_wheel(version="1.0.1") + with patch.object(release_build, "version", return_value="1.4.0"): + output = release_build.write_build_receipt(self.root) + receipt = json.loads(output.read_text()) + self.assertEqual(receipt["prepared_version"], "1.0.1") + self.assertEqual(receipt["wheel"]["sha256"], release_build._sha256(self.wheel)) + self.assertTrue(receipt["policy_source_matches_head"]) + self.assertEqual(receipt["frontend"]["version"], "1.4.0") + + def test_changed_policy_parameter_is_rejected_before_receipt(self): + self.policy.write_text("branching: false\n") + with self.assertRaises(subprocess.CalledProcessError): + release_build.write_build_receipt(self.root) + self.assertFalse((self.root / "release-build/receipt.json").exists()) + + def test_untracked_policy_source_is_rejected(self): + (self.policy.parent / "extra.py").write_text("pass\n") + with self.assertRaisesRegex(ValueError, "Untracked policy source"): + release_build.write_build_receipt(self.root) + + def test_wrong_generator_or_version_is_rejected(self): + for version, generator in ( + ("1.0.1", "hatchling 1.32.0"), + ("1.0.0", "hatchling 1.31.0"), + ): + self.write_wheel(version=version, generator=generator) + with self.assertRaises(ValueError): + release_build.wheel_identity( + self.wheel, expected_version="1.0.0", hatchling_version="1.32.0" + ) + + def test_frontend_drift_is_rejected(self): + with patch.object(release_build, "version", return_value="1.6.1"): + with self.assertRaisesRegex(ValueError, "frontend differs"): + release_build.write_build_receipt(self.root) diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index 08446d9bcba..3e3a91d52d2 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -38,7 +38,51 @@ jobs: - name: Test guard without importing the country model env: RELEASE_LOCK_REAL_UV: "1" - run: python -m unittest discover -s .github/tests -p test_release_lock.py -v + run: python -m unittest discover -s .github/tests -p 'test_release_*.py' -v + CandidateWheel: + name: Unpublished release wheel (${{ matrix.kind }}) + needs: [ReleaseLock, ReleaseLockTests] + permissions: + contents: read + runs-on: ubuntu-24.04 + strategy: + fail-fast: false + matrix: + kind: [rc1, final] + steps: + - name: Checkout reviewed PR head + uses: actions/checkout@v6 + with: + ref: ${{ github.event.pull_request.head.sha }} + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: "3.14.7" + - name: Install uv + uses: astral-sh/setup-uv@v8.1.0 + with: + version: "0.12.13" + - name: Check committed registry lock + run: python .github/release_lock.py --committed + - name: Predict release version with the release helper + run: python .github/release_build.py --prepare "${{ matrix.kind }}" + - name: Refresh only the release version in the registry lock + run: python .github/release_lock.py --refresh + - name: Install package + run: uv sync --locked --extra dev + - name: Build package from unchanged policy source + run: uv run --no-sync make + - name: Record and verify release build + run: uv run --no-sync python .github/release_build.py + - name: Retain unpublished wheel and build receipt + uses: actions/upload-artifact@v4 + with: + name: release-wheel-${{ matrix.kind }}-${{ github.event.pull_request.head.sha }} + path: | + dist/*.whl + release-build/receipt.json + if-no-files-found: error + retention-days: 14 Lint: needs: ReleaseLock runs-on: ubuntu-latest @@ -245,8 +289,6 @@ jobs: name: codecov-umbrella token: ${{ secrets.CODECOV_TOKEN }} verbose: true - - name: Build package - run: uv build Baseline: needs: ReleaseLock name: Full Suite - Baseline (${{ matrix.group }}) diff --git a/.github/workflows/push.yaml b/.github/workflows/push.yaml index 3353b150b53..62636db7d47 100644 --- a/.github/workflows/push.yaml +++ b/.github/workflows/push.yaml @@ -38,7 +38,7 @@ jobs: - name: Test guard without importing the country model env: RELEASE_LOCK_REAL_UV: "1" - run: python -m unittest discover -s .github/tests -p test_release_lock.py -v + run: python -m unittest discover -s .github/tests -p 'test_release_*.py' -v Lint: needs: ReleaseLock runs-on: ubuntu-latest @@ -328,7 +328,7 @@ jobs: - name: Run microsimulation tests run: uv run --no-sync make test-microsimulation Publish: - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 if: | (github.repository == 'PolicyEngine/policyengine-us') && (github.event.head_commit.message == 'Update PolicyEngine US') @@ -339,7 +339,7 @@ jobs: - name: Set up Python uses: actions/setup-python@v6 with: - python-version: 3.14 + python-version: "3.14.7" - name: Install uv uses: astral-sh/setup-uv@v8.1.0 with: @@ -350,6 +350,8 @@ jobs: run: uv sync --locked --extra dev - name: Build package run: uv run --no-sync make + - name: Record and verify release build + run: uv run --no-sync python .github/release_build.py - name: Publish a Python distribution to PyPI uses: pypa/gh-action-pypi-publish@release/v1 with: diff --git a/changelog.d/care-housing-income.fixed.md b/changelog.d/care-housing-income.fixed.md new file mode 100644 index 00000000000..3ad06524d59 --- /dev/null +++ b/changelog.d/care-housing-income.fixed.md @@ -0,0 +1 @@ +Keep SPM housing subsidy valuation within SPM resources and use actual housing assistance for household benefits and CBO transfers. Exclude housing subsidies from California CARE/FERA income from the August 14, 2014 parameter date (2015 for annual calculations). Preserve earlier modeled housing inclusion using actual assistance; its historical policy basis was not revalidated. diff --git a/changelog.d/release-wheel-candidate.fixed.md b/changelog.d/release-wheel-candidate.fixed.md new file mode 100644 index 00000000000..e6436a61eb2 --- /dev/null +++ b/changelog.d/release-wheel-candidate.fixed.md @@ -0,0 +1,4 @@ +Retain unpublished wheels for the predicted release and its rc1 version in +pull-request CI, built from unchanged policy source with the same pinned build +tools as publication. Record their source, package version, generator, lock, and file hash +for verification before release. diff --git a/changelog.d/spm-housing-allocation.fixed.md b/changelog.d/spm-housing-allocation.fixed.md new file mode 100644 index 00000000000..c8543a7dbe2 --- /dev/null +++ b/changelog.d/spm-housing-allocation.fixed.md @@ -0,0 +1 @@ +Prorate household housing assistance and the tenant contributions of awarded families across SPM units by member share before applying each unit's SPM resource cap, while preserving program awards and general benefit income. Document the tenant-payment allocation as a modeling assumption. diff --git a/docs/spm.md b/docs/spm.md index 231dc38b197..cf198fa5a48 100644 --- a/docs/spm.md +++ b/docs/spm.md @@ -5,9 +5,47 @@ measurement amounts. It does not extrapolate thresholds with country CPI or inco parameters. Each supported year and scenario comes from the verified artifact. The installed 1.0.0 artifact covers 2022 through 2035; unavailable years fail. Taxes, benefits, resources and the housing-assistance cap -remain country-model formulas. The cap uses the canonical housing amount before +remain country-model formulas. General household benefits and CBO transfer +aggregates count actual `housing_assistance`; only SPM resources count +`spm_unit_capped_housing_subsidy`. The cap uses the canonical housing amount before the final model storage conversion. +## Housing allocation and its assumptions + +The country sums modeled `housing_assistance` within each household, then +allocates the total to SPM units in proportion to their member counts. Each +unit's cap applies after this allocation. This follows the subsidy proration in +[Census's technical documentation, page 14](https://www2.census.gov/programs-surveys/supplemental-poverty-measure/datasets/spm/spm_techdoc.pdf#page=14). +The allocated amount is an SPM resource valuation; general benefits still count +the original program awards. Multiple actual awards supplied in a household +are summed, not discarded. + +The associated Microcosm producer uses independently reported household +public-housing or reduced-rent status, with these declared assumptions: + +- **A1, assisted family:** the householder's SPM unit represents the assisted + program family when the survey cannot identify its membership. This does not + imply that housing law permits only one assisted family per household. +- **A2, timing:** interview-time housing receipt represents full-year receipt + for the income year; it is not an observed twelve-month payment history. +- **A3, program coverage:** public housing and reported reduced rent are modeled + through the existing HUD-family calculation, although reduced rent may include + programs outside HUD. +- **A4, tenant contribution:** for SPM valuation, sum `hud_ttp` only for units + with positive modeled housing assistance, then allocate that sum using the + same member shares. This preserves the contribution associated with actual + awards and excludes nonrecipients' hypothetical tenant payments. Census + describes a household contribution but does not specify this multi-unit + allocation; A4 is a consistency assumption, not verified Census parity. + +The model retains its existing HUD program-family approximation, including +family income and utility treatment. These allocation variables do not alter +HUD eligibility, payments or utility allowances. Zero-award households need no +SPM geography for their zero housing resource. In assisted households, every +SPM unit receiving an allocated share needs a valid county and adult composition. + +## Measurement configuration + `Simulation`, `Microsimulation` and `CountryTaxBenefitSystem` accept a serializable `spm` mapping. Its fields are `forecast_content_sha256`, `scenario`, `geography_kind`, `geography_id`, `county_vintage` and `as_of`. The hash, when given, @@ -21,12 +59,26 @@ counties raise `SPM_GEOGRAPHY_UNAVAILABLE`. There is no first-county, congressio district or national fallback in SPM measurement. These geography errors occur only when calculating an SPM measurement or a -dependent resource, such as the housing-assistance cap for units with housing -assistance; units with none are capped at zero without consulting the +dependent SPM resource, such as the housing-assistance cap for units allocated +housing assistance; units with no allocation are capped at zero without consulting the measurement. A state-only tax request can still run. SPM reads the input-only `county_fips` variable and ignores any county inferred or cached by other tax or benefit formulas. Geography and composition errors are `SPMInputError` instances with `code` and `to_dict()`. +California CARE income excludes housing subsidies under +[CPUC Decision 14-08-030, Section 6.2](https://liob.cpuc.ca.gov/wp-content/uploads/sites/14/2020/12/ACF22B3.pdf#page=75) +and [Ordering Paragraph 40(3)](https://liob.cpuc.ca.gov/wp-content/uploads/sites/14/2020/12/ACF22B3.pdf#page=124). +[PG&E's June 2026 reply brief](https://docs.cpuc.ca.gov/PublishedDocs/Efile/G000/M608/K305/608305628.PDF#page=10) +confirms that its CARE/FERA income determination excludes housing subsidies, +despite contrary wording in its application. The shared CARE/FERA parameter +records the decision's effective date, August 14, 2014. Annual calculations +select the list at January 1, so calendar 2014 retains the earlier modeled +inclusion and calendar 2015 first applies the exclusion. Before that transition, +the model retains housing-subsidy inclusion using actual assistance; this update +does not validate the earlier income rule's historical policy basis. CARE/FERA +income never consults the SPM housing cap. The SPM resource calculation retains +its housing cap. + A caller can consciously select national measurement: ```python @@ -58,7 +110,11 @@ household head/spouse primitives support household scenarios; the model never guesses those roles from age ordering. A person counts as an SPM adult at age 18, or at age 15 or above with `is_spm_independent_minor_role`. That role defaults to the input-only `is_household_head | is_household_spouse` and can be supplied from -source data. A unit with no classified adult raises +source data. `policyengine_us.spm.DATASET_SOURCE_INPUTS` explicitly declares +this role as source-owned despite its household fallback formula. Population +producers must supply the observed boolean; the declaration does not authorize +filling missing source roles with the model's default value. +A unit with no classified adult raises `SPM_COMPOSITION_REQUIRED`. Generic age-based adult/child counts and benefit eligibility are unchanged. The dataset loader rejects stored formula-owned SPM outputs; observed source results should use report-only column names. It does not diff --git a/policyengine_us/parameters/gov/household/cbo_means_tested_transfers.yaml b/policyengine_us/parameters/gov/household/cbo_means_tested_transfers.yaml index c988f66ede8..f8081bc5a9d 100644 --- a/policyengine_us/parameters/gov/household/cbo_means_tested_transfers.yaml +++ b/policyengine_us/parameters/gov/household/cbo_means_tested_transfers.yaml @@ -4,7 +4,7 @@ values: - medicaid - chip - snap - - spm_unit_capped_housing_subsidy + - housing_assistance - ssi - tanf - free_school_meals @@ -16,3 +16,6 @@ metadata: unit: list period: year label: CBO means-tested transfers + reference: + - title: Congressional Budget Office, Reconciling the Official Poverty Measure and CBO’s Distributional Analysis of Household Income, Including In-Kind Transfers + href: https://www.cbo.gov/publication/61157 diff --git a/policyengine_us/parameters/gov/household/household_benefits.yaml b/policyengine_us/parameters/gov/household/household_benefits.yaml index be0a6e69cf3..4f7023ff818 100644 --- a/policyengine_us/parameters/gov/household/household_benefits.yaml +++ b/policyengine_us/parameters/gov/household/household_benefits.yaml @@ -27,7 +27,7 @@ values: # Contributed. - basic_income - trump_dividend - - spm_unit_capped_housing_subsidy + - housing_assistance - household_state_benefits - household_health_benefits 2024-01-01: @@ -53,7 +53,7 @@ values: # Contributed. - basic_income - trump_dividend - - spm_unit_capped_housing_subsidy + - housing_assistance - household_state_benefits - commodity_supplemental_food_program - household_health_benefits diff --git a/policyengine_us/parameters/gov/states/ca/cpuc/income_sources.yaml b/policyengine_us/parameters/gov/states/ca/cpuc/income_sources.yaml index 2f43cdfca8a..6728fbb331a 100644 --- a/policyengine_us/parameters/gov/states/ca/cpuc/income_sources.yaml +++ b/policyengine_us/parameters/gov/states/ca/cpuc/income_sources.yaml @@ -1,14 +1,16 @@ description: >- - Income sources counted toward CA CPUC CARE/FERA eligibility. - Total gross annual household income includes all taxable and - nontaxable revenues from all people living in the home, from - whatever sources derived, including, but not limited to, wages, - salaries, interest, dividends, spousal and child support payments, - public assistance payments, Social Security and pensions, housing - and military subsidies, rental income, income from self-employment - and all employment-related, non-cash income. -# PG&E Form 01-9077, CARE/FERA Program Application. + California counts these income sources under the California Alternate + Rates for Energy and Family Electric Rate Assistance programs. +# Housing subsidies are excluded by D.14-08-030, effective August 14, 2014. +# PG&E confirmed in its June 2026 reply brief that its contrary application +# wording does not describe its actual CARE/FERA income determination. +# The decision names CARE/ESA; PG&E's filed CARE/FERA practice supports +# applying this housing exclusion to the shared income definition. +# Annual income calculations select the list at January 1: calendar 2014 +# retains the earlier list, and calendar 2015 first applies the exclusion. values: + # Preserve the earlier model's inclusion before this decision, using the + # actual benefit. The pre-2014 income rule has not been revalidated here. 0000-01-01: # Wages, salaries - employment_income @@ -36,8 +38,49 @@ values: - pension_income - retirement_distributions # Housing subsidies - - spm_unit_capped_housing_subsidy - # Military subsidies + - housing_assistance + # Veterans benefits and military service income. These broad inputs do not + # identify the separately excluded military subsidies. + - veterans_benefits + - military_service_income + # Rental income + - rental_income + - farm_rent_income + # Other income sources (from whatever sources derived) + - capital_gains + - unemployment_compensation + - miscellaneous_income + - gi_cash_assistance + - debt_relief + - illicit_income + 2014-08-14: + # Wages, salaries + - employment_income + # Self-employment + - self_employment_income + - sstb_self_employment_income + - partnership_s_corp_income + - farm_operations_income + # Interest + - interest_income + # Dividends + - dividend_income + # Spousal support + - alimony_income + # Child support + - child_support_received + # Public assistance payments + - tanf + - ssi + - snap + - wic + # Social Security + - social_security + # Pensions + - pension_income + - retirement_distributions + # Veterans benefits and military service income. These broad inputs do not + # identify the separately excluded military subsidies. - veterans_benefits - military_service_income # Rental income @@ -50,15 +93,26 @@ values: - gi_cash_assistance - debt_relief - illicit_income - metadata: unit: list period: year label: CA CPUC CARE/FERA income sources reference: - - title: PG&E Form 01-9077, CARE/FERA Program Application - href: https://bha.berkeleyca.gov/sites/default/files/2022-04/PGE%20CARE%20FERA%20Application.pdf + - title: Statewide ESA Program Policy and Procedures Manual v1.4, Section 2.2.2 + href: https://www.cpuc.ca.gov/-/media/cpuc-website/consumer-support/documents/20250418-statewide-esa-program-pp-manualv14.pdf#page=12 + - title: Statewide ESA Program Policy and Procedures Manual v1.4, Table E-1 + href: https://www.cpuc.ca.gov/-/media/cpuc-website/consumer-support/documents/20250418-statewide-esa-program-pp-manualv14.pdf#page=81 + # Retain the form's general income categories; its housing/military + # subsidy wording is corrected by the decision and PG&E filing below. + - title: PG&E Form 01-9077, CARE/FERA Program Application, Income Definition + href: https://www.pge.com/tariffs/assets/pdf/tariffbook/GAS_FORMS_01-9077.pdf#page=2 + - title: CPUC Decision 14-08-030, Section 6.2, Income Definition + href: https://liob.cpuc.ca.gov/wp-content/uploads/sites/14/2020/12/ACF22B3.pdf#page=75 + - title: CPUC Decision 14-08-030, Ordering Paragraph 40(3) + href: https://liob.cpuc.ca.gov/wp-content/uploads/sites/14/2020/12/ACF22B3.pdf#page=124 + - title: PG&E Reply Brief, June 5, 2026, Section B + href: https://docs.cpuc.ca.gov/PublishedDocs/Efile/G000/M608/K305/608305628.PDF#page=10 - title: CA PUC 739.1 (CARE) href: https://leginfo.legislature.ca.gov/faces/codes_displaySection.xhtml?lawCode=PUC§ionNum=739.1 - title: CA PUC 739.12 (FERA) diff --git a/policyengine_us/reforms/congress/tlaib/boost/boost_middle_class_tax_credit.py b/policyengine_us/reforms/congress/tlaib/boost/boost_middle_class_tax_credit.py index f56b87f3bc9..39d1fa1a645 100644 --- a/policyengine_us/reforms/congress/tlaib/boost/boost_middle_class_tax_credit.py +++ b/policyengine_us/reforms/congress/tlaib/boost/boost_middle_class_tax_credit.py @@ -72,15 +72,13 @@ def formula(household, period, parameters): "unemployment_compensation", # Contributed. "basic_income", - "spm_unit_capped_housing_subsidy", + "housing_assistance", "household_state_benefits", "household_head_start_benefits", ] if parameters(period).gov.hud.abolition: BENEFITS = [ - benefit - for benefit in BENEFITS - if benefit != "spm_unit_capped_housing_subsidy" + benefit for benefit in BENEFITS if benefit != "housing_assistance" ] previous_benefits = add(household, period, BENEFITS) middle_class_credit = add( diff --git a/policyengine_us/reforms/congress/tlaib/economic_dignity_for_all_agenda/edaa_end_child_poverty_act.py b/policyengine_us/reforms/congress/tlaib/economic_dignity_for_all_agenda/edaa_end_child_poverty_act.py index 728643b9908..9b28bed81db 100644 --- a/policyengine_us/reforms/congress/tlaib/economic_dignity_for_all_agenda/edaa_end_child_poverty_act.py +++ b/policyengine_us/reforms/congress/tlaib/economic_dignity_for_all_agenda/edaa_end_child_poverty_act.py @@ -98,9 +98,7 @@ def formula(household, period, parameters): BENEFITS = list(parameters(period).gov.household.household_benefits) if parameters(period).gov.hud.abolition: BENEFITS = [ - benefit - for benefit in BENEFITS - if benefit != "spm_unit_capped_housing_subsidy" + benefit for benefit in BENEFITS if benefit != "housing_assistance" ] # Add ECPA child benefit diff --git a/policyengine_us/reforms/congress/tlaib/end_child_poverty_act.py b/policyengine_us/reforms/congress/tlaib/end_child_poverty_act.py index f24d8e49ffc..c15c853aaaa 100644 --- a/policyengine_us/reforms/congress/tlaib/end_child_poverty_act.py +++ b/policyengine_us/reforms/congress/tlaib/end_child_poverty_act.py @@ -108,16 +108,14 @@ def formula(household, period, parameters): "unemployment_compensation", # Contributed. "basic_income", - "spm_unit_capped_housing_subsidy", + "housing_assistance", "household_state_benefits", "household_head_start_benefits", "ecpa_child_benefit", ] if parameters(period).gov.hud.abolition: BENEFITS = [ - benefit - for benefit in BENEFITS - if benefit != "spm_unit_capped_housing_subsidy" + benefit for benefit in BENEFITS if benefit != "housing_assistance" ] return add(household, period, BENEFITS) diff --git a/policyengine_us/tests/core/test_spm_source_input_contract.py b/policyengine_us/tests/core/test_spm_source_input_contract.py new file mode 100644 index 00000000000..853b5ea5901 --- /dev/null +++ b/policyengine_us/tests/core/test_spm_source_input_contract.py @@ -0,0 +1,42 @@ +"""A household fallback formula must not hide an observed population role.""" + +from policyengine_core.periods import ETERNITY +import pytest + +from policyengine_us import Simulation +from policyengine_us.spm import DATASET_FORMULA_OWNED_INPUTS, DATASET_SOURCE_INPUTS +from policyengine_us.system import system + + +def test_source_contract_is_disjoint_and_matches_registered_role(): + assert DATASET_SOURCE_INPUTS == {"is_spm_independent_minor_role"} + assert not DATASET_SOURCE_INPUTS & DATASET_FORMULA_OWNED_INPUTS + variable = system.variables["is_spm_independent_minor_role"] + assert variable.entity.key == "person" + assert variable.value_type is bool + assert variable.definition_period == ETERNITY + assert variable.formulas # Household fallback remains available. + + +@pytest.mark.parametrize("source_role,adults", [(None, 1), (False, 1), (True, 2)]) +def test_observed_role_can_override_household_fallback(source_role, adults): + head = {"age": {2024: 40}, "is_household_head": True} + minor = {"age": {2024: 17}} + if source_role is not None: + head["is_spm_independent_minor_role"] = True + minor["is_spm_independent_minor_role"] = source_role + simulation = Simulation( + situation={ + "people": { + "head": head, + "minor": minor, + }, + "households": {"household": {"members": ["head", "minor"]}}, + "spm_units": {"unit": {"members": ["head", "minor"]}}, + } + ) + assert simulation.calculate("spm_measurement_adults", 2024).tolist() == [adults] + assert simulation.calculate("is_spm_independent_minor_role", ETERNITY).tolist() == [ + True, + bool(source_role), + ] diff --git a/policyengine_us/tests/policy/baseline/gov/states/ca/cpuc/ca_cpuc_countable_income.yaml b/policyengine_us/tests/policy/baseline/gov/states/ca/cpuc/ca_cpuc_countable_income.yaml index aa9eda413ec..b95a58c4a11 100644 --- a/policyengine_us/tests/policy/baseline/gov/states/ca/cpuc/ca_cpuc_countable_income.yaml +++ b/policyengine_us/tests/policy/baseline/gov/states/ca/cpuc/ca_cpuc_countable_income.yaml @@ -40,3 +40,75 @@ state_code: CA output: ca_cpuc_countable_income: 68_000 + +- name: Case 4, the earlier annual income convention uses actual housing assistance. + period: 2014 + input: + age: 40 + state_code: CA + employment_income: 1_000 + self_employment_income: 0 + sstb_self_employment_income: 0 + partnership_s_corp_income: 0 + farm_operations_income: 0 + interest_income: 0 + dividend_income: 0 + alimony_income: 0 + child_support_received: 0 + tanf: 0 + ssi: 0 + snap: 0 + wic: 0 + social_security: 0 + pension_income: 0 + retirement_distributions: 0 + housing_assistance: 500 + veterans_benefits: 0 + military_service_income: 0 + rental_income: 0 + farm_rent_income: 0 + capital_gains: 0 + unemployment_compensation: 0 + miscellaneous_income: 0 + gi_cash_assistance: 0 + debt_relief: 0 + illicit_income: 0 + spm_unit_capped_housing_subsidy: 25 + output: + ca_cpuc_countable_income: 1_500 + +- name: Case 5, annual income excludes housing assistance after the dated decision. + period: 2015 + input: + age: 40 + state_code: CA + employment_income: 1_000 + self_employment_income: 0 + sstb_self_employment_income: 0 + partnership_s_corp_income: 0 + farm_operations_income: 0 + interest_income: 0 + dividend_income: 0 + alimony_income: 0 + child_support_received: 0 + tanf: 0 + ssi: 0 + snap: 0 + wic: 0 + social_security: 0 + pension_income: 0 + retirement_distributions: 0 + housing_assistance: 500 + veterans_benefits: 0 + military_service_income: 0 + rental_income: 0 + farm_rent_income: 0 + capital_gains: 0 + unemployment_compensation: 0 + miscellaneous_income: 0 + gi_cash_assistance: 0 + debt_relief: 0 + illicit_income: 0 + spm_unit_capped_housing_subsidy: 25 + output: + ca_cpuc_countable_income: 1_000 diff --git a/policyengine_us/tests/policy/baseline/gov/states/ca/cpuc/integration.yaml b/policyengine_us/tests/policy/baseline/gov/states/ca/cpuc/integration.yaml index dd5dac449b4..55dcbe0cb9d 100644 --- a/policyengine_us/tests/policy/baseline/gov/states/ca/cpuc/integration.yaml +++ b/policyengine_us/tests/policy/baseline/gov/states/ca/cpuc/integration.yaml @@ -97,3 +97,75 @@ ca_care_eligible: false ca_fera_eligible: true ca_fera: 18 + +- name: Case 6, housing assistance does not disqualify an income-eligible CARE household. + period: 2024 + input: + people: + person1: + age: 40 + employment_income: 40_700 + pre_subsidy_rent: 36_000 + spm_units: + unit: + members: [person1] + spm_unit_tenure_type: RENTER + receives_housing_assistance: true + pre_subsidy_electricity_expense: 1_200 + households: + household: + members: [person1] + state_code: CA + pha_payment_standard: 36_000 + output: + # Tenant payment is 30% of income. The $36,000 payment standard equals + # gross rent, so the HUD benefit is $36,000 minus the tenant payment. + hud_ttp: 12_210 + housing_assistance: 23_790 + # CPUC D.14-08-030 excludes housing subsidies from countable income. + ca_cpuc_countable_income: 40_700 + ca_care_categorically_eligible: false + ca_care_income_eligible: true + ca_care_eligible: true + ca_care: 390 + ca_fera_eligible: false + ca_fera: 0 + +- name: Case 7, housing assistance does not disqualify an income-eligible FERA household. + period: 2024 + input: + people: + person1: + age: 40 + employment_income: 21_000 + pre_subsidy_rent: 36_000 + person2: + age: 40 + employment_income: 21_000 + person3: + age: 40 + employment_income: 21_000 + spm_units: + unit: + members: [person1, person2, person3] + spm_unit_tenure_type: RENTER + receives_housing_assistance: true + pre_subsidy_electricity_expense: 1_200 + households: + household: + members: [person1, person2, person3] + state_code: CA + pha_payment_standard: 36_000 + output: + # Tenant payment = 30% of $63,000; the $36,000 payment standard equals + # gross rent, so the HUD benefit is $36,000 - $18,900 = $17,100. + hud_ttp: 18_900 + housing_assistance: 17_100 + ca_cpuc_countable_income: 63_000 + ca_care_categorically_eligible: false + ca_care_income_eligible: false + ca_care_eligible: false + ca_care: 0 + ca_fera_eligible: true + # FERA discount = 18% of $1,200. + ca_fera: 216 diff --git a/policyengine_us/tests/policy/baseline/household/income/household/cbo_household_income.yaml b/policyengine_us/tests/policy/baseline/household/income/household/cbo_household_income.yaml index cb61fc7a9a8..275f2797e57 100644 --- a/policyengine_us/tests/policy/baseline/household/income/household/cbo_household_income.yaml +++ b/policyengine_us/tests/policy/baseline/household/income/household/cbo_household_income.yaml @@ -44,7 +44,8 @@ medicaid: 10 chip: 5 snap: 7 - spm_unit_capped_housing_subsidy: 8 + housing_assistance: 8 + spm_unit_capped_housing_subsidy: 1 ssi: 4 tanf: 3 free_school_meals: 2 diff --git a/policyengine_us/tests/policy/baseline/household/income/household/household_benefits.yaml b/policyengine_us/tests/policy/baseline/household/income/household/household_benefits.yaml index 545b50217f6..1a3c84f6175 100644 --- a/policyengine_us/tests/policy/baseline/household/income/household/household_benefits.yaml +++ b/policyengine_us/tests/policy/baseline/household/income/household/household_benefits.yaml @@ -20,7 +20,7 @@ head_start: 0 early_head_start: 0 basic_income: 0 - spm_unit_capped_housing_subsidy: 0 + housing_assistance: 0 household_state_benefits: 0 household_health_benefits: 0 output: @@ -51,7 +51,7 @@ head_start: 0 early_head_start: 0 basic_income: 0 - spm_unit_capped_housing_subsidy: 10 + housing_assistance: 10 household_state_benefits: 0 commodity_supplemental_food_program: 0 household_health_benefits: 0 @@ -80,10 +80,33 @@ acp: 0 ebb: 0 basic_income: 0 - spm_unit_capped_housing_subsidy: 0 + housing_assistance: 0 household_state_benefits: 0 commodity_supplemental_food_program: 0 household_health_benefits: 0 output: household_head_start_benefits: 0 household_benefits: 0 + +- name: Case 4, household benefits use the full housing payment rather than its SPM valuation. + period: 2024 + input: + age: 40 + housing_assistance: 12_000 + spm_unit_capped_housing_subsidy: 1_000 + social_security: 0 + ssi: 0 + tanf: 0 + unemployment_compensation: 0 + snap: 0 + wic: 0 + free_school_meals: 0 + reduced_price_school_meals: 0 + acp: 0 + ebb: 0 + household_state_benefits: 0 + household_health_benefits: 0 + output: + household_benefits: 12_000 + # The SPM resource valuation remains separate. + spm_unit_benefits: 1_000 diff --git a/policyengine_us/tests/policy/baseline/household/income/household/household_net_income.yaml b/policyengine_us/tests/policy/baseline/household/income/household/household_net_income.yaml index bad1d7579a7..8ca8a254933 100644 --- a/policyengine_us/tests/policy/baseline/household/income/household/household_net_income.yaml +++ b/policyengine_us/tests/policy/baseline/household/income/household/household_net_income.yaml @@ -20,7 +20,7 @@ head_start: 0 early_head_start: 0 basic_income: 0 - spm_unit_capped_housing_subsidy: 0 + housing_assistance: 0 household_state_benefits: 0 household_health_benefits: 0 household_refundable_tax_credits: 0 @@ -55,7 +55,7 @@ head_start: 0 early_head_start: 0 basic_income: 0 - spm_unit_capped_housing_subsidy: 10 + housing_assistance: 10 household_state_benefits: 0 commodity_supplemental_food_program: 0 household_health_benefits: 0 @@ -88,7 +88,7 @@ acp: 0 ebb: 0 basic_income: 0 - spm_unit_capped_housing_subsidy: 0 + housing_assistance: 0 household_state_benefits: 0 commodity_supplemental_food_program: 0 household_health_benefits: 0 @@ -122,7 +122,7 @@ acp: 0 ebb: 0 basic_income: 0 - spm_unit_capped_housing_subsidy: 0 + housing_assistance: 0 household_state_benefits: 0 commodity_supplemental_food_program: 0 household_health_benefits: 0 diff --git a/policyengine_us/tests/policy/contrib/congress/tlaib/boost/boost_middle_class_tax_credit.yaml b/policyengine_us/tests/policy/contrib/congress/tlaib/boost/boost_middle_class_tax_credit.yaml index ff1a3215a61..b37e8c153b3 100644 --- a/policyengine_us/tests/policy/contrib/congress/tlaib/boost/boost_middle_class_tax_credit.yaml +++ b/policyengine_us/tests/policy/contrib/congress/tlaib/boost/boost_middle_class_tax_credit.yaml @@ -173,11 +173,12 @@ residential_efficiency_electrification_rebate: 0 unemployment_compensation: 0 basic_income: 0 - spm_unit_capped_housing_subsidy: 0 + housing_assistance: 12_000 + spm_unit_capped_housing_subsidy: 1_000 household_state_benefits: 0 output: - household_benefits: 1 + 2 + 3 + 4 + 5 - spm_unit_benefits: 1 + 2 + 3 + 4 + 5 + household_benefits: 1 + 2 + 3 + 4 + 5 + 12_000 + spm_unit_benefits: 1 + 2 + 3 + 4 + 5 + 1_000 - name: BOOST household benefits exclude housing subsidy if HUD is abolished. period: 2024 @@ -204,7 +205,9 @@ residential_efficiency_electrification_rebate: 0 unemployment_compensation: 0 basic_income: 0 + housing_assistance: 10 spm_unit_capped_housing_subsidy: 10 household_state_benefits: 0 output: household_benefits: 0 + spm_unit_benefits: 0 diff --git a/policyengine_us/tests/policy/contrib/congress/tlaib/economic_dignity_for_all_agenda/edaa_end_child_poverty_act.yaml b/policyengine_us/tests/policy/contrib/congress/tlaib/economic_dignity_for_all_agenda/edaa_end_child_poverty_act.yaml index c58353426fe..cf8c3331d02 100644 --- a/policyengine_us/tests/policy/contrib/congress/tlaib/economic_dignity_for_all_agenda/edaa_end_child_poverty_act.yaml +++ b/policyengine_us/tests/policy/contrib/congress/tlaib/economic_dignity_for_all_agenda/edaa_end_child_poverty_act.yaml @@ -221,12 +221,13 @@ residential_efficiency_electrification_rebate: 0 unemployment_compensation: 0 basic_income: 0 - spm_unit_capped_housing_subsidy: 0 + housing_assistance: 12_000 + spm_unit_capped_housing_subsidy: 1_000 household_state_benefits: 0 household_health_benefits: 0 output: - household_benefits: 1 + 2 + 3 + 4 + 5 - spm_unit_benefits: 1 + 2 + 3 + 4 + 5 + household_benefits: 1 + 2 + 3 + 4 + 5 + 12_000 + spm_unit_benefits: 1 + 2 + 3 + 4 + 5 + 1_000 - name: EDAA ECPA household benefits exclude housing subsidy if HUD is abolished. period: 2026 @@ -254,9 +255,11 @@ head_start: 0 early_head_start: 0 basic_income: 0 + housing_assistance: 10 spm_unit_capped_housing_subsidy: 10 household_state_benefits: 0 commodity_supplemental_food_program: 0 household_health_benefits: 0 output: household_benefits: 0 + spm_unit_benefits: 0 diff --git a/policyengine_us/tests/policy/contrib/congress/tlaib/end_child_poverty_act/integration.yaml b/policyengine_us/tests/policy/contrib/congress/tlaib/end_child_poverty_act/integration.yaml index c4e598fdfda..1d9d3f1e04e 100644 --- a/policyengine_us/tests/policy/contrib/congress/tlaib/end_child_poverty_act/integration.yaml +++ b/policyengine_us/tests/policy/contrib/congress/tlaib/end_child_poverty_act/integration.yaml @@ -121,11 +121,12 @@ residential_efficiency_electrification_rebate: 0 unemployment_compensation: 0 basic_income: 0 - spm_unit_capped_housing_subsidy: 0 + housing_assistance: 12_000 + spm_unit_capped_housing_subsidy: 1_000 household_state_benefits: 0 output: - household_benefits: 1 + 2 + 3 + 4 + 5 - spm_unit_benefits: 1 + 2 + 3 + 4 + 5 + household_benefits: 1 + 2 + 3 + 4 + 5 + 12_000 + spm_unit_benefits: 1 + 2 + 3 + 4 + 5 + 1_000 - name: End Child Poverty Act household benefits exclude housing subsidy if HUD is abolished. period: 2024 @@ -152,7 +153,9 @@ residential_efficiency_electrification_rebate: 0 unemployment_compensation: 0 basic_income: 0 + housing_assistance: 10 spm_unit_capped_housing_subsidy: 10 household_state_benefits: 0 output: household_benefits: 0 + spm_unit_benefits: 0 diff --git a/policyengine_us/tests/policy/reform/ecpa_head_start_benefits_composition.yaml b/policyengine_us/tests/policy/reform/ecpa_head_start_benefits_composition.yaml index 797d3424607..e64a68262af 100644 --- a/policyengine_us/tests/policy/reform/ecpa_head_start_benefits_composition.yaml +++ b/policyengine_us/tests/policy/reform/ecpa_head_start_benefits_composition.yaml @@ -34,7 +34,7 @@ residential_efficiency_electrification_rebate: 0 unemployment_compensation: 0 basic_income: 0 - spm_unit_capped_housing_subsidy: 0 + housing_assistance: 0 household_state_benefits: 0 output: household_head_start_benefits: 29_000 diff --git a/policyengine_us/tests/unit/test_cpuc_spm_independence.py b/policyengine_us/tests/unit/test_cpuc_spm_independence.py new file mode 100644 index 00000000000..b723f2d8d44 --- /dev/null +++ b/policyengine_us/tests/unit/test_cpuc_spm_independence.py @@ -0,0 +1,80 @@ +"""CPUC income rules exclude housing subsidies, irrespective of SPM settings. + +Explicit SPM configuration and provenance comparisons require the Python API; +the ordinary policy outcomes are also covered by CPUC YAML integration cases. +""" + +from copy import deepcopy + +import numpy as np +import pytest + +from policyengine_us import Simulation + + +@pytest.mark.parametrize("assisted", [False, True]) +def test_assisted_care_income_is_independent_of_spm_selection(assisted): + year = 2024 + situation = { + "people": { + "person1": { + "age": {year: 40}, + "employment_income": {year: 40_700}, + "pre_subsidy_rent": {year: 36_000}, + } + }, + "spm_units": { + "unit": { + "members": ["person1"], + "spm_unit_tenure_type": {year: "RENTER"}, + "receives_housing_assistance": {year: assisted}, + "takes_up_housing_assistance_if_eligible": {year: assisted}, + "pre_subsidy_electricity_expense": {year: 1_200}, + } + }, + "households": { + "household": { + "members": ["person1"], + "state_code": {year: "CA"}, + "pha_payment_standard": {year: 36_000}, + } + }, + } + outputs = [ + "ca_cpuc_countable_income", + "ca_care_income_eligible", + "ca_care_eligible", + "ca_care", + "ca_fera_eligible", + "ca_fera", + ] + results = [] + for spm, county in [ + (None, None), + ({"geography_kind": "national"}, None), + ({"geography_kind": "county"}, "06037"), + ]: + inputs = deepcopy(situation) + if county is not None: + inputs["households"]["household"]["county_fips"] = {year: county} + simulation = Simulation(situation=inputs, spm=spm) + assistance = simulation.calculate("housing_assistance", year)[0] + assert bool(assistance > 0) is assisted + results.append([simulation.calculate(name, year)[0] for name in outputs]) + assert simulation.spm_provenance()["years"] == {} + + # D.14-08-030 excludes housing subsidies from CARE income. The modeled + # 2024 CARE limit is $40,880 and the discount is 32.5% of $1,200. + for result in results: + np.testing.assert_array_equal(result, [40_700, True, True, 390, False, 0]) + + +def test_cpuc_housing_exclusion_effective_date(): + parameters = Simulation.default_tax_benefit_system_instance.parameters + before = parameters("2014-08-13").gov.states.ca.cpuc.income_sources + after = parameters("2014-08-14").gov.states.ca.cpuc.income_sources + assert "housing_assistance" in before + assert list(after) == [name for name in before if name != "housing_assistance"] + assert list(parameters("2026-01-01").gov.states.ca.cpuc.income_sources) == list( + after + ) diff --git a/policyengine_us/tests/unit/test_spm_housing_allocation.py b/policyengine_us/tests/unit/test_spm_housing_allocation.py new file mode 100644 index 00000000000..5e232d37faf --- /dev/null +++ b/policyengine_us/tests/unit/test_spm_housing_allocation.py @@ -0,0 +1,296 @@ +"""Conserve program awards while allocating housing resources for the SPM.""" + +from copy import deepcopy + +import numpy as np +import pytest + +from policyengine_us import Simulation +from policyengine_core.periods import period +from policyengine_core.reforms import Reform +from spm_calculator.errors import SPMInputError + + +YEAR = 2025 + + +def split_households(): + # Unequal unit sizes, interleaved people, and an independent household + # distinguish member-share allocation from duplication or array alignment. + return { + "people": {name: {"age": {YEAR: 40}} for name in ("a", "b", "c", "d", "e")}, + "spm_units": { + "head_family": { + "members": ["a"], + "housing_assistance": {YEAR: 12_000}, + "hud_ttp": {YEAR: 0}, + "spm_unit_tenure_type": {YEAR: "RENTER"}, + }, + "other_household": { + "members": ["b", "d"], + "housing_assistance": {YEAR: 6_000}, + "hud_ttp": {YEAR: 0}, + "spm_unit_tenure_type": {YEAR: "RENTER"}, + }, + "co_residents": { + "members": ["c", "e"], + "housing_assistance": {YEAR: 0}, + "hud_ttp": {YEAR: 0}, + "spm_unit_tenure_type": {YEAR: "RENTER"}, + }, + }, + "households": { + "shared_home": {"members": ["a", "c", "e"]}, + "separate_home": {"members": ["b", "d"]}, + }, + } + + +def test_subsidy_allocation_conserves_each_households_actual_awards(): + sim = Simulation(situation=split_households()) + np.testing.assert_array_equal( + sim.calculate("spm_unit_allocated_housing_subsidy", YEAR), + [4_000, 6_000, 8_000], + ) + np.testing.assert_array_equal( + sim.calculate("housing_assistance", YEAR), [12_000, 6_000, 0] + ) + np.testing.assert_array_equal( + sim.calculate("spm_unit_allocated_housing_subsidy", YEAR, map_to="household"), + [12_000, 6_000], + ) + # Allocation uses no threshold or geography and records no SPM calculation. + assert sim.spm_provenance()["years"] == {} + + +def test_multiple_actual_awards_are_preserved_before_spm_allocation(): + situation = split_households() + situation["spm_units"]["co_residents"]["housing_assistance"] = {YEAR: 3_000} + sim = Simulation(situation=situation) + np.testing.assert_array_equal( + sim.calculate("spm_unit_allocated_housing_subsidy", YEAR), + [5_000, 6_000, 10_000], + ) + np.testing.assert_array_equal( + sim.calculate("housing_assistance", YEAR), [12_000, 6_000, 3_000] + ) + + +def test_allocated_subsidy_reaches_nonrecipient_spm_unit_before_its_cap(): + sim = Simulation(situation=split_households(), spm={"geography_kind": "national"}) + np.testing.assert_array_equal( + sim.calculate("spm_unit_capped_housing_subsidy", YEAR), + [4_000, 6_000, 8_000], + ) + + +@pytest.mark.parametrize("amount", [0, 1, 12_345.67]) +def test_single_unit_allocation_preserves_program_amount(amount): + sim = Simulation( + situation={ + "people": {"a": {"age": {YEAR: 40}}}, + "spm_units": { + "unit": { + "members": ["a"], + "housing_assistance": {YEAR: amount}, + } + }, + } + ) + np.testing.assert_array_equal( + sim.calculate("spm_unit_allocated_housing_subsidy", YEAR), + sim.calculate("housing_assistance", YEAR), + ) + + +def test_zero_awards_need_no_spm_geography(): + situation = deepcopy(split_households()) + for unit in situation["spm_units"].values(): + unit["housing_assistance"] = {YEAR: 0} + sim = Simulation(situation=situation) + np.testing.assert_array_equal( + sim.calculate("spm_unit_capped_housing_subsidy", YEAR), [0, 0, 0] + ) + assert sim.spm_provenance()["years"] == {} + np.testing.assert_array_equal( + sim.calculate("spm_unit_allocated_tenant_payment", YEAR), [0, 0, 0] + ) + + +@pytest.mark.parametrize("nonrecipient_ttp", [25, 45_000]) +def test_only_awarded_families_contribute_tenant_payment(nonrecipient_ttp): + situation = split_households() + units = situation["spm_units"] + units["head_family"]["hud_ttp"] = {YEAR: 3_600} + units["other_household"]["hud_ttp"] = {YEAR: 900} + units["co_residents"]["hud_ttp"] = {YEAR: nonrecipient_ttp} + sim = Simulation(situation=situation, spm={"geography_kind": "national"}) + np.testing.assert_array_equal( + sim.calculate("spm_unit_allocated_tenant_payment", YEAR), [1_200, 900, 2_400] + ) + np.testing.assert_array_equal( + sim.calculate("spm_unit_allocated_tenant_payment", YEAR, map_to="household"), + [3_600, 900], + ) + # Even the high counterfactual liability must leave a positive resource + # for the co-resident unit that receives part of the household subsidy. + assert sim.calculate("spm_unit_capped_housing_subsidy", YEAR)[2] > 0 + + +def test_independent_unit_caps_do_not_redistribute_unused_housing_value(): + situation = split_households() + situation["spm_units"]["head_family"]["hud_ttp"] = {YEAR: 90_000} + sim = Simulation(situation=situation, spm={"geography_kind": "national"}) + np.testing.assert_array_equal( + sim.calculate("spm_unit_allocated_tenant_payment", YEAR), [30_000, 0, 60_000] + ) + np.testing.assert_array_equal( + sim.calculate("spm_unit_capped_housing_subsidy", YEAR), [0, 6_000, 0] + ) + np.testing.assert_array_equal( + sim.calculate("housing_assistance", YEAR), [12_000, 6_000, 0] + ) + + +def real_hud_household(lodger_income=0, lodger_receipt=False): + return { + "people": { + "head": { + "age": {YEAR: 40}, + "is_household_head": {YEAR: True}, + "employment_income": {YEAR: 12_000}, + "pre_subsidy_rent": {YEAR: 36_000}, + }, + "lodger": { + "age": {YEAR: 30}, + "employment_income": {YEAR: lodger_income}, + }, + "co_resident": {"age": {YEAR: 25}}, + }, + "spm_units": { + "head_family": { + "members": ["head"], + "receives_housing_assistance": {YEAR: True}, + "takes_up_housing_assistance_if_eligible": {YEAR: True}, + "spm_unit_tenure_type": {YEAR: "RENTER"}, + }, + "lodger_family": { + "members": ["lodger", "co_resident"], + "receives_housing_assistance": {YEAR: lodger_receipt}, + "takes_up_housing_assistance_if_eligible": {YEAR: lodger_receipt}, + "spm_unit_tenure_type": {YEAR: "RENTER"}, + }, + }, + "households": { + "shared_home": { + "members": ["head", "lodger", "co_resident"], + "state_code": {YEAR: "CA"}, + "county_fips": {YEAR: "06037"}, + "bedrooms": {YEAR: 2}, + "tenant_pays_utilities": {YEAR: True}, + "tenure_type": {YEAR: "RENTED"}, + } + }, + "tax_units": { + name: {"members": [name]} for name in ("head", "lodger", "co_resident") + }, + "families": { + name: {"members": [name]} for name in ("head", "lodger", "co_resident") + }, + "marital_units": { + name: {"members": [name]} for name in ("head", "lodger", "co_resident") + }, + } + + +@pytest.mark.parametrize( + "lodger_income,lodger_receipt", [(0, False), (150_000, False), (150_000, True)] +) +def test_actual_hud_award_ignores_nonawarded_units_hypothetical_payment( + lodger_income, lodger_receipt +): + sim = Simulation(situation=real_hud_household(lodger_income, lodger_receipt)) + np.testing.assert_allclose( + sim.calculate("housing_assistance", YEAR), [28_392, 0], atol=0.01 + ) + np.testing.assert_allclose( + sim.calculate("spm_unit_allocated_housing_subsidy", YEAR), + [9_464, 18_928], + atol=0.01, + ) + np.testing.assert_allclose( + sim.calculate("spm_unit_allocated_tenant_payment", YEAR), + [1_200, 2_400], + atol=0.01, + ) + capped = sim.calculate("spm_unit_capped_housing_subsidy", YEAR) + assert (capped > 0).all() + assert ( + sim.calculate("spm_unit_capped_housing_subsidy", YEAR, map_to="household")[0] + <= 28_392 + ) + + +def test_multiple_actual_awards_conserve_subsidy_and_contributions(): + sim = Simulation(situation=real_hud_household(lodger_receipt=True)) + np.testing.assert_allclose( + sim.calculate("housing_assistance", YEAR), [28_392, 2_639], atol=0.01 + ) + for name, expected in [ + ("spm_unit_allocated_housing_subsidy", 31_031), + ("spm_unit_allocated_tenant_payment", 3_625), + ]: + np.testing.assert_allclose( + sim.calculate(name, YEAR, map_to="household"), [expected], atol=0.01 + ) + + +def test_spm_selection_changes_measurement_without_changing_general_benefits(): + outputs = [] + for selection in ({"geography_kind": "county"}, {"geography_kind": "national"}): + sim = Simulation(situation=real_hud_household(), spm=selection) + outputs.append( + { + name: sim.calculate(name, YEAR) + for name in ( + "housing_assistance", + "household_benefits", + "spm_unit_capped_housing_subsidy", + ) + } + ) + for name in ("housing_assistance", "household_benefits"): + np.testing.assert_array_equal(outputs[0][name], outputs[1][name]) + assert not np.array_equal( + outputs[0]["spm_unit_capped_housing_subsidy"], + outputs[1]["spm_unit_capped_housing_subsidy"], + ) + + +def test_hud_abolition_zeroes_allocations_and_needs_no_measurement(): + def abolish(parameters): + parameters.gov.hud.abolition.update(period=period(YEAR), value=True) + return parameters + + class AbolishHUD(Reform): + def apply(self): + self.modify_parameters(abolish) + + sim = Simulation(situation=real_hud_household(), reform=AbolishHUD) + for name in ( + "housing_assistance", + "spm_unit_allocated_housing_subsidy", + "spm_unit_allocated_tenant_payment", + "spm_unit_capped_housing_subsidy", + ): + np.testing.assert_array_equal(sim.calculate(name, YEAR), [0, 0]) + assert sim.spm_provenance()["years"] == {} + + +def test_allocated_unit_with_no_classified_adult_raises_composition_error(): + situation = real_hud_household() + for name in ("lodger", "co_resident"): + situation["people"][name]["age"] = {YEAR: 10} + sim = Simulation(situation=situation) + with pytest.raises(SPMInputError, match="no classified adult"): + sim.calculate("spm_unit_capped_housing_subsidy", YEAR) diff --git a/policyengine_us/tests/unit/test_spm_measurement_boundary.py b/policyengine_us/tests/unit/test_spm_measurement_boundary.py new file mode 100644 index 00000000000..2c75172b353 --- /dev/null +++ b/policyengine_us/tests/unit/test_spm_measurement_boundary.py @@ -0,0 +1,104 @@ +"""Keep SPM valuation caps out of policy benefits and general income measures.""" + +import ast +from pathlib import Path + +import yaml + + +def test_spm_caps_have_only_spm_resource_consumers(): + root = Path(__file__).resolve().parents[2] + # New reporting leaves require an explicit update to this allowed graph. + consumers = { + "spm_unit_allocated_housing_subsidy": {"spm_unit_capped_housing_subsidy"}, + "spm_unit_allocated_tenant_payment": {"spm_unit_capped_housing_subsidy"}, + "spm_unit_capped_housing_subsidy": {"spm_unit_benefits"}, + "spm_unit_capped_work_childcare_expenses": {"spm_unit_spm_expenses"}, + "spm_unit_head_spouse_earned_cap": {"spm_unit_capped_work_childcare_expenses"}, + "spm_unit_spm_expenses": {"spm_unit_net_income"}, + "spm_unit_benefits": {"spm_unit_net_income"}, + "spm_unit_net_income": { + "spm_unit_is_in_spm_poverty", + "spm_unit_is_in_deep_spm_poverty", + "poverty_gap", + "deep_poverty_gap", + "spm_unit_oecd_equiv_net_income", + }, + "spm_unit_oecd_equiv_net_income": {"spm_unit_income_decile"}, + "spm_unit_income_decile": set(), + "poverty_gap": {"in_poverty"}, + "deep_poverty_gap": {"in_deep_poverty"}, + "spm_unit_is_in_spm_poverty": set(), + "spm_unit_is_in_deep_spm_poverty": set(), + "in_poverty": {"person_in_poverty"}, + "in_deep_poverty": set(), + "person_in_poverty": set(), + } + violations = [] + + class Visitor(ast.NodeVisitor): + def __init__(self, path): + self.path = path + self.consumer = None + + def visit_ClassDef(self, node): + previous = self.consumer + self.consumer = node.name + self.generic_visit(node) + self.consumer = previous + + def visit_Constant(self, node): + if isinstance(node.value, str) and node.value in consumers: + # References outside a variable class also fail explicitly. + if self.consumer not in consumers[node.value]: + violations.append( + f"{self.path.relative_to(root)}:{node.lineno}: " + f"{self.consumer} consumes {node.value}" + ) + + def visit_Expr(self, node): + # A docstring describes a variable; it does not calculate one. + if not isinstance(node.value, ast.Constant) or not isinstance( + node.value.value, str + ): + self.generic_visit(node) + + def visit_Assign(self, node): + metadata = {"documentation", "label", "reference"} + if not all( + isinstance(target, ast.Name) and target.id in metadata + for target in node.targets + ): + self.generic_visit(node) + + for directory in ("variables", "reforms"): + for path in (root / directory).rglob("*.py"): + Visitor(path).visit(ast.parse(path.read_text(), filename=str(path))) + + def scalar_values(value): + if isinstance(value, dict): + for key, child in value.items(): + if key == "description": + continue + if key == "metadata": + # A breakdown can name a variable; descriptive metadata + # cannot introduce a calculated dependency. + if isinstance(child, dict): + yield from scalar_values(child.get("breakdown", [])) + continue + yield from scalar_values(child) + elif isinstance(value, list): + for child in value: + yield from scalar_values(child) + else: + yield value + + # Include nested values in brackets/breakdowns. BaseLoader preserves + # year-zero date keys without constructing unsupported datetime objects. + for path in (root / "parameters").rglob("*.yaml"): + parameter = yaml.load(path.read_text(), Loader=yaml.BaseLoader) + for value in scalar_values(parameter): + if value in consumers: + violations.append(f"{path.relative_to(root)} names {value}") + + assert not violations, "\n".join(violations) diff --git a/policyengine_us/variables/household/income/household/household_benefits.py b/policyengine_us/variables/household/income/household/household_benefits.py index 274b1892c61..f5e709b7764 100644 --- a/policyengine_us/variables/household/income/household/household_benefits.py +++ b/policyengine_us/variables/household/income/household/household_benefits.py @@ -12,8 +12,6 @@ def formula(household, period, parameters): BENEFITS = list(parameters(period).gov.household.household_benefits) if parameters(period).gov.hud.abolition: BENEFITS = [ - benefit - for benefit in BENEFITS - if benefit != "spm_unit_capped_housing_subsidy" + benefit for benefit in BENEFITS if benefit != "housing_assistance" ] return add(household, period, BENEFITS) diff --git a/policyengine_us/variables/household/income/spm_unit/spm_unit_allocated_housing_subsidy.py b/policyengine_us/variables/household/income/spm_unit/spm_unit_allocated_housing_subsidy.py new file mode 100644 index 00000000000..992860bc208 --- /dev/null +++ b/policyengine_us/variables/household/income/spm_unit/spm_unit_allocated_housing_subsidy.py @@ -0,0 +1,20 @@ +from policyengine_us.model_api import * + + +class spm_unit_allocated_housing_subsidy(Variable): + value_type = float + entity = SPMUnit + label = "Housing subsidy allocated to the SPM unit before its resource cap" + definition_period = YEAR + unit = USD + reference = "https://www2.census.gov/programs-surveys/supplemental-poverty-measure/datasets/spm/spm_techdoc.pdf#page=14" + + def formula(spm_unit, period, parameters): + # Census prorates the household subsidy by each SPM unit's member + # share. This allocation is a measurement step; it does not move the + # underlying program award or change another family's eligibility. + simulation = spm_unit.simulation + household_subsidy = simulation.calculate( + "housing_assistance", period, map_to="household" + ) + return simulation.map_result(household_subsidy, "household", "spm_unit") diff --git a/policyengine_us/variables/household/income/spm_unit/spm_unit_allocated_tenant_payment.py b/policyengine_us/variables/household/income/spm_unit/spm_unit_allocated_tenant_payment.py new file mode 100644 index 00000000000..4db1c988620 --- /dev/null +++ b/policyengine_us/variables/household/income/spm_unit/spm_unit_allocated_tenant_payment.py @@ -0,0 +1,24 @@ +from policyengine_us.model_api import * + + +class spm_unit_allocated_tenant_payment(Variable): + value_type = float + entity = SPMUnit + label = "Tenant payment allocated to the SPM unit for its housing resource cap" + definition_period = YEAR + unit = USD + reference = "https://www2.census.gov/programs-surveys/supplemental-poverty-measure/datasets/spm/spm_techdoc.pdf#page=14" + + def formula(spm_unit, period, parameters): + awarded = spm_unit("housing_assistance", period) > 0 + if not awarded.any(): + return np.zeros(spm_unit.count) + # A4: allocate the contribution of actually awarded program families + # alongside their subsidy. Census describes a household contribution + # but does not specify its multi-SPM-unit allocation. This assumption + # conserves that contribution; a nonrecipient's hypothetical HUD TTP + # must not reduce another family's allocated housing resource. + payments = where(awarded, spm_unit("hud_ttp", period), 0).astype("float64") + simulation = spm_unit.simulation + household_payments = simulation.map_result(payments, "spm_unit", "household") + return simulation.map_result(household_payments, "household", "spm_unit") diff --git a/policyengine_us/variables/household/income/spm_unit/spm_unit_capped_housing_subsidy.py b/policyengine_us/variables/household/income/spm_unit/spm_unit_capped_housing_subsidy.py index b24f0ecf20b..8e123bf641e 100644 --- a/policyengine_us/variables/household/income/spm_unit/spm_unit_capped_housing_subsidy.py +++ b/policyengine_us/variables/household/income/spm_unit/spm_unit_capped_housing_subsidy.py @@ -11,8 +11,10 @@ class spm_unit_capped_housing_subsidy(Variable): reference = "https://www2.census.gov/programs-surveys/supplemental-poverty-measure/datasets/spm/spm_techdoc.pdf" def formula(spm_unit, period, parameters): - housing_assistance = spm_unit("housing_assistance", period).astype("float64") - # Only a unit with housing assistance has anything to cap. Consulting + housing_assistance = spm_unit( + "spm_unit_allocated_housing_subsidy", period + ).astype("float64") + # Only a unit allocated housing assistance has anything to cap. Consulting # the canonical housing portion for the other units would demand SPM # geography and composition from resource and benefit consumers that # never use the measurement, so the cap is evaluated for assisted units @@ -25,6 +27,8 @@ def formula(spm_unit, period, parameters): housing_portion = masked_policyengine_amount( spm_unit, period, "housing_portion", assisted ) - tenant_payment = spm_unit("hud_ttp", period).astype("float64") + tenant_payment = spm_unit("spm_unit_allocated_tenant_payment", period).astype( + "float64" + ) cap = max_(housing_portion - tenant_payment, 0) return where(assisted, min_(housing_assistance, cap), 0) diff --git a/pyproject.toml b/pyproject.toml index 445483baaf3..dce3dcb61ab 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -33,7 +33,16 @@ dependencies = [ policyengine-us = "policyengine_us:main" [build-system] -requires = ["hatchling"] +# Exact build requirements also constrain the isolated backend environment +# created by python -m build; the runtime uv.lock does not pin that environment. +requires = [ + "hatchling==1.32.0", + "packaging==26.3", + "pathspec==1.1.1", + "pluggy==1.6.0", + "tomlkit==0.15.1", + "trove-classifiers==2026.6.1.19", +] build-backend = "hatchling.build" [project.optional-dependencies] From c4209228d7471ef8314b046841d719df5511686c Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 20:10:09 -0400 Subject: [PATCH 06/21] Guard dataset inputs, the default build and the batch runner's exit status From #9464, reduced. `REJECTED_DATASET_INPUTS` extends the calculator's `FORMULA_OWNED_INPUTS` with the country-side poverty aliases (`in_poverty`, `in_deep_poverty`, `deep_poverty_line`, `deep_poverty_gap`, `person_in_poverty`) and with the two SPM housing-allocation outputs #9448 adds, so a dataset can no longer store a formula-owned measurement under a name the calculator does not police. `DATASET_SOURCE_INPUTS` keeps `is_spm_independent_minor_role` declarable by a producer, and the two sets are asserted disjoint. `DEFAULT_DATASET_SHA256` pins the certified build the mutable Hugging Face tag must resolve to, verified once per process and only for the country default; an explicit `dataset=` argument remains the caller's own artifact. The batch runner now decides a batch on the child's real exit status, reported through a marker written before interpreter teardown, and enumerates `.yml` alongside `.yaml` everywhere it lists files. Dropped from #9464: `reported_housing_assistance` and the CPUC entry that counted it, plus the three tests that pinned housing assistance as countable - CPUC D.14-08-030 excludes housing subsidies outright, so there is nothing to gate on receipt and the variable had no other consumer. Its citation research is folded into the CPUC parameter instead. Adaptations: the dataset guard is grafted onto #9463's `set_input`; `test_spm_system.py`'s import block is the union of both PRs' needs; `test_care_eligibility_is_the_same_under_every_spm_geography` expects `(40_700, True)` because $40,700 of countable income is inside the 2024 one-person CARE limit of $40,880 once the subsidy is excluded; and the two counting non-vacuity tests are replaced by their exclusion analogues so the suite still fails if the housing entry is silently dropped rather than excluded. Co-Authored-By: Claude Fable 5.1 --- changelog.d/batched-exit-status.fixed.md | 1 + .../spm-dataset-poverty-alias-guard.fixed.md | 1 + changelog.d/spm-default-dataset-hash.fixed.md | 1 + policyengine_us/spm.py | 49 ++- policyengine_us/system.py | 57 ++++ .../code_health/test_batched_exit_status.py | 298 ++++++++++++++++ .../test_batched_per_subdir_budget.py | 50 +++ .../code_health/test_reform_combo_budget.py | 5 +- .../core/test_spm_source_input_contract.py | 4 +- policyengine_us/tests/core/test_spm_system.py | 236 ++++++++++++- policyengine_us/tests/test_batched.py | 319 ++++++++++++++---- .../unit/test_spm_integration_contract.py | 210 +++++++++++- .../variables/gov/hud/housing_assistance.py | 9 +- 13 files changed, 1151 insertions(+), 89 deletions(-) create mode 100644 changelog.d/batched-exit-status.fixed.md create mode 100644 changelog.d/spm-dataset-poverty-alias-guard.fixed.md create mode 100644 changelog.d/spm-default-dataset-hash.fixed.md create mode 100644 policyengine_us/tests/code_health/test_batched_exit_status.py diff --git a/changelog.d/batched-exit-status.fixed.md b/changelog.d/batched-exit-status.fixed.md new file mode 100644 index 00000000000..5d9ba9142a3 --- /dev/null +++ b/changelog.d/batched-exit-status.fixed.md @@ -0,0 +1 @@ +Decide a test batch by the child process exit status rather than its printed summary, and enumerate `.yml` test files alongside `.yaml`. diff --git a/changelog.d/spm-dataset-poverty-alias-guard.fixed.md b/changelog.d/spm-dataset-poverty-alias-guard.fixed.md new file mode 100644 index 00000000000..dcbc91a6109 --- /dev/null +++ b/changelog.d/spm-dataset-poverty-alias-guard.fixed.md @@ -0,0 +1 @@ +Reject derived poverty aliases such as `in_poverty` from stored datasets. diff --git a/changelog.d/spm-default-dataset-hash.fixed.md b/changelog.d/spm-default-dataset-hash.fixed.md new file mode 100644 index 00000000000..45d28e64013 --- /dev/null +++ b/changelog.d/spm-default-dataset-hash.fixed.md @@ -0,0 +1 @@ +Verify the default dataset content hash so a moved build id cannot silently change every standalone-country result. diff --git a/policyengine_us/spm.py b/policyengine_us/spm.py index 8a276d7b062..8cd58f73642 100644 --- a/policyengine_us/spm.py +++ b/policyengine_us/spm.py @@ -34,6 +34,53 @@ } ) +# Poverty outputs this country derives from the calculator's measurement. +# +# ``FORMULA_OWNED_INPUTS`` names what the calculator owns: the thresholds, the +# SPM resource total, and the canonical indicators ``poverty_line``, +# ``poverty_gap``, ``spm_unit_is_in_spm_poverty`` and +# ``spm_unit_is_in_deep_spm_poverty``. These names are country-side functions +# of those same values - ``in_poverty`` of ``poverty_gap``, +# ``deep_poverty_line`` of ``poverty_line``, ``deep_poverty_gap`` of +# ``deep_poverty_line`` and ``spm_unit_net_income``, ``in_deep_poverty`` of +# ``deep_poverty_gap``, and ``person_in_poverty`` of ``in_poverty`` - so a +# dataset column for any of them is an alias for a formula-owned output under a +# name the calculator does not police. Accepting one lets the poverty +# calculation documented in ``docs/usage/microsimulation.md`` and the canonical +# indicator publish different poverty rates for the same population: a dataset +# storing ``in_poverty=[False, False]`` alongside a population every unit of +# which is below its threshold reports 0% and 100% at once. +DERIVED_POVERTY_OUTPUTS = frozenset( + { + "deep_poverty_gap", + "deep_poverty_line", + "in_deep_poverty", + "in_poverty", + "person_in_poverty", + } +) + +# The household-to-SPM-unit housing allocation is computed here, from the +# modelled award and the awarded family's tenant contribution. A dataset that +# stored either quantity would override an allocation the country derives. +SPM_ALLOCATION_OUTPUTS = frozenset( + { + "spm_unit_allocated_housing_subsidy", + "spm_unit_allocated_tenant_payment", + } +) + +# Every name a dataset must not store, from either side of the boundary. +REJECTED_DATASET_INPUTS = ( + frozenset(FORMULA_OWNED_INPUTS) | DERIVED_POVERTY_OUTPUTS | SPM_ALLOCATION_OUTPUTS +) + +# This source role has a head/spouse fallback for household situations. A +# population producer must retain its observed boolean instead of treating the +# fallback formula as ownership of the input. This declaration permits source +# delivery; it does not permit synthesizing a default value when data are absent. +DATASET_SOURCE_INPUTS = frozenset({"is_spm_independent_minor_role"}) + COUNTY_FIPS_PATTERN = re.compile(r"[0-9]{5}") COUNTY_INPUT_FIX = ( @@ -878,7 +925,7 @@ def set_input(self, variable_name, period, value): # public consumers validate their household input contract separately. if ( getattr(self, "is_over_dataset", False) - and variable_name in FORMULA_OWNED_INPUTS + and variable_name in REJECTED_DATASET_INPUTS ): raise ValueError( f"Dataset supplies formula-owned SPM output {variable_name}. " diff --git a/policyengine_us/system.py b/policyengine_us/system.py index 6b83b8b3acd..f7798fd3cd6 100644 --- a/policyengine_us/system.py +++ b/policyengine_us/system.py @@ -1,3 +1,5 @@ +import hashlib +from functools import lru_cache from pathlib import Path from policyengine_core.taxbenefitsystems import TaxBenefitSystem from policyengine_us.entities import * @@ -53,6 +55,16 @@ # Populace ships from a Hugging Face *dataset* repo, hence the `hf://datasets/` # prefix handled in `_resolve_dataset_path`. DEFAULT_DATASET = "hf://datasets/policyengine/populace-us/populace_us_2024.h5@populace-us-2024-spm-20260909" +# Content hash of the certified build the default URI must resolve to. +# +# A Hugging Face tag is a mutable pointer: re-tagging the repository, or a +# truncated or substituted download, would hand the model another schema-valid +# H5 and change every standalone-country result with no rejection anywhere. +# Only the country's own default is checked - an explicit `dataset=` argument +# names the caller's own artifact and remains the caller's responsibility. +DEFAULT_DATASET_SHA256 = ( + "6496cc4393d4d3c6574f76eca231de5898c803b9067645591fd5c4d3e65aee84" +) class CountryTaxBenefitSystem(TaxBenefitSystem): @@ -312,6 +324,44 @@ def _resolve_dataset_path(dataset_str): raise FileNotFoundError(f"Dataset file not found: {dataset_str}") +def _file_sha256(file_path): + digest = hashlib.sha256() + with open(file_path, "rb") as handle: + for block in iter(lambda: handle.read(1024 * 1024), b""): + digest.update(block) + return digest.hexdigest() + + +@lru_cache(maxsize=None) +def _checked_default_dataset_digest(file_path, size, mtime_ns): + """Digest the resolved default build once per process. + + The certified build is ~830 MB, so a per-construction digest would add + about a third of a second to every default `Microsimulation`. Key the + digest on the resolved path together with its size and modification time, + so a replaced or re-downloaded cache entry is digested again rather than + inheriting the previous file's verdict. + """ + return _file_sha256(file_path) + + +def _verify_default_dataset(file_path): + """Reject a default build whose bytes are not the certified ones.""" + stat = Path(file_path).stat() + actual = _checked_default_dataset_digest( + str(file_path), stat.st_size, stat.st_mtime_ns + ) + if actual != DEFAULT_DATASET_SHA256: + raise ValueError( + f"The default dataset {DEFAULT_DATASET} resolved to {file_path}, " + f"whose content is not the certified build: expected sha256 " + f"{DEFAULT_DATASET_SHA256}, got {actual}. The build id in the " + "default URI points at different bytes than this model version " + "was certified against; upgrade policyengine-us, or pass an " + "explicit dataset= argument to use that artifact deliberately." + ) + + def _is_hdfstore_format(file_path): """Check if an HDF5 file uses entity-level HDFStore format. @@ -362,6 +412,9 @@ def __init__(self, *args, **kwargs): ) dataset = kwargs.get("dataset") + # Only the build this model version chose for itself is content-checked + # below; a caller-supplied dataset= is the caller's own artifact. + uses_country_default = dataset is None if dataset is None: # Route the class default through the same interception below as an # explicit dataset, so an entity-level (HDFStore) default such as @@ -381,6 +434,10 @@ def __init__(self, *args, **kwargs): # entity-level datasets, making this interception unnecessary. if dataset is not None and isinstance(dataset, str): local_path = _resolve_dataset_path(dataset) + if uses_country_default and dataset == DEFAULT_DATASET: + # A subclass may point `default_dataset` elsewhere; only the + # shipped default is certified by DEFAULT_DATASET_SHA256. + _verify_default_dataset(local_path) if _is_hdfstore_format(local_path): from policyengine_us.data.economic_assumptions import ( extend_single_year_dataset, diff --git a/policyengine_us/tests/code_health/test_batched_exit_status.py b/policyengine_us/tests/code_health/test_batched_exit_status.py new file mode 100644 index 00000000000..10d5421f9c8 --- /dev/null +++ b/policyengine_us/tests/code_health/test_batched_exit_status.py @@ -0,0 +1,298 @@ +"""The batch runner's verdict must come from the runner's own exit status. + +`test_batched.py` used to decide a batch by parsing the pytest summary line: +absent a `"N failed"` count it called the batch passed, then terminated the +child without ever reading its status. pytest prints no failure count for an +errored run, so a real child producing + + 1 passed, 1 error in 0.01s (actual exit status: 1) + +was reported as a passing batch, and `make test-yaml-*` exited 0 on it. + +The status cannot simply be waited for: policyengine-core takes tens of seconds +to tear down after pytest.main() returns, which is why the child is terminated +as soon as it reports. So the command runs through a shim that writes pytest's +return value to stdout the instant it has one, and the runner reads that. + +Each test here drives the real `run_batch` against a real subprocess whose +output and status are scripted, so the runner's own Popen/select/terminate path +is what is under test, not a stand-in for it. +""" + +import importlib.util +import sys +from pathlib import Path + +import pytest +import yaml + +BATCHER = Path(__file__).resolve().parents[1] / "test_batched.py" +SPEC = importlib.util.spec_from_file_location("test_batched", BATCHER) +batched = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(batched) + + +def scripted_child( + summary: str, + exit_code: int, + then_hang: bool = False, + report: bool = True, +): + """A command that prints a pytest-shaped summary, then finishes as told. + + With report=True it also writes the shim's exit marker, standing in for + the real runner; with report=False it stands in for a child that died + before pytest could hand back a status. + """ + body = f"import sys, time\nprint({summary!r}, flush=True)\n" + if report: + body += ( + f"print('{batched.BATCH_EXIT_MARKER} ' + str({exit_code}), flush=True)\n" + ) + if then_hang: + body += "time.sleep(600)\n" + body += f"sys.exit({exit_code})\n" + return [sys.executable, "-u", "-c", body] + + +PASSING_SUMMARY = "============ 2 passed in 0.01s ============" +# pytest prints no failure count for an errored run: this is the exact string +# from the reproduction, and the one the old parser scored as a pass. +ERRORED_SUMMARY = "============ 1 passed, 1 error in 0.01s ============" +FAILING_SUMMARY = "============ 1 failed, 1 passed in 0.01s ============" + + +def run_scripted(summary: str, exit_code: int, **kwargs): + return batched.run_batch( + ["unused/path.yaml"], + "Batch 1", + stream=False, + command=scripted_child(summary, exit_code, **kwargs), + ) + + +def test_summary_without_a_failure_count_does_not_override_an_error_exit(): + """The reproduced defect: "1 passed, 1 error" with child exit 1.""" + result = run_scripted(ERRORED_SUMMARY, 1) + assert result["returncode"] == 1 + assert result["status"] == "failed" + + +def test_a_clean_summary_still_fails_on_a_nonzero_exit(): + """Interrupted, internal-error and usage-error runs print no count either.""" + for exit_code in (2, 3, 4): + result = run_scripted(PASSING_SUMMARY, exit_code) + assert result["returncode"] == exit_code + assert result["status"] == "failed", exit_code + + +def test_no_tests_collected_is_a_failure_for_the_batch_runner(): + """This runner enumerates the suite, so an empty batch is broken enumeration. + + `run_selective_tests.py` deliberately skips pytest's exit 5 because it + selects paths from a changed-file list, where a changed test helper + legitimately collects nothing. Nothing selects paths here but this file. + """ + result = run_scripted("============ no tests ran in 0.01s ============", 5) + assert result["returncode"] == batched.PYTEST_NO_TESTS_COLLECTED + assert result["status"] == "failed" + assert "collected no tests" in batched.describe_exit_status(5) + + +def test_a_passing_batch_still_passes(): + """The guard must not turn the ordinary green run red.""" + result = run_scripted(PASSING_SUMMARY, 0) + assert result["returncode"] == 0 + assert result["status"] == "passed" + + +def test_a_reported_failure_stays_a_failure(): + result = run_scripted(FAILING_SUMMARY, 1) + assert result["status"] == "failed" + + +def test_a_slow_teardown_after_a_clean_run_still_passes(): + """The real runner takes tens of seconds to exit; that is not a failure. + + Waiting the child out instead of reading its report turned every large + green batch red: measured on this suite, a single small state took 18.2s + to exit after its summary and the large state batches took over 30s. + """ + result = run_scripted(PASSING_SUMMARY, 0, then_hang=True) + assert result["returncode"] == 0 + assert result["status"] == "passed" + + +def test_a_slow_teardown_after_a_failed_run_still_fails(): + result = run_scripted(FAILING_SUMMARY, 1, then_hang=True) + assert result["returncode"] == 1 + assert result["status"] == "failed" + + +def test_a_child_killed_before_reporting_is_not_credited_a_pass(): + """No report and a nonzero status: an OOM kill mid-run, not a pass.""" + result = run_scripted(PASSING_SUMMARY, 137, report=False) + assert result["returncode"] == 137 + assert result["status"] == "failed" + + +def test_the_shim_reports_the_real_runner_status(): + """End to end against policyengine-core, not a scripted stand-in. + + An empty directory collects nothing, so pytest exits 5 - a status the + summary line never carries and the runner now reports. + """ + import tempfile + + with tempfile.TemporaryDirectory() as empty: + result = batched.run_batch([empty], "Batch 1", stream=False) + assert batched.BATCH_EXIT_MARKER in result["output"] + assert result["returncode"] == batched.PYTEST_NO_TESTS_COLLECTED + assert result["status"] == "failed" + + +def test_a_child_that_never_prints_a_summary_is_judged_by_its_exit_status(): + """Crashes before the summary line (OOM kill, import error) still count.""" + crashing = [sys.executable, "-u", "-c", "import sys; sys.exit(1)"] + result = batched.run_batch( + ["unused/path.yaml"], "Batch 1", stream=False, command=crashing + ) + assert result["status"] == "failed" + + clean = [sys.executable, "-u", "-c", "pass"] + assert ( + batched.run_batch(["unused/path.yaml"], "Batch 1", stream=False, command=clean)[ + "status" + ] + == "passed" + ) + + +@pytest.mark.parametrize("returncode", [1, 2, 3, 4, 5, -9, -15, None]) +def test_batch_status_passes_nothing_but_a_zero_exit(returncode): + assert batched.batch_status(returncode) == "failed" + + +def test_batch_status_passes_a_zero_exit(): + assert batched.batch_status(0) == "passed" + + +# --- .yml enumeration -------------------------------------------------------- +# +# policyengine-core collects both suffixes (tools/test_runner.py:413), so a +# `.yml` test omitted from a file-level batch would never run, and the batch it +# belonged to would still report green. + + +def write_cases(path: Path, reforms: list) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + yaml.safe_dump( + [ + { + "name": f"{path.stem} {reform}", + "period": 2026, + "reforms": reform, + "input": {"people": {"person": {"age": 30}}}, + "output": {"age": 30}, + } + for reform in reforms + ] + ) + ) + + +def test_yaml_enumeration_covers_both_suffixes(tmp_path): + write_cases(tmp_path / "a.yaml", ["r1"]) + write_cases(tmp_path / "nested" / "b.yml", ["r2"]) + assert batched.yaml_files(tmp_path) == [ + tmp_path / "a.yaml", + tmp_path / "nested" / "b.yml", + ] + assert batched.root_yaml_files(tmp_path) == [tmp_path / "a.yaml"] + assert batched.count_yaml_files(tmp_path) == 2 + + +def test_heavy_subdir_splitting_includes_yml_files(tmp_path): + """The split that packs a heavy folder file by file must see every file.""" + heavy = tmp_path / "heavy" + write_cases(heavy / "a.yaml", ["r1", "r2", "r3", "r4"]) + write_cases(heavy / "b.yml", ["r5", "r6"]) + batches = batched.subdir_batches(heavy) + packed = {path for batch in batches for path in batch} + assert str(heavy / "b.yml") in packed + assert str(heavy / "a.yaml") in packed + + +def test_heavy_subdir_weight_counts_yml_files(tmp_path): + """A folder pushed over budget by `.yml` files alone must still split.""" + heavy = tmp_path / "heavy" + write_cases(heavy / "a.yml", ["r1", "r2", "r3", "r4"]) + write_cases(heavy / "b.yml", ["r5", "r6"]) + assert batched.subdir_batches(heavy) != [[str(heavy)]] + + +def test_per_file_mode_includes_yml_files(tmp_path): + write_cases(tmp_path / "a.yaml", ["r1"]) + write_cases(tmp_path / "b.yml", ["r2"]) + batches = batched.split_into_batches(tmp_path, 2, mode="per-file") + assert batches == [[str(tmp_path / "a.yaml")], [str(tmp_path / "b.yml")]] + + +def test_per_subdir_mode_collects_root_yml_files(tmp_path): + write_cases(tmp_path / "light" / "a.yaml", ["r1"]) + write_cases(tmp_path / "root.yml", ["r2"]) + batches = batched.split_into_batches(tmp_path, 2, mode="per-subdir") + assert batches == [[str(tmp_path / "light")], [str(tmp_path / "root.yml")]] + + +def test_default_split_and_exclude_branches_include_yml_files(tmp_path): + write_cases(tmp_path / "keep" / "a.yml", ["r1"]) + write_cases(tmp_path / "drop" / "b.yaml", ["r2"]) + write_cases(tmp_path / "root.yml", ["r3"]) + assert batched.split_into_batches(tmp_path, 1, exclude=["drop"]) == [ + [str(tmp_path / "keep"), str(tmp_path / "root.yml")] + ] + assert batched.split_into_batches(tmp_path, 2) == [ + [str(tmp_path / "drop" / "b.yaml"), str(tmp_path / "keep" / "a.yml")], + [str(tmp_path / "root.yml")], + ] + + +# --- The monitoring loop is bounded ------------------------------------------ +# +# The loop ends when the child exits or reports, and nothing else. The 30-minute +# batch budget sat below it, where the child had already gone and the wait it +# guarded returned instantly, so it never fired. Reading the status from the +# runner rather than terminating a second after the summary also opened a +# second window: pytest's session-finish hooks, between the summary line and +# pytest.main() returning. Both are bounded inside the loop now. + + +def test_a_child_that_never_reports_after_its_summary_is_abandoned(monkeypatch): + """Not a hang: terminated, and reported as the failure it is.""" + monkeypatch.setattr(batched, "MARKER_GRACE_SECONDS", 1) + result = run_scripted(PASSING_SUMMARY, 0, then_hang=True, report=False) + assert result["status"] == "failed" + assert "No status" in result["output"] + + +def test_a_child_that_never_says_anything_hits_the_batch_budget(monkeypatch): + """The 30-minute budget the runner documents now actually fires.""" + monkeypatch.setattr(batched, "BATCH_TIMEOUT_SECONDS", 1) + silent = [sys.executable, "-u", "-c", "import time; time.sleep(600)"] + result = batched.run_batch( + ["unused/path.yaml"], "Batch 1", stream=False, command=silent + ) + assert result["status"] == "timeout" + assert "Timeout" in result["output"] + # main() counts anything but "passed" as a failure. + assert result["status"] != "passed" + + +def test_the_bounds_do_not_fire_on_an_ordinary_run(): + """A batch that reports promptly is untouched by either bound.""" + result = run_scripted(PASSING_SUMMARY, 0) + assert result["status"] == "passed" + assert "Timeout" not in result["output"] + assert "No status" not in result["output"] diff --git a/policyengine_us/tests/code_health/test_batched_per_subdir_budget.py b/policyengine_us/tests/code_health/test_batched_per_subdir_budget.py index 28436103caa..d02bfb5179d 100644 --- a/policyengine_us/tests/code_health/test_batched_per_subdir_budget.py +++ b/policyengine_us/tests/code_health/test_batched_per_subdir_budget.py @@ -12,6 +12,7 @@ from pathlib import Path import yaml +import pytest BATCHER = Path(__file__).resolve().parents[1] / "test_batched.py" SPEC = importlib.util.spec_from_file_location("test_batched", BATCHER) @@ -74,3 +75,52 @@ def test_per_subdir_mode_splits_only_heavy_folders(tmp_path): [str(tmp_path / "light")], [str(tmp_path / "root.yaml")], ] + + +def test_heavy_subdir_keeps_yml_files(tmp_path): + heavy = tmp_path / "heavy" + _write_cases(heavy / "a.yaml", ["r1", "r2", "r3", "r4"]) + _write_cases(heavy / "b.yaml", ["r5", "r6"]) + _write_cases(heavy / "nested" / "must_run.yml", ["r7"]) + batches = batched.subdir_batches(heavy) + files = [file for batch in batches for file in batch] + assert files == [ + str(heavy / "a.yaml"), + str(heavy / "b.yaml"), + str(heavy / "nested" / "must_run.yml"), + ] + + +@pytest.mark.parametrize("mode", ["per-file", "per-subdir", "auto"]) +def test_explicit_batches_keep_both_yaml_extensions(tmp_path, mode): + _write_cases(tmp_path / "a.yaml", ["r1"]) + _write_cases(tmp_path / "b.yml", ["r2"]) + batches = batched.split_into_batches(tmp_path, 2, mode=mode) + assert sorted(file for batch in batches for file in batch) == [ + str(tmp_path / "a.yaml"), + str(tmp_path / "b.yml"), + ] + assert batched.count_yaml_files(tmp_path) == 2 + + +@pytest.mark.parametrize( + "relative", + ["policy/contrib", "contrib/states", "reform", "gov/states", "arbitrary"], +) +def test_auto_path_routes_keep_root_yml_files(tmp_path, relative): + root = tmp_path / relative + _write_cases(root / "a.yaml", ["r1"]) + _write_cases(root / "b.yml", ["r2"]) + batches = batched.split_into_batches(root, 2) + assert sorted(file for batch in batches for file in batch) == [ + str(root / "a.yaml"), + str(root / "b.yml"), + ] + + +def test_exclusion_preserves_yml_outside_excluded_directory(tmp_path): + _write_cases(tmp_path / "exclude_me" / "a.yml", ["r1"]) + _write_cases(tmp_path / "b.yml", ["r2"]) + assert batched.split_into_batches(tmp_path, 2, exclude=["exclude_me"]) == [ + [str(tmp_path / "b.yml")] + ] diff --git a/policyengine_us/tests/code_health/test_reform_combo_budget.py b/policyengine_us/tests/code_health/test_reform_combo_budget.py index 369725ea272..af502fe4c4f 100644 --- a/policyengine_us/tests/code_health/test_reform_combo_budget.py +++ b/policyengine_us/tests/code_health/test_reform_combo_budget.py @@ -17,8 +17,9 @@ """ import importlib.util +from pathlib import Path -from policyengine_us.model_api import REPO +REPO = Path(__file__).resolve().parents[2] _spec = importlib.util.spec_from_file_location( "test_batched", REPO / "tests" / "test_batched.py" @@ -31,7 +32,7 @@ def test_no_test_file_exceeds_reform_combo_budget(): over_budget = [] - for yaml_file in sorted(POLICY_TESTS_ROOT.rglob("*.yaml")): + for yaml_file in _test_batched.yaml_files(POLICY_TESTS_ROOT): combos = _test_batched.file_reform_combos(yaml_file) weight = _test_batched.combo_weight(combos) if weight > _test_batched.MAX_BATCH_COMBO_WEIGHT: diff --git a/policyengine_us/tests/core/test_spm_source_input_contract.py b/policyengine_us/tests/core/test_spm_source_input_contract.py index 853b5ea5901..76e1e340079 100644 --- a/policyengine_us/tests/core/test_spm_source_input_contract.py +++ b/policyengine_us/tests/core/test_spm_source_input_contract.py @@ -4,13 +4,13 @@ import pytest from policyengine_us import Simulation -from policyengine_us.spm import DATASET_FORMULA_OWNED_INPUTS, DATASET_SOURCE_INPUTS +from policyengine_us.spm import DATASET_SOURCE_INPUTS, REJECTED_DATASET_INPUTS from policyengine_us.system import system def test_source_contract_is_disjoint_and_matches_registered_role(): assert DATASET_SOURCE_INPUTS == {"is_spm_independent_minor_role"} - assert not DATASET_SOURCE_INPUTS & DATASET_FORMULA_OWNED_INPUTS + assert not DATASET_SOURCE_INPUTS & REJECTED_DATASET_INPUTS variable = system.variables["is_spm_independent_minor_role"] assert variable.entity.key == "person" assert variable.value_type is bool diff --git a/policyengine_us/tests/core/test_spm_system.py b/policyengine_us/tests/core/test_spm_system.py index 9177cbe660d..517f238547b 100644 --- a/policyengine_us/tests/core/test_spm_system.py +++ b/policyengine_us/tests/core/test_spm_system.py @@ -1,7 +1,10 @@ """Construction and dataset contracts that require the Python simulation API.""" +import ast import hashlib +import inspect import json +import textwrap import numpy as np import pandas as pd @@ -15,8 +18,21 @@ from policyengine_us import Microsimulation, Simulation from policyengine_us.data.dataset_schema import USMultiYearDataset, USSingleYearDataset from policyengine_us.entities import Person -from policyengine_us.spm import create_spm_provider, is_county_fips -from policyengine_us.system import DEFAULT_DATASET, _resolve_dataset_path, system +from policyengine_us.spm import ( + DERIVED_POVERTY_OUTPUTS, + REJECTED_DATASET_INPUTS, + create_spm_provider, + is_county_fips, +) +from policyengine_us import system as system_module +from policyengine_us.system import ( + DEFAULT_DATASET, + DEFAULT_DATASET_SHA256, + _file_sha256, + _resolve_dataset_path, + _verify_default_dataset, + system, +) def single_person_situation(): @@ -599,3 +615,219 @@ def test_microsimulation_rejects_stored_measurements_without_mutating_dataset( pd.testing.assert_frame_equal(original, current) if dataset_format == "hdfstore": assert path.read_bytes() == content + + +@pytest.mark.parametrize("name", sorted(DERIVED_POVERTY_OUTPUTS)) +def test_dataset_storing_a_derived_poverty_output_is_refused(name): + """A poverty alias the calculator does not own must still be refused. + + ``FORMULA_OWNED_INPUTS`` stops at the calculator's own outputs, so a + dataset supplying ``in_poverty=[False, False]`` was accepted unchanged + while every unit in the population sat below its threshold: the poverty + calculation documented in ``docs/usage/microsimulation.md`` reported 0% + and ``spm_unit_is_in_spm_poverty`` reported 100% for the same population. + """ + variable = system.variables[name] + source = small_dataset() + table = getattr(source, variable.entity.key) + table[name] = [False, False] if variable.value_type == bool else [0.0, 0.0] + before = [frame.copy(deep=True) for frame in source.tables] + with pytest.raises(ValueError) as error: + Microsimulation(dataset=source) + message = str(error.value) + assert f"formula-owned SPM output {name}" in message + assert "Use primitive inputs" in message + for original, current in zip(before, source.tables): + pd.testing.assert_frame_equal(original, current) + + +def test_accepted_population_publishes_one_poverty_rate(): + """The two published indicators agree once the aliases cannot be stored.""" + simulation = Microsimulation(dataset=small_dataset()) + np.testing.assert_array_equal( + simulation.calculate("in_poverty", 2024), + simulation.calculate("spm_unit_is_in_spm_poverty", 2024), + ) + np.testing.assert_array_equal( + simulation.calculate("in_deep_poverty", 2024), + simulation.calculate("spm_unit_is_in_deep_spm_poverty", 2024), + ) + + +def test_rejection_set_extends_rather_than_replaces_the_calculator_contract(): + for name in FORMULA_OWNED_INPUTS: + assert name in REJECTED_DATASET_INPUTS + for name in DERIVED_POVERTY_OUTPUTS: + # A rejected name must be computed, never a legitimate stored input. + assert system.variables[name].formula is not None + + +ENTITY_KEYS = frozenset( + {"person", "marital_unit", "tax_unit", "family", "spm_unit", "household"} +) +# policyengine-core helpers that name the variables they read as string +# literals, exactly as an entity call does, so the constant scan below sees +# their reads too (policyengine_core/commons/formulas.py: for_each_variable +# and the aggregators built on it). +NAME_LISTING_HELPERS = frozenset( + {"add", "and_", "or_", "max_", "min_", "for_each_variable", "sum_of_variables"} +) + + +def _static_variable_reads(variable): + """Variable names a registered variable reads, or None when not static. + + Over-collects rather than under-collects: every string constant in the + class body that names a registered variable counts, so a helper this scan + does not model cannot make a variable look like a pure function of the + poverty chain when it is not. + + A formula is only trusted when its reads are visibly named — through an + entity call or a name-listing helper. A variable with no formula at all is + fully described by its ``adds``/``subtracts`` list, which is how most of + the poverty chain is written: ``poverty_line`` is ``adds = + ["spm_unit_spm_threshold"]`` and nothing else. Requiring an entity call of + those too discarded 884 of the system's variables, this measurement's own + aliases among them, leaving the closure below blind to the very idiom the + next alias would most likely use. + """ + for attribute in ("adds", "subtracts"): + if isinstance(getattr(variable, attribute, None), str): + # A parameter path: its contents are not statically known here. + return None + reads = { + item + for attribute in ("adds", "subtracts") + for item in (getattr(variable, attribute, None) or []) + } + try: + source = textwrap.dedent(inspect.getsource(type(variable))) + except (OSError, TypeError): + # Calculator-built variables have no country source file. + return None + tree = ast.parse(source) + named_reads = False + has_formula = False + for node in ast.walk(tree): + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + if node.name.startswith("formula"): + has_formula = True + if isinstance(node, ast.Call): + function = node.func + name = getattr(function, "id", None) or getattr(function, "attr", None) + if name in ENTITY_KEYS or name in NAME_LISTING_HELPERS: + named_reads = True + if ( + isinstance(node, ast.Constant) + and isinstance(node.value, str) + and node.value in system.variables + ): + reads.add(node.value) + if has_formula and not named_reads: + # A formula reaching for its inputs some other way; what it reads is + # not knowable from the source. + return None + return reads - {variable.name} + + +def test_every_pure_function_of_the_poverty_chain_is_rejected(): + """The rejection set is closed: no further alias can be stored. + + A variable whose every read is already rejected and which reads at least + one threshold or poverty output is another name for the same measurement. + ``in_poverty`` was exactly that, and this closure is what stops the next + one from shipping unpoliced. + """ + chain = { + name + for name in REJECTED_DATASET_INPUTS + if "poverty" in name or "threshold" in name + } + unrejected_aliases = [] + checked = [] + for name, variable in sorted(system.variables.items()): + reads = _static_variable_reads(variable) + if not reads or not reads <= REJECTED_DATASET_INPUTS or not reads & chain: + continue + checked.append(name) + if name not in REJECTED_DATASET_INPUTS: + unrejected_aliases.append(f"{name} reads {sorted(reads)}") + assert not unrejected_aliases, ( + "These variables are pure functions of rejected SPM poverty outputs " + "but a dataset may still store them; add them to " + "DERIVED_POVERTY_OUTPUTS in policyengine_us/spm.py:\n" + + "\n".join(unrejected_aliases) + ) + # Guard the guard: a scan that silently matched nothing proves nothing. + assert set(DERIVED_POVERTY_OUTPUTS) <= set(checked) + + +@pytest.fixture +def decoy_default_dataset(monkeypatch, tmp_path): + """Resolve the default URI to a valid but uncertified build. + + The decoy loads and calculates, so anything that rejects it rejects it on + content rather than on shape. + """ + path = tmp_path / "decoy.h5" + small_dataset().save(path) + monkeypatch.setattr( + system_module, + "_resolve_dataset_path", + lambda dataset_str: str(path), + ) + return path + + +def test_default_dataset_rejects_content_that_is_not_the_certified_build( + decoy_default_dataset, +): + """A moved build id must fail loudly instead of changing every result. + + The default pins a Hugging Face tag, which is a mutable pointer: without + this check, re-tagging the repository silently substitutes another + schema-valid population. + """ + with pytest.raises(ValueError) as error: + Microsimulation() + message = str(error.value) + assert DEFAULT_DATASET in message + assert str(decoy_default_dataset) in message + assert DEFAULT_DATASET_SHA256 in message + assert _file_sha256(decoy_default_dataset) in message + + +def test_default_dataset_accepts_the_certified_digest( + decoy_default_dataset, monkeypatch +): + """The check passes exactly the certified bytes and obstructs nothing else.""" + monkeypatch.setattr( + system_module, + "DEFAULT_DATASET_SHA256", + _file_sha256(decoy_default_dataset), + ) + simulation = Microsimulation() + assert simulation.calculate("spm_measurement_adults", 2024).tolist() == [1, 1] + + +def test_explicit_dataset_argument_is_not_content_checked(decoy_default_dataset): + """A caller naming its own artifact owns that artifact's provenance.""" + simulation = Microsimulation(dataset=str(decoy_default_dataset)) + assert simulation.calculate("spm_measurement_adults", 2024).tolist() == [1, 1] + + +def test_default_dataset_is_digested_once_per_resolved_file(monkeypatch, tmp_path): + """The certified build is ~830 MB; construction must not re-digest it.""" + path = tmp_path / "counted.h5" + path.write_bytes(b"populace") + digests = [] + + def counting_sha256(file_path): + digests.append(str(file_path)) + return "0" * 64 + + monkeypatch.setattr(system_module, "_file_sha256", counting_sha256) + monkeypatch.setattr(system_module, "DEFAULT_DATASET_SHA256", "0" * 64) + _verify_default_dataset(path) + _verify_default_dataset(path) + assert digests == [str(path)] diff --git a/policyengine_us/tests/test_batched.py b/policyengine_us/tests/test_batched.py index bf84fbee3f3..54b73ff1f8d 100644 --- a/policyengine_us/tests/test_batched.py +++ b/policyengine_us/tests/test_batched.py @@ -21,11 +21,33 @@ import yaml +# policyengine-core collects both YAML suffixes +# (policyengine_core/tools/test_runner.py accepts ".yaml" and ".yml"), so any +# batch built by listing files has to list both. Enumerating only "*.yaml" +# would hand the subprocess a path list that silently omits a ".yml" test — +# invisibly, because the batch it belonged to still passes. +YAML_SUFFIXES = (".yaml", ".yml") + + +def yaml_files(directory: Path) -> List[Path]: + """Every YAML test file under a directory, recursively, in a stable order.""" + return sorted( + path for suffix in YAML_SUFFIXES for path in directory.rglob(f"*{suffix}") + ) + + +def root_yaml_files(directory: Path) -> List[Path]: + """YAML test files directly in a directory, not in its subdirectories.""" + return sorted( + path for suffix in YAML_SUFFIXES for path in directory.glob(f"*{suffix}") + ) + + def count_yaml_files(directory: Path) -> int: """Count YAML files in a directory recursively.""" if not directory.exists(): return 0 - return len(list(directory.rglob("*.yaml"))) + return len(yaml_files(directory)) def batch_cost(batch_paths: List[str]) -> int: @@ -178,7 +200,7 @@ def subdir_batches(subdir: Path) -> List[List[str]]: minutes before CI killed it. Pack such a folder's files by combo weight instead. """ - files = sorted(subdir.rglob("*.yaml")) + files = yaml_files(subdir) combos: set = set() for file in files: combos |= set(file_reform_combos(file)) @@ -214,7 +236,7 @@ def split_into_batches( # Explicit modes — bypass the per-path auto heuristics so new files # added to the target auto-route without a Makefile edit. if mode == "per-file": - return [[str(f)] for f in sorted(base_path.rglob("*.yaml"))] + return [[str(f)] for f in yaml_files(base_path)] if mode == "per-subdir": subdirs = sorted( @@ -222,7 +244,7 @@ def split_into_batches( for item in base_path.iterdir() if item.is_dir() and item.name not in exclude ) - root_files = sorted(base_path.glob("*.yaml")) + root_files = root_yaml_files(base_path) batches = [batch for s in subdirs for batch in subdir_batches(s)] if root_files: batches.append([str(f) for f in root_files]) @@ -264,7 +286,7 @@ def split_into_batches( for item in base_path.iterdir() if item.is_dir() and item.name not in exclude ) - root_files = sorted(base_path.glob("*.yaml")) + root_files = root_yaml_files(base_path) # One batch per file for per-file folders, then one batch per # heavy subdir (if present). @@ -272,7 +294,7 @@ def split_into_batches( [str(file)] for subdir in subdirs if subdir.name in PER_FILE - for file in sorted(subdir.rglob("*.yaml")) + for file in yaml_files(subdir) ] batches += [[str(subdir)] for subdir in subdirs if subdir.name in HEAVY] @@ -303,10 +325,10 @@ def split_into_batches( subdirs = sorted([item for item in base_path.iterdir() if item.is_dir()]) batches = [] for subdir in subdirs: - batches.extend(pack_files_by_combo_weight(sorted(subdir.rglob("*.yaml")))) + batches.extend(pack_files_by_combo_weight(yaml_files(subdir))) # Also include any root-level YAML files as trailing batches - root_files = sorted(base_path.glob("*.yaml")) + root_files = root_yaml_files(base_path) if root_files: batches.extend(pack_files_by_combo_weight(root_files)) @@ -319,7 +341,7 @@ def split_into_batches( # shutdown signal". A fresh subprocess per file frees each peak between # files. if "reform" in str(base_path): - return [[str(f)] for f in sorted(base_path.rglob("*.yaml"))] + return [[str(f)] for f in yaml_files(base_path)] # Special handling for states directory - support excluding specific states # and splitting into multiple sequential batches for memory management @@ -334,7 +356,7 @@ def split_into_batches( # Root-level YAML files (e.g. cross-state filing-status test) are # state-agnostic and would be invisible to subdir-based batching # otherwise — collect them into a dedicated trailing batch. - root_files = sorted(base_path.glob("*.yaml")) + root_files = root_yaml_files(base_path) if not subdirs and not root_files: return [] @@ -395,7 +417,7 @@ def collect_gov_paths(folder_names): if gov_item.is_dir(): if gov_item.name not in heavy_set and gov_item.name != "states": remaining.append(str(gov_item)) - elif gov_item.suffix == ".yaml": + elif gov_item.suffix in YAML_SUFFIXES: remaining.append(str(gov_item)) # Add non-gov directories and root YAML files @@ -404,7 +426,7 @@ def collect_gov_paths(folder_names): if item.name in ["household", "contrib", "gov"]: continue remaining.append(str(item)) - elif item.suffix == ".yaml": + elif item.suffix in YAML_SUFFIXES: remaining.append(str(item)) # Build batches (only include non-empty ones) @@ -435,10 +457,11 @@ def collect_gov_paths(folder_names): paths = sorted( str(item) for item in base_path.iterdir() - if (item.is_dir() or item.suffix == ".yaml") and item.name not in exclude + if (item.is_dir() or item.suffix in YAML_SUFFIXES) + and item.name not in exclude ) else: - paths = sorted(str(f) for f in base_path.rglob("*.yaml")) + paths = [str(f) for f in yaml_files(base_path)] if not paths: return [] @@ -459,7 +482,113 @@ def collect_gov_paths(folder_names): return batches -def run_batch(test_paths: List[str], batch_name: str, stream: bool = True) -> Dict: +# pytest exits 5 when a path collects no tests (see pytest.ExitCode). +PYTEST_NO_TESTS_COLLECTED = 5 +# How long a child gets to die after being asked to, before it is killed. +TERMINATE_GRACE_SECONDS = 5 +# Overall budget for one batch, enforced inside the monitoring loop below. +BATCH_TIMEOUT_SECONDS = 1800 +# How long the runner may take to hand back a status after printing its +# summary. That gap is pytest's own session-finish hooks and unconfigure, not +# the slow interpreter teardown described below, which happens after the +# marker: measured, the marker arrives in the same read as the summary line. +# This is deliberately generous, and exists only so a runner that stops +# responding right at the end is reported instead of hanging the job. +MARKER_GRACE_SECONDS = 300 + +# The runner's process exit status is not reachable in bounded time. Once +# policyengine-core's pytest.main() returns, interpreter teardown frees the +# tax-benefit systems the run cached, which is slow: measured on this suite, a +# single small state took 18.2s to exit after printing its summary, and the +# large state batches took over 30s. That is why the runner is terminated as +# soon as it reports rather than waited out - and why its status used to be +# discarded along with it, leaving the printed summary as the only evidence. +# +# So ask for the status at the only moment it is both known and free: run the +# command through a shim that writes pytest's own return value to stdout the +# instant pytest.main() hands it back, before any teardown. Measured at the +# same moment as the summary line, so this costs no wall clock, and the child +# is still terminated exactly as promptly as before. +BATCH_EXIT_MARKER = "__POLICYENGINE_BATCH_EXIT__" + +RUNNER_SHIM = f""" +import runpy +import sys + +code = 0 +try: + runpy.run_module( + "policyengine_core.scripts.policyengine_command", run_name="__main__" + ) +except SystemExit as requested: + code = requested.code +sys.stdout.write("\\n{BATCH_EXIT_MARKER} %s\\n" % (0 if code is None else code)) +sys.stdout.flush() +sys.exit(code) +""" + +BATCH_EXIT_PATTERN = re.compile(rf"{BATCH_EXIT_MARKER} (-?\d+)") + + +def build_test_command(test_paths: List[str], python_exe: str) -> List[str]: + """The policyengine-core command one batch runs in its own subprocess. + + `python -c` stops consuming options after the program text, so everything + after RUNNER_SHIM reaches policyengine_command's own argument parser + unchanged. + """ + return ( + [python_exe, "-c", RUNNER_SHIM, "test"] + test_paths + ["-c", "policyengine_us"] + ) + + +def describe_exit_status(returncode: Optional[int]) -> str: + """Human-readable form of a batch subprocess's exit status.""" + if returncode is None: + return "no exit status (killed after it stopped responding)" + if returncode == 0: + return "exit 0" + if returncode < 0: + return f"killed by signal {-returncode}" + if returncode == PYTEST_NO_TESTS_COLLECTED: + return f"exit {returncode} (pytest collected no tests)" + return f"exit {returncode}" + + +def batch_status(returncode: Optional[int]) -> str: + """Batch outcome, decided by the runner's exit status and nothing else. + + policyengine-core's `test` command is `sys.exit(pytest.main(...))` + (scripts/run_test.py), so the status is pytest's own ExitCode and only 0 is + a pass. Reading the printed summary instead loses every outcome pytest does + not express as a failure count: + + * "1 passed, 1 error" prints no failed count at all, so the old parse read + an errored batch as success while the child exited 1. + * 2 (interrupted) prints a count for whatever ran before the interrupt and + no failure count, which the old parse also read as success: under + policyengine-core's argv, a session whose second test raises + KeyboardInterrupt exits 2 beneath "1 passed in 0.10s" (measured). + * 3 and 4 (internal error, usage error) print no summary count at all, so + the old parse fell through to a status it had already discarded. + * 5 (no tests collected) is a failure *for this runner*. It differs from + run_selective_tests.py, which skips 5 deliberately: that runner selects + paths from a changed-file list, where a changed test helper legitimately + collects nothing. This runner enumerates the YAML suite itself, so a + batch that collects nothing means the suite or this enumeration is + broken — a result to report, not to pass over. + * None means the child never reported a status, having been killed after + it stopped responding. An unreported batch is not a passing batch. + """ + return "passed" if returncode == 0 else "failed" + + +def run_batch( + test_paths: List[str], + batch_name: str, + stream: bool = True, + command: Optional[List[str]] = None, +) -> Dict: """Run a batch of tests in an isolated subprocess. With stream=True output is echoed to stdout in real time (sequential @@ -485,17 +614,10 @@ def emit(text: str, end: str = "\n") -> None: peak_rss_mb = None last_rss_sample = 0.0 - # Build command - direct policyengine-core with timeout protection - cmd = ( - [ - python_exe, - "-m", - "policyengine_core.scripts.policyengine_command", - "test", - ] - + test_paths - + ["-c", "policyengine_us"] - ) + # Build command - direct policyengine-core with timeout protection. + # `command` is an injection point for the code-health tests, which drive a + # real subprocess with scripted output and exit statuses. + cmd = command or build_test_command(test_paths, python_exe) emit(f" Running {batch_name}...") emit(f" Paths: {len(test_paths)} items") @@ -520,10 +642,41 @@ def sample_rss() -> None: try: test_completed = False - test_passed = False + summary_reported_pass = False + abandoned: Optional[str] = None + reported_returncode: Optional[int] = None output_lines = [] output_text = "" + summary_at: Optional[float] = None + + def stop_process() -> None: + """End a child, whether or not it told us how it finished.""" + process.terminate() + try: + process.wait(timeout=TERMINATE_GRACE_SECONDS) + except subprocess.TimeoutExpired: + emit(" Force killing process...") + process.kill() + process.wait() + + def overdue() -> Optional[str]: + """Why this batch should stop being waited on, if it should. + + The monitoring loop is otherwise unbounded, in this revision and + the one before it: it ends only when the child exits or reports, + and the 30-minute budget below it was only ever consulted after + the child had already gone. Enforce that budget here, where it can + fire, and bound the window this file opened by reading the status + from the runner instead of terminating a second after its summary. + """ + now = time.time() + if now - start_time >= BATCH_TIMEOUT_SECONDS: + return "timeout" + if summary_at is not None and now - summary_at >= MARKER_GRACE_SECONDS: + return "unreported" + return None + # Monitor output line by line while True: ready, _, _ = select.select([process.stdout], [], [], 0.1) @@ -535,6 +688,20 @@ def sample_rss() -> None: if poll_result is not None: # Process terminated break + give_up = overdue() + if give_up is not None: + abandoned = give_up + emit( + "\n ⏱️ " + + ( + "Timeout" + if give_up == "timeout" + else f"No status {MARKER_GRACE_SECONDS}s after the summary" + ) + + " - terminating process..." + ) + stop_process() + break now = time.time() # In buffered (concurrent) mode the shared heartbeat in # run_batches_concurrently reports liveness instead. @@ -572,41 +739,38 @@ def sample_rss() -> None: # Detect pytest completion # Look for patterns like "====== 5638 passed in 491.24s ======" # or "====== 2 failed, 5636 passed in 500s ======" - if re.search( + if not test_completed and re.search( r"=+.*\d+\s+(passed|failed).*in\s+[\d.]+s.*=+", output_text, ): test_completed = True - # Check if tests passed by parsing actual failure count + summary_at = time.time() + # The summary line only decides how loudly a disagreement is + # reported below; batch_status() reads the runner's status. failed_match = re.search(r"(\d+) failed", output_text) - if failed_match: - failed_count = int(failed_match.group(1)) - test_passed = failed_count == 0 - else: - # No "X failed" in line means all passed - test_passed = True - - emit(f"\n Tests completed, terminating process...") + summary_reported_pass = not failed_match or ( + int(failed_match.group(1)) == 0 + ) - # Give 1 second grace period for cleanup - time.sleep(1) + # The runner has reported pytest's return value and is now only + # tearing down, which takes tens of seconds. Stop it here, as + # before, but keep the status it just gave us. + marker_match = BATCH_EXIT_PATTERN.search(output_text) + if marker_match: + reported_returncode = int(marker_match.group(1)) + emit( + "\n Runner reported " + f"{describe_exit_status(reported_returncode)}; " + "terminating process..." + ) # Final sample while the pid still exists — captures the # end-of-run high-water mark. sample_rss() - - # Terminate the process - process.terminate() - try: - process.wait(timeout=5) - except subprocess.TimeoutExpired: - # Force kill if it won't terminate - emit(f" Force killing process...") - process.kill() - process.wait() + stop_process() break - # If we didn't detect completion, wait for process with timeout - if not test_completed: + # If the runner never reported, wait for the process with a timeout + if reported_returncode is None and process.poll() is None: try: # Wait up to 30 minutes total elapsed = time.time() - start_time @@ -626,36 +790,53 @@ def sample_rss() -> None: return { "elapsed": elapsed, "status": "timeout", + "returncode": process.poll(), "peak_rss_mb": peak_rss_mb, "output": "".join(buf), } elapsed = time.time() - start_time - if test_completed: + # The runner's own report wins: the process status after a terminate is + # the signal we sent it, not how the tests went. + returncode = ( + reported_returncode if reported_returncode is not None else process.poll() + ) + + if abandoned == "timeout": emit( - f"\n Batch completed in {elapsed:.1f}s " + f"\n Batch abandoned after {elapsed:.1f}s " f"(peak RSS: {_format_rss(peak_rss_mb)})" ) return { "elapsed": elapsed, - "status": "passed" if test_passed else "failed", + "status": "timeout", + "returncode": returncode, "peak_rss_mb": peak_rss_mb, "output": "".join(buf), } - else: - # Process ended without detecting test completion - returncode = process.poll() + + status = batch_status(returncode) + if test_completed and summary_reported_pass and status != "passed": + # The case that used to be reported as a pass: pytest printed no + # failure count (e.g. "1 passed, 1 error") yet finished nonzero. emit( - f"\n Batch completed in {elapsed:.1f}s " - f"(peak RSS: {_format_rss(peak_rss_mb)})" + f" ❌ {batch_name} printed a summary with no failures but " + f"finished with {describe_exit_status(returncode)}; " + "the runner's status decides." ) - return { - "elapsed": elapsed, - "status": "passed" if returncode == 0 else "failed", - "peak_rss_mb": peak_rss_mb, - "output": "".join(buf), - } + emit( + f"\n Batch completed in {elapsed:.1f}s " + f"(peak RSS: {_format_rss(peak_rss_mb)}, " + f"{describe_exit_status(returncode)})" + ) + return { + "elapsed": elapsed, + "status": status, + "returncode": returncode, + "peak_rss_mb": peak_rss_mb, + "output": "".join(buf), + } except Exception as e: elapsed = time.time() - start_time @@ -675,6 +856,7 @@ def sample_rss() -> None: return { "elapsed": elapsed, "status": "error", + "returncode": process.poll(), "peak_rss_mb": peak_rss_mb, "output": "".join(buf), } @@ -743,6 +925,7 @@ def run_one(idx: int, batch_paths: List[str]) -> Dict: result = { "elapsed": 0.0, "status": "error", + "returncode": None, "peak_rss_mb": None, "output": f"Worker crashed: {str(e)[:200]}\n", } @@ -754,6 +937,7 @@ def run_one(idx: int, batch_paths: List[str]) -> Dict: f"\n[Batch {idx}/{total}] finished in " f"{result['elapsed']:.1f}s | peak RSS: " f"{_format_rss(result.get('peak_rss_mb'))} | " + f"{describe_exit_status(result.get('returncode'))} | " f"{result['status']}", flush=True, ) @@ -897,7 +1081,7 @@ def main(): print(" Cleaning up memory...") gc.collect() - all_failed = any(result["status"] != "passed" for _, result in results) + any_failed = any(result["status"] != "passed" for _, result in results) total_elapsed = sum(result["elapsed"] for _, result in results) # Final summary @@ -909,6 +1093,7 @@ def main(): print( f" {name}: {result['elapsed']:.1f}s | " f"peak RSS: {_format_rss(result.get('peak_rss_mb'))} | " + f"{describe_exit_status(result.get('returncode'))} | " f"{result['status']}" ) print(f"Workers: {args.workers}") @@ -917,7 +1102,7 @@ def main(): print(f"Wall time: {time.time() - run_start:.1f}s") # Exit code - sys.exit(1 if all_failed else 0) + sys.exit(1 if any_failed else 0) if __name__ == "__main__": diff --git a/policyengine_us/tests/unit/test_spm_integration_contract.py b/policyengine_us/tests/unit/test_spm_integration_contract.py index e03e00f18a1..638b3535fac 100644 --- a/policyengine_us/tests/unit/test_spm_integration_contract.py +++ b/policyengine_us/tests/unit/test_spm_integration_contract.py @@ -269,24 +269,38 @@ def test_resource_consumers_without_housing_assistance_never_touch_geography( assert simulation.spm_provenance()["years"] == {} -@pytest.mark.parametrize("variable", ["household_net_income", "marginal_tax_rate"]) -def test_resource_consumers_with_housing_assistance_require_geography(variable): - """Once there is assistance to cap, the county requirement applies. - - Earnings stay low so the tenant payment sits below the housing portion - and the cap binds on the assistance rather than on zero. - """ - situation = household(earnings=6_000) - situation["spm_units"]["spm_unit"]["housing_assistance"] = {YEAR: 5_000} +@pytest.mark.parametrize( + "variable", + [ + "household_benefits", + "household_net_income", + "marginal_tax_rate", + "cbo_household_means_tested_transfers", + ], +) +def test_general_income_with_housing_assistance_is_independent_of_spm(variable): + """Benefit aggregates use the actual HUD payment, not its SPM valuation.""" + situation = household(earnings=24_000) + results = [] + for config in [None, {"geography_kind": "national"}]: + simulation = Simulation(situation=situation, spm=config) + assert simulation.calculate("housing_assistance", YEAR)[0] > 0 + results.append(simulation.calculate(variable, YEAR)) + assert np.all(np.isfinite(results[-1])) + assert simulation.spm_provenance()["years"] == {} + np.testing.assert_array_equal(*results) + + +def test_spm_resources_with_housing_assistance_still_require_geography(): + situation = household(earnings=24_000) with pytest.raises(SPMInputError) as error: - Simulation(situation=situation).calculate(variable, YEAR) + Simulation(situation=situation).calculate("spm_unit_net_income", YEAR) assert error.value.code == "SPM_GEOGRAPHY_REQUIRED" - assert error.value.to_dict()["code"] == "SPM_GEOGRAPHY_REQUIRED" national = Simulation(situation=situation, spm={"geography_kind": "national"}) - result = national.calculate(variable, YEAR) - assert np.all(np.isfinite(result)) + assistance = national.calculate("housing_assistance", YEAR)[0] capped = national.calculate("spm_unit_capped_housing_subsidy", YEAR)[0] - assert 0 < capped <= 5_000 + assert 0 < capped < assistance + assert np.all(np.isfinite(national.calculate("spm_unit_net_income", YEAR))) assert str(YEAR) in national.spm_provenance()["years"] @@ -419,3 +433,171 @@ def test_generic_benefit_consumers_do_not_use_spm_measurement_composition( simulation.calculate("spm_measurement_children", YEAR), [1, 0] ) assert simulation.spm_provenance()["years"] == {} + + +# --- CPUC CARE/FERA countable income ----------------------------------------- +# +# CARE and FERA are utility discounts, not poverty measurement. +# gov.states.ca.cpuc.income_sources used to count +# spm_unit_capped_housing_subsidy, which made the discount depend on the SPM +# geography the request selected. CPUC D.14-08-030 section 6.2 and Ordering +# Paragraph 40(3) exclude housing subsidies from the income definition +# altogether, effective August 14, 2014, so from calendar 2015 the list carries +# no housing entry at any valuation. PG&E's application form still lists +# "housing and military subsidies", but PG&E told the Commission on June 5, +# 2026 that it does not use them in its income analysis and will correct the +# form. + + +def assisted_la_renter(earnings, *, county=None): + situation = household(earnings=earnings, county=county) + situation["spm_units"]["spm_unit"]["pre_subsidy_electricity_expense"] = { + YEAR: 1_800 + } + # Pin the categorical route off so income is what decides, and the + # equal-eligibility assertions below cannot pass for an unrelated reason. + situation["households"]["household"]["ca_care_categorically_eligible"] = { + YEAR: False + } + return situation + + +CARE_OUTPUTS = ("ca_cpuc_countable_income", "ca_care_income_eligible", "ca_care") + + +@pytest.mark.parametrize( + "earnings,eligible", + [ + # $0 earnings. The assistance is excluded, so only the household's + # other income counts, well inside the 2024 one-person CARE limit of + # 2 x $20,440. + (0, True), + # The reported case: a Los Angeles renter earning $40,700 with $23,790 + # of assistance. With the subsidy excluded, $40,700 of countable + # income sits just inside the $40,880 limit under every geography. + # Counting the capped subsidy flipped this household out of CARE at + # $310.88 under county selection and left it in under national. + (40_700, True), + ], +) +def test_care_eligibility_is_the_same_under_every_spm_geography(earnings, eligible): + """An assisted household's utility discount must not move with SPM geography. + + The capped subsidy is a poverty-measurement construct: it nets the + assistance against the housing portion of the SPM threshold, which is + geographically adjusted. Counting it made one Los Angeles renter + CARE-eligible nationally and ineligible under county selection. CPUC + D.14-08-030 excludes housing subsidies from CARE/FERA income outright, so + neither valuation reaches the discount now. + """ + national = Simulation( + situation=assisted_la_renter(earnings), + spm={"geography_kind": "national"}, + ) + county = Simulation( + situation=assisted_la_renter(earnings, county="06037"), + spm={"geography_kind": "county"}, + ) + unconfigured = Simulation(situation=assisted_la_renter(earnings)) + + # Guard the guard: the capped subsidy this parameter used to count still + # differs between the two selections, so equal CARE outputs below are the + # income definition's doing and not a quiet loss of geographic adjustment. + assert ( + national.calculate("spm_unit_capped_housing_subsidy", YEAR)[0] + != (county.calculate("spm_unit_capped_housing_subsidy", YEAR)[0]) + ) + + for variable in CARE_OUTPUTS: + values = [sim.calculate(variable, YEAR)[0] for sim in (national, county)] + assert values[0] == values[1], variable + assert bool(national.calculate("ca_care_eligible", YEAR)[0]) is eligible + + # And the same answer with no SPM configuration and no county at all: this + # is what raised SPM_GEOGRAPHY_REQUIRED for partners requesting CARE. + for variable in (*CARE_OUTPUTS, "ca_care_eligible"): + assert ( + unconfigured.calculate(variable, YEAR)[0] + == national.calculate(variable, YEAR)[0] + ), variable + # Nothing was looked up, so no measurement was received. + assert unconfigured.spm_provenance()["years"] == {} + + +def test_cpuc_countable_income_excludes_the_housing_assistance_entirely(): + """Non-vacuity: the subsidy is live, and none of it reaches countable income. + + Equal answers across geographies would also hold if the household simply + had no assistance, so pin that the assistance exists and is excluded + rather than merely revalued. + """ + simulation = Simulation(situation=assisted_la_renter(40_700)) + assistance = simulation.calculate("housing_assistance", YEAR)[0] + assert assistance > 0 + assert simulation.calculate("ca_cpuc_countable_income", YEAR)[0] == pytest.approx( + 40_700 + ) + + +def income_eligible_la_renter(*, receives, county="06037"): + """A renter HUD would income-qualify, so the imputation has something to bite. + + Earnings of $35,000 against $24,000 of rent leave this household inside the + HUD low-income limit for Los Angeles County, so it is eligible for housing + assistance whatever it reports, and `takes_up_housing_assistance_if_eligible` + defaults to true. + """ + situation = household(earnings=35_000, county=county, rent=24_000) + situation["spm_units"]["spm_unit"]["receives_housing_assistance"] = {YEAR: receives} + situation["spm_units"]["spm_unit"]["pre_subsidy_electricity_expense"] = { + YEAR: 1_800 + } + # Pin the categorical route off so income is what decides. + situation["households"]["household"]["ca_care_categorically_eligible"] = { + YEAR: False + } + return situation + + +@pytest.mark.parametrize("spm", [None, {"geography_kind": "national"}]) +@pytest.mark.parametrize("county", ["06037", None]) +def test_cpuc_countable_income_for_a_non_recipient_needs_no_geography(spm, county): + """The unreported case computes under every selection, county input or not. + + Without a county the HUD income limits do not resolve, so the household is + not even modelled as voucher-eligible; with one it is. Either way nothing + consults SPM geography, so no configuration is required and no measurement + is received. + """ + simulation = Simulation( + situation=income_eligible_la_renter(receives=False, county=county), + spm=spm, + ) + assert not simulation.calculate("receives_housing_assistance", YEAR)[0] + assert simulation.calculate("ca_cpuc_countable_income", YEAR)[0] == pytest.approx( + 35_000 + ) + assert bool(simulation.calculate("ca_care_eligible", YEAR)[0]) + assert simulation.spm_provenance()["years"] == {} + + +def test_cpuc_excludes_the_housing_assistance_a_household_reports_receiving(): + """Reported receipt is excluded too, so the discount survives the voucher. + + The same household as above, now reporting the voucher: D.14-08-030 + excludes housing subsidies from the income definition whether or not the + household reports them, so $35,000 of earnings alone decides and the + household keeps CARE. Counting the subsidy would have taken it out. + """ + simulation = Simulation( + situation=income_eligible_la_renter(receives=True), + spm={"geography_kind": "national"}, + ) + assistance = simulation.calculate("housing_assistance", YEAR)[0] + assert assistance > 0 + assert simulation.calculate("ca_cpuc_countable_income", YEAR)[0] == pytest.approx( + 35_000 + ) + assert bool(simulation.calculate("ca_care_income_eligible", YEAR)[0]) + assert bool(simulation.calculate("ca_care_eligible", YEAR)[0]) + assert simulation.calculate("ca_care", YEAR)[0] == pytest.approx(0.325 * 1_800) diff --git a/policyengine_us/variables/gov/hud/housing_assistance.py b/policyengine_us/variables/gov/hud/housing_assistance.py index 4060700f6cb..0c7f5a737d0 100644 --- a/policyengine_us/variables/gov/hud/housing_assistance.py +++ b/policyengine_us/variables/gov/hud/housing_assistance.py @@ -6,7 +6,14 @@ class housing_assistance(Variable): entity = SPMUnit label = "Housing assistance" unit = USD - documentation = "Housing assistance" + documentation = """ + Housing assistance the model computes for this unit: HUD's housing + assistance payment where the unit is eligible and takes up. + + Eligibility is `receives_housing_assistance | (is_renter & + is_income_eligible)` and take-up defaults to true, so this is a modelled + amount rather than a report of receipt. + """ definition_period = YEAR defined_for = "is_eligible_for_housing_assistance" From 28e7ac9422476076093b421f2269a9ac9d40204b Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 20:22:58 -0400 Subject: [PATCH 07/21] Cite the CPUC income sources and record the FERA basis Folds #9464's citation research into #9448's dated exclusion list. The ~25 non-housing entries regain a source: PG&E Form 01-9077 (Rev. 6.26) at PG&E's own URL, plus Revised Cal. P.U.C. Sheet No. 61313-E (Electric Sample Form 79-1055, Advice 7905-E, effective June 1, 2026), which carries the same income footnote as a CPUC filing. Both are annotated as being corrected: PG&E's June 5, 2026 reply brief in A.26-01-003 states it does not use housing and military subsidies in its income analysis and will revise the application. The dead bha.berkeleyca.gov href is gone; all nine references return HTTP 200. Records review item R8: FERA coverage of the exclusion rests on the 2026 brief, not on D.14-08-030. Verified first-hand against the decision PDF - it contains zero case-sensitive occurrences of "FERA", "Family Electric" or "739.12", while the brief's Section B answers the income definition under Public Utilities Code 739.1(a) and 739.12(a) for CARE and FERA alike. Verified against the live parameter tree: 2013-01-01, 2014-01-01 and 2014-08-13 resolve to 27 entries including housing_assistance; 2014-08-14, 2015-01-01 and 2026-01-01 resolve to 26 without it, the only set difference being housing_assistance, and all 27 names resolve to model variables. Co-Authored-By: Claude Fable 5.1 --- .../gov/states/ca/cpuc/income_sources.yaml | 66 ++++++++++++++----- 1 file changed, 51 insertions(+), 15 deletions(-) diff --git a/policyengine_us/parameters/gov/states/ca/cpuc/income_sources.yaml b/policyengine_us/parameters/gov/states/ca/cpuc/income_sources.yaml index 6728fbb331a..b68cbb486f0 100644 --- a/policyengine_us/parameters/gov/states/ca/cpuc/income_sources.yaml +++ b/policyengine_us/parameters/gov/states/ca/cpuc/income_sources.yaml @@ -1,16 +1,47 @@ description: >- California counts these income sources under the California Alternate Rates for Energy and Family Electric Rate Assistance programs. -# Housing subsidies are excluded by D.14-08-030, effective August 14, 2014. -# PG&E confirmed in its June 2026 reply brief that its contrary application -# wording does not describe its actual CARE/FERA income determination. -# The decision names CARE/ESA; PG&E's filed CARE/FERA practice supports -# applying this housing exclusion to the shared income definition. +# Each entry implements a phrase of the income footnote carried by the +# CARE/FERA program application: total gross annual household income includes +# all taxable and non-taxable revenues from all people living in the home, +# from whatever sources derived, including, but not limited to, wages, +# salaries, interest, dividends, spousal and child support payments, public +# assistance payments, Social Security and pensions, housing and military +# subsidies, rental income, income from self-employment and all +# employment-related, non-cash income. The same footnote appears on the +# CPUC-filed tariff sheet referenced below. +# +# Housing subsidies are the one exception. D.14-08-030, issued August 14, +# 2014, holds in Section 6.2 that the income definition remains unchanged +# except that housing subsidies will not be considered income, and Ordering +# Paragraph 40(3) adopts "Housing subsidies are not counted as income". The +# recommendation originates in the decision's multifamily segment study at +# Section 6.1.1.3, but Section 6.2 applies it to the income definition. The +# Statewide ESA Program Policy and Procedures Manual v1.4, Section 2.2.2, +# likewise lists housing subsidies among the receipts not counted as +# household income. +# +# The application's contrary wording is being corrected, not relied on: +# PG&E's June 5, 2026 reply brief in A.26-01-003 states that while its +# enrollment application lists housing and military subsidies as part of +# income, in practice PG&E does not use housing and military subsidies in its +# income analysis, and it plans to revise the application to remove the +# reference. +# +# FERA coverage of the exclusion rests on that 2026 brief, not on +# D.14-08-030 itself. The decision never names FERA, Family Electric Rate +# Assistance or Public Utilities Code 739.12; neither Section 6.2 nor +# Ordering Paragraph 40(3) reaches past CARE and ESA. The brief's Section B +# answers the definition of "income" under Public Utilities Code Sections +# 739.1(a) and 739.12(a) - 739.12(a) is FERA - and states the practice for +# CARE and FERA alike under their shared income definition. +# # Annual income calculations select the list at January 1: calendar 2014 # retains the earlier list, and calendar 2015 first applies the exclusion. values: # Preserve the earlier model's inclusion before this decision, using the - # actual benefit. The pre-2014 income rule has not been revalidated here. + # benefit the household receives rather than the poverty measure's capped + # subsidy. The pre-2014 income rule has not been revalidated here. 0000-01-01: # Wages, salaries - employment_income @@ -53,6 +84,7 @@ values: - gi_cash_assistance - debt_relief - illicit_income + # D.14-08-030 drops housing subsidies from the list. 2014-08-14: # Wages, salaries - employment_income @@ -99,20 +131,24 @@ metadata: period: year label: CA CPUC CARE/FERA income sources reference: - - title: Statewide ESA Program Policy and Procedures Manual v1.4, Section 2.2.2 - href: https://www.cpuc.ca.gov/-/media/cpuc-website/consumer-support/documents/20250418-statewide-esa-program-pp-manualv14.pdf#page=12 - - title: Statewide ESA Program Policy and Procedures Manual v1.4, Table E-1 - href: https://www.cpuc.ca.gov/-/media/cpuc-website/consumer-support/documents/20250418-statewide-esa-program-pp-manualv14.pdf#page=81 - # Retain the form's general income categories; its housing/military - # subsidy wording is corrected by the decision and PG&E filing below. - - title: PG&E Form 01-9077, CARE/FERA Program Application, Income Definition - href: https://www.pge.com/tariffs/assets/pdf/tariffbook/GAS_FORMS_01-9077.pdf#page=2 + - title: PG&E Form 01-9077, CARE/FERA Program Application (Rev. 6.26) + href: https://www.pge.com/assets/pge/localized/en/docs/account/billing-and-assistance/care-fera-application.pdf + - title: >- + Revised Cal. P.U.C. Sheet No. 61313-E, PG&E Electric Sample Form + No. 79-1055, CARE/FERA Program Application for Sub-Metered + Residential Customers (Large Print), Advice 7905-E, effective + June 1, 2026 + href: https://www.pge.com/tariffs/assets/pdf/tariffbook/ELEC_FORMS_79-1055.pdf#page=3 - title: CPUC Decision 14-08-030, Section 6.2, Income Definition href: https://liob.cpuc.ca.gov/wp-content/uploads/sites/14/2020/12/ACF22B3.pdf#page=75 - title: CPUC Decision 14-08-030, Ordering Paragraph 40(3) href: https://liob.cpuc.ca.gov/wp-content/uploads/sites/14/2020/12/ACF22B3.pdf#page=124 - - title: PG&E Reply Brief, June 5, 2026, Section B + - title: PG&E Reply Brief on Definitional Questions, A.26-01-003, June 5, 2026, Section B href: https://docs.cpuc.ca.gov/PublishedDocs/Efile/G000/M608/K305/608305628.PDF#page=10 + - title: Statewide ESA Program Policy and Procedures Manual v1.4, Section 2.2.2 + href: https://www.cpuc.ca.gov/-/media/cpuc-website/consumer-support/documents/20250418-statewide-esa-program-pp-manualv14.pdf#page=12 + - title: Statewide ESA Program Policy and Procedures Manual v1.4, Table E-1 + href: https://www.cpuc.ca.gov/-/media/cpuc-website/consumer-support/documents/20250418-statewide-esa-program-pp-manualv14.pdf#page=81 - title: CA PUC 739.1 (CARE) href: https://leginfo.legislature.ca.gov/faces/codes_displaySection.xhtml?lawCode=PUC§ionNum=739.1 - title: CA PUC 739.12 (FERA) From ee072e348724c1e4ec1035d9c0522a56e050b58a Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 20:23:53 -0400 Subject: [PATCH 08/21] Record step 2-3 completion and verification exit codes Co-Authored-By: Claude Fable 5.1 --- PROGRESS.md | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/PROGRESS.md b/PROGRESS.md index fd850781548..50b4f050927 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -17,13 +17,28 @@ Verified PR heads (`gh pr view --json headRefOid`, 2026-09-14): - [x] Read reconciliation memo, FINAL-REPORT, EXPANDED-REVIEW, FINDINGS-RECOVERY - [x] Worktree + branch from `upstream/main` - [x] Step 2a: #9466 cherry-picked as-is (`c5d8abfb44`) -- [x] Step 2b: #9463 applied (`a6f1e8da14`) — isolation mechanism, county FIPS, docs, 16 regression - tests, pinned_tbs.py cache key; dropped its PROGRESS.md +- [x] Step 2b: #9463 applied (`a6f1e8da14`) +- [x] Step 2c: #9448 reduced (`d1251c47cb`) +- [x] Step 2d: #9464 reduced (`c4209228d7`) +- [x] Step 3: R1/R2 already satisfied by #9448's head; R8 + citations (`28e7ac9422`); + R9 satisfied by #9448's CPUC Cases 4/5 + `test_cpuc_housing_exclusion_effective_date` +- [x] Lane 20260914-144927-spm-fix-isolation-202 finished; #9463 head re-verified UNCHANGED at d8cbd4b7 -## Next -- [ ] Step 2c: #9448 reduced -- [ ] Step 2d: #9464 reduced -- [ ] Step 3: EXPANDED-REVIEW R1/R2 fixes; R8 FERA note in CPUC parameter -- [ ] Step 4: verification suites + exit codes -- [ ] Step 5: push, draft PR, CI +## Verified so far (exit codes) +| Suite | Result | Exit | +|---|---|---| +| `ruff format --check .` / `ruff check .` | 6444 files formatted, all checks passed | 0 / 0 | +| CPUC YAML (`policy/baseline/gov/states/ca/cpuc`) | 36 passed | 0 | +| Tlaib YAML (`policy/contrib/congress/tlaib`) | 22 passed | 0 | +| Reform YAML (`policy/reform`) | 51 passed | 0 | +| `test_spm_integration_contract` + `test_cpuc_spm_independence` + `test_spm_source_input_contract` | 46 passed | 0 | +| `.github/release_lock.py --committed` | resolved 149 packages | 0 | +| `.github/tests/test_release_*.py` | 33 tests, 1 skipped | 0 | + +## Open +- [ ] #9448's `test_spm_policy_family.py` is NOT carryable as-is: 16 of its 27 cases fail against + #9463's mechanism (they assert a child branch's reform propagates to parent and siblings; + #9463 deliberately isolates it). Adjudication workflow running. +- [ ] Full `tests/core` + `tests/unit` + `tests/code_health` run +- [ ] Push, draft PR, CI - [ ] REMOVE PROGRESS.md before final push From 1bc9a835462b46006443c6f4d963452514309953 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 20:36:31 -0400 Subject: [PATCH 09/21] Keep the copy-on-write barrier on a cloned system Found while adjudicating whether #9448's `test_spm_policy_family.py` could be carried onto #9463's mechanism, and reproduced here with public-API calls. `clone_spm_system` hands core an ordinary instance to clone through - `plain_policy_copy` -> `SharedParameterPolicy.unshared_copy()` resets `__class__` to the base class - and never put the barrier back. So `Simulation.clone()` returned a plain `CountryTaxBenefitSystem`, and `apply_reform` on anything reading that tree took the unguarded path: clone = Simulation(situation=...).clone() clone.get_branch("probe").apply_reform({SINGLE standard deduction: 100_000}) # clone's own SINGLE standard deduction moved 14,600 -> 100,000 That is the defect `SharedParameterPolicy` exists to prevent, and it contradicts `test_a_branch_reform_does_not_rewrite_its_parents_policy`, which only passed because it never cloned. `with_parameter_barrier`'s own docstring already states the rule: every simulation's system carries the barrier, because a tree it starts out owning can acquire readers later. Bounded: a clone gets its own tree, so neither the shipped module-level system nor the simulation the clone came from was reachable this way; the leak stayed inside one cloned simulation's branch family. Verified before and after. `test_cloning_a_shared_policy_system_stops_sharing_its_tree` asserted `"shared_parameters" not in cloned.__dict__`, which pinned the defective implementation rather than the invariant. It now asserts that the clone holds its own tree behind the barrier; its two substantive assertions are unchanged. Co-Authored-By: Claude Fable 5.1 --- .../spm-clone-parameter-barrier.fixed.md | 1 + policyengine_us/spm.py | 7 +++ .../core/test_spm_simulation_isolation.py | 51 ++++++++++++++++++- 3 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 changelog.d/spm-clone-parameter-barrier.fixed.md diff --git a/changelog.d/spm-clone-parameter-barrier.fixed.md b/changelog.d/spm-clone-parameter-barrier.fixed.md new file mode 100644 index 00000000000..387f25a2ae1 --- /dev/null +++ b/changelog.d/spm-clone-parameter-barrier.fixed.md @@ -0,0 +1 @@ +Reinstall the copy-on-write parameter barrier on a cloned tax-benefit system, so a branch of a cloned simulation no longer rewrites the clone's own policy tree. diff --git a/policyengine_us/spm.py b/policyengine_us/spm.py index 8cd58f73642..4419a4432a5 100644 --- a/policyengine_us/spm.py +++ b/policyengine_us/spm.py @@ -662,6 +662,13 @@ def clone_spm_system(system, *, copy_receipts=True): cloned.spm_forecast_provider = system.spm_forecast_provider.snapshot( copy_receipts=copy_receipts ) + # A clone owns its tree outright, but it does not keep it to itself: a + # branch copies the system, and another simulation can be built on it, at + # which point a reform must clone before it writes. ``plain_policy_copy`` + # strips the barrier so core can clone through an ordinary instance, so + # put it back - without it, a branch of a clone rewrites the clone's own + # policy, which is the defect this class exists to prevent. + with_parameter_barrier(cloned) return cloned diff --git a/policyengine_us/tests/core/test_spm_simulation_isolation.py b/policyengine_us/tests/core/test_spm_simulation_isolation.py index 1c88396d872..e3f955ade3a 100644 --- a/policyengine_us/tests/core/test_spm_simulation_isolation.py +++ b/policyengine_us/tests/core/test_spm_simulation_isolation.py @@ -265,6 +265,12 @@ def test_cloning_a_shared_policy_system_stops_sharing_its_tree(): A shared system resolves ``parameters`` through a property, which would shadow that write and hand back a "clone" still sharing the lender's tree, with nothing raised. + + The clone keeps the barrier rather than reverting to a plain system: it is + the sole reader of its own tree, so nothing is marked shared and no copy is + owed, but the tree can acquire readers later and must then be copied before + it is written. See + ``test_a_clone_keeps_the_copy_on_write_barrier_its_parent_had``. """ shared = Simulation(situation=earner_situation()).tax_benefit_system assert shared.parameters is system.parameters @@ -273,7 +279,7 @@ def test_cloning_a_shared_policy_system_stops_sharing_its_tree(): assert cloned.parameters is not system.parameters assert not getattr(cloned, "shares_parameters", False) - assert "shared_parameters" not in cloned.__dict__ + assert cloned.__dict__["shared_parameters"] is cloned.parameters assert parameter_fingerprint(cloned) == parameter_fingerprint(system) @@ -486,3 +492,46 @@ def test_a_tuple_of_variable_only_reforms_keeps_sharing_the_tree(): assert simulation.calculate("clone_only_income", 2024)[0] == 123 assert policy.parameters is system.parameters assert policy.shares_parameters + + +def test_a_clone_keeps_the_copy_on_write_barrier_its_parent_had(): + """A clone owns its tree, but it still must not be written in place. + + ``clone_spm_system`` hands core an ordinary instance to clone through, by + stripping the barrier class in ``plain_policy_copy``. Core's clone returns + that ordinary instance, so without putting the barrier back the clone's own + tree is writable in place - and a branch of the clone, which copies the + system and so becomes a second reader, rewrites it. + """ + simulation = Simulation(situation=earner_situation()) + clone = simulation.clone() + + assert clone.tax_benefit_system.shares_parameters is not None + + branch = clone.get_branch("reforming_branch") + branch.apply_reform({SINGLE_STANDARD_DEDUCTION: {"2024": 100_000}}) + + assert branch.calculate("standard_deduction", 2024)[0] == 100_000 + assert clone.calculate("standard_deduction", 2024)[0] == 14_600 + assert simulation.calculate("standard_deduction", 2024)[0] == 14_600 + + +def test_a_reform_on_a_clone_still_reaches_the_clones_own_policy(): + """The barrier must not make a clone's own reform a no-op. + + The clone is the only reader of its tree until something else copies it, so + its own reform is free to detach and apply; what it must not do is reach + the simulation it was cloned from. + """ + simulation = Simulation(situation=earner_situation()) + assert simulation.calculate("income_tax", 2024)[0] == BASELINE_INCOME_TAX + clone = simulation.clone() + + clone.apply_reform({SINGLE_STANDARD_DEDUCTION: {"2024": 100_000}}) + + assert clone.calculate("standard_deduction", 2024)[0] == 100_000 + assert simulation.calculate("standard_deduction", 2024)[0] == 14_600 + assert ( + system.parameters.gov.irs.deductions.standard.amount.SINGLE("2024-01-01") + == 14_600 + ) From 4256c3fa2d2122859437200701b0a5eb77b74202 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 20:40:41 -0400 Subject: [PATCH 10/21] Carry the mechanism-independent half of #9448's policy-family tests #9448's `test_spm_policy_family.py` cannot be carried whole: run unchanged against #9463's mechanism, 16 of its 27 cases fail. They split four ways. Nine assert #9448's own internals - the `_parameter_view` / `_parameter_source` / `parameters.modified` provenance protocol and `get_parameters_at_instant` - which this branch does not implement, because #9463 owns the mechanism. Six turn on `Simulation.tracer` being a plain attribute in core, so a bare `simulation.tracer = X` re-primes nothing and X records no parameter receipts. #9463 hooks the `trace` setter, `__init__`, `get_branch`, `apply_reform`, `_adopt_detached_parameters` and `clone`, but not a bare `tracer` write. No production caller does one that matters, so these are dropped rather than fixed here. Four assert that a child branch's reform propagates up to its parent and siblings. Stated plainly: that is stock policyengine-core behaviour - `Simulation.clone` hands a `clone_system=False` branch the SAME tax-benefit system object and `apply_reform` mutates it in place - so #9463 is a deliberate deviation from core here, not a restoration of it. It is still the right deviation to ship: every branch in this repository that actually mutates policy already passes `clone_system=True` (`tob_revenue_oasdi.py:42`, `behavioral_response_measurements.py:49`), while the `clone_system=False` branches only set inputs, and a formula-internal branch that silently moved its parent's answer would be order-dependent. But it is a design decision, not something core's contract compels. One depends on a `reforms/reforms.py` hunk this branch drops, which returns `None` from `create_structural_reforms_from_parameters` when no structural reform is active. That case is unreachable - `create_mi_surtax_reform` always returns a reform, so the filtered tuple is never empty - and carrying the hunk verbatim is unsafe: `CountryTaxBenefitSystem.__init__` packs the result into a reform tuple, and `CountryTaxBenefitSystem(reform=((), None))` raises `AttributeError: 'NoneType' object has no attribute 'apply'` (verified). The eleven cases kept here are the mechanism-independent remainder: a nested branch clone's inherited inputs and private reform, a construction-time structural reform detaching before it mutates parameters, the warm-cache contract for a caller-supplied system across clone hops and a reform wrapper, an at-instant view refreshing after an in-place leaf update, and a reform reading back its own edit before it returns. All eleven pass. Also pins the boundary of the barrier in `test_spm_simulation_isolation.py`: it covers the parameter tree, not the variable registry, so a variable-only reform on a shared-policy branch does still reach its parent and siblings. #9463 never claimed otherwise, but nothing recorded the limit. Co-Authored-By: Claude Fable 5.1 --- .../tests/core/test_spm_policy_family.py | 194 ++++++++++++++++++ .../core/test_spm_simulation_isolation.py | 40 ++++ 2 files changed, 234 insertions(+) create mode 100644 policyengine_us/tests/core/test_spm_policy_family.py diff --git a/policyengine_us/tests/core/test_spm_policy_family.py b/policyengine_us/tests/core/test_spm_policy_family.py new file mode 100644 index 00000000000..0c077a4c1b4 --- /dev/null +++ b/policyengine_us/tests/core/test_spm_policy_family.py @@ -0,0 +1,194 @@ +"""Policy sharing boundaries a reform, a clone or a prepared system must respect. + +These cases come from #9448, which proposed a different isolation mechanism from +the one this branch carries (#9463's). Most of that file asserted #9448's own +internals, or asserted that a child branch's reform propagates up to its parent +and siblings - which #9463 deliberately reverses, and which +``test_a_branch_reform_does_not_rewrite_its_parents_policy`` in +``test_spm_simulation_isolation.py`` pins the other way. What is kept here is +the part that is mechanism-independent and that #9463's own suite leaves +uncovered: a nested branch clone's inherited inputs, a structural reform at a +changed start instant, the warm-cache contract for a system supplied by a +caller, and a reform reading back its own in-progress edit. +""" + +import importlib + +import numpy as np +import pytest +from policyengine_core.parameters import ParameterNode +from policyengine_core.reforms import Reform + +from policyengine_us import CountryTaxBenefitSystem, Simulation +from policyengine_us.system import system + + +def earning_household(): + return { + "people": { + "person": { + "age": {2024: 40}, + "employment_income": {2024: 50_000}, + } + }, + "households": { + "household": { + "members": ["person"], + "county_fips": {2024: "06037"}, + } + }, + } + + +class ReplaceParameterRoot(Reform): + def apply(self): + def replace(parameters): + parameters = parameters.clone() + parameters.gov.irs.deductions.standard.amount.SINGLE.update( + period="2024", value=100_000 + ) + # A public modifier may return an ordinary core root, without the + # country adapter's per-simulation root subclass. + parameters.__class__ = ParameterNode + return parameters + + self.modify_parameters(replace) + + +class VariableOnlyReform(Reform): + def apply(self): + self.neutralize_variable("income_tax") + + +@pytest.mark.parametrize("replace_root", [False, True]) +def test_detached_branch_clone_preserves_inherited_inputs_and_private_reform( + replace_root, +): + original = Simulation(situation=earning_household()) + middle = original.get_branch("middle") + middle.set_input("employment_income_before_lsr", 2024, [70_000]) + leaf = middle.get_branch("leaf") + before = [ + member.calculate("income_tax", 2024).copy() + for member in (original, middle, leaf) + ] + detached = leaf.clone() + detached.calculate("income_tax", 2024) + reform = ( + ReplaceParameterRoot + if replace_root + else {"gov.irs.deductions.standard.amount.SINGLE": {"2024": 100_000}} + ) + detached.apply_reform(reform) + reference_inputs = earning_household() + reference_inputs["people"]["person"]["employment_income"] = {2024: 70_000} + reference = Simulation(situation=reference_inputs, reform=reform) + assert detached.calculate("employment_income", 2024)[0] == 70_000 + np.testing.assert_array_equal( + detached.calculate("income_tax", 2024), reference.calculate("income_tax", 2024) + ) + for member, expected in zip((original, middle, leaf), before): + np.testing.assert_array_equal(member.calculate("income_tax", 2024), expected) + + +@pytest.mark.parametrize("use_modifier", [False, True]) +def test_changed_structural_start_detaches_before_parameter_mutation( + monkeypatch, use_modifier +): + module = importlib.import_module("policyengine_us.system") + source = system.parameters + children = source.children + before = source("2024-01-01").gov.irs.deductions.standard.amount.SINGLE + cached = dict(source.gov._at_instant_cache) + + def mutate(parameters): + assert parameters.children is not children + parameters.gov.irs.deductions.standard.amount.SINGLE.update( + period="2024", value=100_000 + ) + return parameters + + class StructuralParameterChange(Reform): + def apply(self): + if use_modifier: + self.modify_parameters(mutate) + else: + mutate(self.parameters) + + monkeypatch.setattr( + module, + "create_structural_reforms_from_parameters", + lambda parameters, instant: StructuralParameterChange, + ) + simulation = Simulation(situation=earning_household(), start_instant="2026-01-01") + assert ( + simulation.tax_benefit_system.parameters.gov.irs.deductions.standard.amount.SINGLE( + 2024 + ) + == 100_000 + ) + assert source.gov.irs.deductions.standard.amount.SINGLE(2024) == before + assert source.children is children + for instant, node in cached.items(): + assert source.gov._at_instant_cache[instant] is node + + +@pytest.mark.parametrize( + "clone_count,reform_wrapper", + [(0, False), (1, False), (2, False), (0, True), (1, True)], +) +def test_supplied_prepared_system_retains_warm_policy_children( + clone_count, reform_wrapper +): + source = CountryTaxBenefitSystem() if not clone_count else system + for _ in range(clone_count): + source = source.clone() + if reform_wrapper: + source = VariableOnlyReform(source) + source.parameters("2024-01-01").gov.irs.deductions.standard.amount.SINGLE + cached = dict(source.parameters.gov._at_instant_cache) + children = source.parameters.children + simulation = Simulation(tax_benefit_system=source, situation=earning_household()) + assert simulation.tax_benefit_system.parameters.children is children + assert source.parameters.gov._at_instant_cache + for instant, node in cached.items(): + assert ( + simulation.tax_benefit_system.parameters.gov._at_instant_cache[instant] + is node + ) + + +def test_a_systems_parameter_view_refreshes_after_an_in_place_leaf_update(): + """An at-instant read taken before an in-place edit must not be served again.""" + source = system.clone() + assert ( + source.parameters("2024-01-01").gov.irs.deductions.standard.amount.SINGLE + < 100_000 + ) + source.parameters.gov.irs.deductions.standard.amount.SINGLE.update( + period="2024", value=100_000 + ) + assert ( + source.parameters("2024-01-01").gov.irs.deductions.standard.amount.SINGLE + == 100_000 + ) + + +def test_reform_can_read_updated_parameters_before_returning(): + class ReadDuringUpdate(Reform): + def apply(self): + parameters = self.parameters + assert ( + parameters("2024-01-01").gov.irs.deductions.standard.amount.SINGLE + < 100_000 + ) + parameters.gov.irs.deductions.standard.amount.SINGLE.update( + period="2024", value=100_000 + ) + assert ( + parameters("2024-01-01").gov.irs.deductions.standard.amount.SINGLE + == 100_000 + ) + + simulation = Simulation(situation=earning_household()) + simulation.apply_reform(ReadDuringUpdate) diff --git a/policyengine_us/tests/core/test_spm_simulation_isolation.py b/policyengine_us/tests/core/test_spm_simulation_isolation.py index e3f955ade3a..9dcedf13de5 100644 --- a/policyengine_us/tests/core/test_spm_simulation_isolation.py +++ b/policyengine_us/tests/core/test_spm_simulation_isolation.py @@ -535,3 +535,43 @@ def test_a_reform_on_a_clone_still_reaches_the_clones_own_policy(): system.parameters.gov.irs.deductions.standard.amount.SINGLE("2024-01-01") == 14_600 ) + + +def test_a_branch_variable_reform_is_not_isolated_from_its_parent(): + """Record the boundary of the barrier: it covers parameters, not variables. + + ``SharedParameterPolicy`` guards the parameter tree. A branch created with + ``clone_system=False`` still shares its parent's ``variables`` dict - core + hands the branch the same tax-benefit system object + (``Simulation.clone``: ``new.tax_benefit_system = self.tax_benefit_system``) + and every variable operation mutates that registry - so a variable-only + reform on a branch does reach its parent and siblings. + + This is the documented limit of what this module isolates, not an + endorsement: a caller that wants a variable reform to itself must pass + ``clone_system=True``, which every such call site in this repository + already does. The test exists so the limit cannot be mistaken for + isolation, and so that closing it later is a visible change. + """ + parent = Simulation(situation=earner_situation()) + child = parent.get_branch("child") + sibling = parent.get_branch("sibling") + assert parent.calculate("income_tax", 2024)[0] == BASELINE_INCOME_TAX + + child.apply_reform(NeutralizeIncomeTax) + + assert child.calculate("income_tax", 2024)[0] == 0 + assert parent.tax_benefit_system.variables is child.tax_benefit_system.variables + for member in (parent, sibling): + member._invalidate_all_caches() + assert member.calculate("income_tax", 2024)[0] == 0 + + # A branch that asked for its own system keeps the reform to itself. + independent = Simulation(situation=earner_situation()).get_branch( + "independent", clone_system=True + ) + independent.apply_reform(NeutralizeIncomeTax) + assert ( + Simulation(situation=earner_situation()).calculate("income_tax", 2024)[0] + == BASELINE_INCOME_TAX + ) From 646bd95e582959ec45336a0cc9d4db7fa168ac5c Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 20:57:05 -0400 Subject: [PATCH 11/21] Remove the working progress log Co-Authored-By: Claude Fable 5.1 --- PROGRESS.md | 44 -------------------------------------------- 1 file changed, 44 deletions(-) delete mode 100644 PROGRESS.md diff --git a/PROGRESS.md b/PROGRESS.md deleted file mode 100644 index 50b4f050927..00000000000 --- a/PROGRESS.md +++ /dev/null @@ -1,44 +0,0 @@ -# PROGRESS — unified SPM candidate (max/spm-unified-candidate-20260914) - -## State -Base: `upstream/main` @ `2cc6f045ab1f389bc781ffd9f5e874055b3c0a3d` (version 2.1.0). -Worktree: `/Users/maxghenis/spm-rebuild-20260908/worktrees/policyengine-us-spm-unified-20260914`. - -Verified PR heads (`gh pr view --json headRefOid`, 2026-09-14): -| PR | head | merge-base with main | -|---|---|---| -| #9466 | `9f9510b735b3b9b2e375854d66fd315202e31d35` | 2cc6f045ab | -| #9463 | `d8cbd4b7a0b292beafad37cfc72813e1d710fc3e` (lane 20260914-144927 still RUNNING — re-verify before push) | 2cc6f045ab | -| #9448 | `8f74a5152d1e52c33bd7a7cba6340346bd7fe004` | 2ba7daa30e | -| #9464 | `cafcd9443df7f638a5ad547a92c6e0bcfd784fbb` | 4d34e3d1ab | -| #9462 | `47e94fa491bbf65ae4b6bd5667f054445b26de36` | deliberately excluded (Microcosm #925) | - -## Done -- [x] Read reconciliation memo, FINAL-REPORT, EXPANDED-REVIEW, FINDINGS-RECOVERY -- [x] Worktree + branch from `upstream/main` -- [x] Step 2a: #9466 cherry-picked as-is (`c5d8abfb44`) -- [x] Step 2b: #9463 applied (`a6f1e8da14`) -- [x] Step 2c: #9448 reduced (`d1251c47cb`) -- [x] Step 2d: #9464 reduced (`c4209228d7`) -- [x] Step 3: R1/R2 already satisfied by #9448's head; R8 + citations (`28e7ac9422`); - R9 satisfied by #9448's CPUC Cases 4/5 + `test_cpuc_housing_exclusion_effective_date` -- [x] Lane 20260914-144927-spm-fix-isolation-202 finished; #9463 head re-verified UNCHANGED at d8cbd4b7 - -## Verified so far (exit codes) -| Suite | Result | Exit | -|---|---|---| -| `ruff format --check .` / `ruff check .` | 6444 files formatted, all checks passed | 0 / 0 | -| CPUC YAML (`policy/baseline/gov/states/ca/cpuc`) | 36 passed | 0 | -| Tlaib YAML (`policy/contrib/congress/tlaib`) | 22 passed | 0 | -| Reform YAML (`policy/reform`) | 51 passed | 0 | -| `test_spm_integration_contract` + `test_cpuc_spm_independence` + `test_spm_source_input_contract` | 46 passed | 0 | -| `.github/release_lock.py --committed` | resolved 149 packages | 0 | -| `.github/tests/test_release_*.py` | 33 tests, 1 skipped | 0 | - -## Open -- [ ] #9448's `test_spm_policy_family.py` is NOT carryable as-is: 16 of its 27 cases fail against - #9463's mechanism (they assert a child branch's reform propagates to parent and siblings; - #9463 deliberately isolates it). Adjudication workflow running. -- [ ] Full `tests/core` + `tests/unit` + `tests/code_health` run -- [ ] Push, draft PR, CI -- [ ] REMOVE PROGRESS.md before final push From e18473fa0a82a52a927727e9274894a31f934f4f Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 21:56:17 -0400 Subject: [PATCH 12/21] Start the second-pass review progress log Co-Authored-By: Claude Fable 5.1 --- PROGRESS.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 PROGRESS.md diff --git a/PROGRESS.md b/PROGRESS.md new file mode 100644 index 00000000000..57fa0364569 --- /dev/null +++ b/PROGRESS.md @@ -0,0 +1,23 @@ +# Second-pass review work on #9467 + +Working log for the adversarial review at +`rollout/fable-continuation-20260911/out/review-9467.md`. Removed before the +branch is handed back. + +## State + +Addressing the six findings the review raised, one commit each, then the fast +local suites, the release-lock check, a push and CI. + +## Done + +- Read the review and the code each finding names. + +## Next + +- F1 full-population housing-allocation probe in the microsimulation suite. +- F2 changelog retype and consequence text, plus the restored #9448 fragment. +- F3 documented default-build digest and a test pinning the constant to it. +- F4 docs/`release-lock.md` passages describing what actually ships. +- F5 carried tracer tests and the policy-family docstring correction. +- F6 modeled-versus-capped note in the CPUC fragment and parameter comment. From 693d6519a6ed9b8dd2ede808b95222026206175a Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 21:57:33 -0400 Subject: [PATCH 13/21] Probe the housing allocation over the whole default population The review's F1: `spm_unit_allocated_housing_subsidy` prorates a household's modeled `housing_assistance` to every co-resident SPM unit, and `spm_unit_capped_housing_subsidy` masks on a positive allocated share, so `masked_policyengine_amount` now demands a classified measurement adult from units that received no award and held zero housing resource under 2.0.1. A co-resident unit whose members are all under 15, or 15 to 17 without a source-backed independence role, would fail the resource chain for the entire population array. Nothing in the suite ruled that out on the certified build: `test_default_dataset_loads_and_runs` subsamples 1,000 units. This adds a full-population probe with no subsample that computes the allocation, the measurement adult count and `spm_unit_net_income` for 2024, asserts the resource chain completes, and reports the overlap count in the assertion message so a nonzero result names how many units are affected. The same probe runs over `enhanced_cps_2024`, already exercised by this suite and the one population file known to carry units that classify no measurement adult. Only the allocation is probed there: its threshold fails closed on those 18 lone minors whatever the allocation does. No guard is added here. Whether the allocation should be restricted to units that classify an adult is a decision for a reviewer holding the count. Co-Authored-By: Claude Fable 5.1 --- .../tests/microsimulation/test_microsim.py | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/policyengine_us/tests/microsimulation/test_microsim.py b/policyengine_us/tests/microsimulation/test_microsim.py index 25f764eee7e..aeb05c305ad 100644 --- a/policyengine_us/tests/microsimulation/test_microsim.py +++ b/policyengine_us/tests/microsimulation/test_microsim.py @@ -141,3 +141,86 @@ def test_default_dataset_loads_and_runs(): ) for variable in ("employment_income", "self_employment_income"): assert sim.calc(variable, period=2024).sum() > 0, f"{variable} is zero in 2024." + + +def test_default_dataset_allocates_no_housing_share_to_an_unclassified_unit(): + """The member-share housing allocation over the whole certified default build. + + `spm_unit_allocated_housing_subsidy` prorates a household's modeled + `housing_assistance` to every SPM unit in that household by member share, + and `spm_unit_capped_housing_subsidy` consults the canonical housing + portion for every unit holding a positive share. That consultation reaches + `SPM_COMPOSITION_REQUIRED`, so a co-resident unit that classifies no + measurement adult - everyone under 15, or 15 to 17 without a source-backed + independence role - would now fail the whole population's resource chain + where the shipped 2.0.1 valuation left it at zero. + + This probe is over `populace_us_2024`, the certified default build, at full + population with no subsample: `test_default_dataset_loads_and_runs` + subsamples 1,000 units, which cannot rule out a pattern this rare. If the + count below is ever nonzero, the allocation needs a guard - allocate only + among units that classify an adult and leave the remainder with the awarded + unit - and that decision belongs to a reviewer, not to this test. + """ + import numpy as np + from spm_calculator.errors import SPMInputError + + from policyengine_us import Microsimulation + + simulation = Microsimulation() # populace_us_2024, full population. + allocated = np.asarray( + simulation.calculate("spm_unit_allocated_housing_subsidy", 2024) + ) + adults = np.asarray(simulation.calculate("spm_measurement_adults", 2024)) + unclassified = int(((allocated > 0) & (adults < 1)).sum()) + assert unclassified == 0, ( + f"populace_us_2024 (certified default build): {unclassified} of " + f"{allocated.size} SPM units hold an allocated housing share while " + "classifying no measurement adult, so the member-share allocation " + "raises SPM_COMPOSITION_REQUIRED over the whole population." + ) + + try: + net_income = np.asarray(simulation.calculate("spm_unit_net_income", 2024)) + except SPMInputError as error: + pytest.fail( + "populace_us_2024 (certified default build): the SPM resource " + f"chain raised {error.code} even though {unclassified} allocated " + f"units classify no measurement adult, so the housing allocation " + f"is not what failed: {error}" + ) + assert net_income.size == allocated.size, ( + "populace_us_2024 (certified default build): net income and the " + "housing allocation must cover the same SPM units." + ) + + +def test_legacy_enhanced_cps_allocates_no_housing_share_to_an_unclassified_unit(): + """The same allocation probe over `enhanced_cps_2024`. + + That file is already exercised above, and it is the one population file + known to contain units classifying no measurement adult: 18 lone + 15-to-17-year-olds. None of them receives a housing award, which the test + above asserts, but an award is no longer what pulls a unit into the cap - + a member share of a co-resident family's award does. So the overlap is + reported here as its own count. + + Only the allocation is probed on this file. Its SPM threshold fails closed + on those 18 units whatever the housing allocation does, so a resource-chain + assertion here would pin that older defect rather than this one. + """ + import numpy as np + + from policyengine_us import Microsimulation + + simulation = Microsimulation(dataset=ENHANCED_CPS_2024) + allocated = np.asarray( + simulation.calculate("spm_unit_allocated_housing_subsidy", 2024) + ) + adults = np.asarray(simulation.calculate("spm_measurement_adults", 2024)) + unclassified = int(((allocated > 0) & (adults < 1)).sum()) + assert unclassified == 0, ( + f"enhanced_cps_2024 (legacy file): {unclassified} of {allocated.size} " + "SPM units hold an allocated housing share while classifying no " + "measurement adult." + ) From f378f490eeca84b4421319c89deb05f35e3d4154 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 21:58:41 -0400 Subject: [PATCH 14/21] State what the housing changes move, and restore #9448's fragment The review's F2. Two of the carried changes move published numbers, and both fragments described only the mechanism. The member-share allocation shifts SPM resources between co-resident SPM units, so SPM poverty and deep poverty change for multi-unit households where any unit receives housing assistance - and only for those; a single-unit household's allocation is its own award (`test_single_unit_allocation_preserves_program_amount`). An awarded family's tenant payment can now zero a co-resident unit's housing resource (`test_independent_unit_caps_do_not_redistribute_unused_housing_value`), which the fragment names as an assumption. Counting actual `housing_assistance` in `household_benefits` and `cbo_means_tested_transfers` raises household benefits, household net income, marginal tax rates and CBO transfer aggregates for every assisted household by whatever the SPM cap withheld, without moving SPM poverty, and CARE/FERA countable income loses the housing entry from 2015 for annual calculations. Both are retyped `changed` rather than `fixed`: they are not defect repairs to a caller, they are moved numbers. No `breaking` fragment is added. `spm-supplied-policy-caches.fixed.md` returns, dropped from #9448 while `test_supplied_prepared_system_retains_warm_policy_children` and its neighbours were carried. It describes the behaviour this branch ships, not #9448's mechanism for it. Co-Authored-By: Claude Fable 5.1 --- changelog.d/care-housing-income.changed.md | 1 + changelog.d/care-housing-income.fixed.md | 1 - changelog.d/spm-housing-allocation.changed.md | 1 + changelog.d/spm-housing-allocation.fixed.md | 1 - changelog.d/spm-supplied-policy-caches.fixed.md | 1 + 5 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 changelog.d/care-housing-income.changed.md delete mode 100644 changelog.d/care-housing-income.fixed.md create mode 100644 changelog.d/spm-housing-allocation.changed.md delete mode 100644 changelog.d/spm-housing-allocation.fixed.md create mode 100644 changelog.d/spm-supplied-policy-caches.fixed.md diff --git a/changelog.d/care-housing-income.changed.md b/changelog.d/care-housing-income.changed.md new file mode 100644 index 00000000000..9b82493ca48 --- /dev/null +++ b/changelog.d/care-housing-income.changed.md @@ -0,0 +1 @@ +Count actual housing assistance, rather than its SPM-capped valuation, in general household benefits and CBO means-tested transfers, and keep the capped valuation inside SPM resources. Household benefits, household net income, marginal tax rates and CBO transfer aggregates therefore rise for every household receiving housing assistance, by the amount the SPM cap withheld, while SPM poverty is unchanged by this part. Exclude housing subsidies from California CARE and FERA countable income from the August 14, 2014 parameter date, which annual calculations first apply in 2015, so countable income falls and CARE/FERA eligibility widens for assisted California households from that year. Preserve the earlier modeled housing inclusion using actual assistance; its historical policy basis was not revalidated. diff --git a/changelog.d/care-housing-income.fixed.md b/changelog.d/care-housing-income.fixed.md deleted file mode 100644 index 3ad06524d59..00000000000 --- a/changelog.d/care-housing-income.fixed.md +++ /dev/null @@ -1 +0,0 @@ -Keep SPM housing subsidy valuation within SPM resources and use actual housing assistance for household benefits and CBO transfers. Exclude housing subsidies from California CARE/FERA income from the August 14, 2014 parameter date (2015 for annual calculations). Preserve earlier modeled housing inclusion using actual assistance; its historical policy basis was not revalidated. diff --git a/changelog.d/spm-housing-allocation.changed.md b/changelog.d/spm-housing-allocation.changed.md new file mode 100644 index 00000000000..bc195250bc9 --- /dev/null +++ b/changelog.d/spm-housing-allocation.changed.md @@ -0,0 +1 @@ +Allocate a household's housing assistance, and the tenant contributions of the families actually awarded it, across that household's SPM units by member share before each unit's SPM resource cap. This moves SPM resources between co-resident SPM units, so SPM poverty and deep poverty change for households holding more than one SPM unit where any of them receives housing assistance; households holding a single SPM unit are unchanged, and no program award or general benefit income changes. An awarded family's tenant payment now also reduces a co-resident unit's allocated housing resource, which is recorded as a modeling assumption rather than verified Census parity. diff --git a/changelog.d/spm-housing-allocation.fixed.md b/changelog.d/spm-housing-allocation.fixed.md deleted file mode 100644 index c8543a7dbe2..00000000000 --- a/changelog.d/spm-housing-allocation.fixed.md +++ /dev/null @@ -1 +0,0 @@ -Prorate household housing assistance and the tenant contributions of awarded families across SPM units by member share before applying each unit's SPM resource cap, while preserving program awards and general benefit income. Document the tenant-payment allocation as a modeling assumption. diff --git a/changelog.d/spm-supplied-policy-caches.fixed.md b/changelog.d/spm-supplied-policy-caches.fixed.md new file mode 100644 index 00000000000..90b630220a3 --- /dev/null +++ b/changelog.d/spm-supplied-policy-caches.fixed.md @@ -0,0 +1 @@ +Preserve the warm parameter caches of a policy system supplied by a caller across clones and variable-only reform wrappers at the same structural start, detach a structural reform's parameter tree before it writes at a changed start instant, and let a reform read back its own in-progress parameter edit. From bd382b4ccbd4954426ba38c116807d59b1c146c4 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 22:01:01 -0400 Subject: [PATCH 15/21] Record the certified build's digest where it can be checked The review's F3: `DEFAULT_DATASET_SHA256` was the only occurrence of that digest in the repository, and both tests that exercise the check substitute their own value for it, so an edited constant would pass a green suite and every default `Microsimulation()` would then fail closed against bytes nobody could compare with a record. `docs/spm.md` now carries the hash beside the URI it belongs to, with the date it was read from the producer's build, and states where the documented value meets the published bytes: `Microsimulation()` digests whatever the tag resolves to, so the microsimulation CI job that downloads the tag and constructs the default is the check against the Hub. `test_documented_default_build_matches_the_shipped_constants` reads the URI and the hash back out of that section and compares them with the shipped constants, with no monkeypatching, so editing one side alone fails. It skips outside a source checkout, where `docs/` is not present; deleting the section inside one fails. The artifact itself was not downloaded here. Co-Authored-By: Claude Fable 5.1 --- docs/spm.md | 14 +++++++ policyengine_us/tests/core/test_spm_system.py | 42 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/docs/spm.md b/docs/spm.md index cf198fa5a48..9707f9fa7d4 100644 --- a/docs/spm.md +++ b/docs/spm.md @@ -120,6 +120,8 @@ eligibility are unchanged. The dataset loader rejects stored formula-owned SPM outputs; observed source results should use report-only column names. It does not delete or silently recalculate over supplied derived inputs. +## Default population build + `Microsimulation()` defaults to the immutable dataset URI `hf://datasets/policyengine/populace-us/populace_us_2024.h5@populace-us-2024-spm-20260909`. This dataset supplies observed county inputs and source-backed independence roles. @@ -127,6 +129,18 @@ The canonical country release must wait until that exact tag and its certified bytes exist and pass independent readback. The unpublished candidate embeds the same URI; local candidate checks do not establish production default availability. +A Hugging Face tag is a mutable pointer, so the model pins the build's content as +well as its name. The certified `populace_us_2024.h5` has content hash +`sha256:6496cc4393d4d3c6574f76eca231de5898c803b9067645591fd5c4d3e65aee84`, read +on 2026-09-09 from the producer's build of that file. `Microsimulation()` +digests whatever the default URI resolves to and refuses to construct unless it +matches `policyengine_us.system.DEFAULT_DATASET_SHA256`, so the microsimulation +CI job - which downloads the tag and constructs the default - is where the +documented hash is checked against the bytes the tag actually serves. The +constants are held to this paragraph by +`test_documented_default_build_matches_the_shipped_constants`, which reads the +URI and the hash from here: editing either constant alone fails that test. + The published country wheel requires exactly `spm-calculator==1.0.0`; it contains no Git or local-path dependency. For coordinated development before registry resolution is available, install the local calculator wheel into an isolated diff --git a/policyengine_us/tests/core/test_spm_system.py b/policyengine_us/tests/core/test_spm_system.py index 517f238547b..54c94b2e2e2 100644 --- a/policyengine_us/tests/core/test_spm_system.py +++ b/policyengine_us/tests/core/test_spm_system.py @@ -4,6 +4,8 @@ import hashlib import inspect import json +from pathlib import Path +import re import textwrap import numpy as np @@ -816,6 +818,46 @@ def test_explicit_dataset_argument_is_not_content_checked(decoy_default_dataset) assert simulation.calculate("spm_measurement_adults", 2024).tolist() == [1, 1] +DEFAULT_BUILD_SECTION = "## Default population build" + + +def documented_default_build(): + """The default build's URI and content hash, read from `docs/spm.md`. + + The digest is a bare 64-character constant in `system.py` with no other + occurrence in the repository, and the two tests that exercise the check + substitute their own value for it, so a silent edit to the constant would + pass the suite. `docs/spm.md` carries the value a human can compare with + the producer's receipt and with the bytes the Hugging Face tag serves; + this reads it back so the record and the constant cannot drift apart. + """ + repository = Path(__file__).resolve().parents[3] + if not (repository / "pyproject.toml").exists(): + pytest.skip("Not a source checkout: docs/ ships with the repository only.") + document = (repository / "docs" / "spm.md").read_text(encoding="utf-8") + assert DEFAULT_BUILD_SECTION in document, ( + f"docs/spm.md no longer has a '{DEFAULT_BUILD_SECTION}' section " + "recording the default build's URI and content hash." + ) + section = document.split(DEFAULT_BUILD_SECTION, 1)[1].split("\n## ", 1)[0] + return set(re.findall(r"`(hf://[^`]+)`", section)), set( + re.findall(r"`sha256:([0-9a-f]{64})`", section) + ) + + +def test_documented_default_build_matches_the_shipped_constants(): + """One edited side of the default-build record must fail, not ship.""" + uris, digests = documented_default_build() + assert uris == {DEFAULT_DATASET}, ( + "docs/spm.md documents default dataset URIs " + f"{sorted(uris)}, but the model ships {DEFAULT_DATASET!r}." + ) + assert digests == {DEFAULT_DATASET_SHA256}, ( + f"docs/spm.md documents content hashes {sorted(digests)}, but " + f"DEFAULT_DATASET_SHA256 is {DEFAULT_DATASET_SHA256!r}." + ) + + def test_default_dataset_is_digested_once_per_resolved_file(monkeypatch, tmp_path): """The certified build is ~830 MB; construction must not re-digest it.""" path = tmp_path / "counted.h5" From cd04c2c384bfe8f5763b453a039e8be0aff44863 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 22:01:47 -0400 Subject: [PATCH 16/21] Describe the calculator dependency that actually ships The review's F4. `docs/spm.md` said the published wheel requires exactly `spm-calculator==1.0.0`; `pyproject.toml` requires `>=1.0.0,<=1.0.0.post1`, the range #9466 opened so a pinned development consumer resolves either version, with `uv.lock` pinning 1.0.0. `.github/release-lock.md` said the country lock was intentionally blocked until `spm-calculator==1.0.0` reached PyPI. It reached PyPI on 2026-09-11 and the committed lock has resolved it from `https://pypi.org/simple` with sdist and wheel hashes since; the committed-lock check passes. That passage now says so, and records that the guard validates sources, URLs and hashes and never a version string - `.github/release_lock.py` contains no calculator reference, so nothing there enforces the requirement `pyproject.toml` and review enforce. Co-Authored-By: Claude Fable 5.1 --- .github/release-lock.md | 19 ++++++++++++------- docs/spm.md | 11 ++++++----- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/.github/release-lock.md b/.github/release-lock.md index 4dda3650af8..7a2db0ebf99 100644 --- a/.github/release-lock.md +++ b/.github/release-lock.md @@ -75,10 +75,15 @@ The opt-in probe creates an isolated, minimal project with a standard PyPI dependency and exercises actual uv locking. CI enables it. These commands do not install or import the country model. -For the SPM integration, the committed country lock is intentionally still -blocked until `spm-calculator==1.0.0` is available on PyPI and a production -registry lock is regenerated and reviewed separately. A stale root version or -older calculator resolution must fail the full PR and publication gates. -Passing guard tests does not clear that release prerequisite. Do not use local -wheel links or edit lock fields to manufacture a passing lock; the automatic -root-version refresh is not a general dependency update command. +For the SPM integration, `spm-calculator` 1.0.0 reached PyPI on 2026-09-11, and +the committed country lock resolves it from `https://pypi.org/simple` with sdist +and wheel SHA-256 hashes, so `--committed` passes on the committed files. +`pyproject.toml` requires `>=1.0.0,<=1.0.0.post1` and the lock pins 1.0.0; +moving that pin to the post-release is an ordinary reviewed lock change. This +guard validates dependency sources, artifact URLs and hashes, never a version +string, so the calculator requirement is held by `pyproject.toml` and review +rather than by `release_lock.py`. A stale root version or an older calculator +resolution must still fail the full PR and publication gates, and passing guard +tests clears no other release prerequisite. Do not use local wheel links or edit +lock fields to manufacture a passing lock; the automatic root-version refresh is +not a general dependency update command. diff --git a/docs/spm.md b/docs/spm.md index 9707f9fa7d4..bc5fbe9b5a9 100644 --- a/docs/spm.md +++ b/docs/spm.md @@ -141,8 +141,9 @@ constants are held to this paragraph by `test_documented_default_build_matches_the_shipped_constants`, which reads the URI and the hash from here: editing either constant alone fails that test. -The published country wheel requires exactly `spm-calculator==1.0.0`; it contains -no Git or local-path dependency. For coordinated development before registry -resolution is available, install the local calculator wheel into an isolated -environment and run tests with `uv run --no-sync`. Refresh the registry lockfile -when the calculator release is available to the resolver. +The published country wheel requires `spm-calculator>=1.0.0,<=1.0.0.post1`, a +registry requirement with no Git or local-path dependency. The range admits the +post-release so that a development consumer pinned to either version resolves; +`uv.lock` pins 1.0.0, published to PyPI on 2026-09-11, with its sdist and wheel +hashes. Development against an unpublished calculator installs that wheel into +an isolated environment and runs tests with `uv run --no-sync`. From bfe9cf9de06c751ca3bb19d6c656660300bc41d1 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 22:10:42 -0400 Subject: [PATCH 17/21] Name what the retained pre-2014 CPUC inclusion counts The review's F6. The pre-2014-08-14 list counts `housing_assistance`, which is modelled rather than reported: HUD's payment where the unit is eligible - `receives_housing_assistance | (is_renter & is_income_eligible)` - times a take-up flag whose default is true. 2.0.1 counted the SPM-capped valuation in this list, and the modelled amount is at least that, so countable income rises for years before the decision and a household near the limit can lose CARE or FERA eligibility there. The parameter already flagged the inclusion as unrevalidated without saying which quantity it now counts, and the fragment said the same. Both say it now. Co-Authored-By: Claude Fable 5.1 --- changelog.d/care-housing-income.changed.md | 2 +- .../parameters/gov/states/ca/cpuc/income_sources.yaml | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/changelog.d/care-housing-income.changed.md b/changelog.d/care-housing-income.changed.md index 9b82493ca48..526dc07a1ef 100644 --- a/changelog.d/care-housing-income.changed.md +++ b/changelog.d/care-housing-income.changed.md @@ -1 +1 @@ -Count actual housing assistance, rather than its SPM-capped valuation, in general household benefits and CBO means-tested transfers, and keep the capped valuation inside SPM resources. Household benefits, household net income, marginal tax rates and CBO transfer aggregates therefore rise for every household receiving housing assistance, by the amount the SPM cap withheld, while SPM poverty is unchanged by this part. Exclude housing subsidies from California CARE and FERA countable income from the August 14, 2014 parameter date, which annual calculations first apply in 2015, so countable income falls and CARE/FERA eligibility widens for assisted California households from that year. Preserve the earlier modeled housing inclusion using actual assistance; its historical policy basis was not revalidated. +Count actual housing assistance, rather than its SPM-capped valuation, in general household benefits and CBO means-tested transfers, and keep the capped valuation inside SPM resources. Household benefits, household net income, marginal tax rates and CBO transfer aggregates therefore rise for every household receiving housing assistance, by the amount the SPM cap withheld, while SPM poverty is unchanged by this part. Exclude housing subsidies from California CARE and FERA countable income from the August 14, 2014 parameter date, which annual calculations first apply in 2015, so countable income falls and CARE/FERA eligibility widens for assisted California households from that year. Before that date the model keeps counting housing subsidies, now as the modeled housing assistance amount - HUD's payment where the unit is eligible, times a take-up flag defaulting to true - rather than the smaller SPM-capped valuation, so countable income rises for those years and a household near the limit can lose CARE or FERA eligibility there. That earlier income rule's historical policy basis was not revalidated. diff --git a/policyengine_us/parameters/gov/states/ca/cpuc/income_sources.yaml b/policyengine_us/parameters/gov/states/ca/cpuc/income_sources.yaml index b68cbb486f0..066d595d891 100644 --- a/policyengine_us/parameters/gov/states/ca/cpuc/income_sources.yaml +++ b/policyengine_us/parameters/gov/states/ca/cpuc/income_sources.yaml @@ -41,7 +41,13 @@ description: >- values: # Preserve the earlier model's inclusion before this decision, using the # benefit the household receives rather than the poverty measure's capped - # subsidy. The pre-2014 income rule has not been revalidated here. + # subsidy. That benefit is modelled, not reported: `housing_assistance` is + # HUD's payment where the unit is eligible - `receives_housing_assistance | + # (is_renter & is_income_eligible)` - times a take-up flag defaulting to + # true. It is at least the capped valuation 2.0.1 counted in this list, so + # countable income rises for years before the decision and a household near + # the limit can lose CARE or FERA eligibility in those years. The pre-2014 + # income rule has not been revalidated here. 0000-01-01: # Wages, salaries - employment_income From 4be961eedcef13847629afba5f7ce6edf93dfa4b Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 22:11:12 -0400 Subject: [PATCH 18/21] Record second-pass progress Co-Authored-By: Claude Fable 5.1 --- PROGRESS.md | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/PROGRESS.md b/PROGRESS.md index 57fa0364569..972c970e68a 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -6,18 +6,27 @@ branch is handed back. ## State -Addressing the six findings the review raised, one commit each, then the fast -local suites, the release-lock check, a push and CI. +Six findings, one commit each. F1-F4 and F6 are committed; F5 is written and +waiting on the core suites. ## Done -- Read the review and the code each finding names. +- F1 full-population housing-allocation probe over the certified default build + and over `enhanced_cps_2024`, with the overlap count in the failure message. + No guard added. +- F2 `spm-housing-allocation` and `care-housing-income` retyped `changed` and + rewritten around consequence; `spm-supplied-policy-caches.fixed.md` restored. +- F3 digest and URI recorded in `docs/spm.md` with the date read, pinned by + `test_documented_default_build_matches_the_shipped_constants`. +- F4 `docs/spm.md` and `.github/release-lock.md` now describe the shipped + calculator range and the published 1.0.0 resolution. +- F6 the pre-2014 CPUC inclusion names the modelled quantity it counts, in the + parameter and in the fragment. ## Next -- F1 full-population housing-allocation probe in the microsimulation suite. -- F2 changelog retype and consequence text, plus the restored #9448 fragment. -- F3 documented default-build digest and a test pinning the constant to it. -- F4 docs/`release-lock.md` passages describing what actually ships. -- F5 carried tracer tests and the policy-family docstring correction. -- F6 modeled-versus-capped note in the CPUC fragment and parameter comment. +- F5 commit: #9448's two tracer tests fail against #9463's mechanism and are + not carried; the reason is recorded in the isolation suite, and the + policy-family docstring is corrected. +- Fast suites, CPUC/Tlaib YAML, `release_lock.py --committed`, ruff. +- Push, update the PR body, poll CI. From 4f73fdd376361a73828d3ea9ed5ff00b00ef9d94 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 22:11:53 -0400 Subject: [PATCH 19/21] Record why #9448's two tracer tests are not carried The review's F5. Both were run against this branch's mechanism before deciding: test_cached_formula_trace_binds_replacement_tracer[False] FAILED test_cached_formula_trace_binds_replacement_tracer[True] FAILED test_system_parameter_lookup_does_not_cache_tracer FAILED One root cause, observed directly. Core memoises the `TracingParameterNodeAtInstant` in the root parameter node's `_at_instant_cache`, bound to whichever tracer built it. #9463 isolates a traced request by giving it its own root with an empty at-instant cache, keyed on the `trace` setter; replacing a tracer in place afterwards - a bare `simulation.tracer = FullTracer()`, or a `parameters.tracer` write on a system - reaches none of that, and core's per-formula soft recast in `Simulation._run_formula` rewrites only the root's own `trace`/`tracer` fields, not the memo. So the second tracer records nothing: for the system-lookup case, iteration 0 records one parameter and iteration 1 records zero, with the access going to the first tracer's closed tree. Passing them means rebinding that memo at lookup, which is #9448's mechanism rather than this one, so they are not forced. The boundary is recorded on `test_traced_simulations_record_their_own_parameter_accesses`, which covers the supported path, with the note that the only bare tracer write left in this package is `tools/branched_simulation.py`, which nothing constructs. The policy-family docstring said #9463 reverses child-to-parent propagation, full stop. It reverses it for parameter reforms; a variable-only reform on a shared-policy branch still reaches its parent and siblings, and `test_a_branch_variable_reform_is_not_isolated_from_its_parent` pins that. The correction also records what the dropped cases actually carried: #9448's `test_child_reform_updates_warm_parent_and_sibling` parametrizes a parameter dictionary against a root-replacing `modify_parameters` reform, so those cases are contradicted rather than merely duplicated, and the drop stands. Co-Authored-By: Claude Fable 5.1 --- .../tests/core/test_spm_policy_family.py | 24 ++++++++++++++----- .../core/test_spm_simulation_isolation.py | 15 ++++++++++++ 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/policyengine_us/tests/core/test_spm_policy_family.py b/policyengine_us/tests/core/test_spm_policy_family.py index 0c077a4c1b4..9abbe1750dd 100644 --- a/policyengine_us/tests/core/test_spm_policy_family.py +++ b/policyengine_us/tests/core/test_spm_policy_family.py @@ -3,13 +3,25 @@ These cases come from #9448, which proposed a different isolation mechanism from the one this branch carries (#9463's). Most of that file asserted #9448's own internals, or asserted that a child branch's reform propagates up to its parent -and siblings - which #9463 deliberately reverses, and which +and siblings. + +#9463 reverses that propagation for parameter reforms only. ``test_a_branch_reform_does_not_rewrite_its_parents_policy`` in -``test_spm_simulation_isolation.py`` pins the other way. What is kept here is -the part that is mechanism-independent and that #9463's own suite leaves -uncovered: a nested branch clone's inherited inputs, a structural reform at a -changed start instant, the warm-cache contract for a system supplied by a -caller, and a reform reading back its own in-progress edit. +``test_spm_simulation_isolation.py`` pins the reversal; a variable-only reform on +a shared-policy branch still reaches its parent and siblings, and +``test_a_branch_variable_reform_is_not_isolated_from_its_parent`` pins that +retained behaviour. Every propagation case dropped from #9448 carried a +parameter reform - ``test_child_reform_updates_warm_parent_and_sibling`` +parametrizes a parameter dictionary against a root-replacing +``modify_parameters`` reform, and +``test_parameter_reform_updates_only_intentionally_shared_branches`` is a +parameter reform by name - so each of them is contradicted rather than merely +duplicated. + +What is kept here is the part that is mechanism-independent and that #9463's own +suite leaves uncovered: a nested branch clone's inherited inputs, a structural +reform at a changed start instant, the warm-cache contract for a system supplied +by a caller, and a reform reading back its own in-progress edit. """ import importlib diff --git a/policyengine_us/tests/core/test_spm_simulation_isolation.py b/policyengine_us/tests/core/test_spm_simulation_isolation.py index 9dcedf13de5..c171bf35c1b 100644 --- a/policyengine_us/tests/core/test_spm_simulation_isolation.py +++ b/policyengine_us/tests/core/test_spm_simulation_isolation.py @@ -213,6 +213,21 @@ def test_traced_simulations_record_their_own_parameter_accesses(period): first request's tracer - or, with that request's trace frame closed, recorded nowhere - and the shared tree kept a finished request's tracer for every simulation that read it afterwards. + + The isolation is keyed on ``trace``: setting it re-primes this request's + root with an empty at-instant cache. Replacing a tracer in place afterwards + is not covered - neither a bare ``simulation.tracer = ...`` write nor a + ``parameters.tracer`` write on a system - because core memoises the + ``TracingParameterNodeAtInstant`` in the root's ``_at_instant_cache`` bound + to whichever tracer built it, and core's per-formula soft recast in + ``Simulation._run_formula`` refreshes the root's own ``trace``/``tracer`` + fields without rebuilding that memo. #9448's + ``test_cached_formula_trace_binds_replacement_tracer`` and + ``test_system_parameter_lookup_does_not_cache_tracer`` assert that case; + both fail against this mechanism and are not carried, because passing them + means rebinding the memo at lookup, which is #9448's mechanism rather than + this one. The only bare tracer write left in this package is + ``tools/branched_simulation.py``, which nothing constructs. """ first = Simulation(situation=earner_situation()) first.trace = True From 09251870f529d959fda78874b5810f6243983c7a Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 22:25:17 -0400 Subject: [PATCH 20/21] Record second-pass verification results Co-Authored-By: Claude Fable 5.1 --- PROGRESS.md | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/PROGRESS.md b/PROGRESS.md index 972c970e68a..74e3062a7cf 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -6,27 +6,27 @@ branch is handed back. ## State -Six findings, one commit each. F1-F4 and F6 are committed; F5 is written and -waiting on the core suites. +All six findings committed, one commit each. Local verification green. ## Done - F1 full-population housing-allocation probe over the certified default build - and over `enhanced_cps_2024`, with the overlap count in the failure message. - No guard added. + and over `enhanced_cps_2024`, count in the failure message. No guard added. - F2 `spm-housing-allocation` and `care-housing-income` retyped `changed` and rewritten around consequence; `spm-supplied-policy-caches.fixed.md` restored. - F3 digest and URI recorded in `docs/spm.md` with the date read, pinned by `test_documented_default_build_matches_the_shipped_constants`. -- F4 `docs/spm.md` and `.github/release-lock.md` now describe the shipped +- F4 `docs/spm.md` and `.github/release-lock.md` describe the shipped calculator range and the published 1.0.0 resolution. -- F6 the pre-2014 CPUC inclusion names the modelled quantity it counts, in the - parameter and in the fragment. +- F5 #9448's two tracer tests fail against #9463's mechanism and are not + carried; the reason is recorded, and the policy-family docstring corrected. +- F6 the pre-2014 CPUC inclusion names the modelled quantity it counts. + +Verification: `ruff format --check .` exit 0, `ruff check .` exit 0, +`pytest tests/core tests/unit tests/code_health` 535 passed, +CPUC + Tlaib YAML 58 passed exit 0, `release_lock.py --committed` exit 0. ## Next -- F5 commit: #9448's two tracer tests fail against #9463's mechanism and are - not carried; the reason is recorded in the isolation suite, and the - policy-family docstring is corrected. -- Fast suites, CPUC/Tlaib YAML, `release_lock.py --committed`, ruff. -- Push, update the PR body, poll CI. +- Push, update the PR body with a second-pass section, poll CI. +- Drop this log before handing the branch back. From 6c9d4ab569be5e10a1c47caa357e729177f37c12 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 14 Sep 2026 23:17:25 -0400 Subject: [PATCH 21/21] Keep the whole-population housing probe inside the hosted runner's memory The Full Suite microsimulation job was killed twice at this test: the full-population allocation and measurement-adult arrays are the fact the probe exists to establish, but computing the whole SPM resource chain on top of them, in a process that has already loaded three other populations, exceeds the runner. The count stays full-population; the chain-completion check now runs on a 10,000-unit subsample after the arrays are released. Co-Authored-By: Claude Fable 5.1 --- .../tests/microsimulation/test_microsim.py | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/policyengine_us/tests/microsimulation/test_microsim.py b/policyengine_us/tests/microsimulation/test_microsim.py index aeb05c305ad..7c7470a2198 100644 --- a/policyengine_us/tests/microsimulation/test_microsim.py +++ b/policyengine_us/tests/microsimulation/test_microsim.py @@ -162,6 +162,8 @@ def test_default_dataset_allocates_no_housing_share_to_an_unclassified_unit(): among units that classify an adult and leave the remainder with the awarded unit - and that decision belongs to a reviewer, not to this test. """ + import gc + import numpy as np from spm_calculator.errors import SPMInputError @@ -180,16 +182,27 @@ def test_default_dataset_allocates_no_housing_share_to_an_unclassified_unit(): "raises SPM_COMPOSITION_REQUIRED over the whole population." ) + # The whole-population resource chain does not fit the hosted runner's + # memory beside the arrays above (the job was killed twice at this point), + # so the completion check runs on a subsample. The count above is the + # full-population fact; this only shows the chain still completes. + del allocated, adults, simulation + gc.collect() + sample = Microsimulation() + sample.subsample(10_000) try: - net_income = np.asarray(simulation.calculate("spm_unit_net_income", 2024)) + net_income = np.asarray(sample.calculate("spm_unit_net_income", 2024)) except SPMInputError as error: pytest.fail( - "populace_us_2024 (certified default build): the SPM resource " - f"chain raised {error.code} even though {unclassified} allocated " - f"units classify no measurement adult, so the housing allocation " - f"is not what failed: {error}" + "populace_us_2024 (certified default build, 10,000-unit subsample): " + f"the SPM resource chain raised {error.code} even though " + f"{unclassified} allocated units classify no measurement adult, so " + f"the housing allocation is not what failed: {error}" ) - assert net_income.size == allocated.size, ( + assert ( + net_income.size + == np.asarray(sample.calculate("spm_unit_allocated_housing_subsidy", 2024)).size + ), ( "populace_us_2024 (certified default build): net income and the " "housing allocation must cover the same SPM units." )