mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-05-01 23:32:39 +00:00
fix: only pass tools parameter when tools are available
This fixes the CI test failures by only passing the tools parameter to llm.call() and llm.acall() when tools are actually available. This maintains backward compatibility with existing code that checks 'tools' in kwargs to determine if tools were provided. The previous commit always passed tools=None, which caused tests that check 'tools' in kwargs to fail because the key was always present. Co-Authored-By: João <joao@crewai.com>
This commit is contained in:
@@ -310,14 +310,25 @@ def get_llm_response(
|
|||||||
tools = _extract_tools_from_context(executor_context)
|
tools = _extract_tools_from_context(executor_context)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
answer = llm.call(
|
# Only pass tools parameter if tools are available to maintain backward compatibility
|
||||||
messages,
|
# with code that checks "tools" in kwargs
|
||||||
tools=tools,
|
if tools is not None:
|
||||||
callbacks=callbacks,
|
answer = llm.call(
|
||||||
from_task=from_task,
|
messages,
|
||||||
from_agent=from_agent, # type: ignore[arg-type]
|
tools=tools,
|
||||||
response_model=response_model,
|
callbacks=callbacks,
|
||||||
)
|
from_task=from_task,
|
||||||
|
from_agent=from_agent, # type: ignore[arg-type]
|
||||||
|
response_model=response_model,
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
answer = llm.call(
|
||||||
|
messages,
|
||||||
|
callbacks=callbacks,
|
||||||
|
from_task=from_task,
|
||||||
|
from_agent=from_agent, # type: ignore[arg-type]
|
||||||
|
response_model=response_model,
|
||||||
|
)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
raise e
|
raise e
|
||||||
if not answer:
|
if not answer:
|
||||||
@@ -368,14 +379,25 @@ async def aget_llm_response(
|
|||||||
tools = _extract_tools_from_context(executor_context)
|
tools = _extract_tools_from_context(executor_context)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
answer = await llm.acall(
|
# Only pass tools parameter if tools are available to maintain backward compatibility
|
||||||
messages,
|
# with code that checks "tools" in kwargs
|
||||||
tools=tools,
|
if tools is not None:
|
||||||
callbacks=callbacks,
|
answer = await llm.acall(
|
||||||
from_task=from_task,
|
messages,
|
||||||
from_agent=from_agent, # type: ignore[arg-type]
|
tools=tools,
|
||||||
response_model=response_model,
|
callbacks=callbacks,
|
||||||
)
|
from_task=from_task,
|
||||||
|
from_agent=from_agent, # type: ignore[arg-type]
|
||||||
|
response_model=response_model,
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
answer = await llm.acall(
|
||||||
|
messages,
|
||||||
|
callbacks=callbacks,
|
||||||
|
from_task=from_task,
|
||||||
|
from_agent=from_agent, # type: ignore[arg-type]
|
||||||
|
response_model=response_model,
|
||||||
|
)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
raise e
|
raise e
|
||||||
if not answer:
|
if not answer:
|
||||||
|
|||||||
@@ -190,8 +190,8 @@ class TestGetLlmResponse:
|
|||||||
assert len(call_kwargs["tools"]) == 1
|
assert len(call_kwargs["tools"]) == 1
|
||||||
assert call_kwargs["tools"][0]["name"] == "test_tool"
|
assert call_kwargs["tools"][0]["name"] == "test_tool"
|
||||||
|
|
||||||
def test_passes_none_tools_when_no_context(self, mock_llm, mock_printer):
|
def test_does_not_pass_tools_when_no_context(self, mock_llm, mock_printer):
|
||||||
"""Test that tools=None is passed when no executor_context."""
|
"""Test that tools parameter is not passed when no executor_context."""
|
||||||
result = get_llm_response(
|
result = get_llm_response(
|
||||||
llm=mock_llm,
|
llm=mock_llm,
|
||||||
messages=[{"role": "user", "content": "test"}],
|
messages=[{"role": "user", "content": "test"}],
|
||||||
@@ -202,13 +202,14 @@ class TestGetLlmResponse:
|
|||||||
|
|
||||||
mock_llm.call.assert_called_once()
|
mock_llm.call.assert_called_once()
|
||||||
call_kwargs = mock_llm.call.call_args[1]
|
call_kwargs = mock_llm.call.call_args[1]
|
||||||
assert "tools" in call_kwargs
|
# tools should NOT be in kwargs when there are no tools
|
||||||
assert call_kwargs["tools"] is None
|
# This maintains backward compatibility with code that checks "tools" in kwargs
|
||||||
|
assert "tools" not in call_kwargs
|
||||||
|
|
||||||
def test_passes_none_tools_when_context_has_no_tools(
|
def test_does_not_pass_tools_when_context_has_no_tools(
|
||||||
self, mock_llm, mock_printer
|
self, mock_llm, mock_printer
|
||||||
):
|
):
|
||||||
"""Test that tools=None is passed when context has no tools."""
|
"""Test that tools parameter is not passed when context has no tools."""
|
||||||
mock_context = Mock()
|
mock_context = Mock()
|
||||||
mock_context.tools = []
|
mock_context.tools = []
|
||||||
mock_context.messages = [{"role": "user", "content": "test"}]
|
mock_context.messages = [{"role": "user", "content": "test"}]
|
||||||
@@ -233,8 +234,9 @@ class TestGetLlmResponse:
|
|||||||
|
|
||||||
mock_llm.call.assert_called_once()
|
mock_llm.call.assert_called_once()
|
||||||
call_kwargs = mock_llm.call.call_args[1]
|
call_kwargs = mock_llm.call.call_args[1]
|
||||||
assert "tools" in call_kwargs
|
# tools should NOT be in kwargs when there are no tools
|
||||||
assert call_kwargs["tools"] is None
|
# This maintains backward compatibility with code that checks "tools" in kwargs
|
||||||
|
assert "tools" not in call_kwargs
|
||||||
|
|
||||||
|
|
||||||
class TestAgetLlmResponse:
|
class TestAgetLlmResponse:
|
||||||
@@ -293,8 +295,8 @@ class TestAgetLlmResponse:
|
|||||||
assert call_kwargs["tools"][0]["name"] == "async_test_tool"
|
assert call_kwargs["tools"][0]["name"] == "async_test_tool"
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_passes_none_tools_when_no_context(self, mock_llm, mock_printer):
|
async def test_does_not_pass_tools_when_no_context(self, mock_llm, mock_printer):
|
||||||
"""Test that tools=None is passed when no executor_context."""
|
"""Test that tools parameter is not passed when no executor_context."""
|
||||||
result = await aget_llm_response(
|
result = await aget_llm_response(
|
||||||
llm=mock_llm,
|
llm=mock_llm,
|
||||||
messages=[{"role": "user", "content": "test"}],
|
messages=[{"role": "user", "content": "test"}],
|
||||||
@@ -305,8 +307,9 @@ class TestAgetLlmResponse:
|
|||||||
|
|
||||||
mock_llm.acall.assert_called_once()
|
mock_llm.acall.assert_called_once()
|
||||||
call_kwargs = mock_llm.acall.call_args[1]
|
call_kwargs = mock_llm.acall.call_args[1]
|
||||||
assert "tools" in call_kwargs
|
# tools should NOT be in kwargs when there are no tools
|
||||||
assert call_kwargs["tools"] is None
|
# This maintains backward compatibility with code that checks "tools" in kwargs
|
||||||
|
assert "tools" not in call_kwargs
|
||||||
|
|
||||||
|
|
||||||
class TestToolsPassedToGeminiModels:
|
class TestToolsPassedToGeminiModels:
|
||||||
|
|||||||
Reference in New Issue
Block a user