diff --git a/autohands/build_util.py b/autohands/build_util.py index b4c57a4..7c6e039 100644 --- a/autohands/build_util.py +++ b/autohands/build_util.py @@ -358,7 +358,7 @@ def is_clean_skip_exit(output: str) -> bool: return bool(tail) and _SKIP_EXIT_RE.match(tail[-1]) is not None -def regenerate_notebook(nb_path, scripts_dir) -> Path: +def regenerate_notebook(nb_path, scripts_dir, rel=None) -> Path: """ Regenerate one notebook from its source ``.py`` into a temp dir. @@ -376,8 +376,13 @@ def regenerate_notebook(nb_path, scripts_dir) -> Path: The notebook that failed, e.g. ``notebooks/imaging/model_fit.ipynb``. scripts_dir The directory holding the source scripts, e.g. ``/scripts``. - The source is looked up at the notebook's path relative to its own - ``notebooks/`` root, with a ``.py`` suffix. + rel + The notebook's path RELATIVE to its own ``notebooks/`` root, e.g. + ``imaging/model_fit.ipynb``. The source script is that same relative + path under ``scripts_dir`` with a ``.py`` suffix. Omitting it falls back + to the bare filename, which is only correct for a notebook sitting at + the root — every workspace notebook lives in a subdirectory, so callers + iterating a tree must pass this. Returns ------- @@ -390,7 +395,8 @@ def regenerate_notebook(nb_path, scripts_dir) -> Path: """ nb_path = Path(nb_path) scripts_dir = Path(scripts_dir) - script_path = scripts_dir / Path(nb_path.name).with_suffix(".py") + rel = Path(rel) if rel is not None else Path(nb_path.name) + script_path = scripts_dir / rel.with_suffix(".py") if not script_path.exists(): raise FileNotFoundError(f"No source script at {script_path}") @@ -553,7 +559,7 @@ def _classify_notebook_run(run_target, recorded, report, env, timeout_secs): def execute_notebook(f, report=None, env=None, write_back=True, - retry_from_scripts=None, report_as=None): + retry_from_scripts=None, report_as=None, notebook_rel=None): """ Execute one notebook as a subprocess, with the kernel cwd at the repo root. @@ -594,7 +600,7 @@ def execute_notebook(f, report=None, env=None, write_back=True, print(" notebook failed; regenerating from source script and retrying...") try: - regenerated = regenerate_notebook(f, retry_from_scripts) + regenerated = regenerate_notebook(f, retry_from_scripts, rel=notebook_rel) except Exception as exc: # No source script, or generation itself failed. The first attempt's # FAIL stands — the recovery was unavailable, not the notebook fixed. @@ -690,12 +696,20 @@ def execute_notebooks_in_folder( else: from env_config import build_env_for_script env = build_env_for_script(file, env_config) + # The path relative to the notebooks root is what maps a notebook + # to its source script; the bare filename would collide across + # subdirectories and miss the source entirely. + try: + notebook_rel = file.relative_to(Path.cwd() / directory) + except ValueError: # pragma: no cover - file outside the root + notebook_rel = Path(file.name) execute_notebook( file, report=report, env=env, write_back=write_back, retry_from_scripts=retry_from_scripts, + notebook_rel=notebook_rel, ) diff --git a/tests/test_notebook_delegation.py b/tests/test_notebook_delegation.py index 9ec2c23..9959905 100644 --- a/tests/test_notebook_delegation.py +++ b/tests/test_notebook_delegation.py @@ -217,3 +217,60 @@ def counting(*args, **kwargs): assert status == "passed" assert len(calls) == 1 assert report.results[0].status == Status.PASSED + + +class TestNestedNotebooks: + """ + A notebook's source script is found by its path relative to `notebooks/`. + + Every workspace notebook lives in a subdirectory (`imaging/`, `modeling/`, + ...). Resolving the source by bare filename would look in the wrong place + and, worse, could collide across subdirectories — two `model_fit.ipynb` + under different topics map to two different scripts. + """ + + def test_nested_notebook_regenerates_from_its_own_source(self, workspace): + _write_notebook( + workspace / "notebooks" / "imaging" / "model_fit.ipynb", + "raise RuntimeError('stale')", + ) + (workspace / "scripts" / "imaging").mkdir(parents=True) + (workspace / "scripts" / "imaging" / "model_fit.py").write_text("x = 1 + 1\n") + # A decoy at the root: resolving by bare filename would pick this up. + (workspace / "scripts" / "model_fit.py").write_text("raise RuntimeError('wrong source')\n") + report = _report() + + status = execute_notebook( + workspace / "notebooks" / "imaging" / "model_fit.ipynb", + report=report, + write_back=False, + retry_from_scripts=workspace / "scripts", + notebook_rel=Path("imaging/model_fit.ipynb"), + ) + + assert status == "passed", "must regenerate from scripts/imaging/, not the root decoy" + assert len(report.results) == 1 + + def test_the_folder_runner_passes_the_relative_path(self, workspace): + """End-to-end through execute_notebooks_in_folder, which computes it.""" + from build_util import execute_notebooks_in_folder + + _write_notebook( + workspace / "notebooks" / "imaging" / "model_fit.ipynb", + "raise RuntimeError('stale')", + ) + (workspace / "scripts" / "imaging").mkdir(parents=True) + (workspace / "scripts" / "imaging" / "model_fit.py").write_text("x = 1 + 1\n") + (workspace / "scripts" / "model_fit.py").write_text("raise RuntimeError('wrong source')\n") + report = _report() + + execute_notebooks_in_folder( + directory="notebooks", + no_run_list=[], + report=report, + write_back=False, + retry_from_scripts=workspace / "scripts", + ) + + assert len(report.results) == 1 + assert report.results[0].status == Status.PASSED