Address forty-fifth round of CI review findings on PR #318

Round-45 landed one P1 methodology finding: ``BusinessReport`` emitted
the Bacon "re-estimate with a heterogeneity-robust estimator (CS / SA
/ BJS / Gardner)" caveat on every fit whose Bacon block had
``forbidden_weight > 0.10``, including fits that were already produced
by one of those robust estimators.

Goodman-Bacon is explicitly a decomposition of TWFE weights
(``bacon.py`` header, Goodman-Bacon 2021). On a displayed fit that is
already heterogeneity-robust (CS / SA / BJS / Gardner / Wooldridge /
EfficientDiD / Stacked / dCDH / TripleDifference /
StaggeredTripleDiff / SDiD / TROP), a high forbidden-weight share is a
statement about what TWFE WOULD have done on this rollout, not a claim
that the displayed estimator needs replacement. DR partly preserved
this in its prose with an "if not already in use" guard; BR dropped
that distinction and rendered the stronger recommendation in
stakeholder-facing caveats / full reports.

Fix (``business_report.py`` ``_build_caveats``):

- Introduce ``_TWFE_STYLE_RESULTS = {DiDResults, MultiPeriodDiDResults,
  TwoWayFixedEffectsResults}`` — the fits for which the switch-to-
  robust recommendation is load-bearing.
- Keep the original message for TWFE-style fits.
- Rephrase for already-robust fits: "TWFE benchmark would be
  materially biased on this rollout; the displayed estimator is
  already heterogeneity-robust, so this is a statement about the
  rollout design (avoid reporting TWFE alongside this fit), not about
  the current result's validity."

Tests: 3 new regressions in ``TestBaconCaveatEstimatorAware``:
- CS-like fit with high forbidden-weight does NOT recommend switching.
- Spot-check the same rule across SA / Imputation / TwoStage / Stacked
  / Wooldridge / dCDH / EfficientDiD.
- ``MultiPeriodDiDResults`` (TWFE event-study) DOES keep the switch
  recommendation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: igerber

diff_diff/business_report.py

@@ -38,7 +38,7 @@
 
 import re
 from dataclasses import dataclass
-from typing import Any, Dict, List, Optional, Union
+from typing import Any, Dict, FrozenSet, List, Optional, Union
 
 import numpy as np
 
@@ -1521,23 +1521,56 @@ def _build_caveats(
                 )
 
     # Bacon forbidden comparisons.
+    # Round-45 P1 CI review on PR #318: Goodman-Bacon is a
+    # decomposition of TWFE weights (see ``bacon.py`` header and
+    # Goodman-Bacon 2021). On fits already produced by a
+    # heterogeneity-robust estimator (CS / SA / BJS / Gardner /
+    # Wooldridge / EfficientDiD / Stacked / dCDH / TripleDifference /
+    # StaggeredTripleDiff / SDiD / TROP), a high forbidden-weight share
+    # says "TWFE would have been materially biased on this rollout",
+    # not "the displayed estimator needs to be replaced" — the
+    # displayed estimator is already robust to the heterogeneity that
+    # Bacon flags. DR partly preserves this with "if not already in
+    # use" prose; BR must carry the same distinction through to the
+    # caveat. The TWFE-style estimators whose results route through
+    # Bacon and for which the "switch to a robust estimator"
+    # recommendation is load-bearing are the DiDResults-type fits; all
+    # other result classes are already robust.
+    _TWFE_STYLE_RESULTS: FrozenSet[str] = frozenset(
+        {"DiDResults", "MultiPeriodDiDResults", "TwoWayFixedEffectsResults"}
+    )
     if dr_schema:
         bacon = dr_schema.get("bacon") or {}
         if bacon.get("status") == "ran":
             fw = bacon.get("forbidden_weight")
             if isinstance(fw, (int, float)) and fw > 0.10:
+                _estimator_name = type(_results).__name__
+                if _estimator_name in _TWFE_STYLE_RESULTS:
+                    bacon_message = (
+                        f"Goodman-Bacon decomposition places {fw:.0%} "
+                        "of implicit TWFE weight on 'forbidden' "
+                        "later-vs-earlier comparisons. TWFE may be "
+                        "materially biased under heterogeneous effects. "
+                        "Re-estimate with a heterogeneity-robust "
+                        "estimator (CS / SA / BJS / Gardner)."
+                    )
+                else:
+                    bacon_message = (
+                        f"Goodman-Bacon decomposition places {fw:.0%} "
+                        "of TWFE weight on 'forbidden' later-vs-earlier "
+                        "comparisons. A TWFE benchmark on this rollout "
+                        "would be materially biased under heterogeneous "
+                        "effects; the displayed estimator is already "
+                        "heterogeneity-robust, so this is a statement "
+                        "about the rollout design (avoid reporting TWFE "
+                        "alongside this fit), not about the current "
+                        "result's validity."
+                    )
                 caveats.append(
                     {
                         "severity": "warning",
                         "topic": "bacon_contamination",
-                        "message": (
-                            f"Goodman-Bacon decomposition places {fw:.0%} "
-                            "of implicit TWFE weight on 'forbidden' "
-                            "later-vs-earlier comparisons. TWFE may be "
-                            "materially biased under heterogeneous effects. "
-                            "Re-estimate with a heterogeneity-robust "
-                            "estimator (CS / SA / BJS / Gardner)."
-                        ),
+                        "message": bacon_message,
                     }
                 )
 

tests/test_business_report.py

@@ -3975,6 +3975,167 @@ def test_br_rejects_both_passthrough_inputs_names_them(self):
         assert "precomputed['sensitivity']" in msg
 
 
+class TestBaconCaveatEstimatorAware:
+    """Round-45 P1 CI review on PR #318: Goodman-Bacon decomposes TWFE
+    weights. On fits already produced by a heterogeneity-robust
+    estimator (CS / SA / BJS / Gardner / Wooldridge / EfficientDiD /
+    Stacked / dCDH / TripleDifference / StaggeredTripleDiff / SDiD /
+    TROP), a high forbidden-weight share says "TWFE would have been
+    materially biased on this rollout", not "the displayed estimator
+    needs to be replaced" — the displayed fit is already robust.
+
+    BR's caveat must be estimator-aware: keep the "switch to a robust
+    estimator" recommendation for TWFE-style fits only.
+    """
+
+    @staticmethod
+    def _bacon_schema_with_high_forbidden_weight():
+        """Build a fake ``DiagnosticReportResults`` whose schema carries
+        a Bacon block with ``forbidden_weight > 0.10`` so BR's caveat
+        builder fires on the Bacon branch."""
+        from diff_diff.diagnostic_report import DiagnosticReportResults
+
+        schema = {
+            "schema_version": "1.0",
+            "estimator": {"class_name": "Stub", "display_name": "Stub"},
+            "headline_metric": {},
+            "parallel_trends": {"status": "skipped", "reason": "stub"},
+            "pretrends_power": {"status": "skipped", "reason": "stub"},
+            "sensitivity": {"status": "skipped", "reason": "stub"},
+            "placebo": {"status": "skipped", "reason": "stub"},
+            "bacon": {
+                "status": "ran",
+                "twfe_estimate": 1.2,
+                "weight_by_type": {
+                    "treated_vs_never": 0.5,
+                    "earlier_vs_later": 0.1,
+                    "later_vs_earlier": 0.4,
+                },
+                "forbidden_weight": 0.4,  # > 0.10 threshold
+                "verdict": "materially_contaminated",
+                "n_timing_groups": 3,
+            },
+            "design_effect": {"status": "skipped", "reason": "stub"},
+            "heterogeneity": {"status": "skipped", "reason": "stub"},
+            "epv": {"status": "skipped", "reason": "stub"},
+            "estimator_native_diagnostics": {"status": "not_applicable"},
+            "skipped": {},
+            "warnings": [],
+            "overall_interpretation": "",
+            "next_steps": [],
+        }
+        return DiagnosticReportResults(
+            schema=schema,
+            interpretation="",
+            applicable_checks=("bacon",),
+            skipped_checks={},
+            warnings=(),
+        )
+
+    @staticmethod
+    def _make_cs_like_stub(class_name: str):
+        cls = type(class_name, (), {})
+        obj = cls()
+        obj.overall_att = 1.0
+        obj.overall_se = 0.2
+        obj.overall_p_value = 0.001
+        obj.overall_conf_int = (0.6, 1.4)
+        obj.alpha = 0.05
+        obj.n_obs = 500
+        obj.n_treated = 100
+        obj.n_control_units = 400
+        obj.survey_metadata = None
+        obj.event_study_effects = None
+        obj.inference_method = "analytical"
+        return obj
+
+    @staticmethod
+    def _make_twfe_style_stub():
+        class MultiPeriodDiDResults:
+            pass
+
+        obj = MultiPeriodDiDResults()
+        obj.avg_att = 1.0
+        obj.avg_se = 0.2
+        obj.avg_p_value = 0.001
+        obj.avg_conf_int = (0.6, 1.4)
+        obj.alpha = 0.05
+        obj.n_obs = 500
+        obj.n_treated = 100
+        obj.n_control = 400
+        obj.survey_metadata = None
+        obj.pre_period_effects = None
+        obj.inference_method = "analytical"
+        return obj
+
+    def test_cs_like_fit_does_not_recommend_switching_estimators(self):
+        """On an already-robust CS-style fit with high forbidden
+        Bacon weight, BR must not recommend switching to a robust
+        estimator — the displayed fit IS already robust."""
+        stub = self._make_cs_like_stub("CallawaySantAnnaResults")
+        dr = self._bacon_schema_with_high_forbidden_weight()
+        br = BusinessReport(stub, diagnostics=dr)
+        caveats = br.caveats()
+        bacon_caveats = [c for c in caveats if c.get("topic") == "bacon_contamination"]
+        assert len(bacon_caveats) == 1, (
+            f"High forbidden-weight Bacon must surface a caveat. " f"Got caveats: {caveats!r}"
+        )
+        msg = bacon_caveats[0]["message"].lower()
+        # Must NOT tell the user to switch estimators.
+        assert "re-estimate with a heterogeneity-robust" not in msg, (
+            f"CS is already heterogeneity-robust; must not recommend "
+            f"switching. Got message: {msg!r}"
+        )
+        # Must frame this as a TWFE benchmark problem / rollout-design
+        # statement, not a displayed-fit-validity problem.
+        assert (
+            "heterogeneity-robust" in msg
+            or "already" in msg
+            or "twfe benchmark" in msg
+            or "rollout design" in msg
+        ), f"CS Bacon caveat must reframe as rollout/TWFE issue. Got: {msg!r}"
+        # And the full-report rendering must reflect the softer wording.
+        md = br.full_report()
+        assert "Re-estimate with a heterogeneity-robust estimator" not in md, md
+
+    def test_other_robust_estimators_also_avoid_switch_recommendation(self):
+        """Spot-check the same rule holds for multiple
+        heterogeneity-robust estimators on the Bacon path."""
+        for class_name in (
+            "SunAbrahamResults",
+            "ImputationDiDResults",
+            "TwoStageDiDResults",
+            "StackedDiDResults",
+            "WooldridgeDiDResults",
+            "ChaisemartinDHaultfoeuilleResults",
+            "EfficientDiDResults",
+        ):
+            stub = self._make_cs_like_stub(class_name)
+            dr = self._bacon_schema_with_high_forbidden_weight()
+            br = BusinessReport(stub, diagnostics=dr)
+            msgs = [c["message"] for c in br.caveats() if c.get("topic") == "bacon_contamination"]
+            assert msgs, f"{class_name}: Bacon caveat must fire"
+            assert (
+                "Re-estimate with a heterogeneity-robust estimator" not in msgs[0]
+            ), f"{class_name} is already robust; must not recommend switching. Got: {msgs[0]!r}"
+
+    def test_twfe_style_fit_keeps_switch_recommendation(self):
+        """The switch-to-robust recommendation is load-bearing for
+        genuinely TWFE-style fits and must be preserved there."""
+        stub = self._make_twfe_style_stub()
+        dr = self._bacon_schema_with_high_forbidden_weight()
+        br = BusinessReport(stub, diagnostics=dr)
+        caveats = br.caveats()
+        bacon_caveats = [c for c in caveats if c.get("topic") == "bacon_contamination"]
+        assert len(bacon_caveats) == 1
+        msg = bacon_caveats[0]["message"]
+        # TWFE-style fit: keep the explicit switch recommendation.
+        assert "Re-estimate with a heterogeneity-robust estimator" in msg, (
+            f"MultiPeriodDiDResults (TWFE event-study) must keep the "
+            f"switch-to-robust recommendation. Got: {msg!r}"
+        )
+
+
 class TestBusinessReportSurveyDesignPassthrough:
     """Round-40 P1 CI review on PR #318: ``BusinessReport`` must accept
     ``survey_design`` and forward it to the auto-constructed