Skip to content

Notebook execution errors silently pass CI in three lecture repos — standardise -n -W on the sphinx-tojupyter build step #340

Description

@mmcky

Important

Revised twice — current as of 2026-07-30. The original audit was wrong, and so was the first correction. Two things had to change: the fix needs set -eo pipefail as well as the flags (field note in the comments), and the audit has to ask "is the first jb build gated?" rather than "is the sphinx-tojupyter step gated?" — because under execute_notebooks: "cache" only the first build executes anything, and which step that is varies per repo. The corrected table is below.

Summary

A broken notebook can pass CI green in five lecture repos. Two independent defects combine:

  1. Only the first jb build executes. execute_notebooks: "cache" means the first builder in the workflow executes the notebooks and populates the cache; every later builder reads it. So the gate has to be on the first jb build — and depending on whether the LaTeX step is active, that is either "Build PDF from LaTeX" or "Build Download Notebooks (sphinx-tojupyter)". Gating a later step catches only warnings specific to that builder.

  2. The step's exit code is not jb build's. These steps run three commands under shell: bash -l {0}. GitHub only injects -eo pipefail for the bare shell: bash shorthand; an explicit custom shell spec gets neither -e nor -o pipefail. So a failing jb build is followed by mkdir and the copy, and the step exits with the copy's status. --keep-going makes this worse rather than better: it forces Sphinx to emit output despite the errors, so the files exist, the copy succeeds, and the step reports green.

Both fixes are needed, on the right step. Flags alone gate nothing; set -eo pipefail alone gates nothing; and either applied to a step that reads from cache gates nothing that matters.

Single-command steps are unaffected — their exit code is already the command's.

Audit — verified 2026-07-30, on the "is the first jb build gated?" rule

Repo execute First jb build State
lecture-python.zh-cn cache tojupyter ✅ gated — QuantEcon/lecture-python.zh-cn#185
lecture-intro.zh-cn cache tojupyter ✅ gated
lecture-python-programming.zh-cn cache tojupyter ✅ gated
lecture-datascience.myst force Build HTML already gated — single-command step carrying -W; no change needed
lecture-dp cache (none — shared action) uses quantecon/actions/build-lectures@v0.8.0, which is immune by design
lecture-python-intro cache Build PDF from LaTeX ❌ needs -W + set -eo pipefail
lecture-python-advanced.myst cache Build PDF from LaTeX ❌ needs -W + set -eo pipefail
lecture-python.myst cache tojupyter ❌ has -W; needs set -eo pipefail
lecture-python-programming cache tojupyter ❌ has -W; needs set -eo pipefail
lecture-jax cache tojupyter ❌ has -W; needs set -eo pipefail
lecture-python-programming.fa n/a — the whole step is commented out

Five repos need changing, not six. Note the first jb build differs: where the LaTeX step is active it runs before the notebooks step, and in the zh-cn repos the LaTeX step is commented out, which is why gating tojupyter is correct there and wrong elsewhere.

Corrections to earlier versions of this audit, for the record: it cleared lecture-python.myst and lecture-python-programming (they are not gated — flags present but masked); it listed the zh-cn pair as the problem (they are now correct); it omitted lecture-jax and lecture-python-advanced.myst; and it wrongly flagged lecture-datascience.myst, which gates fine via its single-command HTML step under execute_notebooks: "force".

Evidence

Surfaced during the lecture-python.zh-cn resync wave (2026-07): a cold Build Cache run reported success while its execution-reports artifact held 19 notebook execution failures, and the resync only stayed safe because of a manual workaround — every fix was pushed as a new commit to force a cold rebuild, and the reports were read by hand.

The exit-code masking was verified empirically: bash -l -c 'false; echo hi' exits 0; with set -eo pipefail it exits 1.

Fix

Apply to the first jb build in each of the five repos: add set -eo pipefail as the first line of the run: block, and ensure the command carries -n -W --keep-going.

For the two where the first build is the LaTeX step (lecture-python-intro, lecture-python-advanced.myst), that means the "Build PDF from LaTeX" step. Reference implementation — QuantEcon/lecture-python-intro#809:

- name: Build PDF from LaTeX
  shell: bash -l {0}
  run: |
    set -eo pipefail
    jb build lectures --builder pdflatex --path-output ./ -n -W --keep-going
    mkdir -p _build/html/_pdf
    cp -u _build/latex/*.pdf _build/html/_pdf

For the three where the first build is the notebooks step (lecture-python.myst, lecture-python-programming, lecture-jax), the flags are already present, so it is a one-line set -eo pipefail insertion. The merged step in lecture-python.zh-cn is the template.

Two caveats. Any lecture with an intentionally-erroring cell needs a raises-exception tag first, or it will — correctly — start failing; worth a grep per repo. And only multi-command run: blocks using an explicit shell: are affected.

This is the tactical fix. The durable answer is migrating these pipelines to quantecon/actions/build-lectures, which captures and propagates the build exit code explicitly and stages files in separate steps — tracked in #348, and blocked on QuantEcon/actions#97 and #98.

Secondary note — cache.yml

The weekly Build Cache job builds with the default HTML builder under execute_notebooks: "cache", which does not catch execution errors even with -W present (the HTML builder reuses the cache rather than re-executing — this is what let the 19 failures above pass). Aligning ci.yml fixes the PR gate, which is the priority; if the cache job should also gate on execution, that is a separate change (run the tojupyter execution builder in cache.yml, or set a raise-on-error config).

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions