Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .claude/memory.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ print(results.summary())
The canonical inference quintet is `att` / `se` / `t_stat` / `p_value` /
`conf_int` (never `.pvalue` / `.ci`). Every results class exposes all five,
but native STORAGE varies in 3.x (`overall_att` on the staggered family,
`avg_att` on MultiPeriodDiD, with canonical names as properties). `summary()`
`avg_att` on the MultiPeriodDiD class removed at the 4.0 cut, with canonical
names as properties). `summary()`
everywhere; `to_dict()` / `to_dataframe()` on most classes. See
`.claude/../docs/methodology/REGISTRY.md` per estimator.

Expand Down
7 changes: 3 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,9 @@ A new parameter is only complete when it is:
- applied in the **bootstrap/inference** paths, not just the analytical one
- reflected on the result object, so `to_dict()`/`summary()` do not misreport it
- propagated to the estimators that inherit it: `TwoWayFixedEffects` defines
no `__init__` of its own, and `MultiPeriodDiD`'s 3.9 deprecation shim
forwards via `super().__init__(*args, **kwargs)` with an import-time
`__signature__` mirror - so both inherit a new
`DifferenceInDifferences` constructor parameter automatically;
no `__init__` of its own, so it inherits a new `DifferenceInDifferences`
constructor parameter automatically (the `MultiPeriodDiD` 3.9 shim that
mirrored the signature was removed at the 4.0 cut);
`SyntheticDiD` defines its OWN signature (it forwards only `alpha` to
`super().__init__`, pinning `cluster=None`), so a new parent
parameter must be explicitly mirrored — or explicitly rejected, like its
Expand Down
3 changes: 0 additions & 3 deletions DEFERRED.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ provenance and AI-review deviation-documentation: a row here (or in
| LWDiD cohort-relative sensitivity exclusions: `robustness_pre_periods` / `sensitivity_no_anticipation` currently reject multi-cohort staggered inputs because their exclusion windows are defined relative to the EARLIEST adoption (later cohorts' own pre periods fall inside the global post window and survive every restriction, mislabeling the specification). Supporting staggered inputs needs per-cohort window semantics (exclude the last k periods of each cohort's own `t < g` window before its transformation), which the current row-subset design cannot express — a per-cohort masking derivation + its aggregation contract | `diff_diff/lwdid_sensitivity.py` | #588 | Low |
| LWDiD estimation-time anticipation offset (CS-parity gap): no `anticipation=` parameter exists — no-anticipation is hard-assumed and the only surface is the single-cohort `sensitivity_no_anticipation` refit diagnostic. The LW device for anticipation is the anchor-window shift — drop the last k pre-treatment periods from the transformation/anchor window (`Ybar_{i,S0}` with `S0 < S-1`, per the REGISTRY LWDiD Anticipation edge case; NOT a cohort recode g → g − k) — and exposing an `anticipation=` parameter needs that derivation checked against LW 2025 NAC/CNAS before implementation | `diff_diff/lwdid.py` | LWDiD release audit | Low |
| HonestDiD rejects LWDiD event studies: `LWDiDResults.aggregate('event_study')` emits the unified EventStudyResults surface but the `honest_did.py` allowlist admits CS/Stacked sources only. Admission needs a methodology decision, not an allowlist edit: LWDiD pre-period cells are transformation-based placebos on the Appendix D control pools, and Rambachan-Roth requires those coefficients plus a joint pre/post vcov to stand as valid pre-trend evidence | `diff_diff/honest_did.py`, `diff_diff/lwdid_results.py` | LWDiD release audit | Low |
| HonestDiD non-chronological declared partitions (native `MultiPeriodDiDResults` route): the Rambachan-Roth restriction matrices are built POSITIONALLY over the concatenated declared pre/post lists assuming one chronological boundary, but the native route accepts non-suffix `post_periods` / non-last-pre references and returns bounds whose restriction system does not match the Registry equations (pre-existing; surfaced by the Phase 3(a) calendar-route review, which fails closed instead). Fix = transform the declared partition into boundary form where a valid mapping exists, else reject on the native route too - needs the restriction-geometry derivation. REGISTRY HonestDiD Note records the limitation. | `diff_diff/honest_did.py` | 3(a) | Medium |
| `PlaceboTests` `boundary_gap` — a permutation randomization-inference margin (SE-audit item (b)); NOT computed anywhere in code today, so this is a new feature + result field, not a coverage lock. **User-locked 2026-07-09: defer until a derivation/paper source exists** — do not design or implement from scratch. | `tests/test_methodology_placebo.py`, `diff_diff/diagnostics.py` | SE-audit | Low |
| TwoStageDiD honest/pretrends container admission DEFERRED (decision revised from "widen" during the 2(b) PR-3b plan review): analytical fits carry the joint Gardner-GMM event-study covariance (M-092), but the pre-period coefficients are stage-1 residual MEANS — the reference horizon is dropped from the no-intercept Stage-2 design and the zero anchor row appended mechanically — not contrasts against a reference period, while HonestDiD's Δ^RM/Δ^SD arithmetic hard-codes the `delta_0 = 0` normalization into its boundary/bridge constraints. Admission needs either a Stage-2 re-estimation with the reference horizon in the design or a derived residual-to-reference normalization mapping (+ its variance transform). Both consumers' TypeErrors state the deferral; see the REGISTRY TwoStageDiD Note (d). | `diff_diff/honest_did.py`, `diff_diff/pretrends.py`, `diff_diff/two_stage_aggregation.py` | 2(b) PR-3b | Low |
| RDDensityTest binomial windows test (R rddensity's `bino=` block: exact binomial tests of side counts in shrinking windows around the cutoff): methodology is sourced from Cattaneo, Frandsen & Titiunik (2015, JCI) and Cattaneo, Frandsen & Vazquez-Bare (2017), NOT the reviewed CJM 2020 paper, so implementing it needs its own paper review(s) first (the 2-PR methodology convention). | `diff_diff/rddensity.py` | rddensity PR-B | Low |
Expand Down Expand Up @@ -121,7 +120,6 @@ For survey-specific limitations (`NotImplementedError` paths), see the
| `SpilloverDiD` estimator-level end-to-end vcov reconstruction tests (`bread @ meat @ bread` against `res.vcov`): requires exposing the estimator's internal `X_2_kept` design arrays; the surface is currently pinned from different angles (uniform-weight bit-identity, drift goldens, manual lincom reconstruction at rtol=1e-6). | `spillover.py`, `tests/test_spillover.py` | follow-up | Low |
| `SpilloverDiD` Wave E.3 `finite_mask + design-subset` hygiene not yet adopted for TwoStageDiD's analogous pattern (`two_stage.py:567-601`) — separate parity follow-up noted in the `docs/api/spillover.rst` Restrictions block. | `two_stage.py` | Wave E.3 | Low |
| `HeterogeneousAdoptionDiD` `covariates=` (Theorem 6 multivariate-covariate extension) not implemented — `fit(covariates=...)` raises `NotImplementedError` via the shipped future-work trap (locked by the `test_had.py` / `test_methodology_had.py` L73 tests); the deferred work is the Theorem 6 extension itself. | `had.py` | Phase 2a | Low |
| MultiPeriodDiD wild bootstrap not supported (falls back to analytical, n_bootstrap-independent) — user-facing edge-case limitation; the 4.0 removal replaces the fallback with a raise (v4-design §4.1), and the merged TWFE event-study mode already RAISES since 3.9 (Phase 3(a)) — this row now governs only the deprecated class itself. | `estimators.py` (MultiPeriodDiD.fit wild-fallback block) | — | Low |
| `predict()` raises `NotImplementedError` — rarely needed; user-facing limitation. | `estimators.py:890-911` | — | Low |

## Version-gated (v4)
Expand All @@ -147,7 +145,6 @@ decisions (refactor waivers, perf trade-offs, test-infrastructure calls) are rec
| **Fixed-B Stata SE-golden comparisons in the LWDiD suite run unmarked in default CI** (no `ci_params.bootstrap()` scaling, no `slow` marker): the SE tolerance derives from BOTH fixed rep counts (ours B=999, Stata's R from the golden meta) so scaling either side would invalidate the committed-golden comparison — the ci_params convention governs convergence-style tests, not fixed-B golden comparisons — and these tests are the PR #588 acceptance bar, which must run in default CI; module-scoped fit memoization bounds the cost (~1-2 s per fit locally) | `tests/test_methodology_lwdid.py` | 2026-08-16 |
| **The 4.0 migration guide's code blocks are not snippet-executed.** `tests/test_doc_snippets.py` discovers a hardcoded list of `.rst` files and only `.. code-block:: python` / RST `::` bodies, so `docs/migration-4.0.md` gets no coverage. Deliberate: the guide is a MIXED document - most "after" examples (the renames, `results.att`) run on the current release, but the `field-flip` and `df-convention-flip` examples describe 4.0 behaviour that cannot run until 4.0, so a blanket execution lane would fail by construction. Closing the gap fully means a markdown-fence extractor plus a skip-marker convention for the future-API blocks - a harness change, out of scope for a docs PR. What IS gated: the appendix's ledger parity (`test_migration_guide_*`), which pins the row set and every mechanically checkable cell; and, since the first local review found all three merge examples carrying invalid keywords, `test_migration_guide_examples_bind_to_real_signatures`, which ast-parses the guide's python blocks and asserts every constructor/`fit()` keyword exists on the target signature. That is signature binding, NOT execution - it deliberately skips calls whose owner it cannot resolve (e.g. `results.aggregate(...)`), and it cannot catch a wrong VALUE or a wrong sequence of calls. The hand-written `Fix` prose remains unverifiable by any available means. | `docs/migration-4.0.md`, `tests/test_doc_snippets.py` | Phase 4 / 2026-08-09 |
| **MMM launch carousel carries scoped marketing claims, not exporter-contract documentation.** The deck (a LinkedIn marketing artifact, not a docs surface) states capabilities in scoped-but-punchy form: guardrail copy says "the easy mistakes fail loudly" / "you own the design, it owns the math" (the caller-owned estimand/population/window/outcome-scale alignment lives in REGISTRY.md's MMM section and the exporter docstrings, not on slides), and slide 8 shows tutorial 29's worked staggered-boost lift row WITHOUT an on-slide linearity qualifier - the compression's linear-channel scoping is the tutorial's job, which the CTA points to. Absolutes ("no silent mis-calibration", "any estimate exports", "anything with an estimate + SE") are ban-listed by `tests/test_mmm_carousel_claims.py`; local review R1/R2 pressed for compliance-style scoping language on the slides and the marketing-appropriate scoped copy was chosen instead. | `carousel/generate_mmm_carousel.py`, `tests/test_mmm_carousel_claims.py` | mmm-carousel / 2026-08-20 |
| **MultiPeriodDiD deprecation shim loses static constructor-arg checking (3.9 window).** The M-010 shim is `__init__(*args, **kwargs)` + an import-time `__signature__` mirror of DiD's constructor: runtime introspection (get_params/set_params, `inspect.signature`) and eager validation are fully preserved, but static type checkers / IDEs cannot check constructor arguments for the deprecated class until the ledger-tracked M-010 removal. Accepted: the alternative (hand-mirroring ~20 parameters) is a drift magnet on a class with one minor version of remaining life. | `diff_diff/estimators.py` | 3(a) / 2026-08-07 |
| **DCDH `sklearn.base.clone` param-identity failure won't-fix.** `ChaisemartinDHaultfoeuille._validate_paths_of_interest` unconditionally canonicalizes `paths_of_interest` into a fresh `List[Tuple[int, ...]]`, so sklearn `clone()`'s post-construction `param1 is param2` identity check fails for configured instances - a pre-existing normalization the BaseEstimator mixin PR documented rather than changed (get_params/set_params signatures are clone-compatible; the dependency-free `cls(**est.get_params())` config-equality contract is the enforced one, `tests/test_base_estimator.py`). Fixing would mean returning the caller's raw object from a validator whose job is canonicalization. | `chaisemartin_dhaultfoeuille.py` | mixin PR / 2026-08-01 |
| **scikit-learn is a required dev/CI dependency, optional at runtime; clone-identity tests stay `importorskip`-guarded.** Supersedes the 2026-08-01 decision that kept scikit-learn out of the dev extra: #816 added it (unpinned) to the `dev` extra and to both `rust-test.yml` install steps, so the sklearn `clone()` round-trip tests (`test_base_estimator.py`, had/rdd/cic suites) and the DML cross-fit learner tests always run in CI. The library's runtime dependency posture is unchanged (numpy/pandas/scipy only) and the `importorskip` guards remain, so the suite still passes in an environment without scikit-learn; the always-running contract is still the dependency-free re-instantiation config-equality test. | `pyproject.toml`, `tests/test_base_estimator.py` | #816 |
| **Plan-review hash gate threat model: accident prevention, NOT adversarial defense.** The ExitPlanMode content-hash gate (hook + `plan_snapshot.py`) exists to stop accidents — stale approvals, concurrent-worktree cross-talk, plans edited mid-review — all of which it closes by construction (snapshot identity + invocation-unique state tokens, 30+ behavioral tests). It does NOT and cannot defend against a malicious local process: nothing verifies review AUTHORSHIP, and such an actor can simply write a matching review file directly — no userland hook can prevent that short of signed reviews, which is out of scope for a personal workflow aid. Review findings that presuppose a hostile local actor against this gate are waived by this decision (2026-07-20, after 7 local AI-review rounds converged on ever-deeper adversarial-model refinements with no reachable fixpoint). Genuine accident vectors remain in scope and are fixed as found. | `.claude/hooks/check-plan-review.py`, `.claude/scripts/plan_snapshot.py` | 2026-07-20 |
Expand Down
Loading
Loading