fix(dml): fail closed when cross-fit learners cannot be isolated

.github/workflows/rust-test.yml

@@ -154,7 +154,7 @@ jobs:
 
       - name: Install test dependencies
         # Keep in sync with pyproject.toml [project.dependencies] and [project.optional-dependencies.dev]
-        run: pip install pytest pytest-xdist numpy pandas scipy
+        run: pip install pytest pytest-xdist numpy pandas scipy scikit-learn
 
       - name: Install OpenBLAS (Linux)
         if: runner.os == 'Linux'
@@ -255,7 +255,7 @@ jobs:
 
       - name: Install dependencies
         # Keep in sync with pyproject.toml [project.dependencies] and [project.optional-dependencies.dev]
-        run: pip install numpy pandas scipy pytest pytest-xdist
+        run: pip install numpy pandas scipy pytest pytest-xdist scikit-learn
 
       - name: Verify pure Python mode
         run: |

DEFERRED.md

@@ -149,7 +149,7 @@ decisions (refactor waivers, perf trade-offs, test-infrastructure calls) are rec
 | **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 stays out of dev deps; clone-identity tests remain importorskip-only.** The sklearn-`clone()` round-trip tests (`test_base_estimator.py`, had/rdd/cic suites) run only where scikit-learn happens to be installed - deliberate, matching the numpy/pandas/scipy-only dependency posture; the always-running contract is the dependency-free re-instantiation config-equality test. | `tests/test_base_estimator.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 |
 | **`StackedDiD` survey re-resolution intra-file dedup not warranted.** The cross-estimator ContinuousDiD/EfficientDiD panel-to-unit collapse consolidation landed in #226 (`ResolvedSurveyDesign.subset_to_units_by_row_idx` / `build_unit_first_row_index`); StackedDiD deliberately re-resolves at stacked granularity (control units are duplicated across sub-experiments). The residual intra-file dedup (raw-weight extraction ×3, compose-normalize ×3, resolve-on-stacked ×2) is stacked-specific, low value, and touches the numerically-sensitive composed-weight renormalization; cross-estimator unification was assessed and is not warranted (genuinely different mechanisms; all three already delegate to `_resolve_survey_for_fit` / `compute_survey_metadata`). | `stacked_did.py` | #226 review |
 | **`HeterogeneousAdoptionDiD` Phase-4 Pierce-Schott (2016) replication harness waived.** R parity at `atol=1e-8` on the same 3 DGPs is a strictly stronger anchor than reproducing Fig 2's pointwise CIs on the LBD-restricted PNTR panel (paper §5.2 self-acknowledges NP estimators too noisy there). Re-open only on user demand. See REGISTRY HAD Deviations Notes #3/#4. | `benchmarks/`, `tests/` | Phase 2a / 2026-05-20 |

changelog.d/20260903-dml-crossfit-learner-isolation.md

@@ -0,0 +1,9 @@
+### Behavioral Changes
+- **Cross-fit learner isolation now fails closed**: DMLDiD replaces the prior
+  warning-and-reuse fallback. A one-copy preflight raises `TypeError` before
+  any group-time cell is fitted when a custom learner template's `deepcopy`
+  fails or returns the original object; a per-fold clone failure later in the
+  fit propagates as the same sanitized `TypeError` (never a NaN-cell skip).
+  Implement `__deepcopy__` to return an independent instance; custom
+  implementations remain responsible for nested mutable state. A failed
+  re-fit now also clears any previous fitted result.

changelog.d/20260908-deprecation-sentinel-copy.md

@@ -0,0 +1,5 @@
+### Fixed
+- **Preserve omitted-parameter sentinels when copying**: shallow and deep
+  copies of the shared deprecation sentinel retain its identity, keeping
+  StackedDiD parameter dictionaries equal after scikit-learn cloning.
+  Deprecated-parameter warnings and alias resolution are unchanged.

diff_diff/_crossfit.py

@@ -10,11 +10,12 @@
 
 ``cross_fit_predict`` produces out-of-fold nuisance predictions for EVERY
 unit: for each fold k the learner is fit on ``train_mask(k) & fit_mask`` and
-predicts all units in fold k. Each fold fits a DEEP COPY of the user's
-(never-fit) learner template, so no state — nested estimators and container
-parameters included — can carry across folds; an un-deep-copyable learner is
-reused with a loud warning under the fit-reset contract (see
-``diff_diff._learners``).
+predicts all units in fold k. Each fold uses a distinct top-level DEEP COPY of
+the user's (never-fit) learner template, so the template itself cannot carry
+fitted state across folds. Custom ``__deepcopy__`` implementations remain
+responsible for isolating nested mutable state. A template that cannot be
+deep-copied to a distinct top-level object fails closed with ``TypeError``
+(see ``diff_diff._learners``).
 
 Exception semantics (determinate):
 
@@ -32,8 +33,8 @@
 """
 
 import copy
+import inspect
 import pickle
-import warnings
 from dataclasses import dataclass, field
 from typing import Any, Dict, Iterator, Literal, NamedTuple, Optional, Tuple, cast, overload
 
@@ -60,41 +61,58 @@
 _LOG_LOSS_CLIP = 1e-15
 
 
-def _fresh_learner(learner: Any, *, stacklevel: int) -> Any:
-    """Per-fold learner isolation: a deep copy of the (never-fit) template.
-
-    ``copy.deepcopy`` of the user's template gives every fold a fully
-    independent learner — nested estimators, estimators inside lists/dicts,
-    accumulators, and warm-start state included — so no state (and therefore
-    no data from a previous complement, which includes the current evaluation
-    fold) can carry across folds. This is strictly stronger than
-    get_params-based reconstruction (which shares any estimator stored inside
-    a container parameter). The template itself is never fit. A copy FAILURE
-    is never silent: the instance is reused with a loud ``UserWarning`` naming
-    the learner and the fit-reset assumption now being relied on
-    (no-silent-failures rule). ``stacklevel`` is supplied by the caller so the
-    warning is attributed to the entry point's own caller (``cross_fit_predict``
-    passes the level that lands on the user's call site; internal consumers
-    of ``iter_fold_fits`` pass the level that lands on themselves).
+def _clone_learner_template(learner: Any, *, label: str) -> Any:
+    """Return a distinct deep copy or raise a sanitised ``TypeError``.
+
+    The check deliberately proves only top-level identity. A custom
+    ``__deepcopy__`` remains responsible for isolating nested mutable state.
+    Error messages expose the learner class and copy exception class, never
+    foreign exception text that could carry credentials, paths, or data.
     """
     try:
-        return copy.deepcopy(learner)
-    except Exception as exc:  # noqa: BLE001 - loud fallback, never silent
-        # Exception CLASS only, never the message: a foreign learner's
-        # __deepcopy__ error text can embed credentials/paths/data excerpts,
-        # and this warning lands in notebook/CI logs (the same boundary as
-        # DMLDiD's persisted-diagnostics sanitization).
-        warnings.warn(
-            f"_crossfit: could not deep-copy the "
-            f"{type(learner).__name__} template for this fold "
-            f"({type(exc).__name__}); "
-            "REUSING the same instance and relying on its fit-reset behavior. "
-            "A warm-start/stateful learner in this situation can leak data "
-            "across folds.",
-            UserWarning,
-            stacklevel=stacklevel,
+        clone = copy.deepcopy(learner)
+    except Exception as exc:  # noqa: BLE001 - sanitize a foreign exception boundary
+        copy_error_class = type(exc).__name__
+    else:
+        if clone is not learner:
+            return clone
+        raise TypeError(
+            f"{label}: {type(learner).__name__} learner template's __deepcopy__ "
+            "returned the original object; implement __deepcopy__ to return an "
+            "independent instance."
         )
-        return learner
+
+    raise TypeError(
+        f"{label}: could not deep-copy {type(learner).__name__} learner template "
+        f"({copy_error_class}); implement __deepcopy__ to return an independent instance."
+    )
+
+
+def _probe_learner_cloneability(learner: Any, *, kind: str, param_name: str) -> Any:
+    """Return an independent clone after validating its learner protocol."""
+    clone = _clone_learner_template(learner, label=param_name)
+    validate_learner(clone, kind=kind, param_name=param_name)
+    return clone
+
+
+def _validate_sample_weight_support(learner: Any, *, param_name: str) -> None:
+    """Require a learner ``fit`` method that accepts ``sample_weight`` by keyword."""
+    try:
+        sig = inspect.signature(learner.fit)
+    except (TypeError, ValueError):  # pragma: no cover - exotic callables
+        return  # Cannot introspect; let the learner surface any fit-time error.
+    for param in sig.parameters.values():
+        if param.kind is inspect.Parameter.VAR_KEYWORD:
+            return
+        if param.name == "sample_weight" and param.kind in (
+            inspect.Parameter.POSITIONAL_OR_KEYWORD,
+            inspect.Parameter.KEYWORD_ONLY,
+        ):
+            return
+    raise TypeError(
+        f"{param_name}: learner object {type(learner).__name__!r} must accept "
+        "sample_weight by keyword in fit(); add a sample_weight parameter (or **kwargs)."
+    )
 
 
 def _unique_or_raise(arr: np.ndarray, name: str, **kwargs: Any) -> Any:
@@ -531,7 +549,6 @@ def _fit_subset(
     sample_weight: Optional[np.ndarray],
     k: int,
     label: str,
-    warn_stacklevel: int,
 ) -> Tuple[Any, int, Optional[np.ndarray]]:
     """Fit a fresh deep copy of ``learner`` on ``fit_idx`` (fold ``k``).
 
@@ -567,7 +584,10 @@ def _fit_subset(
 
     # (b) Learner errors during the fold -> DegenerateFoldError, chained.
     try:
-        fold_learner = _fresh_learner(learner, stacklevel=warn_stacklevel)
+        fold_learner = _clone_learner_template(learner, label=f"{label}fold {k} learner")
+        validate_learner(fold_learner, kind=kind, param_name=f"{label}fold {k} learner")
+        if w_fit is not None:
+            _validate_sample_weight_support(fold_learner, param_name=f"{label}fold {k} learner")
         # Unweighted path calls fit(X, y) WITHOUT the keyword: the
         # advertised duck-typed contract is fit/predict(_proba), so a
         # learner whose fit signature is only (X, y) must work when no
@@ -647,7 +667,6 @@ def _iter_fold_fits_impl(
     sample_weight: Optional[np.ndarray],
     kind: str,
     label: str,
-    warn_stacklevel: int,
 ) -> Iterator[FoldFit]:
     for k, train_idx, test_idx in folds.iter_folds():
         fit_idx = train_idx[fit_mask_arr[train_idx]]
@@ -661,7 +680,6 @@ def _iter_fold_fits_impl(
             sample_weight=sample_weight,
             k=k,
             label=label,
-            warn_stacklevel=warn_stacklevel,
         )
         yield FoldFit(
             k=k,
@@ -689,7 +707,6 @@ def iter_fold_fits(
     predict_method: str = "predict",
     sample_weight: Optional[np.ndarray] = None,
     context_label: str = "",
-    warn_stacklevel: int = 4,
 ) -> Iterator[FoldFit]:
     """Per-fold fitted learners (the building block under ``cross_fit_predict``).
 
@@ -698,9 +715,8 @@ def iter_fold_fits(
     ``k``, each holding a fresh deep copy of ``learner`` fit on
     ``train_mask(k) & fit_mask``. Predictions are the caller's job via
     ``_predict_subset``; this is what lets a consumer fit a NESTED nuisance on
-    fold ``k``'s own training units. ``warn_stacklevel`` is the frame the
-    deep-copy-failure warning is attributed to (default: the frame advancing
-    the iterator).
+    fold ``k``'s own training units. Clone failures raise a sanitised
+    ``TypeError`` when the iterator advances; the template is never reused.
     """
     X64, y64, fit_mask_arr, sw, kind = _prepare_cross_fit_inputs(
         learner,
@@ -712,9 +728,7 @@ def iter_fold_fits(
         sample_weight=sample_weight,
     )
     label = f"{context_label}: " if context_label else ""
-    return _iter_fold_fits_impl(
-        learner, X64, y64, folds, fit_mask_arr, sw, kind, label, warn_stacklevel
-    )
+    return _iter_fold_fits_impl(learner, X64, y64, folds, fit_mask_arr, sw, kind, label)
 
 
 @overload
@@ -768,8 +782,6 @@ def cross_fit_predict(
     fold_losses = np.empty(folds.n_folds, dtype=np.float64)
     n_fit_per_fold = np.empty(folds.n_folds, dtype=np.int64)
 
-    # stacklevel 5: _fresh_learner -> _fit_subset -> generator frame ->
-    # cross_fit_predict -> the user's call site.
     for ff in iter_fold_fits(
         learner,
         X,
@@ -779,7 +791,6 @@ def cross_fit_predict(
         predict_method=predict_method,
         sample_weight=sample_weight,
         context_label=context_label,
-        warn_stacklevel=5,
     ):
         n_fit_per_fold[ff.k] = ff.n_fit
         pred = _predict_subset(

diff_diff/_deprecation.py

@@ -52,6 +52,13 @@ class _NotSupplied:
     def __repr__(self) -> str:  # pragma: no cover - debugging aid
         return "<not supplied>"
 
+    # Parameter cloning must preserve this stateless marker's identity.
+    def __copy__(self) -> "_NotSupplied":
+        return self
+
+    def __deepcopy__(self, memo: Any) -> "_NotSupplied":
+        return self
+
 
 NOT_SUPPLIED = _NotSupplied()
 

diff_diff/_learners.py

@@ -4,10 +4,10 @@
 ----------------
 Learners ALWAYS receive a raw covariate matrix ``X`` with NO intercept column;
 every learner manages the intercept internally (sklearn convention). Learners
-are INSTANCES with sklearn fit-reset semantics: ``fit`` fully re-initializes
-the fitted state and returns ``self``. A stateful/warm-start user learner that
-violates fit-reset cannot be detected without taking a clone dependency —
-documented accepted limitation.
+are INSTANCES whose ``fit`` returns ``self``. Cross-fitting requires each
+template to ``deepcopy`` to a distinct top-level object; a custom
+``__deepcopy__`` implementation is responsible for isolating nested mutable
+state.
 
 Native learners (``"linear"``, ``"ridge"``, ``"logit"``, ``"sieve"``) wrap
 the ``diff_diff.linalg`` solvers and expose sklearn-style fitted state