From d7f03efd07ca2b8ce737b8f54561c848f5631047 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 22 Jun 2026 02:11:14 +0000 Subject: [PATCH] fix: make doctor crash-proof and inspect tolerant of incomplete runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit End-to-end audit of the recently added features surfaced two robustness/UX bugs in the new diagnostics commands (rest verified OK): - doctor: collect_doctor_checks ran each check unguarded, so a throwing check (e.g. a broken PyBullet/headless env in _examples_check) crashed the command with a raw traceback — exactly when it should diagnose the failure. Each check now runs through a `_safe` guard that turns an exception into a `warn` row. - inspect: it called runs.load_run, which unconditionally reads creature.json (which inspect never uses), so a missing path or a run dir / trace.json without a sibling creature.json raised a raw FileNotFoundError. It now loads only the trace (+ optional task.json) via the friendly _load_spec / _load_task_for_trace helpers: missing path exits 2; a creature-less run dir still summarizes. Tests: doctor stays crash-free when a check raises (returns a warn row); inspect exits 2 on a missing path and summarizes a trace-only run dir. All green: ruff, ruff format --check, pytest (131). https://claude.ai/code/session_01EcH4uu86dMEDrd6TzyAExd --- creature_lab/cli.py | 4 ++-- creature_lab/diagnostics.py | 31 +++++++++++++++++++++++-------- docs/ANALYSIS.md | 10 ++++++++-- tests/test_cli.py | 24 ++++++++++++++++++++++++ tests/test_diagnostics.py | 13 +++++++++++++ 5 files changed, 70 insertions(+), 12 deletions(-) diff --git a/creature_lab/cli.py b/creature_lab/cli.py index 5e22d1d..bd020d4 100644 --- a/creature_lab/cli.py +++ b/creature_lab/cli.py @@ -21,7 +21,6 @@ from creature_lab.library import default_creature, default_task from creature_lab.runs import ( DEFAULT_RUNS_DIR, - load_run, new_run_id, resolve_trace_path, save_run, @@ -436,7 +435,8 @@ def inspect( path: Annotated[Path, typer.Argument(help="Path to a run directory (or trace.json).")], ) -> None: """Print a detailed diagnostic summary of a saved run.""" - _, task, trace = load_run(path) + trace = _load_spec(resolve_trace_path(path), EpisodeTrace) + task = _load_task_for_trace(path, None) summary = summarize_episode(trace, task) meta = trace.meta diff --git a/creature_lab/diagnostics.py b/creature_lab/diagnostics.py index 6d3b32e..c4670dc 100644 --- a/creature_lab/diagnostics.py +++ b/creature_lab/diagnostics.py @@ -12,6 +12,7 @@ import os import platform from collections import Counter +from collections.abc import Callable from dataclasses import dataclass from creature_lab.schema import EpisodeSummary, EpisodeTrace, TaskSpec @@ -39,20 +40,30 @@ def _installed(module: str) -> bool: def collect_doctor_checks() -> list[DoctorCheck]: - """Inspect the environment: platform, optional extras, and an example run.""" - checks = [ + """Inspect the environment: platform, optional extras, and an example run. + + Diagnostics must never crash, so every check is guarded — a failing check + becomes a `warn` row rather than propagating an exception. + """ + return [ DoctorCheck( "platform", "info", f"Python {platform.python_version()} on {platform.platform()}", ), - _extra_check("sim (pybullet)", "pybullet", "uv sync --extra sim"), - _viz_check(), - _export_check(), - _llm_check(), - _examples_check(), + _safe("sim (pybullet)", _sim_check), + _safe("viz (viser)", _viz_check), + _safe("export (imageio)", _export_check), + _safe("llm (litellm)", _llm_check), + _safe("examples run", _examples_check), ] - return checks + + +def _safe(name: str, check: Callable[[], DoctorCheck]) -> DoctorCheck: + try: + return check() + except Exception as exc: # a diagnostic must report failures, not raise them + return DoctorCheck(name, "warn", f"check failed: {exc}") def _extra_check(name: str, module: str, hint: str) -> DoctorCheck: @@ -61,6 +72,10 @@ def _extra_check(name: str, module: str, hint: str) -> DoctorCheck: return DoctorCheck(name, "missing", f"not installed — `{hint}`") +def _sim_check() -> DoctorCheck: + return _extra_check("sim (pybullet)", "pybullet", "uv sync --extra sim") + + def _viz_check() -> DoctorCheck: if not _installed("viser"): return DoctorCheck("viz (viser)", "missing", "not installed — `uv sync --extra viz`") diff --git a/docs/ANALYSIS.md b/docs/ANALYSIS.md index 6f602ec..e0cd61f 100644 --- a/docs/ANALYSIS.md +++ b/docs/ANALYSIS.md @@ -205,7 +205,13 @@ A second pass that ran the whole pipeline and read every module. Findings: `distance_traveled`→`net_displacement`, `forward_distance`→`forward_displacement` (both are net centroid displacements) — and `duration` is now the final frame's timestamp (total simulated time, e.g. 3.00s) rather than the one-timestep-short frame span; `replay` matches. - -Verified clean after the fixes: `ruff check`, `ruff format --check`, and `pytest` (128 tests, +- **D6 ✅ — Robustness of the new diagnostics commands (audit fixes).** `doctor` no longer crashes + when a check throws (e.g. a broken PyBullet/headless env): `collect_doctor_checks` runs each + check through a `_safe` guard that turns a failure into a `warn` row. `inspect` no longer raw- + tracebacks on a missing/incomplete run: it loads only the trace (+ optional `task.json`) via the + friendly `_load_spec`/`_load_task_for_trace` helpers (it never used the creature), so a missing + path exits 2 and a run dir with only `trace.json` still summarizes. + +Verified clean after the fixes: `ruff check`, `ruff format --check`, and `pytest` (131 tests, including end-to-end scenarios) all green, and the full CLI loop (`doctor → validate → run → inspect → replay → export → evolve → ask → demo`) works. diff --git a/tests/test_cli.py b/tests/test_cli.py index 40182a7..ad21614 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -44,6 +44,30 @@ def test_inspect_reports_summary(tmp_path): assert "sha256:" in result.stdout +def test_inspect_missing_path_exits_cleanly(): + result = runner.invoke(app, ["inspect", "does/not/exist"]) + assert result.exit_code == 2 # friendly file-not-found, not a raw traceback + + +def test_inspect_without_creature_json_still_summarizes(tmp_path): + # A run dir / trace.json without a sibling creature.json must not crash inspect. + trace = { + "run_id": "r1", + "creature_name": "c", + "task_name": "t", + "backend": "pybullet", + "score": 0.5, + "frames": [ + {"t": 0.1, "parts": {"a": {"position": [0, 0, 0]}}, "score": 0.0}, + {"t": 0.2, "parts": {"a": {"position": [1, 0, 0]}}, "score": 0.5}, + ], + } + (tmp_path / "trace.json").write_text(json.dumps(trace)) + result = runner.invoke(app, ["inspect", str(tmp_path)]) + assert result.exit_code == 0, result.stdout + assert "final score" in result.stdout + + def test_validate_example_creature(): result = runner.invoke(app, ["validate", str(EXAMPLE)]) assert result.exit_code == 0, result.stdout diff --git a/tests/test_diagnostics.py b/tests/test_diagnostics.py index c680955..8dc63ef 100644 --- a/tests/test_diagnostics.py +++ b/tests/test_diagnostics.py @@ -1,5 +1,6 @@ """Tests for environment checks and episode summaries.""" +import creature_lab.diagnostics as diagnostics from creature_lab.diagnostics import collect_doctor_checks, summarize_episode from creature_lab.schema import EpisodeTrace, TaskSpec @@ -103,3 +104,15 @@ def test_doctor_viz_check_reports_trimesh_and_numpy(): assert viz.status in {"ok", "warn", "missing"} if viz.status == "ok": assert "trimesh" in viz.detail and "numpy" in viz.detail + + +def test_doctor_never_crashes_when_a_check_raises(monkeypatch): + def boom() -> diagnostics.DoctorCheck: + raise RuntimeError("simulated broken environment") + + monkeypatch.setattr(diagnostics, "_examples_check", boom) + checks = {check.name: check for check in collect_doctor_checks()} # must not raise + assert checks["examples run"].status == "warn" + assert "simulated broken environment" in checks["examples run"].detail + # Other checks are unaffected. + assert checks["platform"].status == "info"