mirror of
https://github.com/safishamsi/graphify.git
synced 2026-09-21 04:56:18 +00:00
fix: resolve explain/affected when a source-file path matches multiple nodes (#1503)
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) <noreply@anthropic.com>
This commit is contained in:
committed by
safishamsi
co-authored by
Claude Opus 4.8
parent
11dc819c3f
commit
1b994965bb
@@ -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#.
|
||||
|
||||
@@ -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)
|
||||
|
||||
+26
-5
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
+27
-2
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user