Repository navigation
Conversation
The "Upload Execution Reports" step only looked in _build/html/reports. Since #588 made the tojupyter build the gating step, a CellExecutionError stops the job in that build, which writes its reports to _build/jupyter/reports, so the execution-reports artifact came up empty. Upload the reports directories of all three builds. Mirrors QuantEcon/lecture-python-advanced.myst#410. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Note
Copilot was unable to complete this review at the selected tier due to model policy or plan restrictions.
🟢 Approval recommended
The change is a small, well-scoped CI workflow adjustment with clear documentation that broadens failure-only artifact collection across known Jupyter Book output paths, and the review evidence reports no outstanding issues.
No open issues 🎉
What changed in this PR
Updates CI artifact uploading so notebook execution tracebacks are preserved regardless of which jb build step runs notebooks first, preventing empty execution-reports artifacts when failures occur early.
Changes:
- Upload execution reports from multiple build output directories (
_build/jupyter/reports,_build/latex/reports,_build/html/reports) on failure. - Add an inline comment documenting why reports may be produced under different
_build/*/reportspaths.
| File | Description |
|---|---|
| .github/workflows/ci.yml | Adjusts the failure-only artifact upload to collect execution reports from all Jupyter Book build output roots so failing notebook tracebacks are captured consistently. |
Review effort: Balanced
This branch has not been deployed
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.
When a notebook fails to execute in CI, the
execution-reportsartifact that should hold its traceback comes up empty. This mirrors QuantEcon/lecture-python-advanced.myst#410, which found the same mismatch there.Cause
Notebooks execute in whichever
jb buildruns them first, and each build writes its execution reports under its own output directory. Since #588, the tojupyter build ("Build Download Notebooks") is the first build and the gating step. So aCellExecutionErrorstops the job there, and its traceback is written only to_build/jupyter/reports. The upload step looks in_build/html/reports, which the HTML build never reaches to create.This workflow doesn't upload
_buildwhen a build fails, so in practice the traceback of a failing notebook is lost, and the only way to see it is to reproduce the failure locally.I confirmed where the reports land with a throwaway two-page book whose only notebook raises. I built it with this workflow's tojupyter command (
jb build lectures --path-output ./ --builder=custom --custom-builder=jupyter -n -W --keep-going), using jupyter-book 1.0.4.post1 and myst-nb 1.4.0. The traceback went to_build/jupyter/reports/broken.err.log, and no_build/html/reportsdirectory was created.Change
The upload step now lists the reports directories of all three builds:
_build/jupyter/reports,_build/latex/reportsand_build/html/reports. Whichever build fails, its report is captured.upload-artifactroots a multi-path upload at the common ancestor (_build), so the artifact keeps the sets apart asjupyter/reports/…and so on. I also added a comment explaining the paths. This matches the change in QuantEcon/lecture-python-advanced.myst#410.Testing
This PR's own CI run won't exercise the step, because it runs only
if: failure(). I checked that the workflow parses and thatpathis a valid multi-line list. The first failing notebook on any PR will confirm the upload.🤖 Generated with Claude Code