Skip to content

Read stored county and has_tin inputs with the branch's name - #9748

Merged
MaxGhenis merged 8 commits into
mainfrom
fix-county-branch-carry-over
Oct 7, 2026
Merged

MaxGhenis merged 8 commits into
mainfrom
fix-county-branch-carry-over

Conversation

@MaxGhenis

@MaxGhenis MaxGhenis commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

What and why

Read stored county and TIN inputs using the requesting simulation's branch name. County lookup tries readable periods from latest start date to earliest, resolves branch/ancestor/default storage, skips sibling-only periods, and falls through to FIPS mapping when none is readable. Canonical has_tin takes precedence over legacy has_itin, then default True.

The county carry-over assumes dataset households are geographically static, preserving existing behavior even when the readable input is from a later year. Annual moves would require a separate data-model decision.

This round preserves the existing 40 county and three legacy-TIN cases, the two canonical direct-formula branch/nested cases, the YAML seed, and the two property tests committed during the previous attempt. It merges current main without conflicts, retaining #9738/#9741 and the complementary #9751 SPM county-input fix. The inherited county dataset-path coverage exclusion is removed so changed reads are measured directly.

Test plan

  • Policy YAML expectations from legal sources: not applicable; branch/cache input plumbing changes no policy rule.
  • make format and lint pass; changelog fragment retained; partner expectations preserved.
  • Current head 8ecffe6d29: 51 cases pass on each requested core (3.32.8 and exact 2e22c8f8): TIN 6, county branch/order 41, existing FIPS fallback 4. The two canonical direct-formula cases fail against current main’s exact formula on both cores (True versus expected branch/nested False). The same test file runs green before the harness installs main’s formula; no holder or input behavior is mocked.
  • All 65 source statements and 15 changed executable lines are covered on both cores, with zero exclusions. County statement/branch coverage is 100%; TIN statement coverage is 100% (whole-file combined coverage 93%, with two unchanged None guards partially covered).
  • The actual selective runner passes six TIN cases. Current main intentionally disables source coverage when its mapped directory targets are deferred, even if an explicit Python test exercises that source. Direct single-file coverage commands above establish coverage independently; the runner was preserved.
  • Exported core 2e22c8f8016a4873bfc1e1257ee408a5ecc37ba8 matches all 194 tracked package files by Git blob hash (0 missing/changed); no c574 WIP substitution.
  • Recorded CI snapshot: all 35 checks pass, none pending or failing. Full suites are left to CI.
  • Reused saved checks at 7aa7609340: 73 passes on locked core 3.32.8; 52 passes on core master 757147c7. Saved canonical coverage executes has_tin.py:21. The prior main-side commands failed to locate copied tests and are not counted as regression failures.

Impact evidence and remaining runs

The completed saved fresh 2025 comparison uses main 3266705e42 versus the prior PR worktree at 7aa7609340, with core 757147c7 and pinned default dataset populace_us_2024.h5@populace-us-2024-spm-20260909. All 15 headline arrays and all 207,842 cached arrays across nine simulations are bitwise identical; numeric weighted deltas are exactly zero. No cached county array contains UNKNOWN. This is saved evidence, not a new measurement of the updated head.

Completed saved reused/nested probes covered both 2025→2026→2027 and reverse, 57,240 households and 166,321 people, and 66 reads per run on each core. The PR had zero erroneous UNKNOWN counties, mapping mismatches, or Maryland CCS rate errors. Baseline had eight erroneous reads on 3.32.8 and 20 on 757147c7; each affected 57,240 households including 1,091 in Maryland. A branch-only legacy False input made has_tin=False for all people in the branch and nested branch while default and sibling remained True.

Fresh hub runs at the updated head (2026-10-07): this PR's merge base against 8ecffe6d29, on the default dataset, for 2025 and 2026. They ran under the shared heavy lock, one run per side, compared record by record. has_tin, eitc, ctc, refundable_ctc, income_tax, state_income_tax and household_net_income are identical on every record in both years, which confirms the expected zero ordinary single-year delta at the exact head.

These runs don't exercise the reused and nested-branch cache paths the PR fixes. Those remain covered by the saved probes above, which were run on the prior PR worktree. (The earlier attempt's fresh runs were killed by the memory watchdog or a full disk.)

Status (hub, after independent review APPROVE at 8ecffe6d29): the fresh exact-head 2025/2026 comparisons above are complete, with zero ordinary single-year delta. The saved nested-branch probes still apply. From 7aa7609340 to this head, county code changed only by removing a coverage exclusion and a comment; the TIN code, the Python regressions, the geographic and branch helpers, data loading, SPM and Maryland CCS dependencies are all unchanged. The probe uses .calculate() and ordinary branches, which the intervening core change (method aliases and cloned-system entities) does not touch. No further runs are required.

Ready for review: the impact checks are complete.

axiom: n/a: branch/cache plumbing for reading stored inputs; no policy rule changes

Detailed resume report and saved run evidence: 9748 report.

The county formula's over-dataset path read the latest known period with
no branch name, so it saw only the default branch's arrays. A branch reused
for a later year (itemizing, not_itemizing) holds its own value for the
previous year, the read found nothing, and the formula returned None.
Core then fell back to its own carry-over: core master carried the latest
calculated value (or UNKNOWN when the branch had calculated a later year
first); input-only carry-over (policyengine-core#562) gave UNKNOWN, which
raised ParameterNotFoundError for Maryland child care rates in a
2025-2027 enhanced CPS run.

Read each known period with the simulation's branch name, latest start
first, and fall through to the county_fips mapping when none is readable.
has_tin's has_tin/has_itin input reads now pass the branch name too.

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

codecov Bot commented Oct 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 72.13%. Comparing base (909176a) to head (1419cb8).
⚠️ Report is 120 commits behind head on main.

Files with missing lines Patch % Lines
.../variables/household/demographic/person/has_tin.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              main    #9748       +/-   ##
============================================
- Coverage   100.00%   72.13%   -27.87%     
============================================
  Files            4        2        -2     
  Lines           76       61       -15     
  Branches         2        9        +7     
============================================
- Hits            76       44       -32     
- Misses           0       13       +13     
- Partials         0        4        +4     
Flag Coverage Δ
unittests 72.13% <90.90%> (-27.87%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

MaxGhenis and others added 7 commits October 6, 2026 09:22
…ties

The canonical has_tin read in a branch (has_tin.py line 21) had no test:
calculate returns a has_tin input the branch can read before the formula
runs. Call the formula itself on a branch and a nested branch whose
canonical input differs from the inherited legacy one, and on the
simulation and an unrelated sibling.

Add Hypothesis property tests for the read contract (canonical over
legacy over True, each read along the asker's branch chain, siblings
invisible) and for county on randomly grown branch trees and request
orders, plus the canonical-precedence YAML seed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaxGhenis
MaxGhenis marked this pull request as ready for review October 7, 2026 12:03
@MaxGhenis
MaxGhenis merged commit 5945ea4 into main Oct 7, 2026
35 checks passed
@MaxGhenis

Copy link
Copy Markdown
Contributor Author

US + core hub merge audit, us#9748 at 8ecffe6d2980ef54c07ac957c44e5359b5860a73 (squash)

  • Review: independent review (Codex, GPT-6.1 Sol) APPROVE at 8ecffe6, with no P1 or P2.
    • Its METHODOLOGY flag is main's existing, documented assumption that dataset households have static geography. It is retained, not introduced.
    • The P3 (stale status text) is fixed in the body; the head is unchanged.
  • Gates checked live at merge:
    • gh pr checks exits 0;
    • MERGEABLE;
    • not a draft;
    • no CHANGES_REQUESTED review;
    • head pinned with --match-head-commit.
  • Impact: fresh hub microsims at this head, 2025 and 2026, on the default dataset. has_tin, EITC, CTC, refundable CTC, income tax, state income tax and household net income are identical on every record.
    • The PR fixes stored-input lookup in branches and clones: UNKNOWN counties in nested branches, and a legacy has_tin input.
    • The reviewer confirmed the saved nested probes apply at this head.
  • --admin: used only because a required approving review is the sole block.

@MaxGhenis
MaxGhenis deleted the fix-county-branch-carry-over branch October 7, 2026 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant