From 9b3b1ab2cfd5ff4e488be4bf92171bb7f246bcdc Mon Sep 17 00:00:00 2001 From: NiJingzhe <1448976235@qq.com> Date: Thu, 15 Oct 2026 20:02:04 +0800 Subject: [PATCH 1/4] fix(editor): sca cell finds the editor through proxies and reports what it failed to reach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A container's http_proxy swallowed the health check of a 127.0.0.1 editor url and the miss was read as "no editor is running" — both the record's url (now always the loopback) and the check (now a proxy-bypassing opener) are fixed, and a record whose server does not answer is an error naming the record and url, not a silent fallthrough. Reading the file as a fallback says so on stderr. Cell feedback: put/insert/run carry console and the not-healthy dependents, the swept dependents run, and a failing cell names its upstream (the failing ancestors first) with marimo's own dependency semantics. Co-Authored-By: Claude Sonnet 5.5 --- src/simplecadapi/editor/cli.py | 67 ++++++++++++-- src/simplecadapi/editor/server.py | 75 +++++++++++++-- src/simplecadapi/runtime/_marimo.py | 5 +- test/test_editor.py | 136 +++++++++++++++++++++++++++- 4 files changed, 265 insertions(+), 18 deletions(-) diff --git a/src/simplecadapi/editor/cli.py b/src/simplecadapi/editor/cli.py index 912babd..8368c7d 100644 --- a/src/simplecadapi/editor/cli.py +++ b/src/simplecadapi/editor/cli.py @@ -154,23 +154,51 @@ def _print_json(report: dict[str, Any]) -> None: print(json.dumps(report, indent=2, ensure_ascii=False)) +def _opener() -> Any: + """An opener that bypasses the proxy environment for the local editor. + + The editor is on this machine (its url is 127.0.0.1); a container's + ``http_proxy`` does not know that and swallows the request, which once + read as "no editor is running" while one was. + """ + import urllib.request + + return urllib.request.build_opener(urllib.request.ProxyHandler({})) + + def _find_editor(start: Path) -> tuple[Path, str] | None: - """The running editor whose workspace holds *start*: ``(workspace, base url)``.""" + """The running editor whose workspace holds *start*: ``(workspace, base url)``. + + A record whose server does not answer is an error, not a stale record to + skip quietly: "an editor was running here" is exactly the belief that + must not be dropped in silence. + """ import json - import urllib.request + import urllib.error here = start.expanduser().resolve() + unreachable: list[str] = [] + opener = _opener() for directory in (here, *here.parents): record = directory / ".sca-editor" / "server.json" if not record.is_file(): continue try: url = json.loads(record.read_text(encoding="utf-8"))["url"] - with urllib.request.urlopen(f"{url}/api/health", timeout=1.5): + except (OSError, ValueError, KeyError) as error: + unreachable.append(f"{record}: unreadable ({error})") + continue + try: + with opener.open(f"{url}/api/health", timeout=1.5): pass - except (OSError, ValueError, KeyError): - continue # a stale record: the editor is gone + except urllib.error.HTTPError: + pass # an editor that answers at all is running + except OSError as error: + unreachable.append(f"found {record} but {url} is unreachable: {error}") + continue return directory, str(url) + if unreachable: + raise ValueError("; ".join(unreachable)) return None @@ -184,7 +212,7 @@ def _call(url: str, path: str, body: dict | None = None) -> dict[str, Any]: url + path, data=data, headers={"Content-Type": "application/json"}, method="GET" if body is None else "POST" ) try: - with urllib.request.urlopen(request, timeout=300) as response: + with _opener().open(request, timeout=300) as response: return json.loads(response.read()) except urllib.error.HTTPError as error: try: @@ -196,7 +224,13 @@ def _call(url: str, path: str, body: dict | None = None) -> dict[str, Any]: def _session(args: argparse.Namespace, *, write: bool) -> tuple[str | None, Path | None, str]: """``(editor url or None, notebook path or None, relative notebook name)`` for a cell command.""" start = args.workspace or (args.notebook.parent if args.notebook else Path.cwd()) - found = _find_editor(start) + try: + found = _find_editor(start) + except ValueError as error: + if write: + raise + _warn(f"editor not reached: {error}") + found = None notebook = args.notebook.expanduser().resolve() if args.notebook else None if found is None: if write: @@ -206,6 +240,7 @@ def _session(args: argparse.Namespace, *, write: bool) -> tuple[str | None, Path ) if notebook is None: raise ValueError("no editor is running here; name the notebook with --notebook") + _warn(f"no editor is running on this workspace; reading the file: {notebook.name}") return None, notebook, notebook.name workspace, url = found open_path = _call(url, "/api/cells").get("notebook") @@ -221,9 +256,17 @@ def _session(args: argparse.Namespace, *, write: bool) -> tuple[str | None, Path f"the editor has {open_path} open, not {relative}: edit {relative} directly " "(the editor does not hold it), or open it in the editor first" ) + _warn(f"the editor has {open_path} open; reading {relative} from the file") return None, notebook, relative +def _warn(message: str) -> None: + """Say a fallback out loud: silence once read as "the editor agreed".""" + import sys + + print(f"warning: {message}", file=sys.stderr) + + def _cells_from_file(notebook: Path) -> list[dict[str, Any]]: from ..runtime._marimo import parse_notebook @@ -239,7 +282,7 @@ def _cells_from_file(notebook: Path) -> list[dict[str, Any]]: def _slim(report: dict[str, Any], *, code: bool) -> dict[str, Any]: if "cells" not in report: return report - keep = ("index", "key", "name", "title", "status", "errors", "output", "ref", "readonly") + keep = ("index", "key", "name", "title", "status", "errors", "output", "console", "upstream", "ref", "readonly") return { **report, "cells": [ @@ -290,8 +333,12 @@ def _write_op(args: argparse.Namespace, op: str, body: dict[str, Any]) -> tuple[ return _slim(report, code=False), 1 touched = set(report.get("touched", [])) report = _slim(report, code=False) - # What the change did: the touched cells with their state; the rest as a count. - report["cells"] = [c for c in report["cells"] if c.get("index") in touched] or report["cells"] + # What the change did: the touched cells with their state, plus any cell + # that is not healthy after the run — a rerun dependent failing is the + # feedback the edit was after, hiding it once read as "the edit was fine". + report["cells"] = [ + c for c in report["cells"] if c.get("index") in touched or c.get("errors") or c.get("blocked") + ] or report["cells"] return report, 0 diff --git a/src/simplecadapi/editor/server.py b/src/simplecadapi/editor/server.py index 02e4650..1015b14 100644 --- a/src/simplecadapi/editor/server.py +++ b/src/simplecadapi/editor/server.py @@ -37,6 +37,7 @@ from ..runtime.cells import cell_keys from ..runtime.config import NotebookConfig, read_notebook_config +from ..runtime._marimo import compile_cell from ..runtime.params import ( ParamSpec, commit_field_value, @@ -601,7 +602,7 @@ def ingest_external(self, kernel: MarimoKernel) -> None: # -- the same cell edits, for an agent (``sca cell``) ------------------------- def _brief(self, number: int, cell: dict) -> dict: - return { + brief = { "index": number, "id": cell["id"], "key": cell.get("key"), @@ -615,13 +616,34 @@ def _brief(self, number: int, cell: dict) -> dict: "ref": cell.get("ref"), "code": cell["code"], } + if cell.get("blocked"): + # The cell did not itself fail: an ancestor did (its report says which). + brief["blocked"] = True + return brief def cells_report(self, *, refresh: bool = True) -> dict: if refresh and self.kernel is not None: self.push_cells() + upstream = _upstream_of(self.cells) + place = {cell["id"]: (number, cell) for number, cell in enumerate(self.cells, start=1)} + briefs = [] + for number, cell in enumerate(self.cells, start=1): + brief = self._brief(number, cell) + if brief["errors"] or brief.get("blocked"): + # The cell the error names is often not the one at fault: say + # what it reads, the failing ancestors first. + brief["upstream"] = [ + { + "index": place[dep][0], + "name": place[dep][1].get("name"), + "failing": bool(place[dep][1].get("errors")), + } + for dep in sorted(upstream.get(cell["id"], ()), key=lambda cid: place[cid][0]) + ] + briefs.append(brief) return { "notebook": self.notebook.relative_to(self.workspace).as_posix() if self.notebook else None, - "cells": [self._brief(n, c) for n, c in enumerate(self.cells, start=1)], + "cells": briefs, } def _find_cell(self, ref: Any) -> dict: @@ -654,9 +676,16 @@ def cell_command(self, op: str, body: dict) -> dict: if body.get("base") is not None and _module_code(str(body["base"])).strip() != _module_code(cell["code"]).strip(): return {"error": "conflict: the cell changed since you read it", "theirs": cell["code"]} if code.strip() != _module_code(cell["code"]).strip(): - (kernel.edit_cell if body.get("run", True) else kernel.save_cell)(cell["id"], code) + run = bool(body.get("run", True)) + (kernel.edit_cell if run else kernel.save_cell)(cell["id"], code) + if run: + # The edit runs the cell itself; the sweep adds the + # dependents it made stale — their state is the + # feedback the caller asked for. + kernel.wait_idle() + kernel.run_until_settled([]) elif body.get("run", True): - kernel.run_cells([cell["id"]]) + kernel.run_until_settled([cell["id"]]) touched = [cell["id"]] elif op == "insert": after = body.get("after") @@ -668,9 +697,11 @@ def cell_command(self, op: str, body: dict) -> dict: anchor = self._find_cell(after)["id"] code = _module_code(str(body.get("code", ""))) or "# new cell" name = str(body["name"]) if body.get("name") else None - created = _created_id(kernel.create_cell_after(anchor, code, name, run=bool(body.get("run", True)))) + created = _created_id(kernel.create_cell_after(anchor, code, name, run=False)) if created is None: return {"error": "the kernel did not create the cell"} + if body.get("run", True): + kernel.run_until_settled([created]) touched = [created] elif op == "delete": cell = self._find_cell(body.get("cell")) @@ -1430,8 +1461,10 @@ def write_endpoint(self, host: str) -> None: directory.mkdir(parents=True, exist_ok=True) import os + # The record is read on this machine, so the url is always the + # loopback: a 0.0.0.0 url is no address (and walks into proxies). (directory / "server.json").write_text( - json.dumps({"url": f"http://{host}:{self.port}", "pid": os.getpid(), "workspace": str(self.workspace)}), + json.dumps({"url": f"http://127.0.0.1:{self.port}", "pid": os.getpid(), "workspace": str(self.workspace)}), encoding="utf-8", ) except OSError: @@ -1486,6 +1519,36 @@ def _export(self) -> None: self.hub.broadcast({"type": "status", "exported": str(target)}) +def _upstream_of(cells: list[dict]) -> dict[str, set[str]]: + """Per cell id: the ids of the cells it (transitively) reads. + + Compiled with marimo's own cell compiler, so a name is a dependency + exactly when marimo's dataflow says so; a cell mid-edit (its code does + not compile) contributes no edges rather than wrong ones. + """ + owner: dict[str, str] = {} + refs: dict[str, frozenset[str]] = {} + for cell in cells: + try: + impl = compile_cell(_module_code(cell["code"]), cell["id"]) + except Exception: + refs[cell["id"]] = frozenset() + continue + refs[cell["id"]] = impl.refs + for name in impl.defs: + owner.setdefault(name, cell["id"]) + direct = {cid: {owner[name] for name in names if name in owner and owner[name] != cid} for cid, names in refs.items()} + closure: dict[str, set[str]] = {} + + def walk(cid: str) -> set[str]: + if cid not in closure: + closure[cid] = set() # marimo forbids cycles; the placeholder guards one anyway + closure[cid] = set().union(*(walk(dep) | {dep} for dep in direct.get(cid, ()))) if direct.get(cid) else set() + return closure[cid] + + return {cid: walk(cid) for cid in refs} + + def _module_code(code: str) -> str: """marimo's code_mode compiles cell code as a MODULE: a ``return`` in the cell's own scope (normal in marimo's file format) is a SyntaxError there. diff --git a/src/simplecadapi/runtime/_marimo.py b/src/simplecadapi/runtime/_marimo.py index 2760aea..9874a96 100644 --- a/src/simplecadapi/runtime/_marimo.py +++ b/src/simplecadapi/runtime/_marimo.py @@ -14,13 +14,14 @@ from marimo._ast.app import App, InternalApp from marimo._ast.cell import CellImpl from marimo._ast.cell_manager import CellManager -from marimo._ast.compiler import get_filename, solve_source_position +from marimo._ast.compiler import compile_cell, get_filename, solve_source_position from marimo._ast.parse import ( NonMarimoPythonScriptError, is_non_marimo_python_script, parse_notebook, ) from marimo._schemas.serialization import UnparsableCell +from marimo._loggers import suppress_warnings_logs from marimo._session.notebook.serializer import get_notebook_serializer from marimo._runtime.app.script_runner import AppScriptRunner from marimo._runtime.context.types import ( @@ -61,6 +62,7 @@ "MutableGlobals", "RunResult", "Skip", + "compile_cell", "get_filename", "is_unhashable_stub", "load_notebook", @@ -70,6 +72,7 @@ "runtime_context_installed", "safe_get_context", "solve_source_position", + "suppress_warnings_logs", ] def load_notebook(path: Path, *, cell_prefix: str) -> App | None: diff --git a/test/test_editor.py b/test/test_editor.py index f704cfa..24595c4 100644 --- a/test/test_editor.py +++ b/test/test_editor.py @@ -129,7 +129,7 @@ def test_a_function_cell_is_one_def_or_class(self) -> None: def test_editing_one_is_refused_and_unchanged_code_is_not(self) -> None: state = WorkspaceState(Path(tempfile.gettempdir())) - state.kernel = type("K", (), {"run_cells": lambda self, ids: {}, "wait_idle": lambda self: True})() # type: ignore[assignment] + state.kernel = type("K", (), {"run_until_settled": lambda self, ids: None, "wait_idle": lambda self: True})() # type: ignore[assignment] state.cells = [{"id": "f1", "code": "def f():\n return 1"}] state.schedule_settle = lambda: None # type: ignore[method-assign] refused = state.dispatch_command({"type": "code.edit", "cell": "f1", "code": "def f():\n return 2"}) @@ -1101,3 +1101,137 @@ def test_the_ws_handshake_ends_with_the_last_error(self) -> None: with TestClient(create_app(state)) as client, client.websocket_connect("/ws") as ws: self.assertEqual(ws.receive_json()["type"], "hello") self.assertEqual(ws.receive_json(), {"type": "status", "error": self.ERROR}) + + +class EndpointDiscoveryTest(unittest.TestCase): + """How `sca cell` finds the editor, and what it says when it cannot.""" + + def setUp(self) -> None: + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.root = Path(tmp.name).resolve() + + def test_server_json_pins_the_loopback_even_for_a_wildcard_host(self) -> None: + state = WorkspaceState(self.root) + state.port = 17611 + state.write_endpoint("0.0.0.0") + record = json.loads((self.root / ".sca-editor" / "server.json").read_text()) + # The record is read on this machine; a 0.0.0.0 url is no address and + # walks into a container's proxy. + self.assertEqual(record["url"], "http://127.0.0.1:17611") + + def test_a_stale_record_is_an_error_that_names_the_record_and_url(self) -> None: + from simplecadapi.editor.cli import _find_editor + + directory = self.root / ".sca-editor" + directory.mkdir() + (directory / "server.json").write_text(json.dumps({"url": "http://127.0.0.1:17612"})) + with self.assertRaisesRegex(ValueError, "server.json.*17612.*unreachable"): + _find_editor(self.root) + + def test_the_health_check_reaches_a_local_editor_through_a_proxy_environment(self) -> None: + import threading + + import uvicorn + + from simplecadapi.editor.cli import _find_editor + from simplecadapi.editor.server import create_app + + path = create_notebook(self.root, "bracket", "part") + state = WorkspaceState(self.root, path) + state.port = 17613 + server = uvicorn.Server(uvicorn.Config(create_app(state), port=state.port, log_level="error")) + threading.Thread(target=server.run, daemon=True).start() + try: + deadline = time.monotonic() + 10 + healthy = False + while time.monotonic() < deadline: + try: + with urllib.request.urlopen(f"http://127.0.0.1:{state.port}/api/health", timeout=1): + healthy = True + break + except OSError: + time.sleep(0.1) + self.assertTrue(healthy, "the test editor did not come up") + directory = self.root / ".sca-editor" + directory.mkdir(exist_ok=True) + (directory / "server.json").write_text(json.dumps({"url": f"http://127.0.0.1:{state.port}"})) + # The old code let urllib honor this proxy for a 127.0.0.1 url and + # read the resulting failure as "no editor is running". + environment = {"http_proxy": "http://127.0.0.1:1", "https_proxy": "http://127.0.0.1:1"} + saved = {name: os.environ.get(name) for name in environment} + os.environ.update(environment) + try: + found = _find_editor(self.root) + finally: + for name, value in saved.items(): + if value is None: + os.environ.pop(name, None) + else: + os.environ[name] = value + self.assertIsNotNone(found) + self.assertEqual(found[1], f"http://127.0.0.1:{state.port}") + finally: + server.should_exit = True + + def test_reading_the_file_without_an_editor_says_so_on_stderr(self) -> None: + import argparse + import contextlib + import io + + from simplecadapi.editor import cli + + path = create_notebook(self.root, "bracket", "part") + args = argparse.Namespace(notebook=path, workspace=None, cell="1") + err = io.StringIO() + with contextlib.redirect_stderr(err): + report, code = cli._cell_ls(args) + self.assertEqual(code, 0) + self.assertEqual(report["source"], "file") + self.assertIn("reading the file", err.getvalue()) + + # A record that does not answer is the misleading one: the warning + # must say the editor was found but not reached. + directory = self.root / ".sca-editor" + directory.mkdir() + (directory / "server.json").write_text(json.dumps({"url": "http://127.0.0.1:17614"})) + err = io.StringIO() + with contextlib.redirect_stderr(err): + report, code = cli._cell_ls(args) + self.assertEqual(code, 0) + self.assertEqual(report["source"], "file") + self.assertIn("editor not reached", err.getvalue()) + self.assertIn("17614", err.getvalue()) + + def test_slim_keeps_the_console_and_the_upstream(self) -> None: + from simplecadapi.editor.cli import _slim + + report = { + "cells": [ + {"index": 1, "status": "idle"}, + {"index": 2, "status": "idle", "console": ["printed"], "upstream": [{"index": 1, "failing": False}]}, + ] + } + slim = _slim(report, code=False) + self.assertNotIn("console", slim["cells"][0]) + self.assertEqual(slim["cells"][1]["console"], ["printed"]) + self.assertEqual(slim["cells"][1]["upstream"], [{"index": 1, "failing": False}]) + + def test_a_failing_cell_names_its_upstream_and_the_failing_ancestor(self) -> None: + state = WorkspaceState(self.root) + state.notebook = self.root / "bracket.py" + state.cells = [ + {"id": "c1", "code": "width = 20"}, + {"id": "c2", "code": "block = scad.make_box_rsolid(width, 10, 5)"}, + {"id": "c3", "code": 'part = scad.Part(part_id="p", body=block)', "errors": ["boom"]}, + {"id": "c4", "code": "print(part)", "blocked": True}, + ] + report = state.cells_report(refresh=False) + by_index = {cell["index"]: cell for cell in report["cells"]} + self.assertNotIn("upstream", by_index[1]) + self.assertEqual(by_index[4].get("blocked"), True) + upstream = by_index[4]["upstream"] + self.assertEqual([row["index"] for row in upstream], [1, 2, 3]) + self.assertEqual([row["failing"] for row in upstream], [False, False, True]) + self.assertEqual([row["index"] for row in by_index[3]["upstream"]], [1, 2]) + self.assertEqual([row["failing"] for row in by_index[3]["upstream"]], [False, False]) From b2eaf51ae6eb01eb22e0dbb1ffd0ceb02231aa51 Mon Sep 17 00:00:00 2001 From: NiJingzhe <1448976235@qq.com> Date: Thu, 15 Oct 2026 20:02:04 +0800 Subject: [PATCH 2/4] =?UTF-8?q?feat(run):=20validate=20a=20stage=20?= =?UTF-8?q?=E2=80=94=20through/,=20product=3DFalse,=20a=20timeout,=20quiet?= =?UTF-8?q?er=20logs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sca run --through N runs the first N cells the way the editor numbers them (the setup cell takes no number), --no-product runs a work-in-progress notebook that has no product yet, and --timeout (default 120s) fails a run that takes longer instead of hanging the agent. The cache loaders' degraded- state warnings (unsigned entries, stub restores) no longer print at WARNING, and VTK's own chatter (a missing X server) is off; a real failure is still raised and named. Co-Authored-By: Claude Sonnet 5.5 --- src/simplecadapi/inspect/brep/render.py | 3 + .../inspect/brep/render_worker.py | 3 + src/simplecadapi/runtime/cli.py | 43 +++++++- src/simplecadapi/runtime/runner.py | 104 +++++++++++++++--- test/test_notebook_runtime.py | 51 +++++++++ 5 files changed, 184 insertions(+), 20 deletions(-) diff --git a/src/simplecadapi/inspect/brep/render.py b/src/simplecadapi/inspect/brep/render.py index 91bd2bb..475543a 100644 --- a/src/simplecadapi/inspect/brep/render.py +++ b/src/simplecadapi/inspect/brep/render.py @@ -67,6 +67,9 @@ def _vtk_modules(): from vtk.util.numpy_support import numpy_to_vtk, numpy_to_vtkIdTypeArray except ImportError as error: raise ImportError("BREP rendering requires VTK") from error + # VTK's own chatter (a missing X server, driver notes) reads as a render + # failure; a real one is raised and named here. + vtk.vtkObject.GlobalWarningDisplayOff() return vtk, numpy_to_vtk, numpy_to_vtkIdTypeArray _OFFSCREEN_WINDOW: Any | None = None diff --git a/src/simplecadapi/inspect/brep/render_worker.py b/src/simplecadapi/inspect/brep/render_worker.py index 33047ba..0cccbaa 100644 --- a/src/simplecadapi/inspect/brep/render_worker.py +++ b/src/simplecadapi/inspect/brep/render_worker.py @@ -14,6 +14,9 @@ def _read_polydata(path: Path): import vtk + # VTK's own chatter (a missing X server, driver notes) reads as a render + # failure; a real one fails the worker and is named by the caller. + vtk.vtkObject.GlobalWarningDisplayOff() reader = vtk.vtkXMLPolyDataReader() reader.SetFileName(str(path)) reader.Update() diff --git a/src/simplecadapi/runtime/cli.py b/src/simplecadapi/runtime/cli.py index 8765b6d..f07af6d 100644 --- a/src/simplecadapi/runtime/cli.py +++ b/src/simplecadapi/runtime/cli.py @@ -52,6 +52,28 @@ def configure(subparsers: argparse._SubParsersAction) -> None: ) parser.add_argument("--no-cache", action="store_true", help="run every cell") parser.add_argument("--out", type=Path, help="write the product package here") + parser.add_argument( + "--through", + type=int, + default=None, + metavar="N", + help="run only the first N cells, in notebook order: validate a stage " + "the rest of the notebook need not exist for (pair with --no-product " + "unless the stage already defines the product)", + ) + parser.add_argument( + "--no-product", + action="store_true", + help="do not look for the notebook's product: a work-in-progress run", + ) + parser.add_argument( + "--timeout", + type=float, + default=120.0, + metavar="SECONDS", + help="fail the run when it takes longer (default: 120; heavy features " + "can take minutes)", + ) parser.set_defaults(handler=_execute, printer=_print_report) listing = subparsers.add_parser( @@ -96,20 +118,31 @@ def _execute(args: argparse.Namespace) -> tuple[dict[str, Any], int]: id=args.run_id, overrides=parse_overrides(args.overrides), cache=not args.no_cache, + through=args.through, + product=not args.no_product, + timeout=args.timeout, ) - definition = run.definition report: dict[str, Any] = { "notebook": str(run.config.path), - "definition_id": definition.definition_id, - "definition_kind": definition.definition_kind, - "revision": definition.revision, - "content_hash": definition.content_hash, "cells": [ {"name": cell.name, "key": cell.key, "status": cell.status} for cell in run.cells ], } + if run.product is not None: + definition = run.definition + report.update( + definition_id=definition.definition_id, + definition_kind=definition.definition_kind, + revision=definition.revision, + content_hash=definition.content_hash, + ) + else: + report["product"] = None if args.out is not None: + if run.product is None: + raise ValueError("--out needs a product: drop --no-product/--through, or narrow the notebook") + definition = run.definition payload = encode_product_package(build_product_package(definition)) args.out.parent.mkdir(parents=True, exist_ok=True) args.out.write_bytes(payload) diff --git a/src/simplecadapi/runtime/runner.py b/src/simplecadapi/runtime/runner.py index 37bc59e..5e39788 100644 --- a/src/simplecadapi/runtime/runner.py +++ b/src/simplecadapi/runtime/runner.py @@ -51,6 +51,7 @@ load_notebook, runtime_context_installed, safe_get_context, + suppress_warnings_logs, ) from .cache import CellCache, CellDigests, cache_root, prune from .cells import NotebookCells @@ -93,14 +94,15 @@ class NotebookRun: """The outcome of :func:`run_notebook`. ``product`` is the notebook's product and ``definition`` its durable - definition; ``values`` holds every top-level variable, ``cells`` one - report per cell in notebook order, and ``dependencies`` the files - outside the cells the run depends on. + definition (``None`` for a partial run — see ``through``/``product``); + ``values`` holds every top-level variable, ``cells`` one report per cell + in notebook order, and ``dependencies`` the files outside the cells the + run depends on. """ config: NotebookConfig - product: Product - """The product, carrying its definition.""" + product: Product | None + """The product, carrying its definition; ``None`` when none was asked for.""" values: Mapping[str, Any] """Every top-level variable, overrides included.""" cells: tuple[CellReport, ...] @@ -110,6 +112,8 @@ class NotebookRun: @property def definition(self) -> Definition: + if self.product is None: + raise ValueError("this run produced no product (a partial run: through= or product=False)") definition = attached_definition(self.product) # project_product attaches one to every product it returns. assert definition is not None @@ -122,6 +126,9 @@ def run_notebook( id: str | None = None, overrides: Mapping[str, Any] | None = None, cache: bool = True, + through: int | None = None, + product: bool = True, + timeout: float | None = None, ) -> NotebookRun: """Run the notebook at *path* and project its product. @@ -131,6 +138,12 @@ def run_notebook( not run (marimo requires such a cell's variables to be overridden all together). ``cache=False`` neither reads nor writes the cell cache. Errors raised by a cell propagate unchanged. + + *through* runs only the first *through* cells, in notebook order — the + validation of a stage the tail does not have to exist for. With + ``product=False`` the run does not look for the notebook's product at + all. *timeout* (seconds) fails the run with :class:`TimeoutError` when + it takes longer; ``None`` (the default) runs unbounded. """ config = read_notebook_config(path) @@ -144,13 +157,20 @@ def run_notebook( values = dict(overrides or {}) def run() -> NotebookRun: - return _run(config, values, cache=cache) + # The cache loaders warn about the degraded states they heal + # themselves out of (unsigned entries for missing cryptography, a + # value restored as a stub); at WARNING they read as run failures. + # Errors still come through. + with suppress_warnings_logs("lazy"): + return _run(config, values, cache=cache, through=through, product=product) # marimo keeps its runtime context per thread, and a notebook run needs # one of its own; a run started from a cell (``use``) gets a thread. if runtime_context_installed(): return _on_own_thread(run) - return run() + if timeout is None: + return run() + return _with_timeout(run, timeout) def use(path: str | Path, /, *, id: str | None = None, **overrides: Any) -> Product: @@ -167,6 +187,9 @@ def use(path: str | Path, /, *, id: str | None = None, **overrides: Any) -> Prod if not target.is_absolute(): target = _calling_directory() / target child = run_notebook(target, id=id, overrides=overrides) + # use() never asks for a partial run, so the product is always there. + if child.product is None: + raise RuntimeError(f"{child.config.path}: the child run produced no product") record_dependencies({child.config.path, *child.dependencies}) return child.product @@ -176,7 +199,14 @@ def use(path: str | Path, /, *, id: str | None = None, **overrides: Any) -> Prod # --------------------------------------------------------------------------- -def _run(config: NotebookConfig, overrides: dict[str, Any], *, cache: bool) -> NotebookRun: +def _run( + config: NotebookConfig, + overrides: dict[str, Any], + *, + cache: bool, + through: int | None = None, + product: bool = True, +) -> NotebookRun: app = load_notebook(config.path, cell_prefix=_cell_prefix(config)) if app is None: raise NotebookConfigError(f"{config.path}: the notebook has no cells") @@ -186,6 +216,18 @@ def _run(config: NotebookConfig, overrides: dict[str, Any], *, cache: bool) -> N for cell_id in internal.cell_manager.cell_ids() if cell_id in internal.graph.cells ] + if through is not None: + if through < 1: + raise ValueError(f"through must be at least 1, got {through}") + # Counted the way the editor numbers cells: the setup cell is + # infrastructure and takes no number. Disabled is what the script + # runner filters on (graph.disable_cell only sets a runtime status); + # a cell before the cut that reads one behind it fails with its own + # NameError, which is the honest report. + setup_id = internal.cell_manager.setup_cell_id + visible = [cell for cell in ordered if cell.cell_id != setup_id] + for cell in visible[through:]: + cell.config.disabled = True _check_overrides(config, internal, ordered, overrides) cells = NotebookCells(config.path, ordered) scope = NotebookScope(config, cells) @@ -227,12 +269,14 @@ def _run(config: NotebookConfig, overrides: dict[str, Any], *, cache: bool) -> N name: glbls[name] for cell in ordered for name in cell.defs if name in glbls } read = frozenset().union(*(cell.refs for cell in ordered)) - product = project_product( - find_product(config, values, read), - config=config, - cells=cells, - values=values.values(), - ) + product_value: Product | None = None + if product: + product_value = project_product( + find_product(config, values, read), + config=config, + cells=cells, + values=values.values(), + ) # Cells restored from the cache ran no ``use`` this time; the files they # depend on are among those the previous run recorded. @@ -266,13 +310,43 @@ def _run(config: NotebookConfig, overrides: dict[str, Any], *, cache: bool) -> N ) return NotebookRun( config=config, - product=product, + product=product_value, values=values, cells=reports, dependencies=dependencies, ) +def _with_timeout(run: Callable[[], NotebookRun], timeout: float) -> NotebookRun: + """Put a wall clock around *run*. + + The worker thread is a daemon: a run that ignores the deadline (native + code deep in a fillet) cannot keep the process alive past it. + """ + import threading + + outcome: list[NotebookRun] = [] + failure: list[BaseException] = [] + + def work() -> None: + try: + outcome.append(run()) + except BaseException as error: # re-raised on the caller's thread + failure.append(error) + + worker = threading.Thread(target=work, daemon=True, name="notebook-run") + worker.start() + worker.join(timeout) + if worker.is_alive(): + raise TimeoutError( + f"the notebook did not finish in {timeout:g}s (pass a larger " + "timeout; heavy features can take minutes)" + ) + if failure: + raise failure[0] + return outcome[0] + + def _execute( internal: InternalApp, scope: NotebookScope, diff --git a/test/test_notebook_runtime.py b/test/test_notebook_runtime.py index 563b172..88a7b66 100644 --- a/test/test_notebook_runtime.py +++ b/test/test_notebook_runtime.py @@ -766,3 +766,54 @@ def test_errors_are_reported_not_raised(self) -> None: if __name__ == "__main__": unittest.main() + + +class TestPartialRuns(NotebookTestCase): + """Validation of a stage: a prefix of the cells, no product required, a wall clock.""" + + def setUp(self) -> None: + super().setUp() + self.bracket = self.write("bracket.py", BRACKET) + self.headless = self.write( + "headless.py", + notebook_text( + cell("width = 20.0", returns="width"), + cell("block = scad.make_box_rsolid(width, 10.0, 5.0)", reads="width", returns="block"), + table='id = "headless"', + ), + ) + self.sleepy = self.write( + "sleepy.py", + notebook_text( + cell("import time\ntime.sleep(30)\nwidth = 1.0", returns="width"), + table='id = "sleepy"', + ), + ) + + def test_through_runs_the_first_cells_and_marks_the_rest_skipped(self) -> None: + run = run_notebook(self.bracket, through=2, product=False) + # setup + the first two visible cells; the tail is named skipped + self.assertEqual(self.statuses(run), ["ran", "ran", "ran", "skipped"]) + self.assertIsNone(run.product) + with self.assertRaisesRegex(ValueError, "no product"): + run.definition + + def test_through_counts_cells_the_way_the_editor_numbers_them(self) -> None: + # the setup cell is infrastructure and takes no number + run = run_notebook(self.bracket, through=1, product=False) + self.assertEqual(self.statuses(run), ["ran", "ran", "skipped", "skipped"]) + with self.assertRaisesRegex(ValueError, "through must be at least 1"): + run_notebook(self.bracket, through=0) + + def test_product_false_runs_a_notebook_that_has_no_product_yet(self) -> None: + from simplecadapi.runtime import ProductNotFoundError + + with self.assertRaises(ProductNotFoundError): + run_notebook(self.headless, cache=False) + run = run_notebook(self.headless, cache=False, product=False) + self.assertEqual(self.statuses(run), ["ran", "ran", "ran"]) + self.assertIsNone(run.product) + + def test_timeout_fails_the_run_and_names_the_knob(self) -> None: + with self.assertRaisesRegex(TimeoutError, "timeout"): + run_notebook(self.sleepy, timeout=0.5) From 29db7680114396ebcb410ffa2c968a26bdddfefc Mon Sep 17 00:00:00 2001 From: NiJingzhe <1448976235@qq.com> Date: Thu, 15 Oct 2026 20:02:05 +0800 Subject: [PATCH 3/4] docs(inspect+skill): a summary repr for BRepInspection; sca-cell-only wording; git discipline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Printing an inspection now shows a one-line digest (the per-face records are to_dict()/.faces). The skill says what an agent in a shared editor needed told: while an editor runs, sca cell is the only way to change a regular cell — the file only when no editor runs or for the cells sca cell refuses. dof=0 says the constraint system closed, not that the shape is right (mirror solutions); verify by measurement. And a modeling project is managed like software: git from the first file, commits at verified milestones, generated state ignored. Co-Authored-By: Claude Sonnet 5.5 --- docs/skill/SKILL.md | 14 +++++++++++--- .../references/domains/editor-collaboration.md | 14 ++++++++++---- .../references/domains/sketch-and-features.md | 4 ++++ src/simplecadapi/inspect/brep/inspect.py | 17 +++++++++++++++++ test/test_inspect_brep.py | 15 +++++++++++++++ 5 files changed, 57 insertions(+), 7 deletions(-) diff --git a/docs/skill/SKILL.md b/docs/skill/SKILL.md index 12ee87b..d6eeb57 100644 --- a/docs/skill/SKILL.md +++ b/docs/skill/SKILL.md @@ -50,9 +50,10 @@ Paths are relative to this file's directory. selected, and you MUST read `references/domains/editor-collaboration.md` first to see what they see (selection cell, `sca snapshot`, cell references) and how to - hand the work back. While the editor runs, change its notebooks with - `sca cell` (or the file; never both blindly): the editor is the - file's writer. + hand the work back. While the editor runs, `sca cell` is the only way to + change a regular cell; edit the file only when no editor is running, or + for the setup / `@app.function` cells `sca cell` refuses: the editor is + the file's writer. - Creating, installing, updating, or removing a SimpleCADAPI addon (a third-party skill+tooling package around `.scadpkg`) enters no workflow; you MUST use `references/domains/addon-development.md` @@ -147,6 +148,13 @@ it and stop that route; never invent an alternative route. them in the editor for a person); never enable marimo's `[tool.marimo.runtime] cache_cells` in the editor, and never commit the generated `__marimo__/` directory. +12. Manage the modeling project the way software is managed: put the + workspace under git from the first file, commit at verified + milestones (a passed verification, not a wall clock) with messages + that say what changed and why, and use a branch per design + direction when alternatives are on the table. Generated state — + `__marimo__/`, `.sca-editor/`, `out/` — belongs in `.gitignore`, + never in a commit. ## Boundaries diff --git a/docs/skill/references/domains/editor-collaboration.md b/docs/skill/references/domains/editor-collaboration.md index ab65bcd..fc32156 100644 --- a/docs/skill/references/domains/editor-collaboration.md +++ b/docs/skill/references/domains/editor-collaboration.md @@ -12,11 +12,17 @@ tuned value) is also something you can read from disk. Never rely on what the person says they selected when the file or the snapshot can tell you. The editor holds the open notebook in memory and is the one writer of its -file. You have two safe ways to change it while it runs; use the first: - -1. **`sca cell` (preferred).** It edits through the editor: the page shows the +file. While it runs, **`sca cell` is the only way to change a regular cell**: +writing the file behind the editor's back races its next save, and the page's +version or yours is lost. Edit the file directly only when no editor is +running (then nothing else holds it), or for the cells `sca cell` refuses +(the setup cell and `@app.function` cells): the editor takes those file +changes in. + +1. **`sca cell` (always, for regular cells).** It edits through the editor: the page shows the change live (no reload, no flicker), the cell runs, and you get its state back. -2. **Edit the file.** The editor notices a saved change within a second, compares +2. **Edit the file** — only with no editor running, or for a setup / `@app.function` cell. The + editor notices a saved change within a second, compares it with its cells and applies only what differs (changed cells are edited in place and run again, added or removed cells appear or go away). A change to the setup cell or an `@app.function` cell restarts the kernel instead. A file that diff --git a/docs/skill/references/domains/sketch-and-features.md b/docs/skill/references/domains/sketch-and-features.md index b4e9350..3cfab83 100644 --- a/docs/skill/references/domains/sketch-and-features.md +++ b/docs/skill/references/domains/sketch-and-features.md @@ -81,6 +81,10 @@ Read the exact page under `references/docs/api/` for every API used: ## Validation gates - Record plane mapping, closure, solve status, and remaining DOF in the brief. +- `dof=0` says the constraint system closed, not that the shape is the intended + one: angle and symmetry constraints can solve to the mirror solution. Verify + the promoted geometry by measurement (bounding box, area, edge counts), never + by the solver status alone. - When sketch promotion may change measured geometry, build Wire and Sketch from one parameter source and compare closure/area/bounds/segmentation before promoting. diff --git a/src/simplecadapi/inspect/brep/inspect.py b/src/simplecadapi/inspect/brep/inspect.py index ad7ea49..f0f0aae 100644 --- a/src/simplecadapi/inspect/brep/inspect.py +++ b/src/simplecadapi/inspect/brep/inspect.py @@ -46,6 +46,23 @@ class BRepInspection: def to_dict(self) -> dict[str, Any]: return asdict(self) + def summary(self) -> str: + """One digest line: the facts an agent checks first, no per-face dump. + + The full record (tens of thousands of characters on a real part) is + ``.to_dict()`` / the ``faces`` / ``edges`` lists. + """ + box = self.bounding_box + size = [round(box[i + 3] - box[i], 3) for i in range(3)] + return ( + f"" + ) + + def __repr__(self) -> str: + return self.summary() + def write_json(self, path: str | Path, *, indent: int = 2) -> Path: output = Path(path) output.parent.mkdir(parents=True, exist_ok=True) diff --git a/test/test_inspect_brep.py b/test/test_inspect_brep.py index 5e8f521..3f1b0b3 100644 --- a/test/test_inspect_brep.py +++ b/test/test_inspect_brep.py @@ -706,3 +706,18 @@ def test_step_model_combines_multiple_transferred_roots(tmp_path: Path): assert model.summary()["volume"] == pytest.approx(48.0) brep.clear_step_model_cache_rnone() + + +def test_inspection_repr_is_a_summary_not_a_face_dump(): + """The default repr is a digest: printing an inspection in a REPL must + not spill the per-face records (tens of thousands of characters on a + real part). The full record is to_dict() / .faces / .edges.""" + inspection = brep.inspect_shape_rbrepinspection(_box().wrapped) + text = repr(inspection) + assert "valid=True" in text + assert "unique_faces" in text + assert "uv_bounds" not in text + assert len(text) < 600 + # the full record is untouched + assert len(inspection.faces) == 6 + assert inspection.to_dict()["faces"] == inspection.faces From b1a5ff44b3b8197df1606a33656a8425a8028cec Mon Sep 17 00:00:00 2001 From: NiJingzhe <1448976235@qq.com> Date: Thu, 15 Oct 2026 20:11:10 +0800 Subject: [PATCH 4/4] =?UTF-8?q?fix(editor+run):=20review=20follow-ups=20?= =?UTF-8?q?=E2=80=94=20timeout=20is=20opt-in=20at=20the=20CLI,=20a=20write?= =?UTF-8?q?=20that=20outlives=20its=20call=20says=20the=20edit=20landed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sca run --timeout now defaults to no limit (the runner's own default): a command that never timed out does not grow a hard failure; --timeout 240 was verified against a real fillet notebook through the worker thread. sca cell write calls time out with "the edit was applied and the rerun is still going; check sca cell ls" instead of a raw socket error, and the wait is SCA_EDITOR_CALL_TIMEOUT (default 300s). A server.json whose pid is gone reports the stale record and the remedy, not an unreachable url. Co-Authored-By: Claude Sonnet 5.5 --- src/simplecadapi/editor/cli.py | 44 +++++++++++++++++++++++++++++---- src/simplecadapi/runtime/cli.py | 6 ++--- test/test_editor.py | 25 +++++++++++++++++++ 3 files changed, 67 insertions(+), 8 deletions(-) diff --git a/src/simplecadapi/editor/cli.py b/src/simplecadapi/editor/cli.py index 8368c7d..22acf6e 100644 --- a/src/simplecadapi/editor/cli.py +++ b/src/simplecadapi/editor/cli.py @@ -171,9 +171,11 @@ def _find_editor(start: Path) -> tuple[Path, str] | None: A record whose server does not answer is an error, not a stale record to skip quietly: "an editor was running here" is exactly the belief that - must not be dropped in silence. + must not be dropped in silence. The one definite stale case is a record + whose process is gone (a crashed editor leaves its server.json behind). """ import json + import os import urllib.error here = start.expanduser().resolve() @@ -184,10 +186,22 @@ def _find_editor(start: Path) -> tuple[Path, str] | None: if not record.is_file(): continue try: - url = json.loads(record.read_text(encoding="utf-8"))["url"] + body = json.loads(record.read_text(encoding="utf-8")) + url = body["url"] except (OSError, ValueError, KeyError) as error: unreachable.append(f"{record}: unreadable ({error})") continue + pid = body.get("pid") + if isinstance(pid, int): + try: + os.kill(pid, 0) + except ProcessLookupError: + unreachable.append( + f"stale record {record} (pid {pid} is gone); delete it or restart the editor" + ) + continue + except OSError: + pass # a live process we cannot signal try: with opener.open(f"{url}/api/health", timeout=1.5): pass @@ -202,8 +216,17 @@ def _find_editor(start: Path) -> tuple[Path, str] | None: return None -def _call(url: str, path: str, body: dict | None = None) -> dict[str, Any]: +def _call_timeout() -> float: + """How long one editor call may take; a heavy downstream rerun is on the + clock of the write that triggered it. ``SCA_EDITOR_CALL_TIMEOUT`` seconds.""" + import os + + return float(os.environ.get("SCA_EDITOR_CALL_TIMEOUT", "300")) + + +def _call(url: str, path: str, body: dict | None = None, timeout: float | None = None) -> dict[str, Any]: import json + import socket import urllib.error import urllib.request @@ -211,14 +234,17 @@ def _call(url: str, path: str, body: dict | None = None) -> dict[str, Any]: request = urllib.request.Request( url + path, data=data, headers={"Content-Type": "application/json"}, method="GET" if body is None else "POST" ) + seconds = _call_timeout() if timeout is None else timeout try: - with _opener().open(request, timeout=300) as response: + with _opener().open(request, timeout=seconds) as response: return json.loads(response.read()) except urllib.error.HTTPError as error: try: return json.loads(error.read()) except ValueError: return {"error": f"editor answered {error.code}"} + except (socket.timeout, TimeoutError) as error: + raise TimeoutError(f"the editor did not answer {path} within {seconds:g}s") from error def _session(args: argparse.Namespace, *, write: bool) -> tuple[str | None, Path | None, str]: @@ -328,7 +354,15 @@ def _read_code(args: argparse.Namespace) -> str: def _write_op(args: argparse.Namespace, op: str, body: dict[str, Any]) -> tuple[dict[str, Any], int]: url, _notebook, _name = _session(args, write=True) assert url is not None - report = _call(url, f"/api/cells/{op}", body) + try: + report = _call(url, f"/api/cells/{op}", body) + except TimeoutError: + # The server took the write and reruns on its own; the clock that ran + # out is this call's, not the editor's. + return { + "error": "the edit was applied and the rerun is still going; check " + "`sca cell ls` (SCA_EDITOR_CALL_TIMEOUT raises the wait)" + }, 1 if "error" in report: return _slim(report, code=False), 1 touched = set(report.get("touched", [])) diff --git a/src/simplecadapi/runtime/cli.py b/src/simplecadapi/runtime/cli.py index f07af6d..7db5ee1 100644 --- a/src/simplecadapi/runtime/cli.py +++ b/src/simplecadapi/runtime/cli.py @@ -69,10 +69,10 @@ def configure(subparsers: argparse._SubParsersAction) -> None: parser.add_argument( "--timeout", type=float, - default=120.0, + default=None, metavar="SECONDS", - help="fail the run when it takes longer (default: 120; heavy features " - "can take minutes)", + help="fail the run when it takes longer (default: no limit; heavy " + "features can take minutes)", ) parser.set_defaults(handler=_execute, printer=_print_report) diff --git a/test/test_editor.py b/test/test_editor.py index 24595c4..d320189 100644 --- a/test/test_editor.py +++ b/test/test_editor.py @@ -1129,6 +1129,31 @@ def test_a_stale_record_is_an_error_that_names_the_record_and_url(self) -> None: with self.assertRaisesRegex(ValueError, "server.json.*17612.*unreachable"): _find_editor(self.root) + def test_a_record_whose_process_is_gone_names_the_stale_pid(self) -> None: + from simplecadapi.editor.cli import _find_editor + + directory = self.root / ".sca-editor" + directory.mkdir() + (directory / "server.json").write_text(json.dumps({"url": "http://127.0.0.1:17615", "pid": 2**22})) + with self.assertRaisesRegex(ValueError, r"pid \d+ is gone"): + _find_editor(self.root) + + def test_a_write_that_outlives_its_call_says_the_edit_landed(self) -> None: + import argparse + from unittest import mock + + from simplecadapi.editor import cli + + args = argparse.Namespace(notebook=None, workspace=self.root, cell="1", file=None, no_run=False) + with mock.patch("simplecadapi.editor.cli._session", return_value=("http://127.0.0.1:1", None, "nb.py")): + with mock.patch( + "simplecadapi.editor.cli._call", side_effect=TimeoutError("the editor did not answer") + ): + report, code = cli._write_op(args, "edit", {"cell": "c1", "code": "x = 1"}) + self.assertEqual(code, 1) + self.assertIn("the edit was applied", report["error"]) + self.assertIn("sca cell ls", report["error"]) + def test_the_health_check_reaches_a_local_editor_through_a_proxy_environment(self) -> None: import threading