diff --git a/CHANGELOG.md b/CHANGELOG.md index bcef198b7..f25174693 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ Full release notes with details on each version: [GitHub Releases](https://githu ## 0.9.11 (2026-07-08) +- Fix: file enumeration no longer silently drops a directory subtree. `detect()`'s `os.walk` had no `onerror` handler, so an `os.scandir` failure (a permission error, or a directory created/deleted mid-walk by concurrent writes) was swallowed and that whole subtree vanished from the scan with no log, yielding a silently partial `graph.json`. The walk now records every skipped directory (surfaced in the result's `walk_errors`) and warns to stderr, while still enumerating the rest. Relatedly, `to_json`'s anti-shrink guard (#479) now fails safe: a non-empty but unreadable existing `graph.json` refuses the overwrite (pass `force=True` to override) instead of silently clobbering a good graph; an empty file still proceeds. +- Fix: Pascal/Delphi extractors no longer emit duplicate `method`/`contains`/`inherits` edges. A class method declared in the interface section and defined in the implementation section each emitted an edge to the same node, so ~half of a Pascal graph's method edges were doubled (skewing degree/centrality and tripping the new cross-file resolver's god-node guard). Both extractors now dedup edges on (source, target, relation), mirroring the existing node dedup. - Fix: Pascal/Delphi call resolution is scoped to the caller's class + inherits chain, and calls to methods inherited across file boundaries now resolve (#1739, thanks @richtext). Both extractors previously resolved every call via a single file-wide `{name: node_id}` dict, so two unrelated classes with a same-named method (property accessors, generated COM/TLB wrappers) collapsed onto whichever was inserted last, producing wrong cross-class `calls` edges. Resolution now walks own-class then ancestor chain then file-level free functions, emitting no edge when ambiguous (same god-node guard as the Ruby resolver). A new corpus-wide resolver (`graphify/pascal_resolution.py`) resolves calls from a descendant to a base-class method declared in a different file (the common generated-base/manual-descendant split). Also stops emitting a duplicate cross-file base-class stub carrying the wrong `source_file`. - Fix: query ranking no longer lets a lone generic term that exact-matches a short leaf label hijack seed selection in multi-term queries (#1602/#1724, thanks @fkhawajagh). `_score_nodes` scales the per-term exact/prefix tiers by squared term coverage; single-term and full-coverage queries are unchanged. - Fix: Kotlin enum entries are extracted as nodes with `case_of` edges to their enum (#1700, thanks @ivanzhilovich). Closes the Kotlin half of #1700 (the Java half shipped in 0.9.10 via #1719); `enum class ChatType { NORMAL, GROUP, SYSTEM }` now yields NORMAL/GROUP/SYSTEM nodes and "where is ChatType.X used" works for Kotlin. diff --git a/graphify/detect.py b/graphify/detect.py index 85f4b8b8b..d6eaf3318 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -1107,9 +1107,29 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace: seen: set[Path] = set() all_files: list[Path] = [] + # os.walk swallows os.scandir errors by default (no onerror -> the failing + # directory subtree is silently skipped). That turns a transient + # PermissionError, or a directory created/deleted mid-walk (e.g. concurrent + # writes racing the scan), into a partial file list and, downstream, a + # silently partial graph.json. Record and surface every skipped directory + # so an incomplete enumeration is visible rather than silent. + walk_errors: list[str] = [] + + def _on_walk_error(err: OSError) -> None: + import sys as _sys + target = getattr(err, "filename", None) or "" + walk_errors.append(f"{target}: {err}") + print( + f"[graphify] WARNING: could not scan {target} ({err}); " + f"its files are missing from this run's enumeration.", + file=_sys.stderr, + ) + for scan_root in scan_paths: in_memory_tree = memory_dir.exists() and str(scan_root).startswith(str(memory_dir)) - for dirpath, dirnames, filenames in os.walk(scan_root, followlinks=follow_symlinks): + for dirpath, dirnames, filenames in os.walk( + scan_root, followlinks=follow_symlinks, onerror=_on_walk_error + ): dp = Path(dirpath) if follow_symlinks and os.path.islink(dirpath): real = os.path.realpath(dirpath) @@ -1245,6 +1265,7 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace: "warning": warning, "skipped_sensitive": skipped_sensitive, "unclassified": sorted(unclassified), + "walk_errors": walk_errors, "graphifyignore_patterns": len(ignore_patterns), "scan_root": str(root.resolve()), } diff --git a/graphify/export.py b/graphify/export.py index bf27eedc4..be44a9d03 100644 --- a/graphify/export.py +++ b/graphify/export.py @@ -184,11 +184,44 @@ def to_json(G: nx.Graph, communities: dict[int, list[str]], output_path: str, *, # Safety check: refuse to silently shrink an existing graph (#479) existing_path = Path(output_path) if not force and existing_path.exists(): + from graphify.security import check_graph_file_size_cap try: - from graphify.security import check_graph_file_size_cap check_graph_file_size_cap(existing_path) - existing_data = json.loads(existing_path.read_text(encoding="utf-8")) - existing_n = len(existing_data.get("nodes", [])) + except Exception: + # Existing graph.json trips the size cap; reading it to compare would + # be the very DoS the cap guards against. Can't verify — let the new + # graph replace the oversized file. + oversized = True + else: + oversized = False + if not oversized: + try: + raw = existing_path.read_text(encoding="utf-8") + except Exception: + raw = "" + if not raw.strip(): + # Empty/whitespace existing file (e.g. a freshly touched path): + # no nodes to lose, so any new graph is a growth — proceed. + existing_n = 0 + else: + try: + existing_data = json.loads(raw) + existing_n = len(existing_data.get("nodes", [])) + except Exception as exc: + # Non-empty but unparseable existing graph (corrupt or a + # mid-write): we cannot verify the new graph is not a silent + # shrink. Fail SAFE — refuse rather than overwrite. A + # fail-OPEN here (the prior behavior) is the silent data-loss + # path #479 exists to prevent: a transiently unreadable + # graph.json would let a partial rebuild clobber a good one. + import sys as _sys + print( + f"[graphify] WARNING: existing {existing_path} could not be " + f"read to verify the new graph is not smaller ({exc}). " + f"Refusing to overwrite; pass force=True to override.", + file=_sys.stderr, + ) + return False new_n = G.number_of_nodes() if new_n < existing_n: import sys as _sys @@ -203,8 +236,6 @@ def to_json(G: nx.Graph, communities: dict[int, list[str]], output_path: str, *, file=_sys.stderr, ) return False - except Exception: - pass # unreadable existing file — proceed with write node_community = _node_community_map(communities) _labels: dict[int, str] = {int(k): v for k, v in (community_labels or {}).items()} diff --git a/graphify/extractors/pascal.py b/graphify/extractors/pascal.py index 8b488feaa..398edb22e 100644 --- a/graphify/extractors/pascal.py +++ b/graphify/extractors/pascal.py @@ -243,6 +243,7 @@ def _extract_pascal_regex(path: Path) -> dict: edges: list[dict] = [] seen_ids: set[str] = set() seen_call_pairs: set[tuple[str, str]] = set() + seen_edges: set[tuple[str, str, str]] = set() def _add_node(nid: str, label: str, line: int) -> None: if nid not in seen_ids: @@ -256,6 +257,14 @@ def _extract_pascal_regex(path: Path) -> dict: }) def _add_edge(src: str, tgt: str, relation: str, line: int, context: str | None = None) -> None: + # A class method declared in the interface section and defined in the + # implementation section both emit a `method` edge to the same node, so + # dedup on (src, tgt, relation) to keep the graph from carrying doubled + # method/contains/inherits edges (mirrors _add_node's seen_ids guard). + key = (src, tgt, relation) + if key in seen_edges: + return + seen_edges.add(key) edge: dict = { "source": src, "target": tgt, @@ -459,6 +468,7 @@ def extract_pascal(path: Path) -> dict: nodes: list[dict] = [] edges: list[dict] = [] seen_ids: set[str] = set() + seen_edges: set[tuple[str, str, str]] = set() proc_bodies: list[tuple[str, Any, str, str]] = [] # (proc_nid, body_node, container, name_lower) @@ -478,6 +488,14 @@ def extract_pascal(path: Path) -> dict: confidence: str = "EXTRACTED", weight: float = 1.0, context: str | None = None, ) -> None: + # A class method declared in the interface section and defined in the + # implementation section both emit a `method` edge to the same node, so + # dedup on (src, tgt, relation) to keep the graph from carrying doubled + # method/contains/inherits edges (mirrors add_node's seen_ids guard). + key = (src, tgt, relation) + if key in seen_edges: + return + seen_edges.add(key) edge: dict[str, Any] = { "source": src, "target": tgt, "relation": relation, "confidence": confidence, "source_file": str_path, diff --git a/tests/test_detect.py b/tests/test_detect.py index b26d37ec5..7ff769768 100644 --- a/tests/test_detect.py +++ b/tests/test_detect.py @@ -1597,3 +1597,38 @@ def test_detect_unclassified_empty_when_all_supported(tmp_path): (tmp_path / "README.md").write_text("# hi\n") res = detect(tmp_path) assert res.get("unclassified", []) == [] + + +def test_detect_reports_walk_errors_key(): + """detect() always surfaces a walk_errors list so callers can tell whether + enumeration was complete.""" + import tempfile + d = Path(tempfile.mkdtemp()) + (d / "a.py").write_text("def f(): pass\n") + res = detect(d) + assert "walk_errors" in res + assert res["walk_errors"] == [] + + +def test_detect_surfaces_unreadable_dir_instead_of_silent_skip(tmp_path, capsys): + """os.walk silently skips a subtree whose scandir raises (permissions, or a + dir deleted mid-walk); that under-enumeration used to be invisible and could + yield a silently partial graph. detect() now records it in walk_errors and + warns, while still enumerating the rest of the tree.""" + import os + if os.geteuid() == 0: + import pytest + pytest.skip("running as root: chmod 000 does not block scandir") + (tmp_path / "a.py").write_text("def f(): pass\n") + locked = tmp_path / "locked" + locked.mkdir() + (locked / "b.py").write_text("def g(): pass\n") + os.chmod(locked, 0o000) + try: + res = detect(tmp_path) + finally: + os.chmod(locked, 0o755) # restore for cleanup + code = res["files"]["code"] + assert any(f.endswith("a.py") for f in code) # rest of tree still enumerated + assert len(res["walk_errors"]) >= 1 + assert "could not scan" in capsys.readouterr().err diff --git a/tests/test_export.py b/tests/test_export.py index be4743bc5..772331826 100644 --- a/tests/test_export.py +++ b/tests/test_export.py @@ -603,3 +603,39 @@ def test_backup_env_disable(tmp_path, monkeypatch): (tmp_path / "graph.json").write_text('{"nodes":[],"links":[]}') (tmp_path / ".graphify_semantic_marker").write_text("{}") assert backup_if_protected(tmp_path) is None + + +def _mkG(n): + import networkx as nx + G = nx.Graph() + for i in range(n): + G.add_node(f"n{i}", label=f"n{i}", community=0) + return G + + +def test_to_json_refuses_shrink(tmp_path): + """#479: refuse to silently overwrite an existing graph with fewer nodes.""" + p = tmp_path / "graph.json" + json.dump({"nodes": [{"id": f"n{i}"} for i in range(5)]}, p.open("w")) + assert to_json(_mkG(2), {}, str(p), force=False) is False + assert to_json(_mkG(2), {}, str(p), force=True) is True # force overrides + + +def test_to_json_fails_safe_on_corrupt_existing(tmp_path): + """A non-empty but unparseable existing graph.json (corrupt or mid-write) + must NOT be silently overwritten — we can't verify the new graph isn't a + partial shrink, so fail safe (refuse) unless force is given.""" + p = tmp_path / "graph.json" + p.write_text("{ this has content but is not valid json") + assert to_json(_mkG(10), {}, str(p), force=False) is False + assert to_json(_mkG(10), {}, str(p), force=True) is True + + +def test_to_json_proceeds_on_empty_existing(tmp_path): + """An empty/whitespace existing file has no nodes to lose, so it is not a + shrink risk — the write proceeds.""" + p = tmp_path / "graph.json" + p.write_text("") + assert to_json(_mkG(3), {}, str(p), force=False) is True + data = json.loads(p.read_text()) + assert len(data["nodes"]) == 3 diff --git a/tests/test_pascal.py b/tests/test_pascal.py index 36c1b8747..e54564bcf 100644 --- a/tests/test_pascal.py +++ b/tests/test_pascal.py @@ -320,3 +320,26 @@ def test_dfm_dispatch_registered(): def test_dfm_detect_extension_registered(): from graphify.detect import CODE_EXTENSIONS assert ".dfm" in CODE_EXTENSIONS + + +def _dup_edges(r): + from collections import Counter + triples = Counter((e["source"], e["target"], e["relation"]) for e in r["edges"]) + return {k: v for k, v in triples.items() if v > 1} + + +def test_pascal_no_duplicate_method_edges_tree_sitter(): + """A class method appears in both the interface declaration and the + implementation; each used to emit a `method` edge to the same node, so the + graph carried doubled method/contains/inherits edges (skewing degree and + breaking the cross-file inherited-call resolver's god-node guard). Edges are + now deduped on (source, target, relation).""" + from graphify.extract import extract_pascal + r = extract_pascal(FIXTURES / "sample.pas") + assert _dup_edges(r) == {}, f"duplicate edges: {_dup_edges(r)}" + + +def test_pascal_no_duplicate_method_edges_regex(): + from graphify.extract import _extract_pascal_regex + r = _extract_pascal_regex(FIXTURES / "sample.pas") + assert _dup_edges(r) == {}, f"duplicate edges: {_dup_edges(r)}"