ci(run-notebook): replace whole-cell bash partition with nbclient#573
Merged
Conversation
## Purpose The previous run-notebook action partitioned each code cell whole-cell: cells containing !pip went to a bash script, the rest to a Python script. Cells that mixed !pip install with Python imports — a standard Jupyter pattern — failed with `import: command not found` because bash tried to execute Python as a shell command (see PR #572 for an example). ## Solution Drive notebooks through nbclient, the same tool Project Jupyter ships behind `jupyter nbconvert --execute`. nbclient launches a real ipykernel and processes cells line-by-line: !pip and other shell magics route through the kernel's magic handlers (subshell), Python runs as Python. Mixed cells work exactly as they do in Colab and Jupyter Lab. ## Notes - Notebook deps install into the runner's Python (kernel shares the runner's interpreter), not a nested venv. Fine for CI (ephemeral runner) and simpler than the previous tempdir-venv dance. - Per-cell timeout is 600s, overridable via NOTEBOOK_CELL_TIMEOUT. - check-structure's "imports must be in the first code cell" rule may now warrant relaxation — under the new runner, a single cell holding !pip install + imports is the correct pattern again. Follow-up.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6f37ce1. Configure here.
`CellTimeoutError` (sibling of `CellExecutionError`, inherits from `TimeoutError`) and `DeadKernelError` (inherits from `RuntimeError`) escape an `except CellExecutionError`. A cell hitting NOTEBOOK_CELL_TIMEOUT or a kernel that OOMs/segfaults would surface a raw traceback instead of the clean failure path. Catch both alongside `CellExecutionError`, with distinct messages so the failure mode is obvious in CI logs. Each path still exits 1. Reported by Cursor Bugbot on PR #573.
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.

Purpose
The current
run-notebookaction partitions each code cell whole-cell: cells containing!pipgo entirely to a bash script, the rest go to a Python script. A cell that mixes!pip installwith Python imports — a standard Jupyter pattern — fails withimport: command not foundbecause bash tries to execute Python as a shell command. See PR #572 / run 25743389812 for a live example.Solution
Drive notebooks through
nbclient, the same tool Project Jupyter ships behindjupyter nbconvert --execute.nbclientlaunches a realipykerneland processes cells line-by-line:!pipand other shell magics route through the kernel's magic handlers (subshell), Python runs as Python. Mixed cells work exactly as they do in Colab and Jupyter Lab.Changes
run-notebook.py: 50-line nbclient driver. Takes the notebook path as its only arg. Exits 0 on success, 1 on any cell error, 2 on usage error.action.yml: installsnbformat nbclient ipykernelinto the runner Python, then invokes the new script. No tempdir, no nested venv, no bash conversion.convert-notebook.py(~120 lines).Net diff: 66 insertions, 137 deletions.
Verification
Tested locally against the notebook from PR #572 (
docs/quick-tour/hello-pinecone.ipynb, with!pip installand Python imports in the same cell — the case that breaks today's runner):Notes
check-structurefollow-up. The lint rule "imports must be in the first code cell" was designed around today's runner: under the new runner, a single cell holding!pip install+ imports is once again the correct pattern. Either relax the rule to skip pip-only cells when locating "the first code cell," or remove the rule entirely. Separate PR.NOTEBOOK_CELL_TIMEOUT.Note
Medium Risk
Changes the CI notebook execution mechanism from a custom conversion/venv+bash flow to
nbclientdriving a real Jupyter kernel, which may alter dependency/runtime behavior and failure modes across the notebook suite.Overview
Updates the
run-notebookcomposite action to execute notebooks end-to-end usingnbclient/ipykernelinstead of converting cells into separate bash/python scripts.This removes
convert-notebook.py, installsnbformat nbclient ipykernelon the runner, and addsrun-notebook.pywhich runs the notebook with a real kernel, returning clearer per-cell errors and supporting mixed!pip+ Python cells (with an optionalNOTEBOOK_CELL_TIMEOUT).Reviewed by Cursor Bugbot for commit f87187a. Bugbot is set up for automated code reviews on this repo. Configure here.