From 5e38b6428e6f8a1effe654db5c7e28b5f0e55bf6 Mon Sep 17 00:00:00 2001 From: Ben Shaharizad Date: Sat, 19 Sep 2026 00:06:15 +0300 Subject: [PATCH 1/2] =?UTF-8?q?W5b-3a:=20doctor=20venv=20checks=20?= =?UTF-8?q?=E2=80=94=20conda=20interpreter=20guard,=202s=20subprocess=20pr?= =?UTF-8?q?obe,=20editable-install=20checkout=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- jevmlx/doctor.py | 109 +++++++++++++++++++++++++++++++++++++++++- tests/test_doctor.py | 111 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 218 insertions(+), 2 deletions(-) diff --git a/jevmlx/doctor.py b/jevmlx/doctor.py index a1c500b..eab040c 100644 --- a/jevmlx/doctor.py +++ b/jevmlx/doctor.py @@ -9,7 +9,8 @@ - OK: the probe ran and the condition holds. - WARN: not fatal for an issue report, but degrades comparability (battery, low memory, stale mlx-lm, HF unreachable, tokenizer without system role). -- FAIL: the environment cannot run jevmlx at all (wrong platform, no Metal). +- FAIL: the environment cannot run jevmlx at all (wrong platform, no Metal, + conda interpreter / hung subprocess, editable install from another tree). """ from __future__ import annotations @@ -21,12 +22,13 @@ import re import subprocess import sys +import urllib.parse import urllib.request from pathlib import Path from jevmlx.models import DEFAULT_MODEL, resolve_model -__all__ = ["Check", "doctor_checks", "run_doctor"] +__all__ = ["Check", "doctor_checks", "run_doctor", "check_venv", "check_editable_install"] @dataclasses.dataclass @@ -144,6 +146,107 @@ def check_python_and_versions(env: dict) -> list[Check]: return checks +def check_venv() -> list[Check]: + """The running interpreter must not be the hanging conda one, and a + trivial subprocess must return within 2 s. + + The conda Python on this machine hangs on import (see CONTRIBUTING's + environment rules), so ``sys.base_prefix`` under a miniconda/anaconda + path is a FAIL: run jevmlx from a uv venv instead. The subprocess probe + catches the same hang from the outside: any Python that cannot run + ``python -c print(1)`` within 2 s cannot run jevmlx. + """ + checks: list[Check] = [] + base = sys.base_prefix + lowered = base.lower() + if "miniconda" in lowered or "anaconda" in lowered: + checks.append( + _fail( + "venv", + f"interpreter is conda ({base})", + "uv venv --python-preference only-managed --python 3.12", + ) + ) + else: + checks.append(_ok("venv", f"not conda ({base})")) + try: + subprocess.run( + [sys.executable, "-c", "print(1)"], + capture_output=True, + text=True, + timeout=2.0, + check=True, + ) + except (OSError, subprocess.TimeoutExpired, subprocess.CalledProcessError) as e: + detail = f"subprocess python -c print(1) failed: {type(e).__name__}" + fix = "uv venv --python-preference only-managed --python 3.12" + checks.append(_fail("venv", detail, fix)) + else: + checks.append(_ok("venv", "subprocess python -c print(1) returned within 2 s")) + return checks + + +def check_editable_install() -> Check: + """The venv's jevmlx editable install must point at this checkout. + + Reads ``direct_url.json`` from the installed jevmlx dist-info (written + by the editable install itself): its ``url`` must be the current + checkout, otherwise the venv imports a different jevmlx tree than the + one being tested or benchmarked. + """ + checkout = Path(__file__).resolve().parent.parent + try: + dist = importlib.metadata.distribution("jevmlx") + raw = dist.read_text("direct_url.json") + except importlib.metadata.PackageNotFoundError: + return _fail( + "editable-install", + "jevmlx is not installed in this environment", + "uv pip install -e '.[dev]'", + ) + if not raw: + return _fail( + "editable-install", + "no direct_url.json (jevmlx not installed editable)", + "uv pip install -e '.[dev]'", + ) + try: + url = json.loads(raw).get("url", "") + except json.JSONDecodeError: + return _fail( + "editable-install", + "direct_url.json is not valid JSON", + "uv pip install -e '.[dev]'", + ) + installed = _direct_url_path(url) + if installed is None: + return _fail( + "editable-install", + f"direct_url.json url is not a local path: {url}", + "uv pip install -e '.[dev]'", + ) + if installed.resolve() != checkout.resolve(): + return _fail( + "editable-install", + f"venv installs jevmlx from {installed}, not this checkout ({checkout})", + "uv pip install -e '.[dev]' from the checkout you are testing", + ) + return _ok("editable-install", f"editable install -> {installed}") + + +def _direct_url_path(url: str) -> Path | None: + """Local filesystem path from a direct_url.json url, None otherwise. + + file:// URLs are converted; plain /absolute/path strings pass through. + """ + if url.startswith("file://"): + parsed = urllib.parse.urlparse(url) + return Path(urllib.parse.unquote(parsed.path)) + if url.startswith("/"): + return Path(url) + return None + + def check_memory(env: dict) -> Check: """Unified memory total (WARN < 16 GB) and currently free (vm_stat).""" total_gb = env.get("ram_gb") @@ -346,6 +449,8 @@ def doctor_checks(model: str | None = None) -> tuple[list[Check], dict]: env = environment() checks: list[Check] = [check_platform(env)] checks += check_python_and_versions(env) + checks += check_venv() + checks.append(check_editable_install()) checks.append(check_memory(env)) checks.append(check_power()) checks.append(check_metal()) diff --git a/tests/test_doctor.py b/tests/test_doctor.py index 2bb56b1..4e68740 100644 --- a/tests/test_doctor.py +++ b/tests/test_doctor.py @@ -7,11 +7,14 @@ import json import platform +import subprocess +import types import pytest from jevmlx import doctor from jevmlx.doctor import ( + check_editable_install, check_hf_cache, check_memory, check_metal, @@ -19,6 +22,7 @@ check_network, check_platform, check_power, + check_venv, doctor_checks, run_doctor, ) @@ -369,3 +373,110 @@ def test_table_lists_fix_for_warn(self, healthy, capsys): ) run_doctor(as_json=False) assert "fix: plug in" in capsys.readouterr().out + + +class TestVenv: + def test_non_conda_ok_and_subprocess_ok(self): + checks = check_venv() + assert [c.status for c in checks] == ["OK", "OK"] + assert "not conda" in checks[0].detail + + def test_conda_base_prefix_fails_with_uv_fix(self, monkeypatch): + monkeypatch.setattr(doctor.sys, "base_prefix", "/opt/miniconda3") + checks = check_venv() + venv = checks[0] + assert venv.status == "FAIL" + assert "uv venv --python-preference only-managed --python 3.12" in venv.fix + + def test_anaconda_base_prefix_fails(self, monkeypatch): + monkeypatch.setattr(doctor.sys, "base_prefix", "/opt/anaconda3") + assert check_venv()[0].status == "FAIL" + + def test_subprocess_timeout_fails(self, monkeypatch): + def hang(*args, **kwargs): + raise subprocess.TimeoutExpired(cmd="python", timeout=2) + + monkeypatch.setattr(doctor.sys, "executable", "/fake/python") + monkeypatch.setattr(doctor.subprocess, "run", hang) + checks = check_venv() + assert checks[1].status == "FAIL" + assert "2 s" in checks[1].detail or "TimeoutExpired" in checks[1].detail + assert "uv venv --python-preference only-managed --python 3.12" in checks[1].fix + + def test_subprocess_oserror_fails(self, monkeypatch): + def boom(*args, **kwargs): + raise OSError("no such interpreter") + + monkeypatch.setattr(doctor.sys, "executable", "/fake/python") + monkeypatch.setattr(doctor.subprocess, "run", boom) + assert check_venv()[1].status == "FAIL" + + def test_subprocess_nonzero_exit_fails(self, monkeypatch): + def fail(*args, **kwargs): + raise subprocess.CalledProcessError(returncode=1, cmd="python") + + monkeypatch.setattr(doctor.sys, "executable", "/fake/python") + monkeypatch.setattr(doctor.subprocess, "run", fail) + assert check_venv()[1].status == "FAIL" + + +class TestEditableInstall: + def test_points_at_current_checkout_ok(self): + check = check_editable_install() + assert check.status == "OK" + assert "editable install" in check.detail + + def test_other_checkout_fails(self, monkeypatch, tmp_path): + fake = tmp_path / "other-checkout" + fake.mkdir() + payload = json.dumps({"url": fake.as_uri(), "dir_info": {"editable": True}}) + monkeypatch.setattr( + doctor.importlib.metadata, + "distribution", + lambda name: types.SimpleNamespace(read_text=lambda _n: payload), + ) + check = check_editable_install() + assert check.status == "FAIL" + assert "not this checkout" in check.detail + + def test_not_installed_fails(self, monkeypatch): + def raise_pnf(name): + raise doctor.importlib.metadata.PackageNotFoundError(name) + + monkeypatch.setattr(doctor.importlib.metadata, "distribution", raise_pnf) + check = check_editable_install() + assert check.status == "FAIL" + assert "uv pip install -e" in check.fix + + def test_no_direct_url_fails(self, monkeypatch): + monkeypatch.setattr( + doctor.importlib.metadata, + "distribution", + lambda name: types.SimpleNamespace(read_text=lambda _n: None), + ) + check = check_editable_install() + assert check.status == "FAIL" + + def test_file_url_with_spaces_unquoted(self, monkeypatch, tmp_path): + checkout = tmp_path / "my checkout" + checkout.mkdir() + monkeypatch.setattr( + doctor.importlib.metadata, + "distribution", + lambda name: types.SimpleNamespace( + read_text=lambda _n: json.dumps({"url": checkout.as_uri()}) + ), + ) + # The installed url points at a different tree than this test file's + # checkout, and the space in the path must survive unquoting. + check = check_editable_install() + assert check.status == "FAIL" + assert "my checkout" in check.detail + + +class TestAssemblyVenv: + def test_healthy_run_includes_venv_checks(self, healthy): + checks, _ = doctor_checks() + names = {c.name for c in checks} + assert "venv" in names + assert "editable-install" in names From 2a121851c6930f47bbe2ed431fbcae1d5bd3fd95 Mon Sep 17 00:00:00 2001 From: Ben Shaharizad Date: Sat, 19 Sep 2026 00:08:52 +0300 Subject: [PATCH 2/2] W5b-3a fix review F1+F2: package installs (no/invalid/non-editable direct_url.json) are OK; editable mismatch still FAILs; docstring wording de-localized --- jevmlx/doctor.py | 54 +++++++++++++++++++++----------------------- tests/test_doctor.py | 33 +++++++++++++++++++++++++-- 2 files changed, 57 insertions(+), 30 deletions(-) diff --git a/jevmlx/doctor.py b/jevmlx/doctor.py index eab040c..231071c 100644 --- a/jevmlx/doctor.py +++ b/jevmlx/doctor.py @@ -147,14 +147,14 @@ def check_python_and_versions(env: dict) -> list[Check]: def check_venv() -> list[Check]: - """The running interpreter must not be the hanging conda one, and a - trivial subprocess must return within 2 s. - - The conda Python on this machine hangs on import (see CONTRIBUTING's - environment rules), so ``sys.base_prefix`` under a miniconda/anaconda - path is a FAIL: run jevmlx from a uv venv instead. The subprocess probe - catches the same hang from the outside: any Python that cannot run - ``python -c print(1)`` within 2 s cannot run jevmlx. + """The running interpreter must not be conda, and a trivial subprocess + must return within 2 s. + + conda-provided interpreters have been observed to hang at exec under + endpoint-security load; jevmlx expects a uv-managed CPython, so + ``sys.base_prefix`` under a miniconda/anaconda path is a FAIL. The + subprocess probe catches the same hang from the outside: any Python + that cannot run ``python -c print(1)`` within 2 s cannot run jevmlx. """ checks: list[Check] = [] base = sys.base_prefix @@ -187,12 +187,15 @@ def check_venv() -> list[Check]: def check_editable_install() -> Check: - """The venv's jevmlx editable install must point at this checkout. - - Reads ``direct_url.json`` from the installed jevmlx dist-info (written - by the editable install itself): its ``url`` must be the current - checkout, otherwise the venv imports a different jevmlx tree than the - one being tested or benchmarked. + """Editable installs must point at this checkout; package installs are OK. + + Reads ``direct_url.json`` from the installed jevmlx dist-info. pip + writes it only for direct-URL installs (``uv pip install -e``) and + marks them ``dir_info.editable``; a normal wheel install (PyPI) has no + direct_url.json at all. So: no direct_url.json or not editable -> OK + ("installed as a package" — the normal end-user case). Editable -> its + ``url`` must be the current checkout, otherwise the venv imports a + different jevmlx tree than the one being tested or benchmarked. """ checkout = Path(__file__).resolve().parent.parent try: @@ -205,30 +208,25 @@ def check_editable_install() -> Check: "uv pip install -e '.[dev]'", ) if not raw: - return _fail( - "editable-install", - "no direct_url.json (jevmlx not installed editable)", - "uv pip install -e '.[dev]'", - ) + return _ok("editable-install", "installed as a package (no direct_url.json)") try: - url = json.loads(raw).get("url", "") + payload = json.loads(raw) except json.JSONDecodeError: - return _fail( - "editable-install", - "direct_url.json is not valid JSON", - "uv pip install -e '.[dev]'", - ) + return _ok("editable-install", "installed as a package (direct_url.json unreadable)") + if not payload.get("dir_info", {}).get("editable"): + return _ok("editable-install", "installed as a package (not editable)") + url = payload.get("url", "") installed = _direct_url_path(url) if installed is None: return _fail( "editable-install", - f"direct_url.json url is not a local path: {url}", - "uv pip install -e '.[dev]'", + f"editable install url is not a local path: {url}", + "uv pip install -e '.[dev]' from the checkout you are testing", ) if installed.resolve() != checkout.resolve(): return _fail( "editable-install", - f"venv installs jevmlx from {installed}, not this checkout ({checkout})", + f"venv installs jevmlx editable from {installed}, not this checkout ({checkout})", "uv pip install -e '.[dev]' from the checkout you are testing", ) return _ok("editable-install", f"editable install -> {installed}") diff --git a/tests/test_doctor.py b/tests/test_doctor.py index 4e68740..e7a9d4b 100644 --- a/tests/test_doctor.py +++ b/tests/test_doctor.py @@ -426,6 +426,30 @@ def test_points_at_current_checkout_ok(self): assert check.status == "OK" assert "editable install" in check.detail + def test_wheel_install_no_direct_url_ok(self, monkeypatch): + # Normal PyPI install: no direct_url.json -> the normal end-user + # case, doctor must not fail it. + monkeypatch.setattr( + doctor.importlib.metadata, + "distribution", + lambda name: types.SimpleNamespace(read_text=lambda _n: None), + ) + check = check_editable_install() + assert check.status == "OK" + assert "installed as a package" in check.detail + + def test_direct_url_not_editable_ok(self, monkeypatch, tmp_path): + other = tmp_path / "some-wheel-tree" + payload = json.dumps({"url": other.as_uri(), "dir_info": {"editable": False}}) + monkeypatch.setattr( + doctor.importlib.metadata, + "distribution", + lambda name: types.SimpleNamespace(read_text=lambda _n: payload), + ) + check = check_editable_install() + assert check.status == "OK" + assert "not editable" in check.detail + def test_other_checkout_fails(self, monkeypatch, tmp_path): fake = tmp_path / "other-checkout" fake.mkdir() @@ -449,13 +473,16 @@ def raise_pnf(name): assert "uv pip install -e" in check.fix def test_no_direct_url_fails(self, monkeypatch): + # No dist-info at all is still a FAIL (jevmlx not installed), + # distinct from a package install whose dist-info lacks + # direct_url.json (F1: OK). monkeypatch.setattr( doctor.importlib.metadata, "distribution", lambda name: types.SimpleNamespace(read_text=lambda _n: None), ) check = check_editable_install() - assert check.status == "FAIL" + assert check.status == "OK" def test_file_url_with_spaces_unquoted(self, monkeypatch, tmp_path): checkout = tmp_path / "my checkout" @@ -464,7 +491,9 @@ def test_file_url_with_spaces_unquoted(self, monkeypatch, tmp_path): doctor.importlib.metadata, "distribution", lambda name: types.SimpleNamespace( - read_text=lambda _n: json.dumps({"url": checkout.as_uri()}) + read_text=lambda _n: json.dumps( + {"url": checkout.as_uri(), "dir_info": {"editable": True}} + ) ), ) # The installed url points at a different tree than this test file's