Skip to content

Commit 43b694d

Browse files
igerberclaude
andauthored
fix(tests): sandbox doc-snippet env mutations; order-robust dCDH baseline backend detection (#637)
The doc-snippet runner exec'd documentation code blocks without sandboxing os.environ, so the troubleshooting page's backend-override snippet (os.environ['DIFF_DIFF_BACKEND'] = 'python') leaked into every later test in a full-suite run (7,008 teardowns with perturbed state, traced with a teardown-hook leak detector). The only victim was the dCDH pinned bootstrap baseline test, whose call-time env read selected the pure-Python baseline arm while the fit still dispatched to the already-imported Rust backend (fails under full-suite order, passes standalone — the TODO row's exact signature). Root cause: snapshot/restore os.environ around each snippet exec. Defense in depth: the dCDH test now derives its baseline arm from the same dispatch globals the fit consumes (bootstrap_utils / linalg, bound at import), with a coherence assert between the two. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 689c31c commit 43b694d

4 files changed

Lines changed: 48 additions & 7 deletions

File tree

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
7777
- **`fixest` cluster-robust SE band**: the DiD/TWFE cluster-at-unit SE is pinned within the
7878
documented ~0.25% fixest-CR1 small-sample DOF-convention band (guards an unintended SE-formula
7979
change; the machine-precision hetero/cluster lock is deferred — needs an unbalanced-DGP golden).
80+
- **Doc-snippet env leak fixed; dCDH pinned-baseline backend detection made order-robust.** The
81+
doc-snippet runner (`tests/test_doc_snippets.py`) executed documentation code blocks without
82+
sandboxing `os.environ`, so the troubleshooting page's backend-override snippet leaked
83+
`DIFF_DIFF_BACKEND='python'` into every later test in a full-suite run. The only victim was
84+
`test_survey_dcdh.py::test_bootstrap_se_matches_pre_pr4_baseline`, whose call-time env read then
85+
selected the pure-Python baseline arm while the fit still dispatched to the already-imported Rust
86+
backend (fails under full-suite order, passes standalone). The runner now snapshot/restores
87+
`os.environ` around each snippet (root cause), and the dCDH test derives its baseline arm from
88+
the same dispatch globals the fit consumes (`bootstrap_utils` / `linalg`), with a coherence
89+
assert between the two (defense in depth).
8090

8191
### Added
8292
- **`ImputationDiD` leave-one-out conservative variance** (`leave_one_out`, default `False`) — the

TODO.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ generic sparse-FE, QR+SVD rank-detection redundancy, `check_finite` bypass — m
6868
| Render `docs/methodology/REPORTING.md` and `REGISTRY.md` as in-site Sphinx pages so cross-refs can use `:doc:` instead of off-site `blob/main` URLs (stable-docs readers can otherwise land on a different revision than their package version). Two paths: (a) add `myst-parser` to `conf.py` + docs extras and link with `:doc:`, or (b) convert both to `.rst`. **Note:** REGISTRY.md is ~4.5k lines of LaTeX-heavy markdown — high risk under the `-W` (warnings-as-errors) Sphinx build; budget multiple rounds. | `docs/conf.py`, `docs/api/business_report.rst`, `docs/api/diagnostic_report.rst`, tutorials 18 & 19 | follow-up | Mid | Low |
6969
| `ImputationDiD` covariate-path variance lacks a dedicated parity anchor — only the no-covariate staggered panel is R-parity'd, though the covariate path shares the same validated projection code. Add a small dense-design **hand-calc** for the covariate projection (no external tooling), or a covariate (time-varying X) R `didimputation` golden asserting overall/ES SE parity (the golden variant needs local R). | `tests/test_methodology_imputation.py`, `benchmarks/R/generate_didimputation_golden.R` | imputation-validation | Mid | Low |
7070
| Add true half-sample BRR replicate-weight regressions per estimator family (current tests use Fay-like 0.5/1.5 perturbations; `test_survey_phase6.py` covers true BRR at the helper level). | `tests/test_replicate_weight_expansion.py` | #253 | Mid | Low |
71-
| `test_bootstrap_se_matches_pre_pr4_baseline` (dCDH bit-identity guard) fails under FULL-SUITE order but passes standalone/as-a-file: its `pure_python` backend detection reads a state some earlier test perturbs, so it compares against the wrong per-backend baseline arm (observed failure message says `backend=pure-python` while the run produces the other arm's value). Reproduced identically on pristine `origin/main` (2026-07-05, `b56931a6`), so it pre-dates the CS reg/ipw IF fix. Make the backend detection order-robust (e.g. re-resolve from `diff_diff._backend` at call time, or isolate via monkeypatch). | `tests/test_survey_dcdh.py::TestBootstrapCellPeriod` | CS-scaling | Quick | Low |
7271
| Port the CI `<notebook-prose>` extraction into the reviewer-eval harness so `docs/tutorials/*.ipynb` cases (currently guarded out of `verify-corpus`/`run`) can be reviewed with CI-equivalent context. | `tools/reviewer-eval/adapters/ci_prompt.py` | local-review | Mid | Low |
7372

7473
---

tests/test_doc_snippets.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
snippets and optional-dependency guards like matplotlib).
99
"""
1010

11+
import os
1112
import re
1213
import textwrap
1314
from pathlib import Path
@@ -375,11 +376,20 @@ def _restore_datasets_module():
375376
[pytest.param(tid, c, s, id=tid) for tid, c, s in _CASES],
376377
)
377378
def test_doc_snippet(test_id: str, code: str, skip_reason: Optional[str]):
378-
"""Execute a documentation code snippet and assert no API/runtime errors."""
379+
"""Execute a documentation code snippet and assert no API/runtime errors.
380+
381+
``os.environ`` is snapshot/restored around the exec: snippets may
382+
legitimately mutate the environment (e.g. the troubleshooting
383+
backend-override block sets ``DIFF_DIFF_BACKEND='python'``), and an
384+
unreverted mutation leaks process state into every later test in the
385+
session (it flipped the backend-arm selection of the dCDH pinned
386+
bootstrap baseline under full-suite order).
387+
"""
379388
if skip_reason:
380389
pytest.skip(skip_reason)
381390

382391
ns = _build_namespace()
392+
env_snapshot = os.environ.copy()
383393
try:
384394
exec(compile(code, f"<{test_id}>", "exec"), ns)
385395
except NameError as exc:
@@ -411,3 +421,8 @@ def test_doc_snippet(test_id: str, code: str, skip_reason: Optional[str]):
411421
f"Snippet {test_id} raised {type(exc).__name__}: {exc}\n\n"
412422
f"Code:\n{textwrap.indent(code, ' ')}"
413423
)
424+
finally:
425+
# Revert any environment mutation the snippet made (pytest.fail
426+
# raises, so this must be a finally, not a trailing statement).
427+
os.environ.clear()
428+
os.environ.update(env_snapshot)

tests/test_survey_dcdh.py

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
"""Survey support tests for ChaisemartinDHaultfoeuille (dCDH)."""
22

3-
import os
43
from typing import Optional
54

65
import numpy as np
@@ -1865,12 +1864,30 @@ def test_bootstrap_se_matches_pre_pr4_baseline(self):
18651864
to ULP precision. The baseline values were captured on
18661865
`origin/main` at `ac181b7f` (the PR #329 merge) under each
18671866
backend independently.
1867+
1868+
The baseline arm is selected from the SAME dispatch globals
1869+
the fit consumes (bound at import into bootstrap_utils /
1870+
linalg). Reading ``os.environ`` or ``diff_diff._backend`` at
1871+
call time is order-fragile: an env mutation after import
1872+
(e.g. a leaked doc-snippet backend override) flips that
1873+
detection without changing the already-imported dispatch,
1874+
selecting the wrong pinned-baseline arm under full-suite
1875+
order.
18681876
"""
1869-
from diff_diff._backend import HAS_RUST_BACKEND
1870-
pure_python = (
1871-
os.environ.get("DIFF_DIFF_BACKEND", "auto").lower() == "python"
1872-
or not HAS_RUST_BACKEND
1877+
from diff_diff import bootstrap_utils as _bu
1878+
from diff_diff import linalg as _la
1879+
1880+
bootstrap_rust = bool(
1881+
_bu.HAS_RUST_BACKEND and _bu._rust_bootstrap_weights is not None
1882+
)
1883+
ols_rust = bool(_la.HAS_RUST_BACKEND and _la._rust_solve_ols is not None)
1884+
assert bootstrap_rust == ols_rust, (
1885+
"Incoherent backend dispatch state between bootstrap_utils "
1886+
f"(rust={bootstrap_rust}) and linalg (rust={ols_rust}) — a "
1887+
"leaked monkeypatch? No pinned baseline exists for a mixed "
1888+
"backend; refusing to compare against either arm."
18731889
)
1890+
pure_python = not bootstrap_rust
18741891
expected = (
18751892
self._BASELINE_OVERALL_SE_PYTHON
18761893
if pure_python

0 commit comments

Comments
 (0)