diff --git a/README.md b/README.md index 272c5f6f81..d0e9fae4e2 100644 --- a/README.md +++ b/README.md @@ -748,6 +748,7 @@ graphify extract ./docs --no-cluster # raw extraction only, skip clust graphify extract ./docs --timing # print per-stage wall-clock timings to stderr (also works on cluster-only) graphify extract ./docs --force # overwrite graph.json even if new graph has fewer nodes (use after refactors or to clear ghost duplicates) graphify extract ./docs --dedup-llm # LLM tiebreaker for ambiguous entity pairs (uses same API key) +graphify extract ./src --no-dedup # skip entity dedup; on an incremental merge this also arms the shrink guard that refuses to drop untouched files' nodes graphify extract ./docs --global --as myrepo # extract and register into the cross-project global graph GRAPHIFY_MAX_OUTPUT_TOKENS=32768 graphify extract ./docs --backend claude # raise output cap for dense corpora diff --git a/graphify/cli.py b/graphify/cli.py index cb30420473..2cca47f2d4 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -2849,7 +2849,7 @@ def _to_simple(g: "_nx.Graph") -> "_nx.Graph": print( "Usage: graphify extract [--backend gemini|kimi|claude|openai|deepseek|ollama] " "[--model M] [--mode deep] [--out DIR|--output DIR] [--google-workspace] [--no-cluster] " - "[--no-gitignore] [--code-only] " + "[--no-gitignore] [--code-only] [--no-dedup] " "[--max-workers N] [--token-budget N] [--max-concurrency N] " "[--api-timeout S] [--postgres DSN] [--cargo] [--allow-partial] [--timing]", file=sys.stderr, @@ -2875,6 +2875,13 @@ def _to_simple(g: "_nx.Graph") -> "_nx.Graph": cli_allow_partial: bool = False no_cluster = False dedup_llm = False + # --no-dedup: skip entity deduplication entirely. On an incremental + # merge the fuzzy pass runs over the COMBINED node set (existing graph + + # new chunk), so a small diff merged into a large graph can collapse + # pre-existing nodes from files the diff never touched. Turning dedup + # off also arms build_merge's #479 shrink guard, which is disabled while + # dedup is on because fuzzy merging shrinks the graph legitimately (#2881). + no_dedup = False google_workspace = False global_merge = False code_only = False @@ -2943,6 +2950,8 @@ def _parse_float(name: str, raw: str) -> float: no_cluster = True; i += 1 elif a == "--dedup-llm": dedup_llm = True; i += 1 + elif a == "--no-dedup": + no_dedup = True; i += 1 elif a == "--code-only": code_only = True; i += 1 elif a == "--google-workspace": @@ -3001,6 +3010,16 @@ def _parse_float(name: str, raw: str) -> float: print("error: must specify a path to scan or a --postgres DSN", file=sys.stderr) sys.exit(1) + if no_dedup and dedup_llm: + # --dedup-llm is pass 3 of the dedup pipeline, so with dedup off it + # would be a silent no-op that still demands an API key. + print( + "error: --no-dedup and --dedup-llm are mutually exclusive " + "(--dedup-llm is a tiebreaker inside the dedup pass)", + file=sys.stderr, + ) + sys.exit(2) + _VALID_MODES = {"deep"} if extract_mode is not None and extract_mode not in _VALID_MODES: print( @@ -3906,16 +3925,24 @@ def _invalidate_file_manifest_for_db_graph() -> None: for _src in list(excluded_files) + graph_stale_sources: if _src not in _prune_sources: _prune_sources.append(_src) - G = _build_merge( - [merged], - graph_path=existing_graph_path, - prune_sources=_prune_sources or None, - dedup=True, - dedup_llm_backend=dedup_backend, - root=target, - ) + try: + G = _build_merge( + [merged], + graph_path=existing_graph_path, + prune_sources=_prune_sources or None, + dedup=not no_dedup, + dedup_llm_backend=dedup_backend, + root=target, + ) + except ValueError as exc: + # --no-dedup arms build_merge's #479 shrink guard, which refuses + # to drop nodes belonging to files this run neither re-extracted + # nor pruned. Report the refusal instead of a traceback (#2881): + # graph.json on disk is untouched, so the old graph is intact. + print(f"[graphify extract] {exc}", file=sys.stderr) + sys.exit(1) else: - G = _build([merged], dedup=True, dedup_llm_backend=dedup_backend, root=target) + G = _build([merged], dedup=not no_dedup, dedup_llm_backend=dedup_backend, root=target) stages.mark("build") if G.number_of_nodes() == 0: print( diff --git a/tests/test_no_dedup_flag.py b/tests/test_no_dedup_flag.py new file mode 100644 index 0000000000..c0f2caf5ac --- /dev/null +++ b/tests/test_no_dedup_flag.py @@ -0,0 +1,129 @@ +"""`graphify extract --no-dedup` (#2881). + +The incremental merge path hardcoded `dedup=True`, so fuzzy dedup always ran +over the COMBINED node set (existing graph + new chunk). On a large graph a +small diff could therefore collapse pre-existing nodes belonging to files the +diff never touched, and the #479 shrink guard — the one thing that would have +caught it — is deliberately skipped while dedup is on, because fuzzy merging +shrinks the graph legitimately. There was no way to opt out from the CLI. +""" +from __future__ import annotations + +import graphify.__main__ as mainmod + + +def _corpus(tmp_path): + corpus = tmp_path / "corpus" + corpus.mkdir() + (corpus / "main.go").write_text("package main\nfunc main() {}\n") + return corpus + + +def _run(monkeypatch, argv): + """Run the CLI and return its exit code (0 when main() simply returns).""" + monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None) + monkeypatch.setattr(mainmod.sys, "argv", argv) + try: + mainmod.main() + except SystemExit as exc: + return exc.code or 0 + return 0 + + +def _capture_dedup(monkeypatch): + """Record the `dedup` kwarg both build entry points are called with. + + Patching `graphify.build` does reach the CLI's `_build` / `_build_merge` + aliases, even though it refers to them by those names: the + + from graphify.build import build as _build, build_merge as _build_merge + + is function-local to `dispatch_command`, so the alias is bound when the + command runs, which is after this patch is installed. A module-level import + would bind at import time and make the patch inert — `_assert_spied` below + turns that into a loud failure rather than a test that quietly asserts + nothing. + """ + import graphify.build as buildmod + + seen: dict[str, bool] = {} + real_build = buildmod.build + real_merge = buildmod.build_merge + + def fake_build(chunks, *a, **kw): + seen["build"] = kw.get("dedup", True) + return real_build(chunks, *a, **kw) + + def fake_merge(chunks, *a, **kw): + seen["build_merge"] = kw.get("dedup", True) + return real_merge(chunks, *a, **kw) + + monkeypatch.setattr(buildmod, "build", fake_build) + monkeypatch.setattr(buildmod, "build_merge", fake_merge) + return seen + + +def _assert_spied(seen: dict, entry_point: str) -> None: + """Fail loudly if the spy never fired, so no assertion is vacuous.""" + assert entry_point in seen, ( + f"{entry_point}() was never called through the patched " + f"graphify.build symbol — the spy is inert and every dedup assertion " + f"below it would be vacuous. Did the CLI's import of it move to module " + f"scope, or did this run take a path that skips the build stage?" + ) + + +def test_no_dedup_flag_disables_dedup(monkeypatch, tmp_path): + corpus = _corpus(tmp_path) + seen = _capture_dedup(monkeypatch) + code = _run(monkeypatch, [ + "graphify", "extract", str(corpus), "--code-only", + "--no-dedup", "--out", str(tmp_path / "out"), + ]) + assert code == 0 + _assert_spied(seen, "build") + assert seen["build"] is False + + +def test_dedup_is_on_by_default(monkeypatch, tmp_path): + corpus = _corpus(tmp_path) + seen = _capture_dedup(monkeypatch) + code = _run(monkeypatch, [ + "graphify", "extract", str(corpus), "--code-only", + "--out", str(tmp_path / "out"), + ]) + assert code == 0 + _assert_spied(seen, "build") + assert seen["build"] is True + + +def test_no_dedup_reaches_the_incremental_merge(monkeypatch, tmp_path): + corpus = _corpus(tmp_path) + out = tmp_path / "out" + # First run establishes graph.json, so the second run takes the + # build_merge (incremental) path rather than build(). + assert _run(monkeypatch, [ + "graphify", "extract", str(corpus), "--code-only", + "--out", str(out), + ]) == 0 + + (corpus / "other.go").write_text("package main\nfunc other() {}\n") + seen = _capture_dedup(monkeypatch) + assert _run(monkeypatch, [ + "graphify", "extract", str(corpus), "--code-only", + "--no-dedup", "--out", str(out), + ]) == 0 + _assert_spied(seen, "build_merge") + assert seen["build_merge"] is False, ( + "the incremental path hardcoded dedup=True, which is the bug" + ) + + +def test_no_dedup_conflicts_with_dedup_llm(monkeypatch, tmp_path, capsys): + corpus = _corpus(tmp_path) + code = _run(monkeypatch, [ + "graphify", "extract", str(corpus), "--code-only", + "--no-dedup", "--dedup-llm", "--out", str(tmp_path / "out"), + ]) + assert code == 2 + assert "mutually exclusive" in capsys.readouterr().err