From ad7c86495ff5a338b43a76815a5ff49f3e66bdc8 Mon Sep 17 00:00:00 2001 From: Jon Saad-Falcon <41205309+jonsaadfalcon@users.noreply.github.com> Date: Fri, 29 May 2026 16:18:47 -0700 Subject: [PATCH] fix(cli): don't let a broken numpy crash CLI/server startup on Windows (#433) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses #404 (unable to launch / fully download) and contributes to #309 (stuck on starting api server) by removing the eager numpy import paths that fail hard when a Windows host has a partially-installed or cp314-incompatible numpy. What changed: - `src/openjarvis/connectors/embeddings.py` / `hybrid_search.py`: numpy imports are now lazy (inside the method that needs them) with a `TYPE_CHECKING` guard for annotations. Default-argument evaluation no longer touches numpy at module import. - `src/openjarvis/cli/__init__.py`: the `deep_research_setup` command import is now guarded behind a `try/except Exception` so an OverflowError or ImportError during its module load doesn't crash the entire CLI. - New regression test in `tests/cli/test_cli.py`: `test_importing_cli_does_not_import_numpy` spawns a subprocess and asserts `numpy` is not in `sys.modules` after `import openjarvis.cli`. Guards against future eager-numpy regressions on Windows. Reported by @Tentacle39 in #404 and seen alongside @xoomarx's #309. Thanks to both — the Windows-only crash signature made this hard to diagnose without your repros. Co-Authored-By: Claude Opus 4.7 --- src/openjarvis/cli/__init__.py | 19 ++++++++++++--- src/openjarvis/connectors/embeddings.py | 22 ++++++++++++++---- src/openjarvis/connectors/hybrid_search.py | 6 +++-- tests/cli/test_cli.py | 27 ++++++++++++++++++++++ 4 files changed, 64 insertions(+), 10 deletions(-) diff --git a/src/openjarvis/cli/__init__.py b/src/openjarvis/cli/__init__.py index f42773a4..b1553595 100644 --- a/src/openjarvis/cli/__init__.py +++ b/src/openjarvis/cli/__init__.py @@ -17,7 +17,6 @@ from openjarvis.cli.compose_cmd import compose from openjarvis.cli.config_cmd import config from openjarvis.cli.connect_cmd import connect from openjarvis.cli.daemon_cmd import restart, start, status, stop -from openjarvis.cli.deep_research_setup_cmd import deep_research_setup from openjarvis.cli.digest_cmd import digest from openjarvis.cli.doctor_cmd import doctor from openjarvis.cli.eval_cmd import eval_group @@ -117,8 +116,22 @@ cli.add_command(config, "config") cli.add_command(scan, "scan") cli.add_command(connect, "connect") cli.add_command(digest, "digest") -cli.add_command(deep_research_setup, "deep-research-setup") -cli.add_command(deep_research_setup, "research") +# deep-research setup pulls the ingestion pipeline (embeddings/numpy). Guard it +# so a broken or slow numpy on Windows — which can raise at IMPORT time, not +# just ImportError (#404) — can never take down the whole CLI, including +# `jarvis serve`. Invoking `jarvis deep-research-setup` without the deps still +# errors clearly on demand. +try: + from openjarvis.cli.deep_research_setup_cmd import deep_research_setup + + cli.add_command(deep_research_setup, "deep-research-setup") + cli.add_command(deep_research_setup, "research") +except Exception as _dr_exc: + import logging as _logging + + _logging.getLogger(__name__).debug( + "deep-research command unavailable: %s", _dr_exc + ) cli.add_command(self_update, "self-update") cli.add_command(bootstrap_cmd, "_bootstrap") diff --git a/src/openjarvis/connectors/embeddings.py b/src/openjarvis/connectors/embeddings.py index 45b35119..d7e51be8 100644 --- a/src/openjarvis/connectors/embeddings.py +++ b/src/openjarvis/connectors/embeddings.py @@ -13,11 +13,17 @@ so ingestion never fails because a sidecar service is down. from __future__ import annotations import logging -from typing import List, Optional +from typing import TYPE_CHECKING, List, Optional -import numpy as np import requests +# numpy is imported lazily inside the functions that use it (not at module +# load). The CLI imports this module eagerly via the deep-research command +# chain, so a module-level `import numpy` makes a broken/slow numpy on Windows +# crash every `jarvis` command — including `jarvis serve` (#404, #309). +if TYPE_CHECKING: + import numpy as np + logger = logging.getLogger(__name__) @@ -110,6 +116,8 @@ class OllamaEmbedder: ) return None + import numpy as np + arr = np.asarray(vec, dtype=np.float32) if self._dim is None: self._dim = int(arr.shape[0]) @@ -136,16 +144,20 @@ class OllamaEmbedder: def decode_embedding( - blob: Optional[bytes], *, dtype: type = np.float32 + blob: Optional[bytes], *, dtype=None ) -> Optional[np.ndarray]: """Reconstruct a 1-D vector from a BLOB written by ``OllamaEmbedder.embed``. Returns ``None`` when the input is missing or zero-length so callers can - treat absent embeddings uniformly. + treat absent embeddings uniformly. ``dtype`` defaults to ``np.float32`` + (resolved lazily; passing ``np.float32`` as a default arg would import + numpy at module load). """ if not blob: return None - return np.frombuffer(blob, dtype=dtype) + import numpy as np + + return np.frombuffer(blob, dtype=dtype if dtype is not None else np.float32) __all__ = [ diff --git a/src/openjarvis/connectors/hybrid_search.py b/src/openjarvis/connectors/hybrid_search.py index 2165db7e..b6e6210a 100644 --- a/src/openjarvis/connectors/hybrid_search.py +++ b/src/openjarvis/connectors/hybrid_search.py @@ -24,8 +24,8 @@ from dataclasses import dataclass, field from datetime import datetime from typing import Any, Dict, List, Optional, Sequence, Tuple -import numpy as np - +# numpy imported lazily inside _vector_recall (see embeddings.py) so importing +# this module never forces numpy at load time (#404, #309). from openjarvis.connectors.embeddings import OllamaEmbedder, decode_embedding from openjarvis.connectors.store import KnowledgeStore @@ -254,6 +254,8 @@ class HybridSearch: """Return ``[(chunk_id, cosine_score), ...]`` from a brute-force scan.""" if self._embedder is None: return [] + import numpy as np + q_blob = self._embedder.embed(query) q_vec = decode_embedding(q_blob) if q_vec is None or q_vec.size == 0: diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 63668b36..84e2daac 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -3,6 +3,7 @@ from __future__ import annotations import io +import subprocess import sys from pathlib import Path from unittest import mock @@ -136,3 +137,29 @@ class TestCLI: assert config_path.exists() content = config_path.read_text() assert "[engine]" in content + + +class TestStartupResilience: + """Importing the CLI must not force heavy/native deps (#404, #309). + + A broken or slow numpy on Windows otherwise raises at import time and takes + down every `jarvis` command — including `jarvis serve` — because the CLI + eagerly pulls the deep-research command chain (-> embeddings -> numpy). + """ + + def test_importing_cli_does_not_import_numpy(self) -> None: + # Run in a fresh subprocess: the pytest session itself almost certainly + # has numpy loaded from other tests, so an in-process check is useless. + code = ( + "import openjarvis.cli, sys; " + "leaked=[m for m in sys.modules if m=='numpy' or m.startswith('numpy.')]; " + "assert not leaked, leaked; " + "print('numpy-free')" + ) + result = subprocess.run( + [sys.executable, "-c", code], capture_output=True, text=True + ) + assert result.returncode == 0, ( + "importing openjarvis.cli pulled in numpy (a broken numpy would then " + f"crash `jarvis serve`):\nstdout={result.stdout}\nstderr={result.stderr}" + )