diff --git a/Makefile b/Makefile index 70a0baa..77babeb 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: help install lint format typecheck test check clean serve bench eval chart docker-cpu docker-cuda +.PHONY: help install lint format typecheck test check clean serve bench eval chart docker-cpu docker-cuda doctor rehearse PYTHON := uv run python CONFIG ?= bench-smoke @@ -18,6 +18,7 @@ help: @echo " eval Run quality eval" @echo " chart Rebuild all charts from results/" @echo " rehearse Pre-flight rehearsal of deploy/runpod-run.sh against tiny model" + @echo " doctor Diagnose venv drift (vllm + transformers + huggingface-hub pins)" @echo "" @echo " docker-cpu Build CPU vLLM image (Dockerfile.cpu) for M1 dev" @echo " docker-cuda Build production CUDA image (Dockerfile)" @@ -57,6 +58,9 @@ chart: rehearse: bash deploy/runpod-run.sh --rehearsal +doctor: + uv run --no-sync python -m scripts.doctor + docker-cpu: docker build -f Dockerfile.cpu -t forge:cpu . diff --git a/pyproject.toml b/pyproject.toml index 83deccb..6fbd99a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -11,6 +11,7 @@ dependencies = [ "huggingface-hub<1.0", "matplotlib>=3.10.9", "openai>=2.38.0", + "packaging>=26.2", "pyyaml>=6.0.3", "types-pyyaml>=6.0.12.20260518", ] diff --git a/scripts/doctor.py b/scripts/doctor.py new file mode 100644 index 0000000..55a37c6 --- /dev/null +++ b/scripts/doctor.py @@ -0,0 +1,206 @@ +"""Diagnose Forge's environment against what the harness, server, and eval need. + +``make doctor`` parses the constraint files, inspects the active venv, and +prints a green/yellow/red report per check. Exits 0 when everything's healthy, +1 when at least one required component is missing or mispinned. + +This exists because ``uv sync`` and the out-of-band ``uv pip install -c +constraints/...`` flow can drift — running ``uv run`` after a fresh ``uv sync`` +silently unwinds the GPU-coupled installs and the next ``make rehearse`` fails +deep into vLLM imports. Catch that here in one second instead. +""" + +from __future__ import annotations + +import argparse +import importlib.metadata as md +import shutil +import sys +from collections.abc import Iterator +from dataclasses import dataclass +from pathlib import Path + +from packaging.requirements import Requirement +from packaging.version import Version + + +@dataclass(frozen=True) +class Check: + name: str + ok: bool + detail: str + fix: str | None = None + + def emit(self) -> None: + glyph = "✓" if self.ok else "✗" + color_start = "\033[32m" if self.ok else "\033[31m" + color_end = "\033[0m" + if sys.stdout.isatty(): + print(f"{color_start}{glyph}{color_end} {self.name:<42s} {self.detail}") + else: + print(f"{glyph} {self.name:<42s} {self.detail}") + if not self.ok and self.fix: + print(f" ↳ fix: {self.fix}") + + +def _installed_version(distribution: str) -> Version | None: + try: + return Version(md.version(distribution)) + except md.PackageNotFoundError: + return None + + +def _parse_constraint_file(path: Path) -> Iterator[Requirement]: + """Yield each requirement line from a uv-style constraints file. + + Skips blank lines and comments. Tolerates trailing comments. + """ + if not path.exists(): + return + for raw in path.read_text(encoding="utf-8").splitlines(): + line = raw.split("#", 1)[0].strip() + if not line: + continue + try: + yield Requirement(line) + except Exception as exc: + # Permissive on purpose: a malformed constraint shouldn't take down + # the diagnostic; just warn and move on so the rest of the checks run. + print(f"WARNING: could not parse '{line}' in {path}: {exc}", file=sys.stderr) + + +def check_constraint_file(path: Path, label: str) -> list[Check]: + """For each requirement in ``path``, check the installed version satisfies it.""" + checks: list[Check] = [] + for req in _parse_constraint_file(path): + installed = _installed_version(req.name) + if installed is None: + checks.append( + Check( + name=f"{label}: {req.name}", + ok=False, + detail="not installed", + fix=_install_hint(path, req.name), + ) + ) + continue + if req.specifier and installed not in req.specifier: + checks.append( + Check( + name=f"{label}: {req.name}", + ok=False, + detail=f"{installed} does not satisfy {req.specifier}", + fix=_install_hint(path, req.name), + ) + ) + continue + spec = str(req.specifier) if req.specifier else "any" + checks.append(Check(name=f"{label}: {req.name}", ok=True, detail=f"{installed} ({spec})")) + return checks + + +def _install_hint(constraint_path: Path, package: str) -> str: + return f'uv pip install -c {constraint_path.as_posix()} "{package}"' + + +def check_executable(name: str, hint: str) -> Check: + found = shutil.which(name) + return Check( + name=f"executable: {name}", + ok=found is not None, + detail=found or "not on PATH", + fix=None if found else hint, + ) + + +def check_python_version() -> Check: + required = (3, 12) + current = sys.version_info[:2] + ok = current == required + return Check( + name="python version", + ok=ok, + detail=f"{sys.version.split()[0]}", + fix=None if ok else "uv python install 3.12 && uv venv --python 3.12", + ) + + +def check_huggingface_hub_for_transformers() -> Check: + """transformers 4.x has a runtime guard on huggingface-hub<1.0.""" + transformers = _installed_version("transformers") + if transformers is None: + return Check( + name="transformers ↔ huggingface-hub", + ok=False, + detail="transformers not installed; skipping", + ) + hub = _installed_version("huggingface-hub") + if hub is None: + return Check( + name="transformers ↔ huggingface-hub", + ok=False, + detail="huggingface-hub not installed", + ) + transformers_4x = transformers.major == 4 + hub_lt_1 = hub.major == 0 + ok = (not transformers_4x) or hub_lt_1 + return Check( + name="transformers ↔ huggingface-hub", + ok=ok, + detail=f"transformers={transformers}, huggingface-hub={hub}", + fix=(None if ok else "uv pip install -c constraints/serve.txt vllm transformers"), + ) + + +def run_checks(*, eval_too: bool) -> list[Check]: + repo_root = Path(__file__).resolve().parents[1] + checks: list[Check] = [check_python_version()] + + # The serve constraint family — vllm + transformers + the bits they pull in. + serve_constraint = repo_root / "constraints" / "serve.txt" + checks.extend(check_constraint_file(serve_constraint, "serve")) + checks.append(check_huggingface_hub_for_transformers()) + checks.append( + check_executable( + "vllm", + f"uv pip install -c {serve_constraint.as_posix()} vllm", + ) + ) + + if eval_too: + eval_constraint = repo_root / "constraints" / "eval.txt" + checks.extend(check_constraint_file(eval_constraint, "eval")) + checks.append( + check_executable( + "lm-eval", + f'uv pip install -c {eval_constraint.as_posix()} "lm-eval[api]"', + ) + ) + + return checks + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--no-eval", + action="store_true", + help="Skip the constraints/eval.txt + lm-eval checks (use for serve-only environments).", + ) + args = parser.parse_args(argv) + + checks = run_checks(eval_too=not args.no_eval) + for check in checks: + check.emit() + + failed = [c for c in checks if not c.ok] + print() + if failed: + print(f"{len(failed)} of {len(checks)} checks failed.", file=sys.stderr) + return 1 + print(f"all {len(checks)} checks passed.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_doctor.py b/tests/test_doctor.py new file mode 100644 index 0000000..f384a82 --- /dev/null +++ b/tests/test_doctor.py @@ -0,0 +1,187 @@ +"""Tests for the env-drift diagnostic.""" + +from __future__ import annotations + +from pathlib import Path +from textwrap import dedent +from unittest.mock import patch + +import pytest +from packaging.version import Version + +from scripts.doctor import ( + Check, + check_constraint_file, + check_executable, + check_huggingface_hub_for_transformers, + check_python_version, + main, +) + + +def _constraint(tmp_path: Path, body: str) -> Path: + path = tmp_path / "serve.txt" + path.write_text(dedent(body).strip() + "\n", encoding="utf-8") + return path + + +def test_check_python_version_pass() -> None: + with patch("sys.version_info", new=(3, 12, 13, "final", 0)): + result = check_python_version() + assert result.ok + assert "fix" not in (result.fix or "") + + +def test_check_python_version_fail_records_fix() -> None: + with patch("sys.version_info", new=(3, 11, 9, "final", 0)): + result = check_python_version() + assert not result.ok + assert result.fix and "uv python install 3.12" in result.fix + + +def test_constraint_file_missing_package(tmp_path: Path) -> None: + path = _constraint(tmp_path, "vllm>=0.11.0,<0.12") + with patch("scripts.doctor._installed_version", return_value=None): + checks = check_constraint_file(path, "serve") + assert len(checks) == 1 + assert not checks[0].ok + assert checks[0].detail == "not installed" + assert checks[0].fix and "uv pip install" in checks[0].fix + + +def test_constraint_file_version_mismatch(tmp_path: Path) -> None: + path = _constraint(tmp_path, "transformers>=4.45,<5.0") + with patch("scripts.doctor._installed_version", return_value=Version("5.8.1")): + checks = check_constraint_file(path, "serve") + assert len(checks) == 1 + assert not checks[0].ok + assert "5.8.1" in checks[0].detail + assert "does not satisfy" in checks[0].detail + + +def test_constraint_file_satisfied(tmp_path: Path) -> None: + path = _constraint(tmp_path, "transformers>=4.45,<5.0") + with patch("scripts.doctor._installed_version", return_value=Version("4.57.6")): + checks = check_constraint_file(path, "serve") + assert checks[0].ok + + +def test_constraint_file_handles_comments_and_blanks(tmp_path: Path) -> None: + path = _constraint( + tmp_path, + """ + # heading + vllm>=0.11.0,<0.12 + + transformers>=4.45,<5.0 # inline + """, + ) + + def fake(name: str) -> Version: + return Version({"vllm": "0.11.0", "transformers": "4.57.6"}[name]) + + with patch("scripts.doctor._installed_version", side_effect=fake): + checks = check_constraint_file(path, "serve") + + names = {c.name for c in checks} + assert names == {"serve: vllm", "serve: transformers"} + assert all(c.ok for c in checks) + + +def test_constraint_file_missing_path_is_noop(tmp_path: Path) -> None: + checks = check_constraint_file(tmp_path / "does-not-exist.txt", "serve") + assert checks == [] + + +def test_executable_present(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("shutil.which", lambda name: f"/fake/bin/{name}") + result = check_executable("vllm", "fix-cmd") + assert result.ok + assert result.detail == "/fake/bin/vllm" + + +def test_executable_missing(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("shutil.which", lambda name: None) + result = check_executable("vllm", "fix-cmd") + assert not result.ok + assert result.fix == "fix-cmd" + + +def test_huggingface_hub_guard_pass() -> None: + # transformers 4.x + huggingface-hub 0.x → OK. + def fake(name: str) -> Version: + return Version({"transformers": "4.57.6", "huggingface-hub": "0.36.2"}[name]) + + with patch("scripts.doctor._installed_version", side_effect=fake): + result = check_huggingface_hub_for_transformers() + assert result.ok + + +def test_huggingface_hub_guard_fail() -> None: + # transformers 4.x + huggingface-hub 1.x → broken combo. + def fake(name: str) -> Version: + return Version({"transformers": "4.57.6", "huggingface-hub": "1.16.1"}[name]) + + with patch("scripts.doctor._installed_version", side_effect=fake): + result = check_huggingface_hub_for_transformers() + assert not result.ok + assert "1.16.1" in result.detail + assert result.fix and "constraints/serve.txt" in result.fix + + +def test_huggingface_hub_guard_skipped_when_transformers_5x() -> None: + # transformers 5.x doesn't have the pinning bug we're guarding against. + def fake(name: str) -> Version: + return Version({"transformers": "5.8.1", "huggingface-hub": "1.16.1"}[name]) + + with patch("scripts.doctor._installed_version", side_effect=fake): + result = check_huggingface_hub_for_transformers() + assert result.ok + + +def test_huggingface_hub_guard_handles_missing_transformers() -> None: + with patch("scripts.doctor._installed_version", return_value=None): + result = check_huggingface_hub_for_transformers() + assert not result.ok + assert "not installed" in result.detail + + +def test_check_emit_works(capsys: pytest.CaptureFixture[str]) -> None: + Check(name="x", ok=True, detail="ok").emit() + out = capsys.readouterr().out + assert "x" in out + assert "ok" in out + + Check(name="y", ok=False, detail="bad", fix="run this").emit() + out = capsys.readouterr().out + assert "y" in out + assert "bad" in out + assert "fix: run this" in out + + +def test_main_exits_nonzero_when_check_fails(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + "scripts.doctor.run_checks", + lambda **_: [Check(name="x", ok=False, detail="d", fix=None)], + ) + assert main([]) == 1 + + +def test_main_exits_zero_when_all_pass(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + "scripts.doctor.run_checks", + lambda **_: [Check(name="x", ok=True, detail="d")], + ) + assert main([]) == 0 + + +def test_main_no_eval_flag_skips_eval(monkeypatch: pytest.MonkeyPatch) -> None: + captured = {} + + def fake_run(*, eval_too: bool) -> list[Check]: + captured["eval_too"] = eval_too + return [Check(name="x", ok=True, detail="d")] + + monkeypatch.setattr("scripts.doctor.run_checks", fake_run) + main(["--no-eval"]) + assert captured == {"eval_too": False} diff --git a/uv.lock b/uv.lock index dc8c8db..02d102d 100644 --- a/uv.lock +++ b/uv.lock @@ -268,6 +268,7 @@ dependencies = [ { name = "huggingface-hub" }, { name = "matplotlib" }, { name = "openai" }, + { name = "packaging" }, { name = "pyyaml" }, { name = "types-pyyaml" }, ] @@ -287,6 +288,7 @@ requires-dist = [ { name = "huggingface-hub", specifier = "<1.0" }, { name = "matplotlib", specifier = ">=3.10.9" }, { name = "openai", specifier = ">=2.38.0" }, + { name = "packaging", specifier = ">=26.2" }, { name = "pyyaml", specifier = ">=6.0.3" }, { name = "types-pyyaml", specifier = ">=6.0.12.20260518" }, ]