MultiPeriodDiD: streamline fixed-effects dummy expansion

[igerber]

Minor cleanup of the fixed-effects dummy-expansion loop in MultiPeriodDiD.fit. Also bumps the CI Codex review model to gpt-5.5.

[github-actions]

⛔ Blocker

Executive Summary

• The PR removes the covariate/design-term collision guards from DifferenceInDifferences, MultiPeriodDiD, and TwoWayFixedEffects, reintroducing silent corruption of result.coefficients.
• MultiPeriodDiD now expands fixed_effects=[time] again, directly contradicting the current REGISTRY note and breaking the documented coefficients-vs-vcov alignment contract.
• The deleted tests were the exact regression coverage for these silent correctness failures.
• I could not run the suite here: pytest and numpy are not installed. Review is static, but the failure paths are direct from var_names construction and dict zipping.
• The CI model bump to gpt-5.5 is not flagged; official OpenAI API docs list gpt-5.5 as a valid model ID. (platform.openai.com)

Methodology

P0 — MultiPeriodDiD no longer skips redundant time fixed effects

Impact: MultiPeriodDiD.fit(..., fixed_effects=[time]) and HC2/HC2-BM absorb=[..., time] now emit a second <time>_<period> dummy block even though the event-study design already includes period_<p> terms. The current REGISTRY explicitly documents the required redundant time-FE skip and says without it MultiPeriodDiDResults.coefficients silently drops duplicate names and breaks coefficients-vs-vcov alignment. The PR removes that skip at diff_diff/estimators.py:L1607-L1613, while coefficients are still built with a duplicate-dropping dict at diff_diff/estimators.py:L1973-L1974. The existing regression test documents and pins this exact contract at tests/test_estimators_vcov_type.py:L1825-L1870.

Concrete fix: Restore the if fe == time: continue guard in the MultiPeriodDiD fixed-effects loop, including the absorb-auto-route path. Keep or restore the invariant test asserting len(res.coefficients) == res.vcov.shape[0] for both absorb=["unit", time] and fixed_effects=["unit", time].

P0 — Covariate/design-term name collisions silently corrupt coefficient output

Impact: The PR removes validate_covariate_names, validate_design_term_names, and fe_dummy_names, then appends user covariates and FE dummy names directly into var_names. The result is still created with {name: coef for name, coef in zip(...)}, so duplicate names silently overwrite earlier statistical coefficients. This affects:

• DifferenceInDifferences: covariates appended at diff_diff/estimators.py:L472-L486; dict zip at diff_diff/estimators.py:L663-L664.
• MultiPeriodDiD: covariates/FE dummies appended at diff_diff/estimators.py:L1601-L1613; dict zip at diff_diff/estimators.py:L1973-L1974.
• TwoWayFixedEffects full-dummy HC2/HC2-BM path: covariates included in _twfe_var_names at diff_diff/twfe.py:L336-L340; dict zip at diff_diff/twfe.py:L650-L663.

Examples include covariates named const, treated:post, period_2, treated:period_2, ATT, _treatment_post, or an FE dummy name. These produce wrong coefficient dictionaries with no warning, which is a silent statistical-output bug.

Concrete fix: Restore the shared validation helpers and call them before design construction in all three estimators. Also keep the final validate_design_term_names(var_names, ...) backstop before any coefficient dict is created.

Code Quality

P1 — Internal working-column collisions are no longer fail-closed

Impact: TwoWayFixedEffects.fit() writes data["_treatment_post"] = data[treatment] * data[time] at diff_diff/twfe.py:L278-L280. Without the removed guard, a user covariate named _treatment_post is overwritten and then can be included again as a covariate in the demeaned/full-dummy design. Similar risks exist for DiD _treat_time and MPD _did_* columns.

Concrete fix: Include all internal working columns in the reserved-name validation set and raise ValueError before mutating the working frame.

Performance

No performance findings beyond the correctness regressions above. Removing fe_dummy_names() avoids an O(G) name derivation, but that saving is not acceptable because it removes the fail-closed collision check.

Maintainability

P2 — Safety behavior was removed from code, docs, and tests together

Impact: The PR deletes the helper functions, removes their docstring/API references, removes methodology notes, and deletes the regression tests. That makes a known silent-corruption class harder to rediscover and reintroduces the same failure mode across three estimators at once.

Concrete fix: Restore the helpers and tests. If the implementation is refactored, keep the behavior pinned by estimator-level tests rather than relying only on utility tests.

Tech Debt

No mitigating TODO entry exists under “Tech Debt from Code Reviews” for these regressions. In any case, these are P0/P1 correctness issues, so TODO tracking would not mitigate them under the review policy.

Security

No security findings. The workflow model change itself is not a security issue.

Documentation/Tests

P1 — Regression tests for silent coefficient corruption were removed

Impact: The deleted tests in tests/test_estimators.py, tests/test_estimators_vcov_type.py, and tests/test_utils.py were covering the exact P0 paths above: structural-name covariate collisions, duplicate covariates, FE dummy collisions, TWFE _treatment_post, and MPD time-FE duplicate expansion.

Concrete fix: Restore tests for:

• DiD covariates colliding with const, treatment/time names, interaction name, _treat_time, and FE dummy names.
• MPD covariates colliding with period_<p>, treatment-period interactions, _did_*, and FE dummy names.
• TWFE covariates colliding with ATT, _treatment_post, and unit/time dummy names on both HC1 and HC2 paths.
• MPD fixed_effects=[time] and absorb=[..., time] coefficient/vcov alignment.

Path To Approval

1. Restore validate_covariate_names, validate_design_term_names, and fe_dummy_names.
2. Re-add estimator calls to those validators before design construction and before coefficient dict creation.
3. Restore the MultiPeriodDiD if fe == time: continue skip in the fixed-effects dummy loop.
4. Restore the removed collision and MPD time-FE regression tests.
5. Restore the REGISTRY notes documenting the fail-closed collision behavior and redundant time-FE skip.

[igerber]

Canary smoke test complete (validating the gpt-5.5 CI reviewer end-to-end). Closing without merging; not a real change.