mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-27 00:17:53 +00:00
fix(tools): stop capping tool_search's select: at MAX_RESULTS (#4054)
`select:` names its targets explicitly, so capping it silently drops schemas the
model asked for by name -- and picks the survivors by catalog order, not request
order. The model is told the tool it wanted was not returned by nothing at all;
it then tries to call a tool that is still deferred.
The rule is already stated three times in the repo, and this is the one place
that breaks it:
- backend/AGENTS.md:447 -- "select: returns all requested skills without a
result cap; other modes cap at MAX_RESULTS=5"
- skills/catalog.py:71 -- SkillCatalog.search returns select: uncapped; the
ranked modes slice. DeferredToolCatalog shares its query grammar and its
MAX_RESULTS = 5, and is capped.
- tool_search's own docstring -- "select:Read,Edit -- fetch these exact tools
by name" versus "notebook jupyter -- keyword search, up to max_results best
matches". Only the ranked form promises a cap.
The cap is applied twice: once inside `DeferredToolCatalog.search` and again in
the tool closure. `search` already caps the ranked branches internally, so the
closure's slice is redundant for them and is the only thing capping `select:`
once the first is removed -- fixing one site alone changes nothing the model can
observe. Its sibling closure, `skills/describe.py::describe_skill`, calls
`catalog.search(name)` with no slice.
Both slices are removed. The ranked modes keep their cap.
This commit is contained in:
parent
9101fb6a67
commit
1ebf59fe24
@ -81,8 +81,12 @@ class DeferredToolCatalog:
|
||||
return []
|
||||
|
||||
if query.startswith("select:"):
|
||||
# No cap: ``select:`` names the tools explicitly, so returning a
|
||||
# subset silently drops schemas the model asked for by name. Mirrors
|
||||
# ``SkillCatalog.search`` (``skills/catalog.py``); the ranked modes
|
||||
# below stay capped at ``MAX_RESULTS``.
|
||||
wanted = {n.strip() for n in query[7:].split(",")}
|
||||
return [t for t in self.tools if t.name in wanted][:MAX_RESULTS]
|
||||
return [t for t in self.tools if t.name in wanted]
|
||||
|
||||
if query.startswith("+"):
|
||||
parts = query[1:].split(None, 1)
|
||||
@ -150,7 +154,7 @@ def build_tool_search_tool(catalog: DeferredToolCatalog) -> BaseTool:
|
||||
- "notebook jupyter" -- keyword search, up to max_results best matches
|
||||
- "+slack send" -- require "slack" in the name, rank by remaining terms
|
||||
"""
|
||||
matched = catalog.search(query)[:MAX_RESULTS]
|
||||
matched = catalog.search(query)
|
||||
if not matched:
|
||||
content, names = f"No tools found matching: {query}", []
|
||||
else:
|
||||
|
||||
@ -1,7 +1,7 @@
|
||||
import pytest
|
||||
from langchain_core.tools import tool as as_tool
|
||||
|
||||
from deerflow.tools.builtins.tool_search import DeferredToolCatalog
|
||||
from deerflow.tools.builtins.tool_search import MAX_RESULTS, DeferredToolCatalog
|
||||
|
||||
|
||||
@as_tool
|
||||
@ -30,6 +30,48 @@ def test_search_select(catalog):
|
||||
assert [t.name for t in got] == ["alpha_search"]
|
||||
|
||||
|
||||
def _make_tool(name: str):
|
||||
@as_tool(name)
|
||||
def _t(query: str) -> str:
|
||||
"A searchable deferred tool."
|
||||
return query
|
||||
|
||||
return _t
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def wide_catalog() -> DeferredToolCatalog:
|
||||
"""More tools than ``MAX_RESULTS`` so the cap boundary is reachable."""
|
||||
return DeferredToolCatalog(tuple(_make_tool(f"tool_{c}") for c in "abcdefgh"))
|
||||
|
||||
|
||||
def test_search_select_returns_all_requested(wide_catalog):
|
||||
"""``select:`` returns every named tool without capping.
|
||||
|
||||
Mirrors ``test_skill_catalog.py::test_select_returns_all_requested``. The
|
||||
two catalogs share the same query grammar and the same ``MAX_RESULTS = 5``;
|
||||
``select:`` names its targets explicitly, so capping it silently drops
|
||||
schemas the model asked for by name -- and picks the survivors by catalog
|
||||
order, not request order.
|
||||
"""
|
||||
wanted = [f"tool_{c}" for c in "abcdef"] # 6 > MAX_RESULTS
|
||||
got = [t.name for t in wide_catalog.search("select:" + ",".join(wanted))]
|
||||
|
||||
assert got == wanted
|
||||
|
||||
|
||||
@pytest.mark.parametrize("query", ["+tool_", "searchable"])
|
||||
def test_search_ranked_modes_stay_capped(wide_catalog, query):
|
||||
"""Only ``select:`` is uncapped; the ranked modes keep their ``MAX_RESULTS`` cap.
|
||||
|
||||
Guards the fix from being widened into the branches whose docstring does
|
||||
promise "up to max_results best matches".
|
||||
"""
|
||||
got = wide_catalog.search(query)
|
||||
|
||||
assert len(got) == MAX_RESULTS
|
||||
|
||||
|
||||
def test_search_plus_keyword(catalog):
|
||||
got = catalog.search("+beta translate")
|
||||
assert [t.name for t in got] == ["beta_translate"]
|
||||
|
||||
@ -55,6 +55,33 @@ def test_tool_search_returns_command_with_hash_scoped_promotion():
|
||||
assert "mcp_calc" in msg.content
|
||||
|
||||
|
||||
def test_tool_search_promotes_every_selected_tool():
|
||||
"""``select:`` promotes all named tools -- the tool closure must not re-cap.
|
||||
|
||||
``DeferredToolCatalog.search`` already caps the ranked modes internally, so
|
||||
a second ``[:MAX_RESULTS]`` in the closure only truncates ``select:``. Its
|
||||
sibling closure, ``skills/describe.py::describe_skill``, calls
|
||||
``catalog.search(name)`` with no slice. Without this test, dropping the cap
|
||||
inside ``search`` alone would still leave ``select:`` capped here.
|
||||
"""
|
||||
|
||||
def _t(name: str):
|
||||
@as_tool(name)
|
||||
def _f(query: str) -> str:
|
||||
"A deferred tool."
|
||||
return query
|
||||
|
||||
return _f
|
||||
|
||||
names = [f"mcp_t{i}" for i in range(6)] # 6 > MAX_RESULTS
|
||||
catalog = DeferredToolCatalog(tuple(_t(n) for n in names))
|
||||
ts = build_tool_search_tool(catalog)
|
||||
|
||||
out = ts.invoke({"type": "tool_call", "name": "tool_search", "args": {"query": "select:" + ",".join(names)}, "id": "tc3"})
|
||||
|
||||
assert out.update["promoted"]["names"] == names
|
||||
|
||||
|
||||
def test_tool_search_no_match_empty_names():
|
||||
catalog = DeferredToolCatalog((mcp_calc,))
|
||||
ts = build_tool_search_tool(catalog)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user