Repository navigation
Read stored county and has_tin inputs with the branch's name - #9748
Merged
Merged
Conversation
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 Report❌ Patch coverage is
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…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
marked this pull request as ready for review
October 7, 2026 12:03
Contributor
Author
|
US + core hub merge audit, us#9748 at
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_tintakes precedence over legacyhas_itin, then defaultTrue.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
make formatand lint pass; changelog fragment retained; partner expectations preserved.8ecffe6d29: 51 cases pass on each requested core (3.32.8 and exact2e22c8f8): 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 (Trueversus expected branch/nestedFalse). The same test file runs green before the harness installs main’s formula; no holder or input behavior is mocked.Noneguards partially covered).2e22c8f8016a4873bfc1e1257ee408a5ecc37ba8matches all 194 tracked package files by Git blob hash (0 missing/changed); noc574WIP substitution.7aa7609340: 73 passes on locked core 3.32.8; 52 passes on core master757147c7. Saved canonical coverage executeshas_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
3266705e42versus the prior PR worktree at7aa7609340, with core757147c7and pinned default datasetpopulace_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 containsUNKNOWN. 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
UNKNOWNcounties, mapping mismatches, or Maryland CCS rate errors. Baseline had eight erroneous reads on 3.32.8 and 20 on757147c7; each affected 57,240 households including 1,091 in Maryland. A branch-only legacyFalseinput madehas_tin=Falsefor all people in the branch and nested branch while default and sibling remainedTrue.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_taxandhousehold_net_incomeare 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. From7aa7609340to 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.