mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-28 17:26:48 +00:00
fix(path): prefer full-token label matches when resolving path endpoints (#1785)
`graphify path` committed each endpoint to _score_nodes()[0]. The full-query
bonus tier only fires when the query equals/prefixes a label, so a query that is
a token subset of the intended label ("Reject-everything judge" vs "Degenerate
Reject-Everything Judge") got no bonus and a node prefix-matching one rare token
("Rejection Summary") could out-score it on IDF alone — anchoring the path on an
unrelated, often disconnected node and returning a false "No path found".
_pick_scored_endpoint() scans the score-ordered list and takes the first
candidate whose label contains EVERY query token, falling back to scored[0] when
none does — so when the head already full-matches (the common case) resolution
is unchanged. Wired into both the `path` CLI and the MCP _tool_shortest_path.
The close-runner-up ambiguity warning now fires only when the picked endpoint is
the raw score head (a full-token override was chosen on coverage, not score, so
the head's margin is irrelevant).
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
15a86536c4
commit
b688a26f71
@@ -4,6 +4,8 @@ Full release notes with details on each version: [GitHub Releases](https://githu
|
||||
|
||||
## 0.9.13 (unreleased)
|
||||
|
||||
- Fix: `graphify path` resolves each endpoint to the first candidate whose label contains every query token, instead of blindly taking the top-scored node (#1785, thanks @CJNA). `_score_nodes`' full-query bonus only fires when the query equals/prefixes a label, so a query that is a token *subset* of the intended label (`"Reject-everything judge"` vs `"Degenerate Reject-Everything Judge"`) got no bonus and a node prefix-matching one rare token could outscore it — anchoring the path on an unrelated, often disconnected node and yielding a false "No path found". When the top candidate already full-matches (the common case) the pick is unchanged. Applied to both the `path` CLI and the MCP shortest-path tool; the close-runner-up ambiguity warning now fires only when the score head is what was actually picked.
|
||||
|
||||
- Fix: the report's "Suggested Questions" weakly-connected-node count now matches its "Knowledge Gaps" count (#1768, thanks @balloon72). `suggest_questions()` omitted the `file_type != "rationale"` filter that `report.py`'s Knowledge Gaps section applies, so the same `GRAPH_REPORT.md` showed two different numbers for the same concept (e.g. 757 vs 245), making a healthy graph look like it had a major documentation gap. Both computations now use the same filter.
|
||||
|
||||
- Fix: Bash scripts that run each other by execution now get a cross-file edge (#1756, thanks @balloon72). `extract_bash` only linked `source x.sh` / `. x.sh`; the two most common forms — `bash x.sh` and `./x.sh` — produced no edge, so execution topology was missing. They now emit a `calls` edge (context `script_invocation`) to the invoked script's entry node when the target resolves to a real file on disk (script runners `bash`/`sh`/`zsh`/`ksh`/`dash` and bare `./x.sh`), skipping missing or shadowed targets.
|
||||
|
||||
+11
-4
@@ -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(
|
||||
|
||||
+33
-3
@@ -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 _build_server(graph_path: 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 _build_server(graph_path: 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(
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user