diff --git a/docs/guides/skills/skill/index.md b/docs/guides/skills/skill/index.md index b1b128b3ff..bb1dda32e1 100644 --- a/docs/guides/skills/skill/index.md +++ b/docs/guides/skills/skill/index.md @@ -190,7 +190,9 @@ model: `list_skills`, `load_skill`, `load_skill_resource`, and configured. The model reaches the three levels by calling those tools in order: 1. `list_skills` returns the name and description of every installed skill, - which is level 1. + which is level 1. Pass `include_list_skills=False` on `SkillToolset` to skip + this turn: the catalog is injected as `` XML in the system + instruction, and the model can call `load_skill` directly. 2. `load_skill` returns the body of the one it picked, which is level 2. 3. `load_skill_resource` or `run_skill_script` reaches a single file or script, which is level 3. @@ -332,6 +334,20 @@ from google.adk.skills import load_skills_from_dir skills = load_skills_from_dir(pathlib.Path(__file__).parent / "skills") ``` +### Inject the L1 catalog instead of calling list_skills + +* **Problem solved**: `list_skills` costs an extra model turn before + `load_skill`. With a small, stable local catalog that tax is not worth it. +* **Implementation**: `SkillToolset(..., include_list_skills=False)` hides + the `list_skills` tool and injects the names and descriptions as + `` XML in the system instruction. The model still calls + `load_skill` for the body. Registry skills, if a registry is configured, are + still discovered only through `search_skills`. + +```python +SkillToolset(skills=[weather_skill], include_list_skills=False) +``` + ### List without loading * **Problem solved**: you want a catalog of names and descriptions without diff --git a/docs/guides/skills/skill_registry/index.md b/docs/guides/skills/skill_registry/index.md index d731f0cf01..03d675c577 100644 --- a/docs/guides/skills/skill_registry/index.md +++ b/docs/guides/skills/skill_registry/index.md @@ -203,7 +203,9 @@ per result, skipping and logging any entry that fails validation. * **A registry cannot list its whole catalog.** The interface has no `list_skills`, and the model's `list_skills` tool reports only the toolset's local skills. A model that never thinks to search never learns the registry - is there at all. + is there at all. `include_list_skills=False` injects those local skills into + the system instruction instead; registry skills are still discovered only + through `search_skills`. * **Skill names must satisfy the frontmatter rules.** A store whose keys are not kebab-case cannot round-trip through `Frontmatter` validation, so map them at the boundary. diff --git a/src/google/adk/tools/skill_toolset.py b/src/google/adk/tools/skill_toolset.py index 4903e0312a..32746cef1b 100644 --- a/src/google/adk/tools/skill_toolset.py +++ b/src/google/adk/tools/skill_toolset.py @@ -1331,6 +1331,7 @@ def __init__( additional_tools: list[ToolUnion] | None = None, tool_name_prefix: str | None = None, tool_filter: ToolPredicate | list[str] | None = None, + include_list_skills: bool = True, ): """Initializes the SkillToolset. @@ -1349,6 +1350,11 @@ def __init__( to be made available to the agent when certain skills are activated. tool_name_prefix: Optional prefix to prepend to tool names. tool_filter: Optional filter to select specific tools. + include_list_skills: Whether to expose the `list_skills` discovery tool. + When True (default), the model lists the L1 catalog through a tool + call. When False, the catalog is injected into the system instruction + as `` XML so the model can call `load_skill` + directly, without a discovery turn. """ super().__init__(tool_filter=tool_filter, tool_name_prefix=tool_name_prefix) @@ -1401,13 +1407,17 @@ def __init__( ft = FunctionTool(tool_union) self._provided_tools_by_name[ft.name] = ft - # Initialize core skill tools + # Initialize core skill tools. Omitting list_skills injects the L1 catalog + # into the system instruction in process_llm_request, so the model can call + # load_skill without a discovery turn. + self._include_list_skills = include_list_skills self._tools = [ - ListSkillsTool(self), LoadSkillTool(self), LoadSkillResourceTool(self), RunSkillScriptTool(self), ] + if include_list_skills: + self._tools.insert(0, ListSkillsTool(self)) if self._registry: self._tools.append(SearchSkillsTool(self)) @@ -1586,6 +1596,7 @@ def clone_with_updated_skills( additional_tools=additional_tools, tool_name_prefix=self.tool_name_prefix, tool_filter=self.tool_filter, + include_list_skills=self._include_list_skills, ) async def process_llm_request( diff --git a/tests/unittests/tools/test_skill_toolset.py b/tests/unittests/tools/test_skill_toolset.py index d88471e16e..6f38bb9e6d 100644 --- a/tests/unittests/tools/test_skill_toolset.py +++ b/tests/unittests/tools/test_skill_toolset.py @@ -250,6 +250,25 @@ async def test_clone_with_updated_skills_keeps_filter_and_prefix( assert clone_names == original_names == ["list_skills"] +@pytest.mark.asyncio +async def test_clone_with_updated_skills_keeps_include_list_skills( + mock_skill1, mock_skill2, tool_context_instance +): + """The clone keeps include_list_skills=False, so list_skills stays hidden.""" + toolset = skill_toolset.SkillToolset([mock_skill1], include_list_skills=False) + + new_toolset = toolset.clone_with_updated_skills([mock_skill2]) + + original_names = [ + t.name for t in await toolset.get_tools(tool_context_instance) + ] + clone_names = [ + t.name for t in await new_toolset.get_tools(tool_context_instance) + ] + assert "list_skills" not in original_names + assert clone_names == original_names + + def test_init_accepts_environment(mock_skill1): """SkillToolset stores the provided environment.""" mock_env = mock.create_autospec(BaseEnvironment, instance=True) @@ -3115,6 +3134,68 @@ async def test_process_llm_request_injects_skills_xml_when_list_skills_filtered( assert "skill2" in instructions[1] +@pytest.mark.asyncio +async def test_get_tools_omits_list_skills_when_disabled(mock_skill1): + """include_list_skills=False hides list_skills and keeps the other tools.""" + toolset = skill_toolset.SkillToolset([mock_skill1], include_list_skills=False) + + tool_names = [t.name for t in await toolset.get_tools()] + + assert "list_skills" not in tool_names + assert "load_skill" in tool_names + assert "load_skill_resource" in tool_names + assert "run_skill_script" in tool_names + + +@pytest.mark.asyncio +async def test_process_llm_request_injects_skills_xml_when_list_skills_disabled( + mock_skill1, mock_skill2, tool_context_instance +): + """include_list_skills=False injects the L1 catalog into the system prompt.""" + toolset = skill_toolset.SkillToolset( + [mock_skill1, mock_skill2], include_list_skills=False + ) + llm_req = mock.create_autospec(llm_request_model.LlmRequest, instance=True) + + await toolset.process_llm_request( + tool_context=tool_context_instance, llm_request=llm_req + ) + + args, _ = llm_req.append_instructions.call_args + instructions = args[0] + assert len(instructions) == 2 + assert "NOT available: `list_skills`" in instructions[0] + assert "" in instructions[1] + assert "skill1" in instructions[1] + assert "skill2" in instructions[1] + + +@pytest.mark.asyncio +async def test_include_list_skills_false_keeps_search_skills( + mock_skill1, mock_registry, tool_context_instance +): + """Disabling list_skills still exposes search_skills when a registry is set.""" + toolset = skill_toolset.SkillToolset( + [mock_skill1], + registry=mock_registry, + include_list_skills=False, + ) + + tool_names = [t.name for t in await toolset.get_tools(tool_context_instance)] + assert "list_skills" not in tool_names + assert "search_skills" in tool_names + assert "load_skill" in tool_names + + llm_req = mock.create_autospec(llm_request_model.LlmRequest, instance=True) + await toolset.process_llm_request( + tool_context=tool_context_instance, llm_request=llm_req + ) + args, _ = llm_req.append_instructions.call_args + instructions = args[0] + assert "" in instructions[1] + assert "search_skills" in instructions[2] + + @pytest.mark.asyncio async def test_process_llm_request_omits_search_skills_hint_when_filtered( mock_registry, tool_context_instance