From ddc24c76d882e165943dd4355a3986c13bd54f56 Mon Sep 17 00:00:00 2001 From: Allionix Date: Sun, 11 Oct 2026 11:52:54 +0200 Subject: [PATCH] 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 (cherry picked from commit 13dc2400e1d7a2546637fa39fad9c524ef8859d5) --- README.md | 2 +- graphify/__main__.py | 2 + graphify/cli.py | 19 +++-- graphify/serve.py | 17 +++- tests/test_cli_export.py | 12 +-- tests/test_explain_cli.py | 3 +- tests/test_explain_same_file_tie.py | 5 +- tests/test_lookup_exit_codes.py | 128 ++++++++++++++++++++++++++++ tests/test_path_cli.py | 7 +- 9 files changed, 173 insertions(+), 22 deletions(-) create mode 100644 tests/test_lookup_exit_codes.py diff --git a/README.md b/README.md index a1d7be6c5..ab739f5f2 100644 --- a/README.md +++ b/README.md @@ -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)**. --- diff --git a/graphify/__main__.py b/graphify/__main__.py index 92a06b6b2..5f6a9d515 100644 --- a/graphify/__main__.py +++ b/graphify/__main__.py @@ -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 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)") diff --git a/graphify/cli.py b/graphify/cli.py index 183ce5e8e..45f1af93e 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -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 \"\" [--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 \"\" [--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 "" [--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}") diff --git a/graphify/serve.py b/graphify/serve.py index d1eb81e34..a4ad56079 100644 --- a/graphify/serve.py +++ b/graphify/serve.py @@ -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"::{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." ) diff --git a/tests/test_cli_export.py b/tests/test_cli_export.py index 70b192b5f..0bdffc316 100644 --- a/tests/test_cli_export.py +++ b/tests/test_cli_export.py @@ -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 diff --git a/tests/test_explain_cli.py b/tests/test_explain_cli.py index c8e90e54c..b9ad10802 100644 --- a/tests/test_explain_cli.py +++ b/tests/test_explain_cli.py @@ -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 diff --git a/tests/test_explain_same_file_tie.py b/tests/test_explain_same_file_tie.py index bc05063c2..1f26fd72e 100644 --- a/tests/test_explain_same_file_tie.py +++ b/tests/test_explain_same_file_tie.py @@ -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): diff --git a/tests/test_lookup_exit_codes.py b/tests/test_lookup_exit_codes.py new file mode 100644 index 000000000..f658b768f --- /dev/null +++ b/tests/test_lookup_exit_codes.py @@ -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 diff --git a/tests/test_path_cli.py b/tests/test_path_cli.py index 1deb9b151..03c10fb26 100644 --- a/tests/test_path_cli.py +++ b/tests/test_path_cli.py @@ -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