diff --git a/autohands/build_util.py b/autohands/build_util.py index 4c718d0b..e652758c 100644 --- a/autohands/build_util.py +++ b/autohands/build_util.py @@ -225,8 +225,19 @@ def execute_notebook(f, report=None, env=None): # stderr is always captured so a clean `sys.exit(0)` skip guard can be # told apart from a genuine cell failure (is_clean_skip_exit); stdout # keeps streaming live unless the report collector wants it. + # Run via run_notebook.py rather than `jupyter nbconvert --execute`: + # nbconvert starts the kernel in the notebook's own directory, but the + # workspaces document (and their auto-simulate guards require) execution + # from the repo root. nbconvert has no CLI flag for the kernel cwd, so + # the runner sets resources['metadata']['path'] via the Python API. + # Still a subprocess, so isolation/timeout/env are unchanged. subprocess.run( - ["jupyter", "nbconvert", "--to", "notebook", "--execute", "--output", f, f], + [ + sys.executable, + str(Path(__file__).parent / "run_notebook.py"), + str(f), + str(Path.cwd()), + ], check=True, timeout=TIMEOUT_SECS, stdout=subprocess.PIPE if report is not None else None, diff --git a/autohands/run_notebook.py b/autohands/run_notebook.py new file mode 100644 index 00000000..96d1e09c --- /dev/null +++ b/autohands/run_notebook.py @@ -0,0 +1,71 @@ +""" +Execute one notebook with the kernel's working directory pinned to the +workspace root. + +Why this exists +--------------- +``jupyter nbconvert --execute`` starts the kernel in the **notebook's own +directory**, not in the directory nbconvert was launched from. The workspaces +document the opposite convention (autolens_workspace/AGENTS.md: "Scripts are +run from the repository root so relative paths to ``dataset/`` and ``output/`` +resolve correctly"), and the auto-simulate guards rely on it:: + + if al.util.dataset.should_simulate(str(dataset_path)): + subprocess.run([sys.executable, "scripts/imaging/simulator.py"], check=True) + +That root-relative path resolves from a script run at the root and cannot +resolve from a notebook run in ``notebooks//`` — the subprocess exits 2 +("can't open file") and every auto-simulating notebook fails. + +nbconvert exposes **no CLI flag** for the kernel's working directory. The knob +is ``resources['metadata']['path']``, which nbclient turns into the kernel's +``cwd`` (see ``nbclient/client.py``, ``_async_start_new_kernel``), and that is +reachable only from the Python API. Hence this module: ``build_util`` still +runs it as a **subprocess**, so process isolation, the ``BUILD_SCRIPT_TIMEOUT`` +and the per-notebook environment are all unchanged — only the kernel's cwd +moves. + +Failure contract +---------------- +On a cell error the ``CellExecutionError`` is deliberately left **uncaught** so +Python prints its own traceback to stderr. That output contains the +``CellExecutionError`` marker and ends with the failing cell's terminal +``: `` line, which is exactly what +``build_util.is_clean_skip_exit`` parses to tell an intentional +``sys.exit(0)`` skip from a genuine failure. Do not wrap it in a handler that +reformats the message, or that skip detection breaks. + +Usage:: + + python run_notebook.py +""" + +import sys +from pathlib import Path + +import nbformat +from nbconvert.preprocessors import ExecutePreprocessor + + +def run(notebook_path: str, root: str) -> None: + nb_path = Path(notebook_path) + nb = nbformat.read(nb_path, as_version=4) + + ep = ExecutePreprocessor(timeout=None, kernel_name="python3") + + try: + # The whole point: pin the kernel's cwd to the workspace root rather + # than letting it default to the notebook's own directory. + ep.preprocess(nb, {"metadata": {"path": str(root)}}) + finally: + # Write back even on failure so a partially-executed notebook keeps its + # outputs, matching `nbconvert --output ` (which writes in place). + nbformat.write(nb, nb_path) + + +if __name__ == "__main__": + if len(sys.argv) != 3: + print(f"usage: {Path(__file__).name} ", + file=sys.stderr) + sys.exit(2) + run(sys.argv[1], sys.argv[2]) diff --git a/tests/test_run_notebook_cwd.py b/tests/test_run_notebook_cwd.py new file mode 100644 index 00000000..36c6c89c --- /dev/null +++ b/tests/test_run_notebook_cwd.py @@ -0,0 +1,97 @@ +"""Regression tests for the notebook kernel's working directory. + +`jupyter nbconvert --execute` starts the kernel in the **notebook's own +directory**, but the workspaces document execution from the repo root and their +auto-simulate guards shell out to root-relative simulator paths:: + + if al.util.dataset.should_simulate(str(dataset_path)): + subprocess.run([sys.executable, "scripts/imaging/simulator.py"], check=True) + +Under nbconvert that subprocess exits 2 ("can't open file") and every +auto-simulating notebook fails. `autohands/run_notebook.py` fixes this by +pinning the kernel cwd to the workspace root via +`resources['metadata']['path']`. + +These tests lock in the two properties that matter: the kernel really runs at +the given root (not the notebook's directory), and a root-relative subprocess +therefore resolves. They are skipped when a Jupyter kernel is unavailable. +""" + +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +PROJECT_ROOT = Path(__file__).parent.parent +RUNNER = PROJECT_ROOT / "autohands" / "run_notebook.py" + +jupyter = pytest.importorskip("nbformat") +pytest.importorskip("nbconvert") + + +def _write_nb(path: Path, source: str) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + nb = { + "cells": [{ + "cell_type": "code", + "execution_count": None, + "metadata": {}, + "outputs": [], + "source": source, + }], + "metadata": {"kernelspec": { + "display_name": "Python 3", "language": "python", "name": "python3", + }}, + "nbformat": 4, + "nbformat_minor": 5, + } + path.write_text(json.dumps(nb)) + + +def _outputs(path: Path) -> str: + nb = json.loads(path.read_text()) + return "".join( + "".join(o.get("text", "")) for o in nb["cells"][0].get("outputs", []) + ) + + +def _run(nb_path: Path, root: Path): + return subprocess.run( + [sys.executable, str(RUNNER), str(nb_path), str(root)], + capture_output=True, text=True, timeout=300, + ) + + +def test_kernel_cwd_is_the_given_root_not_the_notebook_dir(tmp_path): + """The whole point of the runner: cwd is the root, not notebooks/sub/.""" + nb = tmp_path / "notebooks" / "sub" / "cwd.ipynb" + _write_nb(nb, 'import os\nprint("CWD:", os.getcwd())') + + result = _run(nb, tmp_path) + + assert result.returncode == 0, result.stderr + out = _outputs(nb) + assert f"CWD: {tmp_path}" in out + # the failure mode this exists to prevent + assert str(nb.parent) not in out + + +def test_root_relative_subprocess_resolves(tmp_path): + """The auto-simulate guard shape: a root-relative script path must run.""" + (tmp_path / "scripts").mkdir(parents=True, exist_ok=True) + (tmp_path / "scripts" / "sim.py").write_text('print("simulated")') + + nb = tmp_path / "notebooks" / "deep" / "guard.ipynb" + _write_nb( + nb, + "import subprocess, sys\n" + 'r = subprocess.run([sys.executable, "scripts/sim.py"], check=True)\n' + 'print("GUARD_OK")', + ) + + result = _run(nb, tmp_path) + + assert result.returncode == 0, result.stderr + assert "GUARD_OK" in _outputs(nb)