diff --git a/graphify/benchmark.py b/graphify/benchmark.py index ee9c3925e..cf2bc6ba2 100644 --- a/graphify/benchmark.py +++ b/graphify/benchmark.py @@ -1,10 +1,7 @@ """Token-reduction benchmark - measures how much context graphify saves vs naive full-corpus approach.""" from __future__ import annotations -import json import sys -from pathlib import Path import networkx as nx -from networkx.readwrite import json_graph from graphify.build import edge_data from graphify.serve import _query_terms @@ -100,13 +97,11 @@ def run_benchmark( Returns dict with: corpus_tokens, avg_query_tokens, reduction_ratio, per_question """ graph_path = graph_path or _default_graph_json() - from graphify.security import check_graph_file_size_cap - check_graph_file_size_cap(Path(graph_path)) - data = json.loads(Path(graph_path).read_text(encoding="utf-8")) - try: - G = json_graph.node_link_graph(data, edges="links") - except TypeError: - G = json_graph.node_link_graph(data) + # Size-cap check + links/edges normalization + node-link parse. A raw + # --no-cluster graph stores edges under "edges" and used to KeyError + # here (#2212). + from graphify.paths import load_node_link_graph + G = load_node_link_graph(graph_path) if corpus_words is None: # Rough estimate: each node label is ~3 words, plus source context diff --git a/graphify/build.py b/graphify/build.py index 7e7ed6157..1f840bbc0 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -1408,7 +1408,8 @@ def build_merge( n_nodes = len(to_remove) if n_nodes: print( - f"[graphify] Pruned {n_nodes} node(s) from {n_files} deleted source file(s).", + f"[graphify] Pruned {n_nodes} node(s) from {n_files} deleted or " + f"excluded source file(s).", file=sys.stderr, ) @@ -1419,14 +1420,15 @@ def build_merge( if edges_to_remove: G.remove_edges_from(edges_to_remove) print( - f"[graphify] Pruned {len(edges_to_remove)} edge(s) from deleted source file(s).", + f"[graphify] Pruned {len(edges_to_remove)} edge(s) from deleted or " + f"excluded source file(s).", file=sys.stderr, ) if not n_nodes and not edges_to_remove: print( - f"[graphify] {n_files} source file(s) deleted since last run — " - f"no matching nodes or edges in graph, already clean.", + f"[graphify] {n_files} source file(s) deleted or excluded since " + f"last run — no matching nodes or edges in graph, already clean.", file=sys.stderr, ) diff --git a/graphify/callflow_html.py b/graphify/callflow_html.py index 181e7493b..dabb2997c 100644 --- a/graphify/callflow_html.py +++ b/graphify/callflow_html.py @@ -227,12 +227,13 @@ def _node_link_payload(data: dict) -> tuple[list, list] | None: return None try: - from networkx.readwrite import json_graph + # Shared loader normalizes the raw writer's "edges" key to "links" + # before parsing; without it an edges-keyed payload raised + # KeyError: 'links' and this function silently returned None even + # though the shape check above accepts "edges" (#2212). + from graphify.paths import load_node_link_graph - try: - graph = json_graph.node_link_graph(data, edges="links") - except TypeError: - graph = json_graph.node_link_graph(data) + graph = load_node_link_graph(data) except Exception: return None diff --git a/graphify/cli.py b/graphify/cli.py index 88f5b51ed..bd8f12bca 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -149,6 +149,7 @@ def _stale_graph_sources( graph_path: Path, scan_root: Path, seen_files: set[str], + detection: dict | None = None, ) -> list[str]: """Source files graph.json still references but the current scan no longer contains (#1909). @@ -173,7 +174,22 @@ def _stale_graph_sources( ``seen_files`` must be the FULL detect output including unclassified files, so nodes from walked-but-unsupported sources (e.g. introspected Cargo.toml manifests) are not misread as stale. + + Paths are compared NFC-normalized on both sides: macOS reports NFD + filenames while graph ``source_file`` entries are typically NFC, and a + raw-string membership test misread every accented live file as stale + (#2210; same class as the manifest-layer #2221/#2224). + + Fail-closed liveness guard (#2210, mirrors watch.py's excluded-vs-deleted + distinction): a source missing from the scan corpus is only pruned when + the file is gone from disk, or when its exclusion is PROVABLE from the + same scan that produced ``seen_files`` — ``detection``'s ``ignored`` / + ``pruned_noise_dirs`` / ``skipped_sensitive`` output, or detect's + sensitivity predicate. An alive file that merely failed the membership + test (path-spelling drift the normalization didn't cover, walk errors, + …) is KEPT and reported, never mass-evicted. """ + from graphify.paths import nfc try: data = json.loads(graph_path.read_text(encoding="utf-8")) except Exception: @@ -203,15 +219,57 @@ def _stale_graph_sources( except (ValueError, OSError, RuntimeError): return False + seen_nfc = {nfc(s) for s in seen_files} + seen_basenames = {nfc(os.path.basename(s)) for s in seen_files} + def _in_seen(p: Path) -> bool: - if str(p) in seen_files: + if nfc(str(p)) in seen_nfc: return True try: - return str(p.resolve()) in seen_files + return nfc(str(p.resolve())) in seen_nfc except (OSError, RuntimeError): return False + # Provable-exclusion evidence from the scan that produced seen_files: + # individually ignored files are exact entries; ignored/noise-pruned + # directories are recorded once with a trailing separator and cover + # their whole subtree. skipped_sensitive entries may carry a + # " [reason]" suffix. + excluded_exact: set[str] = set() + excluded_prefixes: list[str] = [] + if detection: + for entry in list(detection.get("ignored", [])) + list( + detection.get("pruned_noise_dirs", []) + ): + e = nfc(str(entry)) + if e.endswith(os.sep) or e.endswith("/"): + excluded_prefixes.append(e) + else: + excluded_exact.add(e) + for entry in detection.get("skipped_sensitive", []): + excluded_exact.add(nfc(str(entry).split(" [", 1)[0])) + + def _provably_excluded(c: Path) -> bool: + spellings = [nfc(str(c))] + try: + spellings.append(nfc(str(c.resolve()))) + except (OSError, RuntimeError): + pass + for s in spellings: + if s in excluded_exact: + return True + if any(s.startswith(pref) for pref in excluded_prefixes): + return True + try: + from graphify.detect import _is_sensitive as _det_sensitive + if _det_sensitive(c): + return True + except Exception: + pass + return False + stale: list[str] = [] + kept_alive: list[str] = [] checked: set[str] = set() for n in data.get("nodes", []): if not isinstance(n, dict): @@ -238,7 +296,37 @@ def _stale_graph_sources( continue # out-of-root under every anchor: never prune if any(_in_seen(c) for c in in_root): continue # still part of the scan corpus + # Fail-closed liveness guard (#2210): absence from the corpus is + # only deletion evidence when the file is actually gone from disk. + alive = [] + for c in in_root: + try: + if c.exists(): + alive.append(c) + except OSError: + pass + if alive: + if all(_provably_excluded(c) for c in alive): + stale.append(sf) # alive but excluded under current rules (#1909) + else: + kept_alive.append(sf) + continue + # No anchored candidate exists, but a legacy bare-basename spelling + # can't be anchored reliably — a live corpus file with the same name + # means deletion is unproven; keep. + rel_sf = sf.replace("\\", "/") + if "/" not in rel_sf and nfc(rel_sf) in seen_basenames: + kept_alive.append(sf) + continue stale.append(sf) + if kept_alive: + print( + f"[graphify] fail-closed: kept node(s) from {len(kept_alive)} " + "source file(s) that left the scan corpus but still exist on disk " + "(ignore rules or filters changed?). Run a full re-extraction to " + "purge them if the exclusion is intentional.", + file=sys.stderr, + ) return stale @@ -1946,10 +2034,10 @@ def dispatch_command(cmd: str) -> None: f"graph.json {p} is {size} bytes, exceeds {_MERGE_MAX_BYTES}-byte cap" ) data = json.loads(path_obj.read_text(encoding="utf-8")) - try: - return _jg.node_link_graph(data, edges="links"), data - except TypeError: - return _jg.node_link_graph(data), data + # A committed raw (--no-cluster) graph stores edges under "edges"; + # parse via the shared links/edges-normalizing loader (#2212). + from graphify.paths import load_node_link_graph as _lnlg + return _lnlg(data), data try: G_cur, _ = _load_graph(_current_path) G_oth, _ = _load_graph(_other_path) @@ -2729,7 +2817,7 @@ def dispatch_command(cmd: str) -> None: _seen_files = {f for _fl in files_by_type.values() for f in _fl} _seen_files.update(detection.get("unclassified", [])) graph_stale_sources = _stale_graph_sources( - existing_graph_path, target, _seen_files + existing_graph_path, target, _seen_files, detection=detection ) else: print(f"[graphify extract] scanning {target}") diff --git a/graphify/paths.py b/graphify/paths.py index d43bc7766..a1adaf9f2 100644 --- a/graphify/paths.py +++ b/graphify/paths.py @@ -302,3 +302,44 @@ def default_graph_json() -> str: the path is passed explicitly (#1423). """ return str(out_path("graph.json")) + + +def nfc(s: str) -> str: + """NFC-normalize a path string. + + macOS (HFS+/APFS) reports filenames in NFD while manifests, graph + ``source_file`` entries and user input are typically NFC. Comparing raw + strings makes the same file look like two different paths, so any path + membership test must normalize BOTH sides (#2210, #2221/#2224). + """ + import unicodedata + return unicodedata.normalize("NFC", s) + + +def load_node_link_graph(path_or_data): + """Load a graphify graph.json into a networkx graph, accepting both writers. + + The clustered writer stores edges under ``links`` (networkx's node-link + default); the raw ``--no-cluster`` writer stores them under ``edges``. + Consumers that call ``node_link_graph(data, edges="links")`` directly + raise ``KeyError: 'links'`` on a raw graph (#2212) — the ``except + TypeError`` fallback only covers old networkx without the ``edges`` + kwarg, not the missing key. Normalize before parsing, same idiom as + affected.py/serve.py. + + Accepts a path (size-cap-checked via the security module, then parsed) + or an already-parsed dict (no size check — the caller owns any cap). + """ + from networkx.readwrite import json_graph + data = path_or_data + if not isinstance(data, dict): + p = Path(data) + from graphify.security import check_graph_file_size_cap # lazy: security imports paths + check_graph_file_size_cap(p) + data = json.loads(p.read_text(encoding="utf-8")) + if isinstance(data, dict) and "links" not in data and "edges" in data: + data = dict(data, links=data["edges"]) + try: + return json_graph.node_link_graph(data, edges="links") + except TypeError: # networkx too old for the edges kwarg; default is "links" + return json_graph.node_link_graph(data) diff --git a/tests/test_benchmark_raw_graph.py b/tests/test_benchmark_raw_graph.py new file mode 100644 index 000000000..51da5fe02 --- /dev/null +++ b/tests/test_benchmark_raw_graph.py @@ -0,0 +1,103 @@ +"""#2212: run_benchmark must accept a raw --no-cluster graph.json. + +The clustered writer stores edges under "links" (networkx node-link default); +the raw --no-cluster writer stores them under "edges". Consumers calling +node_link_graph(data, edges="links") raised KeyError: 'links' on the raw +shape — the `except TypeError` fallback only covered old networkx versions, +not the missing key. +""" +from __future__ import annotations +import json + +from graphify.benchmark import run_benchmark + + +def _graph_payload(edges_key: str) -> dict: + # Raw extract shape: top-level nodes/edges/hyperedges + token counters. + nodes = [ + { + "id": "auth_flow", + "label": "authentication flow", + "source_file": "auth.py", + "source_location": "L1", + }, + { + "id": "login_handler", + "label": "user login authentication handler", + "source_file": "auth.py", + "source_location": "L10", + }, + { + "id": "main_entry", + "label": "main entry point", + "source_file": "main.py", + "source_location": "L1", + }, + ] + edges = [ + { + "id": "edge_1", + "source": "auth_flow", + "target": "login_handler", + "relation": "calls", + "confidence": "EXTRACTED", + "confidence_score": 1.0, + }, + { + "id": "edge_2", + "source": "login_handler", + "target": "main_entry", + "relation": "used_by", + "confidence": "EXTRACTED", + "confidence_score": 1.0, + }, + ] + return { + "nodes": nodes, + edges_key: edges, + "hyperedges": [], + "input_tokens": 0, + "output_tokens": 0, + } + + +def test_run_benchmark_raw_edges_keyed_graph(tmp_path): + """A --no-cluster graph.json ("edges" key) must not raise KeyError.""" + graph_file = tmp_path / "graph.json" + graph_file.write_text(json.dumps(_graph_payload("edges")), encoding="utf-8") + + result = run_benchmark(graph_path=str(graph_file), corpus_words=5_000) + + assert "error" not in result + assert result["nodes"] == 3 + assert result["edges"] == 2 + assert result["reduction_ratio"] > 0 + assert any( + "authentication" in p["question"] for p in result["per_question"] + ) + + +def test_run_benchmark_links_keyed_graph(tmp_path): + """The clustered writer's "links" key keeps working identically.""" + graph_file = tmp_path / "graph.json" + graph_file.write_text(json.dumps(_graph_payload("links")), encoding="utf-8") + + result = run_benchmark(graph_path=str(graph_file), corpus_words=5_000) + + assert "error" not in result + assert result["nodes"] == 3 + assert result["edges"] == 2 + assert result["reduction_ratio"] > 0 + + +def test_raw_and_links_graphs_benchmark_identically(tmp_path): + """Both spellings of the same graph must produce the same stats.""" + raw_file = tmp_path / "raw.json" + raw_file.write_text(json.dumps(_graph_payload("edges")), encoding="utf-8") + links_file = tmp_path / "links.json" + links_file.write_text(json.dumps(_graph_payload("links")), encoding="utf-8") + + raw = run_benchmark(graph_path=str(raw_file), corpus_words=5_000) + links = run_benchmark(graph_path=str(links_file), corpus_words=5_000) + + assert raw == links diff --git a/tests/test_stale_prune.py b/tests/test_stale_prune.py new file mode 100644 index 000000000..b4997b1ce --- /dev/null +++ b/tests/test_stale_prune.py @@ -0,0 +1,118 @@ +"""#2210: incremental extract's graph-layer prune must not evict ALIVE files. + +_stale_graph_sources compared stored graph source_file spellings against the +current scan with a raw string membership test: (a) no NFC/NFD normalization, +so a macOS NFD on-disk path never matched an NFC graph entry; (b) no liveness +check, so any membership miss was declared "deleted" and pruned even though +the file exists on disk and is in the scan. These tests exercise the NFC +normalization, the fail-closed liveness guard, and that genuinely-deleted +sources are still pruned. +""" +from __future__ import annotations +import json +import unicodedata + +from graphify.cli import _stale_graph_sources +from graphify.detect import detect + +NFC_NAME = unicodedata.normalize("NFC", "café.md") # café.md, composed +NFD_NAME = unicodedata.normalize("NFD", "café.md") # cafe + combining accent + + +def _write_graph(tmp_path, source_files: list[str]): + out = tmp_path / "graphify-out" + out.mkdir(exist_ok=True) + graph_path = out / "graph.json" + nodes = [ + {"id": f"n{i}", "label": f"node {i}", "source_file": sf} + for i, sf in enumerate(source_files) + ] + graph_path.write_text( + json.dumps({"nodes": nodes, "links": []}), encoding="utf-8" + ) + return graph_path + + +def _scan(tmp_path): + detection = detect(tmp_path) + seen = {f for flist in detection["files"].values() for f in flist} + seen.update(detection.get("unclassified", [])) + return detection, seen + + +def test_nfd_disk_nfc_graph_source_not_pruned(tmp_path): + """(a) NFD spelling on disk vs NFC spelling in the graph: NOT stale.""" + docs = tmp_path / "docs" + docs.mkdir() + (docs / NFD_NAME).write_text("# cafe notes\n\nhello\n", encoding="utf-8") + + graph_path = _write_graph(tmp_path, ["docs/" + NFC_NAME]) + detection, seen = _scan(tmp_path) + + stale = _stale_graph_sources(graph_path, tmp_path, seen, detection=detection) + assert stale == [] + + +def test_bare_basename_alive_elsewhere_not_pruned(tmp_path, capsys): + """(b) fail-closed: a legacy bare-basename source_file whose file is + alive at docs/café.md cannot be proven deleted — keep it.""" + docs = tmp_path / "docs" + docs.mkdir() + (docs / NFD_NAME).write_text("# cafe notes\n\nhello\n", encoding="utf-8") + + graph_path = _write_graph(tmp_path, [NFC_NAME]) # bare legacy spelling + detection, seen = _scan(tmp_path) + + stale = _stale_graph_sources(graph_path, tmp_path, seen, detection=detection) + assert stale == [] + + +def test_genuinely_deleted_source_still_pruned(tmp_path): + """(c) a source_file with no file on disk anywhere IS pruned.""" + docs = tmp_path / "docs" + docs.mkdir() + (docs / "keep.md").write_text("# keep\n\nstill here\n", encoding="utf-8") + + graph_path = _write_graph(tmp_path, ["docs/keep.md", "docs/gone.md"]) + detection, seen = _scan(tmp_path) + + stale = _stale_graph_sources(graph_path, tmp_path, seen, detection=detection) + assert stale == ["docs/gone.md"] + + +def test_alive_but_ignored_source_is_pruned(tmp_path): + """#1909 must keep working: an alive file excluded by ignore rules is + provably excluded, so its nodes ARE pruned.""" + docs = tmp_path / "docs" + docs.mkdir() + (docs / "keep.md").write_text("# keep\n\nstill here\n", encoding="utf-8") + (docs / "secret.md").write_text("# secret\n\nexcluded\n", encoding="utf-8") + (tmp_path / ".graphifyignore").write_text("docs/secret.md\n", encoding="utf-8") + + graph_path = _write_graph(tmp_path, ["docs/keep.md", "docs/secret.md"]) + detection, seen = _scan(tmp_path) + + stale = _stale_graph_sources(graph_path, tmp_path, seen, detection=detection) + assert stale == ["docs/secret.md"] + + +def test_alive_unproven_exclusion_kept_with_warning(tmp_path, capsys): + """Fail-closed: an alive in-root file missing from the corpus without + provable exclusion evidence is kept, and the keep is reported.""" + docs = tmp_path / "docs" + docs.mkdir() + (docs / "keep.md").write_text("# keep\n\nstill here\n", encoding="utf-8") + (docs / "other.md").write_text("# other\n\nalive\n", encoding="utf-8") + + graph_path = _write_graph(tmp_path, ["docs/keep.md", "docs/other.md"]) + detection, seen = _scan(tmp_path) + # Simulate a scan that lost docs/other.md for a reason that is NOT a + # provable exclusion (e.g. a walk error): drop it from seen and from + # the detection evidence. + seen = {s for s in seen if not s.endswith("other.md")} + detection = dict(detection, ignored=[], pruned_noise_dirs=[], skipped_sensitive=[]) + + stale = _stale_graph_sources(graph_path, tmp_path, seen, detection=detection) + assert stale == [] + err = capsys.readouterr().err + assert "fail-closed" in err