From 930187e1924768b32bf134a954d32a8465d3e276 Mon Sep 17 00:00:00 2001 From: sakenuGOD Date: Mon, 20 Apr 2026 19:07:51 +0300 Subject: [PATCH] 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). --- .../mcp_server/semantic_tool_filter.py | 17 ++++++++--- .../mcp_server/test_semantic_tool_filter.py | 28 +++++++++++++++++-- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/semantic_tool_filter.py b/litellm/proxy/_experimental/mcp_server/semantic_tool_filter.py index 4412599dc0c..7530e5e2905 100644 --- a/litellm/proxy/_experimental/mcp_server/semantic_tool_filter.py +++ b/litellm/proxy/_experimental/mcp_server/semantic_tool_filter.py @@ -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)) diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_semantic_tool_filter.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_semantic_tool_filter.py index 15917ff8a0f..97969ef9d8f 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_semantic_tool_filter.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_semantic_tool_filter.py @@ -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()