Skip to content

fix(tests): sandbox doc-snippet env mutations; order-robust dCDH baseline backend detection#637

Merged
igerber merged 1 commit into
mainfrom
fix/dcdh-baseline-test-order-robust
Jul 7, 2026
Merged

fix(tests): sandbox doc-snippet env mutations; order-robust dCDH baseline backend detection#637
igerber merged 1 commit into
mainfrom
fix/dcdh-baseline-test-order-robust

Conversation

@igerber

@igerber igerber commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Summary

  • Root cause: the doc-snippet runner (tests/test_doc_snippets.py) exec'd documentation code blocks without sandboxing os.environ, so the troubleshooting page's backend-override snippet (os.environ['DIFF_DIFF_BACKEND'] = 'python', docs/troubleshooting.rst:329) leaked into every later test of a full-suite run — 7,008 teardowns with perturbed state, traced with a teardown-hook leak detector. The runner now snapshot/restores os.environ in a finally around each snippet exec.
  • Victim hardened (defense in depth): test_survey_dcdh.py::test_bootstrap_se_matches_pre_pr4_baseline selected its pinned-baseline arm from a call-time os.environ / diff_diff._backend read — order-fragile, since an env mutation after import flips the detection without changing the already-imported dispatch. It now derives the arm from the same dispatch globals the fit consumes (bootstrap_utils.HAS_RUST_BACKEND/_rust_bootstrap_weights, linalg.HAS_RUST_BACKEND/_rust_solve_ols) with a coherence assert between the two.
  • Closes the Testing/docs TODO row for the order-dependent dCDH baseline failure (this was the exact documented signature: fails under full-suite order with backend=pure-python while producing the rust arm's value, passes standalone).

Methodology references (required if estimator / math changes)

  • Method name(s): N/A — no methodology changes (test-harness + test-detection only; the pinned dCDH bootstrap baselines themselves are unchanged)
  • Paper / source link(s): N/A
  • Any intentional deviations from the source (and why): None

Validation

  • Tests added/updated: tests/test_doc_snippets.py (env sandbox), tests/test_survey_dcdh.py (dispatch-true arm selection).
  • Repro/verification: minimal order repro pytest tests/test_doc_snippets.py tests/test_survey_dcdh.py::TestBootstrapCellPeriod::test_bootstrap_se_matches_pre_pr4_baseline fails on main, passes with the fix; full local suite previously 1 failed / 8558 passed (this test the sole failure), the leak detector confirms the first perturbed teardown is test_doc_snippet[troubleshooting:block11].

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

Generated with Claude Code

…line backend detection

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>
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Overall Assessment

✅ Looks good. No unmitigated P0/P1 findings.

Executive Summary

  • The PR changes test harness behavior and test baseline detection only; no estimator, math, weighting, variance, SE, or default behavior changes were introduced.
  • dCDH survey/bootstrap behavior matches the documented registry contract for PSU=group identity routing and bootstrap inference.
  • tests/test_doc_snippets.py now restores os.environ in a finally, which addresses the leaked backend override.
  • tests/test_survey_dcdh.py now selects the baseline from imported dispatch globals rather than call-time environment state, matching the backend path consumed by the test.
  • Syntax was verified with an in-memory compile check. py_compile could not run because the sandbox is read-only and blocked tests/__pycache__.

Methodology

No findings.

Affected method: ChaisemartinDHaultfoeuille dCDH survey bootstrap regression guard only. The Registry documents dCDH sources/scope at docs/methodology/REGISTRY.md:L798-L804, multiplier bootstrap as a library extension at docs/methodology/REGISTRY.md:L916, and the PSU=group legacy group-level fast path at docs/methodology/REGISTRY.md:L934-L936. The PR does not change estimator implementation or inference formulas.

Code Quality

No findings.

The environment sandbox is correctly placed around snippet execution with finally restoration at tests/test_doc_snippets.py:L391-L428. The dCDH baseline selection now uses bootstrap_utils / linalg imported backend globals at tests/test_survey_dcdh.py:L1877-L1894.

Performance

No findings.

The changes are test-only and add negligible overhead: one os.environ.copy() plus clear/update per doc snippet.

Maintainability

No findings.

The new comments explain the order-dependent failure mode and why call-time os.environ reads are avoided.

Tech Debt

No findings.

The PR removes the already-tracked TODO row for the order-dependent dCDH baseline failure at TODO.md:L65-L73, which is consistent with the documented deferred-work policy.

Security

No findings.

The environment restoration reduces cross-test process-state leakage. No secrets or unsafe external behavior were introduced.

Documentation/Tests

No blocking findings.

CHANGELOG.md:L66-L75 documents the fix. I could not execute the affected pytest tests in this read-only environment, but both changed test files passed an in-memory Python syntax compile.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Jul 7, 2026
@igerber igerber merged commit 43b694d into main Jul 7, 2026
31 of 32 checks passed
@igerber igerber deleted the fix/dcdh-baseline-test-order-robust branch July 7, 2026 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-ci Triggers CI test workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant