[MaxGhenis]

PolicyEngine/policyengine-core#552 adds an optional branch argument to Holder.get_known_periods, and core's _calculate passes the simulation's branch when it looks for fallback periods. PE-US wraps that method in policyengine_us/tools/social_security_inputs.py. Main's wrapper takes no branch, so under core #552 it raises a TypeError and stops root fallback calculations.

The wrapper now forwards an explicit branch to the physical method, _allow_fallback, _public_periods and _effective_value. A call without a branch makes the same calls as before, so the wrapper keeps working with core's one-argument method. PE-US main needs this change before core releases #552. #10671 carries the separate subsampling assertion change.

test_dataset_rebuild_keeps_new_input_provenance_when_old_cache_shares_key now stores the derived 2023 entry that the rebuilt dataset reuses, with put_in_cache(..., derived=True), and keeps its assertions. Core #552 can leave a missing input's default uncached, so the test no longer relies on the calculation to store that entry.

impact: none on current core; only branch-aware callers reach the changed path

axiom: n/a: core compatibility infrastructure; no policy change

Tests

Local runs at de480911d0, one capped process per file:

File	Core master 7786d3dca (3.34.8)	Core #552 6ed1b40 merged into that master	Installed core 3.33.2
test_social_security_input_state_machine.py	1 passed	1 passed	1 passed
test_social_security_known_periods.py	9 passed, 5 skipped	14 passed	not run
test_social_security_real_core.py	120 passed	120 passed	120 passed

The five skips on core master come from the branch cases, which read the holder method's signature and skip when it takes no branch.

Mutants, each run alone at 626fcc7fab under a 4 GB and 1,800-second cap:

Mutant	Core	Result
Always forward the branch argument	master	9 failed, 5 skipped; test_known_periods_without_branch_keeps_reported_sources[social_security_retirement] kills it
Omit the branch from _allow_fallback	#552	1 failed, 13 passed; test_known_periods_forwards_branch_to_fallback_check kills it
Omit the branch from _effective_value	#552	4 failed, 10 passed; every component of test_known_periods_lists_only_sources_readable_by_requested_branch kills it

The outer fallback-permission call ignores the branch when it gets no period, so a call-through spy checks that the wrapper forwards it.

🤖 Generated with Claude Code

[DTrim99]

PR Review: Forward branches through Social Security period discovery

Summary

PolicyEngine-core PR #552 (open, head 6ed1b40, not in any release) adds an optional branch_name to Holder.get_known_periods. Its Simulation._calculate then calls holder.get_known_periods(self.branch_name) positionally on the no-formula fallback path, and _dump_holder does the same. PE-US replaces Holder.get_known_periods for every variable with a one-argument wrapper in policyengine_us/tools/social_security_inputs.py. Without this PR, every input default or carry-over calculation under #552 would raise TypeError: get_known_periods() takes 1 positional argument but 2 were given.

The PR gives the wrapper branch_name=None and forwards a branch only when one is passed: to the physical method, _allow_fallback, _public_periods and _effective_value. A call without a branch makes exactly main's calls. No core release through 3.34.9 and no PE-US caller passes a branch, so nothing changes on any supported core today. The test-side changes are:

• the state-machine backend now loads Core source from the imported module;
• it runs Add guaranteed income / cash assistance variable #552's three new fallback-cache helpers when they exist;
• a new country-free test file covers known periods with and without a branch;
• one provenance test now stores, itself, the derived entry that Add guaranteed income / cash assistance variable #552 no longer caches.

I found no correctness problems. The new forwarding passes a local run against core #552. The remaining notes are suggestions.

Critical (must fix)

None.

Should address

None.

Suggestions

1. The branch cases sleep in CI until core Add guaranteed income / cash assistance variable #552 ships (policyengine_us/tests/core/test_social_security_known_periods.py:14-18). The fixture branch_aware_core skips 5 of the 14 cases whenever the physical method takes no branch. The PR's CI run therefore reports 9 passed and 5 skipped. Today, forwarding the branch to _public_periods, _effective_value and _allow_fallback is pinned only on a branch-aware Core. That is acceptable, since no supported Core can reach the path, and the cases do kill those mutants on Add guaranteed income / cash assistance variable #552 (see Verified). Once Add guaranteed income / cash assistance variable #552 is released, raise the floor in pyproject.toml (policyengine-core>=<that release>) and delete the skip, so these cases cannot stay skipped unnoticed. To pin the forwarding in CI before then, monkeypatch hooks._PHYSICAL["get_known_periods"] with a branch-aware stand-in that filters holder.get_known_branch_periods() by holder._readable_branches(branch_name), then run the readable-branch case against it.
2. Forwarding to _allow_fallback has no effect (social_security_inputs.py:899-901). _allow_fallback reads branch_name only when it gets a period (social_security_inputs.py:176-180). With period=None it returns the request-context value or True. test_known_periods_forwards_branch_to_fallback_check therefore pins the call's shape, not behavior. The PR body says this, but the code doesn't. A one-line comment would stop a later reader from treating the spy as behavioral coverage, or from deleting the argument as dead. The comment can note that the argument mirrors get_input_periods (line 931) and stays correct if _allow_fallback starts to use the branch.
3. Two setup assertions now check the fixture's own write (test_social_security_real_core.py:446-450). The explicit old_holder.put_in_cache(np.array([0.0]), period("2023"), derived=True) is a reasonable way to keep the test independent of Add guaranteed income / cash assistance variable #552's uncached root fallback. After it, assert old_holder.is_derived(period("2023")) and sequence_number(...) >= since verify the write just made, not Core's caching. The assertions after the rebuild, which are what the test is about, are unchanged. Consider dropping the two setup assertions or saying in the comment that they now only confirm the setup.
4. Skip reason wording (test_social_security_known_periods.py:18). "Core master Holder.get_known_periods accepts no branch argument" will be wrong once Add guaranteed income / cash assistance variable #552 is on master but not in the installed release. "Installed Core's Holder.get_known_periods accepts no branch argument" would stay accurate.

Verified correct

Core API and the wrapper on old and new Core

• Add guaranteed income / cash assistance variable #552's API. In Add guaranteed income / cash assistance variable #552, holder.py reads def get_known_periods(self, branch_name: str = None). With a branch, it lists periods from get_known_branch_periods() whose branch is in _readable_branches(branch_name). Without one, it keeps today's listing of every branch. simulation.py changes holder.get_known_periods() to holder.get_known_periods(self.branch_name), and simulation_dumper._dump_holder loops over holder.get_known_periods(branch_name). Both calls are positional. The wrapper's (holder, branch_name=None) accepts them, and its keyword name matches the physical parameter.
• Old Core. Core master is now 3.34.9, after Remove fica parameter layer #652 and Add parameter for $400 minimum self-employment net earnings to pay self-employment tax #653. It and the installed 3.33.12 still define get_known_periods(self) and call it with no argument everywhere: at simulation.py lines 894, 1754, 1835, 2593, 3117, 3141 and 3239 on master, and in the dumper. The pin is policyengine-core>=3.33.1. The lock has 3.34.2, and the Python jobs run uv lock --upgrade-package policyengine-core.
• PE-US callers. Every PE-US caller calls with no argument. That covers the following, plus tests:
  • system.py (four sites), spm.py:1079, tools/cloning.py:8 and tools/branched_simulation.py:9;
  • has_tin.py, county.py and foreign_earned_income.py.
• No-branch path. On supported Cores the wrapper always takes branch_kwargs = {} and makes the same four calls as main. The "always forward" mutant passes branch_name=None to the old physical method, raises TypeError, and is killed on old Core by test_known_periods_without_branch_keeps_reported_sources.

Default and reform branches under #552

• Core passes the branch of the holder's own simulation. _branches(sim, sim.branch_name) then returns sim._get_visible_branch_names(), which is identical to Holder._readable_branches(sim.branch_name) in Core: the branch, its parent_branch ancestors, then default.
• The virtual source periods the wrapper adds for *_reported holders are therefore the same set main adds today, on the root and on a reform branch alike. Only Core's physical half is narrowed, which is Add guaranteed income / cash assistance variable #552's intended change.
• For a branch other than the holder's own, _readable_branches(X) is the same filter Add guaranteed income / cash assistance variable #552's physical method applies, so the physical and virtual halves agree.

Memory and disk storage

• _physical_periods and _physical_value read both the memory and disk tiers. Add guaranteed income / cash assistance variable #552's physical method uses get_known_branch_periods() across both tiers.
• A virtual period with no physical array is skipped by Add guaranteed income / cash assistance variable #552's _dump_holder (stored_on is None: continue), matching the existing PE-US dump wrapper's handling of virtual sources.

Hand traces of the new expectations (fixture index 1/2/4/8 for 2023–2026; public 2024 inputs land on the reserved source branch at build, or on later through a later set_input)

Call	Expected	Why
get_known_periods(), no branch	{2024, 2026}	Physical gives reported's own 2026. The virtual half adds 2024 because no explicit reported period starts on or before 2024, so the fallback is allowed and public 2024 is found on __social_security_source__default.
get_known_periods(None)	{2024, 2026}	Same path as the no-branch call.
get_array(2024, "later")[0]	12,000	Branches are later and default. There is no explicit reported value at 2024 and the fallback is allowed (2026 does not start on or before 2024). Public 2024 is found on later, one of the four source branches.
get_array(2024, "default")	None	The source branches are only default and its reserved twin, so public 2024 on later is invisible. The physical read is empty.
get_known_periods("later")	{2024, 2026}	Physical gives 2026. Virtual gives 2024 through the source branches of later.
get_known_periods("default")	[]	The physical filter is default only, and the virtual half has no visible source.
get_known_periods("unrelated")	[]	The readable branches are unrelated and default, and neither stores a value.
Spy, get_known_periods("later") on a fresh simulation	[], with branches == ["later"]	The only call with no period is the wrapper's own. With no public periods there are no _effective_value calls.
calculate("<component>_reported", 2024) with no inputs	0	The uprated default with no source.
calculate("independent_input", 2024) after delete_arrays	0	The carry-over default. These two cases are the direct regression tests for the TypeError: main's one-argument wrapper fails both on #552.

Mutants

• Dropping **branch_kwargs from _public_periods empties the virtual half for later, because the public value is stored only on later. That fails the readable-branch case.
• Dropping it from the physical call lists 2026 for default, which fails the same case.
• The author's table covers _effective_value, _allow_fallback and "always forward".

Local run against core #552 head 6ed1b40. I put #552's Core source first on the import path and confirmed the signature is (self, branch_name: str = None). test_social_security_known_periods.py and test_social_security_real_core.py gave 134 passed in one process (14 + 120), with no skips. So the branch cases and the reworked provenance test both pass on the new API. I did not run the state-machine test on #552 alone, because #552's head is 27 commits behind Core master; the author reports it passing on #552 merged into 3.34.8 master.

State backend

• Path(core_simulation.__file__).resolve().parents[1] is the policyengine_core package directory of the module actually imported. It replaces distribution("policyengine-core").locate_file(...), which points at the installed distribution even when a Core checkout is first on the import path. The removed importlib.metadata import has no other use.
• _set_fallback_cache_status, _fallback_is_uncached and _uncached_fallback exist only in Add guaranteed income / cash assistance variable #552, which calls them from _calculate, calculate_add and _cache_option_result. The hasattr gate leaves older Cores untouched.

Overlap with #10684. #10684 merged on 2026-11-06 as d26686105a and is in this PR's merge base. It added _bind_to_tax_benefit_system to the backend's sim_methods for Core 3.34.8's apply_reform. This PR appends its own hasattr-gated names after that list: it builds on #10684 and neither duplicates nor reverts it. #10671 (open), which the PR body names as the separate subsampling change, touches only test_spm_system.py.

Test design and CI cost

• The new file uses the country-free tiny TaxBenefitSystem from test_social_security_real_core.py, with no country model, dataset or reform tree. Python is justified, since this is Core-integration behavior that YAML cannot reach.
• CI timings from run 42317487869:
  • test_social_security_known_periods.py: 14 cases, 0.04 s of case time, in the existing Rest core group (chunk 1: 23:53 wall, 9.0 GiB peak RSS for the whole group);
  • test_social_security_input_state_machine.py: 5.67 s, in chunk 2;
  • test_social_security_real_core.py: 120 cases, 0.29 s, in chunk 2.
• No job, matrix entry or group was added. The new file falls in core/ by path, so the group-coverage guard picks it up.
• The bare import test_social_security_real_core relies on pytest's default prepend import mode (tests/core has no __init__.py). It worked in CI even though the two files ran in different chunks.

Changelog. changelog.d/known-periods-branch-wrapper.fixed.md is a top-level fragment with a sensible type.

Axiom line assessment

axiom: n/a: core compatibility infrastructure; no policy change is valid. The PR changes only a Core-API compatibility wrapper and tests. No formula, parameter or household result changes on any released Core, so there is no provision to encode in rulespec-us.

CI status

All green: 36 pass and 3 skipping. The skips are the workflow gates of an earlier superseded run (42314572037). Run 42317487869 at head de480911d0 started 2026-11-07 00:19 UTC and installed the latest compatible Core then, 3.34.8. Core 3.34.9 (#653, "Keep holders on their system's current definitions") was released at 12:15 UTC, after that run. It touches holder binding, not get_known_periods. A re-run before merge would confirm the state-machine backend against it. This is a note, not a finding.

Branch status

The branch is 4 commits ahead of and 8 behind main. git merge-tree --write-tree PolicyEngine/main de480911d0 merges cleanly (tree 9cc64d3e84). Main's 8 commits since the merge base touch none of this PR's files: they cover AR/MS/NJ/IA/PA income rules, two unrelated tests/core invariant files, and the version bump in pyproject.toml/uv.lock. Among the 100 most recent open PRs, no other touches social_security_inputs.py or these Social Security Core tests.

Verdict: APPROVE

[DTrim99]

Approving per the review above: 0 critical, 0 should, 4 optional suggestions.

[MaxGhenis]

US + core hub merge audit, us#10676 at de480911d02a656d912b2973b00d5319ca602be4 (squash): Forward branches through Social Security period discovery.

• Reviews: a GPT-6.1 Sol reviewer approved de480911d0 in round 1 with no P1 or P2 findings.
  • It traced every path from core's _calculate on master and on Keep branch period reads, dumps, and disk deletion consistent policyengine-core#552. A call without a branch makes the same calls as main's wrapper, and an explicit branch reaches the physical method, _allow_fallback, _public_periods and _effective_value.
  • It found that the provenance test change hides no production regression, and that all 14 new cases assert what they state.
• Follow-ups for the core Add guaranteed income / cash assistance variable #552 gate:
  • The review's one P3 asks for a stronger fixture in test_social_security_real_core.py. The test would supply the 2022 input at construction instead of inserting the cache entry by hand, then check that the 2023 carry-over reaches the pre-rebuild snapshot.
  • It also proposes two mutants that only core Add guaranteed income / cash assistance variable #552 can run.
  • Both run with the PE-US gate for core Add guaranteed income / cash assistance variable #552, before that PR merges.
• Gates checked live at merge:
  • gh pr checks exits 0;
  • the latest Pull request run at the head concluded success (42317487869, 36 of 36 jobs);
  • MERGEABLE;
  • not a draft;
  • no reviewer's latest review requests changes;
  • the semantic-overlap guard;
  • the merge pins the head.
• Impact: none on current core. Core's master never passes a branch to get_known_periods, so only core Add guaranteed income / cash assistance variable #552 reaches the changed path.
• Axiom: n/a, since the PR changes core compatibility code and tests, not policy.

🤖 Generated with Claude Code