fix: Address review findings for economic impact consolidation

Fix DecileImpact.run() StopIteration bug, move inline imports to
top-level, improve exception logging in compute_program_statistics,
use model_construct in compute_decile_impacts, and add 11 tests
for the new economic impact output modules.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: anth-volk

changelog.d/consolidate-economic-impact.added.md

@@ -0,0 +1 @@
+Shared compute functions for economic impact analysis: CountryConfig, BudgetSummaryItem, compute_program_statistics, compute_decile_impacts, and PolicyReformAnalysis

src/policyengine/outputs/decile_impact.py

@@ -34,10 +34,15 @@ def run(self):
         """Calculate impact for this specific decile."""
         # Get variable object to determine entity
         var_obj = next(
-            v
-            for v in self.baseline_simulation.tax_benefit_model_version.variables
-            if v.name == self.income_variable
+            (
+                v
+                for v in self.baseline_simulation.tax_benefit_model_version.variables
+                if v.name == self.income_variable
+            ),
+            None,
         )
+        if var_obj is None:
+            raise ValueError(f"Variable '{self.income_variable}' not found in model")
 
         # Get target entity
         target_entity = self.entity or var_obj.entity
@@ -120,7 +125,7 @@ def compute_decile_impacts(
     """
     results = []
     for decile in range(1, quantiles + 1):
-        impact = DecileImpact(
+        impact = DecileImpact.model_construct(
             baseline_simulation=baseline_simulation,
             reform_simulation=reform_simulation,
             income_variable=income_variable,

src/policyengine/outputs/program_statistics.py

@@ -83,8 +83,8 @@ def compute_program_statistics(
             )
             stats.run()
             results.append(stats)
-        except (KeyError, ValueError):
-            logger.warning("Skipping program %s: variable not found", prog_name)
+        except (KeyError, ValueError) as exc:
+            logger.warning("Skipping program %s: %s", prog_name, exc, exc_info=True)
             continue
 
     df = pd.DataFrame(

src/policyengine/tax_benefit_models/us/analysis.py

@@ -4,6 +4,7 @@
 from pathlib import Path
 from typing import Any
 
+import numpy as np
 import pandas as pd
 from microdf import MicroDataFrame
 from pydantic import BaseModel, Field
@@ -16,7 +17,12 @@
 from policyengine.outputs.inequality import calculate_us_inequality
 from policyengine.outputs.intra_decile_impact import compute_intra_decile_impacts
 from policyengine.outputs.policy_reform_analysis import PolicyReformAnalysis
-from policyengine.outputs.poverty import calculate_us_poverty_rates
+from policyengine.outputs.poverty import (
+    calculate_us_poverty_by_age,
+    calculate_us_poverty_by_gender,
+    calculate_us_poverty_by_race,
+    calculate_us_poverty_rates,
+)
 from policyengine.outputs.program_statistics import compute_program_statistics
 
 from .datasets import PolicyEngineUSDataset, USYearData
@@ -219,8 +225,6 @@ def economic_impact_analysis(
     )
 
     # Household counts — raw weight sums to avoid MicroSeries double-weighting
-    import numpy as np
-
     hh_weight_baseline = baseline_simulation.output_dataset.data.household[
         "household_weight"
     ]
@@ -242,12 +246,6 @@ def economic_impact_analysis(
     reform_poverty = calculate_us_poverty_rates(reform_simulation)
 
     # Poverty by demographics
-    from policyengine.outputs.poverty import (
-        calculate_us_poverty_by_age,
-        calculate_us_poverty_by_gender,
-        calculate_us_poverty_by_race,
-    )
-
     baseline_poverty_by_age = calculate_us_poverty_by_age(baseline_simulation)
     reform_poverty_by_age = calculate_us_poverty_by_age(reform_simulation)
     baseline_poverty_by_gender = calculate_us_poverty_by_gender(baseline_simulation)

tests/test_economic_impact_outputs.py

@@ -0,0 +1,201 @@
+"""Tests for the new economic impact output modules."""
+
+from dataclasses import FrozenInstanceError
+from unittest.mock import MagicMock
+
+import numpy as np
+import pandas as pd
+import pytest
+from microdf import MicroDataFrame
+
+from policyengine.outputs.change_aggregate import ChangeAggregate, ChangeAggregateType
+from policyengine.outputs.country_config import UK_CONFIG, US_CONFIG
+from policyengine.outputs.decile_impact import DecileImpact, compute_decile_impacts
+
+# ---------------------------------------------------------------------------
+# Helpers (same pattern as test_intra_decile_impact.py)
+# ---------------------------------------------------------------------------
+
+
+def _make_variable_mock(name: str, entity: str) -> MagicMock:
+    """Create a mock Variable with name and entity attributes."""
+    var = MagicMock()
+    var.name = name
+    var.entity = entity
+    return var
+
+
+def _make_sim(household_data: dict, variables: list | None = None) -> MagicMock:
+    """Create a mock Simulation with household-level data."""
+    hh_df = MicroDataFrame(
+        pd.DataFrame(household_data),
+        weights="household_weight",
+    )
+    sim = MagicMock()
+    sim.output_dataset.data.household = hh_df
+    sim.id = "test-sim"
+    if variables is not None:
+        sim.tax_benefit_model_version.variables = variables
+    return sim
+
+
+# ---------------------------------------------------------------------------
+# CountryConfig tests
+# ---------------------------------------------------------------------------
+
+
+def test_us_config_is_frozen():
+    """US_CONFIG should be immutable."""
+    with pytest.raises(FrozenInstanceError):
+        US_CONFIG.country_id = "uk"
+
+
+def test_uk_config_is_frozen():
+    """UK_CONFIG should be immutable."""
+    with pytest.raises(FrozenInstanceError):
+        UK_CONFIG.country_id = "us"
+
+
+def test_us_config_has_correct_country_id():
+    assert US_CONFIG.country_id == "us"
+
+
+def test_uk_config_has_correct_country_id():
+    assert UK_CONFIG.country_id == "uk"
+
+
+def test_us_config_programs():
+    """US_CONFIG should contain expected program keys."""
+    expected = {
+        "income_tax",
+        "employee_payroll_tax",
+        "snap",
+        "tanf",
+        "ssi",
+        "social_security",
+    }
+    assert set(US_CONFIG.programs.keys()) == expected
+
+
+def test_uk_config_programs():
+    """UK_CONFIG should contain expected programme keys."""
+    expected = {
+        "income_tax",
+        "national_insurance",
+        "vat",
+        "council_tax",
+        "universal_credit",
+        "child_benefit",
+        "pension_credit",
+        "income_support",
+        "working_tax_credit",
+        "child_tax_credit",
+    }
+    assert set(UK_CONFIG.programs.keys()) == expected
+
+
+def test_country_config_program_structure():
+    """Each program entry should have 'entity' and 'is_tax' keys."""
+    for name, info in US_CONFIG.programs.items():
+        assert "entity" in info, f"US program {name} missing 'entity'"
+        assert "is_tax" in info, f"US program {name} missing 'is_tax'"
+    for name, info in UK_CONFIG.programs.items():
+        assert "entity" in info, f"UK programme {name} missing 'entity'"
+        assert "is_tax" in info, f"UK programme {name} missing 'is_tax'"
+
+
+# ---------------------------------------------------------------------------
+# DecileImpact tests
+# ---------------------------------------------------------------------------
+
+
+def test_decile_impact_variable_not_found():
+    """DecileImpact.run() should raise ValueError for a nonexistent variable."""
+    variables = [_make_variable_mock("household_net_income", "household")]
+    sim = _make_sim(
+        {"household_net_income": [50000.0], "household_weight": [1.0]},
+        variables=variables,
+    )
+
+    di = DecileImpact.model_construct(
+        baseline_simulation=sim,
+        reform_simulation=sim,
+        income_variable="nonexistent_variable",
+        entity="household",
+        decile=1,
+    )
+    with pytest.raises(ValueError, match="not found in model"):
+        di.run()
+
+
+def test_compute_decile_impacts_returns_10():
+    """compute_decile_impacts should return 10 DecileImpact objects by default."""
+    n = 100
+    incomes = np.linspace(10000, 100000, n)
+    reform_incomes = incomes + 500
+    variables = [_make_variable_mock("household_net_income", "household")]
+
+    baseline = _make_sim(
+        {"household_net_income": incomes, "household_weight": np.ones(n)},
+        variables=variables,
+    )
+    reform = _make_sim(
+        {"household_net_income": reform_incomes, "household_weight": np.ones(n)},
+        variables=variables,
+    )
+
+    result = compute_decile_impacts(
+        baseline, reform, income_variable="household_net_income", entity="household"
+    )
+
+    assert len(result.outputs) == 10
+    assert len(result.dataframe) == 10
+
+    # Each decile should have absolute_change ~500
+    for di in result.outputs:
+        assert abs(di.absolute_change - 500.0) < 1e-6
+
+
+def test_compute_decile_impacts_custom_quantiles():
+    """compute_decile_impacts with quantiles=5 should return 5 outputs."""
+    n = 100
+    incomes = np.linspace(10000, 100000, n)
+    variables = [_make_variable_mock("household_net_income", "household")]
+
+    sim = _make_sim(
+        {"household_net_income": incomes, "household_weight": np.ones(n)},
+        variables=variables,
+    )
+
+    result = compute_decile_impacts(
+        sim,
+        sim,
+        income_variable="household_net_income",
+        entity="household",
+        quantiles=5,
+    )
+
+    assert len(result.outputs) == 5
+
+
+# ---------------------------------------------------------------------------
+# ChangeAggregate error test
+# ---------------------------------------------------------------------------
+
+
+def test_change_aggregate_variable_not_found():
+    """ChangeAggregate should raise ValueError for a nonexistent variable."""
+    variables = [_make_variable_mock("employment_income", "person")]
+    sim = _make_sim(
+        {"household_net_income": [50000.0], "household_weight": [1.0]},
+        variables=variables,
+    )
+
+    ca = ChangeAggregate.model_construct(
+        baseline_simulation=sim,
+        reform_simulation=sim,
+        variable="nonexistent_variable",
+        aggregate_type=ChangeAggregateType.COUNT,
+    )
+    with pytest.raises(ValueError, match="not found in model"):
+        ca.run()