From e1bfdb4f24ec7f8a2483ad7ae8b454d08c3658b0 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Tue, 6 Oct 2026 16:52:32 -0400 Subject: [PATCH 1/3] Guard mapped bracket thresholds and NaN values; fix the contrib MFS bracket test_parameter_scale_thresholds only checked ParameterScale brackets, so federal-style schedules (a `thresholds` node mapping bracket numbers to a value per filing status, as gov.irs.income.bracket does) could repeat or invert a threshold unnoticed, and nothing checked parameter values for NaN, which YAML loads silently in scalars and inside list values. The new mapped-threshold guard found one live inversion: the additional_tax_bracket contrib reform's SEPARATE bracket-5 top for 2026-2035 copied the JOINT values (541,550 in 2026, above bracket 6's 305,875). #6235 fixed the same copy in gov.irs.income.bracket; the contrib file kept it. The values are now half the joint thresholds, as in every other bracket and year of the file and in #6235. Under the reform, a separate filer's income from 270,775 to 305,875 was taxed at 33% instead of 35% in 2026. Co-Authored-By: Claude Opus 5.5 --- ...d-threshold-and-nested-nan-guards.fixed.md | 1 + .../additional_tax_bracket/bracket.yaml | 20 +-- .../additional_tax_bracket_reform.yaml | 14 ++ .../tests/test_parameter_scale_thresholds.py | 170 +++++++++++++++++- 4 files changed, 194 insertions(+), 11 deletions(-) create mode 100644 changelog.d/mapped-threshold-and-nested-nan-guards.fixed.md diff --git a/changelog.d/mapped-threshold-and-nested-nan-guards.fixed.md b/changelog.d/mapped-threshold-and-nested-nan-guards.fixed.md new file mode 100644 index 00000000000..d597de8c06e --- /dev/null +++ b/changelog.d/mapped-threshold-and-nested-nan-guards.fixed.md @@ -0,0 +1 @@ +The married-filing-separately 33% bracket in the additional tax bracket contrib reform now ends at half the joint threshold for 2026-2035, as main's schedule does, instead of a copy of the joint threshold that inverted it; tests now check that federal-style bracket thresholds strictly increase and that no parameter value holds NaN, including inside list values. diff --git a/policyengine_us/parameters/gov/contrib/additional_tax_bracket/bracket.yaml b/policyengine_us/parameters/gov/contrib/additional_tax_bracket/bracket.yaml index d3ad5c54d8a..feaaea7ba9b 100644 --- a/policyengine_us/parameters/gov/contrib/additional_tax_bracket/bracket.yaml +++ b/policyengine_us/parameters/gov/contrib/additional_tax_bracket/bracket.yaml @@ -680,16 +680,16 @@ thresholds: 2024-01-01: 243_725 2025-01-01: 250_525 # CBO forecast. - 2026-01-01: 541_550 - 2027-01-01: 553_050 - 2028-01-01: 564_200 - 2029-01-01: 575_400 - 2030-01-01: 586_750 - 2031-01-01: 598_350 - 2032-01-01: 610_200 - 2033-01-01: 622_300 - 2034-01-01: 634_650 - 2035-01-01: 647_250 + 2026-01-01: 270_775 + 2027-01-01: 276_525 + 2028-01-01: 282_100 + 2029-01-01: 287_700 + 2030-01-01: 293_375 + 2031-01-01: 299_175 + 2032-01-01: 305_100 + 2033-01-01: 311_150 + 2034-01-01: 317_325 + 2035-01-01: 323_625 metadata: uprating: parameter: gov.irs.uprating diff --git a/policyengine_us/tests/policy/contrib/additional_tax_bracket/additional_tax_bracket_reform.yaml b/policyengine_us/tests/policy/contrib/additional_tax_bracket/additional_tax_bracket_reform.yaml index 369dc5f9c36..bc05ab6c157 100644 --- a/policyengine_us/tests/policy/contrib/additional_tax_bracket/additional_tax_bracket_reform.yaml +++ b/policyengine_us/tests/policy/contrib/additional_tax_bracket/additional_tax_bracket_reform.yaml @@ -172,3 +172,17 @@ taxable_income_taxed_below_25_percent: 201_775 dwks19: 200_000 capital_gains_excluded_from_taxable_income: 100_000 + +- name: A separate filer's 33% bracket ends at half the joint threshold under the reform. + period: 2026 + absolute_error_margin: 0.01 + reforms: policyengine_us.reforms.additional_tax_bracket.additional_tax_bracket_reform.additional_tax_bracket + input: + gov.contrib.additional_tax_bracket.in_effect: true + taxable_income: 300_000 + filing_status: SEPARATE + output: + # 2026 separate thresholds 12,150 / 49,300 / 99,500 / 151,625 / 270,775 / + # 305,875 at 10 / 15 / 25 / 28 / 33 / 35%: + # 1,215 + 5,572.50 + 12,550 + 14,595 + 39,319.50 + 0.35 * 29,225. + income_tax_main_rates: 83_480.75 diff --git a/policyengine_us/tests/test_parameter_scale_thresholds.py b/policyengine_us/tests/test_parameter_scale_thresholds.py index 0899577cbed..79886f4b76c 100644 --- a/policyengine_us/tests/test_parameter_scale_thresholds.py +++ b/policyengine_us/tests/test_parameter_scale_thresholds.py @@ -13,6 +13,16 @@ Duplicate `+inf` thresholds are exempt: tables whose length changes over time park unused brackets there, and no finite value reaches them. `-inf` is a reachable bottom row, so duplicates there are flagged. + +Federal-style rate schedules such as `gov.irs.income.bracket` are not scales: +their `thresholds` node maps bracket numbers 1..N to a value per filing status, +and `tax_at_main_rates` reads bracket i's threshold as its top. A repeated +threshold empties a bracket and an inverted one is clamped (#9084), so either +silently taxes a band at the wrong rate. Each filing status's thresholds must +therefore strictly increase at every instant, again exempting `+inf`. + +YAML's `.nan` loads as a float in a scalar or inside a list value, and every +comparison with it is false, so no parameter value may hold NaN at any depth. """ import math @@ -20,7 +30,7 @@ from datetime import date import pytest -from policyengine_core.parameters import ParameterNode, ParameterScale +from policyengine_core.parameters import Parameter, ParameterNode, ParameterScale from policyengine_core.parameters.operations.uprate_parameters import ( uprate_parameters, ) @@ -260,3 +270,161 @@ def test_parameter_scales_have_no_duplicate_thresholds(): assert scales assert not errors, "\n".join(errors) + + +def _mapped_threshold_tables(): + return [ + node + for node in system.parameters.get_descendants() + if isinstance(node, ParameterNode) + and node.name.rsplit(".", 1)[-1] == "thresholds" + and node.children + and sorted(node.children) + == sorted(str(i) for i in range(1, len(node.children) + 1)) + ] + + +def _mapped_threshold_errors(node): + brackets = sorted(node.children, key=int) + columns = defaultdict(dict) + for bracket in brackets: + child = node.children[bracket] + if isinstance(child, ParameterNode): + for column, leaf in child.children.items(): + columns[column][bracket] = leaf + else: + columns[None][bracket] = child + instants_by_error = defaultdict(list) + for column, leaves in columns.items(): + instants = sorted( + { + value_at_instant.instant_str + for leaf in leaves.values() + for value_at_instant in leaf.values_list + } + ) + for instant_str in instants: + present = [ + (bracket, value) + for bracket in brackets + if bracket in leaves + and (value := leaves[bracket]._get_at_instant(instant_str)) is not None + ] + for (lower, low), (upper, high) in zip(present, present[1:]): + if not low < high and not low == high == math.inf: + instants_by_error[(column, lower, upper)].append( + (instant_str, low, high) + ) + errors = [] + for (column, lower, upper), cases in instants_by_error.items(): + instant_str, low, high = cases[0] + when = instant_str + if len(cases) > 1: + when = ( + f"{instant_str} and {len(cases) - 1} later instants to {cases[-1][0]}" + ) + name = node.name if column is None else f"{node.name}.*.{column}" + errors.append( + f"{name}: bracket {upper} threshold {high:,.15g} does not exceed " + f"bracket {lower} threshold {low:,.15g} at {when}" + ) + return errors + + +def _mapped_table(thresholds): + return ParameterNode("root", data={"thresholds": thresholds}).thresholds + + +def test_mapped_threshold_guard_flags_repeats_and_inversions(): + table = _mapped_table( + { + 1: {"SINGLE": {"2024-01-01": 100}, "JOINT": {"2024-01-01": 200}}, + 2: { + "SINGLE": {"2024-01-01": 100}, + "JOINT": {"2024-01-01": 400, "2025-01-01": 150}, + }, + 3: {"SINGLE": {"2024-01-01": 300}, "JOINT": {"2024-01-01": 600}}, + } + ) + + assert _mapped_threshold_errors(table) == [ + "root.thresholds.*.SINGLE: bracket 2 threshold 100 does not exceed " + "bracket 1 threshold 100 at 2024-01-01", + "root.thresholds.*.JOINT: bracket 2 threshold 150 does not exceed " + "bracket 1 threshold 200 at 2025-01-01", + ] + + +def test_mapped_threshold_guard_ignores_only_parked_positive_infinity(): + def table_with_top(top): + return _mapped_table( + { + 1: {"SINGLE": {"2024-01-01": 100}}, + 2: {"SINGLE": {"2024-01-01": math.inf}}, + 3: {"SINGLE": {"2024-01-01": top}}, + } + ) + + assert _mapped_threshold_errors(table_with_top(math.inf)) == [] + assert _mapped_threshold_errors(table_with_top(500)) == [ + "root.thresholds.*.SINGLE: bracket 3 threshold 500 does not exceed " + "bracket 2 threshold inf at 2024-01-01" + ] + + +def test_mapped_bracket_thresholds_strictly_increase(): + tables = _mapped_threshold_tables() + errors = [error for table in tables for error in _mapped_threshold_errors(table)] + + assert { + "gov.irs.income.bracket.thresholds", + "gov.irs.capital_gains.thresholds", + } <= {table.name for table in tables} + assert not errors, "\n".join(errors) + + +def _nan_paths(value, path=""): + if isinstance(value, float) and math.isnan(value): + yield path or "value" + elif isinstance(value, (list, tuple)): + for index, item in enumerate(value): + yield from _nan_paths(item, f"{path}[{index}]") + + +def _nan_values(parameter): + return [ + f"{value_at_instant.instant_str} {path}" + for value_at_instant in parameter.values_list + for path in _nan_paths(value_at_instant.value) + ] + + +def test_nan_guard_flags_nan_in_scalars_and_nested_lists(): + parameter = Parameter( + "test_parameter", + { + "values": { + "2023-01-01": [1.0, [2.0, 3.0]], + "2024-01-01": [1.0, [2.0, math.nan]], + "2025-01-01": math.nan, + "2026-01-01": [math.nan], + } + }, + ) + + assert sorted(_nan_values(parameter)) == [ + "2024-01-01 [1][1]", + "2025-01-01 value", + "2026-01-01 [0]", + ] + + +def test_parameter_values_are_not_nan(): + errors = [ + f"{parameter.name}: {error}" + for parameter in system.parameters.get_descendants() + if isinstance(parameter, Parameter) + for error in _nan_values(parameter) + ] + + assert not errors, "\n".join(errors) From 24f75b08f1518395ef10fa4c21f257de89478204 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Tue, 6 Oct 2026 17:22:50 -0400 Subject: [PATCH 2/3] Address review: recreate the contrib inversion in the clamp test test_shipped_additional_tax_bracket_reform_is_clamped relied on the shipped SEPARATE inversion, which the previous commit removed, so a zero-income filer owed nothing with or without the clamp. The test now recreates the old 541,550 bracket-5 top itself. The mapped-threshold guard now requires a `rates` sibling, the node tax_at_main_rates reads with `thresholds`, so a household-size table named `thresholds` cannot be swept in. New unit cases show it flags a repeated -inf and a NaN threshold, and the NaN test asserts it visited parameters. A second reform case covers the 39.6% band. The old data also taxed a separate filer's income from 305,875 to 541,550 at 33% instead of 39.6% (not only 270,775 to 305,875 at 33% instead of 35%, as the previous commit said): taxable income of 400,000 owed 115,896.25, not 122,810.50. Co-Authored-By: Claude Opus 5.5 --- ...d-threshold-and-nested-nan-guards.fixed.md | 2 +- .../additional_tax_bracket_reform.yaml | 13 ++++++ ...t_negative_main_rates_inverted_brackets.py | 17 ++++--- .../tests/test_parameter_scale_thresholds.py | 45 ++++++++++++++++--- 4 files changed, 62 insertions(+), 15 deletions(-) diff --git a/changelog.d/mapped-threshold-and-nested-nan-guards.fixed.md b/changelog.d/mapped-threshold-and-nested-nan-guards.fixed.md index d597de8c06e..26d33caeaf5 100644 --- a/changelog.d/mapped-threshold-and-nested-nan-guards.fixed.md +++ b/changelog.d/mapped-threshold-and-nested-nan-guards.fixed.md @@ -1 +1 @@ -The married-filing-separately 33% bracket in the additional tax bracket contrib reform now ends at half the joint threshold for 2026-2035, as main's schedule does, instead of a copy of the joint threshold that inverted it; tests now check that federal-style bracket thresholds strictly increase and that no parameter value holds NaN, including inside list values. +The married-filing-separately 33% bracket in the additional tax bracket contrib reform now ends at half the joint threshold from 2026 (270,775 in 2026), as the baseline schedule does, instead of a copy of the joint threshold that inverted it and taxed separate filers' income from 270,775 to 541,550 at 33%. Tests now check that federal-style bracket thresholds strictly increase and that no parameter value holds NaN, including inside list values. diff --git a/policyengine_us/tests/policy/contrib/additional_tax_bracket/additional_tax_bracket_reform.yaml b/policyengine_us/tests/policy/contrib/additional_tax_bracket/additional_tax_bracket_reform.yaml index bc05ab6c157..2f24594ab5a 100644 --- a/policyengine_us/tests/policy/contrib/additional_tax_bracket/additional_tax_bracket_reform.yaml +++ b/policyengine_us/tests/policy/contrib/additional_tax_bracket/additional_tax_bracket_reform.yaml @@ -186,3 +186,16 @@ # 305,875 at 10 / 15 / 25 / 28 / 33 / 35%: # 1,215 + 5,572.50 + 12,550 + 14,595 + 39,319.50 + 0.35 * 29,225. income_tax_main_rates: 83_480.75 + +- name: A separate filer above the 35% bracket pays 39.6% under the reform. + period: 2026 + absolute_error_margin: 0.01 + reforms: policyengine_us.reforms.additional_tax_bracket.additional_tax_bracket_reform.additional_tax_bracket + input: + gov.contrib.additional_tax_bracket.in_effect: true + taxable_income: 400_000 + filing_status: SEPARATE + output: + # 83,480.75 on the first 300,000 (case above), plus 35% of 5,875 up to + # 305,875, plus 39.6% of the remaining 94,125. + income_tax_main_rates: 122_810.50 diff --git a/policyengine_us/tests/test_negative_main_rates_inverted_brackets.py b/policyengine_us/tests/test_negative_main_rates_inverted_brackets.py index 6af6c56020d..3d950319df4 100644 --- a/policyengine_us/tests/test_negative_main_rates_inverted_brackets.py +++ b/policyengine_us/tests/test_negative_main_rates_inverted_brackets.py @@ -194,15 +194,18 @@ def four_status_situation(): def test_shipped_additional_tax_bracket_reform_is_clamped(): - # The contrib reform replaces both bracket loops and its parameter - # tree still carries the expiration projection's inverted SEPARATE - # thresholds (bracket 5 top 541,550 above bracket 6 top 305,875), - # so it must clamp too: before the fix a zero-income 2026 MFS filer - # owed a flat −$82,486 under this shipped reform. The reform's - # user-supplied extra bracket is filled as in its own yaml tests; - # brackets 1–6 stay at the shipped (inverted) values. + # The contrib reform replaces both bracket loops, so it must clamp too. + # Its parameter tree used to carry the expiration projection's inverted + # SEPARATE thresholds (bracket 5 top 541,550 above bracket 6 top + # 305,875), and before the clamp a zero-income 2026 MFS filer owed a flat + # −$82,486 under the shipped reform. The file now has the correct + # 270,775, so this reform recreates that inversion. The reform's + # user-supplied extra bracket is filled as in its own yaml tests. fill_extra_bracket = Reform.from_dict( { + "gov.contrib.additional_tax_bracket.bracket.thresholds.5.SEPARATE": { + "2026-01-01.2026-12-31": 541_550 + }, "gov.contrib.additional_tax_bracket.bracket.thresholds.7.SEPARATE": { "2026-01-01.2026-12-31": 800_000 }, diff --git a/policyengine_us/tests/test_parameter_scale_thresholds.py b/policyengine_us/tests/test_parameter_scale_thresholds.py index 79886f4b76c..69c8c703d9d 100644 --- a/policyengine_us/tests/test_parameter_scale_thresholds.py +++ b/policyengine_us/tests/test_parameter_scale_thresholds.py @@ -273,14 +273,18 @@ def test_parameter_scales_have_no_duplicate_thresholds(): def _mapped_threshold_tables(): + # A rate schedule's `thresholds` node sits beside the `rates` node that + # `tax_at_main_rates` reads with it, and maps bracket numbers 1..N. return [ - node + node.children["thresholds"] for node in system.parameters.get_descendants() if isinstance(node, ParameterNode) - and node.name.rsplit(".", 1)[-1] == "thresholds" - and node.children - and sorted(node.children) - == sorted(str(i) for i in range(1, len(node.children) + 1)) + and "rates" in node.children + and isinstance(node.children.get("thresholds"), ParameterNode) + and sorted(node.children["thresholds"].children) + == sorted( + str(i) for i in range(1, len(node.children["thresholds"].children) + 1) + ) ] @@ -355,6 +359,28 @@ def test_mapped_threshold_guard_flags_repeats_and_inversions(): ] +def test_mapped_threshold_guard_flags_negative_infinity_repeats_and_nan(): + table = _mapped_table( + { + 1: {"SINGLE": {"2024-01-01": -math.inf}, "JOINT": {"2024-01-01": 100}}, + 2: { + "SINGLE": {"2024-01-01": -math.inf}, + "JOINT": {"2024-01-01": math.nan}, + }, + 3: {"SINGLE": {"2024-01-01": 300}, "JOINT": {"2024-01-01": 600}}, + } + ) + + assert _mapped_threshold_errors(table) == [ + "root.thresholds.*.SINGLE: bracket 2 threshold -inf does not exceed " + "bracket 1 threshold -inf at 2024-01-01", + "root.thresholds.*.JOINT: bracket 2 threshold nan does not exceed " + "bracket 1 threshold 100 at 2024-01-01", + "root.thresholds.*.JOINT: bracket 3 threshold 600 does not exceed " + "bracket 2 threshold nan at 2024-01-01", + ] + + def test_mapped_threshold_guard_ignores_only_parked_positive_infinity(): def table_with_top(top): return _mapped_table( @@ -420,11 +446,16 @@ def test_nan_guard_flags_nan_in_scalars_and_nested_lists(): def test_parameter_values_are_not_nan(): - errors = [ - f"{parameter.name}: {error}" + parameters = [ + parameter for parameter in system.parameters.get_descendants() if isinstance(parameter, Parameter) + ] + errors = [ + f"{parameter.name}: {error}" + for parameter in parameters for error in _nan_values(parameter) ] + assert parameters assert not errors, "\n".join(errors) From f0d858eb838c877af21548d1c4cc8ec4f49140f3 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Tue, 6 Oct 2026 18:13:33 -0400 Subject: [PATCH 3/3] Check the contrib clamp inside the inverted band Round-2 review: a zero-income filer owes nothing under any loop that is never negative per bracket, so the clamp test could not tell today's unclamped form from the clamped one, nor notice if the recreated inversion failed to apply. A separate filer with 400,000 of taxable income now must owe exactly 115,896.25 under the recreated inversion: 122,810.50 if the inversion did not apply, other figures if the loop did not clamp. (The previous commit's "owed nothing with or without the clamp" held only for the old clip form.) The table finder also skips an empty `thresholds` node, and the 2035 SEPARATE line loses a trailing space. Co-Authored-By: Claude Opus 5.5 --- .../contrib/additional_tax_bracket/bracket.yaml | 2 +- .../test_negative_main_rates_inverted_brackets.py | 14 ++++++++++++++ .../tests/test_parameter_scale_thresholds.py | 1 + 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/policyengine_us/parameters/gov/contrib/additional_tax_bracket/bracket.yaml b/policyengine_us/parameters/gov/contrib/additional_tax_bracket/bracket.yaml index feaaea7ba9b..c5c69ad20dd 100644 --- a/policyengine_us/parameters/gov/contrib/additional_tax_bracket/bracket.yaml +++ b/policyengine_us/parameters/gov/contrib/additional_tax_bracket/bracket.yaml @@ -689,7 +689,7 @@ thresholds: 2032-01-01: 305_100 2033-01-01: 311_150 2034-01-01: 317_325 - 2035-01-01: 323_625 + 2035-01-01: 323_625 metadata: uprating: parameter: gov.irs.uprating diff --git a/policyengine_us/tests/test_negative_main_rates_inverted_brackets.py b/policyengine_us/tests/test_negative_main_rates_inverted_brackets.py index 3d950319df4..490c33dfdbc 100644 --- a/policyengine_us/tests/test_negative_main_rates_inverted_brackets.py +++ b/policyengine_us/tests/test_negative_main_rates_inverted_brackets.py @@ -223,3 +223,17 @@ def test_shipped_additional_tax_bracket_reform_is_clamped(): before_credits = sim.calculate("income_tax_before_credits", 2026)[0] assert main == 0 assert before_credits == 0 + + # A zero-income filer owes nothing under any non-negative loop, so also + # check a filer inside the inverted band. Clamped, bracket 6 is empty and + # 151,625-400,000 is all taxed at bracket 5's 33%: 115,896.25. Without + # the inversion the tax is 122,810.50, and an unclamped loop gives yet + # another figure. + situation = separate_filer(0) + situation["tax_units"]["tu"]["taxable_income"] = {"2026": 400_000} + sim = Simulation( + reform=(fill_extra_bracket, additional_tax_bracket), situation=situation + ) + assert sim.calculate("income_tax_main_rates", 2026)[0] == pytest.approx( + 115_896.25, abs=0.01 + ) diff --git a/policyengine_us/tests/test_parameter_scale_thresholds.py b/policyengine_us/tests/test_parameter_scale_thresholds.py index 69c8c703d9d..6d8d032fb3f 100644 --- a/policyengine_us/tests/test_parameter_scale_thresholds.py +++ b/policyengine_us/tests/test_parameter_scale_thresholds.py @@ -281,6 +281,7 @@ def _mapped_threshold_tables(): if isinstance(node, ParameterNode) and "rates" in node.children and isinstance(node.children.get("thresholds"), ParameterNode) + and node.children["thresholds"].children and sorted(node.children["thresholds"].children) == sorted( str(i) for i in range(1, len(node.children["thresholds"].children) + 1)