Lift hc2_bm + weights gates via clubSandwich WLS-CR2 port

[igerber]

Summary

• Lifts the two NotImplementedError gates in _validate_vcov_args blocking vcov_type="hc2_bm" + weights (TODO.md rows 104-105, "Gates 4 and 5")
• Ports clubSandwich's specific WLS-CR2 algebra into _compute_cr2_bm / _compute_cr2_bm_contrast_dof / _compute_bm_dof_from_contrasts (W not √W in hat matrix, W² in bias-correction term, unweighted residuals in score, full H1/H2/H3 array Satterthwaite DOF)
• Threads weighted Bell-McCaffrey DOF through LinearRegression._bm_dof and LinearRegression.get_inference()
• Adds NaN-guard for FP-noise-floor high-leverage FE-dummy contrasts (where BLAS reduction order differences between NumPy and R produce 15-30% DOF disagreement despite vcov matching at machine precision)
• 6 new weighted scenarios in benchmarks/data/clubsandwich_cr2_golden.json pin Python vs R parity at atol=1e-10 (vcov + non-noise-floor DOF + compound-contrast DOF); existing 6 unweighted scenarios unchanged
• Public surface restrictions: weight_type ∈ {"aweight", "fweight"} + hc2_bm + weights raises NotImplementedError (port matches pweight only); zero-total-weight clusters dropped with effective-cluster ≥ 2 guard

Methodology references

• Method: WLS-CR2 Bell-McCaffrey cluster-robust variance + Satterthwaite DOF
• Source: clubSandwich v0.7.0 (Pustejovsky 2024) R source — R/CR-adjustments.R::CR2, R/clubSandwich.R::vcov_CR, R/coef_test.R::Satterthwaite_df, R/get_arrays.R::get_GH. Foundational papers: Bell & McCaffrey (2002), Pustejovsky & Tipton (2018) JBES, Imbens & Kolesar (2016) ReStat.
• Intentional deviation from textbook: The diff-diff port matches clubSandwich's specific algebra, NOT a textbook PT2018 §3.3 transform-once derivation. Per prior empirical finding (feedback_wls_cr2_clubsandwich_parity), the textbook reading diverges from clubSandwich by 0.5-30% on weighted designs. clubSandwich uses W (not √W) in the hat matrix, W² in the bias term, and unweighted residuals in the score construction. Documented in docs/methodology/REGISTRY.md Phase 1a section.
• Documented precision limit: high-leverage FE-dummy contrasts (e.g., unit dummies in a balanced DiD) have Satterthwaite DOF at the float64 noise floor; the helper returns NaN with a UserWarning rather than ship BLAS-implementation-dependent values.

Validation

• Tests added: tests/test_methodology_wls_cr2.py (19 tests: clubSandwich parity at atol=1e-10 across 6 weighted scenarios + compound-contrast DOF + unweighted regression safety + dual-path equivalence + weight-type rejection + zero-weight cluster rejection + LinearRegression _bm_dof threading + LinearRegression NaN inference end-to-end + FE-dummy noise-floor guard)
• Tests updated: tests/test_linalg_hc2_bm.py flipped two "gate raises NotImplementedError" tests to "gate lifted, produces finite vcov+DOF" smoke tests
• Step 0 R/Python smoke parity verified at atol=1e-15 before source edits (per feedback_r_source_smoke_test_before_implementing)
• Local codex review iterated R1-R8 to ✅ clean (8 rounds)
• 340 tests pass across linalg + methodology + estimators + TWFE + SA + estimators-vcov-type

Security / privacy

• Confirm no secrets/PII in this PR: Yes (canonical secret-pattern scan clean)

Test plan

• Verify R-parity tests pass: pytest tests/test_methodology_wls_cr2.py
• Verify unweighted regression safety: pytest tests/test_linalg_hc2_bm.py
• Verify broader regression: pytest tests/test_estimators_vcov_type.py tests/test_methodology_twfe.py tests/test_methodology_sun_abraham.py tests/test_estimators.py
• CI codex review (local convergence is necessary but not sufficient per feedback_local_codex_vs_ci_codex_divergence)

---

🤖 Generated with Claude Code

[github-actions]

Overall Assessment

✅ Looks good

Executive Summary

• The core methodology is aligned with the Methodology Registry: this is a documented clubSandwich WLS-CR2 port, not a textbook PT2018 transform-once implementation, and that deviation is explicitly documented in docs/methodology/REGISTRY.md:L2651-L2656, so it is not a defect.
• The main public-surface risks called out in the PR are covered in code and tests: pweight-only gating, zero-total-weight cluster handling, weighted one-way singleton-cluster reduction, and LinearRegression BM-DOF threading.
• I found no unmitigated P0/P1 issues in the changed public surfaces (compute_robust_vcov, solve_ols, LinearRegression).
• P2 The new noise-floor NaN guard is batch-relative, so weighted single-contrast calls to _compute_cr2_bm_contrast_dof can still emit BLAS-dependent finite DOF even though the Registry/CHANGELOG describe the helper as guarded. diff_diff/linalg.py:L1852-L1882, diff_diff/linalg.py:L1887-L2005, docs/methodology/REGISTRY.md:L2651-L2656, CHANGELOG.md:L13, tests/test_methodology_wls_cr2.py:L169-L215, tests/test_methodology_wls_cr2.py:L476-L533.
• I could not run pytest here because the sandbox is missing the Python numerical stack.

Methodology

• P2 Impact: _cr2_bm_dof_inner_weighted only suppresses a contrast when its max|P| is tiny relative to a larger contrast in the same batch. For a single weighted contrast, max|P|_overall is the contrast’s own scale, so the 1e-10 × max|P|_overall rule can never classify it as degenerate. That leaves direct single-contrast weighted calls to _compute_cr2_bm_contrast_dof able to return BLAS-dependent finite DOF on the exact noise-floor case the docs say should be suppressed. Current public estimator surfaces do not appear to rely on that path, so this is non-blocking. Concrete fix: add a single-contrast instability criterion (absolute/self-scaled, not batch-relative), or narrow the docs to say the NaN guard applies only to batched per-coefficient calls; add a regression test with a weighted FE-dummy single contrast. diff_diff/linalg.py:L1852-L1882, diff_diff/linalg.py:L1887-L2005, docs/methodology/REGISTRY.md:L2651-L2656
• P3 Impact: the documented deviation from textbook PT2018 to clubSandwich-specific WLS algebra is clearly disclosed and matches the implementation, so I am treating it as an accepted implementation choice, not a defect. Concrete fix: none. diff_diff/linalg.py:L1495-L1654, docs/methodology/REGISTRY.md:L2651-L2656, CHANGELOG.md:L13

Code Quality

No findings.

Performance

• P3 Impact: LinearRegression.fit() now pays for CR2 twice on the new weighted hc2_bm path: once inside solve_ols(..., return_vcov=True) and again via compute_robust_vcov(..., return_dof=True) to fill _bm_dof. Correctness is fine, but it adds a second full CR2 pass on the heaviest new path. Concrete fix: let solve_ols optionally return DOF from the same CR2 computation, or cache/share the CR2 precomputes. diff_diff/linalg.py:L3235-L3276

Maintainability

No separate findings beyond the duplicated CR2 work above.

Tech Debt

No findings.

Security

No findings.

Documentation/Tests

• No documentation defect: the Registry, TODO lift, and changelog all match the implemented pweight-only contract and the clubSandwich-specific algebra. docs/methodology/REGISTRY.md:L2651-L2656, TODO.md:L104-L110, CHANGELOG.md:L13
• Test coverage is strong for parity, weight-type rejection, zero-weight clusters, unweighted regression safety, and LinearRegression NaN propagation; the remaining gap is the single-contrast noise-floor case noted above. tests/test_methodology_wls_cr2.py:L73-L215, tests/test_methodology_wls_cr2.py:L218-L600
• I did not execute the suite here because the sandbox lacks NumPy/pytest.

Assumption

• I treated _compute_cr2_bm_contrast_dof as an internal helper. If it is considered supported API rather than internal implementation detail, I would raise the single-contrast noise-floor issue one severity level.

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: 8f00e976d33dcd3600239bebcdfaf8ce5d72960b

---

Overall Assessment

⛔ Blocker

Executive Summary

• The prior non-blocking single-contrast noise-floor concern appears resolved: _cr2_bm_dof_inner_weighted() now has an absolute single-contrast floor, and the new regression tests cover both the guarded and non-guarded single-contrast cases. diff_diff/linalg.py:L1852-L1903, tests/test_methodology_wls_cr2.py:L544-L623
• P0 The new weighted clustered CR2 path is still wrong on supported pweight inputs when a positive-weight cluster contains some zero-weight padding rows. Those zero-weight rows still enter the CR2 adjustment blocks and can silently change SE/DOF instead of being inert. diff_diff/linalg.py:L1548-L1654, diff_diff/linalg.py:L1951-L2024, diff_diff/linalg.py:L2179-L2188
• The documented methodology deviation from a textbook PT2018 transform-once reading is properly recorded in the Methodology Registry, so I do not treat that clubSandwich-specific algebra choice as a defect. docs/methodology/REGISTRY.md:L2651-L2656
• Weight-type rejection (aweight/fweight) and LinearRegression._bm_dof threading look correctly implemented and covered on the changed public paths. diff_diff/linalg.py:L2263-L2317, diff_diff/linalg.py:L3272-L3315, tests/test_methodology_wls_cr2.py:L214-L298, tests/test_methodology_wls_cr2.py:L358-L457
• I could not run pytest here because the sandbox does not have NumPy/pytest.

Methodology

• Severity: P0. Impact: weighted clustered CR2 is not subpopulation-invariant on supported pweight inputs with mixed zero/nonzero weights inside a cluster. The new code drops only zero-total-weight clusters, but it still builds S_W, H_gg, G_g, and A_g on X_g for all rows in a positive-weight cluster. Because zero-weight rows remain in the row side of H_gg and in bias_term = X_g @ MUWTWUM @ X_g.T, excluded observations can still alter the adjustment matrix and therefore the reported vcov/DOF. That is silent wrong statistical output on an allowed input, and it contradicts the surrounding linalg contract that zero-weight rows “contribute nothing to the sandwich.” The new tests only cover all-zero clusters, not this mixed-zero case. Concrete fix: before weighted clustered CR2/contrast-DOF construction, filter to weights > 0 (globally or per cluster) so mixed-zero rows are fully removed from X, residuals, cluster_ids, and weights; then add an invariance test against the physically dropped positive-weight subset. Refs: diff_diff/linalg.py:L1548-L1654, diff_diff/linalg.py:L1951-L2024, diff_diff/linalg.py:L2179-L2188, tests/test_methodology_wls_cr2.py:L301-L355, docs/methodology/REGISTRY.md:L2651-L2656
• Severity: P3 informational. Impact: the clubSandwich-specific WLS algebra (W, not sqrt(W); W^2 bias term; unweighted residual score) is documented in the Registry, so that deviation from a textbook reading is acceptable and not a defect. Concrete fix: none. Refs: docs/methodology/REGISTRY.md:L2651-L2656, diff_diff/linalg.py:L1504-L1527

Code Quality

No findings.

Performance

• Severity: P3 informational, tracked in TODO.md. Impact: LinearRegression.fit() still pays for CR2 twice on the weighted hc2_bm path: once via solve_ols(..., return_vcov=True) and again via compute_robust_vcov(..., return_dof=True) to populate _bm_dof. Concrete fix: thread return_dof through solve_ols or cache/share the CR2 precomputes. Refs: diff_diff/linalg.py:L3251-L3295, TODO.md:L111-L111

Maintainability

No findings.

Tech Debt

• Severity: P3. Impact: TODO.md now says rows 104-105 are lifted, but Tier C still separately lists “Weighted one-way Bell-McCaffrey + weighted CR2 Bell-McCaffrey” as outstanding, so the deferred-work inventory is self-contradictory. Concrete fix: remove or rewrite the stale Tier C bullet. Refs: TODO.md:L104-L111, TODO.md:L210-L210

Security

No findings.

Documentation/Tests

• Severity: P3. Impact: the Registry and changelog still describe the noise-floor guard only as the batch-relative 1e-10 × max|P| rule, but the shipped code now also applies an absolute single-contrast floor to fix the previous review item. The docs therefore underdescribe when NaN suppression can fire on direct single-contrast calls. Concrete fix: update REGISTRY.md and CHANGELOG.md to mention the absolute single-contrast floor (or describe the full max(relative_floor, abs_noise_floor) rule). Refs: diff_diff/linalg.py:L1862-L1885, docs/methodology/REGISTRY.md:L2651-L2651, CHANGELOG.md:L13-L13, tests/test_methodology_wls_cr2.py:L544-L623
• I did not execute the suite here because NumPy/pytest are unavailable in the sandbox.

Path to Approval

1. Make weighted clustered CR2 ignore zero-weight rows inside otherwise positive-weight clusters, not just zero-total-weight clusters.
2. Add regression tests that compare clustered vcov_type="hc2_bm" + weight_type="pweight" on mixed-zero clusters to the physically dropped positive-weight subset, covering both per-coefficient DOF and contrast DOF on at least one public entry point.

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: a2fa8016660437d5972b394a32ca4965f4bd6c88

---

Overall Assessment

✅ Looks good

Executive Summary

• The prior blocker from the last review appears resolved: both weighted CR2 builders now physically drop weights == 0 rows before any cluster-block construction, and the new invariance tests cover the mixed-zero case.
• The clubSandwich-specific WLS-CR2 algebra and the FE-dummy noise-floor NaN guard are documented in the Methodology Registry, so I do not treat those deviations from a textbook PT2018 reading as defects.
• LinearRegression now threads weighted Bell-McCaffrey DOF into get_inference() and returns all-NaN inference when the noise-floor guard fires, which fixes the earlier fallback-to-n-k / normal-theory problem.
• Remaining items are non-blocking: the Registry row should explicitly say the lift is pweight-only, and weighted hc2_bm still recomputes CR2 twice in LinearRegression.fit() (already tracked in TODO.md).
• I could not run the test suite here because the sandbox lacks NumPy/pytest.

Methodology

No P0/P1 findings. The registry now documents the load-bearing methodology choices and precision-limit behavior in docs/methodology/REGISTRY.md:L2651-L2656, and the prior mixed-zero-weight blocker appears resolved by the new weights > 0 filtering in both weighted CR2 builders diff_diff/linalg.py:L1548-L1583 and diff_diff/linalg.py:L1969-L1993, with regression coverage in tests/test_methodology_wls_cr2.py:L536-L724.

• Severity: P3. Impact: The new Phase 1a registry row says weighted hc2_bm + weights is supported on the analytical linalg surface, but unlike the in-code docs it does not record the load-bearing pweight-only restriction. That overstates the supported surface in the project’s methodology registry even though the code correctly rejects aweight and fweight. Refs: docs/methodology/REGISTRY.md:L2651-L2656, diff_diff/linalg.py:L1259-L1267. Concrete fix: Add one sentence to the Phase 1a registry row stating that the clubSandwich WLS-CR2 port matches weight_type="pweight" only and that aweight / fweight remain intentionally unsupported.

Code Quality

No findings.

Performance

• Severity: P3. Impact: LinearRegression.fit() still pays for the weighted hc2_bm CR2 path twice: once via solve_ols(..., return_vcov=True) and again via compute_robust_vcov(..., return_dof=True) to populate _bm_dof. This is correct but redundant on the newly opened path. Refs: diff_diff/linalg.py:L3306-L3336, TODO.md:L111-L111. Concrete fix: Thread return_dof through solve_ols() or cache/share the weighted-CR2 precomputes (A_g, MUWTWUM) between the vcov and DOF calls.

Maintainability

No findings.

Tech Debt

No additional findings beyond the tracked weighted-CR2 double-compute follow-up in TODO.md:L111-L111.

Security

No findings.

Documentation/Tests

No blocking findings. The new methodology harness covers parity, weight-type rejection, effective-cluster rejection, mixed-zero-row subpopulation invariance, single-contrast noise-floor guarding, and LinearRegression end-to-end NaN inference propagation in tests/test_methodology_wls_cr2.py:L218-L808.

I could not execute the suite here because the sandbox lacks NumPy/pytest.

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: 4fe217e8d2cbb1cf9e4b3347ff0025fd602a71a6

---

Overall Assessment

✅ Looks good

Executive Summary

• The prior re-review’s remaining documentation nit is resolved: the Phase 1a registry row now explicitly scopes weighted hc2_bm to the analytical linalg surface and weight_type="pweight" only, matching the new code paths.
• I did not find any unmitigated P0/P1 issues in the weighted CR2 lift. The two earlier correctness risks this PR family had been carrying are addressed in the changed code: zero-weight padded rows are dropped before CR2 block algebra, and LinearRegression.get_inference() now suppresses all inference fields when BM DOF is NaN instead of silently falling back to normal-theory inference.
• The methodology change is consistent with the cited source material and the Methodology Registry; I do not treat the documented clubSandwich-specific algebra or the documented noise-floor NaN behavior as defects.
• Test coverage is strong for the new surface: R-golden parity, weight-type rejection, effective-cluster rejection, zero-weight subpopulation invariance, noise-floor guarding, and LinearRegression end-to-end threading.
• The only remaining issue I saw is the already-tracked double-compute on weighted hc2_bm inside LinearRegression.fit(), which is P3/non-blocking.

Methodology

• No P0/P1 findings. The weighted CR2 lift matches the clubSandwich source path the PR says it ports: weighted lm objects default to the non-inverse_var branch, CR2 builds the weighted H_jj / MUWTWUM adjustment, and get_GH + get_P_array + Satterthwaite_df define the H-array/P-array Satterthwaite DOF used by coef_test. The local implementation in diff_diff/linalg.py:L1495-L2052 and the registry note in docs/methodology/REGISTRY.md:L2651-L2656 are consistent with that contract. (rdrr.io)

Code Quality

• No findings.

Performance

• P3 Impact: LinearRegression.fit() still computes weighted CR2 twice on the opened hc2_bm path, once through solve_ols(..., return_vcov=True) and again through compute_robust_vcov(..., return_dof=True). This is already tracked in TODO.md:L111-L111, so it is mitigated and non-blocking. Concrete fix: thread return_dof through solve_ols() or cache the CR2 precomputes reused by both calls. Refs: diff_diff/linalg.py:L3300-L3339, TODO.md:L111-L111.

Maintainability

• No findings.

Tech Debt

• No new untracked findings.

Security

• No findings.

Documentation/Tests

• No findings. The prior registry scope gap is fixed in docs/methodology/REGISTRY.md:L2651-L2656, and the new coverage in tests/test_methodology_wls_cr2.py:L73-L884 plus tests/test_linalg_hc2_bm.py:L302-L429 hits the right edge cases for this lift. I could not execute the tests here because this environment lacks numpy and pytest.