review: strengthen the suffix-fallback tie-breaker and the

deduplication regression test (Greptile comments on #26117)

- test_same_tool_not_returned_twice now passes two distinct canonicals
  ("read_file" and "file") that both suffix-match the same incoming
  tool, rather than the same canonical twice, so the assertion
  actually exercises the used_ids dedup path instead of the
  duplicate-input-list path.

- The suffix fallback in _get_tools_by_names now prefers the shortest
  incoming name that still qualifies under the separator-anchored
  match. In the one-prefix-per-client opencode scenario this is a
  no-op, but in multi-namespace configurations the shortest qualifying
  name is the least-wrapped one and is the most defensible deterministic
  choice, replacing the dict-insertion-order fallback.

- Adds test_suffix_fallback_prefers_shortest_candidate covering the
  new tie-breaker directly.

Still 15 tests passing locally (was 14).
This commit is contained in:
sakenuGOD 2026-04-20 19:07:51 +03:00
parent 981bc3614a
commit 930187e192
2 changed files with 38 additions and 7 deletions

View file

@ -268,10 +268,19 @@ class SemanticMCPToolFilter:
for canonical in tool_names:
tool = available_by_name.get(canonical)
if tool is None:
for client_name, candidate in available_by_name.items():
if self._name_matches_canonical(client_name, canonical):
tool = candidate
break
# Prefer the shortest qualifying name. When several
# incoming tools suffix-match the same canonical (e.g.
# "my_search" and "my_tag_search" both end in "search"),
# the one closest in length to the canonical is the
# least-wrapped and most likely the intended target.
best_name: Optional[str] = None
for client_name in available_by_name:
if not self._name_matches_canonical(client_name, canonical):
continue
if best_name is None or len(client_name) < len(best_name):
best_name = client_name
if best_name is not None:
tool = available_by_name[best_name]
if tool is not None and id(tool) not in used_ids:
matched.append(tool)
used_ids.add(id(tool))

View file

@ -500,8 +500,10 @@ class TestGetToolsByNames:
def test_same_tool_not_returned_twice(self):
"""
Two canonicals that both suffix-match the same incoming tool
must not produce a duplicate in the output list.
Two distinct canonicals that both suffix-match the same incoming
tool must not produce a duplicate in the output list.
``read_file`` and ``file`` are both valid separator-anchored
suffixes of ``srv_read_file``.
"""
filter_instance = self._make_filter()
available_tools = [
@ -509,11 +511,31 @@ class TestGetToolsByNames:
]
matched = filter_instance._get_tools_by_names(
["read_file", "read_file"], available_tools
["read_file", "file"], available_tools
)
assert len(matched) == 1
def test_suffix_fallback_prefers_shortest_candidate(self):
"""
When no exact match exists and several incoming tools
suffix-match the same canonical, the one closest in length to
the canonical (i.e. the least-wrapped) should be chosen.
"""
filter_instance = self._make_filter()
canonical = "search"
available_tools = [
{"name": "my_tag_search", "description": "tag search"},
{"name": "my_search", "description": "plain search"},
]
matched = filter_instance._get_tools_by_names(
[canonical], available_tools
)
assert len(matched) == 1
assert matched[0]["name"] == "my_search"
def test_ordering_follows_router_output(self):
"""Returned tools follow the order the semantic router chose."""
filter_instance = self._make_filter()