diff --git a/changelog.d/docs-test-hygiene.fixed.md b/changelog.d/docs-test-hygiene.fixed.md new file mode 100644 index 00000000000..80d7d00ec06 --- /dev/null +++ b/changelog.d/docs-test-hygiene.fixed.md @@ -0,0 +1 @@ +Fix the documentation helper's chart import for current Core, list the PIA-ratio guide in the docs table of contents, update stale interpolation comments in the pinned-system test, and make the calculator reference test classify every leaf under kept parameter nodes and flag missing calculator-only paths. diff --git a/docs/_toc.yml b/docs/_toc.yml index aef7dfab381..0e526a13322 100644 --- a/docs/_toc.yml +++ b/docs/_toc.yml @@ -68,7 +68,13 @@ parts: - file: usage/social-security-calculator sections: - file: usage/social-security/RULINGS + - file: usage/social-security-pia-ratio + - file: social-security-benefit-levers + sections: + - file: social-security-cola-means-test - file: usage/social-security-oasdi-accounting + sections: + - file: usage/social-security-scoring-preflight - file: usage/speed - caption: Examples chapters: diff --git a/policyengine_us/tests/code_health/test_source_conflicts.py b/policyengine_us/tests/code_health/test_source_conflicts.py index 690aa04a9ed..bbdccc0ff26 100644 --- a/policyengine_us/tests/code_health/test_source_conflicts.py +++ b/policyengine_us/tests/code_health/test_source_conflicts.py @@ -47,7 +47,8 @@ These are structural checks: they tie markers, the register and the backlog together; they do not judge whether code follows a statute. rulespec-us's own ``verify`` checks the other half at pinned commits: that each listed file and -test exists and that ``cites_entry`` is true. +test exists and that each ``cites_entry`` claim matches whether the file names +the entry. """ from __future__ import annotations @@ -114,9 +115,10 @@ ) # Files with conflict-sounding comments and no marker, and how many such -# lines each has. A file may only go down: register and mark the conflict, -# or reword a comment that is not one, then lower or delete its count here. -# A new file or a higher count fails. +# lines each has. A new file or a higher count fails, so reword a comment that +# describes no conflict. A real conflict stays: record it as a candidate and add +# or raise its line here in the same PR, and the line goes when the marker lands +# (CONTRIBUTING.md, "Source conflicts"). A lower count must be recorded here too. KNOWN_UNMARKED: dict[str, int] = { "policyengine_us/parameters/gov/irs/ald/charitable_contributions/cap.yaml": 1, "policyengine_us/parameters/gov/irs/credits/residential_clean_energy/fuel_cell_cap_per_kw.yaml": 1, @@ -447,8 +449,11 @@ def test_conflict_sounding_comments_do_not_grow(): assert not grown, ( "These files gained comments that read like a conflict between sources, with no " "`# Source conflict :` marker. If a statute and a form, instructions, manual or " - "agency practice disagree, register it (rulespec-us known-source-conflicts.yaml) and " - f"mark the deciding line; otherwise reword the comment. Files and counts: {grown}" + "agency practice disagree, record it in source_conflicts/candidates.yaml (unless the " + "pinned register already lists it) and add or raise the file's KNOWN_UNMARKED line in " + "this PR; the register entry and the marker come after " + 'it merges (CONTRIBUTING.md, "Source conflicts"). ' + f"Otherwise reword the comment. Files and counts: {grown}" ) assert not shrunk, ( "These files now have fewer unmarked conflict-sounding comments; lower or delete " diff --git a/policyengine_us/tests/test_pinned_tbs_sharing.py b/policyengine_us/tests/test_pinned_tbs_sharing.py index 84a0c941e8b..49fa3a0855a 100644 --- a/policyengine_us/tests/test_pinned_tbs_sharing.py +++ b/policyengine_us/tests/test_pinned_tbs_sharing.py @@ -183,7 +183,9 @@ def change_small_tree(tree, change, index, value): ) parameter.values_list.sort(key=lambda item: item.instant_str, reverse=True) elif change == "change metadata": - # What interpolation does to a parameter's metadata. + # What interpolation did to a parameter's metadata in core before + # 3.34.1, which recorded it there; core now sets the parameter's + # `interpolated` attribute instead. parameter.metadata.setdefault("interpolation", {})["completed"] = value elif f"added_{value}" not in tree.tax.children: added = Parameter(f"added_{value}", {"values": {AT: value}}) @@ -302,7 +304,8 @@ def test_a_copy_of_the_nodes_reads_what_a_clone_reads(changes, later_changes): def test_each_tree_interpolates_for_itself(): - """Core records in a parameter's metadata that it interpolated the parameter. + """Core records on each parameter that it interpolated it, and core before + 3.34.1 recorded it in the parameter's `interpolation` metadata. A record shared with the source would stop the copy interpolating at all. """ diff --git a/policyengine_us/tests/unit/test_ss_calculator_differential.py b/policyengine_us/tests/unit/test_ss_calculator_differential.py index d59466ec760..b5c15d881ef 100644 --- a/policyengine_us/tests/unit/test_ss_calculator_differential.py +++ b/policyengine_us/tests/unit/test_ss_calculator_differential.py @@ -15,6 +15,7 @@ import numpy as np import pytest +from policyengine_core.parameters import ParameterNode from policyengine_core.reforms import Reform from policyengine_us import Simulation @@ -31,26 +32,110 @@ "gov.ssa.social_security.pia.bend_points", "gov.ssa.social_security.cola", ) -# Contributed namespaces whose every child node must be classified: either -# calculator-only (listed in NEW_PARAMETER_NODES, so the reference drops it) -# or kept in the reference (listed below with the code outside the household -# calculator that reads it). +# Contributed namespaces whose every leaf must be classified: either +# calculator-only (at or under a path in NEW_PARAMETER_NODES, so the reference +# drops it) or kept in the reference (listed below under its kept node, whose +# comment names the code outside the household calculator that reads it). CLASSIFIED_CONTRIB_NAMESPACES = ("gov.contrib.congress.s770", "gov.contrib.ssa") -REFERENCE_PARAMETER_NODES = ( +# Each kept node maps to every leaf beneath it, so a calculator-only leaf added +# under a kept node fails the classification test until someone lists it here +# or in NEW_PARAMETER_NODES. +REFERENCE_PARAMETER_NODES = { # tools/social_security/population.py: reported-benefit PIA ratios. - "gov.contrib.congress.s770.section_2", + "gov.contrib.congress.s770.section_2": ( + "in_effect", + "recompute_pre2026_death_before_eligibility", + ), # reforms/congress/s770/section_8.py reads it at simulation start; the # s770 tax variables read it in formulas. - "gov.contrib.congress.s770.section_8", + "gov.contrib.congress.s770.section_8": ( + "active_business.deny_nol", + "active_business.in_effect", + "active_business.oasdi_share", + "active_business.rate", + "active_business.start_year", + "nii_addon.in_effect", + "nii_addon.oasdi_share", + "nii_addon.rate", + "nii_addon.start_year", + "transfer.bill_share", + "transfer.mode", + "transfer.oasdi_rate", + "transfer.start_year", + ), # The social_security_*_payable_factor and across-the-board variables, # the COLA means-test reform and the reported-benefit PIA ratios. - "gov.contrib.ssa.benefit_levers", + "gov.contrib.ssa.benefit_levers": ( + "across_the_board.rate", + "across_the_board.start_year", + "cola.alternative_rate", + "cola.effective_month", + "cola.effective_year", + "cola.end_year", + "cola.means_test.below_threshold_cola_index", + "cola.means_test.effective_month", + "cola.means_test.effective_year", + "cola.means_test.enabled", + "cola.means_test.filing_groups.HEAD_OF_HOUSEHOLD", + "cola.means_test.filing_groups.JOINT", + "cola.means_test.filing_groups.SEPARATE", + "cola.means_test.filing_groups.SINGLE", + "cola.means_test.filing_groups.SURVIVING_SPOUSE", + "cola.means_test.first_eligible_year", + "cola.means_test.income_sensitivity", + "cola.means_test.pre_receipt_cola_index", + "cola.means_test.scope", + "cola.means_test.thresholds.base_year", + "cola.means_test.thresholds.index_lag", + "cola.means_test.thresholds.joint", + "cola.means_test.thresholds.rounding", + "cola.means_test.thresholds.single", + "cola.pp_change", + "cola.start_year", + "cola.withholding_enabled", + "payable.oasi_only", + "payable.share", + "pia_ratio.annual_rounding_bound", + "pia_ratio.claiming_scenario", + "pia_ratio.disability_eligibility_proxy", + ), # reforms/ssa/owners_draw.py and s770_active_business_income; system.py # writes current_law_cap into it at every system build. - "gov.contrib.ssa.owners_draw", + "gov.contrib.ssa.owners_draw": ( + "benefit_credit", + "current_law_cap", + "half_seca_deduction", + "in_effect", + "include_hi", + "material_participation_only", + "net_earnings_factor", + "phase_in_share", + "phase_in_years", + "room_cap_basis", + "start_year", + ), # tools/social_security/population.py: reported-benefit PIA ratios. - "gov.contrib.ssa.pia", -) + "gov.contrib.ssa.pia": ( + "bend_point_multipliers.first", + "bend_point_multipliers.second", + "effective_year", + "factors.first", + "factors.second", + "factors.start_year", + "factors.third", + "full_price_indexing.in_effect", + "full_price_indexing.real_wage_scale", + "full_price_indexing.start_year", + "in_effect", + "progressive_price_indexing.bend_point_fraction", + "progressive_price_indexing.in_effect", + "progressive_price_indexing.maximum_aime", + "progressive_price_indexing.maximum_pia_target", + "progressive_price_indexing.start_year", + "progressive_price_indexing.threshold", + "recompute_existing", + ), +} YEAR = 2026 @@ -108,44 +193,86 @@ def apply(self): def _node(root, path): + """Return the node at ``path``, or None if any part of it is missing.""" node = root for part in path.split("."): - node = node.children[part] + children = getattr(node, "children", None) + if children is None or part not in children: + return None + node = children[part] return node -def test_every_contributed_calculator_namespace_child_is_classified(): - """A new node under these namespaces must be calculator-only or kept. +def _leaves(node, prefix): + """Yield the path of every leaf parameter under ``node``, at any depth.""" + for name, child in node.children.items(): + path = f"{prefix}.{name}" + if isinstance(child, ParameterNode): + yield from _leaves(child, path) + else: + yield path + + +def _under(path, ancestor): + return path == ancestor or path.startswith(f"{ancestor}.") - If an unlisted calculator-only node stayed in the reference, a legacy + +def test_every_contributed_calculator_namespace_leaf_is_classified(): + """A new leaf under these namespaces must be calculator-only or kept. + + If an unlisted calculator-only leaf stayed in the reference, a legacy formula that read it by accident would compute the same in both models, - and the differential below could not detect the dependency. + and the differential below could not detect the dependency. The walk goes + to every depth, so a calculator-only leaf added under a kept node fails + here too. Every listed path must also exist, because + ``WithoutHouseholdCalculator.apply`` silently skips a missing one. """ parameters = Simulation.default_tax_benefit_system_instance.parameters - classified = NEW_PARAMETER_NODES + REFERENCE_PARAMETER_NODES + classified = NEW_PARAMETER_NODES + tuple(REFERENCE_PARAMETER_NODES) problems = [ f"{path} is listed {classified.count(path)} times" for path in sorted(set(classified)) if classified.count(path) > 1 ] - children = set() - for namespace in CLASSIFIED_CONTRIB_NAMESPACES: - for name in _node(parameters, namespace).children: - path = f"{namespace}.{name}" - children.add(path) - if path not in classified: - problems.append( - f"{path} is unclassified: add it to NEW_PARAMETER_NODES if " - "only the household calculator reads it, or to " - "REFERENCE_PARAMETER_NODES with a comment naming the " - "legacy formula, reform start-up or system build that " - "reads it" - ) problems += [ - f"{path} is listed in REFERENCE_PARAMETER_NODES but does not exist" - for path in REFERENCE_PARAMETER_NODES - if path not in children + f"{path} is listed in NEW_PARAMETER_NODES but does not exist" + for path in NEW_PARAMETER_NODES + if _node(parameters, path) is None ] + kept_leaves = set() + for kept, leaves in REFERENCE_PARAMETER_NODES.items(): + if _node(parameters, kept) is None: + problems.append( + f"{kept} is listed in REFERENCE_PARAMETER_NODES but does not exist" + ) + for leaf in leaves: + path = f"{kept}.{leaf}" + if path in kept_leaves: + problems.append(f"{path} is listed more than once") + kept_leaves.add(path) + if _node(parameters, path) is None: + problems.append( + f"{path} is listed under {kept} in " + "REFERENCE_PARAMETER_NODES but does not exist" + ) + for namespace in CLASSIFIED_CONTRIB_NAMESPACES: + for path in _leaves(_node(parameters, namespace), namespace): + if any(_under(path, new) for new in NEW_PARAMETER_NODES): + continue + if path in kept_leaves: + continue + kept = [node for node in REFERENCE_PARAMETER_NODES if _under(path, node)] + where = ( + f"under {kept[0]} in REFERENCE_PARAMETER_NODES if the code " + "named in that node's comment reads it" + if kept + else "to REFERENCE_PARAMETER_NODES with a comment naming the " + "legacy formula, reform start-up or system build that reads it" + ) + problems.append( + f"{path} is unclassified: add it to NEW_PARAMETER_NODES if " + f"only the household calculator reads it, or list it {where}" + ) assert not problems, "\n".join(problems) diff --git a/policyengine_us/tools/documentation.py b/policyengine_us/tools/documentation.py index 6f2b9ba463e..25d4ecbd24b 100644 --- a/policyengine_us/tools/documentation.py +++ b/policyengine_us/tools/documentation.py @@ -6,13 +6,16 @@ warnings.filterwarnings("ignore") from policyengine_core.enums import Enum -from policyengine_core.charts import ( - LIGHTER_BLUE, - LIGHT_BLUE, - BLUE, - DARK_BLUE, - format_fig, - display_fig, +from policyengine_core.charts import format_fig, display_fig + +# Core's chart palette has no LIGHTER_BLUE, LIGHT_BLUE or DARK_BLUE, and +# the package exports only BLUE and the three-shade BLUE_COLOUR_SCALE, so +# take four blues, light to dark, from the module that defines them. +from policyengine_core.charts.formatting import ( + BLUE_LIGHT, + BLUE_PRIMARY, + BLUE_PRESSED, + DARKEST_BLUE, ) from policyengine_us.system import system from policyengine_us import Simulation @@ -117,7 +120,12 @@ def variation_chart( color="Children", facet_col="Adults", facet_col_spacing=0.1, - color_discrete_sequence=[LIGHTER_BLUE, LIGHT_BLUE, BLUE, DARK_BLUE], + color_discrete_sequence=[ + BLUE_LIGHT, + BLUE_PRIMARY, + BLUE_PRESSED, + DARKEST_BLUE, + ], ) fig.update_xaxes( tickformat="$,.0f",