Skip to content

Add replicate weight support to 7 estimators#253

Merged
igerber merged 11 commits into
mainfrom
replicate-weight-expansion
Apr 2, 2026
Merged

Add replicate weight support to 7 estimators#253
igerber merged 11 commits into
mainfrom
replicate-weight-expansion

Conversation

@igerber

@igerber igerber commented Apr 1, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add compute_replicate_refit_variance() to survey.py — generic callback-based replicate variance for estimators with weight-dependent transformations
  • DiD: LinearRegression dispatch (no-absorb) + estimator-level refit loop (absorb path)
  • MultiPeriodDiD: compute_replicate_vcov (no-absorb) + refit loop (absorb)
  • TWFE: refit loop re-doing weighted within-transformation per replicate
  • SunAbraham: refit loop wrapping _fit_saturated_regression(), replaces vcov_cohort
  • StackedDiD: refit loop with Q-weight × replicate weight composition, propagates replicate columns through stacked dataset
  • ImputationDiD: two-stage refit (_fit_untreated_model + _impute_treatment_effects)
  • TwoStageDiD: two-stage refit (_fit_untreated_model + _stage2_static)
  • All 7 estimators reject bootstrap + replicate weights (mutual exclusion)
  • Updated REGISTRY.md support matrix

Methodology references (required if estimator / math changes)

  • Method name(s): Replicate weight variance (BRR, Fay, JK1, JKn) — extends Phase 6 survey support
  • Paper / source link(s): Rao & Wu (1988), Judkins (1990); existing compute_replicate_vcov and compute_replicate_if_variance in survey.py
  • Any intentional deviations from the source (and why): None — uses same scaling/centering logic as existing replicate functions

Validation

  • Tests added/updated:
    • tests/test_replicate_weight_expansion.py — 15 new smoke tests (7 estimators × {BRR, JK1} + mutual exclusion guards)
    • tests/test_survey_phase6.py — Updated 7 "rejected" tests to "accepted" tests verifying estimators now work with replicate weights
  • 72 replicate-related tests pass (15 new + 57 existing)

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

Generated with Claude Code

@github-actions

github-actions Bot commented Apr 1, 2026

Copy link
Copy Markdown

Overall Assessment
Needs changes.

Executive Summary

  • The callback-based compute_replicate_refit_variance() approach is methodologically fine; the problems are in estimator-specific propagation, not in the generic Rao/Wu-Judkins replicate-refit pattern. citeturn1search0turn2search1
  • ImputationDiD only switches the overall ATT to replicate-weight variance. Its event-study and group outputs still use the old conservative SE path, so aggregate='event_study', 'group', and 'all' silently return non-replicate inference.
  • TwoStageDiD has the same propagation gap for event-study and group outputs, and it also mishandles replicate designs after filtering always-treated units.
  • SunAbraham fixes vcov_cohort after replicate refits, but it still stores stale cohort-level SEs from the pre-override fit in results.cohort_effects.
  • The PR also removes the public generate_survey_did_data helper and its tests/tutorial coverage without deprecation or mention.
  • I could not execute the test suite here because the local Python environment is missing project dependencies (numpy import failed).

Path to Approval

  1. Implement replicate-based event-study and group inference for ImputationDiD, not just the overall ATT, and add a regression test covering aggregate='all' with replicate weights.
  2. Implement replicate-based event-study and group inference for TwoStageDiD, and subset resolved_survey.replicate_weights whenever rows are dropped (for example, always-treated exclusion); add a test with at least one first_treat <= min(time) unit.
  3. In SunAbraham, either disable the initial replicate dispatch on the already-demeaned fit or recompute cohort_ses from the post-override replicate vcov_cohort; add a test on results.cohort_effects / to_dataframe(level='cohort').
  4. Keep generate_survey_did_data in this PR, or deprecate it in a separate change with release-note coverage and a documented replacement path.

Methodology
Cross-check result: the generic replicate-refit helper is acceptable, and the registry’s use of BRR/Fay/JK scaling in a callback-based refit utility is consistent with the cited replicate-weight literature. The defects are in the estimator-level outputs that still bypass that helper. citeturn1search0turn2search1

  • Severity: P1. Impact: ImputationDiD only applies replicate refits to overall_se in diff_diff/imputation.py:L469, but aggregate='event_study' and aggregate='group' still run through _compute_conservative_variance() in diff_diff/imputation.py:L1482 and diff_diff/imputation.py:L1677. The registry now advertises replicate support for ImputationDiD in docs/methodology/REGISTRY.md:L2236, with no note limiting that support to the simple overall ATT, so these aggregate outputs are an undocumented variance mismatch. Concrete fix: produce per-replicate event-study/group estimates from the same two-stage refit callback and derive their SEs/CIs from replicate dispersion instead of the full-sample conservative variance helper.
  • Severity: P1. Impact: TwoStageDiD has the same propagation gap: only overall_se is overridden in diff_diff/two_stage.py:L501, while _stage2_event_study() and _stage2_group() still use _compute_gmm_variance() in diff_diff/two_stage.py:L1024 and diff_diff/two_stage.py:L1242. Under replicate designs, that silently returns non-replicate SEs/p-values/CIs for aggregate='event_study', 'group', and 'all', despite the support claim in docs/methodology/REGISTRY.md:L2246. Concrete fix: extend the replicate refit to return the full stage-2 coefficient vector per replicate and compute event-study/group inference from that replicate distribution.
  • Severity: P1. Impact: SunAbraham still computes cohort_ses from the initial saturated fit in diff_diff/sun_abraham.py:L612 and diff_diff/sun_abraham.py:L781, where LinearRegression is given survey_design=resolved_survey on an already-demeaned design matrix. The PR correctly replaces vcov_cohort later in diff_diff/sun_abraham.py:L633, but results.cohort_effects is then populated from the stale pre-override cohort_ses in diff_diff/sun_abraham.py:L746. That leaves a user-visible SE field inconsistent with the documented replicate path. Concrete fix: recompute cohort-level SEs from the post-override replicate vcov_cohort, or suppress cohort SEs under replicate support until they are recomputed correctly.

Code Quality

  • Severity: P1. Impact: when TwoStageDiD excludes always-treated units, it subsets weights, strata, psu, and fpc in diff_diff/two_stage.py:L293 through diff_diff/two_stage.py:L355, but it never subsets resolved_survey.replicate_weights. That leaves the replicate matrix out of sync with df for the later refit path in diff_diff/two_stage.py:L503, so panels with first_treat <= min(time) can produce failed refits or NaN inference. Concrete fix: subset replicate_weights with the same keep_mask, or replace this manual slicing with a helper that subsets every row-aligned field of ResolvedSurveyDesign together.

Performance

  • No findings.

Maintainability

  • Severity: P2. Impact: the diff removes the public generate_survey_did_data API from diff_diff/init.py:L92, diff_diff/init.py:L319, and diff_diff/prep.py:L29, while deleting the implementation in diff_diff/prep_dgp.py:L1132. That is a breaking API change unrelated to the replicate-weight feature and not presented as a deprecation. Concrete fix: keep the helper for this PR, or stage its removal behind a formal deprecation cycle with docs and release notes.

Tech Debt

  • Severity: P3. Impact: none. I did not downgrade any of the P1s above via TODO.md; the tracked items near TODO.md:L47 do not cover the new replicate-aggregation gaps or the TwoStageDiD filtering bug. Concrete fix: none; these are untracked correctness issues and should be fixed before merge.

Security

  • No findings.

Documentation/Tests

  • Severity: P2. Impact: the new replicate tests in tests/test_replicate_weight_expansion.py and tests/test_survey_phase6.py only exercise overall-att smoke paths for the newly supported estimators. There is no coverage for aggregate='event_study'|'group'|'all' in ImputationDiD/TwoStageDiD or for SunAbraham cohort-level SE outputs, which is why the propagation bugs above slip through. Concrete fix: add targeted replicate-weight tests that assert finite, replicate-based inference on those outputs and include at least one always-treated-unit regression for TwoStageDiD.

@igerber

igerber commented Apr 1, 2026

Copy link
Copy Markdown
Owner Author

/ai-review

@github-actions

github-actions Bot commented Apr 1, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: adfac85b301fa8c0592ce539f00ff15a466f1308

Overall Assessment

⛔ Blocker

Executive Summary

Methodology

  • Severity: P0. Impact: the new manual replicate paths in diff_diff/estimators.py#L420, diff_diff/estimators.py#L1321, diff_diff/twfe.py#L317, diff_diff/imputation.py#L589, diff_diff/two_stage.py#L485, diff_diff/sun_abraham.py#L676, diff_diff/stacked_did.py#L516, and diff_diff/stacked_did.py#L557 treat df_survey=None as “use normal/default df” instead of the existing replicate-design sentinel df=0 -> NaN inference. The registry and existing code explicitly say undefined replicate d.f. should yield undefined inference, not finite p-values/CIs. Concrete fix: add one shared helper that resolves replicate inference df as 0 when uses_replicate_variance and df_survey is None, overrides to n_valid - 1 when replicates are dropped, and use that helper in all seven new estimator paths before every safe_inference() call.
  • Severity: P1. Impact: TwoStageDiD’s replicate event-study covariance is built on the wrong state vector. _sorted_es_periods_ts is derived from all raw treated relative times at diff_diff/two_stage.py#L573, but _stage2_event_study() legitimately omits horizons under horizon_max, balance_e, and Proposition 5 at diff_diff/two_stage.py#L1144 and diff_diff/two_stage.py#L1191. The refit callback then appends np.nan for missing horizons at diff_diff/two_stage.py#L540, and compute_replicate_refit_variance() drops any replicate with any non-finite component at diff_diff/survey.py#L1761. On top of that, the full-sample vector uses zero placeholders at diff_diff/two_stage.py#L581, which is wrong whenever mse=True per docs/methodology/REGISTRY.md#L2229. Concrete fix: compute full-sample event-study/group outputs first, derive the exact returned horizon/group order from those outputs after filtering/identification checks, and populate full_sample_estimate with the actual full-sample effects rather than placeholders.
  • Severity: P1. Impact: ImputationDiD explicitly marks Proposition 5 horizons as unidentified with all-NaN event-study output at diff_diff/imputation.py#L1642, but the replicate override unconditionally rewrites their se field at diff_diff/imputation.py#L645. That leaves user-visible output with effect = NaN but a finite SE for a horizon the estimator has already declared unidentified. Concrete fix: skip replicate overrides for any event-study/group entry whose full-sample effect is non-finite, or better, build the replicate vector only from identified horizons/groups.

Code Quality

  • No additional findings.

Performance

  • No findings.

Maintainability

Tech Debt

  • No findings. TODO.md does not track the P0/P1 issues above, so they are not accepted deferred work.

Security

  • No findings.

Documentation/Tests

Path to Approval

  1. Add a shared replicate-df resolver and use it in DifferenceInDifferences, MultiPeriodDiD, TwoWayFixedEffects, SunAbraham, StackedDiD, ImputationDiD, and TwoStageDiD so df_survey=None on replicate designs becomes df=0 and yields all-NaN inference.
  2. In TwoStageDiD, derive the replicate event-study/group vector from the actual full-sample returned outputs after horizon_max/balance_e/Proposition 5 filtering, and replace the zero placeholders with the true full-sample effects before calling compute_replicate_refit_variance().
  3. In ImputationDiD, skip replicate SE overrides for non-finite / Proposition-5 event-study entries, and add a regression test with no never-treated cohorts under replicate weights to ensure those horizons remain fully NaN.
  4. Restore generate_survey_did_data() and docs/tutorials/16_survey_did.ipynb, or move their removal to a separate deprecation PR with release-note coverage and an explicit replacement path.

@igerber

igerber commented Apr 2, 2026

Copy link
Copy Markdown
Owner Author

/ai-review

@github-actions

github-actions Bot commented Apr 2, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 97d7040cc456a99f0373cd8bbae76e4c21420ee9

Overall Assessment

⛔ Blocker

Executive Summary

Methodology

  • Severity: P0. Impact: MultiPeriodDiD does not honor the registry’s replicate-inference contract. The new replicate path correctly maps undefined replicate d.f. to df = 0, but the unchanged fallback at the end of fit() rewrites any non-positive df to None, so rank-deficient replicate designs can still get normal-theory p-values/CIs instead of all-NaN inference. That is an undocumented methodology mismatch for a newly enabled estimator path. See diff_diff/estimators.py:L1320-L1338, diff_diff/estimators.py:L1366-L1415, docs/methodology/REGISTRY.md:L2238-L2263, tests/test_survey_phase6.py:L1102-L1128. Concrete fix: make the df <= 0 fallback replicate-aware, or bypass it entirely when uses_replicate_variance is true, so undefined replicate d.f. stays on the df=0 -> NaN inference path.
  • Severity: P1. Impact: ImputationDiD still defines the replicate refit vector on raw treated relative times and all treatment groups before horizon_max, balance_e, and Proposition 5 filtering. compute_replicate_refit_variance() then drops any replicate with any non-finite component, so hidden NaN horizons can poison the whole replicate covariance calculation and make overall or otherwise identified event-study/group inference unusable. The previous “finite SE on Prop-5 horizon” symptom is partially addressed, but the underlying state-vector mismatch remains. See diff_diff/imputation.py:L472-L483, diff_diff/imputation.py:L509-L552, diff_diff/imputation.py:L1595-L1609, diff_diff/imputation.py:L1646-L1656, diff_diff/survey.py:L1761-L1786. Concrete fix: build the replicate vector from the actual full-sample event_study_effects and group_effects dictionaries after filtering, keeping only returned entries with finite effects, mirroring the new TwoStageDiD approach.

Code Quality

  • No separate findings beyond the methodology issues above.

Performance

  • No findings.

Maintainability

Tech Debt

  • Severity: P3. Impact: TODO.md does not track either blocking statistical issue, so neither is mitigated under the project’s deferred-work policy. The only nearby entry marks the lower-level replicate-df plumbing as resolved, not these estimator-level regressions. See TODO.md:L47-L57. Concrete fix: treat the P0/P1 issues as merge blockers and only use TODO.md for non-blocking residual follow-up after the fixes land.

Security

  • No findings.

Documentation/Tests

Path to Approval

  1. In MultiPeriodDiD.fit(), preserve the df=0 sentinel for replicate designs with undefined survey d.f. instead of falling back to df=None, and add a regression test that asserts all inference fields are NaN for a rank-one replicate design.
  2. In ImputationDiD.fit(), derive the replicate refit vector from the actual returned event-study/group outputs after horizon_max, balance_e, and Proposition 5 filtering, and add regressions for no-never-treated and filtered event-study cases under replicate weights.
  3. Restore generate_survey_did_data() and its tests/test_prep.py coverage in this PR, or move that API removal to a separate deprecation PR with a documented replacement.
  4. Restore the survey tutorial notebook and its README/quickstart/estimator-guide links, or replace them with equivalent user-facing documentation before merge.

@igerber

igerber commented Apr 2, 2026

Copy link
Copy Markdown
Owner Author

/ai-review

@github-actions

github-actions Bot commented Apr 2, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: dd3316d443a9dc73ee05deeea9be43b0e2336dc9

Overall Assessment

Needs changes

Reviewed as a re-review against the provided unified diff and the methodology registry. The prior MultiPeriodDiD, TwoStageDiD, and SunAbraham P1/P0 items appear addressed; the remaining blockers are both in the new ImputationDiD replicate path.

Executive Summary

  • The prior MultiPeriodDiD replicate-df blocker looks fixed: replicate df=0 is now preserved instead of falling back to normal-theory inference.
  • The prior TwoStageDiD replicate-state issue also looks fixed: replicate arrays are subset after always-treated filtering, and the refit vector is derived from the returned aggregates.
  • The prior SunAbraham cohort-SE issue looks fixed: cohort SEs are now recomputed from the replicate override vcov.
  • Severity P1: ImputationDiD replicate event-study refits still ignore the estimator’s balance_e cohort filter, so reported SEs can be for a different estimand than the reported point estimate.
  • Severity P1: ImputationDiD also discards effect-specific n_valid from scalar replicate refits and reuses the overall ATT df, which can understate p-values/CIs for sparse horizons or cohorts.
  • Non-blocking but still unresolved from the prior review: the PR also removes the public generate_survey_did_data() helper and the survey tutorial/docs with no deprecation or replacement path.

Methodology

  • Severity: P1. Impact: ImputationDiD’s new replicate event-study path does not preserve the analytical aggregation contract when balance_e is used. The full-sample event-study effect is built from the balanced-cohort mask in _aggregate_event_study(), but the replicate refit closure averages over all finite treated observations at horizon e via mask_e = fin & (_rel_times_treated == _e), with no balanced-cohort filter. That means the replicate SE/t-stat/p-value/CI can describe a different estimand than the reported effect whenever balance_e excludes some but not all cohorts at a returned horizon. This is an undocumented methodology mismatch and a broken parameter interaction. See diff_diff/imputation.py:L594, diff_diff/imputation.py:L1574, and docs/methodology/REGISTRY.md:L848. Concrete fix: reuse the same balanced-cohort mask inside _refit_es that _aggregate_event_study() uses, or factor the horizon-specific observation mask into a shared helper and drive both the point estimate and replicate refits from that helper.
  • Severity: P1. Impact: ImputationDiD’s per-effect replicate inference ignores effect-specific dropped-replicate counts. compute_replicate_refit_variance() explicitly returns (vcov, n_valid) and the registry requires df = n_valid - 1 when invalid replicates are dropped, but the event-study and group override paths discard that n_valid (vcov_e, _ = ..., vcov_g, _ = ...) and reuse _survey_df derived from the overall ATT path. Sparse horizons/cohorts can therefore get finite inference using too-large df after replicate failures. See diff_diff/imputation.py:L609, diff_diff/imputation.py:L636, diff_diff/survey.py:L1735, and docs/methodology/REGISTRY.md:L2238. Concrete fix: capture n_valid for each scalar horizon/group refit and pass effect-specific df = 0 if n_valid <= 1 else n_valid - 1 into safe_inference().

Code Quality

  • No separate findings beyond the methodology issues above.

Performance

  • No separate findings.

Maintainability

  • Severity: P2. Impact: Still unresolved from the prior review: this PR removes the public generate_survey_did_data() helper, its top-level exports, and its regression coverage as part of replicate-weight support. That is an unrelated public API break with no deprecation path or replacement guidance. See diff_diff/prep_dgp.py:L1132, diff_diff/prep.py:L20, diff_diff/init.py:L80, diff_diff/init.py:L312, and tests/test_prep.py:L1177. Concrete fix: keep the helper in this PR, or move the removal to a separate deprecation PR with explicit replacement guidance and preserved tests until removal.

Tech Debt

  • No separate findings. TODO.md does not mitigate the P1 issues above, and the remaining P2 API/docs regressions are not currently tracked there.

Security

  • No findings.

Documentation/Tests

Path to Approval

  1. In ImputationDiD, make replicate event-study refits use the exact same filtered treated-observation mask as _aggregate_event_study() when balance_e is active, and add a regression with replicate weights where balance_e excludes some but not all cohorts at a returned horizon.
  2. In ImputationDiD, capture n_valid from each scalar event-study/group replicate refit and use effect-specific df = n_valid - 1 (or 0 when n_valid <= 1) in safe_inference(), with a regression where a sparse horizon or cohort loses replicates while the overall ATT remains estimable.

@igerber

igerber commented Apr 2, 2026

Copy link
Copy Markdown
Owner Author

/ai-review

@github-actions

github-actions Bot commented Apr 2, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: d6ab824336640ce4af5a172bf5ecdfbbec57acc4

Overall Assessment

⚠️ Needs changes

Re-reviewing the provided diff against the methodology registry and surrounding code, the prior ImputationDiD balance_e / per-effect n_valid blockers look addressed, as do the prior TwoStageDiD always-treated replicate-state issue and the SunAbraham cohort-SE recomputation issue. The remaining blockers are different: one new SunAbraham methodology problem, one new rank-deficiency edge-case failure across the absorb/within replicate refit paths, and one narrower ImputationDiD balance_e mismatch.

Executive Summary

  • The prior ImputationDiD replicate fixes are mostly in place: the new code now applies a balanced-cohort mask and carries effect-specific n_valid for scalar replicate refits.
  • Severity P1: SunAbraham’s new replicate path only refits cohort coefficients, then reuses full-sample survey-weighted interaction weights for aggregation, so the reported replicate SEs/CIs are for a partially refit estimator rather than the documented interaction-weighted estimator.
  • Severity P1: DifferenceInDifferences, MultiPeriodDiD, and TWFE all have a new absorb/within replicate edge case where a dropped nuisance coefficient makes every replicate look invalid, turning an otherwise identified ATT into all-NaN replicate inference.
  • Severity P1: ImputationDiD’s replicate balance_e mask still does not exactly match _aggregate_event_study() when late horizons are filtered by Proposition 5; the refit path derives max_h from returned finite horizons instead of the full horizon set.
  • Severity P2: The PR still removes the public generate_survey_did_data() helper and its regression coverage with no deprecation or replacement path.
  • Severity P2: The PR still removes the survey tutorial and its discoverability links while survey support is expanding.

Methodology

  • Severity: P1. Impact: SunAbraham’s replicate-weight support does not fully refit the documented interaction-weighted estimator. The new path only recomputes vcov_cohort, then _compute_iw_effects() and _compute_overall_att() rebuild aggregation weights from the full-sample survey-weight column. For survey-weighted SA, those weights are part of the estimator (ŵ_{g,e} = n_{g,e} / Σ_g n_{g,e}), so replicate inference must recompute them under each replicate weight set. The same file’s Rao-Wu bootstrap path already does this correctly by passing the rescaled weight column into _compute_iw_effects() / _compute_overall_att(). As written, the new replicate SEs/CIs can be for a different, partially refit estimator with no warning. See diff_diff/sun_abraham.py:L636-L712, diff_diff/sun_abraham.py:L964-L1088, diff_diff/sun_abraham.py:L1459-L1490, and docs/methodology/REGISTRY.md:L718-L727. Concrete fix: make the replicate refit closure return the aggregated event-study and overall ATT estimates after recomputing cohort-share weights from w_r, or otherwise rerun the aggregation step with replicate-weighted masses before forming replicate variances.
  • Severity: P1. Impact: the new absorb/within-transformation replicate refit paths for DifferenceInDifferences, MultiPeriodDiD, and TwoWayFixedEffects pass the full coefficient vector into compute_replicate_refit_variance(), including any NaN placeholders for nuisance covariates dropped by rank deficiency. That helper marks a replicate invalid if any coefficient is non-finite, so a collinear covariate can cause all replicates to be discarded even when the treatment effect is still identified in the full-sample fit. The result is all-NaN replicate SE/t/p/CI for an otherwise estimable ATT. See diff_diff/estimators.py:L385-L418, diff_diff/estimators.py:L1231-L1278, diff_diff/twfe.py:L288-L315, and compare the valid-replicate contract in docs/methodology/REGISTRY.md:L2238-L2245. Concrete fix: mirror the existing reduced-column handling already used in the no-absorb MultiPeriod path: detect nan_mask from the full-sample fit, refit only the identified columns, compute a reduced replicate vcov, then expand it back to the full shape.
  • Severity: P1. Impact: ImputationDiD’s new replicate balance_e fix is still not fully aligned with _aggregate_event_study(). The scalar replicate path builds all_horizons from _sorted_rel_times with finite returned effects, while _aggregate_event_study() builds all_horizons from the full treated relative-time support before later Proposition 5 filtering. In the case balance_e is active and late horizons are dropped by Prop 5, the replicate mask can admit cohorts that the reported point estimate excluded, so the SE can still target a different estimand. See diff_diff/imputation.py:L594-L603, diff_diff/imputation.py:L1590-L1603, and docs/methodology/REGISTRY.md:L848-L848. Concrete fix: derive the replicate all_horizons exactly the same way _aggregate_event_study() does, before finite-effect / Prop 5 filtering, or factor the balanced-cohort-mask construction into a shared helper used by both paths.

Code Quality

  • No separate findings beyond the methodology issues above.

Performance

  • No separate findings.

Maintainability

  • Severity: P2. Impact: the PR still removes the public generate_survey_did_data() helper, its top-level export, and its dedicated regression coverage as part of a replicate-weight feature PR. That is an unrelated API break with no deprecation path, replacement guidance, or TODO tracking. See diff_diff/prep_dgp.py:L1132-L1410, diff_diff/init.py:L87-L92, diff_diff/init.py:L314-L319, and tests/test_prep.py:L1177-L1432. Concrete fix: keep the helper in this PR, or move the removal to a separate deprecation PR with explicit replacement guidance and preserved tests until removal.

Tech Debt

  • No separate findings. TODO.md does not currently track the remaining P1/P2 issues, so none of them are mitigated by deferred-work policy.

Security

  • No findings.

Documentation/Tests

Path to Approval

  1. In SunAbraham, make the replicate path refit the full aggregated estimator, not just vcov_cohort: recompute cohort-share weights from each w_r and form replicate distributions for event-study and overall ATT from those fully re-aggregated estimates.
  2. In DifferenceInDifferences, MultiPeriodDiD (absorb path), and TwoWayFixedEffects, handle rank-deficient nuisance columns the same way the no-absorb MultiPeriod replicate path already does: refit only identified columns, compute reduced replicate vcov, then expand.
  3. In ImputationDiD, build the replicate balance_e mask from the same full treated horizon set used by _aggregate_event_study(), not from the filtered finite-output horizon list.
  4. Add targeted regressions for:
    1. SunAbraham with replicate weights and unequal survey-weighted cohort masses, asserting replicate inference matches a manual full-refit replicate aggregation.
    2. DiD/TWFE/MultiPeriod replicate paths with a collinear nuisance covariate but identified ATT, asserting finite ATT SE.
    3. ImputationDiD with balance_e plus no never-treated units so a late horizon is dropped by Proposition 5, asserting replicate SEs use the same balanced cohorts as the point estimate.
  5. Restore generate_survey_did_data() plus its exports/tests, or move its removal to a separate documented deprecation PR.
  6. Restore the survey tutorial notebook and discoverability links, or replace them with equivalent survey documentation in this PR.

igerber and others added 5 commits April 2, 2026 06:47
@igerber @claude
Add replicate weight support to 7 estimators
0439a38 
Expand replicate weight variance (BRR, Fay, JK1, JKn) from 4 estimators
to 11. New compute_replicate_refit_variance() in survey.py generalizes
the existing compute_replicate_vcov() for estimators whose estimation
procedure depends on weights (within-transformation, two-stage imputation).

Estimator-level changes:
- DiD: LinearRegression dispatch (no-absorb) + refit loop (absorb)
- MultiPeriodDiD: compute_replicate_vcov (no-absorb) + refit loop (absorb)
- TWFE: refit loop re-doing within-transformation per replicate
- SunAbraham: refit loop wrapping _fit_saturated_regression, replaces vcov_cohort
- StackedDiD: refit loop with Q-weight × replicate weight composition
- ImputationDiD: two-stage refit (_fit_untreated_model + _impute)
- TwoStageDiD: two-stage refit (_fit_untreated_model + _stage2_static)

All estimators reject bootstrap + replicate (mutual exclusion).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@igerber @claude
Address AI review: propagate replicate SE to all aggregation modes
4eb65be 
- SunAbraham: pass resolved_survey=None to initial fit (prevents bogus
  replicate dispatch on demeaned data); recompute cohort_ses from the
  replicate vcov diagonal so results.cohort_effects carries correct SEs
- TwoStageDiD: subset replicate_weights when always-treated units are
  excluded (fixes misaligned arrays); extend refit to return event-study
  and group effects so aggregate='event_study'/'group'/'all' use
  replicate-based SEs instead of GMM SEs
- ImputationDiD: extend refit to return per-relative-time and per-group
  ATTs alongside overall ATT; override event-study/group SEs from
  replicate vcov after running existing aggregation for point estimates
- Tests: event-study/group aggregation with replicate weights (Imputation
  + TwoStage), always-treated unit subsetting, SunAbraham cohort SEs

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@igerber @claude
Address AI review round 2: replicate df=0 for rank-deficient, fix Two…
b7c6ab2 
…StageDiD/ImputationDiD edge cases

P0: All 7 estimators now set df=0 (→ NaN inference) when replicate
design has undefined df_survey (rank-deficient replicate matrix).
Previously fell through to normal distribution with finite p-values.

P1: TwoStageDiD replicate event-study/group vector now derived from
actual full-sample outputs after horizon_max/balance_e/Prop5 filtering,
with real effect values instead of zero placeholders.

P1: ImputationDiD replicate SE override now skips non-finite effects
(Prop 5 unidentified horizons), preserving their all-NaN status.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@igerber @claude
Address AI review round 3: MultiPeriodDiD df fallback, ImputationDiD …
634552c 
…vector mismatch

P0: MultiPeriodDiD df<=0 fallback now skipped for replicate designs
(df=0 is intentional for NaN inference on rank-deficient replicates).

P1: ImputationDiD replicate refit restructured: overall ATT uses
separate scalar refit (robust); event-study/group use per-effect
scalar refits from actual outputs after Prop5/filtering. Prevents
NaN horizons from poisoning the overall ATT variance.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@igerber @claude
Address AI review round 4: ImputationDiD balance_e filter and per-eff…
cfa36af 
…ect df

- Event-study replicate refits now apply the same balanced-cohort mask
  as _aggregate_event_study() when balance_e is set, so replicate SEs
  match the reported estimand
- Per-effect replicate refits now capture n_valid and compute
  effect-specific df (n_valid-1 when replicates dropped) instead of
  reusing the overall ATT df

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@igerber igerber force-pushed the replicate-weight-expansion branch from d6ab824 to cfa36af Compare April 2, 2026 10:47
@igerber @claude
Address AI review round 5: SunAbraham full IW refit, rank-deficient n…
9c80b95 
…uisance, balance_e horizon set

- SunAbraham: replicate refit now returns fully re-aggregated event-study
  and overall ATT effects (recomputes IW cohort-share weights from w_r
  via _compute_iw_effects/_compute_overall_att), not just vcov_cohort
- DiD/MultiPeriodDiD/TWFE absorb paths: detect NaN nuisance coefficients
  from rank-deficient full-sample fit, refit only identified columns,
  compute reduced replicate vcov, expand back with _expand_vcov_with_nan
- ImputationDiD: balance_e mask now uses full treated horizon set (before
  Prop 5 filtering), matching _aggregate_event_study() exactly

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

igerber commented Apr 2, 2026

Copy link
Copy Markdown
Owner Author

/ai-review

@github-actions

github-actions Bot commented Apr 2, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 9c80b95e9a516e42427a13eea8b89a73ddd287ae

Overall Assessment

⚠️ Needs changes

Re-reviewing the changed replicate-weight paths against docs/methodology/REGISTRY.md and the touched estimator code, the prior blockers on SunAbraham’s partial re-aggregation, absorb-path rank deficiency, and ImputationDiD balance_e alignment look addressed. One unmitigated P1 remains: SunAbraham still reports cohort-level SEs from the analytical saturated regression, not the replicate refit.

Executive Summary

  • The prior P1s from the last rerun appear resolved in the updated DifferenceInDifferences / MultiPeriodDiD / TwoWayFixedEffects, ImputationDiD, and aggregated SunAbraham paths.
  • P1: SunAbraham’s new replicate support does not update the stored cohort-level SEs, so cohort_effects still expose analytical SEs under replicate-weight designs.
  • P2: The new tests are mostly happy-path smoke tests and do not cover the new absorb/within replicate branches or the new generic refit helper directly.
  • No new security findings.
  • No REGISTRY.md note or TODO.md entry mitigates the remaining issues.

Methodology

  • Severity: P1. Impact: SunAbraham now refits replicate-weighted event-study and overall ATT uncertainty, but the stored cohort-level outputs still copy cohort_ses from the initial analytical LinearRegression fit. That means result.cohort_effects and to_dataframe(level="cohort") silently report the wrong variance estimator under replicate-weight designs, even though the new registry support matrix says the estimator-level refit “replaces vcov_cohort.” Concrete fix: compute replicate vcov/SEs for the raw (g,e) cohort coefficients and store those in cohort_effects, or set cohort-level SEs to NaN and document cohort-level replicate inference as unsupported until it is implemented. Location: diff_diff/sun_abraham.py:L618-L633, diff_diff/sun_abraham.py:L723-L812, diff_diff/sun_abraham.py:L221-L238, docs/methodology/REGISTRY.md:L2246-L2256.

Code Quality

  • No separate findings beyond the methodology issue above.

Performance

  • No new findings.

Maintainability

  • No new findings.

Tech Debt

  • No separate findings. The P1/P2 above are not tracked in TODO.md, so they remain unmitigated.

Security

  • No findings.

Documentation/Tests

  • Severity: P2. Impact: the new coverage is mostly smoke testing. It does not exercise the newly added absorb/within replicate branches for DifferenceInDifferences, MultiPeriodDiD, and TwoWayFixedEffects, and it adds no direct tests for compute_replicate_refit_variance() despite that helper carrying copied method-scaling logic. The new SunAbraham cohort-SE test also only checks finiteness, so it would not catch the stale analytical-SE bug above. Concrete fix: add targeted regressions for DifferenceInDifferences(absorb=[...]), MultiPeriodDiD(absorb=[...]), and TwoWayFixedEffects with a dropped nuisance covariate under replicate weights, plus direct helper tests for scaling / invalid-replicate handling and a SunAbraham cohort-SE parity test against a manual replicate refit. Location: diff_diff/estimators.py:L385-L435, diff_diff/estimators.py:L1237-L1289, diff_diff/twfe.py:L287-L329, diff_diff/survey.py:L1706-L1785, tests/test_replicate_weight_expansion.py:L115-L220, tests/test_replicate_weight_expansion.py:L392-L406, tests/test_survey_phase6.py:L1288-L1476.
  • Execution note: I could not run the suite in this environment because python is missing numpy and pytest, so the test assessment is from static review only.

Path to Approval

  1. In SunAbraham, stop returning analytical cohort SEs for replicate-weight fits: either compute replicate cohort vcov/SEs for the raw (g,e) coefficients, or set those SEs to NaN and add a clearly labeled **Note:** / deviation entry in docs/methodology/REGISTRY.md.
  2. Add a regression test that fails if SunAbraham(..., survey_design=replicate_design).fit(...).cohort_effects[*]["se"] falls back to analytical SEs.
  3. Add targeted replicate tests for the absorb/within refit branches and the new compute_replicate_refit_variance() helper.

@igerber @claude
Address AI review round 6: SunAbraham cohort-level replicate SEs
38427d4 
Add second replicate refit loop for raw (g,e) cohort coefficients so
cohort_ses in results.cohort_effects uses replicate-based variance
instead of stale analytical SEs from the initial saturated regression.

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

igerber commented Apr 2, 2026

Copy link
Copy Markdown
Owner Author

/ai-review

@github-actions

github-actions Bot commented Apr 2, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 38427d4b7a1819db2a558da895287a3fe02dee93

Overall Assessment

✅ Looks good

Executive Summary

Methodology

No unmitigated findings. The changed replicate-weight variance paths are consistent with the Methodology Registry’s updated support matrix, and the Sun-Abraham cohort-level replicate SE gap from the prior review appears resolved (docs/methodology/REGISTRY.md:L2282-L2296, diff_diff/sun_abraham.py:L723-L776).

Code Quality

  • Severity: P2. Impact: ImputationDiD’s replicate event-study override treats the normalized reference period as if it were an estimand. Since omega_1_mask never contains e = -1 - anticipation, every replicate refit for that marker returns NaN, which can replace the intended 0.0/0.0 reference-period row and add noisy warnings. Concrete fix: filter _sorted_rel_times to exclude ref_period or any horizon with n_obs == 0 before entering the replicate loop. (diff_diff/imputation.py:L585-L644, diff_diff/imputation.py:L1627-L1634)

Performance

No findings.

Maintainability

No findings.

Tech Debt

No findings. The P2 items above are not already tracked in TODO.md:L47-L88.

Security

No findings.

Documentation/Tests

Review note: this was a static diff review. I could not run pytest in this environment because numpy/pytest are not installed.

@igerber @claude
Address P2s: skip ref-period in ImputationDiD replicate loop, add abs…
5375c70 
…orb test

- ImputationDiD: exclude synthetic reference period (n_obs==0) from
  replicate event-study override loop to avoid spurious NaN warnings
- Add DiD absorb-path replicate test (BRR + absorbed group FE)
- TODO.md: track BRR test gap (Fay-like perturbations, not true
  half-sample) as low-priority deferred item

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

igerber commented Apr 2, 2026

Copy link
Copy Markdown
Owner Author

/ai-review

@github-actions

github-actions Bot commented Apr 2, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 5375c704cb9dde24471f98f468d89fba4c8f791f

Overall Assessment

✅ Looks good

Executive Summary

  • All previously raised P1+ issues appear resolved. I did not find a new unmitigated P0/P1 in the changed replicate-weight support.
  • The methodology change affects replicate-weight variance support for DifferenceInDifferences, MultiPeriodDiD, TwoWayFixedEffects, SunAbraham, StackedDiD, ImputationDiD, and TwoStageDiD, and the implementation matches the updated support matrix in docs/methodology/REGISTRY.md#L2299.
  • The prior ImputationDiD reference-period issue is fixed by excluding synthetic n_obs == 0 outputs from the replicate override loop in diff_diff/imputation.py#L601.
  • The prior Sun-Abraham blocker remains fixed: cohort-level SEs are recomputed from a second replicate refit before storing cohort_effects in diff_diff/sun_abraham.py#L757.
  • The prior BRR smoke-test concern is now properly tracked in TODO.md#L72, so it is informational only.
  • Remaining non-blocker: P2 metadata consistency. Several new branches override inference d.f. to n_valid - 1 after dropped replicates but do not propagate that effective d.f. back into survey_metadata, so summaries/exported metadata can disagree with the actual inference run.

Methodology

Code Quality

  • No findings.

Performance

  • No findings.

Maintainability

Tech Debt

  • Severity: P3-informational. Impact: the remaining BRR-test realism gap is explicitly tracked in TODO.md#L72, so it is not a blocker. The helper in tests/test_replicate_weight_expansion.py#L91 still generates Fay-like 0.5/1.5 perturbations rather than true half-sample BRR. Concrete fix: no merge blocker for this PR; later add at least one true BRR regression per estimator family or relabel those smoke tests.

Security

  • No findings.

Documentation/Tests

  • No unmitigated findings.
  • Static review only: pytest and numpy are not installed in this environment, so I could not execute the added replicate-weight tests.

@igerber @claude
Write effective replicate df back to survey_metadata.df_survey
c806683 
When dropped replicates reduce the effective df, write it back to
survey_metadata.df_survey so summary()/exported metadata is consistent
with the p-values/CIs actually reported. Matches existing pattern in
ContinuousDiD and TripleDifference. Applied to all 7 new estimator
replicate paths.

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

igerber commented Apr 2, 2026

Copy link
Copy Markdown
Owner Author

/ai-review

@github-actions

github-actions Bot commented Apr 2, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: c806683fbe1d43d6541a72661fd4fa8419ba42d8

Overall Assessment

⚠️ Needs changes

Executive Summary

  • Severity P1: the new replicate-refit paths that rely on weighted within/absorb demeaning still break when a valid replicate gives an entire unit/group zero total weight, so the PR overstates JK1 / true half-sample BRR support for TwoWayFixedEffects, SunAbraham, and absorb-enabled DifferenceInDifferences / MultiPeriodDiD.
  • I did not find a helper-level scaling/centering defect in compute_replicate_refit_variance() itself; the blocker is the estimator-specific zero-weight handling around weighted demeaning.
  • Previously raised P1+ items appear addressed: ImputationDiD now excludes synthetic n_obs == 0 event-study outputs from replicate overrides in diff_diff/imputation.py:L600-L607, and Sun-Abraham now recomputes cohort-level SEs from a second replicate refit in diff_diff/sun_abraham.py:L759-L778.
  • The earlier metadata d.f. issue is only partially fixed: several branches still leave survey_metadata.df_survey stale when dropped replicates leave <=1 valid draw.
  • The Fay-like BRR smoke-test limitation is tracked in TODO.md:L72 and is informational only.

Methodology

Code Quality

  • No findings.

Performance

  • No findings.

Maintainability

  • Severity: P2. Impact: the previous df_survey consistency problem is only partially fixed. Several branches only write back survey_metadata.df_survey when the effective d.f. is positive, so if dropped replicates leave 0 or 1 valid draws, metadata can still report the old rank-based d.f. while inference is actually run with undefined replicate d.f. Representative examples are diff_diff/imputation.py:L589-L593, diff_diff/two_stage.py:L627-L631, diff_diff/sun_abraham.py:L736-L740, and diff_diff/stacked_did.py:L522-L526.
    Concrete fix: after every dropped-replicate override, write the effective value back into survey_metadata: use n_valid - 1 when positive, otherwise clear the metadata d.f. to None.

Tech Debt

  • Severity: P3-informational. Impact: the added estimator smoke tests still use Fay-like nonzero BRR perturbations rather than true half-sample BRR, but that limitation is already tracked in TODO.md:L72. That TODO does not mitigate the P1 above; it only tracks missing regression coverage.
    Concrete fix: no merge blocker from the TODO item itself.

Security

  • No findings.

Documentation/Tests

Static review only: pytest, numpy, pandas, and scipy are not installed in this environment, so I could not execute the added tests.

Path to Approval

  1. Make weighted within/absorb demeaning zero-weight safe for replicate refits in diff_diff/utils.py:L1763-L1774 and diff_diff/utils.py:L1842-L1854, or filter out w_r == 0 rows before the refits in diff_diff/twfe.py:L300-L314, diff_diff/sun_abraham.py:L647-L670, diff_diff/estimators.py:L398-L419, and diff_diff/estimators.py:L1250-L1285.

  2. Add regression tests with actual zeroing replicate designs, not just nonzero Fay-like perturbations: at minimum one true BRR or JK1 case each for TwoWayFixedEffects, SunAbraham, DifferenceInDifferences(absorb=...), and MultiPeriodDiD(absorb=...).

  3. Keep the registry honest: either document/restrict unsupported replicate methods for these weighted-demean estimators, or keep the expanded support matrix only after step 1 is fixed.

@igerber @claude
Address P1/P2: zero-weight demeaning safety, df_survey writeback for …
fba8ca3 
…df=0

P1: Replicate refit closures for TWFE, SunAbraham, DiD/MultiPeriodDiD
absorb paths now drop zero-weight observations before weighted demeaning,
preventing division-by-zero in within-transformation group means. Matches
R's survey::withReplicates() convention. Documented in REGISTRY.md.

P2: survey_metadata.df_survey writeback now covers df=0 case (set to
None) so summary/metadata is consistent when all replicates fail.

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

igerber commented Apr 2, 2026

Copy link
Copy Markdown
Owner Author

/ai-review

@github-actions

github-actions Bot commented Apr 2, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: fba8ca3b220c906aa145f55f5ce31440eb92705c

Overall Assessment

✅ Looks good

Executive Summary

  • The prior P1 blocker appears resolved: the changed absorb/TWFE/Sun-Abraham replicate paths now drop w_r == 0 observations before weighted demeaning, and the registry now documents that behavior.
  • I did not find a methodology defect in the new generic refit helper; its callback-based replicate workflow and centering/scaling rules are consistent with the survey package’s documented withReplicates/svrepdesign/svrVar contracts for replicate-weight designs. diff_diff/survey.py:1706 (r-survey.r-forge.r-project.org)
  • Replicate d.f. handling is materially better across the changed estimators: undefined replicate d.f. now flow through safe_inference() as NaN inference, and dropped-replicate paths overwrite survey_metadata.df_survey instead of leaving stale values.
  • Remaining issues are non-blocking: one performance regression in ImputationDiD, one index-safety issue in SunAbraham, a local REGISTRY doc lag, and a BRR test limitation that is already tracked in TODO.md.

Methodology

Code Quality

  • Severity P2. Impact: SunAbraham._refit_sa() writes replicate weights through a label-based .loc[...] assignment even though w_r is positional. That is fragile on non-unique indexes and makes the refit depend on pandas label semantics rather than row order. Concrete fix: replace the write with positional assignment (df["_rep_wt"] = w_r) or reset the local working copy’s index before the replicate loop. diff_diff/sun_abraham.py:660

Performance

  • Severity P2. Impact: ImputationDiD now reruns the full Step 1 + imputation pipeline once for the overall ATT and again once per horizon and once per cohort, so aggregate="all" scales as R × (1 + H + G) full refits. That is a noticeable runtime regression relative to the vectorized replicate override already used in diff_diff/two_stage.py:551. Concrete fix: build one vector-returning replicate closure for overall/horizon/group outputs and call compute_replicate_refit_variance() once. diff_diff/imputation.py:562

Maintainability

  • No findings.

Tech Debt

  • Severity P3-informational. Impact: the new estimator smoke suite still uses Fay-like 0.5/1.5 BRR perturbations rather than true half-sample BRR, but the PR now tracks that explicitly in TODO.md, so it is not a blocker. Concrete fix: none required before merge beyond the existing follow-up item. TODO.md:72

Security

  • No findings.

Documentation/Tests

@igerber @claude
Address P2/P3: SA positional assign, ImputationDiD vectorized refit, …
692dab4 
…REGISTRY notes

- SunAbraham: replace fragile .loc label-based assignment with
  positional df["_rep_wt"] = w_r for replicate weight column
- ImputationDiD: consolidate per-effect scalar refits into single
  vectorized refit returning [overall, es_e0..., grp_g0...], reducing
  from R×(1+H+G) to R full refits for aggregate="all"
- REGISTRY.md: add replicate-refit cross-references to SunAbraham
  and StackedDiD per-estimator survey notes

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

igerber commented Apr 2, 2026

Copy link
Copy Markdown
Owner Author

/ai-review

@github-actions

github-actions Bot commented Apr 2, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 692dab4517a04090e6369bc24b7a87144ca6d0bc

Overall Assessment

✅ Looks good

Executive Summary

  • No unmitigated P0/P1 findings in the changed code.
  • The new replicate-refit machinery does not show an undocumented methodology deviation: the helper and estimator overrides match the survey package’s replicate-weight contract for callback refits, centering, scaling, and replicate-design degrees of freedom. (r-survey.r-forge.r-project.org)
  • The zero-weight weighted-demeaning blocker is resolved in the new absorb/TWFE/Sun-Abraham replicate paths and is now explicitly documented in the registry.
  • The prior ImputationDiD refit-loop performance concern appears resolved: the replicate path now uses a single vectorized closure rather than rerunning separate full pipelines per output.
  • Remaining issues are non-blocking: TwoStageDiD still does avoidable GMM work inside each replicate, the local REGISTRY notes for ImputationDiD/TwoStageDiD lag the new support, and the Fay-like BRR smoke-test limitation is now properly tracked in TODO.md.

Methodology

Code Quality

  • No findings.

Performance

  • Severity P2. Impact: TwoStageDiD replicate refits still pay for full GMM variance computation inside every replicate even though the refit only consumes point estimates. _refit_ts() in diff_diff/two_stage.py:571 calls _stage2_static(), _stage2_event_study(), and _stage2_group(), and each of those helpers computes _compute_gmm_variance() at diff_diff/two_stage.py:1080, diff_diff/two_stage.py:1249, and diff_diff/two_stage.py:1371. That will materially slow replicate-weight inference for aggregate="event_study" and aggregate="all" on larger panels. Concrete fix: add point-estimate-only Stage 2 helpers, or a return_variance=False fast path, and use that inside the replicate closure.

Maintainability

  • No findings.

Tech Debt

  • Severity P3-informational. Impact: the new BRR smoke tests still use Fay-like 0.5/1.5 perturbations rather than true half-sample BRR, but the PR now tracks that limitation in TODO.md:72. Concrete fix: none before merge; the tracked follow-up is sufficient.

Security

  • No findings.

Documentation/Tests

  • Severity P3. Impact: the estimator-local REGISTRY notes for ImputationDiD and TwoStageDiD still describe only the legacy TSL/bootstrap survey paths, so readers can miss that these estimators now also support replicate weights via estimator-level refits and reject replicate+bootstrap. Concrete fix: update docs/methodology/REGISTRY.md:883 and docs/methodology/REGISTRY.md:962 to mirror the new support-matrix language in the replicate-weight section.
  • Severity P3. Impact: compute_replicate_refit_variance() at diff_diff/survey.py:1706 is covered indirectly by estimator smoke tests, but I did not see helper-level tests mirroring the existing compute_replicate_vcov() cases for mse, custom rscales, and dropped-replicate n_valid behavior. Concrete fix: add direct helper tests in tests/test_survey_phase6.py:414, tests/test_survey_phase6.py:830, and tests/test_survey_phase6.py:1103 alongside the existing replicate-vcov coverage.
  • Static review only; I did not run the added tests locally.

@igerber igerber merged commit a729f00 into main Apr 2, 2026
14 checks passed
@igerber igerber deleted the replicate-weight-expansion branch April 2, 2026 18:00
igerber added a commit that referenced this pull request Apr 2, 2026
@igerber @claude
Bump version to 2.8.3, add Phase 8 survey maturity roadmap
b421259 
Version bump covering silent warning audit (#256), replicate weight
expansion (#253), and imputation pre-period coefficients (#252).
Adds Phase 8 survey maturity roadmap (SDR, FPC expansion, lonely PSU,
CV, weight trimming, compatibility matrix).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@igerber igerber mentioned this pull request Apr 2, 2026
Merged
@igerber igerber mentioned this pull request Jul 7, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@igerber