CI: align build safety with the English source repo (closes #184)#185
Merged
Conversation
Resolves the four failures reported in #182. All four are target-side defects; the English source is healthy in every case. likelihood_bayes — the font block called `mpl.font_manager.fontManager` and `plt.rcParams` before `import matplotlib.pyplot as plt`. Newer matplotlib no longer imports `font_manager` as a side effect of `import matplotlib`, so this raised AttributeError (and would have raised NameError on `plt`). Moved the pyplot import above both uses. An audit of the font block across all 69 lectures found this the only file with either ordering hazard. kalman_2 — a `.text()` annotation drew the covariance matrix with `\begin{bmatrix}`, which matplotlib mathtext cannot render (it supports no LaTeX environments). Rewritten as `\left[ \substack{...} \right]`, which renders an equivalent bracketed 2x2 matrix under mathtext, using `\;` as the column separator rather than `&`. This keeps the drop-usetex standard from #22 intact with no loss of information. The lecture's other `\begin{bmatrix}` uses are inside `{math}` blocks, rendered by MathJax, and are unaffected. sir_model — the header directive had lost its argument: `{raw}` instead of `{raw} jupyter`. The other 50 lectures carrying this directive kept theirs. ak_aiyagari — the file carried a stray fence at line 38 followed by a duplicated copy of the original frontmatter, and a doubled closing fence at EOF. The stray opener ran to line 56, swallowing both the H1 title and the `_admonition/gpu.md` include, which is what produced the 13 "headings start at H2" warnings. Removed both strays; the file's fence structure now matches the English source exactly (122). The `translation.title` field was also corrupt ("V, σ, μ") and has been set to the lecture title. An edition-wide sweep found no other file with unbalanced fences. The last two are content corruption introduced in translation and warrant engine issues against action-translation; they are not covered by the existing #107/#115/#116/#117 set. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolves #184. Brings ci.yml's execution safety in line with lecture-python.myst, which is the standard the fleet builds to. - `-n -W --keep-going` on the sphinx-tojupyter build. Previously this step ran with no flags, so a notebook whose cells raised still exited zero and the job went green. That is how the two execution breaks in #182 (likelihood_bayes, kalman_2) reached main with passing CI. - Upload Execution Reports (Download Notebooks), so a failure in that step leaves a traceback artifact. Previously there was none, which is why #182 had to be diagnosed from the weekly Build Cache workflow's artifact rather than from CI directly. - Clear stale Sphinx environment (`rm -rf _build/.doctrees`) before the builds, matching the source repo. The equivalent was present here only as a commented-out note. The HTML artifact is renamed `execution-reports` -> `execution-reports-html`. This is required, not cosmetic: upload-artifact@v7 runs with `overwrite: false`, so adding a second upload under the same name would collide when both steps fail in one run. The new names match the source repo's convention (-notebooks / -latex / -html). The PDF/LaTeX build is left commented out — see the issue for why that one needs a human decision rather than mechanical alignment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Aligns this repo’s CI workflow with the upstream English source repository so notebook execution/build failures are surfaced reliably and produce actionable artifacts (as requested in #184).
Changes:
- Clear Sphinx doctree cache before builds (
rm -rf _build/.doctrees). - Run the sphinx-tojupyter build with
-n -W --keep-goingto fail on warnings/execution issues. - Upload execution report artifacts on failure, and rename the HTML report artifact to avoid name collisions.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses Copilot review feedback on #185. `shell: bash -l {0}` is a custom shell spec, so GitHub does not inject `-eo pipefail` — it only does that for the bare `shell: bash` shorthand, which expands to `bash --noprofile --norc -eo pipefail {0}`. Without it, this step's exit code is that of the trailing `cp`, so a failing `jb build` was masked. `--keep-going` made that worse rather than better: it forces Sphinx to emit output despite the errors, so the .ipynb files exist, `cp` succeeds, and the step goes green. The flag added in this PR to surface failures was producing the artifacts that hid them. Scope is exactly one step — an audit of ci.yml shows every other step is either single-command (its exit code is the command's) or uses the default shell, which does carry `-e`. Note the English source repo lecture-python.myst has the byte-identical pattern, so this is inherited from the standard rather than introduced here, and it deviates from the strict parity this PR set out to establish. Reported upstream so the standard can catch up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for astonishing-narwhal-a8fc64 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This was referenced Jul 20, 2026
mmcky
added a commit
to QuantEcon/lecture-intro.zh-cn
that referenced
this pull request
Jul 20, 2026
…#270) Ports QuantEcon/lecture-python.zh-cn#185 to this edition, closing the QuantEcon/meta#340 gap here. ci.yml - `-n -W --keep-going` on the sphinx-tojupyter build. It previously ran with no flags, so a notebook whose cells raised still exited zero. - `set -eo pipefail` in that same block. The flag alone is insufficient: `shell: bash -l {0}` is a custom shell spec, so GitHub injects no `-eo pipefail`, and the trailing mkdir/cp become the step's exit code. `--keep-going` makes it worse, guaranteeing the .ipynb files exist for `cp` to succeed on. Found by Copilot review on zh-cn#185. - Upload Execution Reports (Download Notebooks), so a failure in that step leaves a traceback artifact. - Clear stale Sphinx environment before the builds. - HTML artifact renamed execution-reports -> execution-reports-html. Required, not cosmetic: upload-artifact runs with `overwrite: false`, so a second upload under the same name would collide when both steps fail in one run. cache.yml - `-W --keep-going` -> `-n -W --keep-going`, so the weekly cold build is strict on missing references as well as warnings, matching lecture-python.zh-cn. ci-cn.yml is deliberately unchanged: it has no sphinx-tojupyter step and its HTML build is a single command, so its exit code is already the build's. That is why it — and not ci.yml — caught the prob_dist break in #269. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Resolves #184. Split out of #183 so that PR stayed scoped to content fixes.
Stacked on #183. The base is
fix/182-execution-and-doc-failures, notmain, so the diff here is justci.ymland the CI run reflects reality — turning on-n -Wagainst a main that still contains the four #182 defects would only re-report bugs #183 already fixes. GitHub will retarget this tomainautomatically when #183 merges.What changes
-n -W --keep-goingif: failure()-n -W --keep-goingThe sphinx-tojupyter step is the substantive fix. It ran with no flags, so a notebook whose cells raised still exited zero and the job went green — that is how the two execution breaks in #182 (
likelihood_bayes,kalman_2) reached main with passing CI.The missing artifact upload compounded it: when that step did fail there was no traceback to read, which is why #182 had to be diagnosed from the weekly Build Cache workflow's artifact instead of from CI.
One non-obvious change
The HTML artifact is renamed
execution-reports→execution-reports-html. This is required, not cosmetic.upload-artifact@v7runs withoverwrite: false, so adding a second upload under the existingexecution-reportsname would collide whenever both steps fail in the same run. The new names follow the source repo's convention (-notebooks/-latex/-html).Expect this to go red, and that is the point
Turning on
-n -Wsurfaces every remaining latent failure at once, and there are very likely more than the four in #182. The Build Cache run that produced #182 is content-addressed: it re-executed only the lectures the resync wave changed, and reused old-environment cache for roughly 15 unchanged lectures that have never executed under anaconda 2026.07. Whatever this run reports is the real backlog.The useful reading: a red run here is new information, not a regression. Work the list, then merge.
Verification
lecture-python.mystconfirmed fromDownload "build" folder (cache)throughUpload Execution Reports (Download Notebooks). Remaining divergence is the PDF/LaTeX pair and the source repo's GPU/JAX setup steps, neither of which applies here.jb buildinvocations now carry-n -W --keep-going.Left deliberately undone
The PDF/LaTeX build stays commented out. CJK under LaTeX is hard, and more so since #22 dropped
text.usetex, so this looks like a considered choice rather than an oversight. It needs a human decision, not mechanical alignment — flagged in #184.Related: QuantEcon/meta#340 tracks the same gap across the fleet (
lecture-intro.zh-cnand thelecture-python-introsource repo have it too;lecture-python-programmingis already aligned).Closes #184
🤖 Generated with Claude Code