Skip to content

Enforce single H1 in docs notebooks (#863) + add pr-review skill#864

Closed
drbenvincent wants to merge 1 commit intomainfrom
issue-863-single-h1-check
Closed

Enforce single H1 in docs notebooks (#863) + add pr-review skill#864
drbenvincent wants to merge 1 commit intomainfrom
issue-863-single-h1-check

Conversation

@drbenvincent
Copy link
Copy Markdown
Collaborator

Summary

Two related-but-distinct changes bundled in one PR.

1. Single-H1 hook for docs notebooks (closes #863)

Extends the existing validate-notebooks pre-commit hook (scripts/validate_notebooks.py) with a single-H1 check, scoped to docs/source/notebooks/ so dev/scratch notebooks elsewhere are unaffected.

The check is robust against the two false-positive sources that were called out in #863:

  • Code cells are skipped entirely — Python comments cannot trigger a violation.
  • Within markdown cells, fenced code blocks (``` and ~~~) are skipped — Python comments embedded in markdown code samples (the its_lift_test.ipynb pattern) cannot trigger a violation.

Failure messages name each offending cell index and heading text, plus a short remediation hint.

Test coverage in causalpy/tests/test_notebook_validation.py:

  • Single-H1 valid case.
  • Multi-H1 / no-H1 violations (three parametrised variants).
  • Code-cell comment false-positive guard.
  • Fenced-code-block (```python and ~~~python) false-positive guards.
  • Out-of-scope exemption (notebooks outside docs/source/notebooks/ ignored).
  • Regression test that walks every shipped docs notebook and asserts the check passes — currently green now that Add outcome falsification, random placebo folds, and model clone support #826's notebook fix has landed (see audit below).

2. New pr-review agent skill

.github/skills/pr-review/ — orchestrator SKILL.md plus six focused reference files (workflow.md, what-to-look-for.md, code-patterns.md, docs-patterns.md, posting-comments.md, how-to-extend.md).

Captures the review patterns surfaced during the #826 review iteration into a reusable skill: how to fetch PR context, severity-sorted patterns to look for, CausalPy-specific code conventions (BaseExperiment, _clone(), Literal for constrained strings, helper-promotion checklist), notebook/docs conventions, comment-drafting templates, and a built-in extension/pruning protocol so the skill stays sharp as patterns get formalised into hooks.

Designed to complement (not duplicate) the existing pr-to-green and pr-workflows skills; explicitly defers mechanical checks to hooks/CI.

Why these are bundled

The branch was originally scoped to #863, but the skill captures (among other things) "single-H1 enforcement should be a hook, not an agent task" as an explicit pattern. Bundling them keeps the skill's canonical examples accurate from day one. Happy to split into two PRs if a maintainer prefers — they're cleanly separable (no shared files, no shared test surface).

Audit — current docs notebook corpus

Scanned 32 notebooks
Violations: 0

(Was 1 prior to #826 landing; the H1 fix in that PR cleared the only outstanding violation.)

Test plan

  • prek run --all-files passes locally.
  • $CONDA_EXE run -n CausalPy python -m pytest causalpy/tests/test_notebook_validation.py -v passes.
  • CI prek job picks up the new check via the existing validate-notebooks hook.
  • Manual smoke: introducing a deliberate second # heading in any docs/source/notebooks/*.ipynb causes the hook to fail with the expected message.

Out of scope

  • Pre-commit / CI check for orphan notebooks (notebooks not referenced from any toctree). Discussed in chat, not yet filed as an issue.
  • Setting -W on the docs build so Sphinx orphan warnings become errors. Tracked separately from this PR.

Related

Made with Cursor

…skill

Closes #863 by extending scripts/validate_notebooks.py with a single-H1
check scoped to docs/source/notebooks/. The check ignores code cells and
fenced code blocks inside markdown cells, so Python comments cannot
trigger false positives. Tests cover valid notebooks, multi-H1 / no-H1
violations, code-cell comments, fenced-code-block comments, the
out-of-scope exemption, and a regression test that walks every shipped
docs notebook.

Also adds a new .github/skills/pr-review/ skill — orchestrator SKILL.md
plus six focused reference files — capturing the review patterns
distilled from the PR #826 review iteration: workflow, severity-sorted
look-fors, CausalPy code conventions, docs/notebook conventions,
comment-drafting templates, and an extension/pruning protocol.

Made-with: Cursor
@github-actions github-actions Bot added devops DevOps related documentation Improvements or additions to documentation labels Apr 24, 2026
@drbenvincent
Copy link
Copy Markdown
Collaborator Author

Closing in favour of a properly-scoped PR — this branch and PR conflated the #863 hook with the unrelated pr-review skill. Skill-only PR is being opened from add-pr-review-skill; the H1 hook will follow as a separate PR from issue-863-single-h1-check.

@drbenvincent drbenvincent mentioned this pull request Apr 24, 2026
3 tasks
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

❌ Patch coverage is 92.72727% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.60%. Comparing base (deb8774) to head (d14bfc5).

Files with missing lines Patch % Lines
causalpy/tests/test_notebook_validation.py 92.72% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #864      +/-   ##
==========================================
- Coverage   94.60%   94.60%   -0.01%     
==========================================
  Files          80       80              
  Lines       12764    12816      +52     
  Branches      770      772       +2     
==========================================
+ Hits        12076    12124      +48     
- Misses        485      487       +2     
- Partials      203      205       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devops DevOps related documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enforce single top-level markdown heading per docs notebook via pre-commit hook

1 participant