fix(query): report real call-site lines + stop silent query truncation (benchmark)
Two correctness defects found in a head-to-head benchmark of 0.9.22. Caller line numbers: explain/affected/get_neighbors/query printed the caller node's def line for an incoming call, presented as a precise citation, so click-through landed in the wrong place. The `calls` edge already carries the true call-site line (engine.py sets it); every caller/relation listing now reads the traversed edge's source_file:source_location, falling back to the node's own line only when the edge lacks one. Silent query truncation: rendered nodes were degree-ordered (a low-degree definition node ranked last, cut first), the queried symbol wasn't guaranteed to appear, and the truncation marker sat only at the end so silence read as absence. Nodes are now ranked by hop distance from the seeds (deterministic), the seed the question named is rendered first and never truncated, and a prominent TRUNCATED notice at the top states shown/total counts and how to widen the budget. Also rewires the seed-first ordering the renderer already supported — a branch merge had silently dropped the `seeds=` argument, leaving it dead code.
This commit is contained in:
@@ -4,6 +4,8 @@ Full release notes with details on each version: [GitHub Releases](https://githu
|
||||
|
||||
## 0.9.23 (unreleased)
|
||||
|
||||
- Fix: caller / "call sites" listings now report the actual call-site line, not the caller function's definition line. `explain`, `affected`, and the MCP `get_neighbors`/`query` tools printed the caller node's `source_location` (its `def` line) for an incoming call, so a precise-looking citation sent users to the wrong line. The `calls` edge already carries the true call-site line; every caller/relation listing now reads the traversed edge's `source_file`:`source_location`, falling back to the node's own line only when the edge has none.
|
||||
- Fix: `query` no longer silently drops the answer past its output budget. Rendered nodes were ordered by degree (so a low-degree definition node ranked last and was cut first), the queried symbol was not guaranteed to appear, and the truncation marker sat only at the end so silence read as absence. Nodes are now ranked by hop distance from the query seeds (deterministically), the seed the question named is always rendered first and never truncated, and a prominent notice at the TOP states how many of how many nodes were shown and how to widen the budget. (A branch merge had also silently dropped the seed-first ordering the renderer already supported; it is rewired.)
|
||||
- Fix: `graphify uninstall` no longer deletes a user-authored `### graphify` section (#2062). The uninstall strip used an unanchored `## graphify` pattern that matched inside a user's H3 heading (and the "already installed" guard was a substring test), so hand-written content was destroyed. The heading is now matched only when a line is exactly the marker (mirroring the install-side #1688 hardening), across all six strip sites (CLAUDE.md, AGENTS.md, GEMINI.md, copilot-instructions.md, CODEBUDDY.md, and the H1 skill registration).
|
||||
- Fix: `graphify path` (and the MCP `shortest_path` tool) now return a deterministic route and label each hop with the edge's actual stored relation (#2074). The route was computed over a hash-seeded undirected view, so it varied run-to-run among equal-length paths; and the printed relation was read from an arbitrarily-collapsed parallel edge, so it could show `calls` on a pair that only carries `references`. The traversal is now over a sorted graph, and each hop shows the real relation(s), falling back to an honest `related` when none is stored.
|
||||
- Fix: `cluster-only --no-label` no longer permanently suppresses real community labels (#2073). It wrote `Community N` placeholders (plus a matching signature) into `.graphify_labels.json`, which the reuse path then treated as fresh forever. Placeholder-only runs no longer persist the sidecar, a stored placeholder is treated as absent so already-polluted graphs self-heal, and the watch/update rebuild got the same treatment.
|
||||
|
||||
+21
-2
@@ -30,6 +30,11 @@ class AffectedHit:
|
||||
node_id: str
|
||||
depth: int
|
||||
via_relation: str
|
||||
# The traversed edge's location — the actual call/import/reference SITE in
|
||||
# this node's file, not the node's own definition line (#BUG1). Defaults keep
|
||||
# existing constructors/tests working; None falls back to the node's def line.
|
||||
via_file: "str | None" = None
|
||||
via_location: "str | None" = None
|
||||
|
||||
|
||||
def _node_label(graph: nx.Graph, node_id: str) -> str:
|
||||
@@ -190,7 +195,15 @@ def affected_nodes(
|
||||
if source in seen:
|
||||
continue
|
||||
seen.add(source)
|
||||
hit = AffectedHit(source, current_depth + 1, relation)
|
||||
# Carry the matched edge's location (taken from the SAME edge dict
|
||||
# whose relation passed the filter, so relation and location stay
|
||||
# consistent) — that is the call/import/reference site in `source`'s
|
||||
# own file, which is where the user should click (#BUG1).
|
||||
hit = AffectedHit(
|
||||
source, current_depth + 1, relation,
|
||||
via_file=str(data.get("source_file") or "") or None,
|
||||
via_location=str(data.get("source_location") or "") or None,
|
||||
)
|
||||
hits.append(hit)
|
||||
queue.append((source, current_depth + 1))
|
||||
|
||||
@@ -221,8 +234,14 @@ def format_affected(
|
||||
|
||||
for hit in hits:
|
||||
data = graph.nodes[hit.node_id]
|
||||
if hit.via_location:
|
||||
# The relation SITE in this node's file (call/import/reference line),
|
||||
# labeled by [via_relation] so it's never mistaken for a def line.
|
||||
location = f"{hit.via_file or data.get('source_file') or '-'}:{hit.via_location}"
|
||||
else:
|
||||
location = _format_location(data) # honest fallback: the node's own def line
|
||||
lines.append(
|
||||
f"- {_node_label(graph, hit.node_id)} [{hit.via_relation}] {_format_location(data)}"
|
||||
f"- {_node_label(graph, hit.node_id)} [{hit.via_relation}] {location}"
|
||||
)
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
+7
-1
@@ -1260,7 +1260,13 @@ def dispatch_command(cmd: str) -> None:
|
||||
rel = edata.get("relation", "")
|
||||
conf = edata.get("confidence", "")
|
||||
arrow = "-->" if direction == "out" else "<--"
|
||||
print(f" {arrow} {G.nodes[nb].get('label', nb)} [{rel}] [{conf}]")
|
||||
# Append the edge's location — the actual call/import/reference
|
||||
# SITE (in the caller's file for an incoming call), not a def
|
||||
# line (#BUG1). Labeled by [rel] so the meaning is unambiguous.
|
||||
loc = edata.get("source_location") or ""
|
||||
sfile = edata.get("source_file") or ""
|
||||
at = f" {sfile}:{loc}" if loc else ""
|
||||
print(f" {arrow} {G.nodes[nb].get('label', nb)} [{rel}] [{conf}]{at}")
|
||||
if len(connections) > 20:
|
||||
print(f" ... and {len(connections) - 20} more")
|
||||
from graphify import querylog
|
||||
|
||||
+68
-7
@@ -805,8 +805,34 @@ def _subgraph_to_text(G: nx.Graph, nodes: set[str], edges: list[tuple], token_bu
|
||||
# Empty when no sidecar exists, so un-annotated output stays byte-identical.
|
||||
overlay = getattr(G, "graph", {}).get("_learning_overlay", {}) or {}
|
||||
seed_set = set(seeds or [])
|
||||
ordered = [n for n in (seeds or []) if n in nodes] + \
|
||||
sorted(nodes - seed_set, key=lambda n: G.degree(n), reverse=True)
|
||||
seed_hits = [n for n in (seeds or []) if n in nodes]
|
||||
# Rank non-seed nodes by hop distance from the seeds so the node that answers
|
||||
# the query (a direct hit or its close neighbors) survives the budget cut
|
||||
# instead of being pushed past it by incidental high-degree hubs (#BUG2). BFS
|
||||
# discovery order was discarded upstream (_bfs returns a set), so recompute
|
||||
# layers here over BOTH edge directions. Deterministic: neighbor iteration is
|
||||
# insertion-ordered and the sort key ends in str(n) (no hash-order).
|
||||
def _adj(n):
|
||||
if G.is_directed():
|
||||
yield from G.successors(n)
|
||||
yield from G.predecessors(n)
|
||||
else:
|
||||
yield from G.neighbors(n)
|
||||
dist: dict[str, int] = {n: 0 for n in seed_hits}
|
||||
frontier, hop = seed_hits, 0
|
||||
while frontier:
|
||||
hop += 1
|
||||
nxt = []
|
||||
for n in frontier:
|
||||
for nb in _adj(n):
|
||||
if nb in nodes and nb not in dist:
|
||||
dist[nb] = hop
|
||||
nxt.append(nb)
|
||||
frontier = nxt
|
||||
ordered = seed_hits + sorted(
|
||||
nodes - seed_set,
|
||||
key=lambda n: (dist.get(n, 1 << 30), -G.degree(n), str(n)),
|
||||
)
|
||||
for nid in ordered:
|
||||
d = G.nodes[nid]
|
||||
# Every LLM-derived field passes through sanitize_label before being
|
||||
@@ -836,22 +862,46 @@ def _subgraph_to_text(G: nx.Graph, nodes: set[str], edges: list[tuple], token_bu
|
||||
d = next(iter(raw.values()), {}) if isinstance(G, (nx.MultiGraph, nx.MultiDiGraph)) else raw
|
||||
context = d.get("context")
|
||||
context_suffix = f" context={sanitize_label(str(context))}" if context else ""
|
||||
# The relation SITE (call/import/reference line in the source's
|
||||
# file), not a def line — so "who calls X" cites a clickable call
|
||||
# location, not the caller's def (#BUG1).
|
||||
_loc = str(d.get("source_location") or "")
|
||||
at_suffix = (
|
||||
f" at={sanitize_label(str(d.get('source_file') or ''))}:{sanitize_label(_loc)}"
|
||||
if _loc else ""
|
||||
)
|
||||
line = (
|
||||
f"EDGE {sanitize_label(G.nodes[u].get('label', u))} "
|
||||
f"--{sanitize_label(str(d.get('relation', '')))} "
|
||||
f"[{sanitize_label(str(d.get('confidence', '')))}{context_suffix}]--> "
|
||||
f"{sanitize_label(G.nodes[v].get('label', v))}"
|
||||
f"{sanitize_label(G.nodes[v].get('label', v))}{at_suffix}"
|
||||
)
|
||||
lines.append(line)
|
||||
output = "\n".join(lines)
|
||||
if len(output) > char_budget:
|
||||
cut_at = output[:char_budget].rfind("\n")
|
||||
cut_at = cut_at if cut_at > 0 else char_budget
|
||||
# Never cut the seed nodes: they render first, so if the budget lands
|
||||
# inside the seed block, extend the cut to cover it. The symbol the
|
||||
# question named must always be in the answer (#BUG2). Seeds are bounded
|
||||
# (_pick_seeds max_k + one per term), so the overshoot is a few lines.
|
||||
if seed_hits:
|
||||
seed_block_end = sum(len(lines[i]) + 1 for i in range(len(seed_hits))) - 1
|
||||
cut_at = max(cut_at, min(seed_block_end, len(output)))
|
||||
total_nodes = sum(1 for l in lines if l.startswith("NODE "))
|
||||
shown_nodes = output[:cut_at].count("\nNODE ") + (1 if output.startswith("NODE ") else 0)
|
||||
cut_count = total_nodes - shown_nodes
|
||||
# Prominent notice at the TOP so a truncated answer can never be mistaken
|
||||
# for a complete one — silence used to read as absence (#BUG2). The
|
||||
# notice + end marker sit OUTSIDE char_budget by design (two bounded
|
||||
# wrapper lines, like the existing end marker).
|
||||
output = (
|
||||
output[:cut_at]
|
||||
f"[!] TRUNCATED: showing {shown_nodes} of {total_nodes} nodes "
|
||||
f"(~{token_budget}-token budget). The answer may be among the "
|
||||
f"{cut_count} cut nodes — raise the token budget (CLI: --budget) or "
|
||||
f"narrow the query (e.g. context_filter=['call'], or get_node for a "
|
||||
f"specific symbol).\n\n"
|
||||
+ output[:cut_at]
|
||||
+ f"\n... (truncated — {cut_count} more nodes cut by ~{token_budget}-token budget."
|
||||
f" Narrow with context_filter=['call'] or use get_node for a specific symbol)"
|
||||
)
|
||||
@@ -889,7 +939,10 @@ def _query_graph_text(
|
||||
header_parts.append(f"Context: {', '.join(resolved_filters)} ({filter_source})")
|
||||
header_parts.append(f"{len(nodes)} nodes found")
|
||||
header = " | ".join(header_parts) + "\n\n"
|
||||
return header + _subgraph_to_text(traversal_graph, nodes, edges, token_budget)
|
||||
# Pass the seeds so the queried symbol renders first and survives truncation
|
||||
# (#BUG2): a branch merge had silently dropped this argument, leaving the
|
||||
# seed-first ordering as dead code.
|
||||
return header + _subgraph_to_text(traversal_graph, nodes, edges, token_budget, seeds=start_nodes)
|
||||
|
||||
|
||||
def _find_node(G: nx.Graph, label: str) -> list[str]:
|
||||
@@ -1288,6 +1341,14 @@ def _build_server(graph_path: str):
|
||||
return f"No node matching '{label}' found."
|
||||
nid = matches[0]
|
||||
lines = [f"Neighbors of {sanitize_label(G.nodes[nid].get('label', nid))}:"]
|
||||
def _edge_at(d: dict) -> str:
|
||||
# Edge location = the relation SITE (call/import line) in the source
|
||||
# node's file, not a def line (#BUG1).
|
||||
loc = str(d.get("source_location") or "")
|
||||
return (
|
||||
f" at={sanitize_label(str(d.get('source_file') or ''))}:{sanitize_label(loc)}"
|
||||
if loc else ""
|
||||
)
|
||||
for nb in G.successors(nid):
|
||||
d = edge_data(G, nid, nb)
|
||||
rel = d.get("relation", "")
|
||||
@@ -1295,7 +1356,7 @@ def _build_server(graph_path: str):
|
||||
continue
|
||||
lines.append(
|
||||
f" --> {sanitize_label(G.nodes[nb].get('label', nb))} "
|
||||
f"[{sanitize_label(str(rel))}] [{sanitize_label(str(d.get('confidence', '')))}]"
|
||||
f"[{sanitize_label(str(rel))}] [{sanitize_label(str(d.get('confidence', '')))}]{_edge_at(d)}"
|
||||
)
|
||||
for nb in G.predecessors(nid):
|
||||
d = edge_data(G, nb, nid)
|
||||
@@ -1304,7 +1365,7 @@ def _build_server(graph_path: str):
|
||||
continue
|
||||
lines.append(
|
||||
f" <-- {sanitize_label(G.nodes[nb].get('label', nb))} "
|
||||
f"[{sanitize_label(str(rel))}] [{sanitize_label(str(d.get('confidence', '')))}]"
|
||||
f"[{sanitize_label(str(rel))}] [{sanitize_label(str(d.get('confidence', '')))}]{_edge_at(d)}"
|
||||
)
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
@@ -268,3 +268,46 @@ def test_affected_cli_source_file_path_uses_file_level_node(monkeypatch, tmp_pat
|
||||
assert "consumer.ts" in out
|
||||
assert "imports_from" in out
|
||||
assert "No unique node matched" not in out
|
||||
|
||||
|
||||
# ── BUG1: caller lists must show the call-SITE line, not the caller def line ──
|
||||
|
||||
def _write_callsite_graph(tmp_path):
|
||||
"""A caller whose call site (L158) differs from its own def line (L90)."""
|
||||
g = nx.DiGraph()
|
||||
g.add_node("loader", label="_load_apollo_app_state()",
|
||||
source_file="apollo_pipeline_status.py", source_location="L90")
|
||||
g.add_node("transition", label="transition_state()",
|
||||
source_file="state.py", source_location="L56")
|
||||
# The call happens at line 158 inside the caller's file.
|
||||
g.add_edge("loader", "transition", relation="calls", context="call",
|
||||
confidence="EXTRACTED", source_file="apollo_pipeline_status.py",
|
||||
source_location="L158")
|
||||
gp = tmp_path / "graph.json"
|
||||
gp.write_text(json.dumps(json_graph.node_link_data(g, edges="links")), encoding="utf-8")
|
||||
return gp
|
||||
|
||||
|
||||
def test_affected_reports_call_site_line_not_def_line(monkeypatch, tmp_path, capsys):
|
||||
gp = _write_callsite_graph(tmp_path)
|
||||
monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None)
|
||||
monkeypatch.setattr(mainmod.sys, "argv",
|
||||
["graphify", "affected", "transition_state", "--graph", str(gp)])
|
||||
mainmod.main()
|
||||
out = capsys.readouterr().out
|
||||
assert "apollo_pipeline_status.py:L158" in out, "must report the call SITE line (BUG1)"
|
||||
assert "apollo_pipeline_status.py:L90" not in out, "must NOT report the caller's def line"
|
||||
|
||||
|
||||
def test_affected_falls_back_to_def_line_when_edge_has_no_location(monkeypatch, tmp_path, capsys):
|
||||
"""An edge with no stored location honestly falls back to the node's def line."""
|
||||
g = nx.DiGraph()
|
||||
g.add_node("loader", label="load()", source_file="a.py", source_location="L90")
|
||||
g.add_node("t", label="target()", source_file="b.py", source_location="L5")
|
||||
g.add_edge("loader", "t", relation="calls", confidence="INFERRED") # no source_location
|
||||
gp = tmp_path / "graph.json"
|
||||
gp.write_text(json.dumps(json_graph.node_link_data(g, edges="links")), encoding="utf-8")
|
||||
monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None)
|
||||
monkeypatch.setattr(mainmod.sys, "argv", ["graphify", "affected", "target", "--graph", str(gp)])
|
||||
mainmod.main()
|
||||
assert "a.py:L90" in capsys.readouterr().out
|
||||
|
||||
@@ -123,3 +123,30 @@ def test_explain_no_lesson_line_for_unannotated_node(monkeypatch, tmp_path, caps
|
||||
p = _write_graph(tmp_path)
|
||||
out = _run(monkeypatch, p, "validateSanitySession", capsys)
|
||||
assert "Lesson:" not in out
|
||||
|
||||
|
||||
def test_explain_connection_shows_call_site_line(monkeypatch, tmp_path, capsys):
|
||||
"""BUG1: an explain connection shows the edge's call-SITE line (in the
|
||||
caller's file), not the caller's def line."""
|
||||
graph_data = {
|
||||
"directed": False, "multigraph": False, "graph": {},
|
||||
"nodes": [
|
||||
{"id": "loader", "label": "load_state()",
|
||||
"source_file": "apollo.py", "source_location": "L90", "community": 0},
|
||||
{"id": "trans", "label": "transition_state()",
|
||||
"source_file": "state.py", "source_location": "L56", "community": 0},
|
||||
],
|
||||
"links": [
|
||||
{"source": "loader", "target": "trans", "relation": "calls",
|
||||
"confidence": "EXTRACTED", "source_file": "apollo.py", "source_location": "L158"},
|
||||
],
|
||||
}
|
||||
p = tmp_path / "graph.json"
|
||||
p.write_text(json.dumps(graph_data))
|
||||
out = _run(monkeypatch, p, "transition_state", capsys)
|
||||
# The inbound caller line must cite the call site apollo.py:L158.
|
||||
caller_line = next(l for l in out.splitlines() if "<-- load_state()" in l)
|
||||
assert "apollo.py:L158" in caller_line, f"call site missing from: {caller_line!r}"
|
||||
assert "apollo.py:L90" not in caller_line # never the caller's def line
|
||||
# The queried node's own header still shows its def line (correct).
|
||||
assert "state.py" in out and "L56" in out
|
||||
|
||||
@@ -1270,3 +1270,68 @@ def test_score_query_collect_per_term_seeds_false_omits_tracking(monkeypatch):
|
||||
assert qs.best_seed_by_term == {}
|
||||
# And the combined output is still byte-identical to _score_nodes.
|
||||
assert qs.ranked == _score_nodes(G, ["foo", "bar", "baz"])
|
||||
|
||||
|
||||
# --- BUG2: seed survival, truncation notice, deterministic ordering ----------
|
||||
|
||||
def _star_graph(n_spokes=40):
|
||||
"""A high-degree hub plus a low-degree answer node, to force the answer past
|
||||
a pure degree-sorted / BFS cut unless seed-first ordering protects it."""
|
||||
G = nx.Graph()
|
||||
G.add_node("hub", label="Hub", source_file="hub.py", source_location="L1", community=0)
|
||||
for i in range(n_spokes):
|
||||
G.add_node(f"s{i}", label=f"spoke{i}", source_file=f"s{i}.py", source_location="L1", community=0)
|
||||
G.add_edge("hub", f"s{i}", relation="calls", confidence="EXTRACTED")
|
||||
# low-degree answer node, attached to one spoke
|
||||
G.add_node("answer", label="CompanySpacingGate", source_file="gate.py",
|
||||
source_location="L12", community=0)
|
||||
G.add_edge("s0", "answer", relation="calls", confidence="EXTRACTED")
|
||||
return G
|
||||
|
||||
|
||||
def test_subgraph_to_text_seed_survives_truncation():
|
||||
"""BUG2: a low-degree answer node passed as a seed is rendered first and
|
||||
survives a tiny budget, and truncation is announced."""
|
||||
G = _star_graph()
|
||||
nodes = set(G.nodes)
|
||||
text = _subgraph_to_text(G, nodes, list(G.edges()), token_budget=30, seeds=["answer"])
|
||||
assert "CompanySpacingGate" in text, "seed node was cut (BUG2)"
|
||||
node_lines = [l for l in text.splitlines() if l.startswith("NODE ")]
|
||||
assert "CompanySpacingGate" in node_lines[0], "seed must render first"
|
||||
assert "TRUNCATED" in text
|
||||
|
||||
|
||||
def test_query_graph_text_passes_seeds_so_answer_survives():
|
||||
"""BUG2 regression guard: the query path must pass seeds to the renderer (a
|
||||
branch merge had dropped the argument), so a queried low-degree symbol
|
||||
appears in the body even when the output is truncated."""
|
||||
G = _star_graph()
|
||||
text = _query_graph_text(G, "CompanySpacingGate", mode="bfs", depth=2, token_budget=40)
|
||||
# Present in the body, not merely the Start: header.
|
||||
body = text.split("\n\n", 1)[-1]
|
||||
assert "CompanySpacingGate" in body
|
||||
|
||||
|
||||
def test_subgraph_to_text_truncation_notice_at_top():
|
||||
G = _star_graph()
|
||||
text = _subgraph_to_text(G, set(G.nodes), list(G.edges()), token_budget=30, seeds=["answer"])
|
||||
assert text.startswith("[!] TRUNCATED"), f"notice not at top: {text[:60]!r}"
|
||||
assert "of" in text.splitlines()[0] and "nodes" in text.splitlines()[0]
|
||||
assert "truncated" in text # end marker still present
|
||||
|
||||
|
||||
def test_subgraph_to_text_no_notice_when_under_budget():
|
||||
G = _make_graph()
|
||||
text = _subgraph_to_text(G, {"n1", "n2"}, [("n1", "n2")], token_budget=2000)
|
||||
assert "TRUNCATED" not in text and "truncated" not in text
|
||||
|
||||
|
||||
def test_subgraph_to_text_order_is_deterministic():
|
||||
"""Equal-degree nodes render in a stable order regardless of set iteration."""
|
||||
G = nx.Graph()
|
||||
for i in range(10):
|
||||
G.add_node(f"z{i}", label=f"z{i}", source_file=f"z{i}.py", source_location="L1", community=0)
|
||||
nodes = set(G.nodes)
|
||||
a = _subgraph_to_text(G, nodes, [])
|
||||
b = _subgraph_to_text(G, set(reversed(list(nodes))), [])
|
||||
assert a == b
|
||||
|
||||
Reference in New Issue
Block a user