Skip to content

test(se-audit): CI-lock flagship + previously-unasserted SE parity (G2 + C1-C3/C5)#624

Merged
igerber merged 1 commit into
mainfrom
feature/se-audit-ci-locks
Jul 6, 2026
Merged

test(se-audit): CI-lock flagship + previously-unasserted SE parity (G2 + C1-C3/C5)#624
igerber merged 1 commit into
mainfrom
feature/se-audit-ci-locks

Conversation

@igerber

@igerber igerber commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • CI-lock standard-error parity for flagship + previously-unasserted paths (SE-audit coverage batch: G2 + C1-C3/C5). No source (diff_diff/) changes — tests, a new fixest golden, and changelog/TODO only. These surfaces already computed SEs matching R but had no CI assertion pinning them (the latent-risk pattern that once hid the CallawaySantAnna reg-method gap).
  • G2: new committed fixest DiD/TWFE golden (benchmarks/data/fixest_did_twfe_golden.json + benchmarks/R/generate_fixest_did_twfe_golden.R + tests/test_fixest_did_twfe_parity.py). The flagship 2×2 DiD and TWFE classical/iid ATT and SE match fixest::feols to ~1e-16 in CI without R at test time (the prior live-Rscript tests asserted only att at rtol=1e-3 and skipped in CI). The TWFE lock also pins the D4 full-K within-transform rescale.
  • C2: TwoWayFixedEffects(vcov_type="hc2_bm")'s default CR2-clustered-at-unit SE + the one-way HC2-BM SE (both were DOF-only) + MultiPeriodDiD average-effect CR2 SE (was finite-only).
  • C1: StackedDiD event-study coefficient point estimates.
  • C3: PlaceboTests leave-one-out t_stat / p_value.
  • C5: DIDHAD QUG order-statistic test t_stat / p_value (was entirely untested vs the golden).

Methodology references (required if estimator / math changes)

  • Method name(s): fixest::feols (2×2 DiD, TWFE); clubSandwich CR2 / Bell-McCaffrey (TWFE/MPD hc2_bm); StackedDiD; PlaceboTests leave-one-out; de Chaisemartin–D'Haultfœuille DIDHAD QUG.
  • Paper / source link(s): documented in docs/methodology/REGISTRY.md (TWFE full-K within-transform; CR2/HC2-BM; StackedDiD CR1S/CR2; QUG). No methodology behavior changed — this PR only pins existing, R-matching SEs.
  • Any intentional deviations from the source (and why): None new. Deferred coverage (cluster/hetero SE, intercept SEs, DOF/df, Yatchew p/sigma2) is tracked in TODO.md.

Validation

  • Tests added/updated: tests/test_fixest_did_twfe_parity.py (new), test_methodology_twfe.py, test_estimators_vcov_type.py, test_methodology_stacked_did.py, test_methodology_placebo.py, test_did_had_parity.py; new golden benchmarks/data/fixest_did_twfe_golden.json (+ generator). 232 tests pass across the touched files; classical/iid fixest SE parity to ~1e-16, coverage locks to ~1e-10.
  • Backtest / simulation / notebook evidence (if applicable): N/A.

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

🤖 Generated with Claude Code

https://claude.ai/code/session_0178r8ZK6VRiXbaBiknrWjQB

…2 + C1-C3/C5)

Commit standard-error assertions for surfaces that already matched R but had no
CI lock pinning them (the latent-risk pattern that once hid the CallawaySantAnna
reg-method gap). No source (diff_diff/) changes.

- G2: new fixest DiD/TWFE golden (benchmarks/data/fixest_did_twfe_golden.json +
  benchmarks/R/generate_fixest_did_twfe_golden.R + tests/test_fixest_did_twfe_parity.py).
  The flagship 2x2 DiD and TWFE classical/iid ATT+SE match fixest::feols to
  ~1e-16 in CI without R at test time (the prior live-Rscript tests asserted only
  att at rtol=1e-3 and skipped in CI); the TWFE lock also pins the D4 full-K
  within-transform rescale.
- C2: TWFE(hc2_bm) default CR2-clustered-at-unit SE + the one-way HC2-BM SE
  (prior tests asserted only the Satterthwaite DOF) + MultiPeriodDiD
  average-effect CR2 SE (prior test only checked finite).
- C1: StackedDiD event-study coefficient point estimates.
- C3: PlaceboTests leave-one-out t_stat / p_value.
- C5: DIDHAD QUG order-statistic test t_stat / p_value (was untested vs golden).

All at ~1e-10 (coverage) / ~1e-16 (fixest classical iid). 232 tests pass across
the touched files, no regressions.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0178r8ZK6VRiXbaBiknrWjQB
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

Overall Assessment

✅ Looks good

Executive Summary

  • No source estimator code changed; this PR adds parity locks, a committed fixest golden, changelog notes, and tracked follow-up debt.
  • Methodology checks align with docs/methodology/REGISTRY.md for DiD/TWFE fixest, CR2/HC2-BM, StackedDiD, PlaceboTests, and HAD QUG.
  • The deferred SE gaps are explicitly tracked in TODO.md, so they are P3 informational, not blockers.
  • No new inline inference computation or NaN/Inf anti-pattern appears in modified code.
  • I could not run tests in this environment because pytest is not installed.

Methodology

No P0/P1 findings.

P3 informational: Cluster SE parity and other “fiddly bits” are deferred but tracked.

  • Impact: tests/test_fixest_did_twfe_parity.py:L79-L91 asserts cluster ATT only, not cluster SE, but TODO.md:L69 documents the CR1 DOF-convention difference vs fixest and the remaining follow-ups.
  • Concrete fix: No action required for this PR; follow the tracked TODO when addressing that parity gap.

Code Quality

No findings.

Performance

No findings.

Maintainability

No findings.

Tech Debt

P3 informational: Follow-up coverage debt is properly recorded.

  • Impact: Deferred items for cluster SE, intercept SEs, Placebo df/boundary_gap, Yatchew fields, and DOF exposure are listed under TODO.md:L69.
  • Concrete fix: No action required for approval.

Security

No findings. No secrets or credential-like content observed in the changed files.

Documentation/Tests

No P0/P1 findings.

The added tests cover the intended previously-unasserted fields:

  • fixest DiD/TWFE ATT and iid SE: tests/test_fixest_did_twfe_parity.py:L58-L77
  • MPD average CR2 SE: tests/test_estimators_vcov_type.py:L651-L658
  • TWFE HC2-BM/CR2 SEs: tests/test_methodology_twfe.py:L1299-L1312, tests/test_methodology_twfe.py:L1364-L1382
  • StackedDiD event-study coefficients: tests/test_methodology_stacked_did.py:L243-L250
  • Placebo LOO t_stat/p_value: tests/test_methodology_placebo.py:L242-L250
  • HAD QUG t_stat/p_value: tests/test_did_had_parity.py:L522-L540

Verification note: I attempted to run the touched tests, but this environment lacks pytest (python -m pytest reports No module named pytest).

@igerber igerber added the ready-for-ci Triggers CI test workflows label Jul 6, 2026
@igerber igerber merged commit fec5357 into main Jul 6, 2026
25 of 26 checks passed
@igerber igerber deleted the feature/se-audit-ci-locks branch July 6, 2026 16:30
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