Skip to content

Add BOOST composition test for the Head Start net income switch - #9324

Open
MaxGhenis wants to merge 1 commit into
mainfrom
boost-head-start-composition-test
Open

Add BOOST composition test for the Head Start net income switch#9324
MaxGhenis wants to merge 1 commit into
mainfrom
boost-head-start-composition-test

Conversation

@MaxGhenis

Copy link
Copy Markdown
Contributor

Follow-up to #9259, which added reform-composition coverage for ECPA but not BOOST (flagged as a coverage asymmetry in the merge review).

Adds tests/policy/reform/boost_head_start_benefits_composition.yaml: BOOST reform active (gov.contrib.harris.lift.middle_class_tax_credit.in_effect: true, administered_through_ssa: false so household_benefits is the static list alone), include_head_start_benefits_in_net_income: true, head_start: 8_000 + early_head_start: 21_000, every other list member zeroed → household_head_start_benefits: 29_000 and household_benefits: 29_000. Without the wrapper entry in BOOST's static list the aggregate would read 0, so the case is non-vacuous.

Placed under tests/policy/reform (one subprocess per file) for the same reason as the ECPA case: the congress structural batch runs each proposal directory in one process at its memory ceiling, and an extra reformed-system variant stalls that job.

Test-only; changed fragment. Passes locally (1 passed).

🤖 Generated with Claude Code

Pins that head_start/early_head_start values flow through BOOST's static
household_benefits list when
gov.simulation.include_head_start_benefits_in_net_income is on — the
partner to the ECPA composition case from #9259, which left BOOST
uncovered. Lives under tests/policy/reform (per-file subprocess) so it
does not add a reformed-system variant to the memory-bound congress
batch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@PavelMakarchuk PavelMakarchuk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Program Review

PR 9324 — "Add BOOST composition test for the Head Start net income switch" (MaxGhenis)

Source Documents

  • PDF: None — test-only PR, no source documents in scope (PDF audit not applicable)
  • Year: 2023 (test period)
  • Scope: PR changes only
  • Reviewed head SHA: 6e9dc5a
  • Mode: full

Scope note. This PR changes zero parameter YAML files and zero variable Python files. The
entire functional content is one new YAML test plus a changelog fragment (and an incidental
uv.lock delta, see A4). Because no behavior-bearing file and no reference: field is touched,
the regulatory-reviewer and reference-checker roles were correctly SKIPPED per the workflow's
scope rules, and there was no PDF phase — not because those checks passed, but because they had
nothing in scope. The verification queue was resolved as NONE: no value in this diff can be
cross-referenced or externally checked. Remaining review value is test correctness/non-vacuity,
placement convention, and lockfile hygiene, which is what the code and test roles audited.

Branch Status

The branch is BEHIND=17, AHEAD=1 against main (merge base
350ecc766e4467dc05093d197a3404b53e0c9e4d). A rebase onto current main is recommended before
merge — chiefly because the uv.lock delta discussed in A4 is the most likely source of a merge
conflict at this staleness. Staleness did not affect any finding below: every finding was
derived from the PR head snapshot, and all 31 CI checks pass at that head. Informational only, not
a finding.

Critical (Must Fix)

None.

Explicitly confirmed clean on the critical checklist:

  • No hard-coded legal values (no .py changed).
  • No missing/incorrect references (no reference: fields, none required).
  • No CI failures — 31/31 checks pass.
  • No non-functional test: both outputs are currency, there is no boolean output carrying a
    numeric absolute_error_margin.
  • The test is not vacuous. Both roles independently established this — the tests role by hand
    re-derivation (head_start 8,000 + early_head_start 21,000 = 29,000 through
    parameters/gov/household/household_head_start_benefits.yaml), and the code role empirically:
    flipping gov.simulation.include_head_start_benefits_in_net_income to false makes the case
    fail (household_head_start_benefits@2023: [0.] differs from 29000.0), and removing
    household_head_start_benefits from BOOST's static BENEFITS list
    (policyengine_us/reforms/congress/tlaib/boost/boost_middle_class_tax_credit.py:77) also makes
    it fail.

Should Address

A1. The aggregate assertion is degenerate: the case passes unchanged with the reform removed, and cannot detect a drop of any other list entry

policyengine_us/tests/policy/reform/boost_head_start_benefits_composition.yaml:12 (the reforms:
key) and :42-43 (the two outputs).

Two independent observations converge on one root cause. The code role deleted the reforms: line
and re-ran the case: it still passes. The reason is that the baseline
gov.household.household_benefits list parameter also contains
household_head_start_benefits, and the two baseline-only members that BOOST's hard-coded list
omits (ak_energy_relief, household_health_benefits) both default to 0 — so the reformed and the
baseline formulas return the identical 29,000 for this household. Separately, the tests role noted
that the two assertions (household_head_start_benefits: 29_000 and household_benefits: 29_000)
are the same number, because the other 20 components are all pinned to 0.

Stated plainly, what the case does and does not pin:

Injected regression Caught?
include_head_start_benefits_in_net_income regresses → wrapper returns 0 yes (verified empirically)
household_head_start_benefits removed from BOOST's BENEFITS list (boost_middle_class_tax_credit.py:77) yes (→ 0)
that entry duplicated in the list yes (→ 58,000)
the switch is ignored and Head Start is always included no — with the switch on, 29,000 either way
snap, tanf, household_state_benefits, or any of the other 19 entries dropped from the list no
self.update_variable(household_benefits) deleted from the reform (boost_middle_class_tax_credit.py:154), or the reform failing to apply at all no — baseline silently returns the same 29,000

Jointly this means: the case is a genuine, non-vacuous guard on the switch and on this one
entry's membership
in BOOST's list, but the BOOST-specific composition it is named for is
unpinned
— nothing in the file proves the reformed household_benefits is the formula being
evaluated, or that it is a sum rather than an alias. This is a design gap, not a false assertion;
the values asserted are correct.

Cheap fix (both roles proposed the same shape independently, and existing house precedent uses it):
append a second case, identical but with
gov.contrib.congress.tlaib.boost.middle_class_tax_credit.administered_through_ssa: true and
household_benefits: 32_400 — the code role measured 32,400 on the snapshot (29,000 static list +
3,400 BOOST credit for a 30-year-old single filer at AGI 0 in 2023). Baseline returns 29,000 there,
so that case fails the moment the reformed formula stops being the one evaluated, and it also
covers the second branch of the administered_through_ssa fork. Alternatively (or additionally),
set one more component non-zero (e.g. snap: 500household_benefits: 29_500) so the two
outputs differ and the assertion proves an actual sum. The existing
policyengine_us/tests/policy/contrib/congress/tlaib/boost/boost_middle_class_tax_credit.yaml
already uses both the false/true pairing and the distinct-summands pattern (:151-180).

A2. No switch-off counter-case under the reformed system

policyengine_us/tests/policy/reform/boost_head_start_benefits_composition.yaml:1-43 (single case).

The file pins only the switch's "on" arm. The "off" arm is pinned in
policyengine_us/tests/policy/baseline/household/income/household/household_head_start_benefits.yaml:1-11
(Case 1) — but only under the baseline system. Under the BOOST reformed system, nothing pins
it, which is the specific regression row "switch ignored, Head Start always included" in the A1
table. Adding a mirrored case with
gov.simulation.include_head_start_benefits_in_net_income: false and outputs
household_head_start_benefits: 0, household_benefits: 0 closes it in about 25 lines. Note this
costs one extra reformed-system variant; the file already lives in tests/policy/reform precisely
so it gets its own per-file subprocess (Makefile:110-117, --mode per-file --workers 1), so this
is the cheapest place in the repo to pay that cost.

A3. No absolute_error_margin on currency outputs

policyengine_us/tests/policy/reform/boost_head_start_benefits_composition.yaml:42-43.

Both roles flagged this. House convention for currency outputs is absolute_error_margin: 0.01;
every sibling carries it —
policyengine_us/tests/policy/baseline/household/income/household/household_head_start_benefits.yaml
(all 6 cases) and policyengine_us/tests/policy/reform/{ctc_linear_phase_out,mo_refundable_eitc,winship}.yaml
(5 occurrences each). It is a convention gap, not a live failure: 8,000, 21,000 and 29,000 are all
exactly representable in float32, so the default margin already passes. One line to fix.

A4. uv.lock churn does not belong in a test-only PR

uv.lock (6 hunks, 48 insertions / 48 deletions).

The delta is purely mechanical, with no semantic dependency change: the policyengine-us
self-version goes 1.800.0 → 1.819.0 (pyproject.toml:3 is already at 1.819.0, so the lock on
main was simply stale — bump_version.py updates pyproject.toml and CHANGELOG.md, not
uv.lock), plus dependency-marker tightening for pexpect (ptyprocess), the two sphinx
resolution-marker blocks (9.0.4 / 9.1.0) and the two spm_calculator blocks (0.2.0 / 0.3.1). Every
package name, version, sdist hash and wheel list is unchanged — nothing added, removed, or
upgraded
. Reported here at SHOULD ADDRESS as hygiene, not correctness: it breaks nothing.

Evidence it is an incidental local artifact rather than an intentional change:

  • The identical 48/48-line churn is currently sitting uncommitted in this working tree on an
    unrelated branch, produced by nothing but a local uv sync. The marker re-normalization depends
    on the contributor's local uv version, so it will flip back and forth between contributors.
  • The repo has a dedicated owner for lockfile refreshes:
    .github/workflows/weekly-uv-lock.yaml runs uv lock --upgrade on main weekly and opens
    bot/weekly-uv-lock-update with its own changelog fragment.
  • No CI job requires a current lock — every workflow step uses uv sync --extra dev, never
    --locked / --frozen. Dropping the file costs nothing.

Recommendation: git checkout origin/main -- uv.lock and force-push, leaving a clean 2-file PR.
This also removes the most likely merge-conflict source given BEHIND=17. (Precedent is mixed —
commit 714844b335 did land a deliberate lock self-version bump — but there that was the PR's
purpose, not a side effect of a test-only change.)

A5. The composition guard is partial: three list entries have already drifted out of BOOST's hard-coded list

policyengine_us/reforms/congress/tlaib/boost/boost_middle_class_tax_credit.py:54-78 vs
policyengine_us/parameters/gov/household/household_benefits.yaml; test file
policyengine_us/tests/policy/reform/boost_head_start_benefits_composition.yaml:42-43.

Baseline household_benefits reads its member list from the parameter; the BOOST reform
hard-codes a Python list. Comparing them at the snapshot, the reform list omits three entries
the baseline carries: ak_energy_relief (baseline from 2022-01-01), household_health_benefits
(from 2022-01-01) and commodity_supplemental_food_program (from 2024-01-01). The new test proves
household_head_start_benefits did not drift out, while those three already have — invisibly,
because at period 2023 with default zeros they contribute nothing.

Nothing guards reform-local list copies: policyengine_us/tests/test_aggregate_list_integrity.py
only walks parameter list files. That is an argument for landing this PR — it is currently
the only guard on that list — but the guard's partiality should be visible. Minimum ask, in diff:
a comment in the test file naming the three omitted entries so the next reader knows the guard is
deliberately partial. The drift itself is out of this diff; if unintended, it belongs in a
follow-up PR (reading the parameter list and appending, instead of duplicating it, would remove the
drift class entirely).

Suggestions

S1. Case-name convention — disputed between roles; recommend no change.
policyengine_us/tests/policy/reform/boost_head_start_benefits_composition.yaml:10. The code role
noted the Case N, description. convention in references/tests.md; the tests role checked local
usage and found free-form sentence names are exactly what tests/policy/reform/*.yaml and
tests/policy/contrib/congress/tlaib/boost/*.yaml already use, with Case N applying to baseline
variable tests. The file mirrors its ECPA sibling. Consolidated view: not a defect — changing
only this file would create a new inconsistency.

S2. The in_effect input silently activates a second reform.
...composition.yaml:14. With reforms: already force-applying BOOST (the module-level object at
boost_middle_class_tax_credit.py:174-176 uses bypass=True),
gov.contrib.harris.lift.middle_class_tax_credit.in_effect: true is redundant for applying BOOST —
and it additionally activates the Harris LIFT structural reform, since both reforms are gated
on the same parameter (policyengine_us/reforms/reforms.py:391 and :397). No confound today:
LIFT only overrides middle_class_tax_credit and income_tax_refundable_credits, never
household_benefits, and the existing BOOST contrib test sets the same line. Worth a one-line
comment, since "BOOST is gated on the LIFT parameter" is a genuine trap for a future reader.

S3. Period 2023 exercises no bracket under the reform. ...composition.yaml:11. The reform's
list is a static Python literal, so 2023 vs 2026 changes nothing here. The baseline parameter's
pre/post-2024 bracket split is already covered in
tests/policy/baseline/household/income/household/household_head_start_benefits.yaml:25-37.
Choosing 2026, or adding a second period, would align with current law. 2023 is legal and correct —
preference only.

S4. Demographically implausible household. ...composition.yaml:15-17. head_start: 8_000 and
early_head_start: 21_000 are both assigned to a single 30-year-old. Arithmetic is unaffected
(direct inputs are returned as set, bypassing defined_for = is_head_start_eligible), but
tests/policy/baseline/household/income/household/household_head_start_benefits.yaml:67-86
(Case 6) shows the realistic shape — parent 30, child aged 4 with head_start, child aged 1 with
early_head_start — which additionally proves person→household aggregation.

S5. Optionally pin the intentional spm_unit_benefits asymmetry. The reform also overrides
spm_unit_benefits (boost_middle_class_tax_credit.py:96-148) and that list correctly does not
contain household_head_start_benefits, matching baseline
(policyengine_us/variables/household/income/spm_unit/spm_unit_benefits.py) — Head Start is
household-level only. Its absence from the test is not a gap; adding spm_unit_benefits: 0
would pin the asymmetry cheaply.

S6. Stale Makefile target (out of diff). Makefile:89test-yaml-no-structural-other still
ends with $(BATCH) $(TESTS)/policy/reform --mode per-file, even though the comment at
Makefile:122-124 and the CI matrix say policy/reform moved to its own runner. CI never invokes
that target, so it is stale rather than harmful, but a developer running other locally pays for
policy/reform twice.

S7. Identity comparison against a parameter value (out of diff).
boost_middle_class_tax_credit.py:92 (and :146 in spm_unit_benefits) use
if p.administered_through_ssa is False:. This works today only because the loader hands back a
real Python bool; == False or not ... would be safer.

S8. Sweep the ECPA sibling when fixing.
policyengine_us/tests/policy/reform/ecpa_head_start_benefits_composition.yaml is structurally
identical — same period, same 20 zero-pinned inputs, same two assertions at 29,000, same missing
absolute_error_margin, same single-adult household, and no switch-off counter-case either.
Findings A1, A2, A3 and S4 apply to it equally. (ECPA is in fact looser: its list contains a
computed component, ecpa_child_benefit at
policyengine_us/reforms/congress/end_child_poverty_act.py:114, which its test leaves unpinned —
it evaluates to 0 for a childless 30-year-old, so the derivation holds, but by luck.)

PDF Audit Summary

Not applicable (no source-document values in scope).

Validation Summary

Check Result
Regulatory reviewer SKIPPED — out of scope (0 parameter files, 0 variable files changed; nothing regulatory in the diff)
Reference checker SKIPPED — out of scope (no reference: fields added, no URLs or PDFs cited)
PDF audit NOT RUN — no source documents in scope
Verification queue NONE — no value in the diff can be cross-referenced or externally checked
Code pattern audit PASS with 2 should-address, 5 suggestions (0 critical)
Test coverage audit PASS with 4 should-address, 4 suggestions (0 critical)
Test non-vacuity PASS — empirically confirmed against the switch and against list membership (0 / 58,000 both fail)
Test placement / CI execution PASStests/policy/reform runs as matrix group reform (.github/workflows/pr.yaml:239-240make test-yaml-reformMakefile:110-117, --mode per-file); no Makefile edit needed
Period usage PASS — bare YYYY case period, all inputs and outputs YEAR-defined
Entity placement PASS — person / SPM-unit / household inputs all resolve at size 1
Hard-coded values in formulas N/A — no .py changed
Parameter formatting N/A — no parameter files
Changelog fragment PASS — changelog.d/boost-head-start-composition-test.changed.md, top-level, matches the CI regex at pr.yaml:40; changed (patch bump) is the defensible type for a test-only PR
TODO / FIXME / placeholder CLEAN
CI PASS — 31/31 checks
Branch freshness BEHIND=17, AHEAD=1 — rebase recommended (informational)

Review Severity: COMMENT

No criticals: the test is correct, non-vacuous, executed by CI, and worth landing — it is currently
the only guard on BOOST's hard-coded benefits list. Not blocking. The one substantive item is A1:
as written the case pins the Head Start switch but not the BOOST-specific composition it is named
for, since it passes identically with reforms: removed. A second case with
administered_through_ssa: true (expected household_benefits: 32_400) closes that and A2's
sibling gap in a few lines. A4 (dropping the incidental uv.lock churn) is hygiene worth doing
before merge, alongside a rebase.

Next Steps

To auto-fix issues: run the fix-pr workflow for this PR.

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.

2 participants