Skip to content

Commit 48a1469

Browse files
committed
docs(v4): gating-completeness amendment - mirrored renames, public-function sweep, un-rowed obligations
Audits the public surface against the 4.0 spec's own contract-rename rules and closes the gaps, so the automated release gate sees every 3.9 obligation. Rename coverage. The Phase 1 inventory was derived from the ESTIMATOR ROSTER, which left two classes of surface unaudited: - RESULTS fields mirroring a renamed param. M-094 (RegressionDiscontinuityResults.treatment_col), M-095 (StackedDiDResults.clean_control), M-114 (ChaisemartinDHaultfoeuilleResults.groups -> .units, whose "groups" are unit ids). All are public dataclass fields emitted by to_dict(), so renaming the params alone would leave deprecated spellings on the serialized surface that section 5 forbids from 4.0. Section 8 gains rule 9. - Module-level public FUNCTIONS. M-097 (twowayfeweights(group=) -> unit=, which section 3.3 RETAINS past 4.0, so no wrapper removal covers it), M-098..M-112 (five _col params on each of joint_pretrends_test, joint_homogeneity_test, did_had_pretest_workflow), M-113 (trim_weights(weight_col=) -> weights=). Section 8 gains rule 10. M-115 adds the fourth `robust` site (LinearRegression): section 7 says the flag dies "everywhere it exists" while M-045..M-047 enumerated only three. M-042/M-094 target `takeup`, not `treatment`: fuzzy RD accepts non-binary dose take-up (matching R's fuzzy=, with the estimand degrading from complier-LATE to a bare local Wald ratio), and rule 7 reserves `treatment` for a 0/1 indicator. Rule 7 records the corollary. No shim had shipped, so the retarget is free. NOT violations, documented so the sweep is not re-litigated: the staggered family's group/groups and GroupTimeEffect.group name the ATT(g,t) COHORT in Callaway-Sant'Anna notation (rule 8 protects it; renaming to unit would be wrong), and TripleDifference.fit[group] is the treated-group indicator rule 3 reserves the name for. Gate coverage. Three Phase 2 obligations could not fail the release gate: M-081 (n_bootstrap semantics) gains introduced_in 3.9; new M-096 covers the inference="wild_bootstrap" selector contract; and the BaseEstimator mixin plus the R-equivalents docs table get a section 9 3.9-cut checklist since neither is rowable. Behavior rows use introduced_in rather than deprecated_in because the early-flip guard keys off deprecated_in and would fail the very PR that implements the obligation (it lands at 3.8.x) - the M-091/M-092 pattern, now written down. M-096 also records that the selector does not fail closed today: inference= is stored with no valid-value check, 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/CI procedure runs. Repairing that is now gated Phase 2 scope. The row is a drift guard, not a uniformity mandate: estimators whose bootstrap IS their inference method run materially different procedures (CS an influence-function multiplier bootstrap; SunAbraham a unit-pairs bootstrap, Rao-Wu rescaled on survey paths; dCDH a group-level IF multiplier upgrading to Hall-Mammen PSU under strictly-coarser survey PSUs) versus DiD/TWFE's residual WCR with test-inverted CIs, so one value spelling for all would make a uniform name carry altered meaning. M-008's notes record that its telemetry soak window is the 3.9 -> 4.0 gap. Ledger 77 -> 99 rows (floor and immutable-id snapshot extended in the same diff); rows gating a 3.9 bump 45 -> 68. Every new row is `planned` - the shims land in Phase 2c. TODO.md tracks the follow-up guard test that makes this completeness mechanical rather than dependent on who runs the audit. Docs, ledger and test-count updates only - no public API or numerical behavior change.
1 parent 6bdd5ac commit 48a1469

5 files changed

Lines changed: 489 additions & 17 deletions

File tree

CHANGELOG.md

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,81 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
88
## [Unreleased]
99

1010
### Added
11+
- **Internal: 4.0 gating-completeness amendment to the design spec and ledger.**
12+
An audit of the public surface against the spec's own contract-rename rules
13+
found two renames the initial inventory missed and three Phase 2 obligations
14+
the automated release gate could not see. (a) Two ledger rows added for
15+
RESULTS fields that mirror already-tracked params: `M-094`
16+
(`RegressionDiscontinuityResults.treatment_col` → `treatment`, mirroring
17+
`M-042`) and `M-095` (`StackedDiDResults.clean_control` → `control_group`,
18+
mirroring `M-043`). Both are public dataclass fields emitted by `to_dict()`,
19+
so renaming the params alone would have left deprecated spellings on the
20+
serialized surface that spec section 5 forbids from 4.0; each flips in the
21+
same diff as its param sibling. `docs/v4-design.md` section 8 gains rule 9
22+
making the mirror obligation normative, so the next rename sweep greps for it
23+
rather than rediscovering it. (b) `M-081` (`n_bootstrap` semantic
24+
unification) gains `introduced_in: 3.9` and a new `M-096` covers the uniform
25+
`inference="wild_bootstrap"` exposure (spec section 7) - both were real Phase
26+
2 obligations that no row gated, so a 3.9 cut could have shipped without
27+
them. Both use `introduced_in` rather than `deprecated_in` deliberately: for
28+
behavior rows the early-flip guard keys off `deprecated_in`, so a flip
29+
version would have failed the very PR that implements the obligation (it
30+
lands while `__version__` is still 3.8.x) - the `M-091`/`M-092` pattern.
31+
`M-096` covers the `inference="wild_bootstrap"` selector contract, which
32+
governs wild CLUSTER bootstrap offered as an alternative to analytical SEs -
33+
a roster of `DifferenceInDifferences` + `TwoWayFixedEffects`. Two things the
34+
audit established. (i) The selector does **not** fail closed today:
35+
`inference=` is stored with no valid-value check anywhere, and DiD routes
36+
`wild_bootstrap` without `cluster=` silently to analytical (a currently
37+
*pinned* behavior), so a typo or a missing prerequisite quietly changes which
38+
SE/p-value/CI procedure runs. Phase 2 must validate the accepted value set on
39+
`__init__` and transactional `set_params` and handle the missing-cluster case
40+
explicitly. (ii) The apparent `inference=` vs `n_bootstrap>0` split is not
41+
drift - estimators whose bootstrap *is* their inference method run materially
42+
different procedures (CallawaySantAnna an influence-function multiplier
43+
bootstrap; SunAbraham a unit-pairs bootstrap, Rao-Wu rescaled on survey
44+
paths; dCDH a group-level influence-function multiplier bootstrap upgrading
45+
to Hall-Mammen PSU wild clustering under strictly-coarser survey PSUs) versus
46+
DiD/TWFE's residual wild cluster bootstrap with test-inverted CIs, so one
47+
value spelling for all of them would make a uniform name carry altered
48+
meaning. They keep `n_bootstrap` as documented domain vocabulary, and section
49+
7 now records the boundary; an `inference=` selector for any of them would
50+
name its own method and is additive post-4.0 work.
51+
(c) The `BaseEstimator` mixin and the R-equivalents docs table are genuine
52+
refactor/docs work that cannot be ledger rows at all, so section 9 gains a
53+
3.9-cut checklist alongside the existing 4.0 one. `M-008`'s notes now record
54+
that its telemetry soak window *is* the 3.9→4.0 gap. (d) A completeness
55+
sweep over surfaces the Phase 1 inventory never audited. That inventory was
56+
derived from the estimator roster, so module-level public **functions** were
57+
never checked against section 8 at all: `M-097` renames
58+
`twowayfeweights(group=)` → `unit=` (section 3.3 **retains** that diagnostic
59+
past 4.0, so no wrapper removal covers it), `M-098`..`M-112` rename the five
60+
`_col` params on each of the three exported HAD pretest entry points
61+
(`joint_pretrends_test`, `joint_homogeneity_test`,
62+
`did_had_pretest_workflow`), `M-113` renames `trim_weights(weight_col=)` →
63+
`weights=`, `M-114` is the dCDH results mirror (`.groups` → `.units`, whose
64+
"groups" really are unit ids), and `M-115` adds the fourth `robust` site
65+
(`LinearRegression`, which section 7 said dies "everywhere it exists" while
66+
only three rows enumerated it). The same sweep found ~26 hits that are NOT
67+
violations and are now documented as such in section 8 so they are not
68+
re-litigated: the staggered family's `group`/`groups` name the ATT(g,t)
69+
**cohort** in Callaway-Sant'Anna's notation (rule 8 protects it; renaming to
70+
`unit` would be wrong), and `TripleDifference.fit[group]` is the
71+
treated-group indicator rule 3 explicitly reserves the name for. Section 8
72+
gains rule 10 binding the rules to public functions, not just classes.
73+
(e) `M-042`/`M-094`'s rename target moves from `treatment` to `takeup`:
74+
fuzzy RD's observed-take-up column accepts non-binary dose values (matching
75+
R's `fuzzy=`, with the estimand label degrading from complier-LATE to a bare
76+
local Wald ratio), while section 8 rule 7 reserves `treatment` for a 0/1
77+
indicator and never a treatment level - so the original target would have
78+
made a uniform name carry altered meaning. `takeup` is the spelling the RD
79+
docstring already uses; no shim had shipped, so the retarget is free. Rule 7
80+
gains the corollary that a column which MAY be non-binary does not take the
81+
`treatment` name even when it is conceptually take-up.
82+
Ledger row count 77 → 99 (enforcement-test floor and id snapshot extended in
83+
the same diff); rows gating a 3.9 bump 45 → 68. Docs + ledger + test-count
84+
updates only - **no public API or numerical behavior change** (every new row
85+
is `planned`; the shims themselves land in Phase 2c).
1186
- **Internal tooling: plan-review engine → dual-reviewer skill (no library
1287
change).** `/review-plan` + `/revise-plan` are retired for a
1388
`.claude/skills/plan-review/` skill that runs the **dual** review engine

TODO.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ generic sparse-FE, QR+SVD rank-detection redundancy, `check_finite` bypass — m
4545
| 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 |
4646
| 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 |
4747
| 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 |
48+
| 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. | `tests/`, `docs/v4-design.md` | gating-completeness amendment | Mid | Medium |
4849
| 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 |
4950
| 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 |
5051
| `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 |

0 commit comments

Comments
 (0)