mirror of
https://github.com/open-jarvis/OpenJarvis.git
synced 2026-07-27 21:05:34 +00:00
fix(cli): don't let a broken numpy crash CLI/server startup on Windows (#433)
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
f1b0df6b6b
commit
ad7c86495f
@@ -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")
|
||||
|
||||
|
||||
@@ -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__ = [
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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}"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user