test_5106 + llama_cpp: extract win32 PATH helper and harden the regression test

Follow-up to PR #5376's review feedback. Three real findings from the
bot reviewers, plus one stale one.

1. (codex P2 line 201, gemini medium line 209) The regression test's
   _build_path_dirs_like_start_llama_server hand-copied the win32
   branch of LlamaCppBackend.start_llama_server, so a future drop or
   reorder of _windows_pip_nvidia_dll_dirs(sys.prefix) in production
   would have passed the test silently.

   Extract a new staticmethod LlamaCppBackend._build_windows_path_dirs
   (binary_dir, prefix, cuda_path). Production start_llama_server now
   calls this helper. The test's wrapper is reduced to a one-line
   delegate that forwards to the staticmethod, so the regression
   asserts against the exact production logic instead of a parallel
   copy of it.

2. (codex P2 line 245) test_nvidia_smi_probe_reports_synthetic_gpu did
   not clear CUDA_VISIBLE_DEVICES. On a shared GPU runner with the
   variable set in the parent shell, _get_gpu_free_memory() filters
   the mocked CSV and returns [] or falls through to the torch
   fallback. Cleared CUDA_VISIBLE_DEVICES and NVIDIA_VISIBLE_DEVICES
   via monkeypatch.delenv(..., raising=False).

3. (codex P2 line 66) _maybe_stub gated on importlib.util.find_spec
   ("loggers"), which returns a spec because studio/backend/loggers/
   is on sys.path. But the actual import chain loads
   loggers/handlers.py which does `from fastapi import Request,
   Response` at module load. In a lightweight env without fastapi
   installed, the stub never lands and `from core.inference.llama_cpp
   import LlamaCppBackend` raises during collection. Switched
   _maybe_stub to a real import attempt under try / except ImportError
   so the stub falls into place when the package is discoverable but
   not importable. CI has fastapi so this is purely a developer-
   machine ergonomics fix.

The fourth comment (codex P1 line 85 "Keep the httpx stub from leaking
across tests") was already addressed by 7437e735, which replaced the
unconditional sys.modules.setdefault with the find_spec-gated
_maybe_stub. No code change needed.

Production behaviour is unchanged: _build_windows_path_dirs returns
exactly the same ordering start_llama_server used inline
([binary_dir, *pip_dirs, cuda_bin?, cuda_bin_x64?]).

Verification (run inside studio/backend):
  pytest tests/test_5106_windows_gpu_detection_mock.py -v
    -> 10 passed
  pytest tests/test_llama_cpp_*.py tests/test_llama_server_args.py
       tests/test_5106_windows_gpu_detection_mock.py -q
    -> 171 passed
  CUDA_VISIBLE_DEVICES=1 pytest tests/test_5106_windows_gpu_detection_mock.py::TestWindowsGpuDetectionAfter5106Fix::test_nvidia_smi_probe_reports_synthetic_gpu
    -> 1 passed
This commit is contained in:
Daniel Han 2026-05-14 11:45:35 +00:00
commit 7a2140d4f5
2 changed files with 68 additions and 44 deletions

View file

@ -1025,6 +1025,33 @@ class LlamaCppBackend:
_add(site_packages / "torch" / "lib")
return out
@staticmethod
def _build_windows_path_dirs(
binary_dir: str, prefix: str, cuda_path: str
) -> list[str]:
"""Ordered PATH entries the win32 branch of
``start_llama_server`` prepends to the inherited env so
llama-server.exe can resolve cudart / cublas DLLs. Extracted as
a staticmethod so the test in
``studio/backend/tests/test_5106_windows_gpu_detection_mock.py``
asserts against the exact production logic instead of a local
reconstruction. Order: binary_dir first (Windows DLL search
step 1, application directory), then pip nvidia wheels (mirrors
Linux LD_LIBRARY_PATH), then optional system CUDA toolkit
(``CUDA_PATH/bin`` and ``CUDA_PATH/bin/x64``). #5106."""
path_dirs = [binary_dir]
path_dirs.extend(
LlamaCppBackend._windows_pip_nvidia_dll_dirs(prefix)
)
if cuda_path:
cuda_bin = os.path.join(cuda_path, "bin")
if os.path.isdir(cuda_bin):
path_dirs.append(cuda_bin)
cuda_bin_x64 = os.path.join(cuda_path, "bin", "x64")
if os.path.isdir(cuda_bin_x64):
path_dirs.append(cuda_bin_x64)
return path_dirs
@staticmethod
def _select_gpus(
model_size_bytes: int,
@ -2408,22 +2435,13 @@ class LlamaCppBackend:
if sys.platform == "win32":
# CUDA DLLs (cudart64_X.dll, cublas64_X.dll, etc.) must
# be on PATH. Order: binary_dir, torch's pip-installed
# nvidia wheels, then a system CUDA toolkit. Pip wheels
# are the canonical source per Studio's install design
# (mirrors the Linux LD_LIBRARY_PATH block below) and
# CUDA_PATH covers users with a system toolkit. #5106.
path_dirs = [binary_dir]
path_dirs.extend(self._windows_pip_nvidia_dll_dirs(sys.prefix))
cuda_path = os.environ.get("CUDA_PATH", "")
if cuda_path:
cuda_bin = os.path.join(cuda_path, "bin")
if os.path.isdir(cuda_bin):
path_dirs.append(cuda_bin)
# Some CUDA installs put DLLs in bin\x64
cuda_bin_x64 = os.path.join(cuda_path, "bin", "x64")
if os.path.isdir(cuda_bin_x64):
path_dirs.append(cuda_bin_x64)
# be on PATH. See _build_windows_path_dirs for the
# ordering rationale (#5106).
path_dirs = self._build_windows_path_dirs(
binary_dir,
sys.prefix,
os.environ.get("CUDA_PATH", ""),
)
existing_path = env.get("PATH", "")
env["PATH"] = ";".join(path_dirs) + ";" + existing_path
else:

View file

@ -49,20 +49,28 @@ if _BACKEND_DIR not in sys.path:
sys.path.insert(0, _BACKEND_DIR)
# Stub heavy deps the rest of the studio backend pulls in IFF they
# are not installed in this environment. ``sys.modules.setdefault``
# alone is not enough: pytest collects test files alphabetically, so
# this file (``test_5106_*``) is imported before any other test
# imports real ``httpx`` / ``structlog`` / ``loggers``. Unconditionally
# installing a stub there would shadow the real httpx for every
# subsequent test in the directory (test_anthropic_messages.py,
# test_training_*, etc) and break their `from httpx import HTTPError,
# Response` imports. Guard each stub with ``find_spec`` so we only
# fall back to the stub when the real module truly is missing.
import importlib.util as _importlib_util # noqa: E402
# fail to import here. Unconditionally installing a stub would shadow
# the real module for every subsequent test in this dir
# (test_anthropic_messages.py, test_training_*, etc) and break their
# ``from httpx import HTTPError, Response`` imports.
#
# Why try-import instead of ``importlib.util.find_spec``: ``find_spec``
# only checks discoverability, not import success. ``studio/backend/
# loggers/__init__.py`` re-exports ``handlers.get_logger``, and
# ``handlers.py`` does ``from fastapi import Request, Response`` at
# module load. In a lightweight env without fastapi, ``find_spec
# ("loggers")`` returns a spec but the actual import raises during
# ``from core.inference.llama_cpp import LlamaCppBackend`` collection.
# Calling ``import_module`` here surfaces that failure and falls back
# to the local stub. CI has fastapi installed so this is purely a
# developer-machine ergonomics fix.
import importlib as _importlib # noqa: E402
def _maybe_stub(name: str, builder):
if _importlib_util.find_spec(name) is None:
try:
_importlib.import_module(name)
except ImportError:
sys.modules[name] = builder()
@ -191,22 +199,15 @@ def _populate_studio_install(install_dir: Path, runtime: str = "13.1") -> None:
def _build_path_dirs_like_start_llama_server(
binary_dir: Path, prefix: Path, cuda_path: str = ""
) -> list[str]:
"""Faithful reproduction of the win32 branch in
LlamaCppBackend.start_llama_server. Returns the ordered list of
PATH entries we prepend to the inherited env. Production code:
studio/backend/core/inference/llama_cpp.py:2340-2363.
"""
pip_dirs = LlamaCppBackend._windows_pip_nvidia_dll_dirs(str(prefix))
path_dirs = [str(binary_dir)]
path_dirs.extend(pip_dirs)
if cuda_path:
cuda_bin = os.path.join(cuda_path, "bin")
if os.path.isdir(cuda_bin):
path_dirs.append(cuda_bin)
cuda_bin_x64 = os.path.join(cuda_path, "bin", "x64")
if os.path.isdir(cuda_bin_x64):
path_dirs.append(cuda_bin_x64)
return path_dirs
"""Thin wrapper around the production
``LlamaCppBackend._build_windows_path_dirs`` helper, kept so the
test reads with ``Path`` arguments. Asserting against the real
staticmethod (rather than a hand-copy of its body) is the whole
point: if the production PATH order ever drops or reorders
``_windows_pip_nvidia_dll_dirs``, these tests fail."""
return LlamaCppBackend._build_windows_path_dirs(
str(binary_dir), str(prefix), cuda_path
)
def _mock_nvidia_smi_run(fake_output: str, returncode: int = 0) -> "mock._patch":
@ -235,10 +236,15 @@ class TestWindowsGpuDetectionAfter5106Fix:
every other layer (resolver, PATH builder, install layout) for
real."""
def test_nvidia_smi_probe_reports_synthetic_gpu(self):
def test_nvidia_smi_probe_reports_synthetic_gpu(self, monkeypatch):
"""Sanity: the production nvidia-smi probe parses CSV output
and returns (index, free_mib) tuples. This is the entry point
Studio uses to decide whether a GPU is reachable at all."""
# Clear inherited visibility masks so the synthetic CSV is not
# filtered or shadowed by the parent shell (e.g. on a shared
# GPU runner with CUDA_VISIBLE_DEVICES=1 set).
monkeypatch.delenv("CUDA_VISIBLE_DEVICES", raising = False)
monkeypatch.delenv("NVIDIA_VISIBLE_DEVICES", raising = False)
# noahterbest's exact #5106 reproducer: RTX 4090, 22805 MiB free.
fake_csv = "0, 22805\n"
with _mock_nvidia_smi_run(fake_csv):