Skip to content

feat(v4): EventStudyResults full-vcov + per-row df round-trip (M-092 follow-up)#715

Merged
igerber merged 2 commits into
mainfrom
feat/v4-esr-vcov-df-roundtrip
Jul 20, 2026
Merged

feat(v4): EventStudyResults full-vcov + per-row df round-trip (M-092 follow-up)#715
igerber merged 2 commits into
mainfrom
feat/v4-esr-vcov-df-roundtrip

Conversation

@igerber

@igerber igerber commented Jul 20, 2026

Copy link
Copy Markdown
Owner

Summary

  • StackedDiD / TwoStageDiD event-study VCV persistence: both estimators already computed a full event-study covariance internally (pooled-regression sub-block / Gardner-GMM matrix) and discarded it after reading the diagonal. New event_study_vcov / event_study_vcov_index container fields persist it (mirroring the CallawaySantAnna/SunAbraham fields), and the unified surface picks it up with zero adapter change. StackedDiD persists in every inference mode (its reported ES SEs are always the matrix diagonal); TwoStageDiD persists on analytical paths and ships None under bootstrap (percentile inference, no covariance) and replicate-weight designs (mixed-layout replicate VCV - CS precedent). The reference period and Proposition-5 horizons are never regression columns and are excluded from the index; all-filtered horizons keep their NaN-filled rank-guard rows, consistent with their NaN marginal SEs.
  • EventStudyResults.df becomes PER-ROW and joins the pinned to_dataframe() schema (between n and is_reference). This is an unreleased-schema change made deliberately before aggregate(type="event_study") exposes the surface publicly: three producers genuinely need per-row df (StackedDiD hc2_bm per-event Bell-McCaffrey Satterthwaite df; MultiPeriodDiD hc2_bm per-period BM df - inference_df stores only the post-average contrast df; LPDiD per-horizon realized cluster df). Scalars broadcast; NaN means normal-theory, undefined, bootstrap-overridden, or unexposed; reference and NaN-p rows are always NaN.
  • event_study_df provenance channels record the df each ES row's safe_inference actually received: CS (scalar; G-1 bare-cluster / conservative min-across-horizons effective df on explicit-survey fits - df_inference and its narrow HonestDiD contract untouched), MPD (per-period dict), StackedDiD (per-event dict), TwoStageDiD (scalar survey df), LPDiD (per-horizon dict). All channels clear under bootstrap; the adapter's df_inference fallback is gated on bootstrap_results is None so a bootstrap fit can never surface a df its stored p-values never used.
  • DiagnosticReport parallel-trends check (behavior change, CHANGELOG'd): Stacked/TwoStage analytical fits upgrade from Bonferroni to the joint-Wald path now that their VCVs are exposed - with three methodology guards: (1) provenance fail-close - any retained pre-row with a non-finite p-value (BM-DOF failure, collapsed replicate df) returns inconclusive rather than publishing a finite verdict through inference the estimator refused to produce; (2) hc2_bm fits skip the generic chi-square Wald (it would discard the CR2/BM small-sample correction; AHT/HTZ tracked in DEFERRED.md) and run Bonferroni over the BM-adjusted per-row p-values; (3) rank/PSD guard - a cluster-sandwich covariance is singular whenever tested leads approach the cluster count, and an unguarded solve produces a negative "Wald statistic" reported as p=1.0; the guard requires a finite full-rank PSD block and a non-negative statistic, else falls back to Bonferroni.
  • Resolves the deferred TODO.md row from feat(v4): results-contract foundation - BaseResults, Diagnostic marker, unified event-study surface (Phase 2a) #710; a narrower follow-up row (SunAbraham/dCDH df threading, LPDiD pooled-window df) replaces it. Ledger row M-092's notes record the pre-cut amendment (same test_ref; introduced_in 3.9 not yet released).

Methodology references (required if estimator / math changes)

  • Method name(s): StackedDiD (Wing, Freedman & Hollingsworth 2024), TwoStageDiD (Gardner 2022 two-stage GMM), CallawaySantAnna, MultiPeriodDiD, LPDiD - provenance exposure only; no estimator equation, weighting, point estimate, or SE changes (surface values are copies of what each fit already computed).
  • Paper / source link(s): existing REGISTRY.md sections; new **Note:** entries under StackedDiD and TwoStageDiD document the exposure semantics.
  • Any intentional deviations from the source (and why): the hc2_bm parallel-trends policy (Bonferroni over BM-adjusted per-row p-values instead of a chi-square joint Wald that would discard the CR2 small-sample correction) is documented in REPORTING.md "hc2_bm parallel-trends policy"; the proper multi-constraint AHT/HTZ CR2 test is tracked in DEFERRED.md (needs clubSandwich parity).

Validation

  • Tests added/updated: tests/test_event_study_surface.py (per-row df container tests, Stacked/TwoStage vcov alignment incl. Prop-5 exclusion, per-producer df threading incl. the CS bootstrap leak pin, ALL_PRODUCERS df invariants), tests/test_two_stage.py (vcov persistence mode gating), tests/test_diagnostic_report.py (joint-Wald upgrade, hc2_bm fail-closed/routing, NaN-p provenance guard, organic singular-VCV downgrades for both estimators, non-negative-statistic contract).
  • Backtest / simulation / notebook evidence (if applicable): StackedDiD sqrt(diag(vcov)) vs native SEs bit-exact at rtol 1e-14 on fixed-seed panels; 1,776 tests green across the 14 targeted suites; Sphinx -W clean; mypy zero errors.

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

…ed/TwoStage VCV persistence, event_study_df channels, PT joint-Wald guards (M-092 follow-up)
@github-actions

Copy link
Copy Markdown

Overall assessment: ✅ Looks good

Executive summary

  • No unmitigated P0/P1 findings.
  • StackedDiD and TwoStageDiD covariance persistence matches their registered variance definitions; estimator math and SEs are unchanged.
  • Per-row df provenance is consistently cleared for bootstrap/reference/undefined-inference rows.
  • The hc2_bm Bonferroni policy is documented and its AHT/HTZ replacement is tracked.
  • One documentation inconsistency and a minor replicate-mode test gap remain.

Methodology

  • Severity: P3 — informational
    Impact: hc2_bm parallel-trends testing uses conservative Bonferroni-adjusted BM p-values instead of a multi-constraint CR2 Wald test. This is explicitly documented and tracked, so it is not a defect.
    Concrete fix: None required for this PR; future AHT/HTZ work is tracked in DEFERRED.md:L51. See docs/methodology/REPORTING.md:L395-L418.

No undocumented estimator, weighting, identification, or variance mismatch found. The new fields copy the registered Gardner-GMM and pooled-regression covariance matrices (docs/methodology/REGISTRY.md:L1761-L1762, L1897-L1899).

Code Quality

No findings. Changed callers correctly handle the new _stage2_event_study() tuple return, and inference continues through safe_inference().

Performance

No findings. Covariance persistence adds only small submatrix copies; the diagnostic eigendecomposition operates on the pre-period block.

Maintainability

No findings. The producer-specific provenance channels are consistently adapted into the unified per-row representation.

Tech Debt

  • Severity: P3 — informational
    Impact: SunAbraham, dCDH, and LPDiD pooled-window df provenance remains unavailable and surfaces as NaN. This is additive metadata only.
    Concrete fix: None required here; explicitly tracked in TODO.md:L26.

Security

No findings. No secrets, unsafe input handling, or security-sensitive changes were identified.

Documentation/Tests

  • Severity: P2
    Impact: docs/methodology/REPORTING.md:L382-L393 says StackedDiD and TwoStageDiD pre-trends-power classification uses the new full covariance, but those classes remain excluded from DiagnosticReport power applicability and compute_pretrends_power() supports only MPD, CS, and SunAbraham (diff_diff/diagnostic_report.py:L82-L88, diff_diff/pretrends.py:L1135-L1243). Users may incorrectly expect power-aware reporting.
    Concrete fix: Remove the pre-trends-power claim and retain only the joint-Wald statement, unless this PR also adds the missing adapters and applicability entries.

  • Severity: P3
    Impact: New tests cover analytical, bootstrap, and survey-TSL modes, but do not directly assert the new StackedDiD/TwoStageDiD fields under replicate-weight designs.
    Concrete fix: Add JK1/BRR event-study tests asserting StackedDiD VCV-diagonal alignment and TwoStageDiD VCV clearing plus final replicate df propagation.

Runtime tests could not be independently executed because the review environment lacks pytest and numerical dependencies.

@github-actions

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 89f027f7c95f6a9af23e2c06d28b57ca4b42b1ea


Overall assessment: ✅ Looks good

Executive summary

  • No unmitigated P0/P1 findings.
  • Gardner-GMM and pooled-regression covariance persistence matches the registered methodology.
  • Bootstrap, replicate-weight, undefined-inference, singular-VCV, and empty-result paths fail closed.
  • Both prior review items are resolved.
  • Static parsing passed; runtime tests were unavailable because numerical dependencies are not installed.

Methodology

  • Severity: P3 — informational
    Impact: hc2_bm parallel-trends testing conservatively uses Bonferroni rather than an AHT/HTZ joint CR2 test. This is documented and tracked.
    Concrete fix: None required; follow-up is recorded in DEFERRED.md:L51.

No undocumented estimator, weighting, variance, SE, or identification mismatch found. See docs/methodology/REGISTRY.md:L1762 and L1898.

Code Quality

No findings. New inference continues through safe_inference() and covariance failures downgrade safely.

Performance

No findings. Added matrix slicing and diagnostic decomposition operate only on event-study blocks.

Maintainability

No findings. Producer-specific df channels are normalized consistently into the per-row surface.

Tech Debt

  • Severity: P3 — informational
    Impact: SunAbraham, dCDH, and LPDiD pooled-window df provenance remains unexposed and appears as NaN.
    Concrete fix: None required here; tracked in TODO.md:L26.

Security

No findings. No secrets or security-sensitive changes identified.

Documentation/Tests

No findings. The prior power-scope inconsistency is corrected in docs/methodology/REPORTING.md:L392-L398, and replicate-weight coverage was added in tests/test_event_study_surface.py:L1350 and tests/test_two_stage.py:L2701.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Jul 20, 2026
@igerber
igerber merged commit 0c61885 into main Jul 20, 2026
39 of 40 checks passed
@igerber
igerber deleted the feat/v4-esr-vcov-df-roundtrip branch July 20, 2026 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-ci Triggers CI test workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant