diff --git a/backend/packages/harness/deerflow/tools/builtins/tool_search.py b/backend/packages/harness/deerflow/tools/builtins/tool_search.py index 57b5ad026..ff46dd55f 100644 --- a/backend/packages/harness/deerflow/tools/builtins/tool_search.py +++ b/backend/packages/harness/deerflow/tools/builtins/tool_search.py @@ -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: diff --git a/backend/tests/test_deferred_catalog.py b/backend/tests/test_deferred_catalog.py index 30aaddb59..131f3f492 100644 --- a/backend/tests/test_deferred_catalog.py +++ b/backend/tests/test_deferred_catalog.py @@ -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"] diff --git a/backend/tests/test_deferred_setup.py b/backend/tests/test_deferred_setup.py index fb6099637..462e9c039 100644 --- a/backend/tests/test_deferred_setup.py +++ b/backend/tests/test_deferred_setup.py @@ -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)