Skip to content

Commit dae62ac

Browse files
igerberclaude
andcommitted
Address twenty-fourth round of CI review findings on PR #318
P2 code quality (SDiD jackknife step-tag bug). The SyntheticDiD practitioner step "Leave-one-out influence (jackknife)" was tagged ``_step_name="sensitivity"``. DR's SDiD native battery covers pre-treatment fit, weight concentration, ``in_time_placebo``, and ``sensitivity_to_zeta_omega`` — but not the jackknife LOO workflow, which requires a separate ``variance_method='jackknife'`` fit before ``get_loo_effects_df`` returns anything. As soon as the native block ran, ``_collect_next_steps`` marked ``"sensitivity"`` complete and suppressed the jackknife recommendation, overstating what the report had actually executed. Same class as round-20 Hausman (``heterogeneity`` -> ``parallel_trends``) and the pre-emptive TROP-placebo retag. Retag the step as ``_step_name="loo_jackknife"`` so it persists regardless of which DR blocks ran. No DR check maps to the new tag — the step stays in ``next_steps`` until the user completes it explicitly. P3 coverage. Add the two regressions the reviewer specified: * ``test_sdid_jackknife_step_persists_via_practitioner_filter`` (unit-level) asserts ``practitioner_next_steps(sdid_stub, completed_steps=["sensitivity"])`` still surfaces the jackknife label; * ``test_sdid_jackknife_step_persists_in_dr_next_steps`` (integration) asserts ``DiagnosticReport(sdid).to_dict() ["next_steps"]`` preserves the recommendation when only the default native SDiD diagnostics ran. 227 BR / DR / practitioner tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 61270b8 commit dae62ac

2 files changed

Lines changed: 78 additions & 1 deletion

File tree

diff_diff/practitioner.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,16 @@ def _handle_synthetic(results: Any):
570570
" 'with positive effective support.')"
571571
),
572572
priority="medium",
573-
step_name="sensitivity",
573+
# DR's SyntheticDiD native battery covers pre-treatment fit,
574+
# weight concentration, in-time placebo, and zeta-omega
575+
# sensitivity, but NOT the jackknife LOO workflow (which
576+
# requires a separate ``variance_method='jackknife'`` fit
577+
# via ``get_loo_effects_df``). Tagging this recommendation
578+
# as ``sensitivity`` caused ``_collect_next_steps`` to
579+
# suppress it as soon as the native block ran, even though
580+
# the jackknife was never executed. Round-24 P2 CI review
581+
# on PR #318; same class as round-20 Hausman mistag.
582+
step_name="loo_jackknife",
574583
),
575584
_step(
576585
baker_step=6,

tests/test_business_report.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2290,6 +2290,74 @@ def test_efficient_compare_control_groups_persists_after_sensitivity_runs(self):
22902290
)
22912291

22922292

2293+
class TestSDiDJackknifeStepPersistsAfterNativeSensitivity:
2294+
"""Round-24 P2 CI review on PR #318: the SyntheticDiD practitioner
2295+
step "Leave-one-out influence (jackknife)" must persist after
2296+
``DiagnosticReport`` marks ``sensitivity`` complete via the SDiD
2297+
native battery (pre-treatment fit, weight concentration,
2298+
``in_time_placebo``, ``sensitivity_to_zeta_omega``). DR does NOT
2299+
run the jackknife LOO workflow — ``get_loo_effects_df`` requires a
2300+
separate ``variance_method='jackknife'`` fit — so suppressing the
2301+
recommendation when the native block fires overstates what the
2302+
report has already executed. Same class as round-20 Hausman and
2303+
pre-emptive TROP-placebo retags: step_name was coarser than DR's
2304+
actual coverage.
2305+
"""
2306+
2307+
def test_sdid_jackknife_step_persists_via_practitioner_filter(self):
2308+
"""Unit-level: ``practitioner_next_steps`` with
2309+
``completed_steps=["sensitivity"]`` still surfaces the jackknife
2310+
recommendation because it is now tagged ``loo_jackknife``.
2311+
"""
2312+
from diff_diff.practitioner import practitioner_next_steps
2313+
2314+
class SyntheticDiDResults:
2315+
pass
2316+
2317+
stub = SyntheticDiDResults()
2318+
stub.att = 1.0
2319+
stub.se = 0.2
2320+
stub.p_value = 0.001
2321+
stub.conf_int = (0.6, 1.4)
2322+
stub.alpha = 0.05
2323+
stub.n_obs = 200
2324+
stub.n_treated = 20
2325+
stub.n_control = 180
2326+
stub.survey_metadata = None
2327+
stub.event_study_effects = None
2328+
2329+
labels = [
2330+
s.get("label", "")
2331+
for s in practitioner_next_steps(
2332+
stub, completed_steps=["sensitivity"], verbose=False
2333+
)["next_steps"]
2334+
]
2335+
assert any(
2336+
"Leave-one-out influence (jackknife)" in lab for lab in labels
2337+
), (
2338+
"SDiD jackknife recommendation must persist after DR marks "
2339+
"sensitivity complete — the SDiD native battery does not run "
2340+
"the jackknife LOO workflow (requires a separate "
2341+
"variance_method='jackknife' fit)."
2342+
)
2343+
2344+
def test_sdid_jackknife_step_persists_in_dr_next_steps(self, sdid_fit):
2345+
"""Integration: ``DiagnosticReport(...).to_dict()["next_steps"]``
2346+
preserves the jackknife recommendation when only the default
2347+
native SDiD diagnostics ran.
2348+
"""
2349+
from diff_diff import DiagnosticReport
2350+
2351+
fit, _ = sdid_fit
2352+
next_steps = DiagnosticReport(fit).to_dict()["next_steps"]
2353+
labels = [s.get("label", "") for s in next_steps]
2354+
assert any("Leave-one-out influence (jackknife)" in lab for lab in labels), (
2355+
"DR next_steps must preserve the SDiD jackknife recommendation "
2356+
"when the SDiD native battery ran but the jackknife workflow "
2357+
f"did not. Got labels: {labels}"
2358+
)
2359+
2360+
22932361
class TestTROPInTimePlaceboStepTaggedAsPlacebo:
22942362
"""Pre-emptive audit regression: the TROP practitioner workflow
22952363
step "In-time or in-space placebo" was previously tagged

0 commit comments

Comments
 (0)