fix(backend): use descriptive suffixes for duplicate tool names

Instead of appending meaningless _1, _2 numeric suffixes, generate
descriptive names from hardcoded defaults (e.g. web_search_for_weather)
so the LLM can distinguish duplicate tools at a glance. Falls back to
numeric suffixes only when no descriptive suffix can be derived.

Addresses reviewer feedback from ntindle on PR #12555.
This commit is contained in:
Zamil Majdy
2026-03-29 11:16:36 +02:00
parent db691aff62
commit 0f7a47ccfa
2 changed files with 96 additions and 42 deletions

View File

@@ -259,12 +259,48 @@ def get_pending_tool_calls(conversation_history: list[Any] | None) -> dict[str,
return {call_id: count for call_id, count in pending_calls.items() if count > 0}
def _slug_from_value(value: Any) -> str:
"""Extract a short alphanumeric slug from a default value.
Used to build descriptive tool-name suffixes so the LLM can tell
duplicate tools apart at a glance (e.g. ``web_search_for_weather``
instead of ``web_search_1``).
"""
text = str(value) if not isinstance(value, str) else value
# Keep only alphanumeric and spaces, collapse whitespace, take first 3 words
text = re.sub(r"[^a-zA-Z0-9 ]", " ", text).strip()
words = text.split()[:3]
slug = "_".join(w.lower() for w in words if w)
return slug[:30] if slug else ""
def _descriptive_suffix(name: str, defaults: dict[str, Any], taken: set[str]) -> str:
"""Try to build a descriptive suffix from hardcoded defaults.
Iterates through defaults looking for a value that produces a usable
slug. Returns the suffix string (e.g. ``_for_weather``) or empty
string if no descriptive suffix could be derived.
"""
for _k, v in defaults.items():
slug = _slug_from_value(v)
if not slug:
continue
suffix = f"_for_{slug}"
candidate = f"{name[: 64 - len(suffix)]}{suffix}"
if candidate not in taken:
return suffix
return ""
def _disambiguate_tool_names(tools: list[dict[str, Any]]) -> None:
"""Ensure all tool names are unique (Anthropic API requires this).
When multiple nodes use the same block type, they get the same tool name.
This appends _1, _2, etc. and enriches descriptions with hardcoded defaults
so the LLM can distinguish them. Mutates the list in place.
This creates descriptive suffixes from hardcoded defaults (e.g.
``web_search_for_weather``) so the LLM understands what each duplicate
does. Falls back to ``_1``, ``_2`` numbering only when no descriptive
suffix can be derived. Also enriches descriptions with the full set of
hardcoded defaults. Mutates the list in place.
Malformed tools (missing ``function`` or ``function.name``) are silently
skipped so the caller never crashes on unexpected input.
@@ -300,14 +336,22 @@ def _disambiguate_tool_names(tools: list[dict[str, Any]]) -> None:
if name not in duplicates:
continue
counters[name] = counters.get(name, 0) + 1
# Skip suffixes that collide with existing (e.g. user-named) tools
while True:
suffix = f"_{counters[name]}"
# Try a descriptive suffix first (e.g. _for_weather)
suffix = ""
if defaults and isinstance(defaults, dict):
suffix = _descriptive_suffix(name, defaults, taken)
if suffix:
candidate = f"{name[: 64 - len(suffix)]}{suffix}"
if candidate not in taken:
break
counters[name] += 1
else:
# Fall back to numbered suffix
counters[name] = counters.get(name, 0) + 1
while True:
suffix = f"_{counters[name]}"
candidate = f"{name[: 64 - len(suffix)]}{suffix}"
if candidate not in taken:
break
counters[name] += 1
func["name"] = candidate
taken.add(candidate)

View File

@@ -64,10 +64,10 @@ async def test_duplicate_block_names_get_suffixed():
names = [t["function"]["name"] for t in tools]
assert len(names) == 2
assert len(set(names)) == 2, f"Tool names are not unique: {names}"
# Should be suffixed with _1, _2
# Should use descriptive suffixes from defaults (match="foo" -> _for_foo)
base = OrchestratorBlock.cleanup(block.name)
assert f"{base}_1" in names
assert f"{base}_2" in names
assert f"{base}_for_foo" in names
assert f"{base}_for_bar" in names
@pytest.mark.asyncio
@@ -96,15 +96,15 @@ async def test_duplicate_tools_include_defaults_in_description():
):
tools = await OrchestratorBlock._create_tool_node_signatures("orch")
# Find each tool by suffix
tool_1 = next(t for t in tools if t["function"]["name"].endswith("_1"))
tool_2 = next(t for t in tools if t["function"]["name"].endswith("_2"))
# Find each tool by descriptive suffix
tool_error = next(t for t in tools if "error" in t["function"]["name"])
tool_warning = next(t for t in tools if "warning" in t["function"]["name"])
# Descriptions should contain the hardcoded defaults (not the linked 'text' field)
assert "[Pre-configured:" in tool_1["function"]["description"]
assert "[Pre-configured:" in tool_2["function"]["description"]
assert '"error"' in tool_1["function"]["description"]
assert '"warning"' in tool_2["function"]["description"]
assert "[Pre-configured:" in tool_error["function"]["description"]
assert "[Pre-configured:" in tool_warning["function"]["description"]
assert '"error"' in tool_error["function"]["description"]
assert '"warning"' in tool_warning["function"]["description"]
@pytest.mark.asyncio
@@ -212,10 +212,11 @@ async def test_three_duplicates_all_get_unique_names():
names = [t["function"]["name"] for t in tools]
assert len(names) == 3
assert len(set(names)) == 3, f"Tool names are not unique: {names}"
# Should use descriptive suffixes from defaults
base = OrchestratorBlock.cleanup(block.name)
assert f"{base}_1" in names
assert f"{base}_2" in names
assert f"{base}_3" in names
assert f"{base}_for_error" in names
assert f"{base}_for_warning" in names
assert f"{base}_for_info" in names
@pytest.mark.asyncio
@@ -248,8 +249,9 @@ async def test_linked_fields_excluded_from_defaults():
):
tools = await OrchestratorBlock._create_tool_node_signatures("orch")
tool_1 = next(t for t in tools if t["function"]["name"].endswith("_1"))
desc = tool_1["function"]["description"]
# Find a tool with a descriptive suffix (from match defaults)
tool_error = next(t for t in tools if "error" in t["function"]["name"])
desc = tool_error["function"]["description"]
# 'text' is linked so should NOT appear in Pre-configured
assert "text=" not in desc
# 'match' is hardcoded so should appear
@@ -288,9 +290,10 @@ async def test_mixed_unique_and_duplicate_names():
names = [t["function"]["name"] for t in tools]
assert len(set(names)) == 3
assert "unique_tool" in names
# Should use descriptive suffixes from defaults
base = OrchestratorBlock.cleanup(block_a.name)
assert f"{base}_1" in names
assert f"{base}_2" in names
assert f"{base}_for_foo" in names
assert f"{base}_for_bar" in names
@pytest.mark.asyncio
@@ -367,7 +370,7 @@ async def test_long_tool_name_truncated():
@pytest.mark.asyncio
async def test_suffix_collision_with_user_named_tool():
"""If a user-named tool is 'my_tool_1', dedup of 'my_tool' should skip to _2."""
"""Duplicate tools with defaults use descriptive suffixes, avoiding collisions."""
block = MatchTextPatternBlock()
# Two nodes with same block name (will collide)
node_a = _make_mock_node(block, "node_a", input_default={"match": "foo"})
@@ -398,9 +401,9 @@ async def test_suffix_collision_with_user_named_tool():
assert len(set(names)) == len(names), f"Tool names are not unique: {names}"
# The user-named tool keeps its name
assert f"{base}_1" in names
# The duplicates should skip _1 (taken) and use _2, _3
assert f"{base}_2" in names
assert f"{base}_3" in names
# The duplicates use descriptive suffixes from defaults
assert f"{base}_for_foo" in names
assert f"{base}_for_bar" in names
def test_disambiguate_skips_malformed_tools():
@@ -418,7 +421,7 @@ def test_disambiguate_skips_malformed_tools():
# Should not raise
_disambiguate_tool_names(tools)
# The two good tools should be disambiguated
# The two good tools should be disambiguated (no defaults -> numeric fallback)
names = [
t.get("function", {}).get("name")
for t in tools
@@ -445,7 +448,7 @@ def test_disambiguate_skips_non_string_tool_names():
# Should not raise
_disambiguate_tool_names(tools)
# The two good tools should be disambiguated
# The two good tools should be disambiguated (no defaults -> numeric fallback)
names = [
t["function"]["name"]
for t in tools
@@ -480,11 +483,12 @@ def test_disambiguate_handles_missing_description():
]
_disambiguate_tool_names(tools)
tool_1 = next(t for t in tools if t["function"]["name"] == "my_tool_1")
tool_2 = next(t for t in tools if t["function"]["name"] == "my_tool_2")
# With defaults key="val1" and key="val2", should get descriptive suffixes
tool_val1 = next(t for t in tools if "val1" in t["function"]["name"])
tool_val2 = next(t for t in tools if "val2" in t["function"]["name"])
# Both should have Pre-configured
assert "[Pre-configured:" in tool_1["function"].get("description", "")
assert "[Pre-configured:" in tool_2["function"].get("description", "")
assert "[Pre-configured:" in tool_val1["function"].get("description", "")
assert "[Pre-configured:" in tool_val2["function"].get("description", "")
# ---------------------------------------------------------------------------
@@ -561,7 +565,7 @@ async def test_very_long_block_names_truncated_with_suffix():
@pytest.mark.asyncio
async def test_five_plus_duplicates_all_unique():
"""Five duplicate blocks should produce _1 through _5, all unique."""
"""Five duplicate blocks should produce unique descriptive names."""
block = MatchTextPatternBlock()
nodes_and_links = []
for i in range(5):
@@ -622,10 +626,11 @@ async def test_mixed_duplicates_and_custom_named_same_type():
assert len(set(names)) == 3, f"Tool names are not unique: {names}"
# Custom-named tool keeps its name
assert "summarizer" in names
# Duplicates use descriptive suffixes from defaults
base = OrchestratorBlock.cleanup(block.name)
assert f"{base}_1" in names
assert f"{base}_2" in names
# "summarizer" should NOT have a numeric suffix
assert f"{base}_for_alpha" in names
assert f"{base}_for_beta" in names
# "summarizer" should NOT have a suffix
assert not any(n.startswith("summarizer_") and n[-1].isdigit() for n in names)
@@ -1014,6 +1019,10 @@ def test_disambiguate_preserves_original_description():
]
_disambiguate_tool_names(tools)
# Should get descriptive suffixes: my_tool_for_fast and my_tool_for_slow
names = [t["function"]["name"] for t in tools]
assert "my_tool_for_fast" in names
assert "my_tool_for_slow" in names
for tool in tools:
desc = tool["function"]["description"]
assert desc.startswith("This is the original description.")
@@ -1073,8 +1082,9 @@ def test_disambiguate_tools_with_boolean_and_numeric_defaults():
names = [t["function"]["name"] for t in tools]
assert len(set(names)) == 2
tool_1 = next(t for t in tools if t["function"]["name"] == "processor_1")
desc = tool_1["function"]["description"]
# Descriptive suffix from first default value (enabled=True -> _for_true)
tool_true = next(t for t in tools if "true" in t["function"]["name"].lower())
desc = tool_true["function"]["description"]
assert "enabled=true" in desc
assert "count=42" in desc
assert "ratio=3.14" in desc