From 1b994965bb82c79cbdd4f16d121b3c4fd8479db2 Mon Sep 17 00:00:00 2001 From: behavio1 Date: Sat, 27 Jun 2026 23:57:39 +0100 Subject: [PATCH] fix: resolve explain/affected when a source-file path matches multiple nodes (#1503) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A path query like `explain "app/api/route.ts"` tokenized to terms that matched no node, so explain/affected returned "No node matching". Source-file paths are now part of the search index and matched exactly (serve._find_node gains a leading source-exact tier; affected.resolve_seed gains a source-file match). When several nodes share a source_file (e.g. a file-level node plus a function node), the lookup prefers the file-level node — the L1 node whose label basename matches the queried filename, falling back to the unique L1 or unique basename match, else None. Ported from PR #1503 by @behavio1. Maintainer fixes on top: aligned trailing- separator handling between resolve_seed and _find_node (affected previously returned None for a trailing-slash path that explain resolved), corrected the stale "three-tier" _find_node docstring, and added regression tests for the trailing-slash parity and the ambiguous-no-file-node -> None case. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 1 + graphify/affected.py | 43 +++++++++++++++++ graphify/serve.py | 31 +++++++++++-- tests/test_affected_cli.py | 94 ++++++++++++++++++++++++++++++++++++++ tests/test_explain_cli.py | 26 +++++++++++ tests/test_serve.py | 29 +++++++++++- 6 files changed, 217 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7040de8a..ba85e0b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ Full release notes with details on each version: [GitHub Releases](https://githu ## Unreleased +- Fix: `graphify explain` and `graphify affected` now resolve a query given as a source-file path even when the graph has multiple nodes from that file (#1503, thanks @behavio1). A path like `app/api/route.ts` tokenized to terms that matched no node, so explain returned "No node matching"; source-file paths are now indexed and matched exactly, and when several nodes share the file the lookup prefers the file-level node (the `L1` node whose name matches the file). Trailing-separator handling is aligned between the two commands. - Docs: clearer install/PATH guidance for `uv tool install graphifyy` on macOS (#1471, thanks @Patsch36). Two expected uv behaviors read as bugs: (1) after `uv tool install`, the `graphify` command lands in uv's tool bin dir (`~/.local/bin`), which a fresh macOS/zsh shell often doesn't have on `PATH` — the README now points to `uv tool update-shell` instead of implying uv always wires `PATH`; (2) `uvx graphify …` / `uv tool run graphify …` resolve the first word as a *package* and fail, because the package is `graphifyy` and `graphify` is only its console script — the docs now show `uvx --from graphifyy graphify install`. README install note + Troubleshooting only; no code change. - Fix: imported type stubs with the same label no longer falsely merge across files when there is no project definition to rewire onto (#1462, thanks @jiangyq9). Two files that both `from pathlib import Path` and use `Path` as a type previously collapsed into one node; the referencing file is now kept as an internal disambiguator (`origin_file`) used only when splitting colliding ids, while `source_file` stays empty so a real project definition can still be rewired onto (the #1402 path is unaffected). - Feat: resolve C# cross-file type references and extract `enum`/`struct`/`record` declarations (#1466, thanks @TheFedaikin). A new `_resolve_csharp_type_references` (the C# counterpart to the Java resolver) re-points dangling `inherits`/`implements`/`references` edges from no-source "shadow" stubs to their real definitions, disambiguating same-named types in different namespaces via the referencing file's `using` directives and enclosing namespace; ambiguous matches are refused rather than guessed. `enum`/`struct`/`record` types are now extracted as definitions so those references resolve too. Advances #1318 for C#. diff --git a/graphify/affected.py b/graphify/affected.py index 8bddce65..69b679f9 100644 --- a/graphify/affected.py +++ b/graphify/affected.py @@ -54,7 +54,46 @@ def _normalize_label(label: str) -> str: return unicodedata.normalize("NFC", label).casefold() +def _prefer_file_node( + graph: nx.Graph, + node_ids: list[str], + query: str, +) -> str | None: + """Return the file-level node when a source_file query matches many nodes.""" + query_basename = _normalize_label(Path(query).name) + exact_file_nodes = [ + node_id + for node_id in node_ids + if str(graph.nodes[node_id].get("source_location", "")) == "L1" + and _normalize_label(str(graph.nodes[node_id].get("label", ""))) == query_basename + ] + if len(exact_file_nodes) == 1: + return exact_file_nodes[0] + + l1_nodes = [ + node_id + for node_id in node_ids + if str(graph.nodes[node_id].get("source_location", "")) == "L1" + ] + if len(l1_nodes) == 1: + return l1_nodes[0] + + basename_nodes = [ + node_id + for node_id in node_ids + if _normalize_label(str(graph.nodes[node_id].get("label", ""))) == query_basename + ] + if len(basename_nodes) == 1: + return basename_nodes[0] + + return None + + def resolve_seed(graph: nx.Graph, query: str) -> str | None: + # A trailing path separator must not change a source-file match — serve's + # _find_node tokenizes the path (which drops it), so strip it here for parity + # (otherwise `affected "src/x.ts/"` returned None while `explain` resolved it). + query = query.rstrip("/\\") or query if query in graph: return query query_lower = _normalize_label(query) @@ -83,6 +122,10 @@ def resolve_seed(graph: nx.Graph, query: str) -> str | None: ] if len(exact_source_matches) == 1: return exact_source_matches[0] + if exact_source_matches: + preferred_file_node = _prefer_file_node(graph, exact_source_matches, query) + if preferred_file_node is not None: + return preferred_file_node contains_matches = [ str(node_id) for node_id, data in graph.nodes(data=True) diff --git a/graphify/serve.py b/graphify/serve.py index 117e70af..53daf0c8 100644 --- a/graphify/serve.py +++ b/graphify/serve.py @@ -152,6 +152,8 @@ def _node_search_text(data: dict, nid: str) -> str: `term in label_tokens` branch, where a multi-word `term` can span a token boundary that punctuation hides in `norm_label` (e.g. query "foo bar" matches label "foo.bar" only via its tokenized form). + - `source_tokens` feeds _find_node's exact source-file path lookup, where a + query like "app/api/example/route.ts" tokenizes to "app api example route ts". - `nid` feeds the whole-query `joined == nid_lower` tier. NUL separators stop a trigram from spanning two fields (a query never contains @@ -160,7 +162,8 @@ def _node_search_text(data: dict, nid: str) -> str: norm_label = data.get("norm_label") or _strip_diacritics(data.get("label") or "").lower() label_tokens = " ".join(_search_tokens(data.get("label") or "")) source = (data.get("source_file") or "").lower() - return "\x00".join((norm_label, label_tokens, str(nid).lower(), source)) + source_tokens = " ".join(_search_tokens(data.get("source_file") or "")) + return "\x00".join((norm_label, label_tokens, str(nid).lower(), source, source_tokens)) def _get_trigram_index(G: nx.Graph) -> dict: @@ -566,12 +569,14 @@ def _query_graph_text( def _find_node(G: nx.Graph, label: str) -> list[str]: """Return node IDs whose label or ID matches the search term (diacritic-insensitive). - Results are ordered by three-tier precedence: exact match, then prefix match, - then substring match. Node-ID exact matches are grouped with label exact matches. + Results are ordered by precedence: exact source-file path match first, then + exact (label/ID) match, then prefix match, then substring match. Node-ID exact + matches are grouped with label exact matches. """ term = " ".join(_search_tokens(label)) if not term: return [] + source_exact: list[str] = [] exact: list[str] = [] prefix: list[str] = [] substring: list[str] = [] @@ -586,8 +591,11 @@ def _find_node(G: nx.Graph, label: str) -> list[str]: norm_label = d.get("norm_label") or _strip_diacritics(d.get("label") or "").lower() bare_label = norm_label.rstrip("()") label_tokens = " ".join(_search_tokens(d.get("label") or "")) + source_tokens = " ".join(_search_tokens(d.get("source_file") or "")) nid_lower = nid.lower() - if term == norm_label or term == bare_label or term == label_tokens or term == nid_lower: + if term == source_tokens: + source_exact.append(nid) + elif term == norm_label or term == bare_label or term == label_tokens or term == nid_lower: exact.append(nid) elif ( norm_label.startswith(term) @@ -598,7 +606,20 @@ def _find_node(G: nx.Graph, label: str) -> list[str]: prefix.append(nid) elif term in norm_label or term in label_tokens: substring.append(nid) - return exact + prefix + substring + + if source_exact: + query_basename = _strip_diacritics(Path(label).name).lower() + preferred = [ + nid + for nid in source_exact + if str(G.nodes[nid].get("source_location", "")) == "L1" + and _strip_diacritics(str(G.nodes[nid].get("label") or "")).lower() + == query_basename + ] + if len(preferred) == 1: + source_exact = preferred + [nid for nid in source_exact if nid != preferred[0]] + + return source_exact + exact + prefix + substring def _filter_blank_stdin() -> None: diff --git a/tests/test_affected_cli.py b/tests/test_affected_cli.py index c7835b86..a50e4bc2 100644 --- a/tests/test_affected_cli.py +++ b/tests/test_affected_cli.py @@ -174,3 +174,97 @@ def test_resolve_seed_bare_name_tie_still_returns_none(): graph.add_node("b", label="dup()", source_file="pkg/two.py") assert resolve_seed(graph, "dup") is None + + +def test_resolve_seed_source_file_path_prefers_file_level_node(): + from graphify.affected import resolve_seed + + graph = nx.DiGraph() + source_file = "app/api/example/route.ts" + graph.add_node( + "example_route_get", + label="GET()", + source_file=source_file, + source_location="L42", + ) + graph.add_node( + "example_route", + label="route.ts", + source_file=source_file, + source_location="L1", + ) + + assert resolve_seed(graph, source_file) == "example_route" + + +def test_resolve_seed_source_file_trailing_slash_parity(): + """A trailing path separator must not change the match (parity with explain's + _find_node, which tokenizes the path and drops the slash).""" + from graphify.affected import resolve_seed + + graph = nx.DiGraph() + source_file = "app/api/example/route.ts" + graph.add_node("get", label="GET()", source_file=source_file, source_location="L42") + graph.add_node("file", label="route.ts", source_file=source_file, source_location="L1") + + assert resolve_seed(graph, source_file + "/") == "file" + + +def test_resolve_seed_source_file_ambiguous_no_file_node_returns_none(): + """Several nodes share a source_file but none is the L1 file node and none's + basename matches the path — must not guess; return None.""" + from graphify.affected import resolve_seed + + graph = nx.DiGraph() + source_file = "pkg/handlers.py" + graph.add_node("a", label="handle_a()", source_file=source_file, source_location="L10") + graph.add_node("b", label="handle_b()", source_file=source_file, source_location="L20") + + assert resolve_seed(graph, source_file) is None + + +def test_affected_cli_source_file_path_uses_file_level_node(monkeypatch, tmp_path, capsys): + graph = nx.DiGraph() + source_file = "app/api/example/route.ts" + graph.add_node( + "example_route_get", + label="GET()", + source_file=source_file, + source_location="L42", + ) + graph.add_node( + "example_route", + label="route.ts", + source_file=source_file, + source_location="L1", + ) + graph.add_node( + "consumer", + label="consumer.ts", + source_file="app/consumer.ts", + source_location="L1", + ) + graph.add_edge( + "consumer", + "example_route", + relation="imports_from", + context="import", + confidence="EXTRACTED", + ) + graph_path = tmp_path / "graph.json" + graph_path.write_text(json.dumps(json_graph.node_link_data(graph, edges="links")), encoding="utf-8") + + monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None) + monkeypatch.setattr( + mainmod.sys, + "argv", + ["graphify", "affected", source_file, "--graph", str(graph_path)], + ) + + mainmod.main() + + out = capsys.readouterr().out + assert "Affected nodes for route.ts" in out + assert "consumer.ts" in out + assert "imports_from" in out + assert "No unique node matched" not in out diff --git a/tests/test_explain_cli.py b/tests/test_explain_cli.py index 1d00955f..a77ac3ec 100644 --- a/tests/test_explain_cli.py +++ b/tests/test_explain_cli.py @@ -54,3 +54,29 @@ def test_caller_shows_callee_as_outbound(monkeypatch, tmp_path, capsys): out = _run(monkeypatch, p, "createPatchHandler", capsys) assert "--> validateSanitySession() [calls]" in out assert "<-- " not in out + + +def test_explain_source_file_path_prefers_file_level_node(monkeypatch, tmp_path, capsys): + source_file = "app/api/example/route.ts" + graph_data = { + "directed": False, "multigraph": False, "graph": {}, + "nodes": [ + {"id": "example_route_get", "label": "GET()", + "source_file": source_file, "source_location": "L42", "community": 0}, + {"id": "example_route", "label": "route.ts", + "source_file": source_file, "source_location": "L1", "community": 0}, + ], + "links": [ + {"source": "example_route", "target": "example_route_get", + "relation": "contains", "confidence": "EXTRACTED"}, + ], + } + p = tmp_path / "graph.json" + p.write_text(json.dumps(graph_data)) + + out = _run(monkeypatch, p, source_file, capsys) + + assert "Node: route.ts" in out + assert "ID: example_route" in out + assert f"Source: {source_file} L1" in out + assert "Node: GET()" not in out diff --git a/tests/test_serve.py b/tests/test_serve.py index 4ffa9aed..4cd477d6 100644 --- a/tests/test_serve.py +++ b/tests/test_serve.py @@ -165,13 +165,14 @@ def test_trigrams_basic(): def test_node_search_text_includes_all_matched_fields(): G = _make_big_graph() text = _node_search_text(G.nodes["punct"], "punct") - # norm_label, the tokenized label (label_tokens), nid, and source are all present, - # NUL-separated so trigrams can't span fields. + # norm_label, tokenized label, nid, raw source, and tokenized source are all + # present, NUL-separated so trigrams can't span fields. parts = text.split("\x00") assert parts[0] == "foo.bar:baz" # norm_label (punctuation kept) assert parts[1] == "foo bar baz" # label_tokens (tokenized) assert parts[2] == "punct" # nid assert parts[3] == "pkg/foobar.py" # source_file + assert parts[4] == "pkg foobar py" # source_file tokens def test_trigram_candidates_fast_path_fires_for_rare_term(): @@ -227,6 +228,30 @@ def test_find_node_label_tokens_branch_covered_by_index(): assert _find_node(G, "Foo Bar Baz") == ["punct"] +def test_find_node_source_file_path_prefers_file_level_node(): + G = _make_big_graph() + source_file = "app/api/example/route.ts" + # Insert the function node first to prove source-file lookup reorders the + # file-level node ahead of other nodes from the same file. + G.add_node( + "example_route_get", + label="GET()", + source_file=source_file, + source_location="L42", + ) + G.add_node( + "example_route", + label="route.ts", + source_file=source_file, + source_location="L1", + ) + + matches = _find_node(G, source_file) + + assert matches[0] == "example_route" + assert "example_route_get" in matches + + def test_trigram_index_cached_and_rebuilt_per_graph(): G = _make_big_graph() idx1 = _get_trigram_index(G)