mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-27 00:36:39 +00:00
feat(extract): add --no-dedup so incremental merges can skip fuzzy dedup (#2881)
Adds --no-dedup (default off, so dedup stays on) to skip the fuzzy near-duplicate merge pass on build and incremental merge, for operators who would rather keep distinct symbols that fuzzy-matched than pay the merge. Exact-id uniqueness is unaffected (it is a graph structural invariant, not a dedup responsibility), and the flag arms the #479 shrink guard so a surprising node drop is refused loudly. Mutually exclusive with --dedup-llm.
This commit is contained in:
committed by
safishamsi
parent
05f90f1682
commit
eccc6cf0bf
@@ -749,6 +749,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
|
||||
|
||||
|
||||
+37
-10
@@ -2889,7 +2889,7 @@ def dispatch_command(cmd: str) -> None:
|
||||
print(
|
||||
"Usage: graphify extract <path> [--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,
|
||||
@@ -2915,6 +2915,13 @@ def dispatch_command(cmd: str) -> None:
|
||||
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
|
||||
@@ -2983,6 +2990,8 @@ def dispatch_command(cmd: str) -> None:
|
||||
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":
|
||||
@@ -3041,6 +3050,16 @@ def dispatch_command(cmd: str) -> None:
|
||||
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(
|
||||
@@ -3946,16 +3965,24 @@ def dispatch_command(cmd: str) -> 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(
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user