From 242cda3f182aea0216527558da3707e65498964e Mon Sep 17 00:00:00 2001 From: Jais Sebastian Date: Sun, 10 May 2026 12:38:42 +0530 Subject: [PATCH 1/2] feat: surface graph staleness, omitted counts, and symbol disambiguation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tool responses now carry the information agents need to self-correct without extra round-trips: graph_meta block — every query_graph, semantic_search_nodes, and get_impact_radius response includes indexed_at (ISO timestamp), indexed_commit / head_commit (8-char SHAs from the graph DB and current HEAD), and is_stale so an agent knows whether to re-index before trusting callers_of results. Omitted counts — query_graph gains max_results (default 100) and always returns results_omitted so agents don't mistake a cap for "no callers found". Minimal mode reports the same. semantic_search_nodes and get_impact_radius minimal modes add their own omitted counts. Symbol disambiguation — when a bare name matches multiple nodes, the response now returns a ranked disambiguation list (exact QN > exact name > partial match) with qualified_name, file_path, and line_start for each candidate, plus a hint field explaining the fix. Replaces the old unranked candidates key. 29 tests in tests/test_agent_transparency.py covering all three features. Co-Authored-By: Claude Sonnet 4.6 --- code_review_graph/tools/_common.py | 36 +++ code_review_graph/tools/query.py | 93 +++++- tests/test_agent_transparency.py | 494 +++++++++++++++++++++++++++++ 3 files changed, 609 insertions(+), 14 deletions(-) create mode 100644 tests/test_agent_transparency.py diff --git a/code_review_graph/tools/_common.py b/code_review_graph/tools/_common.py index fd206a9b..557cc64a 100644 --- a/code_review_graph/tools/_common.py +++ b/code_review_graph/tools/_common.py @@ -2,6 +2,7 @@ from __future__ import annotations +import subprocess from pathlib import Path from typing import Any @@ -85,6 +86,41 @@ def _get_store(repo_root: str | None = None) -> tuple[GraphStore, Path]: return GraphStore(db_path), root +def graph_meta(store: GraphStore, root: Path) -> dict[str, Any]: + """Return graph freshness metadata for inclusion in tool responses. + + Agents use this to detect staleness (graph lags the working branch) + and avoid trusting caller results that may reflect an older index. + Fields: indexed_at (ISO timestamp), indexed_commit (SHA at last build), + head_commit (current HEAD), is_stale (bool, omitted when unknown). + """ + indexed_at = store.get_metadata("last_updated") or "unknown" + # git_head_sha is stored as the full SHA at build time + indexed_commit: str | None = store.get_metadata("git_head_sha") + + try: + result = subprocess.run( + ["git", "-C", str(root), "rev-parse", "HEAD"], + capture_output=True, text=True, timeout=5, + ) + head_commit: str | None = result.stdout.strip() if result.returncode == 0 else None + except Exception: + head_commit = None + + is_stale: bool | None = None + if indexed_commit and head_commit: + is_stale = head_commit != indexed_commit + + meta: dict[str, Any] = {"indexed_at": indexed_at} + if indexed_commit: + meta["indexed_commit"] = indexed_commit[:8] + if head_commit: + meta["head_commit"] = head_commit[:8] + if is_stale is not None: + meta["is_stale"] = is_stale + return meta + + def compact_response( summary: str, key_entities: list[str] | None = None, diff --git a/code_review_graph/tools/query.py b/code_review_graph/tools/query.py index e3d8c3e1..04bff256 100644 --- a/code_review_graph/tools/query.py +++ b/code_review_graph/tools/query.py @@ -7,11 +7,11 @@ from typing import Any from ..embeddings import EmbeddingStore -from ..graph import _sanitize_name, edge_to_dict, node_to_dict +from ..graph import GraphNode, _sanitize_name, edge_to_dict, node_to_dict from ..hints import generate_hints, get_session from ..incremental import get_changed_files, get_db_path, get_staged_and_unstaged from ..search import hybrid_search -from ._common import _BUILTIN_CALL_NAMES, _get_store +from ._common import _BUILTIN_CALL_NAMES, _get_store, graph_meta logger = logging.getLogger(__name__) @@ -96,6 +96,8 @@ def get_impact_radius( f" of {total_impacted} impacted nodes" ) + meta = graph_meta(store, root) + if detail_level == "minimal": impacted_count = len(impacted_dicts) if impacted_count > 20: @@ -104,9 +106,8 @@ def get_impact_radius( risk = "medium" else: risk = "low" - key_entities = [ - n["name"] for n in impacted_dicts[:5] - ] + key_entities = [n["name"] for n in impacted_dicts[:5]] + nodes_omitted = max(0, total_impacted - len(impacted_dicts)) return { "status": "ok", "summary": "\n".join(summary_parts), @@ -114,6 +115,8 @@ def get_impact_radius( "impacted_file_count": len(result["impacted_files"]), "key_entities": key_entities, "truncated": truncated, + "nodes_omitted": nodes_omitted, + "graph_meta": meta, } return { @@ -126,6 +129,7 @@ def get_impact_radius( "edges": edge_dicts, "truncated": truncated, "total_impacted": total_impacted, + "graph_meta": meta, } finally: store.close() @@ -136,11 +140,41 @@ def get_impact_radius( # --------------------------------------------------------------------------- +def _rank_disambiguation_candidates(candidates: list[GraphNode], target: str) -> list[dict]: + """Rank disambiguation candidates by match quality and return enriched dicts. + + Ranking: exact qualified_name match > exact name match > partial match. + Each entry includes qualified_name, name, kind, file_path, and line_start + so the agent can pick the right one without a follow-up call. + """ + def _score(node: GraphNode) -> int: + if node.qualified_name == target: + return 0 + if node.name == target: + return 1 + if target.lower() in node.qualified_name.lower(): + return 2 + return 3 + + ranked = sorted(candidates, key=_score) + return [ + { + "qualified_name": _sanitize_name(n.qualified_name), + "name": _sanitize_name(n.name), + "kind": n.kind, + "file_path": n.file_path, + "line_start": n.line_start, + } + for n in ranked + ] + + def query_graph( pattern: str, target: str, repo_root: str | None = None, detail_level: str = "standard", + max_results: int = 100, ) -> dict[str, Any]: """Run a predefined graph query. @@ -150,12 +184,17 @@ def query_graph( target: The node name, qualified name, or file path to query about. repo_root: Repository root path. Auto-detected if omitted. detail_level: "standard" (full output) or "minimal" (summary only). + max_results: Cap on results returned (default 100). Excess count + is reported in ``results_omitted`` so agents know when to refine. Returns: - Matching nodes and edges for the query. + Matching nodes and edges for the query, plus graph_meta for staleness + awareness and results_omitted when the cap is hit. """ store, root = _get_store(repo_root) try: + meta = graph_meta(store, root) + if pattern not in _QUERY_PATTERNS: return { "status": "error", @@ -183,7 +222,7 @@ def query_graph( f"'{target}' is a common builtin " "— callers_of skipped to avoid noise." ), - "results": [], "edges": [], + "results": [], "edges": [], "graph_meta": meta, } # Resolve target - try as-is, then as absolute path, then search @@ -192,25 +231,33 @@ def query_graph( abs_target = str(root / target) node = store.get_node(abs_target) if not node: - # Search by name - candidates = store.search_nodes(target, limit=5) + # Search by name — return ranked disambiguation on multi-match + candidates = store.search_nodes(target, limit=10) if len(candidates) == 1: node = candidates[0] target = node.qualified_name elif len(candidates) > 1: + ranked = _rank_disambiguation_candidates(candidates, target) return { "status": "ambiguous", "summary": ( - f"Multiple matches for '{target}'. " - "Please use a qualified name." + f"'{target}' matches {len(candidates)} node(s). " + "Re-run with the qualified_name from the " + "disambiguation list." + ), + "disambiguation": ranked, + "hint": ( + "Use the 'qualified_name' field from one of the entries " + "above as the 'target' parameter." ), - "candidates": [node_to_dict(c) for c in candidates], + "graph_meta": meta, } if not node and pattern != "file_summary": return { "status": "not_found", "summary": f"No node found matching '{target}'.", + "graph_meta": meta, } qn = node.qualified_name if node else target @@ -308,9 +355,16 @@ def query_graph( for n in file_nodes: results.append(node_to_dict(n)) + # Apply max_results cap and record how many were omitted + total_results = len(results) + results_omitted = max(0, total_results - max_results) + results = results[:max_results] + edges_out = edges_out[:max_results] + summary = ( - f"Found {len(results)} result(s) " + f"Found {total_results} result(s) " f"for {pattern}('{target}')" + + (f" — showing {max_results}, {results_omitted} omitted" if results_omitted else "") ) if detail_level == "minimal": @@ -322,14 +376,17 @@ def query_graph( } for r in results[:5] ] + results_omitted_minimal = max(0, total_results - 5) return { "status": "ok", "pattern": pattern, "target": target, "description": _QUERY_PATTERNS[pattern], "summary": summary, - "result_count": len(results), + "result_count": total_results, "results": minimal_results, + "results_omitted": results_omitted_minimal, + "graph_meta": meta, } return { @@ -340,6 +397,8 @@ def query_graph( "summary": summary, "results": results, "edges": edges_out, + "results_omitted": results_omitted, + "graph_meta": meta, } finally: store.close() @@ -393,6 +452,8 @@ def semantic_search_nodes( f" (kind={kind})" if kind else "" ) + meta = graph_meta(store, root) + if detail_level == "minimal": minimal_results = [ { @@ -402,12 +463,15 @@ def semantic_search_nodes( } for r in results[:5] ] + results_omitted = max(0, len(results) - 5) return { "status": "ok", "query": query, "search_mode": search_mode, "summary": summary, "results": minimal_results, + "results_omitted": results_omitted, + "graph_meta": meta, } result: dict[str, object] = { @@ -416,6 +480,7 @@ def semantic_search_nodes( "search_mode": search_mode, "summary": summary, "results": results, + "graph_meta": meta, } result["_hints"] = generate_hints( "semantic_search_nodes", result, get_session() diff --git a/tests/test_agent_transparency.py b/tests/test_agent_transparency.py new file mode 100644 index 00000000..1986632a --- /dev/null +++ b/tests/test_agent_transparency.py @@ -0,0 +1,494 @@ +"""Tests for agent-transparency improvements in tool responses: + + graph_meta — confidence + staleness surfaced in every tool response + omitted counts — results_omitted / nodes_omitted when results are capped + disambiguation — ranked FQN list with file:line on multi-match symbols +""" + +from __future__ import annotations + +import shutil +import tempfile +from pathlib import Path +from unittest.mock import patch + +import pytest + +from code_review_graph.graph import GraphStore +from code_review_graph.parser import EdgeInfo, NodeInfo +from code_review_graph.tools._common import graph_meta +from code_review_graph.tools.query import ( + _rank_disambiguation_candidates, + query_graph, + semantic_search_nodes, +) + + +# --------------------------------------------------------------------------- +# Shared fixture helpers +# --------------------------------------------------------------------------- + +def _make_repo(tmp_path: Path) -> tuple[GraphStore, Path]: + """Create a minimal repo dir with .git and a graph db.""" + root = tmp_path / "repo" + root.mkdir() + (root / ".git").mkdir() + (root / ".code-review-graph").mkdir() + db = GraphStore(str(root / ".code-review-graph" / "graph.db")) + return db, root + + +def _seed_basic(store: GraphStore, root: Path) -> None: + store.upsert_node(NodeInfo( + kind="File", name="auth.py", file_path=str(root / "auth.py"), + line_start=1, line_end=50, language="python", + )) + store.upsert_node(NodeInfo( + kind="Function", name="login", file_path=str(root / "auth.py"), + line_start=10, line_end=20, language="python", + parent_name="AuthService", + )) + store.upsert_node(NodeInfo( + kind="Function", name="process", file_path=str(root / "main.py"), + line_start=5, line_end=15, language="python", + )) + # Edge target matches the qualified name: file_path::ParentClass.method + store.upsert_edge(EdgeInfo( + kind="CALLS", + source=str(root / "main.py") + "::process", + target=str(root / "auth.py") + "::AuthService.login", + file_path=str(root / "main.py"), + line=10, + )) + store.commit() + + +# --------------------------------------------------------------------------- +# graph_meta helper +# --------------------------------------------------------------------------- + +class TestGraphMeta: + def test_returns_indexed_at(self, tmp_path): + store, root = _make_repo(tmp_path) + try: + store.set_metadata("last_updated", "2024-01-15T12:00:00") + meta = graph_meta(store, root) + assert meta["indexed_at"] == "2024-01-15T12:00:00" + finally: + store.close() + + def test_unknown_when_no_last_updated(self, tmp_path): + store, root = _make_repo(tmp_path) + try: + meta = graph_meta(store, root) + assert meta["indexed_at"] == "unknown" + finally: + store.close() + + def test_is_stale_false_when_same_commit(self, tmp_path): + store, root = _make_repo(tmp_path) + try: + sha = "abc1234def5678ab1234def5678ab1234def5678" + store.set_metadata("git_head_sha", sha) + + with patch( + "code_review_graph.tools._common.subprocess.run" + ) as mock_run: + mock_run.return_value.returncode = 0 + mock_run.return_value.stdout = sha + "\n" + meta = graph_meta(store, root) + + assert meta["is_stale"] is False + assert meta["indexed_commit"] == sha[:8] + assert meta["head_commit"] == sha[:8] + finally: + store.close() + + def test_is_stale_true_when_different_commit(self, tmp_path): + store, root = _make_repo(tmp_path) + try: + indexed_sha = "aaaa1234" + "a" * 32 + head_sha = "bbbb5678" + "b" * 32 + store.set_metadata("git_head_sha", indexed_sha) + + with patch( + "code_review_graph.tools._common.subprocess.run" + ) as mock_run: + mock_run.return_value.returncode = 0 + mock_run.return_value.stdout = head_sha + "\n" + meta = graph_meta(store, root) + + assert meta["is_stale"] is True + finally: + store.close() + + def test_no_is_stale_when_no_indexed_commit(self, tmp_path): + store, root = _make_repo(tmp_path) + try: + with patch( + "code_review_graph.tools._common.subprocess.run" + ) as mock_run: + mock_run.return_value.returncode = 0 + mock_run.return_value.stdout = "deadbeef12345678\n" + meta = graph_meta(store, root) + + assert "is_stale" not in meta + assert "indexed_commit" not in meta + finally: + store.close() + + def test_git_failure_gives_no_head_commit(self, tmp_path): + store, root = _make_repo(tmp_path) + try: + with patch( + "code_review_graph.tools._common.subprocess.run" + ) as mock_run: + mock_run.return_value.returncode = 128 + mock_run.return_value.stdout = "" + meta = graph_meta(store, root) + + assert "head_commit" not in meta + finally: + store.close() + + +# --------------------------------------------------------------------------- +# graph_meta in query_graph responses +# --------------------------------------------------------------------------- + +class TestQueryGraphMeta: + def setup_method(self): + self.tmp = tempfile.mkdtemp() + self.root = Path(self.tmp) / "repo" + self.root.mkdir() + (self.root / ".git").mkdir() + (self.root / ".code-review-graph").mkdir() + self.db = GraphStore(str(self.root / ".code-review-graph" / "graph.db")) + _seed_basic(self.db, self.root) + self.db.set_metadata("last_updated", "2024-06-01T00:00:00") + self.db.close() + + def teardown_method(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def test_graph_meta_in_ok_response(self): + # QN includes parent class: file_path::AuthService.login + target = str(self.root / "auth.py") + "::AuthService.login" + result = query_graph( + pattern="callers_of", + target=target, + repo_root=str(self.root), + ) + assert result["status"] == "ok" + assert "graph_meta" in result + assert result["graph_meta"]["indexed_at"] == "2024-06-01T00:00:00" + + def test_graph_meta_in_not_found_response(self): + result = query_graph( + pattern="callers_of", + target="nonexistent_symbol_xyz", + repo_root=str(self.root), + ) + assert result["status"] == "not_found" + assert "graph_meta" in result + + def test_graph_meta_in_minimal_mode(self): + result = query_graph( + pattern="callers_of", + target=str(self.root / "auth.py") + "::login", + repo_root=str(self.root), + detail_level="minimal", + ) + assert "graph_meta" in result + + def test_confidence_in_edge_results(self): + # QN includes parent class: file_path::AuthService.login + target = str(self.root / "auth.py") + "::AuthService.login" + result = query_graph( + pattern="callers_of", + target=target, + repo_root=str(self.root), + ) + assert result["status"] == "ok" + if result["edges"]: + edge = result["edges"][0] + assert "confidence" in edge + assert isinstance(edge["confidence"], float) + assert "confidence_tier" in edge + + +# --------------------------------------------------------------------------- +# graph_meta in semantic_search_nodes responses +# --------------------------------------------------------------------------- + +class TestSemanticSearchMeta: + def setup_method(self): + self.tmp = tempfile.mkdtemp() + self.root = Path(self.tmp) / "repo" + self.root.mkdir() + (self.root / ".git").mkdir() + (self.root / ".code-review-graph").mkdir() + self.db = GraphStore(str(self.root / ".code-review-graph" / "graph.db")) + _seed_basic(self.db, self.root) + self.db.set_metadata("last_updated", "2024-06-01T00:00:00") + self.db.close() + + def teardown_method(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def test_graph_meta_in_standard_response(self): + result = semantic_search_nodes( + query="login", repo_root=str(self.root), + ) + assert "graph_meta" in result + assert result["graph_meta"]["indexed_at"] == "2024-06-01T00:00:00" + + def test_graph_meta_in_minimal_response(self): + result = semantic_search_nodes( + query="login", repo_root=str(self.root), detail_level="minimal", + ) + assert "graph_meta" in result + + +# --------------------------------------------------------------------------- +# Truncation counts (results_omitted / nodes_omitted) +# --------------------------------------------------------------------------- + +class TestTruncationCounts: + def setup_method(self): + self.tmp = tempfile.mkdtemp() + self.root = Path(self.tmp) / "repo" + self.root.mkdir() + (self.root / ".git").mkdir() + (self.root / ".code-review-graph").mkdir() + self.db = GraphStore(str(self.root / ".code-review-graph" / "graph.db")) + self._seed_many_callers() + self.db.close() + + def _seed_many_callers(self) -> None: + target_qn = str(self.root / "lib.py") + "::target_fn" + self.db.upsert_node(NodeInfo( + kind="Function", name="target_fn", file_path=str(self.root / "lib.py"), + line_start=1, line_end=5, language="python", + )) + for i in range(15): + caller_qn = str(self.root / f"caller_{i}.py") + f"::caller_{i}" + self.db.upsert_node(NodeInfo( + kind="Function", name=f"caller_{i}", + file_path=str(self.root / f"caller_{i}.py"), + line_start=1, line_end=3, language="python", + )) + self.db.upsert_edge(EdgeInfo( + kind="CALLS", source=caller_qn, target=target_qn, + file_path=str(self.root / f"caller_{i}.py"), line=2, + )) + self.db.commit() + + def teardown_method(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def test_results_omitted_zero_when_under_cap(self): + target_qn = str(self.root / "lib.py") + "::target_fn" + result = query_graph( + pattern="callers_of", target=target_qn, + repo_root=str(self.root), max_results=100, + ) + assert result["status"] == "ok" + assert result["results_omitted"] == 0 + + def test_results_omitted_when_cap_exceeded(self): + target_qn = str(self.root / "lib.py") + "::target_fn" + result = query_graph( + pattern="callers_of", target=target_qn, + repo_root=str(self.root), max_results=5, + ) + assert result["status"] == "ok" + assert result["results_omitted"] == 10 # 15 total - 5 cap + assert len(result["results"]) == 5 + + def test_results_omitted_in_minimal_mode(self): + target_qn = str(self.root / "lib.py") + "::target_fn" + result = query_graph( + pattern="callers_of", target=target_qn, + repo_root=str(self.root), detail_level="minimal", + ) + assert result["status"] == "ok" + # minimal shows only 5, total is 15 → 10 omitted + assert result["results_omitted"] == 10 + assert len(result["results"]) == 5 + + def test_result_count_reflects_total(self): + target_qn = str(self.root / "lib.py") + "::target_fn" + result = query_graph( + pattern="callers_of", target=target_qn, + repo_root=str(self.root), detail_level="minimal", + ) + assert result["result_count"] == 15 + + def test_summary_mentions_omitted_when_capped(self): + target_qn = str(self.root / "lib.py") + "::target_fn" + result = query_graph( + pattern="callers_of", target=target_qn, + repo_root=str(self.root), max_results=3, + ) + assert "omitted" in result["summary"] + + def test_semantic_search_results_omitted_in_minimal(self): + # Seed 10 functions with similar names + db = GraphStore(str(self.root / ".code-review-graph" / "graph.db")) + for i in range(10): + db.upsert_node(NodeInfo( + kind="Function", name=f"do_thing_{i}", + file_path=str(self.root / f"m{i}.py"), + line_start=1, line_end=3, language="python", + )) + db.commit() + db.close() + + result = semantic_search_nodes( + query="do_thing", repo_root=str(self.root), + limit=10, detail_level="minimal", + ) + # minimal caps at 5; results_omitted should reflect the difference + assert "results_omitted" in result + assert isinstance(result["results_omitted"], int) + assert result["results_omitted"] >= 0 + + +# --------------------------------------------------------------------------- +# Symbol disambiguation +# --------------------------------------------------------------------------- + +class TestDisambiguationRanking: + """Unit tests for _rank_disambiguation_candidates.""" + + def _make_node(self, qn: str, name: str, kind: str = "Function", + file_path: str = "/repo/a.py", line_start: int = 1) -> object: + from code_review_graph.graph import GraphNode + return GraphNode( + id=1, kind=kind, name=name, qualified_name=qn, + file_path=file_path, line_start=line_start, line_end=10, + language="python", parent_name=None, params=None, + return_type=None, is_test=False, file_hash=None, extra={}, + ) + + def test_exact_qn_ranked_first(self): + nodes = [ + self._make_node("pkg.foo::login", "login", file_path="/a/foo.py"), + self._make_node("pkg.bar::login", "login", file_path="/a/bar.py"), + self._make_node("pkg.foo::login", "login", file_path="/a/exact.py"), + ] + # Use exact match as target + nodes[2] = self._make_node("exact_target", "login", file_path="/a/exact.py") + result = _rank_disambiguation_candidates(nodes, "exact_target") + assert result[0]["qualified_name"] == "exact_target" + + def test_exact_name_ranked_before_partial(self): + nodes = [ + self._make_node("partial_match::login_helper", "login_helper"), + self._make_node("exact_match::login", "login"), + ] + result = _rank_disambiguation_candidates(nodes, "login") + assert result[0]["name"] == "login" + + def test_enriched_with_file_and_line(self): + nodes = [ + self._make_node("pkg::foo", "foo", file_path="/repo/foo.py", line_start=42), + ] + result = _rank_disambiguation_candidates(nodes, "foo") + assert result[0]["file_path"] == "/repo/foo.py" + assert result[0]["line_start"] == 42 + assert result[0]["qualified_name"] == "pkg::foo" + assert result[0]["kind"] == "Function" + + def test_names_are_sanitized(self): + nodes = [ + self._make_node("pkg::bad\x00name", "bad\x00name"), + ] + result = _rank_disambiguation_candidates(nodes, "badname") + assert "\x00" not in result[0]["qualified_name"] + assert "\x00" not in result[0]["name"] + + +class TestQueryGraphDisambiguation: + """Integration tests for ambiguous query_graph responses.""" + + def setup_method(self): + self.tmp = tempfile.mkdtemp() + self.root = Path(self.tmp) / "repo" + self.root.mkdir() + (self.root / ".git").mkdir() + (self.root / ".code-review-graph").mkdir() + self.db = GraphStore(str(self.root / ".code-review-graph" / "graph.db")) + # Two functions with the same bare name in different files + self.db.upsert_node(NodeInfo( + kind="Function", name="process", file_path=str(self.root / "a.py"), + line_start=1, line_end=10, language="python", + )) + self.db.upsert_node(NodeInfo( + kind="Function", name="process", file_path=str(self.root / "b.py"), + line_start=20, line_end=30, language="python", + )) + self.db.commit() + self.db.close() + + def teardown_method(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def test_ambiguous_status_on_multi_match(self): + result = query_graph( + pattern="callers_of", target="process", + repo_root=str(self.root), + ) + assert result["status"] == "ambiguous" + + def test_disambiguation_key_present(self): + result = query_graph( + pattern="callers_of", target="process", + repo_root=str(self.root), + ) + assert "disambiguation" in result + assert isinstance(result["disambiguation"], list) + assert len(result["disambiguation"]) >= 2 + + def test_disambiguation_includes_required_fields(self): + result = query_graph( + pattern="callers_of", target="process", + repo_root=str(self.root), + ) + for entry in result["disambiguation"]: + assert "qualified_name" in entry + assert "name" in entry + assert "kind" in entry + assert "file_path" in entry + assert "line_start" in entry + + def test_hint_field_present(self): + result = query_graph( + pattern="callers_of", target="process", + repo_root=str(self.root), + ) + assert "hint" in result + assert "qualified_name" in result["hint"] + + def test_graph_meta_in_ambiguous_response(self): + result = query_graph( + pattern="callers_of", target="process", + repo_root=str(self.root), + ) + assert "graph_meta" in result + + def test_summary_mentions_count(self): + result = query_graph( + pattern="callers_of", target="process", + repo_root=str(self.root), + ) + assert "2" in result["summary"] or "multiple" in result["summary"].lower() + + def test_no_more_candidates_key(self): + """Old 'candidates' key should be gone; replaced by 'disambiguation'.""" + result = query_graph( + pattern="callers_of", target="process", + repo_root=str(self.root), + ) + assert "candidates" not in result From cb08a350351986752b38fc640f82a652e3749861 Mon Sep 17 00:00:00 2001 From: Jais Sebastian Date: Sun, 10 May 2026 13:26:37 +0530 Subject: [PATCH 2/2] fix: resolve Java FQN targets in query_graph (package.Class.method format) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agents and IDEs write targets in Java package-qualified form: com.example.service.OrderHandler.process The graph indexes nodes by file-path qualified name: /repo/src/.../OrderHandler.java::OrderHandler.process Without this fix, lookup silently returns not_found even though the node exists, because the Java package prefix prevents both the exact-match and the FTS fallback from resolving it. Fix: four-step resolution cascade in query_graph — 1. Exact graph QN lookup 2. Absolute path lookup 3. FTS search on raw target string 4. Java FQN decomposition: strip package prefix, try "Class.method" (last 2 segments), then bare "method" (last 1 segment) Each step only runs if the previous returned nothing. If decomposition yields one match it resolves transparently; if it yields multiple it returns the ranked disambiguation list so the agent can re-run with the correct qualified name. 4 new tests in TestJavaFQNResolution covering ClassName.method, package.Class.method, unique bare name, and ambiguous decomposition. Co-Authored-By: Claude Sonnet 4.6 --- code_review_graph/tools/query.py | 26 +++++- tests/test_agent_transparency.py | 144 +++++++++++++++++++++++++++++++ 2 files changed, 168 insertions(+), 2 deletions(-) diff --git a/code_review_graph/tools/query.py b/code_review_graph/tools/query.py index 04bff256..e83a2572 100644 --- a/code_review_graph/tools/query.py +++ b/code_review_graph/tools/query.py @@ -225,14 +225,36 @@ def query_graph( "results": [], "edges": [], "graph_meta": meta, } - # Resolve target - try as-is, then as absolute path, then search + # Resolve target — four-step cascade: + # 1. Exact graph node lookup (file-path QN, e.g. "/repo/Foo.java::Bar.method") + # 2. Absolute path lookup (for file nodes passed as relative paths) + # 3. FTS name search on the raw target string + # 4. Java FQN decomposition: "a.b.Class.method" → try "Class.method", + # then "method" — agents and IDEs naturally write Java FQNs; the graph + # stores file-path QNs, so without this step a valid node is silently + # missed and the response says not_found instead of finding the symbol. node = store.get_node(target) if not node: abs_target = str(root / target) node = store.get_node(abs_target) + + candidates: list = [] if not node: - # Search by name — return ranked disambiguation on multi-match candidates = store.search_nodes(target, limit=10) + + # Step 4: Java FQN fallback — only runs when FTS returned nothing and the + # target looks like a dotted package name (no "::" means it's not already + # a file-path QN; multiple dots means it's not a bare name). + if not node and not candidates and "::" not in target and target.count(".") >= 1: + parts = target.split(".") + # Try progressively shorter suffixes: "Class.method", then "method" + for width in (2, 1): + short = ".".join(parts[-width:]) + candidates = store.search_nodes(short, limit=10) + if candidates: + break + + if not node: if len(candidates) == 1: node = candidates[0] target = node.qualified_name diff --git a/tests/test_agent_transparency.py b/tests/test_agent_transparency.py index 1986632a..9d070054 100644 --- a/tests/test_agent_transparency.py +++ b/tests/test_agent_transparency.py @@ -492,3 +492,147 @@ def test_no_more_candidates_key(self): repo_root=str(self.root), ) assert "candidates" not in result + + +# --------------------------------------------------------------------------- +# Java FQN resolution (package.Class.method → graph file-path QN) +# --------------------------------------------------------------------------- + +class TestJavaFQNResolution: + """query_graph must resolve Java package-qualified names, not just graph QNs. + + Agents and IDEs write targets like ``com.example.service.OrderHandler.process``. + The graph stores nodes as ``/path/to/OrderHandler.java::OrderHandler.process``. + Without FQN decomposition the lookup silently returns not_found even though + the node exists. + """ + + def setup_method(self): + self.tmp = tempfile.mkdtemp() + self.root = Path(self.tmp) / "repo" + self.root.mkdir() + (self.root / ".git").mkdir() + (self.root / ".code-review-graph").mkdir() + self.db = GraphStore(str(self.root / ".code-review-graph" / "graph.db")) + + handler_file = str(self.root / "OrderHandler.java") + caller_file = str(self.root / "OrderRouter.java") + + self.db.upsert_node(NodeInfo( + kind="Class", name="OrderHandler", + file_path=handler_file, + line_start=1, line_end=50, language="java", + )) + self.db.upsert_node(NodeInfo( + kind="Function", name="process", + file_path=handler_file, + line_start=10, line_end=30, language="java", + parent_name="OrderHandler", + )) + self.db.upsert_node(NodeInfo( + kind="Function", name="routes", + file_path=caller_file, + line_start=5, line_end=15, language="java", + )) + # routes() calls process() + target_qn = handler_file + "::OrderHandler.process" + self.db.upsert_edge(EdgeInfo( + kind="CALLS", + source=caller_file + "::routes", + target=target_qn, + file_path=caller_file, line=10, + )) + self.db.commit() + self.db.close() + + def teardown_method(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def test_java_fqn_class_dot_method_resolves(self): + """ClassName.method short form resolves to the graph node.""" + result = query_graph( + pattern="callers_of", + target="OrderHandler.process", + repo_root=str(self.root), + ) + assert result["status"] == "ok", ( + f"Expected ok, got {result['status']}: {result.get('summary')}" + ) + names = [r["name"] for r in result["results"]] + assert "routes" in names + + def test_java_fqn_package_qualified_resolves(self): + """Full package.Class.method form resolves via FQN decomposition.""" + result = query_graph( + pattern="callers_of", + target="com.example.service.OrderHandler.process", + repo_root=str(self.root), + ) + assert result["status"] == "ok", ( + f"Expected ok, got {result['status']}: {result.get('summary')}" + ) + names = [r["name"] for r in result["results"]] + assert "routes" in names + + def test_bare_method_name_with_unique_match_resolves(self): + """Plain method name resolves when only one node with that exact name exists. + + The setup includes an OrderHandler class node whose FTS tokens overlap with + 'process', so searching the bare method name can be ambiguous. A + uniquely-named method with no class name collision resolves cleanly. + """ + db = GraphStore(str(self.root / ".code-review-graph" / "graph.db")) + unique_file = str(self.root / "Fulfillment.java") + db.upsert_node(NodeInfo( + kind="Function", name="fulfillOrder", + file_path=unique_file, + line_start=1, line_end=5, language="java", + )) + caller_file = str(self.root / "FulfillmentRouter.java") + db.upsert_node(NodeInfo( + kind="Function", name="routeFulfillment", + file_path=caller_file, + line_start=1, line_end=5, language="java", + )) + db.upsert_edge(EdgeInfo( + kind="CALLS", + source=caller_file + "::routeFulfillment", + target=unique_file + "::fulfillOrder", + file_path=caller_file, line=3, + )) + db.commit() + db.close() + + result = query_graph( + pattern="callers_of", + target="fulfillOrder", + repo_root=str(self.root), + ) + assert result["status"] == "ok" + names = [r["name"] for r in result["results"]] + assert "routeFulfillment" in names + + def test_ambiguous_fqn_returns_disambiguation(self): + """FQN decomposition returns disambiguation when ClassName.method is in two files.""" + db = GraphStore(str(self.root / ".code-review-graph" / "graph.db")) + # Same class name in a second module — same ClassName.method decomposition + file2 = str(self.root / "v2" / "OrderHandler.java") + Path(file2).parent.mkdir(exist_ok=True) + db.upsert_node(NodeInfo( + kind="Function", name="process", + file_path=file2, + line_start=1, line_end=5, language="java", + parent_name="OrderHandler", + )) + db.commit() + db.close() + + # "OrderHandler.process" (width=2) now matches two nodes in different files + result = query_graph( + pattern="callers_of", + target="com.example.v2.OrderHandler.process", + repo_root=str(self.root), + ) + assert result["status"] == "ambiguous" + assert "disambiguation" in result + assert len(result["disambiguation"]) >= 2