Fix MO refundable EITC reform crash and zero-refund bug

[PavelMakarchuk]

Fixes #8640

Problem

Triggering the bundled Missouri refundable-EITC contrib reform (gov.contrib.states.mo.child_poverty_impact_dashboard.eitc.in_effect) crashed the simulation. After working around the crash, the reform also failed to actually make the WFTC refundable for the low-liability filers refundability is meant to help.

Three bugs fixed in mo_refundable_eitc_reform.py

1. add() received a parameter path string (crash). mo_refundable_credits.formula passed the literal string "gov.states.mo.tax.income.credits.refundable" to add(), which iterates it character by character — get_variable("g") returns None → AttributeError: 'NoneType' object has no attribute 'entity'. Now resolves the parameter to its list of variable names first.

2. Mixed computation modes (crash on core ≥3.26.8). mo_refundable_credits redeclared a formula but inherited adds from the baseline variable, tripping the engine's strict mode check (ValueError: ... mixes computation modes). Now sets adds = None and subtracts = None explicitly.

3. Refundable credit capped at tax liability (silent zero-refund). mo_refundable_wftc returned mo_wftc — the applied credit capped at MO tax liability — so a zero-liability filer received $0 even though refundability should pay the full amount. Now uses mo_wftc_potential (the uncapped credit). This is the functional fix that makes the reform do what it claims.

Verification

Reproduced the issue's exact script — runs without raising. Verified the reform's effect against baseline for a Missouri single parent, 2 kids, $20k earnings, 2026 (federal EITC $7,316 → potential WFTC $1,463):

Variable	Baseline	Reform
mo_refundable_credits	$0	$1,463.20
mo_income_tax	$0	−$1,463.20 (refund)
household_net_income	$36,103.81	$37,567.01

Also confirmed no double-counting at positive liability: a moderate-income case yields a delta exactly equal to the previously-lost (nonrefundable-capped) portion of the credit.

Tests

• New tests/policy/reform/mo_refundable_eitc.yaml: verifies the full potential WFTC is paid as refundable at zero liability, the nonrefundable portion is zeroed, and there's no double-counting when liability already absorbs the credit.
• All 361 Missouri baseline tests still pass.

🤖 Generated with Claude Code

[DTrim99]

PR Review — #8642: Fix MO refundable EITC reform crash and zero-refund bug

A focused bug fix to the Missouri refundable-EITC contrib reform (mo_refundable_eitc_reform.py), fixing #8640. Reviewed for correctness, code patterns, test coverage, and CI. All three claimed fixes were independently verified as correct — no silently dropped credits and no double-counting.

🔴 Critical (Must Fix)

None. The fixes are correct and the PR is merge-ready on the code. (The one failing CI job is unrelated infrastructure flakiness — see below — not a PR defect.)

🟡 Should Address

1. Add a multi-refundable-credit regression test (highest value). Bug 1's fix rewrote how the non-WFTC refundable credits are summed (resolving the refundable parameter to its variable list, currently ["mo_property_tax_credit"], then add()-ing it alongside mo_refundable_wftc). No test injects a second MO refundable credit, so a future regression that drops other_refundable (returning only WFTC) would pass silently. Add a case with a property-tax credit present and assert it survives.
2. Currency test cases omit absolute_error_margin: 0.01. Both new/changed YAML tests use exact-integer expectations; adding the one-cent margin is the convention and guards against float drift.
3. Forward-compat fragility (worth a code comment). The reform's mo_non_refundable_credits override returns only mo_non_refundable_wftc, discarding the baseline's ordered credit-cap summation over the full nonrefundable list. Today that list is just [mo_wftc] so behavior is identical — but if MO later adds another nonrefundable credit, the reform would silently zero it. This is pre-existing in the base file (not introduced by this diff); a comment (or summing the full list) would prevent a future surprise.

🟢 Suggestions

1. Add boundary cases in the reform-level test: a zero-EITC filer (potential WFTC = $0) and a partial-liability case (liability between 0 and the potential credit) to lock down the capped-vs-uncapped boundary.
2. Align test case names to the Case N, description. convention.
3. Optional: the Bug-2 "mixes computation modes" ValueError only triggers on core ≥3.26.8 (the installed core 3.26.0 silently ignores inherited adds when a formula exists). The adds = None/subtracts = None change is still the correct forward-compatible pattern — just noting the crash actually fixed on current core is Bug 1 (the string path).

Findings detail

• Bug 1 (string passed to add() → crash): Verified. add() iterates its 3rd arg as variable names; a bare parameter-path string iterates char-by-char → get_variable("g") → AttributeError. The fix resolves the refundable parameter to its list first, then calls add(). Idiomatic and period-correct.
• Bug 2 (adds inherited + custom formula): Correct. update_variable retains inherited attributes; setting adds = None/subtracts = None is the right pattern. Completeness confirmed — the reform's formula reproduces the full baseline refundable set (mo_property_tax_credit) plus mo_refundable_wftc; nothing dropped.
• Bug 3 (capped mo_wftc → $0 at zero liability): Correct. mo_wftc_potential is the uncapped credit (eitc × wftc.match); returning it is right for refundability. No double-counting — the reform also overrides mo_non_refundable_credits to drop WFTC from the nonrefundable side, so it's counted exactly once. The regression test (eitc=5000, before_credits=1000 → mo_income_tax=0, not negative) confirms this.
• Code-health test change is legitimate, not a weakening. Switching consumption from the capped mo_wftc to uncapped mo_wftc_potential removes the exact anti-pattern test_non_refundable_credit_downstream_consumers.py guards, so the allowlist entry must be removed or the test fails. It strengthens pattern adherence.
• Baseline unaffected: …eitc.in_effect defaults false; reform factory returns None and overrides nothing when off.

Validation Summary

Check	Result
Reform correctness (3 fixes)	PASS — all correct, no dropped credits, no double-counting
Code patterns	PASS — idiomatic; code-health test change legitimate
Test coverage	Adequate — all 3 bugs guarded; add multi-credit + boundary cases
References	N/A — no parameters/law values in this PR
CI Status	1 job failed (Contrib states-shard-2) — infrastructure timeout during an unrelated RI batch; the PR's own MO test passed in that job. Re-run to clear.

Review Severity: COMMENT

No blocking issues — the fixes are verified correct. Recommendations are test-coverage enhancements and one pre-existing forward-compat note.

Next Steps

• Re-run the cancelled Contrib (states-shard-2) job (infrastructure, not code).
• To apply the suggested test additions: /fix-pr 8642.

[DTrim99]

PR Review — #8642: Fix MO refundable EITC reform crash and zero-refund bug

The Missouri member of a three-PR family of state refundable-EITC reform fixes (alongside #8645 UT and #8657 OH). Reviewed read-only for correctness, code patterns, and test coverage. All three claimed fixes verified correct; CI fully green (26/26 checks).

🔴 Critical (Must Fix)

None. The fixes are correct and merge-ready.

🟡 Should Address

1. Add absolute_error_margin: 0.01 to the edited contrib YAML (…/child_poverty_impact_dashboard/eitc/mo_refundable_eitc.yaml). The new reform-level test file correctly carries it on all 5 cases; the edited contrib cases do not. Values are exact integers so tests pass, but the one-cent margin is the convention and guards float drift.
2. Forward-compat note on mo_non_refundable_credits (pre-existing). The override returns only mo_non_refundable_wftc, so a future second MO nonrefundable credit would be silently zeroed. The added code comment honestly documents this; today the list is [mo_wftc] so behavior is identical. Consider the filtered-list pattern eventually (low priority).

🟢 Suggestions

1. The new test uses the fully-qualified reform path while the contrib test uses the __init__ re-export — both valid; harmonizing aids consistency.
2. Sibling cross-check (not a defect here): on main, OH oh_refundable_eitc and UT reforms still carry the capped-credit zero-refund pattern — but those are already being fixed by Fix OH refundable EITC reform zero-refund and lost-credit bugs #8657 and Fix UT fully refundable EITC reform crash and zero-refund bug #8645, so no separate follow-up is needed. (Worth confirming SC / any other state reforms in the same allowlist get the same *_potential treatment.)

Findings detail

All three fixes verified against source:

• Bug (a) — string→add() crash: add()/for_each_variable iterates its variable argument; a bare param-path string iterates char-by-char → get_variable("g") → AttributeError. The fix resolves the refundable parameter to its list (["mo_property_tax_credit"]) before add(). Correct.
• Bug (b) — adds = None/subtracts = None: baseline mo_refundable_credits uses adds = "…credits.refundable"; defining a custom formula while inheriting adds trips the engine's mixed-mode check, so clearing both is necessary and correct.
• Bug (c) — capped→uncapped, no double-count: mo_wftc is liability-capped (applied_state_non_refundable_credit) → $0 at zero liability; mo_wftc_potential is uncapped (eitc × match). The reform pays the potential on the refundable side and zeroes WFTC on the non-refundable side, so it is counted exactly once. Match rate 0.2 → all test arithmetic (2,500→500; 5,000→1,000) checks out.
• Code-health allowlist removal is legitimate, not a weakening: the reform no longer references the capped "mo_wftc" string (only "mo_wftc_potential"), so the mo_wftc allowlist entry must be removed or the invariant test fails. It strengthens pattern adherence.

Validation Summary

Check	Result
Reform correctness (3 fixes)	PASS — all correct, no dropped credits, no double-counting
Code patterns	PASS — definition_period = YEAR; no hard-coding; idiomatic
Test coverage	STRONG — zero-liability, partial-liability, multi-credit, zero-EITC, wrong-state all covered
References	N/A — no parameters/law values in this PR
Code-health allowlist change	PASS — required consequence of capped→potential switch
CI status	PASS — all 26 checks SUCCESS

Review Severity: APPROVE

The three fixes are correct and well-tested with strong boundary coverage. The only items are a one-cent margin on the contrib YAML and a pre-existing forward-compat note. Sibling fixes for OH/UT are already in flight (#8657 / #8645).