Skip to content

Commit c6d672e

Browse files
igerberclaude
andcommitted
Address thirtieth round of CI review findings on PR #318
P1 methodology (self-contradictory anticipation prose). ``_apply_anticipation_to_assumption`` previously only APPENDED an anticipation-aware clause. Several base assumption descriptions in ``_describe_assumption`` hard-code a strict "plus no anticipation" (CS / SA / Imputation / TwoStage / Wooldridge generic, StackedDiD sub-experiment, dCDH, TripleDifference, SyntheticDiD, TROP, ContinuousDiD) or "Also assumes no anticipation (Assumption NA) ..." (EfficientDiD PT-All, PT-Post) clause. On an anticipation-enabled fit BR would render both in the same paragraph, contradicting REGISTRY.md's description of anticipation as a SHIFTED effective treatment boundary rather than strict no-anticipation plus an exception. Add ``_strip_strict_no_anticipation`` that removes any of the canonical strict phrasings from a description before the helper appends the relaxed clause. Collapses dangling punctuation and doubled whitespace left by the removal so the rewritten description reads cleanly. The helper still flips ``no_anticipation = False`` and records ``anticipation_periods`` on the block. P3 docs drift. Round-29 added ``power_reason`` alongside ``power_status`` and updated one REPORTING.md reference, but a second reference at line 142 still pointed at ``power_status`` for the fallback explanation. Updated to name ``power_reason`` and note that ``power_status`` carries the enum. Tests: ``TestAnticipationStripsStrictNoAnticipationClause`` with five regressions asserts that every anticipation-capable estimator's rendered description under ``anticipation > 0`` drops both "plus no anticipation" and "Also assumes no anticipation" AND still carries the "Anticipation is allowed / not strict no-anticipation" contract: generic group-time, EfficientDiD PT-All, EfficientDiD PT-Post, StackedDiD sub-experiment, and an integration check of ``full_report()``'s rendered Identifying Assumption section. 249 BR / DR / practitioner tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 59d7df7 commit c6d672e

3 files changed

Lines changed: 148 additions & 5 deletions

File tree

diff_diff/business_report.py

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636

3737
from __future__ import annotations
3838

39+
import re
3940
from dataclasses import dataclass
4041
from typing import Any, Dict, List, Optional, Union
4142

@@ -822,17 +823,62 @@ def _control_group_choice(results: Any) -> Optional[str]:
822823
return None
823824

824825

826+
_STRICT_NO_ANTICIPATION_PATTERNS = (
827+
# Ordered from most specific to least specific so the first match
828+
# wins on strings that could match multiple patterns. Matches are
829+
# case-sensitive because every occurrence in ``_describe_assumption``
830+
# is a fixed canonical phrase.
831+
", plus no anticipation",
832+
"plus no anticipation",
833+
" Also assumes no anticipation (Assumption NA), overlap "
834+
"(Assumption O), and absorbing / irreversible treatment.",
835+
" Also assumes no anticipation.",
836+
"Also assumes no anticipation.",
837+
" and no anticipation",
838+
)
839+
840+
841+
def _strip_strict_no_anticipation(desc: str) -> str:
842+
"""Remove any strict no-anticipation phrasing from ``desc``.
843+
844+
Several base assumption descriptions in ``_describe_assumption``
845+
hard-code a strict "plus no anticipation" / "Also assumes no
846+
anticipation" clause (CS / SA / Imputation / TwoStage / Wooldridge
847+
generic, StackedDiD sub-experiment, EfficientDiD PT-Post, EfficientDiD
848+
PT-All, ContinuousDiD, TripleDifference, SyntheticDiD, TROP, dCDH,
849+
and the fallback unconditional branch). When a fit actually allows
850+
anticipation the helper must REPLACE that wording, not append a
851+
contradictory clause on top of it. Round-30 P1 CI review on PR #318.
852+
"""
853+
if not desc:
854+
return desc
855+
out = desc
856+
for pattern in _STRICT_NO_ANTICIPATION_PATTERNS:
857+
out = out.replace(pattern, "")
858+
# Collapse any doubled whitespace or dangling punctuation left by
859+
# the removal (e.g., "cohorts, with..." -> "cohorts, with...";
860+
# "cohorts . " -> "cohorts.").
861+
out = re.sub(r"\s+\.", ".", out)
862+
out = re.sub(r"\s+,", ",", out)
863+
out = re.sub(r" {2,}", " ", out)
864+
return out.strip()
865+
866+
825867
def _apply_anticipation_to_assumption(block: Dict[str, Any], results: Any) -> Dict[str, Any]:
826-
"""If the fit used ``anticipation > 0``, flip ``no_anticipation`` off and
827-
append an anticipation clause to the description.
868+
"""If the fit used ``anticipation > 0``, flip ``no_anticipation`` off,
869+
strip any strict no-anticipation wording from the base description,
870+
and append an anticipation-aware clause.
828871
829872
Round-17 CI review flagged the strict "plus no anticipation" language
830873
on anticipation-enabled fits. Per REGISTRY.md §CallawaySantAnna lines
831874
355-395 and the matching sections for SA / MultiPeriod / Wooldridge /
832875
EfficientDiD, a fit with ``anticipation=k`` shifts the effective
833876
treatment boundary by ``k`` pre-periods; the identifying assumption
834877
becomes "no treatment effects earlier than ``k`` periods before the
835-
treatment start" rather than strict no-anticipation.
878+
treatment start" rather than strict no-anticipation. Round-30 CI
879+
review caught that the previous implementation only appended — the
880+
resulting prose said both "strict no-anticipation holds" and
881+
"anticipation is allowed" in the same paragraph.
836882
"""
837883
k = _anticipation_periods(results)
838884
if k <= 0:
@@ -849,7 +895,7 @@ def _apply_anticipation_to_assumption(block: Dict[str, Any], results: Any) -> Di
849895
)
850896
desc = block.get("description", "")
851897
if isinstance(desc, str):
852-
block["description"] = desc + clause
898+
block["description"] = _strip_strict_no_anticipation(desc) + clause
853899
return block
854900

855901

docs/methodology/REPORTING.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,8 @@ not new inference.
139139
signal."
140140
- Power analysis not runnable &rarr; fall back to `underpowered`
141141
phrasing; the fallback reason is recorded in
142-
`schema["pre_trends"]["power_status"]`.
142+
`schema["pre_trends"]["power_reason"]` (plain-English explanation;
143+
`power_status` carries the enum).
143144

144145
Rationale: always-hedging phrasing under-sells well-designed
145146
studies; always-confident phrasing over-sells underpowered ones.

tests/test_business_report.py

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1572,6 +1572,102 @@ class Stub:
15721572
assert "beta^{het}_l" in desc
15731573

15741574

1575+
class TestAnticipationStripsStrictNoAnticipationClause:
1576+
"""Round-30 P1 CI review on PR #318: ``_apply_anticipation_to_assumption``
1577+
previously only appended an anticipation clause. Several base
1578+
descriptions already say "plus no anticipation" or "Also assumes
1579+
no anticipation", so an anticipation-enabled fit would render
1580+
self-contradictory prose: the strict clause AND the relaxed one in
1581+
the same paragraph. The helper now strips the strict phrasing
1582+
before appending. These regressions cover every anticipation-
1583+
capable estimator base description that previously carried such
1584+
wording.
1585+
"""
1586+
1587+
_STRICT_PATTERNS = (
1588+
"plus no anticipation",
1589+
"Also assumes no anticipation",
1590+
)
1591+
1592+
@staticmethod
1593+
def _stub(class_name: str, **extras):
1594+
stub_cls = type(class_name, (), {})
1595+
stub = stub_cls()
1596+
stub.overall_att = 1.0
1597+
stub.overall_se = 0.2
1598+
stub.overall_p_value = 0.001
1599+
stub.overall_conf_int = (0.6, 1.4)
1600+
stub.alpha = 0.05
1601+
stub.n_obs = 400
1602+
stub.n_treated = 100
1603+
stub.n_control = 300
1604+
stub.survey_metadata = None
1605+
stub.event_study_effects = None
1606+
stub.anticipation = 2
1607+
for k, v in extras.items():
1608+
setattr(stub, k, v)
1609+
return stub
1610+
1611+
def _assert_no_strict_contract(self, description: str):
1612+
assert isinstance(description, str) and description
1613+
for pat in self._STRICT_PATTERNS:
1614+
assert pat not in description, (
1615+
f"Anticipation-enabled fit description must not carry "
1616+
f"the strict phrase {pat!r}. Got: {description!r}"
1617+
)
1618+
# Must still say anticipation is allowed (relaxed contract).
1619+
assert "Anticipation is allowed" in description
1620+
assert "not strict no-anticipation" in description
1621+
1622+
def test_generic_group_time_strips_strict_clause(self):
1623+
# Generic CS/SA/Imputation/TwoStage/Wooldridge branch.
1624+
stub = self._stub("CallawaySantAnnaResults")
1625+
block = BusinessReport(stub, auto_diagnostics=False).to_dict()["assumption"]
1626+
assert block["no_anticipation"] is False
1627+
assert block["anticipation_periods"] == 2
1628+
self._assert_no_strict_contract(block["description"])
1629+
1630+
def test_efficient_did_pt_all_strips_strict_clause(self):
1631+
stub = self._stub("EfficientDiDResults", pt_assumption="all")
1632+
block = BusinessReport(stub, auto_diagnostics=False).to_dict()["assumption"]
1633+
self._assert_no_strict_contract(block["description"])
1634+
# PT-All identifying content should still be present.
1635+
assert "PT-All" in block["description"]
1636+
1637+
def test_efficient_did_pt_post_strips_strict_clause(self):
1638+
stub = self._stub("EfficientDiDResults", pt_assumption="post")
1639+
block = BusinessReport(stub, auto_diagnostics=False).to_dict()["assumption"]
1640+
self._assert_no_strict_contract(block["description"])
1641+
assert "PT-Post" in block["description"]
1642+
1643+
def test_stacked_did_strips_strict_clause(self):
1644+
stub = self._stub(
1645+
"StackedDiDResults", clean_control="not_yet_treated"
1646+
)
1647+
block = BusinessReport(stub, auto_diagnostics=False).to_dict()["assumption"]
1648+
self._assert_no_strict_contract(block["description"])
1649+
# Stacked sub-experiment identifying content preserved.
1650+
assert "IC1" in block["description"] and "IC2" in block["description"]
1651+
1652+
def test_rendered_full_report_has_no_strict_contract_for_anticipation(self):
1653+
"""Integration: the rendered markdown's Identifying Assumption
1654+
section must also be free of the strict phrase on an
1655+
anticipation-enabled fit.
1656+
"""
1657+
stub = self._stub("CallawaySantAnnaResults")
1658+
md = BusinessReport(stub, auto_diagnostics=False).full_report()
1659+
assumption_section = md.split("## Identifying Assumption", 1)[1].split(
1660+
"\n## ", 1
1661+
)[0]
1662+
for pat in self._STRICT_PATTERNS:
1663+
assert pat not in assumption_section, (
1664+
f"Rendered assumption section must not carry the strict "
1665+
f"phrase {pat!r} under anticipation > 0. Got: "
1666+
f"{assumption_section!r}"
1667+
)
1668+
assert "Anticipation is allowed" in assumption_section
1669+
1670+
15751671
class TestAnticipationAwareAssumptionBlock:
15761672
"""Round-17 P1 regression: ``_describe_assumption`` must drop the
15771673
strict "plus no anticipation" language when the fit allows

0 commit comments

Comments
 (0)