diff --git a/src/serena/agent.py b/src/serena/agent.py index 80e8090..86e93a9 100644 --- a/src/serena/agent.py +++ b/src/serena/agent.py @@ -1370,8 +1370,18 @@ class InitialInstructionsTool(Tool): return self.agent.prompt_factory.create_system_prompt() -def iter_tool_classes() -> Generator[type[Tool], None, None]: - return iter_subclasses(Tool) +def iter_tool_classes(same_module_only: bool = True) -> Generator[type[Tool], None, None]: + """ + Iterate over all Tool subclasses. + + Args: + same_module_only: If True, only iterate over tools defined in the same module as the Tool class. + If False, iterate over all Tool subclasses. + """ + for tool_class in iter_subclasses(Tool): + if same_module_only and tool_class.__module__ != Tool.__module__: + continue + yield tool_class def print_tool_overview() -> None: diff --git a/src/serena/mcp.py b/src/serena/mcp.py index ef8b04c..f52e529 100644 --- a/src/serena/mcp.py +++ b/src/serena/mcp.py @@ -61,9 +61,14 @@ def make_tool( # Mount the tool description as a combination of the docstring description and # the return value description, if it exists. - func_doc = f"{docstring.description.strip().strip('.')}." + if docstring.description: + func_doc = f"{docstring.description.strip().strip('.')}." + else: + func_doc = "" if (docstring.returns) and (docstring_returns := docstring.returns.description): - func_doc = f"{func_doc} Returns {docstring_returns.strip().strip('.')}." + # Only add a space before "Returns" if func_doc is not empty + prefix = " " if func_doc else "" + func_doc = f"{func_doc}{prefix}Returns {docstring_returns.strip().strip('.')}." # Parse the parameter descriptions from the docstring and add pass its description # to the parameters schema. diff --git a/test/serena/test_mcp.py b/test/serena/test_mcp.py index 6e6a8ae..bac38da 100644 --- a/test/serena/test_mcp.py +++ b/test/serena/test_mcp.py @@ -198,3 +198,112 @@ def test_make_tool_missing_apply() -> None: with pytest.raises(AttributeError): make_tool(tool) + + +@pytest.mark.parametrize( + "docstring, expected_description", + [ + ( + """This is a test function. + + :param param: The parameter + :return: A result + """, + "This is a test function. Returns A result.", + ), + ( + """ + :param param: The parameter + :return: A result + """, + "Returns A result.", + ), + ( + """ + :param param: The parameter + """, + "", + ), + ("", ""), + ], +) +def test_make_tool_descriptions(docstring, expected_description) -> None: + """Test make_tool with various docstring formats.""" + + class TestTool(BaseMockTool): + def apply(self, param: str) -> str: + return f"Result: {param}" + + def apply_ex(self, *args, **kwargs) -> str: + return self.apply(**kwargs) + + # Dynamically set the docstring + TestTool.apply.__doc__ = docstring + + tool = TestTool() + mcp_tool = make_tool(tool) + + assert mcp_tool.name == "test" + assert mcp_tool.description == expected_description + + +def is_test_mock_class(tool_class: type) -> bool: + """Check if a class is a test mock class.""" + # Check if the class is defined in a test module + module_name = tool_class.__module__ + return ( + module_name.startswith(("test.", "tests.")) + or "test_" in module_name + or tool_class.__name__ + in [ + "BaseMockTool", + "BasicTool", + "BadTool", + "NoParamsTool", + "NoReturnTool", + "MissingParamTool", + "ComplexDocTool", + "FormatTool", + "NoDescriptionTool", + ] + ) + + +def get_real_tool_classes(): + """Get all non-test, non-abstract tool classes.""" + from serena.agent import iter_tool_classes + + for tool_class in iter_tool_classes(same_module_only=False): + # Skip abstract base classes that can't be instantiated + if tool_class.__name__ == "Tool" or getattr(tool_class, "__abstractmethods__", set()): + continue + + # Skip test mock classes + if is_test_mock_class(tool_class): + continue + + yield tool_class + + +@pytest.mark.parametrize("tool_class", list(get_real_tool_classes())) +def test_make_tool_all_tools(tool_class) -> None: + """Test that make_tool works for all tools in the codebase.""" + + # Create a mock agent for tool initialization + class MockAgent: + def __init__(self): + self.project_config = None + self.serena_config = None + + # Create an instance of the tool + tool_instance = tool_class(MockAgent()) + + # Try to create an MCP tool from it + mcp_tool = make_tool(tool_instance) + + # Basic validation + assert isinstance(mcp_tool, MCPTool) + assert mcp_tool.name == tool_class.get_name() + + # The description should be a string (either from docstring or default) + assert isinstance(mcp_tool.description, str)