fix: compare bound method by equality, not identity
Bound methods are created fresh on each attribute access, so 'self._missing_function_call_handler is self._missing_function_call_handler' is always False. Using 'is' meant the placeholder branch never fired and both warnings logged when a function was missing at queue time. Switch to == so equality compares the underlying function and instance. Strengthen the missing-at-queue-time test to assert the second warning does not fire.
This commit is contained in:
@@ -792,7 +792,7 @@ class LLMService(UserTurnCompletionLLMServiceMixin, AIService):
|
|||||||
item = self._functions[runner_item.function_name]
|
item = self._functions[runner_item.function_name]
|
||||||
elif None in self._functions.keys():
|
elif None in self._functions.keys():
|
||||||
item = self._functions[None]
|
item = self._functions[None]
|
||||||
elif runner_item.registry_item.handler is self._missing_function_call_handler:
|
elif runner_item.registry_item.handler == self._missing_function_call_handler:
|
||||||
item = runner_item.registry_item
|
item = runner_item.registry_item
|
||||||
else:
|
else:
|
||||||
logger.warning(
|
logger.warning(
|
||||||
|
|||||||
@@ -5,7 +5,7 @@
|
|||||||
#
|
#
|
||||||
|
|
||||||
import unittest
|
import unittest
|
||||||
from unittest.mock import AsyncMock
|
from unittest.mock import AsyncMock, patch
|
||||||
|
|
||||||
from pipecat.frames.frames import (
|
from pipecat.frames.frames import (
|
||||||
FunctionCallFromLLM,
|
FunctionCallFromLLM,
|
||||||
@@ -60,16 +60,17 @@ class TestLLMService(unittest.IsolatedAsyncioTestCase):
|
|||||||
|
|
||||||
service.broadcast_frame = mock_broadcast_frame
|
service.broadcast_frame = mock_broadcast_frame
|
||||||
|
|
||||||
await service.run_function_calls(
|
with patch("pipecat.services.llm_service.logger") as mock_logger:
|
||||||
[
|
await service.run_function_calls(
|
||||||
FunctionCallFromLLM(
|
[
|
||||||
function_name="missing_tool",
|
FunctionCallFromLLM(
|
||||||
tool_call_id="call_1",
|
function_name="missing_tool",
|
||||||
arguments={"query": "weather"},
|
tool_call_id="call_1",
|
||||||
context=LLMContext(),
|
arguments={"query": "weather"},
|
||||||
)
|
context=LLMContext(),
|
||||||
]
|
)
|
||||||
)
|
]
|
||||||
|
)
|
||||||
|
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
[type(frame) for frame in recorded_frames],
|
[type(frame) for frame in recorded_frames],
|
||||||
@@ -85,6 +86,12 @@ class TestLLMService(unittest.IsolatedAsyncioTestCase):
|
|||||||
"Error: function 'missing_tool' is not registered.",
|
"Error: function 'missing_tool' is not registered.",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Only the queue-time warning should fire; the execution-time
|
||||||
|
# "just unregistered" warning must not double-log.
|
||||||
|
warnings = [c.args[0] for c in mock_logger.warning.call_args_list]
|
||||||
|
self.assertTrue(any("not registered" in w for w in warnings))
|
||||||
|
self.assertFalse(any("just unregistered" in w for w in warnings))
|
||||||
|
|
||||||
async def test_function_unregistered_between_queue_and_execute(self):
|
async def test_function_unregistered_between_queue_and_execute(self):
|
||||||
"""Function unregistered between queuing and execution still terminates."""
|
"""Function unregistered between queuing and execution still terminates."""
|
||||||
service = MockLLMService()
|
service = MockLLMService()
|
||||||
|
|||||||
Reference in New Issue
Block a user