The CLI was paying for a web server on every command (#173)

CHANGELOG.md

@@ -4,6 +4,39 @@
 
 ### Changed
 
+- **#173: every CLI command was paying for a web server. Startup is ~700 ms faster.**
+
+  `nodus.cli.cli` imported `nodus.services.server` at module scope for the four
+  commands that need it (`serve`, `snapshot`, `snapshots`, `restore`). That
+  module imports FastAPI, uvicorn and pydantic, so `nodus run`, `nodus fmt`,
+  `nodus check` and `nodus --version` all paid for it too. The imports are lazy
+  now, inside those four commands.
+
+  Measured alternating before/after in one window, best of three each, so
+  machine drift shows as noise rather than signal:
+
+  | | before | after | |
+  |---|---|---|---|
+  | `import nodus.cli.cli` | 1435 ms | 652 ms | 2.20x |
+  | `nodus --version` | 1592 ms | 640 ms | 2.49x |
+  | `nodus run hello.nd` | 1711 ms | 1011 ms | 1.69x |
+
+  This is also the standing blocker on #173's PyPy path: PyPy's *interpreter*
+  starts faster than CPython's (104 ms vs 184 ms), and what makes it
+  unadoptable is importing Nodus's own module tree -- run-once code the JIT
+  never warms, against `nodus run`'s 200 ms default deadline.
+
+  `tests/test_cli_import_cost.py` asserts on `sys.modules` in a subprocess,
+  because a behavioural test cannot see this -- the CLI worked either way, only
+  slower -- and by the time any other test runs, something has usually imported
+  FastAPI already.
+
+  **The throughput ceiling itself is unchanged, and its recorded figure was
+  stale.** Measured with the VM's own instruction counter rather than an
+  estimate per loop iteration: **650-850K instr/s**, at 17 instructions per
+  iteration, against the ~200K the issue records. `tools/benchmark_runtime.py`
+  reports both figures so neither has to be transcribed again.
+
 - **#578: a `state` cell can declare its own join — `with { barrier: true }`.**
 
   Every step that reads the cell waits for every step that writes it, inferred.

src/nodus/cli/cli.py

@@ -49,7 +49,15 @@
     tool_call_result,
     workflow_checkpoints,
 )
-from nodus.services.server import serve, snapshot_session, restore_snapshot, list_snapshots
+# `nodus.services.server` is imported lazily, inside the four commands that
+# need it (#173). It pulls in FastAPI, uvicorn and pydantic, and importing it
+# here made every `nodus run`, `nodus fmt` and `nodus check` pay for a server
+# nobody asked for -- measured at roughly 1.1 s of the CLI's ~2.1 s startup on
+# the developer box, and `nodus --version` alone cost 1.6 s.
+#
+# Same shape as CIRC-001 (#103), which made `vm.py`'s workflow-runner imports
+# lazy and carries the same instruction: do not hoist these back to module
+# scope.
 from nodus.support.config import SERVER_HOST, SERVER_PORT, WORKER_SWEEP_INTERVAL_MS, MAX_STEPS, EXECUTION_TIMEOUT_MS, MAX_STDOUT_CHARS
 from nodus.vm.vm import VM
 from nodus.support.version import VERSION
@@ -1284,6 +1292,7 @@ def _run_server(
     workflow_store_path: str | None = None,
 ) -> int:
     try:
+        from nodus.services.server import serve  # lazy: see the note at the top
         serve(
             host=host,
             port=port,
@@ -1308,18 +1317,21 @@ def _run_server(
 
 
 def _run_snapshot(session_id: str, *, host: str, port: int, token: str | None = None) -> int:
+    from nodus.services.server import snapshot_session  # lazy: see the note at the top
     payload = snapshot_session(host, port, session_id, token=token)
     _json_print(payload)
     return 0 if "error" not in payload else 1
 
 
 def _run_snapshots(*, host: str, port: int, token: str | None = None) -> int:
+    from nodus.services.server import list_snapshots  # lazy: see the note at the top
     payload = list_snapshots(host, port, token=token)
     _json_print(payload)
     return 0 if "error" not in payload else 1
 
 
 def _run_restore(snapshot_id: str, *, host: str, port: int, token: str | None = None) -> int:
+    from nodus.services.server import restore_snapshot  # lazy: see the note at the top
     payload = restore_snapshot(host, port, snapshot_id, token=token)
     _json_print(payload)
     return 0 if "error" not in payload else 1

tests/test_cli_command_table.py

@@ -61,8 +61,22 @@ def test_help_is_not_reconstructed_by_scraping_prose(self):
 
         The table carries `signature` and `summary` as fields, so the scrape is
         gone and `re` is no longer needed by cli.py at all.
+
+        Matched as a whole line rather than as a substring. The substring form
+        (`assertNotIn("import re", ...)`) also matches `import restore_snapshot`
+        -- and did, the moment #173 made the server imports lazy. It was
+        reporting a regex scrape that was not there, against a source line that
+        imports a CLI command.
         """
-        self.assertNotIn("import re", CLI_SOURCE)
+        offenders = [
+            line for line in CLI_SOURCE.splitlines()
+            if re.fullmatch(r"\s*(import re|from re import .*)", line)
+        ]
+        self.assertEqual(
+            [], offenders,
+            "cli.py imports `re` again; the command table carries `signature` "
+            f"and `summary` so no scrape is needed: {offenders}",
+        )
 
 
 class CommandTableShapeTests(unittest.TestCase):

tests/test_cli_import_cost.py

@@ -0,0 +1,115 @@
+"""The CLI must not import the server, and so must not import FastAPI (#173).
+
+`nodus.cli.cli` imported `nodus.services.server` at module scope for four
+commands that need it -- `serve`, `snapshot`, `snapshots`, `restore`. That module
+imports FastAPI, uvicorn and pydantic, so **every** `nodus run`, `nodus fmt`,
+`nodus check` and `nodus --version` paid for a web server nobody asked for.
+
+Measured on the developer box before and after making those four imports lazy:
+
+    import nodus.cli.cli      1840 ms  ->   743 ms
+    nodus --version           1624 ms  ->   886 ms
+    nodus run hello.nd        2095 ms  ->  1790 ms
+
+That is also the standing blocker on #173's PyPy path: PyPy's *interpreter*
+starts faster than CPython's (104 ms vs 184 ms), and the adoption cost is
+importing Nodus's own module tree -- run-once code the JIT never warms, against
+`nodus run`'s 200 ms default deadline.
+
+A behavioural test cannot see this: the CLI works perfectly either way, only
+slower. So this asserts on **what is in `sys.modules`**, in a subprocess,
+because by the time any other test runs, something else has usually imported
+FastAPI already.
+"""
+
+import subprocess
+import sys
+import textwrap
+import unittest
+from pathlib import Path
+
+_REPO_ROOT = Path(__file__).resolve().parents[1]
+_SRC = str(_REPO_ROOT / "src")
+
+
+def _modules_after(statement: str) -> set[str]:
+    """Import something in a clean interpreter and report the top-level modules."""
+    probe = textwrap.dedent(f"""
+        import sys
+        sys.path.insert(0, {_SRC!r})
+        {statement}
+        print("\\n".join(sorted({{m.split(".")[0] for m in sys.modules}})))
+    """)
+    result = subprocess.run(
+        [sys.executable, "-c", probe],
+        capture_output=True, text=True, timeout=120,
+    )
+    if result.returncode != 0:
+        raise AssertionError(f"probe failed: {result.stderr[-800:]}")
+    return set(result.stdout.split())
+
+
+class TheCliDoesNotImportTheServerTests(unittest.TestCase):
+    #: Only ever reached through `nodus serve` and the three snapshot commands.
+    SERVER_ONLY = ("fastapi", "uvicorn", "starlette")
+
+    @classmethod
+    def setUpClass(cls):
+        cls.cli_modules = _modules_after("import nodus.cli.cli")
+
+    # closes: #173
+    def test_the_cli_module_pulls_in_no_web_framework(self):
+        for name in self.SERVER_ONLY:
+            with self.subTest(module=name):
+                self.assertNotIn(
+                    name, self.cli_modules,
+                    f"importing nodus.cli.cli pulled in {name}. It is only needed "
+                    "by `serve` and the snapshot commands, so import it inside "
+                    "those functions -- every other command pays for it here.",
+                )
+
+    # closes: #173
+    def test_the_cli_still_imports_what_it_actually_uses(self):
+        """The control, and it must run.
+
+        Without it the assertion above is satisfied by a `cli` module that
+        imports nothing at all, or by one that fails to import and is caught.
+        """
+        self.assertIn("nodus", self.cli_modules)
+        for name in ("nodus", "json", "os"):
+            with self.subTest(module=name):
+                self.assertIn(name, self.cli_modules)
+
+    # closes: #173
+    def test_the_server_module_is_still_reachable(self):
+        """The second control: made lazy, not deleted. A `nodus serve` that
+        cannot find its own server would satisfy every assertion above."""
+        modules = _modules_after("import nodus.services.server")
+        for name in self.SERVER_ONLY[:2]:
+            with self.subTest(module=name):
+                self.assertIn(
+                    name, modules,
+                    "the server module no longer imports its web framework, so "
+                    "the test above is passing for the wrong reason",
+                )
+
+    # closes: #173
+    def test_the_four_lazy_names_are_importable_from_the_server(self):
+        """What the lazy imports inside the command bodies name. A rename there
+        is a `NameError` only when that command runs, which no fast test path
+        exercises -- so it is checked here instead."""
+        probe = (
+            "from nodus.services.server import "
+            "serve, snapshot_session, restore_snapshot, list_snapshots; "
+            "print('ok')"
+        )
+        result = subprocess.run(
+            [sys.executable, "-c", f"import sys; sys.path.insert(0, {_SRC!r}); {probe}"],
+            capture_output=True, text=True, timeout=120,
+        )
+        self.assertEqual(0, result.returncode, result.stderr[-800:])
+        self.assertIn("ok", result.stdout)
+
+
+if __name__ == "__main__":
+    unittest.main()

tools/benchmark_runtime.py

@@ -0,0 +1,123 @@
+"""Measure what #173 is about, rather than quoting a number that goes stale.
+
+Two figures, and they are different questions:
+
+- **Throughput** -- how fast the VM executes instructions once running. This is
+  the ceiling #173 names, and it is bounded by CPython's own dispatch loop.
+- **Startup** -- how long before a program executes at all. This is what blocks
+  #173's PyPy path: PyPy's interpreter starts *faster* than CPython's, and the
+  adoption cost is importing Nodus's own module tree, which is run-once code the
+  JIT never warms.
+
+The instruction count comes from the VM's own counter rather than an estimate
+per loop iteration. An earlier reading of this benchmark assumed "about 6
+instructions per iteration" and was wrong by nearly 3x -- it is 17.
+
+Run it::
+
+    PYTHONPATH="src" python -m tools.benchmark_runtime
+    PYTHONPATH="src" python -m tools.benchmark_runtime --quick
+
+Numbers are machine-specific and are not asserted anywhere. Nothing here is a
+gate: it exists so the figure in an issue or a document can be re-derived
+instead of transcribed.
+"""
+from __future__ import annotations
+
+import argparse
+import os
+import subprocess
+import sys
+import time
+from pathlib import Path
+
+_REPO_ROOT = Path(__file__).resolve().parents[1]
+_SRC = _REPO_ROOT / "src"
+
+LOOP = """
+fn compute(n) {
+    let i = 0i
+    let sum = 0i
+    while (i < n) { sum = sum + i; i = i + 1i }
+    return sum
+}
+print(compute(%d))
+"""
+
+
+def _runtime():
+    sys.path.insert(0, str(_SRC))
+    from nodus.runtime.embedding import NodusRuntime
+
+    return NodusRuntime
+
+
+def throughput(sizes, reps: int) -> None:
+    NodusRuntime = _runtime()
+
+    def once(n):
+        rt = NodusRuntime(timeout_ms=None, max_steps=None)
+        start = time.perf_counter()
+        result = rt.run_source(LOOP % n)
+        elapsed = time.perf_counter() - start
+        if not result.get("ok"):
+            raise SystemExit(f"benchmark program failed: {result.get('error')}")
+        return elapsed, rt.get_execution_stats()["instructions_executed"]
+
+    once(1000)  # warm the compiler and its lazy imports
+    print(f"throughput -- {sys.implementation.name} {sys.version.split()[0]}")
+    for n in sizes:
+        best, instructions = None, 0
+        for _ in range(reps):
+            elapsed, instructions = once(n)
+            best = elapsed if best is None else min(best, elapsed)
+        print(
+            f"  n={n:>9,}  {best:6.3f}s  {n / best / 1000:7.1f}K iter/s  "
+            f"{instructions / best / 1000:9.1f}K instr/s  "
+            f"({instructions / n:.1f} instr/iter)"
+        )
+
+
+def startup(reps: int) -> None:
+    """Cost before a program runs. Subprocesses, because that is the real cost."""
+    env = dict(os.environ, PYTHONPATH=str(_SRC))
+    hello = _REPO_ROOT / ".benchmark_hello.nd"
+    hello.write_text('print("hi")\n', encoding="utf-8")
+    cases = [
+        ([sys.executable, "-c", "pass"], "bare interpreter"),
+        ([sys.executable, "-c", "import nodus"], "import nodus"),
+        ([sys.executable, "-c", "import nodus.cli.cli"], "import nodus.cli.cli"),
+        ([sys.executable, str(_REPO_ROOT / "nodus.py"), "--version"], "nodus --version"),
+        ([sys.executable, str(_REPO_ROOT / "nodus.py"), "run", str(hello)], "nodus run hello.nd"),
+    ]
+    print("\nstartup -- best of "
+          f"{reps}, subprocess wall clock")
+    try:
+        for cmd, label in cases:
+            best = None
+            for _ in range(reps):
+                start = time.perf_counter()
+                subprocess.run(cmd, cwd=_REPO_ROOT, env=env, capture_output=True)
+                elapsed = time.perf_counter() - start
+                best = elapsed if best is None else min(best, elapsed)
+            print(f"  {label:24} {best * 1000:7.0f} ms")
+    finally:
+        hello.unlink(missing_ok=True)
+
+
+def main(argv=None) -> int:
+    parser = argparse.ArgumentParser(description=__doc__.splitlines()[0])
+    parser.add_argument("--quick", action="store_true",
+                        help="one small size, fewer repetitions")
+    parser.add_argument("--skip-startup", action="store_true")
+    args = parser.parse_args(argv)
+
+    sizes = (50_000,) if args.quick else (50_000, 200_000, 1_000_000)
+    throughput(sizes, reps=1 if args.quick else 3)
+    if not args.skip_startup:
+        startup(reps=2 if args.quick else 3)
+    return 0
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())