diff --git a/graphify/cli.py b/graphify/cli.py index 380bee50a..25bf5499b 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -602,7 +602,7 @@ def dispatch_command(cmd: str) -> None: file=sys.stderr, ) sys.exit(1) - from graphify.serve import _score_nodes + from graphify.serve import _pick_scored_endpoint, _score_nodes from networkx.readwrite import json_graph import networkx as _nx @@ -635,7 +635,8 @@ def dispatch_command(cmd: str) -> None: if not tgt_scored: print(f"No node matching '{target_label}' found.", file=sys.stderr) sys.exit(1) - src_nid, tgt_nid = src_scored[0][1], tgt_scored[0][1] + src_nid = _pick_scored_endpoint(G, src_scored, source_label) + tgt_nid = _pick_scored_endpoint(G, tgt_scored, target_label) # Ambiguity guard: when both queries resolve to the same node, the # shortest path is trivially zero hops, which is almost never what the # caller wanted (see bug #828). @@ -646,8 +647,14 @@ def dispatch_command(cmd: str) -> None: file=sys.stderr, ) sys.exit(1) - for _name, _scored in (("source", src_scored), ("target", tgt_scored)): - if len(_scored) >= 2: + for _name, _scored, _nid in ( + ("source", src_scored, src_nid), + ("target", tgt_scored, tgt_nid), + ): + # A close runner-up only made the resolution ambiguous when the raw + # score head is what got picked; a full-token override was chosen on + # token coverage, not score, so the head's margin is irrelevant. + if len(_scored) >= 2 and _nid == _scored[0][1]: _top, _runner = _scored[0][0], _scored[1][0] if _top > 0 and (_top - _runner) / _top < 0.10: print( diff --git a/graphify/serve.py b/graphify/serve.py index 1c9b979b6..03a92c327 100644 --- a/graphify/serve.py +++ b/graphify/serve.py @@ -372,6 +372,30 @@ def _score_nodes(G: nx.Graph, terms: list[str]) -> list[tuple[float, str]]: return scored +def _pick_scored_endpoint(G: nx.Graph, scored: list[tuple[float, str]], query: str) -> str: + """Pick a path endpoint from a _score_nodes result, preferring full-token matches. + + The full-query tier in _score_nodes only fires when the query equals or + prefixes a label, so a query that is a token *subset* of the intended label + (query "Reject-everything judge" vs. label "Degenerate Reject-Everything + Judge") gets no bonus, and a node prefix-matching one rare token (label + "Rejection Summary") can out-score it on IDF alone. Committing to scored[0] + then anchors the path on an unrelated — often disconnected — node and yields + a false "No path found". Scan the score-ordered list and take the first + candidate whose label contains EVERY query token; when the top candidate + already full-matches, or no candidate does, this is exactly scored[0]. + + `scored` must be non-empty (both callers return early on no match). + """ + qtokens = set(_search_tokens(query)) + if not qtokens: + return scored[0][1] + for _score, nid in scored: + if qtokens <= set(_search_tokens(G.nodes[nid].get("label") or nid)): + return nid + return scored[0][1] + + def _pick_seeds( scored: list[tuple[float, str]], max_k: int = 3, @@ -1140,7 +1164,8 @@ def _tool_shortest_path(arguments: dict) -> str: return f"No node matching source '{arguments['source']}' found." if not tgt_scored: return f"No node matching target '{arguments['target']}' found." - src_nid, tgt_nid = src_scored[0][1], tgt_scored[0][1] + src_nid = _pick_scored_endpoint(G, src_scored, arguments["source"]) + tgt_nid = _pick_scored_endpoint(G, tgt_scored, arguments["target"]) # Ambiguity guard: when both queries resolve to the same node, the # shortest path is trivially zero hops, which is almost never what the # caller wanted (see bug #828). @@ -1150,8 +1175,13 @@ def _tool_shortest_path(arguments: dict) -> str: f"the same node '{src_nid}'. Use a more specific label or the exact node ID." ) warnings: list[str] = [] - for name, scored in (("source", src_scored), ("target", tgt_scored)): - if len(scored) >= 2: + for name, scored, nid in ( + ("source", src_scored, src_nid), + ("target", tgt_scored, tgt_nid), + ): + # Only meaningful when the raw score head is what got picked — a + # full-token override was chosen on token coverage, not score. + if len(scored) >= 2 and nid == scored[0][1]: top, runner = scored[0][0], scored[1][0] if top > 0 and (top - runner) / top < 0.10: warnings.append( diff --git a/tests/test_path_cli.py b/tests/test_path_cli.py index de7e8837f..8b979e3a6 100644 --- a/tests/test_path_cli.py +++ b/tests/test_path_cli.py @@ -2,6 +2,7 @@ from __future__ import annotations import json import networkx as nx +import pytest from networkx.readwrite import json_graph import graphify.__main__ as mainmod @@ -46,3 +47,59 @@ def test_reverse_arrow(monkeypatch, tmp_path, capsys): assert "Shortest path (1 hops):" in out assert "validateSanitySession() <--calls [EXTRACTED]-- createPatchHandler()" in out assert "validateSanitySession() --calls [EXTRACTED]--> createPatchHandler()" not in out + + +def _write_misranking_graph(tmp_path): + """Graph where IDF scoring ranks a partial-token decoy above the full match. + + Query "Reject-everything judge": the decoy "Rejection Summary" prefix-matches + the rare token "reject" and out-scores "Degenerate Reject-Everything Judge" + (whose full-query tier never fires — the query is a token subset of the + label, not a prefix). The filler nodes make "judge"/"everything" common so + their IDF stays low. Decoy and target live in different components: resolving + the source to the decoy yields a false "No path found". + """ + nodes = [ + {"id": "target", "label": "Degenerate Reject-Everything Judge", "community": 0}, + {"id": "decoy", "label": "Rejection Summary", "community": 0}, + ] + for i in range(30): + nodes.append({"id": f"j{i}", "label": f"Judge Helper {i}", "community": 0}) + nodes.append({"id": f"e{i}", "label": f"Everything Widget {i}", "community": 0}) + graph_data = { + "directed": False, "multigraph": False, "graph": {}, + "nodes": nodes, + "links": [ + {"source": "target", "target": "j0", + "relation": "verified_by", "confidence": "EXTRACTED"}, + {"source": "decoy", "target": "e0", + "relation": "mentions", "confidence": "EXTRACTED"}, + ], + } + p = tmp_path / "graph.json" + p.write_text(json.dumps(graph_data)) + return p + + +def test_endpoint_prefers_full_token_match(monkeypatch, tmp_path, capsys): + """A token-subset query resolves to the full-match node, not the IDF head.""" + p = _write_misranking_graph(tmp_path) + out = _run(monkeypatch, p, "Reject-everything judge", "Judge Helper 0", capsys) + assert "Shortest path (1 hops):" in out + assert "Degenerate Reject-Everything Judge" in out + assert "No path found" not in out + + +def test_endpoint_falls_back_to_score_head(monkeypatch, tmp_path, capsys): + """No full-token candidate -> behavior identical to the old scored[0] pick.""" + p = _write_misranking_graph(tmp_path) + # "Rejection judge" full-matches nothing ("rejection" only appears in the + # decoy, "judge" never joins it), so the IDF head (the decoy) still wins, + # and the disconnected components make that a "No path found" exit(0). + monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None) + monkeypatch.setattr(mainmod.sys, "argv", + ["graphify", "path", "Rejection judge", "Judge Helper 0", "--graph", str(p)]) + with pytest.raises(SystemExit) as exc_info: + mainmod.main() + assert exc_info.value.code == 0 + assert "No path found" in capsys.readouterr().out