fix(mcp): address PR #701 review comments + green CI

structural.py:
- search_code: relevance floor — drop files with zero raw lexical signal
  (name-exact + path-overlap + raw BM25) so a nonsense query returns [] instead
  of centrality-ranked noise.
- _hybrid_components: deterministic representative-symbol selection via a stable
  sort key (exact query-id match, then src_start, name, src_end) — no longer
  depends on FalkorDB row order; fixes nondeterministic snippet/name.
- find_path: keep the node label (a path is bare nodes with no per-hop relation,
  so the label is the only type signal); restores the edge-leak guard.
- get_file_neighbors: include file_id in both early-return shapes for a
  consistent schema.

tests:
- _find_id resolves a symbol id directly from the graph (search_code is
  file-oriented and no longer returns per-symbol ids) in test_query_tools.py and
  test_impact_analysis.py, with a uniqueness assertion.
- search_code tests use the file-oriented query= API.

53/53 mcp tests pass against live FalkorDB; ruff clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: DvirDukhan

api/mcp/tools/structural.py

@@ -100,6 +100,22 @@ def _minmax(d: dict[str, float]) -> dict[str, float]:
     return {k: (v - lo) / (hi - lo) for k, v in d.items()}
 
 
+def _rep_key(d: dict[str, Any]) -> tuple:
+    """Stable ordering key for a file's representative-symbol candidate.
+
+    Lower is better: exact query-id match first, then lowest ``src_start``,
+    then ``name`` then ``src_end`` as deterministic tie-breakers so the chosen
+    representative never depends on FalkorDB row order (even when ``src_start``
+    ties or is missing).
+    """
+    return (
+        0 if d.get("exact") else 1,
+        d["src_start"] if d.get("src_start") is not None else math.inf,
+        d.get("name") or "",
+        d["src_end"] if d.get("src_end") is not None else math.inf,
+    )
+
+
 def _bm25(query_tokens: set[str], files: list[str],
           tokmap: dict[str, list[str]]) -> dict[str, float]:
     docs = [tokmap.get(f, []) for f in files]
@@ -141,7 +157,11 @@ async def _hybrid_rank(g, query: str, project: Optional[str]) -> list[dict[str,
     symbol per file, used for the snippet). Pure read; no graph mutation.
     """
     files, comps, rep, abs_of, file_id_of = await _hybrid_components(g, query, project)
-    return _hybrid_score(files, comps, rep, abs_of, file_id_of)
+    scored = _hybrid_score(files, comps, rep, abs_of, file_id_of)
+    # Relevance floor: a query with no lexical overlap (e.g. a nonsense token)
+    # would otherwise be ranked purely by query-independent centrality and
+    # return noise. Drop files with no lexical signal so such queries yield [].
+    return [r for r in scored if r.get("lex", 0.0) > 0]
 
 
 async def _hybrid_components(g, query: str, project: Optional[str]):
@@ -194,14 +214,19 @@ def rel(p: Optional[str]) -> str:
             continue
         if name:
             bodytok[rp].extend(_subtokens(name))
-            if name.lower() in qids:
+            is_exact = name.lower() in qids
+            if is_exact:
                 name_exact[rp] += 1.0
-                rep.setdefault(rp, {"name": name, "src_start": start, "src_end": end})
-        if rp not in rep and name:
+            # Representative symbol for the file's snippet: prefer one whose name
+            # exactly matches a query identifier, otherwise the lowest-``src_start``
+            # symbol. Fully deterministic regardless of result-set order via a
+            # stable sort key (exact first, then src_start, then name, then
+            # src_end) so ties / missing ``src_start`` never depend on row order.
+            cand = {"name": name, "src_start": start, "src_end": end,
+                    "exact": is_exact}
             cur = rep.get(rp)
-            if cur is None or (start is not None and (
-                cur.get("src_start") is None or start < cur["src_start"])):
-                rep[rp] = {"name": name, "src_start": start, "src_end": end}
+            if cur is None or _rep_key(cand) < _rep_key(cur):
+                rep[rp] = cand
         if doc and body_used[rp] < _HYBRID_BODY_TOKEN_CAP:
             toks = _tokenize(doc)[: _HYBRID_BODY_TOKEN_CAP - body_used[rp]]
             bodytok[rp].extend(toks)
@@ -221,9 +246,10 @@ def rel(p: Optional[str]) -> str:
         return [], {}, {}, {}, {}
 
     path_overlap = {f: float(len(qtok & set(pathtok.get(f, [])))) for f in files}
+    raw_bm25 = _bm25(qtok, files, bodytok)
     n_name = _minmax(name_exact if name_exact else {f: 0.0 for f in files})
     n_path = _minmax(path_overlap)
-    n_bm25 = _minmax(_bm25(qtok, files, bodytok))
+    n_bm25 = _minmax(raw_bm25)
     n_cent = _minmax({f: centrality.get(f, 0.0) for f in files})
 
     comps: dict[str, dict[str, float]] = {}
@@ -234,6 +260,13 @@ def rel(p: Optional[str]) -> str:
             "bm25": n_bm25.get(f, 0.0),
             "cent": n_cent.get(f, 0.0),
             "pen": _HYBRID_W_PEN if _PENALTY_RE.search(f) else 0.0,
+            # Raw (un-normalized) query-dependent signal. A file with zero
+            # lexical overlap (name/path/body) is not relevant to the query —
+            # only query-independent centrality could rank it — so search_code
+            # drops it rather than returning noise for an unmatched query.
+            "lex": (name_exact.get(f, 0.0)
+                    + path_overlap.get(f, 0.0)
+                    + raw_bm25.get(f, 0.0)),
         }
 
     return files, comps, rep, abs_of, file_id_of
@@ -271,6 +304,7 @@ def _hybrid_score(
             "name": r.get("name"),
             "src_start": r.get("src_start"),
             "src_end": r.get("src_end"),
+            "lex": c.get("lex", 0.0),
         })
     scored.sort(key=lambda d: -d["score"])
     return scored
@@ -575,9 +609,10 @@ def _node_summary(
     ``encode_node`` returns ``{id, labels, properties: {...}}`` because Node
     properties live on a nested attribute. Agents want a flat record. We keep
     the single meaningful ``label`` (File, Class, Function — not the fulltext
-    marker ``Searchable``) only for ``search_code``, where File-vs-Function
-    disambiguation matters; neighbor/path results omit it via ``with_label``
-    since the relation already implies the type.
+    marker ``Searchable``) for ``search_code`` (File-vs-Function disambiguation)
+    and for ``find_path`` (a path is bare nodes with no per-hop relation, so the
+    label is the only type signal). Single-hop neighbor results omit it via
+    ``with_label`` since the relation already implies the node type.
 
     When ``rel_to`` is given (the project/worktree identifier), ``file`` is
     relativized to drop the absolute worktree prefix.
@@ -893,7 +928,7 @@ async def find_path(
     paths: list[dict[str, Any]] = []
     for entry in raw:
         node_seq = [
-            _node_summary(x, rel_to=project, with_label=False)
+            _node_summary(x, rel_to=project, with_label=True)
             for x in entry
             # Discriminate on ``labels``: ``encode_node`` emits a top-level
             # ``labels`` key, while ``encode_edge`` does not (edges carry
@@ -1036,6 +1071,7 @@ async def get_file_neighbors(
         if abs_path is None:
             return {
                 "file": _relativize(str(file), project),
+                "file_id": None,
                 "total_neighbors": 0,
                 "truncated": False,
                 "neighbors": [],
@@ -1051,6 +1087,7 @@ async def get_file_neighbors(
         if not ids:
             return {
                 "file": _relativize(abs_path, project),
+                "file_id": fid,
                 "total_neighbors": 0,
                 "truncated": False,
                 "neighbors": [],

tests/mcp/test_impact_analysis.py

@@ -69,17 +69,25 @@ async def test_impact_analysis_registered_via_app():
 
 
 async def _find_id(indexed_fixture, name: str) -> int:
-    from api.mcp.tools.structural import search_code
+    """Resolve a symbol name to its int node id directly from the graph.
 
-    rows = await search_code(
-        prefix=name,
-        project=indexed_fixture.project,
-        branch=indexed_fixture.branch,
-    )
-    for r in rows:
-        if r["name"] == name:
-            return r["id"]
-    raise AssertionError(f"symbol {name!r} not found")
+    ``search_code`` is file-oriented and no longer returns per-symbol ids.
+    """
+    from api.mcp.tools.structural import _project_arg
+
+    g = _project_arg(indexed_fixture.project, indexed_fixture.branch)
+    try:
+        res = await g._query(
+            "MATCH (n) WHERE (n:Function OR n:Class) AND n.name = $name "
+            "RETURN ID(n)",
+            {"name": name},
+        )
+    finally:
+        await g.close()
+    rows = res.result_set
+    assert rows, f"symbol {name!r} not found in graph"
+    assert len(rows) == 1, f"ambiguous symbol {name!r}: {len(rows)} matches"
+    return rows[0][0]
 
 
 async def test_impact_upstream_of_db(indexed_fixture, expected_contract):

tests/mcp/test_query_tools.py

@@ -28,21 +28,23 @@ def anyio_backend() -> str:
 async def test_search_code_finds_entrypoint(indexed_fixture, expected_contract):
     from api.mcp.tools.structural import search_code
 
+    # search_code is file-oriented: a free-text query naming a symbol must
+    # surface the file that defines it.
+    symbol = expected_contract["search_prefixes"]["ent"]["must_include"][0]
     results = await search_code(
-        prefix="ent",
+        query=symbol,
         project=indexed_fixture.project,
         branch=indexed_fixture.branch,
     )
-    names = {r["name"] for r in results}
-    for required in expected_contract["search_prefixes"]["ent"]["must_include"]:
-        assert required in names, f"expected {required} in {names}"
+    files = [r["file"] for r in results if r.get("file")]
+    assert any(f.endswith(f"{symbol}.py") for f in files), files
 
 
 async def test_search_code_honors_limit(indexed_fixture):
     from api.mcp.tools.structural import search_code
 
     results = await search_code(
-        prefix="r",  # broad prefix
+        query="entrypoint service repo db",  # broad: matches several files
         project=indexed_fixture.project,
         branch=indexed_fixture.branch,
         limit=1,
@@ -54,7 +56,7 @@ async def test_search_code_empty_for_nonsense(indexed_fixture):
     from api.mcp.tools.structural import search_code
 
     results = await search_code(
-        prefix="zzz_no_such_symbol_zzz",
+        query="zzz_no_such_symbol_zzz",
         project=indexed_fixture.project,
         branch=indexed_fixture.branch,
     )
@@ -65,7 +67,7 @@ async def test_search_code_result_serialisable(indexed_fixture):
     from api.mcp.tools.structural import search_code
 
     results = await search_code(
-        prefix="serv",
+        query="service",
         project=indexed_fixture.project,
         branch=indexed_fixture.branch,
     )
@@ -101,7 +103,7 @@ async def test_search_code_returns_relative_paths(indexed_fixture):
     from api.mcp.tools.structural import search_code
 
     results = await search_code(
-        prefix="ent",
+        query="entrypoint",
         project=indexed_fixture.project,
         branch=indexed_fixture.branch,
     )
@@ -112,20 +114,20 @@ async def test_search_code_returns_relative_paths(indexed_fixture):
 
 
 async def test_search_code_ranks_exact_match_within_limit(indexed_fixture, expected_contract):
-    """An exact name==prefix match must survive the ``[:limit]`` cut and rank
-    ahead of the looser prefix matches."""
+    """A query naming a symbol must surface that symbol's file as the top hit,
+    with the matching symbol as the file's representative."""
     from api.mcp.tools.structural import search_code
 
-    # pick a known symbol from the contract and query its exact name
-    exact = next(iter(expected_contract["search_prefixes"]["ent"]["must_include"]))
+    symbol = next(iter(expected_contract["search_prefixes"]["ent"]["must_include"]))
     results = await search_code(
-        prefix=exact,
+        query=symbol,
         project=indexed_fixture.project,
         branch=indexed_fixture.branch,
         limit=1,
     )
-    assert results, f"no results for exact prefix {exact!r}"
-    assert results[0]["name"] == exact
+    assert results, f"no results for query {symbol!r}"
+    assert results[0]["file"].endswith(f"{symbol}.py")
+    assert results[0]["name"] == symbol
 
 
 # ---------------------------------------------------------------------------
@@ -134,18 +136,28 @@ async def test_search_code_ranks_exact_match_within_limit(indexed_fixture, expec
 
 
 async def _find_id(indexed_fixture, name: str) -> int:
-    """Helper: resolve a symbol name to its int node id via search_code."""
-    from api.mcp.tools.structural import search_code
+    """Resolve a symbol name to its int node id directly from the graph.
 
-    rows = await search_code(
-        prefix=name,
-        project=indexed_fixture.project,
-        branch=indexed_fixture.branch,
-    )
-    for r in rows:
-        if r["name"] == name:
-            return r["id"]
-    raise AssertionError(f"symbol {name!r} not found via search_code")
+    ``search_code`` is file-oriented and no longer returns per-symbol ids, so
+    the neighbor/path/impact tests resolve the id straight from FalkorDB. The
+    names used here (entrypoint/service/db) are unique Functions in the fixture,
+    so a uniqueness assertion guards against silently picking the wrong node.
+    """
+    from api.mcp.tools.structural import _project_arg
+
+    g = _project_arg(indexed_fixture.project, indexed_fixture.branch)
+    try:
+        res = await g._query(
+            "MATCH (n) WHERE (n:Function OR n:Class) AND n.name = $name "
+            "RETURN ID(n)",
+            {"name": name},
+        )
+    finally:
+        await g.close()
+    rows = res.result_set
+    assert rows, f"symbol {name!r} not found in graph"
+    assert len(rows) == 1, f"ambiguous symbol {name!r}: {len(rows)} matches"
+    return rows[0][0]
 
 
 async def test_get_callees_of_entrypoint(indexed_fixture, expected_contract):