diff --git a/src/whygraph/cli/commands/scan.py b/src/whygraph/cli/commands/scan.py index 556d199..77592f4 100644 --- a/src/whygraph/cli/commands/scan.py +++ b/src/whygraph/cli/commands/scan.py @@ -12,7 +12,7 @@ from rich.table import Table from rich.text import Text -from whygraph.scan import Crawler, GitCrawler, GitHubCrawler +from whygraph.scan import CodeGraphCrawler, Crawler, GitCrawler, GitHubCrawler from ..console import console @@ -44,12 +44,12 @@ "refresh_codegraph", default=True, help=( - "Refresh the CodeGraph index before crawling — `codegraph sync` when " - "an index exists, `codegraph init -i` on first run. Uses the local " - "`codegraph` binary if present, else runs it inside the WhyGraph " - "Docker image. The crawl itself doesn't need CodeGraph (only the MCP " - "rationale/evidence tools do), so a failure here warns rather than " - "aborting. Default: on." + "Refresh the CodeGraph index concurrently with the crawl — " + "`codegraph sync` when an index exists, `codegraph init -i` on first " + "run. Uses the local `codegraph` binary if present, else runs it " + "inside the WhyGraph Docker image. The crawl itself doesn't need " + "CodeGraph (only the MCP rationale/evidence tools do), so a failure " + "here warns rather than aborting. Default: on." ), ) @click.option( @@ -122,13 +122,19 @@ def scan_cmd( remote_enabled=remote, ) - # CodeGraph refresh runs before the crawl and outside the Progress - # live-display (it streams its own output on first index). It's - # best-effort: the crawl doesn't depend on it. - if refresh_codegraph: - _refresh_codegraph(repository.root, image=codegraph_image) - with 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 + # last). Best-effort: failures land on .warning, not .error. + codegraph_crawler = ( + CodeGraphCrawler( + progress, project_root=repository.root, image=codegraph_image + ) + if refresh_codegraph + else None + ) + # Phase 1 — source crawlers, run concurrently. phase1: list[Crawler] = [GitCrawler(progress, repository=repository)] if github_client is not None: @@ -148,6 +154,8 @@ def scan_cmd( ) ) + if codegraph_crawler is not None: + codegraph_crawler.start() for c in phase1: c.start() for c in phase1: @@ -156,8 +164,15 @@ def scan_cmd( c.start() for c in phase2: c.join() + if codegraph_crawler is not None: + codegraph_crawler.join() + + if codegraph_crawler is not None and codegraph_crawler.warning is not None: + console.print(Text(codegraph_crawler.warning, style="yellow")) crawlers = phase1 + phase2 + if codegraph_crawler is not None: + crawlers.append(codegraph_crawler) failed = [c for c in crawlers if c.error is not None] for c in failed: click.echo(f"crawler {c.name!r} failed: {c.error}", err=True) @@ -196,34 +211,6 @@ def _apply_github_token(config: "Config") -> None: os.environ["GH_TOKEN"] = token -def _refresh_codegraph(project_root: Path, *, image: str | None) -> None: - """Bring the CodeGraph index up to date before the crawl. - - Best-effort: the git / GitHub / analyze crawl does not depend on the - CodeGraph index (only the MCP ``whygraph_rationale_brief`` and - ``whygraph_evidence_for`` tools read it), so a CodeGraph failure is - reported as a warning and the scan continues. - - Parameters - ---------- - project_root : Path - Repository root whose ``.codegraph/`` index is refreshed. - image : str or None - Docker image override for the fallback path; ``None`` uses the - pinned default. Ignored when a local ``codegraph`` binary is found. - """ - from whygraph.services.codegraph import ( - CodeGraphBootstrapError, - refresh_codegraph_index, - ) - - console.print("Refreshing CodeGraph index…") - try: - refresh_codegraph_index(project_root, image=image) - except CodeGraphBootstrapError as exc: - console.print(Text(f"CodeGraph refresh skipped — {exc}", style="yellow")) - - def _select_github_client( provider: str, repository: "Repository" ) -> "GitHubClient | None": diff --git a/src/whygraph/scan/__init__.py b/src/whygraph/scan/__init__.py index a49f51b..5a45dfd 100644 --- a/src/whygraph/scan/__init__.py +++ b/src/whygraph/scan/__init__.py @@ -2,15 +2,24 @@ Exposes :class:`Crawler` (the threaded base class) and the concrete crawlers — :class:`GitCrawler` for local git history, -:class:`GitHubCrawler` for GitHub pull requests and issues, and +:class:`GitHubCrawler` for GitHub pull requests and issues, +:class:`CodeGraphCrawler` which refreshes the CodeGraph index, and :class:`AnalyzeCrawler` which describes each commit's diff with an LLM -(run after :class:`GitCrawler`). The CLI runs the source crawlers -concurrently, then the analyzer, against the shared SQLite database. +(run after :class:`GitCrawler`). The CLI runs the source crawlers (and +CodeGraph) concurrently, then the analyzer, against the shared SQLite +database. """ from whygraph.scan.analyze_crawler import AnalyzeCrawler +from whygraph.scan.codegraph_crawler import CodeGraphCrawler from whygraph.scan.crawler import Crawler from whygraph.scan.git_crawler import GitCrawler from whygraph.scan.github_crawler import GitHubCrawler -__all__ = ["AnalyzeCrawler", "Crawler", "GitCrawler", "GitHubCrawler"] +__all__ = [ + "AnalyzeCrawler", + "CodeGraphCrawler", + "Crawler", + "GitCrawler", + "GitHubCrawler", +] diff --git a/src/whygraph/scan/codegraph_crawler.py b/src/whygraph/scan/codegraph_crawler.py new file mode 100644 index 0000000..9027c12 --- /dev/null +++ b/src/whygraph/scan/codegraph_crawler.py @@ -0,0 +1,84 @@ +"""CodeGraphCrawler — refresh the CodeGraph index alongside the crawl. + +Runs ``codegraph init -i`` (first index) or ``codegraph sync -q`` +(incremental) as one more :class:`Crawler` thread, so the index builds +concurrently with the git / GitHub / analyze crawlers instead of blocking +before them. CodeGraph writes ``.codegraph/`` and has no data dependency +on the WhyGraph DB, so it can safely overlap the entire scan. + +The refresh is **best-effort**: only the MCP rationale / evidence tools +read the index, not the crawl. A :class:`CodeGraphBootstrapError` (tool +missing, non-zero exit) is therefore swallowed into :attr:`warning` +rather than failing the scan; any other exception propagates into the +base class's :attr:`Crawler.error` and surfaces as a real failure. + +Subprocess output is captured (``capture=True``) rather than streamed so +it cannot corrupt the shared :class:`rich.progress.Progress` display; the +captured tail is folded into :attr:`warning` on failure. +""" + +from __future__ import annotations + +from pathlib import Path + +from rich.progress import Progress + +from whygraph.services.codegraph import ( + CodeGraphBootstrapError, + refresh_codegraph_index, +) +from whygraph.services.codegraph.paths import CODEGRAPH_DB_RELPATH + +from .crawler import Crawler + + +class CodeGraphCrawler(Crawler): + """Refresh ``/.codegraph/codegraph.db`` concurrently. + + Drives an indeterminate (pulsing) progress task — CodeGraph reports no + granular progress — and completes it cleanly on success. CodeGraph + bootstrap failures are recorded on :attr:`warning` rather than + :attr:`Crawler.error`, preserving the best-effort contract. + + Parameters + ---------- + progress : rich.progress.Progress + Shared Progress instance owned by the orchestrator. + project_root : Path + Repository root whose ``.codegraph/`` index is refreshed. + image : str or None + Docker image override for the CodeGraph fallback path; ``None`` + uses the pinned default. Ignored when a local ``codegraph`` binary + is found. + + Attributes + ---------- + warning : str or None + Message describing a swallowed :class:`CodeGraphBootstrapError`, + for the orchestrator to surface after the crawl. ``None`` on + success. + """ + + def __init__( + self, progress: Progress, *, project_root: Path, image: str | None + ) -> None: + super().__init__("codegraph", progress, total=None) + self._project_root = project_root + self._image = image + self.warning: str | None = None + + def work(self) -> None: + db_path = self._project_root / CODEGRAPH_DB_RELPATH + verb = "sync" if db_path.exists() else "init -i" + self.advance(0, description=f"codegraph {verb}") + + try: + refresh_codegraph_index(self._project_root, image=self._image, capture=True) + except CodeGraphBootstrapError as exc: + self.warning = f"CodeGraph refresh skipped — {exc}" + return + + # CodeGraph reports no granular progress, so land the pulsing bar + # on a clean "complete" once the refresh returns. + self.set_total(1) + self.advance(1) diff --git a/src/whygraph/services/codegraph/bootstrap.py b/src/whygraph/services/codegraph/bootstrap.py index aa66b49..5c96601 100644 --- a/src/whygraph/services/codegraph/bootstrap.py +++ b/src/whygraph/services/codegraph/bootstrap.py @@ -46,6 +46,7 @@ def ensure_codegraph_db( project_root: Path, *, image: str | None = None, + capture: bool = False, ) -> Path: """Idempotently materialize ``/.codegraph/codegraph.db``. @@ -61,6 +62,11 @@ def ensure_codegraph_db( image : str, optional Docker image tag for the fallback path. Defaults to :data:`DEFAULT_CODEGRAPH_IMAGE`. + capture : bool, optional + When ``True``, capture the subprocess output instead of letting it + stream to the terminal (so it can't corrupt a concurrent progress + display). The captured tail is folded into the error message on + failure. Default ``False`` (stream live). Returns ------- @@ -78,7 +84,7 @@ def ensure_codegraph_db( if db_path.exists(): return db_path - _run_codegraph(project_root, ["init", "-i"], image=image) + _run_codegraph(project_root, ["init", "-i"], image=image, capture=capture) if not db_path.exists(): raise CodeGraphBootstrapError( @@ -92,6 +98,7 @@ def refresh_codegraph_index( project_root: Path, *, image: str | None = None, + capture: bool = False, ) -> Path: """Bring ``/.codegraph/codegraph.db`` up to date. @@ -107,6 +114,11 @@ def refresh_codegraph_index( image : str, optional Docker image tag for the fallback path. Defaults to :data:`DEFAULT_CODEGRAPH_IMAGE`. + capture : bool, optional + When ``True``, capture the subprocess output instead of streaming + it (see :func:`ensure_codegraph_db`). ``whygraph scan`` passes this + so the refresh can run concurrently under a live progress display. + Default ``False``. Returns ------- @@ -122,9 +134,9 @@ def refresh_codegraph_index( project_root = project_root.resolve() db_path = project_root / CODEGRAPH_DB_RELPATH if not db_path.exists(): - return ensure_codegraph_db(project_root, image=image) + return ensure_codegraph_db(project_root, image=image, capture=capture) - _run_codegraph(project_root, ["sync", "-q"], image=image) + _run_codegraph(project_root, ["sync", "-q"], image=image, capture=capture) return db_path @@ -133,6 +145,7 @@ def _run_codegraph( args: list[str], *, image: str | None, + capture: bool = False, ) -> None: """Run a ``codegraph`` subcommand against ``project_root``. @@ -150,6 +163,10 @@ def _run_codegraph( image : str or None Docker image tag for the fallback path; ``None`` uses :data:`DEFAULT_CODEGRAPH_IMAGE`. + capture : bool, optional + When ``True``, capture stdout/stderr rather than streaming them to + the terminal, and fold the captured tail into the error message on + failure. Default ``False`` (stream live). Raises ------ @@ -187,8 +204,14 @@ def _run_codegraph( ) try: - subprocess.run(cmd, check=True, cwd=cwd) + subprocess.run(cmd, check=True, cwd=cwd, capture_output=capture, text=capture) except subprocess.CalledProcessError as exc: + if capture: + tail = (exc.stderr or exc.stdout or "").strip() + detail = f"\n{tail}" if tail else "" + raise CodeGraphBootstrapError( + f"`codegraph {label}` failed (exit {exc.returncode}){detail}" + ) from exc raise CodeGraphBootstrapError( f"`codegraph {label}` failed (exit {exc.returncode}) — see output above" ) from exc diff --git a/tests/test_codegraph_bootstrap.py b/tests/test_codegraph_bootstrap.py index e25d65b..5107323 100644 --- a/tests/test_codegraph_bootstrap.py +++ b/tests/test_codegraph_bootstrap.py @@ -45,10 +45,16 @@ def _capturing_run( """Fake ``subprocess.run`` recording ``cmd`` / ``cwd`` and faking success.""" def fake_run( - cmd: list[str], *, check: bool = True, cwd: Path | None = None + cmd: list[str], + *, + check: bool = True, + cwd: Path | None = None, + capture_output: bool = False, + text: bool = False, ) -> subprocess.CompletedProcess: captured["cmd"] = cmd captured["cwd"] = cwd + captured["capture_output"] = capture_output if create_db_at is not None: _make_existing_db(create_db_at) return subprocess.CompletedProcess(args=cmd, returncode=0) @@ -188,7 +194,12 @@ def test_raises_when_command_exits_nonzero( monkeypatch.setattr(bootstrap.shutil, "which", _which("codegraph")) def fake_run( - cmd: list[str], *, check: bool = True, cwd: Path | None = None + cmd: list[str], + *, + check: bool = True, + cwd: Path | None = None, + capture_output: bool = False, + text: bool = False, ) -> subprocess.CompletedProcess: raise subprocess.CalledProcessError(returncode=7, cmd=cmd) @@ -260,3 +271,58 @@ def test_refresh_sync_via_docker_fallback( assert "codegraph" in cmd assert "sync" in cmd and "-q" in cmd assert DEFAULT_CODEGRAPH_IMAGE in cmd + + +# --------------------------------------------------------------------------- # +# capture=True — used by the concurrent CodeGraphCrawler under rich.Progress +# --------------------------------------------------------------------------- # + + +def test_capture_true_passes_capture_output( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _make_existing_db(tmp_path) + monkeypatch.setattr(bootstrap.shutil, "which", _which("codegraph")) + captured: dict[str, object] = {} + monkeypatch.setattr(bootstrap.subprocess, "run", _capturing_run(captured)) + + refresh_codegraph_index(tmp_path, capture=True) + + assert captured["capture_output"] is True + + +def test_capture_default_streams_output( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _make_existing_db(tmp_path) + monkeypatch.setattr(bootstrap.shutil, "which", _which("codegraph")) + captured: dict[str, object] = {} + monkeypatch.setattr(bootstrap.subprocess, "run", _capturing_run(captured)) + + refresh_codegraph_index(tmp_path) + + assert captured["capture_output"] is False + + +def test_capture_true_folds_stderr_into_error( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _make_existing_db(tmp_path) + monkeypatch.setattr(bootstrap.shutil, "which", _which("codegraph")) + + def fake_run( + cmd: list[str], + *, + check: bool = True, + cwd: Path | None = None, + capture_output: bool = False, + text: bool = False, + ) -> subprocess.CompletedProcess: + raise subprocess.CalledProcessError( + returncode=3, cmd=cmd, stderr="boom: index corrupt" + ) + + monkeypatch.setattr(bootstrap.subprocess, "run", fake_run) + + with pytest.raises(CodeGraphBootstrapError, match="boom: index corrupt"): + refresh_codegraph_index(tmp_path, capture=True) diff --git a/tests/test_codegraph_crawler.py b/tests/test_codegraph_crawler.py new file mode 100644 index 0000000..6429655 --- /dev/null +++ b/tests/test_codegraph_crawler.py @@ -0,0 +1,89 @@ +"""Tests for :class:`whygraph.scan.codegraph_crawler.CodeGraphCrawler`. + +The crawler wraps :func:`refresh_codegraph_index` in a progress-reporting +thread. These tests monkeypatch the refresh so no real ``codegraph`` / +Docker is needed, and drive the crawler via :meth:`Crawler.run` (the +thread entry point, run synchronously here) to exercise the base class's +error capture alongside the crawler's own best-effort handling. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from rich.progress import Progress + +from whygraph.scan import codegraph_crawler +from whygraph.scan.codegraph_crawler import CodeGraphCrawler +from whygraph.services.codegraph.exceptions import CodeGraphBootstrapError + + +def test_work_completes_bar_on_success( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + calls: dict[str, object] = {} + + def fake_refresh( + project_root: Path, *, image: str | None = None, capture: bool = False + ) -> Path: + calls["project_root"] = project_root + calls["capture"] = capture + return project_root / ".codegraph" / "codegraph.db" + + monkeypatch.setattr(codegraph_crawler, "refresh_codegraph_index", fake_refresh) + + with Progress() as progress: + crawler = CodeGraphCrawler(progress, project_root=tmp_path, image=None) + crawler.run() + task = progress.tasks[0] + + assert calls["project_root"] == tmp_path + # The crawler always captures so output can't corrupt the live display. + assert calls["capture"] is True + assert crawler.error is None + assert crawler.warning is None + assert task.total == 1 + assert task.completed == 1 + assert task.finished + + +def test_bootstrap_error_swallowed_into_warning( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + def fake_refresh( + project_root: Path, *, image: str | None = None, capture: bool = False + ) -> Path: + raise CodeGraphBootstrapError("neither `codegraph` nor `docker` is on PATH") + + monkeypatch.setattr(codegraph_crawler, "refresh_codegraph_index", fake_refresh) + + with Progress() as progress: + crawler = CodeGraphCrawler(progress, project_root=tmp_path, image=None) + crawler.run() + + # Best-effort: a CodeGraph failure is a warning, never a scan failure. + assert crawler.error is None + assert crawler.warning is not None + assert "CodeGraph refresh skipped" in crawler.warning + assert "neither `codegraph`" in crawler.warning + + +def test_unexpected_error_propagates_to_error( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + def fake_refresh( + project_root: Path, *, image: str | None = None, capture: bool = False + ) -> Path: + raise RuntimeError("unexpected") + + monkeypatch.setattr(codegraph_crawler, "refresh_codegraph_index", fake_refresh) + + with Progress() as progress: + crawler = CodeGraphCrawler(progress, project_root=tmp_path, image=None) + crawler.run() + + # Only known bootstrap failures are best-effort; anything else is a + # real crawler failure that should fail the scan. + assert crawler.warning is None + assert isinstance(crawler.error, RuntimeError)