diff --git a/CHANGELOG.md b/CHANGELOG.md index 0caa3f3a..38516d22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,87 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- **Internal: 4.0 gating-completeness amendment to the design spec and ledger.** + An audit of the public surface against the spec's own contract-rename rules + found two renames the initial inventory missed and three Phase 2 obligations + the automated release gate could not see. (a) Two ledger rows added for + RESULTS fields that mirror already-tracked params: `M-094` + (`RegressionDiscontinuityResults.treatment_col` → `takeup`, mirroring + `M-042`) and `M-095` (`StackedDiDResults.clean_control` → `control_group`, + mirroring `M-043`). Both are public dataclass fields emitted by `to_dict()`, + so renaming the params alone would have left deprecated spellings on the + serialized surface that spec section 5 forbids from 4.0; each flips in the + same diff as its param sibling. `docs/v4-design.md` section 8 gains rule 9 + making the mirror obligation normative, so the next rename sweep greps for it + rather than rediscovering it. (b) `M-081` (`n_bootstrap` semantic + unification) gains `introduced_in: 3.9` and a new `M-096` covers the uniform + `inference="wild_bootstrap"` exposure (spec section 7) - both were real Phase + 2 obligations that no row gated, so a 3.9 cut could have shipped without + them. Both use `introduced_in` rather than `deprecated_in` deliberately: for + behavior rows the early-flip guard keys off `deprecated_in`, so a flip + version would have failed the very PR that implements the obligation (it + lands while `__version__` is still 3.8.x) - the `M-091`/`M-092` pattern. + `M-096` covers the `inference="wild_bootstrap"` selector contract, which + governs wild CLUSTER bootstrap offered as an alternative to analytical SEs - + a roster of `DifferenceInDifferences` + `TwoWayFixedEffects`. Two things the + audit established. (i) The selector does **not** fail closed today: + `inference=` is stored with no valid-value check anywhere, and DiD routes + `wild_bootstrap` without `cluster=` silently to analytical (a currently + *pinned* behavior), so a typo or a missing prerequisite quietly changes which + SE/p-value/CI procedure runs. Phase 2 must validate the accepted value set on + `__init__` and transactional `set_params` and handle the missing-cluster case + explicitly. (ii) The apparent `inference=` vs `n_bootstrap>0` split is not + drift - estimators whose bootstrap *is* their inference method run materially + different procedures (CallawaySantAnna an influence-function multiplier + bootstrap; SunAbraham a unit-pairs bootstrap, Rao-Wu rescaled on survey + paths; dCDH a group-level influence-function multiplier bootstrap upgrading + to Hall-Mammen PSU wild clustering under strictly-coarser survey PSUs) versus + DiD/TWFE's residual wild cluster bootstrap with test-inverted CIs, so one + value spelling for all of them would make a uniform name carry altered + meaning. They keep `n_bootstrap` as documented domain vocabulary, and section + 7 now records the boundary; an `inference=` selector for any of them would + name its own method and is additive post-4.0 work. + (c) The `BaseEstimator` mixin and the R-equivalents docs table are genuine + refactor/docs work that cannot be ledger rows at all, so section 9 gains a + 3.9-cut checklist alongside the existing 4.0 one. `M-008`'s notes now record + that its telemetry soak window *is* the 3.9→4.0 gap. (d) A completeness + sweep over surfaces the Phase 1 inventory never audited. That inventory was + derived from the estimator roster, so module-level public **functions** were + never checked against section 8 at all: `M-097` renames + `twowayfeweights(group=)` → `unit=` (section 3.3 **retains** that diagnostic + past 4.0, so no wrapper removal covers it), `M-098`..`M-112` rename the five + `_col` params on each of the three exported HAD pretest entry points + (`joint_pretrends_test`, `joint_homogeneity_test`, + `did_had_pretest_workflow`), `M-113` renames `trim_weights(weight_col=)` → + `weights=`, `M-114` is the dCDH results mirror (`.groups` → `.units`, whose + "groups" really are unit ids), and `M-115` adds the fourth `robust` site + (`LinearRegression`, which section 7 said dies "everywhere it exists" while + only three rows enumerated it). The same sweep found ~26 hits that are NOT + violations and are now documented as such in section 8 so they are not + re-litigated: the staggered family's `group`/`groups` name the ATT(g,t) + **cohort** in Callaway-Sant'Anna's notation (rule 8 protects it; renaming to + `unit` would be wrong), and `TripleDifference.fit[group]` is the + treated-group indicator rule 3 explicitly reserves the name for. Section 8 + gains rule 10 binding the rules to public functions, not just classes, and + rule 11 putting every READER of a renamed name in the row's scope — the + `getattr(obj, "old_name", default)` pattern degrades silently to the default + after removal, so a consumer reports the wrong thing instead of crashing. + `M-043`/`M-095` accordingly carry their reporting and power/practitioner + consumers (and the `clean_control` serialized reporting key, retired at 4.0 + by the section 5 canonical-names policy) in `code_refs`. + (e) `M-042`/`M-094`'s rename target moves from `treatment` to `takeup`: + fuzzy RD's observed-take-up column accepts non-binary dose values (matching + R's `fuzzy=`, with the estimand label degrading from complier-LATE to a bare + local Wald ratio), while section 8 rule 7 reserves `treatment` for a 0/1 + indicator and never a treatment level - so the original target would have + made a uniform name carry altered meaning. `takeup` is the spelling the RD + docstring already uses; no shim had shipped, so the retarget is free. Rule 7 + gains the corollary that a column which MAY be non-binary does not take the + `treatment` name even when it is conceptually take-up. + Ledger row count 77 → 99 (enforcement-test floor and id snapshot extended in + the same diff); rows gating a 3.9 bump 45 → 68. Docs + ledger + test-count + updates only - **no public API or numerical behavior change** (every new row + is `planned`; the shims themselves land in Phase 2c). - **Internal tooling: plan-review engine → dual-reviewer skill (no library change).** `/review-plan` + `/revise-plan` are retired for a `.claude/skills/plan-review/` skill that runs the **dual** review engine diff --git a/TODO.md b/TODO.md index 77be276a..31ec3697 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,7 @@ generic sparse-FE, QR+SVD rank-detection redundancy, `check_finite` bypass — m | Tighten the mypy suppressions that back the enforced-zero posture: burn down `prep_dgp`'s per-module `[index]` override (needs a None-vs-array restructure that preserves the seeded RNG stream), and evaluate re-enabling the globally disabled codes (`arg-type`, `return-value`, `var-annotated`, `assignment`) one at a time — `assignment` alone hid several real annotation drifts found during the 2026-07 triage. | `pyproject.toml` `[tool.mypy]`, `diff_diff/prep_dgp.py` | lint-CI | Mid | Low | | MMM interop follow-up: Meridian `roi_calibration_period` mask builder - accept the MMM's time index + channel order and emit the boolean `(n_media_times, n_media_channels)` mask so `.to_code()` scopes the prior to the experiment window automatically (today the caller passes a mask expression / `full_model_window=True`). | `diff_diff/mmm.py` | mmm-interop | Quick | Low | | MMM interop PR-B: calibration tutorial notebook (fit DiD/CS -> scope -> `to_pymc_marketing_lift_test` / `to_meridian_roi_prior`) + a `llms-practitioner.txt` Step 8 pointer to the exporters as the MMM hand-off. | `docs/tutorials/`, `diff_diff/guides/llms-practitioner.txt` | mmm-interop | Mid | Low | +| Section-8 naming-completeness guard test: sweep the public surface (module-level functions, class `__init__`/`fit` params, results dataclass fields) for contract-rename violations and fail unless each hit is either a `docs/v4-deprecations.yaml` row or an allowlist entry carrying a stated reason (seed the allowlist with the documented domain-vocabulary exemptions: the staggered family's ATT(g,t) `group`/`groups`, `TripleDifference.fit[group]`, wrapper params dying with `M-070..M-077`, and params inherited from `DifferenceInDifferences.__init__`). Today the rules are normative prose and the ledger holds only what an audit remembered — two successive manual sweeps each found surfaces the prior one missed (`M-094`/`M-095`, then the 19-row `M-097..M-115` function sweep). Should land BEFORE Phase 2c so the rename PR works from a mechanically-verified list. Two further checks belong in the same guard, each having been missed by hand across successive review rounds: (a) **phase-table agreement** — every row's `phase` must appear in the matching `docs/v4-design.md` section 9 table entry and vice versa (the section 9 checklist asserts this agreement but nothing verifies it, and three separate rounds caught a stale table); (b) **consumer coverage** (section 8 rule 11) — for each rename row, grep `diff_diff/` and `docs/methodology/` for the old name and require every hit to be either in the row's `code_refs` or allowlisted, since `getattr(obj, "old_name", default)` degrades silently rather than raising after removal. | `tests/`, `docs/v4-design.md` | gating-completeness amendment | Mid | Medium | | Tracking-file contract guard test: reject NEW active deferred-work pointers at `TODO.md` (deferred rows live in `DEFERRED.md`; allowlist for historical/past-tense prose and actionable-row pointers) and assert rows cross-linking a `docs/v4-deprecations.yaml` `M-xxx` id don't restate ledger status. Origin: tracking-split local review R2. | `tests/`, `TODO.md`, `DEFERRED.md` | tracking-split | Quick | Low | | Real-data CI canary for dataset-backed replication tests: `test_methodology_lwdid.py`'s Prop 99 / Walmart goldens skip (visibly) when loaders fall back to synthetic; add a lane or canary asserting `df.attrs["source"] == "lwdid_ssc_ancillary"` in CI so network regressions cannot silently de-gate the replication tests. Pairs with the loader-fallback repair row above. | `tests/test_methodology_lwdid.py`, `.github/workflows/` | LWDiD validation suite | Quick | Low | | `worktree-rm` safety via a tested argv helper: the prose rewrite (ask-before-remove confirmation gate, detached-HEAD reachability/rescue, tip-identity force-delete guard) was reverted to the main version because editing the prose repeatedly reintroduced shell-injection (last: sourcing a state file built from a git-derived branch name). Restore those guards in a `worktree_rm.py` that takes the name via file ingress, invokes git through argv arrays, and has a metacharacter-branch/path injection regression test — the pattern that worked for `pr_prepare.py`/`premerge_scan.py`. | `.claude/commands/worktree-rm.md`, `.claude/scripts/` | skill-audit | Heavy | Medium | diff --git a/docs/v4-deprecations.yaml b/docs/v4-deprecations.yaml index f4f52cdd..8acdeed7 100644 --- a/docs/v4-deprecations.yaml +++ b/docs/v4-deprecations.yaml @@ -123,7 +123,7 @@ rows: env_var: DIFF_DIFF_SOLVE_OLS_FASTPATH decision_due: "4.0" code_refs: [diff_diff/linalg.py] - notes: "DIFF_DIFF_SOLVE_OLS_FASTPATH default-ON is a v4-class change GATED on golden recapture + certification telemetry soak (TODO.md row); record go/no-go at the 4.0 cut either way." + notes: "DIFF_DIFF_SOLVE_OLS_FASTPATH default-ON is a v4-class change GATED on golden recapture + certification telemetry soak (TODO.md row); record go/no-go at the 4.0 cut either way. SOAK WINDOW = the 3.9 -> 4.0 gap: opt-in telemetry can only accumulate from a release that advertises the env var, so the 3.9 release notes are the last surface that can start it. A short gap is a legitimate outcome - 'evaluated, kept off' is terminal and needs no soak - but that is a scheduling decision to make deliberately, not to discover at the cut." # ---- The three merges + retiring classes --------------------------------- - id: M-010 @@ -478,7 +478,7 @@ rows: kind: param group: renames-col-suffix old: "diff_diff:RegressionDiscontinuity.fit[treatment_col]" - new: "diff_diff:RegressionDiscontinuity.fit[treatment]" + new: "diff_diff:RegressionDiscontinuity.fit[takeup]" introduced_in: "3.9" deprecated_in: "3.9" removed_in: "4.0" @@ -486,7 +486,20 @@ rows: phase: 2 warning: FutureWarning code_refs: [diff_diff/rdd.py] - notes: "0/1 take-up column for fuzzy RD - consistent with 'treatment' as a 0/1 indicator library-wide." + notes: "Observed take-up column for fuzzy RD. Target RETARGETED from 'treatment' to 'takeup' by the gating-completeness amendment: the column accepts NON-BINARY (dose) take-up, matching R's fuzzy= (REGISTRY 'Estimand and estimator (fuzzy RD)'; the estimand label is data-dependent - complier LATE only when values are in {0,1}, otherwise a bare local Wald ratio). Section 8 rule 7 reserves 'treatment' for a 0/1 indicator and never a treatment level, so 'treatment' would have carried altered meaning here. 'takeup' is already the library's own vocabulary for it (the RD docstring example passes treatment_col='takeup'). No shim had shipped, so the retarget costs nothing." + - id: M-094 + kind: field + group: renames-col-suffix + old: "diff_diff:RegressionDiscontinuityResults.treatment_col" + new: "diff_diff:RegressionDiscontinuityResults.takeup" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/rdd.py] + notes: "Results-side mirror of [M-042]: a public dataclass field carrying the fit param's name, emitted by to_dict(). Renaming the param alone would leave the _col spelling on the serialized surface, which spec section 5 forbids from 4.0 (canonical names only). Target is 'takeup', not 'treatment', for the reason recorded on [M-042] (non-binary take-up is accepted, so 'treatment' would violate section 8 rule 7). Flips WITH [M-042] in the same Phase 2c diff." - id: M-043 kind: param group: renames-control-group @@ -498,8 +511,21 @@ rows: status: planned phase: 2 warning: FutureWarning - code_refs: [diff_diff/stacked_did.py] - notes: "Same concept as control_group elsewhere; underscored value spellings win library-wide (spec section 8)." + code_refs: [diff_diff/stacked_did.py, diff_diff/power.py, diff_diff/practitioner.py] + notes: "Same concept as control_group elsewhere; underscored value spellings win library-wide (spec section 8). CONSUMER MIGRATION: two sites read the ESTIMATOR attribute via getattr(estimator, 'clean_control', ...) and gate on it - power.py (a 'strict' survey_config rejection and a cross-check against control_group) and practitioner.py (guidance text naming the param). Same getattr-returns-None-after-removal failure mode as [M-095]'s results-side consumers, so both rows migrate their consumers in the shared Phase 2c diff; [M-095] covers results.clean_control readers, this row covers estimator.clean_control readers." + - id: M-095 + kind: field + group: renames-control-group + old: "diff_diff:StackedDiDResults.clean_control" + new: "diff_diff:StackedDiDResults.control_group" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/stacked_did_results.py, diff_diff/_reporting_helpers.py, diff_diff/business_report.py, docs/methodology/REPORTING.md] + notes: "Results-side mirror of [M-043]: a public dataclass field carrying the constructor param's name, emitted by to_dict() and rendered by summary() ('Clean control:'). Renaming the param alone would leave the results metadata contradicting the param it reports. The VALUE spellings are already canonical ('not_yet_treated'); only the field name moves. Flips WITH [M-043] in the same Phase 2c diff; the summary() label follows. CONSUMER MIGRATION IS PART OF THIS ROW - two reporting sites read results.clean_control via getattr(..., None) and branch on 'never_treated'/'strict' (_reporting_helpers.py, business_report.py). A getattr default silently returns None after removal, so BOTH would fall through to the default 'not_yet_treated' description and misreport the identifying comparison group - a wrong-answer regression, not a crash. business_report.py also EMITS block['clean_control'] as a serialized reporting key, which section 5's canonical-names-only policy retires at 4.0 (the block keeps 'control_group'). REPORTING.md documents the field and updates with it. Tests must cover a results object exposing ONLY control_group, across all three values." - id: M-044 kind: param group: renames-level @@ -961,12 +987,25 @@ rows: group: policy-n-bootstrap old: "diff_diff:DifferenceInDifferences[n_bootstrap]" new: null + introduced_in: "3.9" deprecated_in: null removed_in: null status: planned phase: 2 code_refs: [diff_diff/estimators.py, diff_diff/synthetic_did.py, diff_diff/changes_in_changes.py] - notes: "Semantic unification only: n_bootstrap=0 = bootstrap off wherever an analytical path exists; per-estimator counts stay tuned (999 light / 200 compute-heavy). No numeric default changes." + notes: "Semantic unification only: n_bootstrap=0 = bootstrap off wherever an analytical path exists; per-estimator counts stay tuned (999 light / 200 compute-heavy). No numeric default changes. introduced_in gates the 3.9 cut (pattern of [M-091]/[M-092]): the unification is a Phase 2 obligation and must not slip past the bump unflipped. deprecated_in stays null deliberately - a flip version would arm the early-flip guard against the Phase 2 PR, which lands while __version__ is still 3.8.x." + - id: M-096 + kind: behavior + group: policy-wild-bootstrap + old: "diff_diff:DifferenceInDifferences[inference]" + new: null + introduced_in: "3.9" + deprecated_in: null + removed_in: null + status: planned + phase: 2 + code_refs: [diff_diff/estimators.py, diff_diff/twfe.py] + notes: "Spec section 7 selector contract + roster guard. inference='wild_bootstrap' selects wild CLUSTER bootstrap offered as an ALTERNATIVE to analytical SEs; the SUPPORTING roster is DifferenceInDifferences + TwoWayFixedEffects. NOT a uniformity mandate over every bootstrap - each estimator whose bootstrap IS its inference method runs a materially different procedure and keeps n_bootstrap as documented domain vocabulary (section 6 precedent): CallawaySantAnna = influence-function multiplier bootstrap; SunAbraham = unit-pairs bootstrap (Rao-Wu rescaled under stratified/PSU survey designs); ChaisemartinDHaultfoeuille = group-level influence-function multiplier bootstrap, upgrading to PSU-level Hall-Mammen wild clustering under survey designs with strictly-coarser PSUs. Spelling any of those 'wild_bootstrap' would make a uniform name carry altered meaning (section 8 rule 8). MultiPeriodDiD EXPOSES the param but has no wild-bootstrap path (warns, falls back to analytical); it is removed at 4.0 [M-010] and section 4.1 replaces the fallback with a raise - so the roster pins SUPPORT, not param presence. SCOPE: the selector does not fail closed today - inference= is stored unvalidated (no valid-value check anywhere), and DiD silently routes wild_bootstrap WITHOUT cluster= to analytical (estimators.py, pinned by tests/test_wild_bootstrap.py::test_did_wild_bootstrap_requires_cluster), so a typo or a missing prerequisite silently changes the SE/p/CI procedure - a no-silent-failures violation. Phase 2 makes it fail closed, and 'fail closed' means REFUSE, not warn-and-degrade: (a) the accepted value set is exactly {'analytical', 'wild_bootstrap'}, validated on __init__ AND transactional set_params (an unrecognized spelling raises, never falls through to analytical); (b) wild_bootstrap WITHOUT cluster= raises ValueError - REGISTRY defines WCR as 'inference=\"wild_bootstrap\" (with cluster=)', so the combination is incoherent rather than merely unsupported, and a warned analytical fallback would still run a procedure the caller did not ask for. Flipping the pinned test is expected and its CHANGELOG entry must say so. done requires a test_ref covering: the exact accepted value set and an invalid spelling, transactional set_params, DiD with and without cluster, TWFE, MultiPeriodDiD's fallback, and the roster itself (a future estimator gaining WCR must adopt the selector or land its own row). Companion to [M-081] (n_bootstrap semantics). introduced_in gates the 3.9 cut for the same reason as [M-081]; behavior-at-done requires test_ref." - id: M-082 kind: param group: renames-post @@ -1015,3 +1054,256 @@ rows: warning: FutureWarning code_refs: [diff_diff/continuous_did.py] notes: "The one estimator with a design-matrix column in the constructor; moves to fit() per the sklearn split (spec section 7)." + + # ---- Completeness sweep (gating-completeness amendment) ------------------- + # The Phase 1 inventory scanned CLASS surfaces only, so module-level public + # FUNCTIONS were never audited against section 8. These rows close that gap. + # Domain-vocabulary NON-violations found by the same sweep are documented in + # spec section 8 (CS-family ATT(g,t) 'group'/'groups'; TripleDifference's + # reserved 'group'), not rowed. + - id: M-097 + kind: param + group: renames-dcdh + old: "diff_diff:twowayfeweights[group]" + new: "diff_diff:twowayfeweights[unit]" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/chaisemartin_dhaultfoeuille.py] + notes: "Companion to [M-033] on the RETAINED diagnostic function (spec section 3.3 keeps twowayfeweights past 4.0 - it is not a class duplicate, so no wrapper removal mitigates this). Same meaning as dCDH's fit[group]: a unit identifier, which section 8 rule 3 reserves 'group' against. Unlike the chaisemartin_dhaultfoeuille() wrapper hit, which dies with [M-077]." + - id: M-098 + kind: param + group: renames-col-suffix + old: "diff_diff:joint_pretrends_test[outcome_col]" + new: "diff_diff:joint_pretrends_test[outcome]" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/had_pretests.py] + notes: "Public retained HAD diagnostic; section 8 rule 5. Mirrors the estimator-side [M-035]. The three exported HAD pretest entry points share one 5-param block ([M-098]..[M-112]) and shim together." + - id: M-099 + kind: param + group: renames-col-suffix + old: "diff_diff:joint_pretrends_test[dose_col]" + new: "diff_diff:joint_pretrends_test[dose]" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/had_pretests.py] + notes: "Mirrors [M-036]; see [M-098]." + - id: M-100 + kind: param + group: renames-col-suffix + old: "diff_diff:joint_pretrends_test[time_col]" + new: "diff_diff:joint_pretrends_test[time]" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/had_pretests.py] + notes: "Mirrors [M-037]; see [M-098]." + - id: M-101 + kind: param + group: renames-col-suffix + old: "diff_diff:joint_pretrends_test[unit_col]" + new: "diff_diff:joint_pretrends_test[unit]" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/had_pretests.py] + notes: "Mirrors [M-038]; see [M-098]." + - id: M-102 + kind: param + group: renames-col-suffix + old: "diff_diff:joint_pretrends_test[first_treat_col]" + new: "diff_diff:joint_pretrends_test[first_treat]" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/had_pretests.py] + notes: "Mirrors [M-039]; see [M-098]." + - id: M-103 + kind: param + group: renames-col-suffix + old: "diff_diff:joint_homogeneity_test[outcome_col]" + new: "diff_diff:joint_homogeneity_test[outcome]" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/had_pretests.py] + notes: "Mirrors [M-035]; see [M-098]." + - id: M-104 + kind: param + group: renames-col-suffix + old: "diff_diff:joint_homogeneity_test[dose_col]" + new: "diff_diff:joint_homogeneity_test[dose]" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/had_pretests.py] + notes: "Mirrors [M-036]; see [M-098]." + - id: M-105 + kind: param + group: renames-col-suffix + old: "diff_diff:joint_homogeneity_test[time_col]" + new: "diff_diff:joint_homogeneity_test[time]" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/had_pretests.py] + notes: "Mirrors [M-037]; see [M-098]." + - id: M-106 + kind: param + group: renames-col-suffix + old: "diff_diff:joint_homogeneity_test[unit_col]" + new: "diff_diff:joint_homogeneity_test[unit]" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/had_pretests.py] + notes: "Mirrors [M-038]; see [M-098]." + - id: M-107 + kind: param + group: renames-col-suffix + old: "diff_diff:joint_homogeneity_test[first_treat_col]" + new: "diff_diff:joint_homogeneity_test[first_treat]" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/had_pretests.py] + notes: "Mirrors [M-039]; see [M-098]." + - id: M-108 + kind: param + group: renames-col-suffix + old: "diff_diff:did_had_pretest_workflow[outcome_col]" + new: "diff_diff:did_had_pretest_workflow[outcome]" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/had_pretests.py] + notes: "Mirrors [M-035]; see [M-098]." + - id: M-109 + kind: param + group: renames-col-suffix + old: "diff_diff:did_had_pretest_workflow[dose_col]" + new: "diff_diff:did_had_pretest_workflow[dose]" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/had_pretests.py] + notes: "Mirrors [M-036]; see [M-098]." + - id: M-110 + kind: param + group: renames-col-suffix + old: "diff_diff:did_had_pretest_workflow[time_col]" + new: "diff_diff:did_had_pretest_workflow[time]" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/had_pretests.py] + notes: "Mirrors [M-037]; see [M-098]." + - id: M-111 + kind: param + group: renames-col-suffix + old: "diff_diff:did_had_pretest_workflow[unit_col]" + new: "diff_diff:did_had_pretest_workflow[unit]" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/had_pretests.py] + notes: "Mirrors [M-038]; see [M-098]." + - id: M-112 + kind: param + group: renames-col-suffix + old: "diff_diff:did_had_pretest_workflow[first_treat_col]" + new: "diff_diff:did_had_pretest_workflow[first_treat]" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/had_pretests.py] + notes: "Mirrors [M-039]; see [M-098]." + - id: M-113 + kind: param + group: renames-col-suffix + old: "diff_diff:trim_weights[weight_col]" + new: "diff_diff:trim_weights[weights]" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/prep.py] + notes: "Public prep helper; section 8 rule 5. Target spelling is 'weights', the established library name for a weight column (5 public class surfaces use it) - NOT bare 'weight'." + - id: M-114 + kind: field + group: renames-dcdh + old: "diff_diff:ChaisemartinDHaultfoeuilleResults.groups" + new: "diff_diff:ChaisemartinDHaultfoeuilleResults.units" + introduced_in: "3.9" + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/chaisemartin_dhaultfoeuille_results.py, diff_diff/chaisemartin_dhaultfoeuille.py] + notes: "Results-side mirror of [M-033] under section 8 rule 9. dCDH's 'groups' are UNIT identifiers ('Group identifiers in the post-filter sample'), the same column fit[group] renames to unit - distinct from the CS-family 'groups', which are ATT(g,t) cohorts and stay as domain vocabulary. summary()/repr() labels follow; needs __setstate__ pickle migration per spec section 5. The n_groups_dropped_* counters are NOT renamed by this row (they count dropped units but read as diagnostics; re-audit at 4.0 if the vocabulary drifts)." + - id: M-115 + kind: param + group: renames-robust-drop + old: "diff_diff:LinearRegression[robust]" + new: null + deprecated_in: "3.9" + removed_in: "4.0" + status: planned + phase: 2 + warning: FutureWarning + code_refs: [diff_diff/linalg.py] + notes: "Fourth site of the 'robust' drop - spec section 7 says the flag dies 'everywhere it exists' but [M-045]..[M-047] enumerated only three. LinearRegression is a top-level export with its OWN __init__ (the TWFE/MultiPeriodDiD hits inherit DifferenceInDifferences.__init__ and ride [M-045]). Redundant with vcov_type, same as its siblings." diff --git a/docs/v4-design.md b/docs/v4-design.md index de2ec853..cf38b98e 100644 --- a/docs/v4-design.md +++ b/docs/v4-design.md @@ -137,16 +137,21 @@ signatures. Lifecycle facts live in the cited rows. `unit` [M-033], `controls` -> `covariates` [M-034]. - `HeterogeneousAdoptionDiD.fit(outcome=, dose=, time=, unit=, first_treat=, ...)` - `_col` suffixes dropped [M-035]..[M-039]. -- `RegressionDiscontinuity.fit(outcome=, running=, treatment=, ...)` - - [M-040]..[M-042]. +- `RegressionDiscontinuity.fit(outcome=, running=, takeup=, ...)` - + [M-040]..[M-042]; `takeup` rather than `treatment` because the column + accepts non-binary dose take-up (rule 7's corollary, section 8), with the + results-field mirror at [M-094]. - `StackedDiD(control_group=...)` - `clean_control` renamed [M-043]. - `WooldridgeDiDResults.to_dataframe(level=...)` - `aggregation` -> `level` [M-044]; the `"event"` value spelling unifies across its existing `aggregate(type=)` surface [M-086] and `summary(aggregation=)` is retired for the uniform `summary(alpha=None)` [M-087]. - `robust` constructor param dropped everywhere it exists - [M-045]..[M-047] - fully redundant with `vcov_type`, and its default even - differed across estimators (True/True/False). + [M-045]..[M-047] [M-115] - fully redundant with `vcov_type`, and its default + even differed across estimators (True/True/False). Four sites, not three: + [M-115] adds `LinearRegression`, which owns its `__init__`; the + TwoWayFixedEffects / MultiPeriodDiD occurrences inherit + `DifferenceInDifferences.__init__` and ride [M-045]. ### 3.5 The diagnostic family @@ -419,12 +424,45 @@ domain vocabulary, not drift. ## 7. Inference surface - `vcov_type` becomes the single SE-type selector wherever analytical SEs - exist; the redundant `robust` flag dies [M-045]..[M-047]. Estimators without + exist; the redundant `robust` flag dies [M-045]..[M-047] [M-115]. Estimators without analytical SEs (SyntheticControl, TROP, CiC) document their native variance methods instead of growing a dead param. - Wild bootstrap is exposed via `inference="wild_bootstrap"` uniformly where - supported (today's split between `inference=` and `n_bootstrap>0` idioms is - resolved in the Phase 2 plan for the affected estimators). + supported [M-096] - "supported" meaning the estimator offers wild CLUSTER + bootstrap as an ALTERNATIVE to analytical SEs. That roster is + DifferenceInDifferences + TwoWayFixedEffects. (MultiPeriodDiD also carries + the param but has no wild-bootstrap path - it warns and falls back to + analytical; it is removed at 4.0 [M-010] and section 4.1 replaces that + fallback with an explicit raise in the merged event-study mode, so the roster + pins estimators that SUPPORT WCR, not those that merely expose the param.) + + **The selector must fail closed** - it does not today. `inference=` is stored + without any valid-value check, and DiD routes `wild_bootstrap` WITHOUT + `cluster=` silently to analytical (pinned by + `tests/test_wild_bootstrap.py::test_did_wild_bootstrap_requires_cluster`), so + a typo or a missing prerequisite quietly changes which SE/p-value/CI + procedure runs - the no-silent-failures principle applied to inference + selection. Fail closed means REFUSE, not warn-and-degrade: Phase 2 pins the + accepted set to exactly `{"analytical", "wild_bootstrap"}` on `__init__` and + transactional `set_params`, and makes `wild_bootstrap` without `cluster=` + raise `ValueError`. REGISTRY specifies WCR as `inference="wild_bootstrap"` + *(with `cluster=`)*, so that combination is incoherent rather than merely + unsupported - and a warning still leaves a procedure running that the caller + did not ask for. + + The apparent `inference=` vs `n_bootstrap>0` split is NOT drift: an estimator + whose bootstrap IS its inference method runs a different procedure - + CallawaySantAnna an influence-function multiplier bootstrap, SunAbraham a + unit-pairs bootstrap (Rao-Wu rescaled under stratified/PSU survey designs), + ChaisemartinDHaultfoeuille a group-level influence-function multiplier + bootstrap that upgrades to PSU-level Hall-Mammen wild clustering under survey + designs with strictly-coarser PSUs - against DiD/TWFE's residual wild cluster + bootstrap with test-inverted CIs and no reference t-distribution. Spelling + any of those `"wild_bootstrap"` would make a uniform name carry altered + meaning, which rule 8 of section 8 forbids; they keep `n_bootstrap` as + documented domain vocabulary on the section 6 precedent. Should one of them + later want an `inference=` selector, the value names its own method - that is + additive, post-4.0, minor-version work, not part of this program. - `n_bootstrap` semantic unification [M-081]: `0` = bootstrap off wherever an analytical path exists; bootstrap-only estimators document their positive defaults. Counts stay tuned per estimator (999 light / 200 compute-heavy) - @@ -472,10 +510,55 @@ missed.** 7. `treatment` names a 0/1 indicator (group membership or take-up), never a cohort and never a treatment level; estimators with non-binary per-period treatment (dCDH) document theirs as domain vocabulary. + Corollary: a column that MAY be non-binary does not get this name even + when it is conceptually take-up. Fuzzy RD's observed-take-up column + accepts dose values (matching R's `fuzzy=`; the estimand label degrades + from complier-LATE to a bare local Wald ratio accordingly), so [M-042] + [M-094] target `takeup` - the spelling the RD docstring already uses - + rather than `treatment`. 8. Hybrid-naming principle (locked 2026-07-12): diff-diff names where the library owns the concept; domain vocabulary where it is the field's language; an R-equivalents mapping table (`yname/tname/idname/gname` -> `outcome/time/unit/first_treat`) ships in the docs, not as param aliases. +9. **A rename carries to every surface that mirrors it.** Renaming a + constructor or `fit()` param renames any RESULTS field that echoes it back + (and the `summary()` label that prints it), in the SAME diff and on the same + shim schedule - a results field is public surface, is emitted by + `to_dict()`, and section 5 forbids deprecated names in serialized output + from 4.0. Mirrors added by amendment: [M-094] (mirrors [M-042]), [M-095] + (mirrors [M-043]), [M-114] (mirrors [M-033]). When adding a rename row, + grep the matching `*_results.py` for the old name before assuming the param + row is the whole job. +10. **These rules bind module-level PUBLIC FUNCTIONS, not just classes.** The + Phase 1 inventory was derived from the estimator roster, so exported + functions went unaudited; the completeness sweep added [M-097] (the + RETAINED `twowayfeweights` diagnostic - section 3.3 keeps it past 4.0, so + no wrapper removal covers it), [M-098]..[M-112] (the three exported HAD + pretest entry points, five `_col` params each), and [M-113] + (`trim_weights`). A function that survives 4.0 gets the same shim schedule + as a class surface; only surfaces scheduled for REMOVAL + ([M-070]..[M-077]) are exempt, because the removal moots the name. + +11. **A rename's scope includes every READER of the old name.** A row is not + done when the definition site is renamed - it is done when nothing still + reads the old name. Internal consumers migrate in the same diff, and the + row's `code_refs` name them so the removal PR cannot miss them (see + [M-043] / [M-095], whose readers span `power.py`, `practitioner.py`, + `_reporting_helpers.py`, `business_report.py` and REPORTING.md). The + specific hazard is `getattr(obj, "old_name", )`: after removal it + returns the DEFAULT instead of raising, so the consumer silently reports + the wrong thing rather than crashing - a wrong-answer regression that no + removal pin catches. Grep for the old name across `diff_diff/` and + `docs/methodology/` before marking any rename row terminal. + +**Domain vocabulary that is NOT a violation** (recorded so the sweep is not +re-litigated): the staggered family's `group`/`groups` on results containers +and `GroupTimeEffect.group` name the ATT(g,t) COHORT in Callaway-Sant'Anna's +own notation - rule 8 protects exactly this, and renaming them to `unit` would +be actively wrong. `TripleDifference.fit[group]` is the treated-group 0/1 +indicator that rule 3 explicitly reserves the name for. dCDH is the one +estimator where `group` genuinely means a unit id, which is why [M-033] and +[M-114] exist and the CS-family fields have no rows. **Naming-checkpoint outcomes (2026-07-18), with losing candidates:** `event_study=` bool (over `effects=` enum, `dynamic=` bool); @@ -496,12 +579,41 @@ above; anything only one PR cares about stays in that PR's plan.** | Phase | Ships in | PRs (each: dedicated shim/removal tests + matrix flips + CHANGELOG naming flipped row ids) | |---|---|---| | 1 (this PR) | - | Spec + matrix + enforcement test + support edits | -| 2: contract foundations | 3.9 | (a) results base + unified event-study representation + to_dict completion + the Diagnostic marker base on the diagnostic result roster [M-091] (section 3.5); (b) `aggregate()` + fit(aggregate=) shims [M-020..M-027]; (c) param renames [M-030..M-047] + BaseEstimator mixin + ContinuousDiD covariates move; (d) alias introductions [M-062] [M-063] + wrapper deprecations [M-070..M-077] + n_bootstrap docs [M-081] | +| 2: contract foundations | 3.9 | (a) results base + unified event-study representation [M-092] + to_dict completion + the Diagnostic marker base on the diagnostic result roster [M-091] (section 3.5); (b) `aggregate()` + fit(aggregate=) shims [M-020..M-027]; (c) param renames [M-030..M-047] [M-084] [M-086..M-089] + their results-field mirrors [M-094] [M-095] (section 8 rule 9) + the public-function completeness sweep [M-097..M-113] (section 8 rule 10) + the dCDH results mirror [M-114] + the fourth `robust` site [M-115] + BaseEstimator mixin + ContinuousDiD covariates move; (d) alias introductions [M-062] [M-063] + wrapper deprecations [M-070..M-077] + the two inference-surface policies: `n_bootstrap` semantic unification [M-081] and the wild-cluster-bootstrap roster guard [M-096] | | 3: merges | 3.9 | (a) TWFE event-study mode [M-010] + EventStudy warn [M-060] (gates: section 4.1's equivalence/divergence/pooled-parity test triple); (b) TripleDifference facade [M-013]; (c) CiC method= [M-015] | | 4: release + soak | 3.9 cut | Migration guide written (skeleton: section 10); maintainer cuts 3.9; maint/3.8 rule active | -| 5: enforcement | 4.0 | Removals [M-010..M-016, M-030..M-047 old names, M-060, M-061, M-070..M-077, M-001..M-003]; storage flips [M-050..M-058]; default policies [M-004..M-006, M-080]; warning retirement [M-007]; fastpath go/no-go [M-008]; diagnostic-family docs/roster reorganization [M-090]; docs/llms.txt/README refresh | +| 5: enforcement | 4.0 | Removals [M-010..M-016, M-030..M-047 old names, M-060, M-061, M-070..M-077, M-001..M-003] + the amendment's old names [M-094] [M-095] [M-097..M-115] (incl. their consumer migrations and the `clean_control` serialized reporting key); storage flips [M-050..M-058]; default policies [M-004..M-006, M-080]; warning retirement [M-007]; fastpath go/no-go [M-008]; diagnostic-family docs/roster reorganization [M-090]; sentinel retirement [M-093]; docs/llms.txt/README refresh | | 6: front door | 4.1 | `event_study(data, outcome, unit, time, first_treat, estimator=...)` comparison entry point over the staggered family (sketch only; specified in its own plan) | +**3.9-cut checklist (un-rowed obligations).** `test_due_rows_are_terminal` +gates everything that HAS a row; the following Phase 2 obligations are real but +not expressible as ledger rows, so the 3.9 release PR asserts them by hand: + +1. The shared `BaseEstimator` mixin has replaced the hand-rolled + `get_params`/`set_params` copies (section 7), with `deep=` uniform and + set_params transactional per the locked rule. A pure refactor - no symbol + changes - so no row can see it. +2. The R-equivalents mapping table (section 8 rule 8) ships in the docs. +3. The migration guide exists (section 10) and its TL;DR table has a row per + breaking change, generated against the matrix rather than hand-listed. +4. `docs/v4-deprecations.yaml` and this document agree on the phase breakdown + in the table above - any PR that re-scoped a phase edited both. + +Everything else queued for 3.9 is row-gated, by one of two mechanisms. Symbol +rows that declare a `warning` gate on `deprecated_in` - the shim must have +shipped ([M-010] [M-013] [M-015], [M-020]..[M-027], [M-030]..[M-047], +[M-070]..[M-077], [M-082], +[M-084], [M-086]..[M-089], [M-094] [M-095], [M-097]..[M-115]). Rows with no +shim to assert gate +on `introduced_in` instead - the new surface must have shipped: the +introduce-only aliases [M-062] [M-063] and the `behavior`-kind policies +[M-081] [M-091] [M-092] [M-096]. That second mechanism is deliberate for +behavior rows: the early-flip guard keys off `deprecated_in`, so a flip version +would fail the very PR that implements the obligation (it lands while +`__version__` is still 3.8.x). Use `introduced_in`, not `deprecated_in`, for +any future behavior row that must ship in a release the repo has not yet +bumped to. + **4.0-cut checklist (final item):** the due-row sweep is AUTOMATED - `tests/test_v4_matrix.py::test_due_rows_are_terminal` fails any release bump whose version reaches a row's scheduled version while its status has not @@ -616,8 +728,10 @@ forever - a removed symbol resurrecting is a test failure. class row (schema-enforced). Top-level `diff_diff:Name` class/function rows and alias rows also assert `__all__` membership consistent with their status (stale `import *` entries fail). The shipped row ids are a - committed snapshot in the enforcement test (77 as of Phase 2a: Phase 1 + - the diagnostic-family amendment + the M-092/M-093 results-contract rows; + committed snapshot in the enforcement test (99 as of the + gating-completeness amendment: Phase 1 + the diagnostic-family amendment + + the M-092/M-093 results-contract rows + the M-094..M-096 amendment rows + + the M-097..M-115 completeness sweep; the snapshot extends by a new id range in the same diff that appends rows): ids are never deleted or reused, and the test fails if any snapshot id disappears. diff --git a/tests/test_v4_matrix.py b/tests/test_v4_matrix.py index 86138532..35aba6f6 100644 --- a/tests/test_v4_matrix.py +++ b/tests/test_v4_matrix.py @@ -110,17 +110,24 @@ _MD_TOKEN_RE = re.compile(r"\[(M-\d{3})\]") # Row-count floor: 75 rows from the Phase 1 spec + diagnostic-family amendment, -# plus the 2 Phase 2a results-contract rows (M-092/M-093) = 77. Ids are never -# reused and terminal rows are never deleted, so the ledger only grows - raise -# the floor when rows are added; a lower parse count means scanner/format drift -# or an illegal row deletion. -ROW_COUNT_FLOOR = 77 +# plus the 2 Phase 2a results-contract rows (M-092/M-093) = 77, plus the 3 +# gating-completeness amendment rows (M-094..M-096) = 80, plus the 19-row +# completeness sweep over public FUNCTIONS and the dCDH results mirror +# (M-097..M-115) = 99. Ids are never reused and terminal rows are never +# deleted, so the ledger only grows - raise the floor when rows are added; a +# lower parse count means scanner/format drift or an illegal row deletion. +ROW_COUNT_FLOOR = 99 # Committed snapshot of the shipped id set ("ids are never deleted or reused" # contract - a delete-one-add-one edit keeps the count above the floor but trips # this). Extend with a NEW tuple, never edit an existing one. Ranges: # (1,8)/(10,16)/(20,27)/(30,47)/(50,58)/(60,64)/(70,77)/(80,91) = Phase 1 + the -# diagnostic-family amendment; (92,93) = Phase 2a results-contract rows. +# diagnostic-family amendment; (92,93) = Phase 2a results-contract rows; +# (94,96) = the gating-completeness amendment (two results-field mirrors of +# existing param rows, plus the wild-bootstrap exposure policy); +# (97,115) = its completeness sweep over module-level public functions +# (twowayfeweights, three HAD pretest entry points, trim_weights) plus the +# dCDH results mirror and the fourth `robust` site. _INITIAL_ID_RANGES = [ (1, 8), (10, 16), @@ -131,6 +138,8 @@ (70, 77), (80, 91), (92, 93), + (94, 96), + (97, 115), ] EXPECTED_INITIAL_IDS = frozenset( f"M-{n:03d}" for lo, hi in _INITIAL_ID_RANGES for n in range(lo, hi + 1) @@ -527,10 +536,12 @@ def test_initial_ids_never_deleted(): """The shipped id set is immutable: ids are never deleted or reused (spec section 11). ROW_COUNT_FLOOR alone would let a delete-one-add-one edit pass; this snapshot cannot. - Extends as rows ship (77 as of Phase 2a: Phase 1 + diagnostic-family + M-092/M-093).""" + Extends as rows ship (99 as of the gating-completeness amendment: Phase 1 + + diagnostic-family + M-092/M-093 + M-094..M-096 + the M-097..M-115 + public-function completeness sweep).""" missing = sorted(EXPECTED_INITIAL_IDS - set(_ROW_IDS)) assert not missing, f"ledger rows deleted (ids are permanent): {missing}" - assert len(EXPECTED_INITIAL_IDS) == 77 + assert len(EXPECTED_INITIAL_IDS) == 99 def test_version_tuple_pads_to_three_components():