Add label-gated CI and documentation dependency map

[igerber]

Summary

• Add ready-for-ci label gate to rust-test.yml and notebooks.yml so CI tests only run when the PR is ready, eliminating wasted intermediate runs during AI review iterations
• Create docs/doc-deps.yaml mapping all 47 source files to their dependent documentation, with a new /docs-impact skill and integration into /pre-merge-check, /submit-pr, and /push-pr-update

Methodology references (required if estimator / math changes)

• Method name(s): N/A - no methodology changes
• Paper / source link(s): N/A
• Any intentional deviations from the source (and why): N/A

Validation

• Tests added/updated: No test changes (workflow and tooling only)
• Backtest / simulation / notebook evidence (if applicable): N/A

Security / privacy

• Confirm no secrets/PII in this PR: Yes

---

Generated with Claude Code

[github-actions]

Overall Assessment

⚠️ Needs changes

Executive Summary

• No estimator, weighting, variance, identification-assumption, or default-behavior code changed in this PR, so I did not find a direct source-methodology mismatch in the library’s numerical implementation.
• P1 The new ready-for-ci gate is implemented with job-level if: conditions in the GitHub Actions jobs, which means the Rust/notebook checks can be marked as skipped/successful instead of actually running. If those checks are required, this can allow merge without executing CI. (docs.github.com)
• P1 The new docs-impact integration weakens the existing REGISTRY.md safeguard: the old explicit warning for methodology-file changes was replaced with a high-drift-only warning, but many methodology-sensitive estimators are marked medium in the new map.
• P3 docs/doc-deps.yaml is internally valid today, but it does not include ROADMAP.md despite the new contributor guidance and the map header claiming roadmap coverage.
• P3 The new dependency map still has no automated validation in repo CI; drift will only be caught if someone manually runs the docs-check flow.

Methodology

No estimator math changed in this PR. The methodology issue here is a guardrail regression, not a numerical implementation bug.

• P1 In .claude/commands/pre-merge-check.md (lines 112-131), .claude/commands/submit-pr.md (lines 157-169), and .claude/commands/push-pr-update.md (lines 131-140 and 172-180), the old explicit “methodology files changed but REGISTRY.md was not updated” warning was replaced by a warning that only surfaces HIGH drift-risk dependencies. But many methodology-bearing estimators are medium in docs/doc-deps.yaml, docs/doc-deps.yaml, docs/doc-deps.yaml, docs/doc-deps.yaml, docs/doc-deps.yaml, docs/doc-deps.yaml, docs/doc-deps.yaml, and docs/doc-deps.yaml. Impact: changes to TwoStageDiD, EfficientDiD, ContinuousDiD, SyntheticDiD, TripleDifference, StackedDiD, TROP, HonestDiD, etc. will no longer trigger a pre-commit/pre-PR warning to update docs/methodology/REGISTRY.md, even though undocumented methodology deviations are treated as P1 by project policy. Concrete fix: keep a dedicated warning for any changed source with a type: methodology dependency, regardless of drift_risk, or retain the old explicit REGISTRY.md check alongside the new map.

Code Quality

• P1 The ready-for-ci enforcement in .github/workflows/rust-test.yml, .github/workflows/rust-test.yml, .github/workflows/rust-test.yml, and .github/workflows/notebooks.yml, combined with the new user instructions in .claude/commands/submit-pr.md and .claude/commands/push-pr-update.md, is advisory rather than enforcing. GitHub documents that conditionally skipped jobs report success, and required checks accept successful, skipped, or neutral conclusions. Impact: a PR opened without ready-for-ci can satisfy required Rust/notebook checks without ever running them. Concrete fix: add a small always-running gate job that fails while the label is absent, and make the expensive jobs depend on that gate; do not rely on job-level conditional skips to enforce required checks. (docs.github.com)

Performance

• P3 In .github/workflows/rust-test.yml and .github/workflows/notebooks.yml, the workflows subscribe to all labeled and unlabeled events, but the job condition only checks whether the current label set contains ready-for-ci. Impact: once a PR already has ready-for-ci, adding or removing any unrelated label can rerun the full Rust/notebook matrices. Concrete fix: narrow label-triggered execution to transitions involving github.event.label.name == 'ready-for-ci', and consider dropping unlabeled if label removal does not need to trigger work.

Maintainability

• P3 CONTRIBUTING.md now says major features should update ROADMAP.md, and docs/doc-deps.yaml says roadmap covers ROADMAP.md, survey-roadmap.md, but the map only contains docs/survey-roadmap.md entries at docs/doc-deps.yaml, docs/doc-deps.yaml, and docs/doc-deps.yaml. Impact: /docs-impact and the new pre-merge warnings will never surface ROADMAP.md as impacted, so the dependency map does not actually cover all roadmap docs it claims to cover. Concrete fix: add ROADMAP.md dependencies where appropriate, or narrow the documented claim if ROADMAP.md is intentionally out of scope.

Tech Debt

No mitigating entry for the P1 items exists in TODO.md, and this PR does not add one, so those findings remain unmitigated and count toward the assessment.

Security

No findings.

Documentation/Tests

• P3 The new manual validation flow in .claude/commands/docs-check.md is useful, but there is still no automated test or CI step that parses docs/doc-deps.yaml, verifies referenced docs exist, and verifies full Python-source coverage. Impact: future stale paths or unmapped modules can drift silently until someone remembers to run /docs-check map. Concrete fix: add a lightweight test or CI check that performs the same validation described in the new command. Current map state is clean: I validated it locally and found no stale doc paths and no unmapped diff_diff/*.py or diff_diff/visualization/*.py files.

Path to Approval

1. Replace the job-level ready-for-ci skip logic with an enforcement mechanism that actually blocks merge until CI is enabled, such as an always-running required gate job plus needs-gated expensive jobs.
2. Restore REGISTRY.md warning coverage in /pre-merge-check, /submit-pr, and /push-pr-update for any source file with a type: methodology dependency, regardless of drift_risk.

[igerber]

/ai-review

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: d4d9f659e6f520e728f4ad143d46d5908f8225c6

---

Overall Assessment

⚠️ Needs changes

Executive Summary

• No estimator, weighting, variance, identification, or default-behavior code changed directly in this PR; this re-review is about whether the new CI/doc-tooling preserves the project’s existing methodology-review safeguards.
• The previous methodology-warning regression is partially fixed: the updated command docs now say to always warn on all type: methodology dependencies, not just high drift items.
• P1 The new dependency map misses methodology links for shared inference/variance helpers, so future SE/bootstrap/inference changes in diff_diff/linalg.py:L450-L458, diff_diff/utils.py:L460-L467, and diff_diff/bootstrap_utils.py:L480-L485 will bypass the new REGISTRY.md warning flow.
• P1 [Previous finding not fully resolved] The new ci-gate job still does not make the label gate self-enforcing in workflow code alone; downstream jobs are skipped through needs, and GitHub documents that skipped jobs can satisfy required checks and that jobs depending on failed/skipped jobs are themselves skipped unless they use a continuing condition. citeturn1view0turn2view0
• The new map itself is syntactically valid today and, as checked locally, currently covers all diff_diff/*.py plus diff_diff/visualization/*.py files with no stale doc paths.
• The lower-severity follow-ups called out in the prior review are now properly tracked in TODO.md:L81-L97, so they are informational only.

Methodology

• Severity P1. The new warning flow in pre-merge-check.md:L112-L134, submit-pr.md:L157-L171, and push-pr-update.md:L131-L142 now depends on docs/doc-deps.yaml having type: methodology entries. But the map classifies diff_diff/linalg.py:L450-L458, diff_diff/utils.py:L460-L467, and diff_diff/bootstrap_utils.py:L480-L485 as performance/internal/roadmap only, even though those files implement cluster-robust/survey variance, safe_inference() NaN gating, and survey-aware/bootstrap inference behavior documented in diff_diff/linalg.py:L437-L513, diff_diff/utils.py:L68-L186, diff_diff/bootstrap_utils.py:L1-L229, docs/methodology/REGISTRY.md:L1275-L1283, and docs/methodology/REGISTRY.md:L2193-L2230. Impact: future changes to shared SE/bootstrap/inference code can slip past the mandatory methodology-doc warning path even though this project treats undocumented variance/SE deviations as P1. Concrete fix: add type: methodology dependencies for these shared inference modules, with appropriate REGISTRY.md section hints.

Code Quality

• Severity P1. [Previous finding not fully resolved] The new gate in rust-test.yml:L25-L45 and notebooks.yml:L23-L40 makes the expensive jobs depend on ci-gate, but it does not by itself guarantee that merge is blocked when the label is absent. GitHub documents that jobs whose dependencies fail or are skipped are themselves skipped unless they use a continuing condition, and that skipped/neutral checks can satisfy required-status rules. Impact: unless repo settings are separately updated to require the new CI Gate check, the existing Rust/notebook checks can still end up skipped rather than executed, so the stated label gate remains bypassable from the workflow code alone. Concrete fix: either make CI Gate the required check and document that rollout explicitly, or change the currently required Rust/notebook jobs so they fail fast when ready-for-ci is absent instead of being skipped via needs alone. citeturn1view0turn2view0

Performance

• Severity P3 (informational, tracked in TODO.md). rust-test.yml:L12-L20 and notebooks.yml:L11-L18 still subscribe to all labeled/unlabeled PR events, so unrelated label changes can retrigger CI once ready-for-ci is already present. Impact: wasted workflow runs, but no correctness risk. Concrete fix: narrow the label-trigger logic to ready-for-ci transitions only. This is already tracked in TODO.md:L81-L87.

Maintainability

No findings.

Tech Debt

• Severity P3 (informational). The PR correctly adds deferred-work tracking in TODO.md:L81-L97 for the non-blocking follow-ups around unrelated label retriggers and automated dependency-map validation. Impact: these items are mitigated under the project’s deferred-work policy and do not affect the assessment. Concrete fix: none required before merge beyond keeping the TODO entries current.

Security

No findings.

Documentation/Tests

• Severity P3 (informational, tracked in TODO.md). The new manual validation flow in docs-check.md:L201-L231 is useful, but there is still no automated CI/test that parses docs/doc-deps.yaml, verifies referenced files exist, and verifies full source coverage. Impact: future map drift can still land unless someone remembers to run the manual command. Concrete fix: add a lightweight automated check mirroring the map validation described there. This is already tracked in TODO.md:L89-L97.

Path to Approval

1. Add type: methodology dependencies in docs/doc-deps.yaml for the shared inference/variance helpers that implement registry-documented behavior, at minimum diff_diff/linalg.py, diff_diff/utils.py, and diff_diff/bootstrap_utils.py.
2. Make the ready-for-ci gate self-enforcing in the workflow rollout: either land the corresponding branch-protection change that requires CI Gate, or change the currently required Rust/notebook jobs so they fail when the label is absent instead of being skipped via needs.

[igerber]

/ai-review

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: f7157f4d6e0c7d7b0322f9a543397a0529110a4c

---

Overall Assessment

⚠️ Needs changes

Executive Summary

• No estimator, weighting, variance, identification, or default-behavior code changed directly in this PR.
• The previous methodology-map gap appears resolved. The new dependency map now marks the shared inference and variance helpers as methodology-sensitive in docs/doc-deps.yaml:L450-L491, which matches the registry coverage for NaN-safe inference and survey-aware variance/bootstrap in docs/methodology/REGISTRY.md:L1275-L1283 and docs/methodology/REGISTRY.md:L2193-L2235.
• I validated docs/doc-deps.yaml locally: it parses, has no stale doc paths, and covers all Python files under diff_diff/ plus diff_diff/visualization/.
• P1: the new ci-gate job reduces wasted CI runs, but the merge-block guarantee is still not self-enforcing from the repo diff alone. The expensive jobs are skipped behind needs: [ci-gate] in .github/workflows/rust-test.yml:L26-L44 and .github/workflows/notebooks.yml:L24-L40, while CLAUDE.md:L148-L150 now states that a required CI Gate check already enforces merge blocking.
• The remaining non-blocking follow-ups are properly tracked in TODO.md:L81-L97.

Methodology

No findings. The only methodology-sensitive part of this PR is the new documentation dependency map and the surrounding warning flow. That map now covers the previously missing shared helpers in docs/doc-deps.yaml:L450-L491, and those entries line up with the actual implementations in diff_diff/utils.py:L152-L186, diff_diff/linalg.py:L437-L513, and diff_diff/bootstrap_utils.py:L424-L457. The command docs are also now consistent about always surfacing type: methodology dependencies in .claude/commands/docs-impact.md:L82-L86, .claude/commands/pre-merge-check.md:L120-L134, .claude/commands/submit-pr.md:L157-L171, and .claude/commands/push-pr-update.md:L131-L142.

Code Quality

• Severity P1. Impact: the workflow change does stop the expensive jobs from starting before the label is added, but it still does not make the documented "PRs cannot merge until the label is added" contract enforceable from versioned repo code alone. rust-tests, python-tests, python-fallback, and execute-notebooks all sit behind needs: [ci-gate] in .github/workflows/rust-test.yml:L42-L45, .github/workflows/rust-test.yml:L76-L79, .github/workflows/rust-test.yml:L177-L180, and .github/workflows/notebooks.yml:L38-L40. That means the rollout still depends on an out-of-band required-status configuration that is not represented in this diff, even though CLAUDE.md:L148-L150 says it is already enforced. Concrete fix: either make the currently required jobs fail fast themselves when ready-for-ci is absent, or land the required-status rollout in a reviewable, versioned place before claiming the gate is enforced.

Performance

• Severity P3 (informational, tracked in TODO.md). Impact: both workflows still subscribe to all labeled and unlabeled PR events, so unrelated label churn can retrigger CI after ready-for-ci is already present in .github/workflows/rust-test.yml:L12-L20 and .github/workflows/notebooks.yml:L11-L18. Concrete fix: none required before merge; already tracked in TODO.md:L81-L87.

Maintainability

No findings.

Tech Debt

No additional findings. The PR correctly records its deferrable follow-ups in TODO.md:L81-L97, so those items are mitigated under the project's deferred-work policy.

Security

No findings.

Documentation/Tests

• Severity P3 (informational, tracked in TODO.md). Impact: the PR adds manual dependency-map validation instructions in .claude/commands/docs-check.md:L203-L232, but there is still no automated CI/test that parses docs/doc-deps.yaml and enforces stale-path and full-coverage checks. Concrete fix: none required before merge; already tracked in TODO.md:L89-L97.

Path to Approval

1. In .github/workflows/rust-test.yml:L26-L44 and .github/workflows/notebooks.yml:L24-L40, make the currently required test jobs fail when ready-for-ci is absent instead of being skipped behind needs: [ci-gate].
2. Update CLAUDE.md:L148-L150 so it does not claim the required CI Gate rollout is already enforced unless that repository-level configuration is reviewable and confirmed as part of the rollout.