Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions chatbot-core/api/services/chat_service.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Chat service layer responsible for processing the requests forwarded by the controller."""

import ast
import inspect
import json
import re
from typing import AsyncGenerator, List, Optional
Expand Down Expand Up @@ -334,6 +335,13 @@ def _execute_search_tools(tool_calls) -> str:
tool_name, params = call.get("tool"), call.get("params")
tool_fn = TOOL_REGISTRY.get(tool_name)

if tool_fn is None:
logger.warning("Unknown tool name '%s'. Skipping.", tool_name)
continue

if "logger" in inspect.signature(tool_fn).parameters:
params = {**params, "logger": logger}

result = tool_fn(**params)
retrieved_results.append({
"tool": tool_name,
Expand Down
77 changes: 77 additions & 0 deletions chatbot-core/tests/unit/services/test_chat_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,3 +233,80 @@ def get_mock_documents(doc_type: str):
}
]
return []



# Tests for _execute_search_tools — Bug fix #241
from api.services.chat_service import _execute_search_tools


def test_execute_search_tools_unknown_tool_name_is_skipped(mocker):
mock_logger = mocker.patch("api.services.chat_service.logger")
mock_registry = {}
mocker.patch("api.services.chat_service.TOOL_REGISTRY", mock_registry)
tool_calls = [{"tool": "nonexistent_tool", "params": {"query": "test"}}]
result = _execute_search_tools(tool_calls)
assert result == ""
mock_logger.warning.assert_called_once_with(
"Unknown tool name '%s'. Skipping.", "nonexistent_tool"
)


def test_execute_search_tools_logger_injected_automatically(mocker):
import inspect
mock_tool = mocker.MagicMock(return_value="tool result")
mocker.patch("api.services.chat_service.TOOL_REGISTRY", {"mock_tool": mock_tool})
mocker.patch(
"api.services.chat_service.inspect.signature",
return_value=inspect.signature(lambda query, logger: None)
)
_execute_search_tools([{"tool": "mock_tool", "params": {"query": "test"}}])
assert "logger" in mock_tool.call_args[1]


def test_execute_search_tools_no_logger_not_injected(mocker):
import inspect
mock_tool = mocker.MagicMock(return_value="result")
mocker.patch("api.services.chat_service.TOOL_REGISTRY", {"mock_tool": mock_tool})
mocker.patch(
"api.services.chat_service.inspect.signature",
return_value=inspect.signature(lambda query: None)
)
_execute_search_tools([{"tool": "mock_tool", "params": {"query": "test"}}])
assert "logger" not in mock_tool.call_args[1]


def test_execute_search_tools_returns_combined_results(mocker):
import inspect
mock_a = mocker.MagicMock(return_value="result A")
mock_b = mocker.MagicMock(return_value="result B")
mocker.patch("api.services.chat_service.TOOL_REGISTRY", {"tool_a": mock_a, "tool_b": mock_b})
mocker.patch(
"api.services.chat_service.inspect.signature",
return_value=inspect.signature(lambda query: None)
)
result = _execute_search_tools([
{"tool": "tool_a", "params": {"query": "test"}},
{"tool": "tool_b", "params": {"query": "test"}},
])
assert "result A" in result
assert "result B" in result


def test_execute_search_tools_mixed_valid_and_invalid(mocker):
import inspect
mock_logger = mocker.patch("api.services.chat_service.logger")
mock_tool = mocker.MagicMock(return_value="valid result")
mocker.patch("api.services.chat_service.TOOL_REGISTRY", {"valid_tool": mock_tool})
mocker.patch(
"api.services.chat_service.inspect.signature",
return_value=inspect.signature(lambda query: None)
)
result = _execute_search_tools([
{"tool": "unknown_tool", "params": {"query": "test"}},
{"tool": "valid_tool", "params": {"query": "test"}},
])
assert "valid result" in result
mock_logger.warning.assert_called_once_with(
"Unknown tool name '%s'. Skipping.", "unknown_tool"
)
Loading