From 9055ade2e04a2eb7a208bd678ff30fba402e5700 Mon Sep 17 00:00:00 2001 From: igerber Date: Sun, 26 Apr 2026 16:51:12 -0400 Subject: [PATCH 1/3] Add docs-tests.yml workflow; carve test_doc_snippets.py out of rust-test.yml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #389 (Batch A) shipped six broken HAD doc snippets in 2026-04 that no CI run caught because rust-test.yml only triggers on rust/, diff_diff/, tests/, pyproject.toml, and the workflow file — none of which include docs/. PR #396 patched the snippets but did not address the structural gap. This PR addresses it. Two changes: 1. New .github/workflows/docs-tests.yml — separate workflow that runs `pytest tests/test_doc_snippets.py -v` on a single ubuntu-latest / py3.14 / pure-Python runner. Triggers on docs/, diff_diff/, tests/test_doc_snippets.py, pyproject.toml, and the workflow file itself; same ready-for-ci label gate as rust-test.yml / notebooks.yml. Mirrors notebooks.yml's shape (the existing precedent for `pytest`-validated docs assets) so the two doc-validation workflows look like siblings. 2. .github/workflows/rust-test.yml: add --ignore=tests/test_doc_snippets.py to all three pytest invocations so doc snippets stop riding the code workflow. The Pure Python Fallback edit (line 193) is the only one that changes CI signal: that job runs from the repo root and was the ONLY place where test_doc_snippets.py actually executed. The two Rust-matrix edits (lines 158, 165) are defensive consistency: the matrix copies tests/ to /tmp/tests (rust-test.yml:138, 142) without docs/, so DOCS_DIR resolves to /tmp/docs/ which doesn't exist; the test collector silently skips every RST file via the guard at tests/test_doc_snippets.py:129. Adding --ignore there prevents the no-op from becoming a real run if anyone later adds `cp -r docs ...` to the copy steps. Each invocation now carries an in-YAML comment documenting which case it's the defensive vs behavior-changing edit. Verification: - python -c "import yaml; yaml.safe_load(open('.github/workflows/ docs-tests.yml')); yaml.safe_load(open('.github/workflows/ rust-test.yml'))" — both files well-formed. - pytest tests/ --ignore=tests/test_doc_snippets.py --ignore=tests/test_rust_backend.py --collect-only — 0 occurrences of test_doc_snippets in the collected set (was 115 cases collected when not ignored), confirming pytest accepts repeated --ignore flags as the existing line-193 pattern with --ignore=tests/ test_rust_backend.py already showed. After this PR opens, the workflow file itself triggers docs-tests.yml on its own change, providing the first end-to-end CI validation. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/docs-tests.yml | 56 ++++++++++++++++++++++++++++++++ .github/workflows/rust-test.yml | 14 ++++++-- 2 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/docs-tests.yml diff --git a/.github/workflows/docs-tests.yml b/.github/workflows/docs-tests.yml new file mode 100644 index 00000000..0a16faa3 --- /dev/null +++ b/.github/workflows/docs-tests.yml @@ -0,0 +1,56 @@ +name: Documentation Tests + +on: + push: + branches: [main] + paths: + - 'docs/**' + - 'diff_diff/**' + - 'tests/test_doc_snippets.py' + - 'pyproject.toml' + - '.github/workflows/docs-tests.yml' + pull_request: + branches: [main] + types: [opened, synchronize, reopened, labeled, unlabeled] + paths: + - 'docs/**' + - 'diff_diff/**' + - 'tests/test_doc_snippets.py' + - 'pyproject.toml' + - '.github/workflows/docs-tests.yml' + schedule: + # Weekly Sunday 6am UTC - smoke test that snippets still execute + # against current upstream deps (mirrors notebooks.yml schedule). + - cron: '0 6 * * 0' + +permissions: + contents: read + +jobs: + doc-snippets: + name: Validate RST code snippets + if: >- + github.event_name != 'pull_request' + || contains(github.event.pull_request.labels.*.name, 'ready-for-ci') + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v6 + + - name: Set up Python + uses: actions/setup-python@v6 + with: + # 3.14 to mirror Pure Python Fallback (the only existing job + # that actually ran these tests). notebooks.yml uses 3.11 for + # nbmake compat, not relevant here. + python-version: '3.14' + + - name: Install dependencies + # Keep in sync with pyproject.toml [project.dependencies] and [project.optional-dependencies.dev] + run: pip install numpy pandas scipy pytest + + - name: Run doc snippet tests in pure Python mode + # PYTHONPATH=. lets the test import diff_diff directly from + # source without invoking the maturin/Rust build (mirrors Pure + # Python Fallback at rust-test.yml:189-193). + run: PYTHONPATH=. DIFF_DIFF_BACKEND=python pytest tests/test_doc_snippets.py -v diff --git a/.github/workflows/rust-test.yml b/.github/workflows/rust-test.yml index 186075fb..d225b3a1 100644 --- a/.github/workflows/rust-test.yml +++ b/.github/workflows/rust-test.yml @@ -155,14 +155,18 @@ jobs: - name: Run tests with Rust backend (Unix) if: runner.os != 'Windows' working-directory: /tmp - run: DIFF_DIFF_BACKEND=rust pytest tests/ -q -n auto --dist worksteal -m '' + # Doc snippet tests own .github/workflows/docs-tests.yml; ignore + # them here to keep one workflow per surface and avoid double + # execution (the matrix copies tests/ to /tmp/tests without + # docs/, so this ignore is defensive on the Rust path). + run: DIFF_DIFF_BACKEND=rust pytest tests/ --ignore=tests/test_doc_snippets.py -q -n auto --dist worksteal -m '' - name: Run tests with Rust backend (Windows) if: runner.os == 'Windows' working-directory: ${{ runner.temp }} run: | $env:DIFF_DIFF_BACKEND="rust" - pytest tests/ -q -n auto --dist worksteal -m '' + pytest tests/ --ignore=tests/test_doc_snippets.py -q -n auto --dist worksteal -m '' shell: pwsh # Test pure Python fallback (without Rust extension) @@ -190,4 +194,8 @@ jobs: PYTHONPATH=. python -c "from diff_diff import HAS_RUST_BACKEND; print(f'HAS_RUST_BACKEND: {HAS_RUST_BACKEND}'); assert not HAS_RUST_BACKEND" - name: Run tests in pure Python mode - run: PYTHONPATH=. DIFF_DIFF_BACKEND=python pytest tests/ -q --ignore=tests/test_rust_backend.py -n auto --dist worksteal + # Doc snippet tests own .github/workflows/docs-tests.yml; ignore + # them here to keep one workflow per surface (this is the only + # invocation that actually executes test_doc_snippets.py since + # this job runs from the repo root, not /tmp/tests). + run: PYTHONPATH=. DIFF_DIFF_BACKEND=python pytest tests/ -q --ignore=tests/test_rust_backend.py --ignore=tests/test_doc_snippets.py -n auto --dist worksteal From 131f6e4bb3bde586e851ce2cb4a2d5c24b110301 Mon Sep 17 00:00:00 2001 From: igerber Date: Sun, 26 Apr 2026 17:14:58 -0400 Subject: [PATCH 2/3] Address PR #399 R1 review (2 P3, all non-blocking) P3 (Performance): rust-test.yml's `tests/**` trigger still fans out into the full Rust matrix on harness-only edits to tests/test_doc_snippets.py, even though every pytest invocation in that workflow now ignores the file. Added the GitHub Actions negative-pattern `!tests/test_doc_snippets.py` to both the push and pull_request paths blocks so a snippet-test-only edit no longer spins up the matrix. The file is owned exclusively by docs-tests.yml which still triggers (since its own `paths:` lists the file explicitly). P3 (Tech Debt): docs-tests.yml repeats the broad `labeled` / `unlabeled` trigger pattern that TODO.md:123 already tracks as avoidable CI churn for rust-test.yml + notebooks.yml under PR #269. Rather than diverge by pre-fixing the new workflow alone, extended the tracking row to include `docs-tests.yml` so the planned cross-cutting fix covers all three uniformly. Verification: python -c "import yaml; yaml.safe_load(open( '.github/workflows/rust-test.yml'))" succeeds; em-dash sweep clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/rust-test.yml | 4 ++++ TODO.md | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/rust-test.yml b/.github/workflows/rust-test.yml index d225b3a1..917390d0 100644 --- a/.github/workflows/rust-test.yml +++ b/.github/workflows/rust-test.yml @@ -7,6 +7,9 @@ on: - 'rust/**' - 'diff_diff/**' - 'tests/**' + # tests/test_doc_snippets.py is owned by docs-tests.yml; exclude it + # so a harness-only edit does not fan out into the Rust matrix. + - '!tests/test_doc_snippets.py' - 'pyproject.toml' - '.github/workflows/rust-test.yml' pull_request: @@ -16,6 +19,7 @@ on: - 'rust/**' - 'diff_diff/**' - 'tests/**' + - '!tests/test_doc_snippets.py' - 'pyproject.toml' - '.github/workflows/rust-test.yml' diff --git a/TODO.md b/TODO.md index 1d6cd918..eee1cf1c 100644 --- a/TODO.md +++ b/TODO.md @@ -120,7 +120,7 @@ Deferred items from PR reviews that were not addressed before merge. |-------|----------|----|----------| | ImputationDiD event-study SEs recompute full conservative variance per horizon (should cache A0/A1 factorization) | `imputation.py` | #141 | Low | | Rust faer SVD ndarray-to-faer conversion overhead (minimal vs SVD cost) | `rust/src/linalg.rs:67` | #115 | Low | -| Unrelated label events (e.g., adding `bug` label) re-trigger CI workflows when `ready-for-ci` is already present; filter `labeled`/`unlabeled` events to only `ready-for-ci` transitions | `.github/workflows/rust-test.yml`, `notebooks.yml` | #269 | Low | +| Unrelated label events (e.g., adding `bug` label) re-trigger CI workflows when `ready-for-ci` is already present; filter `labeled`/`unlabeled` events to only `ready-for-ci` transitions | `.github/workflows/rust-test.yml`, `notebooks.yml`, `docs-tests.yml` | #269 | Low | | `bread_inv` as a performance kwarg on `compute_robust_vcov` to avoid re-inverting `(X'WX)` when the caller already has it. Deferred from Phase 1a for scope. HC2 and HC2+BM both need the bread inverse, so a shared hint would save one `np.linalg.solve` per sandwich. | `linalg.py::compute_robust_vcov` | Phase 1a | Low | | Rust-backend HC2 implementation. Current Rust path only supports HC1; HC2 and CR2 Bell-McCaffrey fall through to the NumPy backend. For large-n fits this is noticeable. | `rust/src/linalg.rs` | Phase 1a | Low | | CR2 Bell-McCaffrey DOF uses a naive `O(n² k)` per-coefficient loop over cluster pairs. Pustejovsky-Tipton (2018) Appendix B has a scores-based formulation that avoids the full `n × n` `M` matrix. Switch when a user hits a large-`n` cluster-robust design. | `linalg.py::_compute_cr2_bm` | Phase 1a | Low | From d9ad1e0efd9697d59044526999d3b46556f7f464 Mon Sep 17 00:00:00 2001 From: igerber Date: Sun, 26 Apr 2026 17:22:15 -0400 Subject: [PATCH 3/3] Address PR #399 R2 review (1 P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P2: docs-tests.yml only path-matched tests/test_doc_snippets.py, but pytest auto-loads tests/conftest.py for any test under tests/ — and that file mutates sys.path (line 14: prepends tests/helpers/) and sets MPLBACKEND=Agg (line 18) before any test imports it. A change to tests/conftest.py that breaks either of those load-time steps would silently break the snippet test on every doc PR until a docs-tests.yml-triggering file happened to change alongside it, leaving a real CI blind spot. Fix: added 'tests/conftest.py' to both the push and pull_request `paths:` blocks in docs-tests.yml. An inline comment on the push block explains the dependency; the pull_request block doesn't repeat the prose to avoid drift between the two copies. Verified the workflow YAML is well-formed via yaml.safe_load. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/docs-tests.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/docs-tests.yml b/.github/workflows/docs-tests.yml index 0a16faa3..1b78e877 100644 --- a/.github/workflows/docs-tests.yml +++ b/.github/workflows/docs-tests.yml @@ -7,6 +7,11 @@ on: - 'docs/**' - 'diff_diff/**' - 'tests/test_doc_snippets.py' + # tests/conftest.py is auto-loaded by pytest for the snippet + # test run and mutates sys.path + MPLBACKEND (conftest.py:14, 18); + # changes there can break snippet exec without touching the test + # file itself. + - 'tests/conftest.py' - 'pyproject.toml' - '.github/workflows/docs-tests.yml' pull_request: @@ -16,6 +21,7 @@ on: - 'docs/**' - 'diff_diff/**' - 'tests/test_doc_snippets.py' + - 'tests/conftest.py' - 'pyproject.toml' - '.github/workflows/docs-tests.yml' schedule: