mirror of
https://github.com/Significant-Gravitas/AutoGPT.git
synced 2026-04-08 03:00:28 -04:00
Revert "fix(orchestrator): use descriptive suffixes for duplicate tool names"
This reverts commit 643b8a04b9.
This commit is contained in:
@@ -259,49 +259,12 @@ 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 _derive_descriptive_suffix(defaults: dict[str, Any]) -> str:
|
||||
"""Derive a short, LLM-readable suffix from the first distinctive default value.
|
||||
|
||||
Prefers short string values (e.g. ``match="error"`` -> ``_error``).
|
||||
Also handles booleans and numbers (e.g. ``enabled=True`` -> ``_enabled_true``).
|
||||
Falls back to the first key name if all values are complex.
|
||||
Returns empty string if no defaults are available.
|
||||
"""
|
||||
import re as _re # noqa: PLC0415
|
||||
|
||||
if not defaults:
|
||||
return ""
|
||||
|
||||
# Try to find a short, descriptive string value first
|
||||
for _key, value in defaults.items():
|
||||
if isinstance(value, str) and 1 <= len(value) <= 30:
|
||||
# Sanitize to valid function-name chars: [a-zA-Z0-9_]
|
||||
sanitized = _re.sub(r"[^a-zA-Z0-9]", "_", value).strip("_")
|
||||
sanitized = _re.sub(r"_+", "_", sanitized) # collapse runs
|
||||
if sanitized:
|
||||
return f"_{sanitized[:20]}"
|
||||
|
||||
# Try to build a key_value suffix from the first boolean/numeric default
|
||||
for key, value in defaults.items():
|
||||
if isinstance(value, bool):
|
||||
return f"_{key[:15]}_{str(value).lower()}"
|
||||
if isinstance(value, (int, float)):
|
||||
return f"_{key[:15]}_{value}"
|
||||
|
||||
# Fall back to first key name (e.g. _match, _url, _query)
|
||||
first_key = next(iter(defaults))
|
||||
return f"_{first_key[:20]}"
|
||||
|
||||
|
||||
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 a descriptive suffix derived from the tool's hardcoded defaults
|
||||
(e.g. ``search_error``, ``search_warning``) so the LLM can tell them apart,
|
||||
and enriches descriptions with the full defaults summary.
|
||||
Falls back to numeric suffixes (``_1``, ``_2``) when descriptive names
|
||||
still collide. Mutates the list in place.
|
||||
This appends _1, _2, etc. and enriches descriptions with hardcoded defaults
|
||||
so the LLM can distinguish them. Mutates the list in place.
|
||||
|
||||
Malformed tools (missing ``function`` or ``function.name``) are silently
|
||||
skipped so the caller never crashes on unexpected input.
|
||||
@@ -337,18 +300,8 @@ def _disambiguate_tool_names(tools: list[dict[str, Any]]) -> None:
|
||||
if name not in duplicates:
|
||||
continue
|
||||
|
||||
# First try a descriptive suffix derived from the defaults
|
||||
desc_suffix = _derive_descriptive_suffix(defaults)
|
||||
if desc_suffix:
|
||||
candidate = f"{name[: 64 - len(desc_suffix)]}{desc_suffix}"
|
||||
if candidate not in taken:
|
||||
func["name"] = candidate
|
||||
taken.add(candidate)
|
||||
_enrich_description(func, defaults)
|
||||
continue
|
||||
|
||||
# Fall back to numeric suffix if descriptive name already taken
|
||||
counters[name] = counters.get(name, 0) + 1
|
||||
# Skip suffixes that collide with existing (e.g. user-named) tools
|
||||
while True:
|
||||
suffix = f"_{counters[name]}"
|
||||
candidate = f"{name[: 64 - len(suffix)]}{suffix}"
|
||||
@@ -358,22 +311,17 @@ def _disambiguate_tool_names(tools: list[dict[str, Any]]) -> None:
|
||||
|
||||
func["name"] = candidate
|
||||
taken.add(candidate)
|
||||
_enrich_description(func, defaults)
|
||||
|
||||
|
||||
def _enrich_description(func: dict[str, Any], defaults: dict[str, Any]) -> None:
|
||||
"""Append a ``[Pre-configured: ...]`` summary to the tool description."""
|
||||
if not defaults or not isinstance(defaults, dict):
|
||||
return
|
||||
parts: list[str] = []
|
||||
for k, v in defaults.items():
|
||||
rendered = json.dumps(v)
|
||||
if len(rendered) > 100:
|
||||
rendered = rendered[:80] + "...<truncated>"
|
||||
parts.append(f"{k}={rendered}")
|
||||
summary = ", ".join(parts)
|
||||
original_desc = func.get("description", "") or ""
|
||||
func["description"] = f"{original_desc} [Pre-configured: {summary}]"
|
||||
if defaults and isinstance(defaults, dict):
|
||||
parts: list[str] = []
|
||||
for k, v in defaults.items():
|
||||
rendered = json.dumps(v)
|
||||
if len(rendered) > 100:
|
||||
rendered = rendered[:80] + "...<truncated>"
|
||||
parts.append(f"{k}={rendered}")
|
||||
summary = ", ".join(parts)
|
||||
original_desc = func.get("description", "") or ""
|
||||
func["description"] = f"{original_desc} [Pre-configured: {summary}]"
|
||||
|
||||
|
||||
class OrchestratorBlock(Block):
|
||||
|
||||
@@ -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 get descriptive suffixes derived from default values
|
||||
# Should be suffixed with _1, _2
|
||||
base = OrchestratorBlock.cleanup(block.name)
|
||||
assert f"{base}_foo" in names
|
||||
assert f"{base}_bar" in names
|
||||
assert f"{base}_1" in names
|
||||
assert f"{base}_2" in names
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -96,9 +96,9 @@ async def test_duplicate_tools_include_defaults_in_description():
|
||||
):
|
||||
tools = await OrchestratorBlock._create_tool_node_signatures("orch")
|
||||
|
||||
# Find each tool by descriptive suffix
|
||||
tool_1 = next(t for t in tools if t["function"]["name"].endswith("_error"))
|
||||
tool_2 = next(t for t in tools if t["function"]["name"].endswith("_warning"))
|
||||
# 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"))
|
||||
|
||||
# Descriptions should contain the hardcoded defaults (not the linked 'text' field)
|
||||
assert "[Pre-configured:" in tool_1["function"]["description"]
|
||||
@@ -213,10 +213,9 @@ async def test_three_duplicates_all_get_unique_names():
|
||||
assert len(names) == 3
|
||||
assert len(set(names)) == 3, f"Tool names are not unique: {names}"
|
||||
base = OrchestratorBlock.cleanup(block.name)
|
||||
# Descriptive suffixes from the "match" default values
|
||||
assert f"{base}_error" in names
|
||||
assert f"{base}_warning" in names
|
||||
assert f"{base}_info" in names
|
||||
assert f"{base}_1" in names
|
||||
assert f"{base}_2" in names
|
||||
assert f"{base}_3" in names
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -249,7 +248,7 @@ 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("_error"))
|
||||
tool_1 = next(t for t in tools if t["function"]["name"].endswith("_1"))
|
||||
desc = tool_1["function"]["description"]
|
||||
# 'text' is linked so should NOT appear in Pre-configured
|
||||
assert "text=" not in desc
|
||||
@@ -290,8 +289,8 @@ async def test_mixed_unique_and_duplicate_names():
|
||||
assert len(set(names)) == 3
|
||||
assert "unique_tool" in names
|
||||
base = OrchestratorBlock.cleanup(block_a.name)
|
||||
assert f"{base}_foo" in names
|
||||
assert f"{base}_bar" in names
|
||||
assert f"{base}_1" in names
|
||||
assert f"{base}_2" in names
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -368,18 +367,15 @@ async def test_long_tool_name_truncated():
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_suffix_collision_with_user_named_tool():
|
||||
"""If a user-named tool collides with a descriptive suffix, dedup falls back to numeric."""
|
||||
"""If a user-named tool is 'my_tool_1', dedup of 'my_tool' should skip to _2."""
|
||||
block = MatchTextPatternBlock()
|
||||
base = OrchestratorBlock.cleanup(block.name)
|
||||
|
||||
# Two nodes with same block name (will collide)
|
||||
node_a = _make_mock_node(block, "node_a", input_default={"match": "foo"})
|
||||
node_b = _make_mock_node(block, "node_b", input_default={"match": "bar"})
|
||||
|
||||
# A third node that a user has customized to match one of the descriptive suffixes
|
||||
node_c = _make_mock_node(
|
||||
block, "node_c", metadata={"customized_name": f"{base}_foo"}
|
||||
)
|
||||
# A third node that a user has customized to match the _1 suffix pattern
|
||||
base = OrchestratorBlock.cleanup(block.name)
|
||||
node_c = _make_mock_node(block, "node_c", metadata={"customized_name": f"{base}_1"})
|
||||
|
||||
link_a = _make_mock_link("tools_^_a_~_text", "text", "node_a", "orch")
|
||||
link_b = _make_mock_link("tools_^_b_~_text", "text", "node_b", "orch")
|
||||
@@ -401,13 +397,10 @@ async def test_suffix_collision_with_user_named_tool():
|
||||
names = [t["function"]["name"] for t in tools]
|
||||
assert len(set(names)) == len(names), f"Tool names are not unique: {names}"
|
||||
# The user-named tool keeps its name
|
||||
assert f"{base}_foo" in names
|
||||
# One duplicate gets its descriptive suffix, the other falls back to numeric
|
||||
assert f"{base}_bar" in names
|
||||
# The "foo" descriptive suffix collided with user-named, so fallback to numeric
|
||||
assert any(
|
||||
n.startswith(base) and n not in (f"{base}_foo", f"{base}_bar") for n in names
|
||||
)
|
||||
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
|
||||
|
||||
|
||||
def test_disambiguate_skips_malformed_tools():
|
||||
@@ -487,9 +480,8 @@ def test_disambiguate_handles_missing_description():
|
||||
]
|
||||
_disambiguate_tool_names(tools)
|
||||
|
||||
# Descriptive suffixes from the "key" default values
|
||||
tool_1 = next(t for t in tools if t["function"]["name"] == "my_tool_val1")
|
||||
tool_2 = next(t for t in tools if t["function"]["name"] == "my_tool_val2")
|
||||
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")
|
||||
# Both should have Pre-configured
|
||||
assert "[Pre-configured:" in tool_1["function"].get("description", "")
|
||||
assert "[Pre-configured:" in tool_2["function"].get("description", "")
|
||||
@@ -569,7 +561,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 all get unique descriptive names."""
|
||||
"""Five duplicate blocks should produce _1 through _5, all unique."""
|
||||
block = MatchTextPatternBlock()
|
||||
nodes_and_links = []
|
||||
for i in range(5):
|
||||
@@ -631,11 +623,10 @@ async def test_mixed_duplicates_and_custom_named_same_type():
|
||||
# Custom-named tool keeps its name
|
||||
assert "summarizer" in names
|
||||
base = OrchestratorBlock.cleanup(block.name)
|
||||
# Descriptive suffixes from the "match" default values
|
||||
assert f"{base}_alpha" in names
|
||||
assert f"{base}_beta" in names
|
||||
# "summarizer" should NOT have a suffix
|
||||
assert not any(n.startswith("summarizer_") for n in names)
|
||||
assert f"{base}_1" in names
|
||||
assert f"{base}_2" in names
|
||||
# "summarizer" should NOT have a numeric suffix
|
||||
assert not any(n.startswith("summarizer_") and n[-1].isdigit() for n in names)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -1082,11 +1073,8 @@ def test_disambiguate_tools_with_boolean_and_numeric_defaults():
|
||||
names = [t["function"]["name"] for t in tools]
|
||||
assert len(set(names)) == 2
|
||||
|
||||
# Descriptive suffix from first boolean default: enabled=True -> _enabled_true
|
||||
tool_true = next(
|
||||
t for t in tools if t["function"]["name"] == "processor_enabled_true"
|
||||
)
|
||||
desc = tool_true["function"]["description"]
|
||||
tool_1 = next(t for t in tools if t["function"]["name"] == "processor_1")
|
||||
desc = tool_1["function"]["description"]
|
||||
assert "enabled=true" in desc
|
||||
assert "count=42" in desc
|
||||
assert "ratio=3.14" in desc
|
||||
|
||||
Reference in New Issue
Block a user