feat(v4): complete event-study df provenance - SunAbraham/dCDH event_study_df, LPDiD pooled_df (M-092 follow-up) - #726
Conversation
…study_df, LPDiD pooled_df (M-092 follow-up)
Overall Assessment✅ Looks good — no unmitigated P0/P1 findings. Executive Summary
Methodology
Cross-checks found no estimator, weighting, SE, identification, or undocumented-methodology mismatch:
Code QualityNo findings. New inference assignments consistently use PerformanceNo findings. Added work is linear in the small number of event-study horizons. Maintainability
Tech DebtNo findings. The completed M-092 follow-up is appropriately removed from SecurityNo findings. No secrets, unsafe input handling, or security-sensitive changes were identified. Documentation / TestsNo findings. Coverage includes BM df, survey and replicate modes, bootstrap clearing, non-finite df propagation, dCDH Phase-1 synchronization, placebos, and LPDiD serialization. Runtime tests could not be independently executed because the review environment lacks pytest and project dependencies; all affected Python files passed static AST parsing. |
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment✅ Looks good — no unmitigated P0/P1 findings. Executive Summary
Methodology
Cross-checks found no undocumented changes to estimator equations, weighting, SEs, or identification assumptions:
Code QualityNo findings. New inference paths use PerformanceNo findings. The added work is constant-time metadata capture or linear in event-study horizons. MaintainabilityNo findings. The previous positional-constructor issue is resolved by declaring the new fields last:
Tech DebtNo findings. The completed M-092 follow-up is appropriately removed from SecurityNo findings. No secrets or security-sensitive behavior were introduced. Documentation / TestsNo findings. Tests cover BM, survey, replicate, bootstrap, non-finite df, dCDH Phase-1 synchronization, placebos, and LPDiD serialization. Runtime tests could not be executed because NumPy and pytest are unavailable; all ten affected Python files passed AST parsing. |
Summary
Completes the TODO.md follow-up from #715: every event-study producer whose inference records a degrees-of-freedom value now exposes it, so
EventStudyResults.dfis no longer NaN-by-omission for any producer.event_study_df(per-relative-time dict). SA computes its event-study inference at four sites, so the record is assembled at fit level: a baseline from_compute_iw_effects' scalar, overwritten per-event by thehc2_bmBell-McCaffrey contrast DOFs, overwritten again by the replicate refit's post-drop survey df, and cleared under bootstrap. That clear is deliberately narrower than the existingevent_study_vcovclear (which also fires under replicate refits): a replicate refit's rows genuinely used the replicate df, so keeping it is faithful provenance, whereas bootstrap percentile p/CIs never used a df at all.event_study_df+ adapter threading. A scalar is faithful because its effect and placebo rows are computed from the same design df in every mode (None analytically, the design df under Taylor linearization, the final effective df under replicate weights, where a late refresh re-runs every stored row to one value). The bootstrap clear is required rather than defensive: TSL survey + bootstrap is a live mode (only replicate weights + bootstrap is rejected), where the df expression evaluates finite while the stored p-values are percentiles.pooled_df({"pre", "post"}) — the pre/post window dfs that fit assembly previously popped and discarded. The pooled window is the headline ATT, outside the unified event-study surface, so this follows theDiDResults.inference_dfheadline-provenance precedent; surfaced into_dict(), native pooled frame schema unchanged.L_max=Nonethe single event-study row is a value copy of the overall inference taken before the final-df refresh, and the refresh's per-horizon loops cover only the Phase 2 surface — so the copy could in principle retain an intermediate df's t/p/CI whilesurvey_metadata.df_surveyreported the final one. It is now mirrored from the refreshed values. I could not construct a configuration where this changes output: in every case reachable (standard replicate fixture, leavers panel, rank-deficient designs) the contributing sites agree on their valid-replicate counts so the two dfs coincide, and heterogeneity — which the existing suite uses to force df drift — requiresL_max>=1and cannot reach the Phase 1 path. The tests are therefore written as invariant pins, and the CHANGELOG entry is filed under Changed with the non-reproducibility stated, not as a Fixed entry that would overclaim.Methodology references (required if estimator / math changes)
safe_inference.**Note:**entries under SunAbraham and ChaisemartinDHaultfoeuille document the provenance semantics per mode.safe_inference's own guard — both are faithfully recorded (the latter as NaN provenance).Validation
tests/test_event_study_surface.py(SA hc2_bm per-row df, survey broadcast, replicate-df-kept-while-vcov-cleared, bootstrap clear; dCDH analytic/TSL-with-placebos/TSL+bootstrap; LPDiD pooled_df round-trip),tests/test_sun_abraham.py(NaN-BM-DOF regression pin + the helper-failure fallback that distinguishes it),tests/test_survey_dcdh_replicate_psu.py(Phase 1 invariant pins).Security / privacy