Skip to content

Gate the docs build: fail when the docutils error count rises #571

Description

@breimanntools

Problem

Nothing in the pipeline fails when a documentation page stops rendering. Three broken references reached master through this gap and were found only because a build was run by hand.

Measured on origin/master (64a0cdef) and at a pre-campaign commit (3e0cd75d), same Sphinx, same environment:

errors warnings
3e0cd75d (before) 24 329
64a0cdef (now) 27 338

Three reasons this passed every existing check:

  1. No workflow builds the docs. grep -rln "sphinx\|make html" .github/workflows/ returns nothing.
  2. Read the Docs does not gate on it. .readthedocs.yaml sets no fail_on_warning, so the docs/readthedocs.org:aaanalysis check reports success with all 27 errors present.
  3. Sphinx exits 0 anyway. The build returns exit code 0 with 27 ERROR lines in its log, so even adding a plain make html step would not catch this. The error count has to be parsed out of the log.

The docstring checker does not overlap with this: check_docstrings.py reports 0 defects on exactly the tree that produces 27 Sphinx errors. It validates numpydoc structure, not RST rendering. Both are needed; neither substitutes for the other.

Correction to this issue's original diagnosis

This issue first blamed broken docstrings, naming aaanalysis/prediction/_reliability_model.py. That was wrong, and anyone working this issue should not go hunting docstrings there — every occurrence in that file was already correct and the fix touched it not at all.

The real cause of 22 of the 27 errors is a single bug, found while fixing them: nbconvert renders a code cell's stream output as .. parsed-literal::, and docutils interprets inline markup inside that block. Raw program output is not RST, so a CPP progress bar (|.........|) was read as a substitution reference and a printed fitted attribute (ad_threshold_ = 5.568) as a link target. One cause, two error classes, 22 errors, all originating in committed notebook output rather than in any docstring.

The remaining five were genuine source-level markup defects, including one in aaanalysis/_schemas.py that surfaced through a generated page, so the generated file was the wrong place to fix it.

Goal

A build-health gate that fails when the docutils error count rises, so a broken reference cannot reach master silently.

Requirements

  • A script under .github/scripts/ that builds the docs, parses the log, and exits non-zero on docutils ERROR lines. It must not rely on Sphinx's exit code.
  • A plain zero-error gate, not a ratchet. This issue originally specified a ratchet against a committed baseline of 24, because blocking on zero would have blocked every pull request. That is no longer true: the errors are now cleared, so the gate can simply require zero. (If the count is not zero when this is implemented, fall back to the ratchet pattern of pyright_baseline.txt: lower when cleared, never raise.)
  • The gate must run on both code and docs changes. Errors originate in .py docstrings and in notebook output, so a workflow scoped only to docs/** would miss half, while the existing code-gated workflows carry paths-ignore: ['docs/**'] and would miss the other half.
  • Decide whether warnings get a second, separate ratchet. 334 of the ~337 are one pre-existing autosummary pattern, so a warning ratchet is mostly noise until that pattern is addressed. Recommend errors-only first.
  • Consider also gating the 167 pre-existing CRITICAL: Unexpected section title lines, which are out of scope here but are the next-largest untracked signal.

KPIs / Acceptance criteria

  • A pull request that introduces a docutils ERROR fails a required check, demonstrated by deliberately adding one.
  • No existing green pull request turns red from adopting the gate.

Scope / non-goals

  • CI tooling, no new dependency: Sphinx already builds in the docs extra.
  • Not the 334 autosummary warnings, which are a pre-existing pattern.
  • Not a change to Read the Docs project settings.

Note on implementation

.github/workflows/* is a confirm-first surface, so the workflow wiring needs explicit maintainer approval before it is written.

Standards checklist

  • Script lives in .github/scripts/ (not dev_scripts/, which is gitignored)
  • If a baseline is used at all, it is read from CI, never from a local build
  • No print() in library code; this is tooling, not library

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    prio:2Importanttype:docsImprovements or additions to documentation

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions