Skip to content

Commit ef0f5a3

Browse files
Jammy2211Jammy2211claude
authored
fix: run notebooks with the kernel cwd pinned to the workspace root (#205)
`jupyter nbconvert --execute` starts the kernel in the notebook's own directory, not the directory nbconvert was launched from. The workspaces document the opposite ("scripts are run from the repository root so relative paths to dataset/ and output/ resolve") and their auto-simulate guards shell out to root-relative simulator paths, so every auto-simulating notebook failed with exit 2 ("can't open file") — the largest contributor to the red run_notebooks shards. nbconvert exposes no CLI flag for the kernel's working directory; the only knob is resources['metadata']['path'], which nbclient turns into the kernel cwd, and that is reachable only from the Python API. So execute_notebook now subprocesses a new autohands/run_notebook.py which sets it. The subprocess boundary is kept deliberately: process isolation, BUILD_SCRIPT_TIMEOUT and the per-notebook environment are all unchanged — only the kernel's cwd moves. run_notebook.py leaves CellExecutionError uncaught on purpose. Python's own traceback then reproduces the exact shape build_util.is_clean_skip_exit parses (a CellExecutionError marker whose last line is `SystemExit: 0`) to tell an intentional optional-dependency skip from a real failure. Verified: sys.exit(0) -> True, sys.exit(1) -> False, ValueError -> False. Two regression tests lock in the kernel cwd and the root-relative-subprocess case. Full suite green (236). Closes #204 Co-authored-by: Jammy2211 <JNightingale2211@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent f046467 commit ef0f5a3

3 files changed

Lines changed: 180 additions & 1 deletion

File tree

autohands/build_util.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,19 @@ def execute_notebook(f, report=None, env=None):
225225
# stderr is always captured so a clean `sys.exit(0)` skip guard can be
226226
# told apart from a genuine cell failure (is_clean_skip_exit); stdout
227227
# keeps streaming live unless the report collector wants it.
228+
# Run via run_notebook.py rather than `jupyter nbconvert --execute`:
229+
# nbconvert starts the kernel in the notebook's own directory, but the
230+
# workspaces document (and their auto-simulate guards require) execution
231+
# from the repo root. nbconvert has no CLI flag for the kernel cwd, so
232+
# the runner sets resources['metadata']['path'] via the Python API.
233+
# Still a subprocess, so isolation/timeout/env are unchanged.
228234
subprocess.run(
229-
["jupyter", "nbconvert", "--to", "notebook", "--execute", "--output", f, f],
235+
[
236+
sys.executable,
237+
str(Path(__file__).parent / "run_notebook.py"),
238+
str(f),
239+
str(Path.cwd()),
240+
],
230241
check=True,
231242
timeout=TIMEOUT_SECS,
232243
stdout=subprocess.PIPE if report is not None else None,

autohands/run_notebook.py

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
"""
2+
Execute one notebook with the kernel's working directory pinned to the
3+
workspace root.
4+
5+
Why this exists
6+
---------------
7+
``jupyter nbconvert --execute`` starts the kernel in the **notebook's own
8+
directory**, not in the directory nbconvert was launched from. The workspaces
9+
document the opposite convention (autolens_workspace/AGENTS.md: "Scripts are
10+
run from the repository root so relative paths to ``dataset/`` and ``output/``
11+
resolve correctly"), and the auto-simulate guards rely on it::
12+
13+
if al.util.dataset.should_simulate(str(dataset_path)):
14+
subprocess.run([sys.executable, "scripts/imaging/simulator.py"], check=True)
15+
16+
That root-relative path resolves from a script run at the root and cannot
17+
resolve from a notebook run in ``notebooks/<topic>/`` — the subprocess exits 2
18+
("can't open file") and every auto-simulating notebook fails.
19+
20+
nbconvert exposes **no CLI flag** for the kernel's working directory. The knob
21+
is ``resources['metadata']['path']``, which nbclient turns into the kernel's
22+
``cwd`` (see ``nbclient/client.py``, ``_async_start_new_kernel``), and that is
23+
reachable only from the Python API. Hence this module: ``build_util`` still
24+
runs it as a **subprocess**, so process isolation, the ``BUILD_SCRIPT_TIMEOUT``
25+
and the per-notebook environment are all unchanged — only the kernel's cwd
26+
moves.
27+
28+
Failure contract
29+
----------------
30+
On a cell error the ``CellExecutionError`` is deliberately left **uncaught** so
31+
Python prints its own traceback to stderr. That output contains the
32+
``CellExecutionError`` marker and ends with the failing cell's terminal
33+
``<ename>: <evalue>`` line, which is exactly what
34+
``build_util.is_clean_skip_exit`` parses to tell an intentional
35+
``sys.exit(0)`` skip from a genuine failure. Do not wrap it in a handler that
36+
reformats the message, or that skip detection breaks.
37+
38+
Usage::
39+
40+
python run_notebook.py <notebook-path> <workspace-root>
41+
"""
42+
43+
import sys
44+
from pathlib import Path
45+
46+
import nbformat
47+
from nbconvert.preprocessors import ExecutePreprocessor
48+
49+
50+
def run(notebook_path: str, root: str) -> None:
51+
nb_path = Path(notebook_path)
52+
nb = nbformat.read(nb_path, as_version=4)
53+
54+
ep = ExecutePreprocessor(timeout=None, kernel_name="python3")
55+
56+
try:
57+
# The whole point: pin the kernel's cwd to the workspace root rather
58+
# than letting it default to the notebook's own directory.
59+
ep.preprocess(nb, {"metadata": {"path": str(root)}})
60+
finally:
61+
# Write back even on failure so a partially-executed notebook keeps its
62+
# outputs, matching `nbconvert --output <f> <f>` (which writes in place).
63+
nbformat.write(nb, nb_path)
64+
65+
66+
if __name__ == "__main__":
67+
if len(sys.argv) != 3:
68+
print(f"usage: {Path(__file__).name} <notebook-path> <workspace-root>",
69+
file=sys.stderr)
70+
sys.exit(2)
71+
run(sys.argv[1], sys.argv[2])

tests/test_run_notebook_cwd.py

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
"""Regression tests for the notebook kernel's working directory.
2+
3+
`jupyter nbconvert --execute` starts the kernel in the **notebook's own
4+
directory**, but the workspaces document execution from the repo root and their
5+
auto-simulate guards shell out to root-relative simulator paths::
6+
7+
if al.util.dataset.should_simulate(str(dataset_path)):
8+
subprocess.run([sys.executable, "scripts/imaging/simulator.py"], check=True)
9+
10+
Under nbconvert that subprocess exits 2 ("can't open file") and every
11+
auto-simulating notebook fails. `autohands/run_notebook.py` fixes this by
12+
pinning the kernel cwd to the workspace root via
13+
`resources['metadata']['path']`.
14+
15+
These tests lock in the two properties that matter: the kernel really runs at
16+
the given root (not the notebook's directory), and a root-relative subprocess
17+
therefore resolves. They are skipped when a Jupyter kernel is unavailable.
18+
"""
19+
20+
import json
21+
import subprocess
22+
import sys
23+
from pathlib import Path
24+
25+
import pytest
26+
27+
PROJECT_ROOT = Path(__file__).parent.parent
28+
RUNNER = PROJECT_ROOT / "autohands" / "run_notebook.py"
29+
30+
jupyter = pytest.importorskip("nbformat")
31+
pytest.importorskip("nbconvert")
32+
33+
34+
def _write_nb(path: Path, source: str) -> None:
35+
path.parent.mkdir(parents=True, exist_ok=True)
36+
nb = {
37+
"cells": [{
38+
"cell_type": "code",
39+
"execution_count": None,
40+
"metadata": {},
41+
"outputs": [],
42+
"source": source,
43+
}],
44+
"metadata": {"kernelspec": {
45+
"display_name": "Python 3", "language": "python", "name": "python3",
46+
}},
47+
"nbformat": 4,
48+
"nbformat_minor": 5,
49+
}
50+
path.write_text(json.dumps(nb))
51+
52+
53+
def _outputs(path: Path) -> str:
54+
nb = json.loads(path.read_text())
55+
return "".join(
56+
"".join(o.get("text", "")) for o in nb["cells"][0].get("outputs", [])
57+
)
58+
59+
60+
def _run(nb_path: Path, root: Path):
61+
return subprocess.run(
62+
[sys.executable, str(RUNNER), str(nb_path), str(root)],
63+
capture_output=True, text=True, timeout=300,
64+
)
65+
66+
67+
def test_kernel_cwd_is_the_given_root_not_the_notebook_dir(tmp_path):
68+
"""The whole point of the runner: cwd is the root, not notebooks/sub/."""
69+
nb = tmp_path / "notebooks" / "sub" / "cwd.ipynb"
70+
_write_nb(nb, 'import os\nprint("CWD:", os.getcwd())')
71+
72+
result = _run(nb, tmp_path)
73+
74+
assert result.returncode == 0, result.stderr
75+
out = _outputs(nb)
76+
assert f"CWD: {tmp_path}" in out
77+
# the failure mode this exists to prevent
78+
assert str(nb.parent) not in out
79+
80+
81+
def test_root_relative_subprocess_resolves(tmp_path):
82+
"""The auto-simulate guard shape: a root-relative script path must run."""
83+
(tmp_path / "scripts").mkdir(parents=True, exist_ok=True)
84+
(tmp_path / "scripts" / "sim.py").write_text('print("simulated")')
85+
86+
nb = tmp_path / "notebooks" / "deep" / "guard.ipynb"
87+
_write_nb(
88+
nb,
89+
"import subprocess, sys\n"
90+
'r = subprocess.run([sys.executable, "scripts/sim.py"], check=True)\n'
91+
'print("GUARD_OK")',
92+
)
93+
94+
result = _run(nb, tmp_path)
95+
96+
assert result.returncode == 0, result.stderr
97+
assert "GUARD_OK" in _outputs(nb)

0 commit comments

Comments
 (0)