dCDH: add by_path per-path event-study disaggregation

[igerber]

Summary

• Adds by_path: Optional[int] = None to ChaisemartinDHaultfoeuille — top-k most common observed treatment paths in the window [F_g-1, F_g-1+L_max] get their own per-horizon DID + SE, mirroring R did_multiplegt_dyn(..., by_path=k). Suggested by Clément de Chaisemartin.
• Per-path SE follows the joiners-only / leavers-only IF precedent: switcher-side contribution zeroed for non-path groups; control pool and cohort structure (D_{g,1}, F_g, S_g) unchanged; plug-in SE with path-specific divisor N_l_path (same pattern as joiners_se using joiner_total).
• Strict first-PR scope — drop_larger_lower=False + L_max >= 1 + binary treatment required; combinations with controls, trends_linear, trends_nonparam, heterogeneity, design2, honest_did, survey_design, and n_bootstrap > 0 raise NotImplementedError with targeted messages.

What practitioners see

The smoke case in the test fixture (6 switchers × 3 distinct paths, treatment effect = 2.0) yields:

• Path (0,1,1,1) (stay on) → effect ≈ 2.0 at all horizons
• Path (0,1,0,0) (single pulse) → effect ≈ 2.0 at l=1, then ≈ 0 at l=2, 3
• Path (0,1,1,0) (two on, then off) → effect ≈ 2.0 at l=1, 2, then ≈ 0 at l=3

Results exposed on results.path_effects: Dict[Tuple[int, ...], Dict[str, Any]] and results.to_dataframe(level="by_path"); summary grows a "Treatment-Path Disaggregation" block. Ties in path frequency are broken lexicographically on the path tuple for deterministic ranking. Overflow (by_path > n_observed_paths) returns all observed paths with a UserWarning.

Methodology

See docs/methodology/REGISTRY.md §ChaisemartinDHaultfoeuille Note (Phase 3 by_path per-path event-study disaggregation) for the full contract (window convention, top-k selection rule + tiebreak, per-path SE derivation, strict-first-PR scope).

Test plan

• tests/test_chaisemartin_dhaultfoeuille.py::TestByPathGates — 13 gate tests (param validation, drop_larger_lower precondition, L_max precondition, n_bootstrap/controls/trends/heterogeneity/design2/honest_did/survey_design/non-binary gates, get_params/set_params plumbing)
• tests/test_chaisemartin_dhaultfoeuille.py::TestByPathBehavior — top-k selection, lexicographic tiebreak, overflow warning, result dict shape, hand-calculable DGP recovery, summary render, to_dataframe("by_path")
• Full dCDH regression — tests/test_chaisemartin_dhaultfoeuille{,_parity,}.py + test_methodology_chaisemartin_dhaultfoeuille.py + test_survey_dcdh*.py + test_dcdh_*_coverage.py (323 tests pass, no regressions)
• R-parity fixture extension for by_path — deferred to a follow-up PR (the SE convention against R's did_multiplegt_dyn is to be confirmed in the parity work)

Deferred (follow-up PRs)

• R-parity fixtures + TestDCDHDynRParityByPath
• paths_of_interest: List[Tuple[int, ...]] for user-specified path selection
• by_path + each of controls / trends_linear / trends_nonparam / heterogeneity / design2 / honest_did / survey_design / non-binary treatment / n_bootstrap > 0
• Per-path placebos (DID^{pl}_l per path) and per-path sup-t bands

🤖 Generated with Claude Code

[github-actions]

Overall Assessment

⚠️ Needs changes — one unmitigated P1 in the new by_path result surface.

Executive Summary

• Affected method: ChaisemartinDHaultfoeuille multi-horizon DID_l event study, specifically the new per-path disaggregation documented in docs/methodology/REGISTRY.md.
• Main blocker: the implementation does not handle the empty-path case correctly; when by_path is requested but no group has a full [F_g-1, F_g-1+L_max] window, the result collapses to path_effects=None and behaves as if by_path was never requested.
• Secondary: path-level degenerate-cohort cases are NaN-consistent, but the user-facing warning promised in the Registry is not surfaced.
• Secondary: the new public docstring says the by-path SE uses divisor N_l, while the implementation and Registry use N_l_path.
• Tests cover happy-path ranking/shape/overflow cases, but not the empty-path or degenerate-path branches.

Methodology

• P1 diff_diff/chaisemartin_dhaultfoeuille.py:L4868-L5002, diff_diff/chaisemartin_dhaultfoeuille.py:L1934-L1958, diff_diff/chaisemartin_dhaultfoeuille_results.py:L1013-L1017, diff_diff/chaisemartin_dhaultfoeuille_results.py:L1363-L1369 — Impact: when by_path is requested but no switcher has a complete [F_g-1, F_g-1+L_max] window, _compute_path_effects() returns None. That collapses “requested but empty” into the same state as “feature not requested”: the summary silently omits the block and to_dataframe(level="by_path") raises “Pass by_path=k” even though the caller already did. This is the empty-result-set edge case the review rubric explicitly flags, and it is reachable whenever L_max is valid overall but all switchers are too late in the panel or have missing cells in the full path window. Concrete fix: return an empty dict instead of None when by_path was requested but zero paths survive, emit a targeted UserWarning, and make the results layer distinguish None from {} the same way linear_trends already distinguishes “not requested” from “requested but empty” at diff_diff/chaisemartin_dhaultfoeuille_results.py:L1329-L1348.

• P2 docs/methodology/REGISTRY.md:L641-L641, diff_diff/chaisemartin_dhaultfoeuille.py:L5059-L5079, diff_diff/chaisemartin_dhaultfoeuille.py:L2167-L2188 — Impact: the Registry says degenerate path subsets should inherit the existing degenerate-cohort warning surface, but the new path code only returns NaN SE / t / p / CI via _plugin_se() and safe_inference(). No path-scoped warning is emitted when a selected path is singleton by (D_{g,1}, F_g, S_g), which is a realistic case for low-frequency paths. Concrete fix: when n_l_path > 0 and the centered path IF is identically zero (or no variance-eligible path groups remain), emit the same warning pattern used for the overall analytical variance, scoped to (path, horizon).

• P3 diff_diff/chaisemartin_dhaultfoeuille.py:L416-L420, diff_diff/chaisemartin_dhaultfoeuille.py:L4977-L4985, docs/methodology/REGISTRY.md:L641-L641 — Impact: the new public docstring says the by-path plug-in SE uses the full-panel divisor N_l, but the implementation and Registry both use the path-specific divisor N_l_path. That is a methodology-facing documentation mismatch on the exact variance formula. Concrete fix: update the class docstring to N_l_path so the code, Registry, and user-facing API text agree.

Code Quality

No additional findings in scope.

Performance

No material performance findings in the changed code.

Maintainability

No additional findings beyond the empty-state/result-contract issue above.

Tech Debt

No new tracked-tech-debt finding beyond the items already noted above.

Security

No findings.

Documentation/Tests

• P3 tests/test_chaisemartin_dhaultfoeuille.py:L3855-L4022 — Impact: the new tests cover non-empty path selection, overflow, rendering, and DataFrame shape, but they do not exercise the two new edge branches where the current issues live: “by_path requested but zero observed paths” and “selected path has degenerate cohort recentering / NaN SE.” Concrete fix: add one late-switcher/truncated-window fixture that expects an empty by-path surface plus warning, and one rare-path fixture that expects NaN inference plus the new degenerate-cohort warning.

Path to Approval

1. Preserve the “requested but empty” by_path state in fit() and the results object instead of collapsing it to None.
2. Make summary() and to_dataframe(level="by_path") handle that empty state explicitly, with a targeted warning/message rather than the current “pass by_path=k” error.
3. Add regression tests for the zero-path and degenerate-path branches, and fix the N_l vs N_l_path docstring mismatch.

Static review only. I could not run the Python test suite in this environment because the local runtime is missing project dependencies such as pandas.

[igerber]

/ai-review

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: d4d9bd5fa99ffcf6c56cdb494d1635e5de56818a

---

Overall Assessment

✅ Looks good — the prior by_path review items are addressed, and I found no new unmitigated P0/P1 issues in the changed code.

Executive Summary

• Affected method: ChaisemartinDHaultfoeuille multi-horizon DID_l event-study surface, specifically the new by_path disaggregation.
• The prior blocker is fixed: by_path now preserves “requested but empty” as {} with a fit-time warning, summary notice, and empty to_dataframe(level="by_path") surface instead of collapsing to “not requested” (diff_diff/chaisemartin_dhaultfoeuille.py:L4959-L5019, diff_diff/chaisemartin_dhaultfoeuille_results.py:L1013-L1069, diff_diff/chaisemartin_dhaultfoeuille_results.py:L1379-L1431).
• The prior degenerate-path warning gap is fixed: path/horizon analytical SE degeneracy now emits a scoped UserWarning, and inference remains NaN-consistent via safe_inference() (diff_diff/chaisemartin_dhaultfoeuille.py:L5097-L5129, diff_diff/utils.py:L175-L209).
• The prior N_l vs N_l_path documentation mismatch is fixed in the public docstring and matches the Registry (diff_diff/chaisemartin_dhaultfoeuille.py:L416-L423, docs/methodology/REGISTRY.md:L641-L641).
• Methodology cross-check is consistent with the documented joiners/leavers IF precedent: non-path switcher contributions are zeroed, control pools/cohort structure are preserved, and the plug-in SE uses the path-specific divisor (diff_diff/chaisemartin_dhaultfoeuille.py:L4724-L4858, diff_diff/chaisemartin_dhaultfoeuille.py:L4959-L5139, diff_diff/chaisemartin_dhaultfoeuille.py:L2202-L2245).
• Static review only; I could not execute the tests because pytest is unavailable in this environment.

Methodology

No findings. The changed by_path implementation matches the Registry contract at docs/methodology/REGISTRY.md:L641-L641 and the existing joiners/leavers variance pattern in diff_diff/chaisemartin_dhaultfoeuille.py:L2202-L2245; I did not find an undocumented estimator, weighting, or SE deviation in the diff.

Code Quality

No findings. by_path is propagated through constructor validation, get_params(), set_params(), fit-time gating, the results object, and the convenience wrapper (diff_diff/chaisemartin_dhaultfoeuille.py:L475-L549, diff_diff/chaisemartin_dhaultfoeuille.py:L553-L590, diff_diff/chaisemartin_dhaultfoeuille.py:L6762-L6775).

Performance

No findings. The added work reuses the existing multi-horizon IF helper per selected path and does not introduce an obvious asymptotic regression in the changed scope.

Maintainability

No findings. The earlier ambiguous None vs {} result contract is now handled consistently across compute, summary, and DataFrame surfaces.

Tech Debt

No findings. The remaining by_path R-parity follow-up is explicitly documented as deferred in docs/methodology/REGISTRY.md:L641-L641, so it is not a blocker under the review policy.

Security

No findings.

Documentation/Tests

No findings in the diff scope. The PR now adds targeted coverage for the previously missing empty-path and degenerate-path branches (tests/test_chaisemartin_dhaultfoeuille.py:L4026-L4196). I could not run the suite locally because pytest is not installed.