CI: gate the first jb build so notebook errors cannot pass green - #367
Merged
Conversation
"Build PDF from LaTeX" is the first `jb build` in this workflow, and
`execute_notebooks: "cache"` means only the first build executes notebooks
— the later tojupyter and HTML builds read the cache. So this step is
where a CellExecutionError surfaces, and it could not fail.
Two defects, both fixed here:
- it ran with `-n --keep-going` and no `-W`, so a CellExecutionError was a
non-fatal warning
- its exit code was `cp`'s, not `jb build`'s. The step runs three commands
under `shell: bash -l {0}`, and GitHub only injects `-eo pipefail` for the
bare `shell: bash` shorthand; an explicit custom shell spec gets neither
`-e` nor `-o pipefail`. `--keep-going` compounds it by guaranteeing the
PDF exists for `cp` to succeed on
Checked before making the change: this repo has no `raises-exception` tags,
and all 64 published lecture pages render without execution-error output.
Refs QuantEcon/meta#340
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for lustrous-melomakarona-3ee73e ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens CI by ensuring the first jb build (the one that actually executes notebooks due to execute_notebooks: "cache") fails the workflow on notebook execution errors and on Sphinx warnings.
Changes:
- Add
set -eo pipefailto ensure the step fails onjb buildnon-zero exit (not masked by subsequent commands). - Add
-Wto treat Sphinx warnings as errors for the initial PDF/LaTeX build. - Document the rationale directly in the workflow for maintainability.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+62
to
+63
| set -eo pipefail | ||
| jb build lectures --builder pdflatex --path-output ./ -n -W --keep-going |
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of QuantEcon/meta#340.
The problem
Build PDF from LaTeXis the firstjb buildin this workflow, andlectures/_config.ymlsetsexecute_notebooks: "cache"— so it is the only build that actually executes notebooks; the later tojupyter and HTML builds read the cache. That makes it the step where aCellExecutionErrorsurfaces.It could not fail, for two independent reasons:
-W. It ran with-n --keep-going, so aCellExecutionErrorwas a non-fatal warning.cp's, notjb build's. The step runs three commands undershell: bash -l {0}, and GitHub only injects-eo pipefailfor the bareshell: bashshorthand — an explicit custom shell spec gets neither-enor-o pipefail. So a failingjb buildwas followed bymkdirandcp, and the step exited withcp's status.--keep-goingmakes this worse rather than better, because it forces Sphinx to emit output despite the errors, guaranteeing the PDF exists forcpto succeed on.Either fix alone is insufficient. Both are applied, to one step.
Note on which step
The rule is "gate the first
jb build", not "gate a particular step name". Here the LaTeX step runs first; inlecture-python.myst,lecture-python-programmingandlecture-jaxthe notebooks step does, so those get a one-lineset -eo pipefailinstead. The audit table in QuantEcon/meta#340 records which step applies per repo.Verified before the change
raises-exceptiontags anywhere inlectures/— nothing is relying on an error being tolerated.output_error, tracebacks, ANSI red). Zero hits.Since this step has never been able to fail, CI on this PR is the first real exercise of it.
-Walso promotes Sphinx warnings to errors, and the LaTeX builder emits warnings the HTML builder does not. If CI goes red it has found something genuine that was previously silent, and that should be fixed rather than the flags softened.Refs QuantEcon/meta#340
🤖 Generated with Claude Code