mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-13 15:40:05 +00:00
fix(cli): give lookups distinct exit codes for ambiguous and missing names
explain and query exited 0 when no node matched, the same status as an
answer, and path exited 1 for an ambiguous endpoint, a missing one and a
missing graph file alike. A script or an agent could not tell a refusal
from an answer without parsing the text.
explain, path and query now exit 3 when a name is ambiguous (the
candidates are listed) and 4 when no node matches. Messages and output
streams are unchanged; "no path found" between two existing nodes is
still an answer. The codes are documented in --help and the README.
Co-Authored-By: Claude <noreply@anthropic.com>
(cherry picked from commit 13dc2400e1)
This commit is contained in:
1 parent
7c4bf5671b
commit
ddc24c76d8
9 files changed
+173
-22
No files matched your search
@@ -381,7 +381,7 @@ Commit or share the graph so your whole team queries the same context, and revie
|
||||
|
||||
## Using the graph directly
|
||||
|
||||
Beyond your AI assistant, query `graph.json` straight from the CLI with `graphify query`, `graphify path`, and `graphify explain`. See **[Ask better graph questions](https://docs.graphify.com/guides/querying)** and the **[query reference](https://docs.graphify.com/reference/query)**.
|
||||
Beyond your AI assistant, query `graph.json` straight from the CLI with `graphify query`, `graphify path`, and `graphify explain`. A name that matches several nodes is refused with the candidates listed rather than guessed. Scripts can read the outcome from the exit code: `0` an answer, `3` an ambiguous name, `4` no matching node. See **[Ask better graph questions](https://docs.graphify.com/guides/querying)** and the **[query reference](https://docs.graphify.com/reference/query)**.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -909,6 +909,8 @@ def _run_cli() -> None:
|
||||
print(" --context C explicit edge-context filter (repeatable)")
|
||||
print(" --budget N cap output at N tokens (default 2000)")
|
||||
print(" --graph <path> path to graph.json (default graphify-out/graph.json)")
|
||||
print(" query, path and explain exit 0 with an answer, 3 when a name is ambiguous")
|
||||
print(" (the candidates are listed), 4 when no node matches")
|
||||
print(" affected \"X\" reverse traversal to find nodes impacted by X")
|
||||
print(" --relation R edge relation to traverse in reverse (repeatable)")
|
||||
print(" --depth N reverse traversal depth (default 2)")
|
||||
|
||||
+13
-6
@@ -84,6 +84,11 @@ _GEMINI_NUDGE_TEXT = (
|
||||
|
||||
_DEFAULT_NUDGE_GRAPH = "graphify-out/graph.json"
|
||||
|
||||
# Exit codes of the lookups (explain, path, query), so a script can tell a
|
||||
# refusal from an answer: 0 answered, 3 ambiguous name, 4 no matching node.
|
||||
EXIT_AMBIGUOUS = 3
|
||||
EXIT_NOT_FOUND = 4
|
||||
|
||||
|
||||
def _nudge_for_out(nudge: str) -> str:
|
||||
"""Name the effective graph.json in a hook-guard reminder (#4040).
|
||||
@@ -1311,7 +1316,7 @@ def dispatch_command(cmd: str) -> None:
|
||||
if len(sys.argv) < 3:
|
||||
print("Usage: graphify query \"<question>\" [--dfs] [--context C] [--budget N] [--graph path]", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
from graphify.serve import _query_graph_text
|
||||
from graphify.serve import _QUERY_NO_MATCH, _query_graph_text
|
||||
from graphify.security import sanitize_label
|
||||
from networkx.readwrite import json_graph
|
||||
from graphify import querylog
|
||||
@@ -1428,6 +1433,8 @@ def dispatch_command(cmd: str) -> None:
|
||||
)
|
||||
_touch_query_stamp(gp)
|
||||
print(_result)
|
||||
if _result == _QUERY_NO_MATCH:
|
||||
sys.exit(EXIT_NOT_FOUND)
|
||||
elif cmd == "affected":
|
||||
if len(sys.argv) < 3:
|
||||
print("Usage: graphify affected \"<node-or-label>\" [--relation R] [--depth N] [--graph path]", file=sys.stderr)
|
||||
@@ -1662,7 +1669,7 @@ def dispatch_command(cmd: str) -> None:
|
||||
file=sys.stderr,
|
||||
)
|
||||
sys.exit(1)
|
||||
from graphify.serve import _resolve_path_endpoint
|
||||
from graphify.serve import _is_ambiguity_refusal, _resolve_path_endpoint
|
||||
from networkx.readwrite import json_graph
|
||||
import networkx as _nx
|
||||
|
||||
@@ -1721,7 +1728,7 @@ def dispatch_command(cmd: str) -> None:
|
||||
):
|
||||
if _err or _nid is None:
|
||||
print(_err or f"No node matching '{_label}' found.", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
sys.exit(EXIT_AMBIGUOUS if _is_ambiguity_refusal(_err) else EXIT_NOT_FOUND)
|
||||
# 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).
|
||||
@@ -1731,7 +1738,7 @@ def dispatch_command(cmd: str) -> None:
|
||||
f"node '{src_nid}'. Use a more specific label or the exact node ID.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
sys.exit(1)
|
||||
sys.exit(EXIT_AMBIGUOUS)
|
||||
for _name, _scored, _nid in (
|
||||
("source", src_scored, src_nid),
|
||||
("target", tgt_scored, tgt_nid),
|
||||
@@ -1838,7 +1845,7 @@ def dispatch_command(cmd: str) -> None:
|
||||
if len(sys.argv) < 3:
|
||||
print('Usage: graphify explain "<node>" [--graph path]', file=sys.stderr)
|
||||
sys.exit(1)
|
||||
from graphify.serve import _find_node, _resolve_single_node
|
||||
from graphify.serve import _is_ambiguity_refusal, _resolve_single_node
|
||||
from networkx.readwrite import json_graph
|
||||
|
||||
label = sys.argv[2]
|
||||
@@ -1864,7 +1871,7 @@ def dispatch_command(cmd: str) -> None:
|
||||
nid, refusal = _resolve_single_node(G, label)
|
||||
if nid is None:
|
||||
print(refusal)
|
||||
sys.exit(1 if _find_node(G, label) else 0)
|
||||
sys.exit(EXIT_AMBIGUOUS if _is_ambiguity_refusal(refusal) else EXIT_NOT_FOUND)
|
||||
d = G.nodes[nid]
|
||||
print(f"Node: {d.get('label', nid)}")
|
||||
print(f" ID: {nid}")
|
||||
|
||||
+14
-3
@@ -1370,6 +1370,9 @@ def _traversal_view(G: nx.Graph) -> nx.Graph:
|
||||
G.graph["_traversal_view"] = H
|
||||
return H
|
||||
|
||||
_QUERY_NO_MATCH = "No matching nodes found."
|
||||
|
||||
|
||||
def _query_graph_text(
|
||||
G: nx.Graph,
|
||||
question: str,
|
||||
@@ -1403,7 +1406,7 @@ def _query_graph_text(
|
||||
}
|
||||
start_nodes = _pick_seeds(qs.ranked, G=G, best_seed_by_term=best_seed_by_term)
|
||||
if not start_nodes:
|
||||
return "No matching nodes found."
|
||||
return _QUERY_NO_MATCH
|
||||
resolved_filters, filter_source = _resolve_context_filters(question, context_filters)
|
||||
traversal_graph = _filter_graph_by_context(_traversal_view(G), resolved_filters)
|
||||
nodes, edges = _dfs(traversal_graph, start_nodes, depth) if mode == "dfs" else _bfs(traversal_graph, start_nodes, depth)
|
||||
@@ -1686,6 +1689,14 @@ def _tier_refusal(G: nx.Graph, label: str, tier: list[str], *, file_query: bool)
|
||||
return None
|
||||
|
||||
|
||||
_AMBIGUOUS = "Ambiguous: "
|
||||
|
||||
|
||||
def _is_ambiguity_refusal(message: str | None) -> bool:
|
||||
"""True for a refusal that lists candidates, False for a miss (no node matched)."""
|
||||
return message is not None and message.startswith(_AMBIGUOUS)
|
||||
|
||||
|
||||
def _ambiguity_message(G: nx.Graph, label: str, rivals: list[str]) -> str:
|
||||
listing = "\n".join(
|
||||
f" {G.nodes[r].get('source_file') or r}\n id: {r}" for r in rivals
|
||||
@@ -1694,7 +1705,7 @@ def _ambiguity_message(G: nx.Graph, label: str, rivals: list[str]) -> str:
|
||||
# nest a path-scoped one (`index.ts::foo()`) behind a second path.
|
||||
symbol = G.nodes[rivals[0]].get("label") or label
|
||||
return (
|
||||
f"Ambiguous: '{label}' matches {len(rivals)} nodes in different files.\n"
|
||||
f"{_AMBIGUOUS}'{label}' matches {len(rivals)} nodes in different files.\n"
|
||||
f"{listing}\n"
|
||||
f"Retry with path::symbol using one of the paths above (e.g. "
|
||||
f"<path>::{symbol}) or the full node id."
|
||||
@@ -1709,7 +1720,7 @@ def _same_file_ambiguity_message(G: nx.Graph, label: str, hits: list[str]) -> st
|
||||
for h in hits
|
||||
)
|
||||
return (
|
||||
f"Ambiguous: '{label}' matches {len(hits)} nodes {where}.\n"
|
||||
f"{_AMBIGUOUS}'{label}' matches {len(hits)} nodes {where}.\n"
|
||||
f"{listing}\n"
|
||||
f"Retry with the full node id."
|
||||
)
|
||||
|
||||
@@ -213,20 +213,20 @@ def test_export_falkordb_creates_cypher(tmp_path):
|
||||
|
||||
def test_query_returns_output(tmp_path):
|
||||
_make_graph(tmp_path)
|
||||
r = _run(["query", "test"], tmp_path)
|
||||
r = _run(["query", "Transformer"], tmp_path)
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert len(r.stdout) > 0
|
||||
|
||||
|
||||
def test_query_dfs_flag(tmp_path):
|
||||
_make_graph(tmp_path)
|
||||
r = _run(["query", "test", "--dfs"], tmp_path)
|
||||
r = _run(["query", "Transformer", "--dfs"], tmp_path)
|
||||
assert r.returncode == 0, r.stderr
|
||||
|
||||
|
||||
def test_query_budget_flag(tmp_path):
|
||||
_make_graph(tmp_path)
|
||||
r = _run(["query", "test", "--budget", "500"], tmp_path)
|
||||
r = _run(["query", "Transformer", "--budget", "500"], tmp_path)
|
||||
assert r.returncode == 0, r.stderr
|
||||
|
||||
|
||||
@@ -242,7 +242,7 @@ def test_query_uses_graphify_out_env(tmp_path):
|
||||
env = os.environ.copy()
|
||||
env["GRAPHIFY_OUT"] = custom_out.name
|
||||
|
||||
r = _run(["query", "test"], tmp_path, env=env)
|
||||
r = _run(["query", "Transformer"], tmp_path, env=env)
|
||||
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert len(r.stdout) > 0
|
||||
@@ -407,7 +407,7 @@ def test_path_flags_mutually_exclusive(tmp_path):
|
||||
|
||||
def test_explain_runs_without_error(tmp_path):
|
||||
_make_graph(tmp_path)
|
||||
r = _run(["explain", "test"], tmp_path)
|
||||
r = _run(["explain", "Transformer"], tmp_path)
|
||||
assert r.returncode == 0, r.stderr
|
||||
|
||||
|
||||
@@ -423,7 +423,7 @@ def test_explain_uses_graphify_out_env(tmp_path):
|
||||
env = os.environ.copy()
|
||||
env["GRAPHIFY_OUT"] = custom_out.name
|
||||
|
||||
r = _run(["explain", "test"], tmp_path, env=env)
|
||||
r = _run(["explain", "Transformer"], tmp_path, env=env)
|
||||
|
||||
assert r.returncode == 0, r.stderr
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
from __future__ import annotations
|
||||
import json
|
||||
import graphify.__main__ as mainmod
|
||||
from graphify.cli import EXIT_AMBIGUOUS
|
||||
|
||||
|
||||
def _write_graph(tmp_path):
|
||||
@@ -282,7 +283,7 @@ def test_explain_ambiguous_label_lists_every_candidate(monkeypatch, tmp_path, ca
|
||||
assert "Ambiguous" in out
|
||||
assert "services/chat/src/application/ports/metrics.port.ts" in out
|
||||
assert "services/scraping/src/application/ports/metrics.port.ts" in out
|
||||
assert code == 1
|
||||
assert code == EXIT_AMBIGUOUS
|
||||
# It must not present one file as the answer.
|
||||
assert "Node: MetricsPort\n ID:" not in out
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ import networkx as nx
|
||||
import pytest
|
||||
|
||||
import graphify.__main__ as mainmod
|
||||
from graphify.cli import EXIT_AMBIGUOUS
|
||||
from graphify.serve import _resolve_single_node
|
||||
|
||||
|
||||
@@ -73,7 +74,7 @@ def test_explain_refuses_same_file_twins(monkeypatch, tmp_path, capsys, label):
|
||||
assert "src_x_alphaclient_nonce" in out
|
||||
assert "src_x_betaclient_nonce" in out
|
||||
assert "Node: .nonce()" not in out
|
||||
assert code == 1
|
||||
assert code == EXIT_AMBIGUOUS
|
||||
|
||||
|
||||
def test_explain_refuses_sourceless_stubs(monkeypatch, tmp_path, capsys):
|
||||
@@ -82,7 +83,7 @@ def test_explain_refuses_sourceless_stubs(monkeypatch, tmp_path, capsys):
|
||||
assert "tests_a_php_entitymanagerinterface" in out
|
||||
assert "tests_b_php_entitymanagerinterface" in out
|
||||
assert "Node: EntityManagerInterface" not in out
|
||||
assert code == 1
|
||||
assert code == EXIT_AMBIGUOUS
|
||||
|
||||
|
||||
def test_explain_refusal_does_not_depend_on_node_order(monkeypatch, tmp_path, capsys):
|
||||
|
||||
@@ -0,0 +1,128 @@
|
||||
"""`explain`, `path` and `query` tell a script apart an answer, an ambiguous name
|
||||
and a name that matches nothing, through the exit code alone.
|
||||
|
||||
`explain` used to exit 0 on a miss, the same as a successful answer, and the
|
||||
ambiguity and miss refusals of `path` shared exit 1 with every other error.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
|
||||
import pytest
|
||||
|
||||
import graphify.__main__ as mainmod
|
||||
from graphify.cli import EXIT_AMBIGUOUS, EXIT_NOT_FOUND
|
||||
|
||||
|
||||
def _write_graph(tmp_path):
|
||||
nodes = [
|
||||
("src_a_alphaclient", "AlphaClient", "src/a.ts", "L1"),
|
||||
("src_a_alphaclient_nonce", ".nonce()", "src/a.ts", "L2"),
|
||||
("src_b_betaclient", "BetaClient", "src/b.ts", "L1"),
|
||||
("src_b_betaclient_nonce", ".nonce()", "src/b.ts", "L2"),
|
||||
("src_c_run", "run()", "src/c.ts", "L1"),
|
||||
("src_d_island", "island()", "src/d.ts", "L1"),
|
||||
]
|
||||
links = [
|
||||
("src_c_run", "src_b_betaclient", "calls"),
|
||||
("src_b_betaclient", "src_b_betaclient_nonce", "method"),
|
||||
("src_a_alphaclient", "src_a_alphaclient_nonce", "method"),
|
||||
]
|
||||
p = tmp_path / "graph.json"
|
||||
p.write_text(json.dumps({
|
||||
"directed": False, "multigraph": False, "graph": {},
|
||||
"nodes": [
|
||||
{"id": i, "label": lbl, "source_file": sf, "source_location": loc}
|
||||
for i, lbl, sf, loc in nodes
|
||||
],
|
||||
"links": [
|
||||
{"source": a, "target": b, "relation": rel, "confidence": "EXTRACTED"}
|
||||
for a, b, rel in links
|
||||
],
|
||||
}))
|
||||
return p
|
||||
|
||||
|
||||
def _run(monkeypatch, capsys, *argv):
|
||||
monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None)
|
||||
monkeypatch.setattr(mainmod.sys, "argv", ["graphify", *argv])
|
||||
try:
|
||||
mainmod.main()
|
||||
except SystemExit as exc:
|
||||
captured = capsys.readouterr()
|
||||
return exc.code, captured.out + captured.err
|
||||
captured = capsys.readouterr()
|
||||
return None, captured.out + captured.err
|
||||
|
||||
|
||||
def test_exit_codes_are_distinct_from_success_and_from_each_other():
|
||||
assert len({0, 1, EXIT_AMBIGUOUS, EXIT_NOT_FOUND}) == 4
|
||||
|
||||
|
||||
def test_explain_answer_exits_zero(monkeypatch, tmp_path, capsys):
|
||||
code, out = _run(monkeypatch, capsys, "explain", "run()", "--graph", str(_write_graph(tmp_path)))
|
||||
assert code is None
|
||||
assert "ID: src_c_run" in out
|
||||
|
||||
|
||||
def test_explain_ambiguous_exits_ambiguous(monkeypatch, tmp_path, capsys):
|
||||
code, out = _run(monkeypatch, capsys, "explain", ".nonce()", "--graph", str(_write_graph(tmp_path)))
|
||||
assert code == EXIT_AMBIGUOUS
|
||||
assert "Ambiguous: '.nonce()' matches 2 nodes in different files." in out
|
||||
|
||||
|
||||
@pytest.mark.parametrize("label", ["NoSuchSymbolXyz", "src/a.ts::NotThere"])
|
||||
def test_explain_miss_exits_not_found(monkeypatch, tmp_path, capsys, label):
|
||||
code, out = _run(monkeypatch, capsys, "explain", label, "--graph", str(_write_graph(tmp_path)))
|
||||
assert code == EXIT_NOT_FOUND
|
||||
assert f"No node matching '{label}' found." in out
|
||||
|
||||
|
||||
def test_path_answer_exits_zero(monkeypatch, tmp_path, capsys):
|
||||
code, out = _run(monkeypatch, capsys,
|
||||
"path", "run()", "src/b.ts::.nonce()", "--graph", str(_write_graph(tmp_path)))
|
||||
assert code is None
|
||||
assert "Shortest path (2 hops):" in out
|
||||
|
||||
|
||||
def test_path_without_route_is_an_answer(monkeypatch, tmp_path, capsys):
|
||||
"""Both endpoints exist and no route joins them: that is the answer, not a miss."""
|
||||
code, out = _run(monkeypatch, capsys,
|
||||
"path", "run()", "island()", "--graph", str(_write_graph(tmp_path)))
|
||||
assert code == 0
|
||||
assert "No directed path found" in out
|
||||
|
||||
|
||||
def test_path_ambiguous_endpoint_exits_ambiguous(monkeypatch, tmp_path, capsys):
|
||||
code, out = _run(monkeypatch, capsys,
|
||||
"path", "run()", ".nonce()", "--graph", str(_write_graph(tmp_path)))
|
||||
assert code == EXIT_AMBIGUOUS
|
||||
assert "Ambiguous: '.nonce()'" in out
|
||||
|
||||
|
||||
def test_path_same_node_both_ends_exits_ambiguous(monkeypatch, tmp_path, capsys):
|
||||
code, out = _run(monkeypatch, capsys,
|
||||
"path", "run()", "src_c_run", "--graph", str(_write_graph(tmp_path)))
|
||||
assert code == EXIT_AMBIGUOUS
|
||||
assert "both resolved to the same node" in out
|
||||
|
||||
|
||||
@pytest.mark.parametrize("endpoint", ["NoSuchSymbolXyz", "src/a.ts::NotThere"])
|
||||
def test_path_missing_endpoint_exits_not_found(monkeypatch, tmp_path, capsys, endpoint):
|
||||
code, out = _run(monkeypatch, capsys,
|
||||
"path", "run()", endpoint, "--graph", str(_write_graph(tmp_path)))
|
||||
assert code == EXIT_NOT_FOUND
|
||||
assert "Shortest path" not in out
|
||||
|
||||
|
||||
def test_query_answer_exits_zero(monkeypatch, tmp_path, capsys):
|
||||
code, out = _run(monkeypatch, capsys, "query", "run", "--graph", str(_write_graph(tmp_path)))
|
||||
assert code is None
|
||||
assert "NODE run()" in out
|
||||
|
||||
|
||||
def test_query_without_any_match_exits_not_found(monkeypatch, tmp_path, capsys):
|
||||
code, out = _run(monkeypatch, capsys,
|
||||
"query", "zzqqxx", "--graph", str(_write_graph(tmp_path)))
|
||||
assert code == EXIT_NOT_FOUND
|
||||
assert "No matching nodes found." in out
|
||||
@@ -9,6 +9,7 @@ import networkx as nx
|
||||
import pytest
|
||||
from networkx.readwrite import json_graph
|
||||
import graphify.__main__ as mainmod
|
||||
from graphify.cli import EXIT_AMBIGUOUS, EXIT_NOT_FOUND
|
||||
|
||||
|
||||
def _write_graph(tmp_path):
|
||||
@@ -385,7 +386,7 @@ def test_path_refuses_ambiguous_endpoint(monkeypatch, tmp_path, capsys, ends):
|
||||
["graphify", "path", *ends, "--graph", str(gp)])
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
mainmod.main()
|
||||
assert exc_info.value.code == 1
|
||||
assert exc_info.value.code == EXIT_AMBIGUOUS
|
||||
captured = capsys.readouterr()
|
||||
assert "Shortest path" not in captured.out
|
||||
assert "Ambiguous: '.nonce()' matches 2 nodes in different files." in captured.err
|
||||
@@ -462,7 +463,7 @@ def test_path_refuses_same_file_duplicate_endpoint(monkeypatch, tmp_path, capsys
|
||||
["graphify", "path", "run()", target, "--graph", str(gp)])
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
mainmod.main()
|
||||
assert exc_info.value.code == 1
|
||||
assert exc_info.value.code == EXIT_AMBIGUOUS
|
||||
captured = capsys.readouterr()
|
||||
assert "Shortest path" not in captured.out
|
||||
assert f"Ambiguous: '{target}' matches 2 nodes in src/x.ts." in captured.err
|
||||
@@ -534,7 +535,7 @@ def test_path_scoped_endpoint_refuses_when_file_has_no_symbol(monkeypatch, tmp_p
|
||||
["graphify", "path", "src/admin/route.ts::readRoute", "chargeCard", "--graph", str(gp)])
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
mainmod.main()
|
||||
assert exc_info.value.code == 1
|
||||
assert exc_info.value.code == EXIT_NOT_FOUND
|
||||
captured = capsys.readouterr()
|
||||
assert "Shortest path" not in captured.out
|
||||
assert "src/admin/route.ts" in captured.err and "readRoute" in captured.err
|
||||
|
||||
Reference in new issue
Block a user