diff --git a/graphify/extract.py b/graphify/extract.py index 3b39f1fb8c..83bb72393d 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -6678,7 +6678,16 @@ def _describe_syntax_error(rel: str, line: "int | None", kept: int) -> str: # marker set in the per-file extractor. Populated just before the pass that uses it. callable_nids: set[str] = set() - _augment_symbol_resolution_edges(paths, all_nodes, all_edges, root) + # #2230: extend cross-file symbol resolution with the same caller-supplied + # unchanged-corpus nodes (resolution_context_nodes) that already widen the + # direct-call/indirect-call indexes below (#2406) — otherwise an + # incremental run that re-extracts only a changed file can never rebind + # its INFERRED calls/imports edges to a symbol defined in an unchanged + # neighbor, and the merge drops the graph's old copies of those edges + # since the changed file's per-file result replaced them. + _augment_symbol_resolution_edges( + paths, all_nodes, all_edges, root, resolution_context_nodes + ) # Merge a header-declared class (and its methods) with its sibling-impl # definition into ONE node (C/C++/ObjC #1547/#1556). Runs BEFORE the id-remap @@ -7154,7 +7163,10 @@ def _learn(e: dict) -> None: if py_paths: py_results = [r for r, p in zip(per_file, paths) if p.suffix == ".py"] try: - cross_file_edges = _resolve_cross_file_imports(py_results, py_paths, all_nodes, all_edges) + cross_file_edges = _resolve_cross_file_imports( + py_results, py_paths, all_nodes, all_edges, + resolution_context_nodes, root, + ) all_edges.extend(cross_file_edges) except Exception as exc: import logging diff --git a/graphify/extractors/resolution.py b/graphify/extractors/resolution.py index 13189609d2..72f7ddcc9e 100644 --- a/graphify/extractors/resolution.py +++ b/graphify/extractors/resolution.py @@ -1104,6 +1104,7 @@ def _apply_symbol_resolution_facts( edges: list[dict], root: Path, facts: _SymbolResolutionFacts, + context_nodes: list[dict] | None = None, ) -> None: """Apply language-provided import/export/use facts to graph edges.""" if not ( @@ -1144,6 +1145,33 @@ def _apply_symbol_resolution_facts( member_symbol_keys.discard(key) symbol_nodes[key] = str(node["id"]) + # #2230: `nodes` covers only files extracted THIS run. On an incremental + # rebuild that re-extracts a single changed file, an import target + # defined in an unchanged neighbor (e.g. `exc.py`) has no node in `nodes`, + # so the lookups below found nothing and the resulting INFERRED + # calls/imports edge was silently dropped — even though the neighbor's + # node still exists in the merged graph. `context_nodes` is the same + # caller-supplied unchanged-corpus set that already widens the direct- + # call/indirect-call indexes further down in extract() (#2406); folding + # it in here too lets cross-file symbol resolution bind to it. Batch + # nodes win on key collision (skip keys `nodes` already populated), and + # nothing here is appended to `nodes`/`all_nodes` — ownership of these + # symbols stays with the unchanged files that already emit them. + for node in context_nodes or (): + source_path = _js_source_path(str(node.get("source_file", "")), root) + if source_path is None: + continue + raw_label = str(node.get("label", "")).strip() + label = raw_label.strip("()").lstrip(".") + if not label or not node.get("id"): + continue + key = (source_path, label) + if key in symbol_nodes: + continue + if raw_label.startswith("."): + member_symbol_keys.add(key) + symbol_nodes[key] = str(node["id"]) + def ensure_symbol_node(path: Path, name: str, line: int) -> str: resolved_path = _resolve_cached(path) existing = symbol_nodes.get((resolved_path, name)) @@ -2386,17 +2414,20 @@ def _augment_symbol_resolution_edges( nodes: list[dict], edges: list[dict], root: Path, + context_nodes: list[dict] | None = None, ) -> None: facts = _SymbolResolutionFacts() _collect_js_symbol_resolution_facts(paths, facts) _collect_python_symbol_resolution_facts(paths, root, facts) - _apply_symbol_resolution_facts(paths, nodes, edges, root, facts) + _apply_symbol_resolution_facts(paths, nodes, edges, root, facts, context_nodes) def _resolve_cross_file_imports( per_file: list[dict], paths: list[Path], all_nodes: list[dict] | None = None, all_edges: list[dict] | None = None, + context_nodes: list[dict] | None = None, + root: Path | None = None, ) -> list[dict]: """ Two-pass import resolution: turn file-level imports into class-level edges. @@ -2448,6 +2479,44 @@ def _resolve_cross_file_imports( if src_path.stem not in bare_to_qualified: bare_to_qualified[src_path.stem] = fq_stem + # #2230: `per_file` covers only files re-extracted THIS run, so on an + # incremental run that re-extracts a single changed file, a class it + # imports from an unchanged neighbor is absent from stem_to_entities and + # `resolve_import` below silently finds nothing — the resulting `uses` + # edge disappears even though the neighbor's node still exists in the + # merged graph. `context_nodes` is the same caller-supplied + # unchanged-corpus set threaded through `_apply_symbol_resolution_facts` + # above; folding it into Pass 1's index (but never into `paths`/`per_file`, + # so Pass 2 below still only walks the re-extracted files) lets an import + # of an unchanged symbol resolve again. Batch entries win on collision. + for node in context_nodes or (): + src = node.get("source_file", "") + if not src: + continue + src_path = Path(src) + # Persisted context nodes (from the caller's stored graph) carry a + # root-relative source_file, while this batch's own nodes carry the + # absolute path form extract() was invoked with; _file_stem stringifies + # whatever it is given, so the two forms mint different keys for the + # same file unless anchored to a common (absolute) form first. + if root is not None and not src_path.is_absolute(): + try: + src_path = (root / src_path).resolve() + except (OSError, RuntimeError): + pass + fq_stem = _file_stem(src_path) + label = node.get("label", "") + nid = node.get("id", "") + if ( + label + and not label.endswith((")", ".py")) + and "_" not in label[:1] + and node.get("file_type") != "rationale" + and label not in stem_to_entities.get(fq_stem, {}) + ): + stem_to_entities.setdefault(fq_stem, {})[label] = nid + bare_to_qualified.setdefault(src_path.stem, fq_stem) + # Pass 2: for each file, find `from .X import A, B, C`, then attribute the # `uses` edge to the specific local symbol (class OR function) whose body # actually references the imported name — not to every class that merely diff --git a/tests/test_imported_export_forwarding.py b/tests/test_imported_export_forwarding.py index 54ad4c535c..14c841bdb0 100644 --- a/tests/test_imported_export_forwarding.py +++ b/tests/test_imported_export_forwarding.py @@ -200,7 +200,7 @@ def test_repointed_target_file_tracks_definition_and_preserves_source_site(tmp_p original = resolution._apply_symbol_resolution_facts observed = [] - def tracked(paths, nodes, edges, root, facts): + def tracked(paths, nodes, edges, root, facts, context_nodes=None): def authored(): return next( e @@ -211,7 +211,7 @@ def authored(): ) before = dict(authored()) - original(paths, nodes, edges, root, facts) + original(paths, nodes, edges, root, facts, context_nodes) observed.append((before, dict(authored()))) monkeypatch.setattr(resolution, "_apply_symbol_resolution_facts", tracked) diff --git a/tests/test_incremental.py b/tests/test_incremental.py index 62c1203257..8338780a6e 100644 --- a/tests/test_incremental.py +++ b/tests/test_incremental.py @@ -338,3 +338,91 @@ def test_update_prunes_a_removed_imports_edge(tmp_path): if e.get("relation") in ("imports", "imports_from") and str(e.get("source_file", "")).endswith("a.py")] assert not stale, f"removed import's edge survived update (stale): {stale}" + + +def _write_cross_file_symbol_corpus(proj: Path) -> tuple[Path, Path]: + """#2230 repro corpus: a.py raises a class it imports from exc.py.""" + pkg = proj / "pkg" + pkg.mkdir(parents=True) + (pkg / "__init__.py").write_text("", encoding="utf-8") + (pkg / "exc.py").write_text( + "class BadData(Exception):\n pass\n", encoding="utf-8" + ) + a = pkg / "a.py" + a.write_text( + "from .exc import BadData\n\n\ndef load():\n raise BadData()\n", + encoding="utf-8", + ) + return a, pkg / "exc.py" + + +def _cross_file_baddata_edges(edges: list[dict]) -> list[dict]: + return [ + e for e in edges + if str(e.get("source_file", "")).endswith("a.py") + and ("baddata" in str(e.get("source", "")).lower() + or "baddata" in str(e.get("target", "")).lower()) + ] + + +def test_extract_no_cluster_incremental_regenerates_cross_file_symbol_edges(tmp_path): + """#2230: an incremental --no-cluster extract of ONE changed file must + regenerate its INFERRED cross-file calls/imports/uses edges to a symbol + defined in an UNCHANGED neighbor, not just to unchanged endpoints already + covered by #2169's target_file canonicalization.""" + proj = tmp_path / "proj" + a, exc = _write_cross_file_symbol_corpus(proj) + + first = _run(["extract", str(proj), "--code-only", "--no-cluster"], tmp_path) + assert first.returncode == 0, first.stderr + gj = proj / "graphify-out" / "graph.json" + base_edges = _cross_file_baddata_edges(_edges(gj)) + assert base_edges, "sanity: full scan should link a.py to exc.py's BadData" + base_relations = {e["relation"] for e in base_edges} + assert "uses" in base_relations, base_relations + + # Change ONLY a.py; exc.py is untouched, so the incremental scan re-extracts + # a.py alone. + a.write_text(a.read_text(encoding="utf-8") + "\n# touched\n", encoding="utf-8") + second = _run(["extract", str(proj), "--code-only", "--no-cluster"], tmp_path) + assert second.returncode == 0, second.stderr + assert "incremental scan" in second.stdout.lower(), second.stdout + + after_edges = _cross_file_baddata_edges(_edges(gj)) + after_relations = {e["relation"] for e in after_edges} + assert after_relations == base_relations, ( + f"incremental --no-cluster lost cross-file symbol edges: " + f"missing={base_relations - after_relations}" + ) + + +def test_extract_clustered_incremental_regenerates_cross_file_symbol_edges(tmp_path): + """#2230, clustered path: a second `graphify extract` (clustering ON) that + only re-extracts one changed file must regenerate its INFERRED cross-file + edges to a symbol defined in an unchanged neighbor, mirroring the + --no-cluster case above. Uses `--code-only` so the run stays fully local + (no LLM backend needed) and takes cli.py's own incremental-scan branch + (the one that threads resolution_context_nodes from the persisted graph), + as opposed to `--no-cluster`'s separate incremental-merge path.""" + proj = tmp_path / "proj" + a, exc = _write_cross_file_symbol_corpus(proj) + + first = _run(["extract", str(proj), "--code-only"], tmp_path) + assert first.returncode == 0, first.stderr + gj = proj / "graphify-out" / "graph.json" + base_edges = _cross_file_baddata_edges(_edges(gj)) + assert base_edges, "sanity: full scan should link a.py to exc.py's BadData" + base_relations = {e["relation"] for e in base_edges} + assert {"calls", "imports"} <= base_relations, base_relations + + a.write_text(a.read_text(encoding="utf-8") + "\n# touched\n", encoding="utf-8") + second = _run(["extract", str(proj), "--code-only"], tmp_path) + assert second.returncode == 0, second.stderr + assert "incremental scan" in second.stdout.lower(), second.stdout + + after_edges = _cross_file_baddata_edges(_edges(gj)) + after_relations = {e["relation"] for e in after_edges} + assert after_relations == base_relations, ( + f"clustered incremental extract lost cross-file symbol edges: " + f"missing={base_relations - after_relations}" + ) diff --git a/tests/test_incremental_cross_file_symbol_resolution.py b/tests/test_incremental_cross_file_symbol_resolution.py new file mode 100644 index 0000000000..877e680919 --- /dev/null +++ b/tests/test_incremental_cross_file_symbol_resolution.py @@ -0,0 +1,71 @@ +"""#2230: incremental symbol resolution must regenerate INFERRED cross-file +edges to symbols defined in an unchanged neighbor. + +Cross-file resolution (`_augment_symbol_resolution_edges` and +`_resolve_cross_file_imports`) used to index only nodes extracted THIS run, +so an incremental rebuild of a single changed file could never re-bind an +edge whose target lives in an unchanged sibling: the merge correctly drops +the graph's old copy of that edge (the changed file's result replaced it), +and nothing regenerates it. Passing the unchanged corpus in via +`resolution_context_nodes` (the same mechanism #2406 added for direct/ +indirect call resolution) must widen these two passes as well. +""" + +from __future__ import annotations + +from pathlib import Path + +from graphify.extract import extract + + +def _write_corpus(root: Path) -> list[Path]: + pkg = root / "pkg" + pkg.mkdir() + (pkg / "__init__.py").write_text("") + (pkg / "exc.py").write_text("class BadData(Exception):\n pass\n") + (pkg / "a.py").write_text( + "from .exc import BadData\n\n\ndef load():\n raise BadData()\n" + ) + return [pkg / "__init__.py", pkg / "exc.py", pkg / "a.py"] + + +def _edge_set(graph: dict) -> set[tuple[str, str, str]]: + return {(e["source"], e["target"], e["relation"]) for e in graph["edges"]} + + +def test_incremental_reextract_regenerates_cross_file_edges(tmp_path): + root = tmp_path + paths = _write_corpus(root) + + full = extract(paths, root=root, cache_root=root, parallel=False) + full_edges = _edge_set(full) + + # Every edge the full scan sourced from a.py must reappear when a.py alone + # is re-extracted incrementally, with exc.py supplied only as read-only + # resolution context (an unchanged neighbor, never re-parsed). + full_a_edges = {e for e in full_edges if e[0].startswith("pkg_a")} + assert full_a_edges, "sanity: full scan should produce edges sourced by a.py" + assert any(e[2] == "uses" for e in full_a_edges), ( + "sanity: full scan should produce a cross-file INFERRED 'uses' edge" + ) + + context_nodes = [ + n for n in full["nodes"] + if str(n.get("source_file", "")).endswith(("exc.py", "__init__.py")) + ] + + incremental = extract( + [root / "pkg" / "a.py"], + root=root, + cache_root=root, + parallel=False, + resolution_context_nodes=context_nodes, + ) + incremental_a_edges = { + e for e in _edge_set(incremental) if e[0].startswith("pkg_a") + } + + assert incremental_a_edges == full_a_edges, ( + f"incremental re-extraction lost cross-file edges: " + f"{full_a_edges - incremental_a_edges}" + )