From 9bcbb50fc17c9af26d952297d38c6963deefae30 Mon Sep 17 00:00:00 2001 From: cvetty Date: Mon, 1 Jun 2026 01:20:41 +0300 Subject: [PATCH] feat(init,scan): remove optional preflight from init; redirect scan logs to file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit whygraph init now runs only the git preflight check — gh and LLM provider checks are deferred to scan time, after the developer has configured whygraph.toml. Showing optional warnings before any config exists was misleading noise. whygraph scan redirects all log output to .whygraph/scan.log for the duration of the Progress block. The RichHandler is temporarily removed so log lines no longer interleave with progress bars; it is restored once the crawlers finish. The scan log path is printed after the bars complete. --- src/whygraph/cli/commands/init.py | 6 +- src/whygraph/cli/commands/scan.py | 6 +- src/whygraph/cli/preflight.py | 27 +++---- src/whygraph/core/logger.py | 50 ++++++++++++ tests/test_init_agents.py | 2 +- tests/test_preflight.py | 130 +++++++++++++++++------------- 6 files changed, 142 insertions(+), 79 deletions(-) diff --git a/src/whygraph/cli/commands/init.py b/src/whygraph/cli/commands/init.py index f6a5616..c8f12df 100644 --- a/src/whygraph/cli/commands/init.py +++ b/src/whygraph/cli/commands/init.py @@ -101,7 +101,7 @@ def init_cmd( project_root = Path.cwd() if not skip_preflight: - _run_preflight(project_root) + _run_preflight() db_path = _ensure_db_initialized() click.echo(f"Initialized WhyGraph database at {db_path}") @@ -130,7 +130,7 @@ def init_cmd( _print_install_summary(target, project_root, result, force=force) -def _run_preflight(project_root: Path) -> None: +def _run_preflight() -> None: """Echo the diagnostics block; ``fail`` with a clean error on missing tools. Imported here (not at module top) so ``--list-agents`` and ``--help`` @@ -139,7 +139,7 @@ def _run_preflight(project_root: Path) -> None: from whygraph.cli.preflight import PreflightError, run_preflight try: - run_preflight(project_root) + run_preflight() except PreflightError as exc: fail(str(exc)) diff --git a/src/whygraph/cli/commands/scan.py b/src/whygraph/cli/commands/scan.py index 77592f4..2cc0946 100644 --- a/src/whygraph/cli/commands/scan.py +++ b/src/whygraph/cli/commands/scan.py @@ -84,6 +84,7 @@ def scan_cmd( # don't fail when the DB or git layers are mid-rewrite. from whygraph.analyze import LlmDescriptor from whygraph.core import get_config + from whygraph.core.logger import scan_log_redirect from whygraph.db import ensure_initialized from whygraph.scan import AnalyzeCrawler from whygraph.services.git import Repository @@ -122,7 +123,8 @@ def scan_cmd( remote_enabled=remote, ) - with Progress() as progress: + scan_log_path = db_path.parent / "scan.log" + with scan_log_redirect(scan_log_path), Progress() as progress: # CodeGraph refresh — runs concurrently as its own crawler. It # writes .codegraph/ and has no data dependency on the WhyGraph DB, # so it overlaps the entire crawl (started with phase 1, joined @@ -167,6 +169,8 @@ def scan_cmd( if codegraph_crawler is not None: codegraph_crawler.join() + console.print(f"Scan log: {scan_log_path}") + if codegraph_crawler is not None and codegraph_crawler.warning is not None: console.print(Text(codegraph_crawler.warning, style="yellow")) diff --git a/src/whygraph/cli/preflight.py b/src/whygraph/cli/preflight.py index 2ca88c9..8295252 100644 --- a/src/whygraph/cli/preflight.py +++ b/src/whygraph/cli/preflight.py @@ -1,13 +1,13 @@ """Preflight diagnostics for the WhyGraph CLI. Runs as the first step of ``whygraph init``: probes the host for the tools -the rest of the workflow expects (``git``, ``gh``, an LLM credential), -prints a one-line status per check, and raises :class:`PreflightError` if -a required tool is missing. +that ``init`` itself requires (currently only ``git``), prints a one-line +status per check, and raises :class:`PreflightError` if a required tool is +missing. -Hard-required tools are collected and reported together so a fresh user -sees the full punch list once. Soft checks print as warnings and don't -affect exit code. +Optional tooling (``gh``, LLM credentials) is *not* checked here — those +are only meaningful at ``whygraph scan`` time, after the developer has +configured ``whygraph.toml``. Designed to be importable from other commands later (``scan``) without restructuring — :func:`run_preflight` is the only public surface. @@ -58,25 +58,20 @@ class _CheckResult: _GLYPH = {"ok": "✓", "missing": "✗", "skipped": "—"} -def run_preflight(project_root: Path) -> None: +def run_preflight() -> None: """Echo a preflight checks block; raise if a hard requirement is missing. - Parameters - ---------- - project_root : Path - Repository root — used to detect whether the project has a - ``github.com`` remote (which makes the ``gh`` probe relevant). + Only hard-required tools are checked here. Optional tooling (``gh``, + LLM credentials) is left to the ``scan`` command, which runs after + the developer has configured ``whygraph.toml``. Raises ------ PreflightError - If ``git`` is missing. All missing hard requirements are reported - in a single error. + If ``git`` is missing. """ checks = [ _check_git(), - _check_gh(project_root), - _check_llm(), ] console.print("Preflight checks:") diff --git a/src/whygraph/core/logger.py b/src/whygraph/core/logger.py index 7145d33..8aa12cd 100644 --- a/src/whygraph/core/logger.py +++ b/src/whygraph/core/logger.py @@ -14,8 +14,10 @@ from __future__ import annotations import logging +from contextlib import contextmanager from enum import IntEnum from logging.handlers import RotatingFileHandler +from pathlib import Path from typing import TYPE_CHECKING from rich.console import Console @@ -181,6 +183,54 @@ def configure_logging( return root +@contextmanager +def scan_log_redirect(log_path: Path): + """Suppress console log output and redirect to *log_path* for the duration. + + Intended to wrap Rich ``Progress`` blocks where ``RichHandler`` output + interleaves with progress-bar rendering, corrupting the terminal display. + + The file is opened in ``'w'`` mode so each scan run starts fresh. + Any user-configured ``RotatingFileHandler`` (via ``[logging]`` in + ``whygraph.toml``) is left in place and continues writing in parallel. + + The root logger level is temporarily lowered to ``DEBUG`` so the file + captures everything; it is restored to the original level on exit + regardless of whether an exception was raised. + + Parameters + ---------- + log_path : Path + Destination file; its parent directory is created if absent. + + Yields + ------ + Path + The resolved *log_path*, for callers that want to print it. + """ + root = logging.getLogger(_ROOT_NAME) + + console_handlers = [h for h in root.handlers if isinstance(h, RichHandler)] + for h in console_handlers: + root.removeHandler(h) + + log_path.parent.mkdir(parents=True, exist_ok=True) + file_handler = logging.FileHandler(log_path, mode="w", encoding="utf-8") + file_handler.setFormatter(logging.Formatter(_FILE_FORMAT)) + prev_level = root.level + root.setLevel(logging.DEBUG) + root.addHandler(file_handler) + + try: + yield log_path + finally: + root.setLevel(prev_level) + root.removeHandler(file_handler) + file_handler.close() + for h in console_handlers: + root.addHandler(h) + + def get_logger(name: str | None = None) -> logging.Logger: """Return the WhyGraph root logger or a named child. diff --git a/tests/test_init_agents.py b/tests/test_init_agents.py index 38bc873..b8d27ed 100644 --- a/tests/test_init_agents.py +++ b/tests/test_init_agents.py @@ -252,7 +252,7 @@ def _fake_db() -> Path: monkeypatch.setattr("whygraph.cli.commands.init._ensure_db_initialized", _fake_db) monkeypatch.setattr( "whygraph.cli.commands.init._run_preflight", - lambda project_root: None, + lambda: None, ) return fake_db diff --git a/tests/test_preflight.py b/tests/test_preflight.py index b2378c4..b000f26 100644 --- a/tests/test_preflight.py +++ b/tests/test_preflight.py @@ -4,6 +4,10 @@ test monkeypatches those so the suite runs hermetic regardless of what the host has installed. ``ANTHROPIC_API_KEY`` is managed via ``monkeypatch.setenv`` / ``delenv`` so test order can't matter. + +``run_preflight()`` only checks ``git`` — optional tooling (``gh``, LLM +credentials) is tested against the private ``_check_gh`` / ``_check_llm`` +helpers directly, since those checks belong to ``scan`` time, not ``init``. """ from __future__ import annotations @@ -48,52 +52,73 @@ def fake_run(*args: object, **kwargs: object) -> subprocess.CompletedProcess: monkeypatch.setattr(preflight.subprocess, "run", fake_run) -def test_happy_path(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: - _git_repo(tmp_path, remote_url="git@github.com:org/repo.git") +# --------------------------------------------------------------------------- +# run_preflight() — git-only surface used by `whygraph init` +# --------------------------------------------------------------------------- + + +def test_happy_path(monkeypatch: pytest.MonkeyPatch) -> None: _patch_which(monkeypatch, missing=set()) - _patch_gh_auth_ok(monkeypatch) - monkeypatch.setenv("ANTHROPIC_API_KEY", "test-key") - # Must not raise. - run_preflight(tmp_path) + # Must not raise regardless of gh / LLM state. + run_preflight() -def test_git_missing_raises(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: - _git_repo(tmp_path, remote_url=None) +def test_git_missing_raises(monkeypatch: pytest.MonkeyPatch) -> None: _patch_which(monkeypatch, missing={"git"}) - monkeypatch.setenv("ANTHROPIC_API_KEY", "x") with pytest.raises(PreflightError, match="git"): - run_preflight(tmp_path) + run_preflight() -def test_docker_absence_is_irrelevant( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path -) -> None: - _git_repo(tmp_path, remote_url=None) +def test_docker_absence_is_irrelevant(monkeypatch: pytest.MonkeyPatch) -> None: _patch_which(monkeypatch, missing={"docker"}) - monkeypatch.setenv("ANTHROPIC_API_KEY", "x") - # Docker is no longer a preflight concern — init doesn't index CodeGraph, - # and `whygraph scan` runs the in-image binary. Must not raise. - run_preflight(tmp_path) + # Docker is no longer a preflight concern — must not raise. + run_preflight() + + +def test_gh_and_llm_not_checked_by_run_preflight( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """``run_preflight`` must succeed even when gh and LLM are absent.""" + _patch_which(monkeypatch, missing={"gh", "claude"}) + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + + # Optional tools are not checked at init time — must not raise. + run_preflight() + + +def test_hard_missing_reported_in_error(monkeypatch: pytest.MonkeyPatch) -> None: + _patch_which(monkeypatch, missing={"git"}) + + with pytest.raises(PreflightError) as exc_info: + run_preflight() + + assert "git" in str(exc_info.value) + + +# --------------------------------------------------------------------------- +# _check_gh() — available for scan-time use +# --------------------------------------------------------------------------- def test_gh_missing_on_github_repo_is_soft_warning( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: - _git_repo(tmp_path, remote_url="git@github.com:org/repo.git") + root = _git_repo(tmp_path, remote_url="git@github.com:org/repo.git") _patch_which(monkeypatch, missing={"gh"}) - monkeypatch.setenv("ANTHROPIC_API_KEY", "x") - # Soft — must not raise. - run_preflight(tmp_path) + result = preflight._check_gh(root) + + assert result.status == "missing" + assert result.soft is True def test_gh_probe_skipped_on_non_github_repo( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: - _git_repo(tmp_path, remote_url="git@gitlab.com:org/repo.git") + root = _git_repo(tmp_path, remote_url="git@gitlab.com:org/repo.git") calls: dict[str, int] = {} @@ -102,17 +127,17 @@ def counting_which(name: str) -> str | None: return f"/usr/bin/{name}" monkeypatch.setattr(preflight.shutil, "which", counting_which) - monkeypatch.setenv("ANTHROPIC_API_KEY", "x") - run_preflight(tmp_path) + result = preflight._check_gh(root) + assert result.status == "skipped" assert "gh" not in calls, "gh probe must not run on non-GitHub repos" def test_gh_auth_status_nonzero_is_soft_warning( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: - _git_repo(tmp_path, remote_url="git@github.com:org/repo.git") + root = _git_repo(tmp_path, remote_url="git@github.com:org/repo.git") _patch_which(monkeypatch, missing=set()) def fake_run(*args: object, **kwargs: object) -> subprocess.CompletedProcess: @@ -121,52 +146,41 @@ def fake_run(*args: object, **kwargs: object) -> subprocess.CompletedProcess: ) monkeypatch.setattr(preflight.subprocess, "run", fake_run) - monkeypatch.setenv("ANTHROPIC_API_KEY", "x") - # Soft — must not raise. - run_preflight(tmp_path) + result = preflight._check_gh(root) + assert result.status == "missing" + assert result.soft is True -def test_llm_missing_is_soft_warning( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path -) -> None: - _git_repo(tmp_path, remote_url=None) + +# --------------------------------------------------------------------------- +# _check_llm() — available for scan-time use +# --------------------------------------------------------------------------- + + +def test_llm_missing_is_soft_warning(monkeypatch: pytest.MonkeyPatch) -> None: _patch_which(monkeypatch, missing={"claude"}) monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) - run_preflight(tmp_path) + result = preflight._check_llm() + assert result.status == "missing" + assert result.soft is True -def test_llm_ok_via_env_var_alone( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path -) -> None: - _git_repo(tmp_path, remote_url=None) + +def test_llm_ok_via_env_var_alone(monkeypatch: pytest.MonkeyPatch) -> None: _patch_which(monkeypatch, missing={"claude"}) monkeypatch.setenv("ANTHROPIC_API_KEY", "real-key") - # Env var alone satisfies the LLM check even without `claude` on PATH. - run_preflight(tmp_path) + result = preflight._check_llm() + assert result.status == "ok" -def test_llm_ok_via_claude_cli_alone( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path -) -> None: - _git_repo(tmp_path, remote_url=None) + +def test_llm_ok_via_claude_cli_alone(monkeypatch: pytest.MonkeyPatch) -> None: _patch_which(monkeypatch, missing=set()) monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) - # `claude` on PATH alone satisfies the LLM check. - run_preflight(tmp_path) - + result = preflight._check_llm() -def test_hard_missing_reported_in_error( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path -) -> None: - _git_repo(tmp_path, remote_url=None) - _patch_which(monkeypatch, missing={"git"}) - monkeypatch.setenv("ANTHROPIC_API_KEY", "x") - - with pytest.raises(PreflightError) as exc_info: - run_preflight(tmp_path) - - assert "git" in str(exc_info.value) + assert result.status == "ok"