forked from phoenix-oss/llama-stack-mirror
fix: multiple tool calls in remote-vllm chat_completion (#2161)
# What does this PR do? This fixes an issue in how we used the tool_call_buf from streaming tool calls in the remote-vllm provider where it would end up concatenating parameters from multiple different tool call results instead of aggregating the results from each tool call separately. It also fixes an issue found while digging into that where we were accidentally mixing the json string form of tool call parameters with the string representation of the python form, which mean we'd end up with single quotes in what should be double-quoted json strings. Closes #1120 ## Test Plan The following tests are now passing 100% for the remote-vllm provider, where some of the test_text_inference were failing before this change: ``` VLLM_URL="http://localhost:8000/v1" INFERENCE_MODEL="RedHatAI/Llama-4-Scout-17B-16E-Instruct-FP8-dynamic" LLAMA_STACK_CONFIG=remote-vllm python -m pytest -v tests/integration/inference/test_text_inference.py --text-model "RedHatAI/Llama-4-Scout-17B-16E-Instruct-FP8-dynamic" VLLM_URL="http://localhost:8000/v1" INFERENCE_MODEL="RedHatAI/Llama-4-Scout-17B-16E-Instruct-FP8-dynamic" LLAMA_STACK_CONFIG=remote-vllm python -m pytest -v tests/integration/inference/test_vision_inference.py --vision-model "RedHatAI/Llama-4-Scout-17B-16E-Instruct-FP8-dynamic" ``` All but one of the agent tests are passing (including the multi-tool one). See the PR at https://github.com/vllm-project/vllm/pull/17917 and a gist at https://gist.github.com/bbrowning/4734240ce96b4264340caa9584e47c9e for changes needed there, which will have to get made upstream in vLLM. Agent tests: ``` VLLM_URL="http://localhost:8000/v1" INFERENCE_MODEL="RedHatAI/Llama-4-Scout-17B-16E-Instruct-FP8-dynamic" LLAMA_STACK_CONFIG=remote-vllm python -m pytest -v tests/integration/agents/test_agents.py --text-model "RedHatAI/Llama-4-Scout-17B-16E-Instruct-FP8-dynamic" ```` --------- Signed-off-by: Ben Browning <bbrownin@redhat.com>
This commit is contained in:
parent
bb5fca9521
commit
10b1056dea
4 changed files with 225 additions and 34 deletions
|
@ -266,6 +266,7 @@ def test_builtin_tool_web_search(llama_stack_client, agent_config):
|
|||
assert found_tool_execution
|
||||
|
||||
|
||||
@pytest.mark.skip(reason="Code interpreter is currently disabled in the Stack")
|
||||
def test_builtin_tool_code_execution(llama_stack_client, agent_config):
|
||||
agent_config = {
|
||||
**agent_config,
|
||||
|
@ -346,7 +347,7 @@ def test_custom_tool(llama_stack_client, agent_config):
|
|||
messages=[
|
||||
{
|
||||
"role": "user",
|
||||
"content": "What is the boiling point of polyjuice?",
|
||||
"content": "What is the boiling point of the liquid polyjuice in celsius?",
|
||||
},
|
||||
],
|
||||
session_id=session_id,
|
||||
|
@ -420,7 +421,7 @@ def run_agent_with_tool_choice(client, agent_config, tool_choice):
|
|||
messages=[
|
||||
{
|
||||
"role": "user",
|
||||
"content": "What is the boiling point of polyjuice?",
|
||||
"content": "What is the boiling point of the liquid polyjuice in celsius?",
|
||||
},
|
||||
],
|
||||
session_id=session_id,
|
||||
|
@ -674,8 +675,8 @@ def test_create_turn_response(llama_stack_client, agent_config, client_tools):
|
|||
|
||||
|
||||
def test_multi_tool_calls(llama_stack_client, agent_config):
|
||||
if "gpt" not in agent_config["model"]:
|
||||
pytest.xfail("Only tested on GPT models")
|
||||
if "gpt" not in agent_config["model"] and "llama-4" not in agent_config["model"].lower():
|
||||
pytest.xfail("Only tested on GPT and Llama 4 models")
|
||||
|
||||
agent_config = {
|
||||
**agent_config,
|
||||
|
@ -689,23 +690,34 @@ def test_multi_tool_calls(llama_stack_client, agent_config):
|
|||
messages=[
|
||||
{
|
||||
"role": "user",
|
||||
"content": "Call get_boiling_point twice to answer: What is the boiling point of polyjuice in both celsius and fahrenheit?",
|
||||
"content": "Call get_boiling_point twice to answer: What is the boiling point of polyjuice in both celsius and fahrenheit?.\nUse the tool responses to answer the question.",
|
||||
},
|
||||
],
|
||||
session_id=session_id,
|
||||
stream=False,
|
||||
)
|
||||
steps = response.steps
|
||||
assert len(steps) == 7
|
||||
assert steps[0].step_type == "shield_call"
|
||||
assert steps[1].step_type == "inference"
|
||||
assert steps[2].step_type == "shield_call"
|
||||
assert steps[3].step_type == "tool_execution"
|
||||
assert steps[4].step_type == "shield_call"
|
||||
assert steps[5].step_type == "inference"
|
||||
assert steps[6].step_type == "shield_call"
|
||||
|
||||
tool_execution_step = steps[3]
|
||||
has_input_shield = agent_config.get("input_shields")
|
||||
has_output_shield = agent_config.get("output_shields")
|
||||
assert len(steps) == 3 + (2 if has_input_shield else 0) + (2 if has_output_shield else 0)
|
||||
if has_input_shield:
|
||||
assert steps[0].step_type == "shield_call"
|
||||
steps.pop(0)
|
||||
assert steps[0].step_type == "inference"
|
||||
if has_output_shield:
|
||||
assert steps[1].step_type == "shield_call"
|
||||
steps.pop(1)
|
||||
assert steps[1].step_type == "tool_execution"
|
||||
tool_execution_step = steps[1]
|
||||
if has_input_shield:
|
||||
assert steps[2].step_type == "shield_call"
|
||||
steps.pop(2)
|
||||
assert steps[2].step_type == "inference"
|
||||
if has_output_shield:
|
||||
assert steps[3].step_type == "shield_call"
|
||||
steps.pop(3)
|
||||
|
||||
assert len(tool_execution_step.tool_calls) == 2
|
||||
assert tool_execution_step.tool_calls[0].tool_name.startswith("get_boiling_point")
|
||||
assert tool_execution_step.tool_calls[1].tool_name.startswith("get_boiling_point")
|
||||
|
|
|
@ -24,6 +24,12 @@ from openai.types.chat.chat_completion_chunk import (
|
|||
from openai.types.chat.chat_completion_chunk import (
|
||||
ChoiceDelta as OpenAIChoiceDelta,
|
||||
)
|
||||
from openai.types.chat.chat_completion_chunk import (
|
||||
ChoiceDeltaToolCall as OpenAIChoiceDeltaToolCall,
|
||||
)
|
||||
from openai.types.chat.chat_completion_chunk import (
|
||||
ChoiceDeltaToolCallFunction as OpenAIChoiceDeltaToolCallFunction,
|
||||
)
|
||||
from openai.types.model import Model as OpenAIModel
|
||||
|
||||
from llama_stack.apis.inference import (
|
||||
|
@ -206,8 +212,164 @@ async def test_tool_call_delta_empty_tool_call_buf():
|
|||
yield chunk
|
||||
|
||||
chunks = [chunk async for chunk in _process_vllm_chat_completion_stream_response(mock_stream())]
|
||||
assert len(chunks) == 1
|
||||
assert chunks[0].event.stop_reason == StopReason.end_of_turn
|
||||
assert len(chunks) == 2
|
||||
assert chunks[0].event.event_type.value == "start"
|
||||
assert chunks[1].event.event_type.value == "complete"
|
||||
assert chunks[1].event.stop_reason == StopReason.end_of_turn
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_tool_call_delta_streaming_arguments_dict():
|
||||
async def mock_stream():
|
||||
mock_chunk_1 = OpenAIChatCompletionChunk(
|
||||
id="chunk-1",
|
||||
created=1,
|
||||
model="foo",
|
||||
object="chat.completion.chunk",
|
||||
choices=[
|
||||
OpenAIChoice(
|
||||
delta=OpenAIChoiceDelta(
|
||||
content="",
|
||||
tool_calls=[
|
||||
OpenAIChoiceDeltaToolCall(
|
||||
id="tc_1",
|
||||
index=1,
|
||||
function=OpenAIChoiceDeltaToolCallFunction(
|
||||
name="power",
|
||||
arguments="",
|
||||
),
|
||||
)
|
||||
],
|
||||
),
|
||||
finish_reason=None,
|
||||
index=0,
|
||||
)
|
||||
],
|
||||
)
|
||||
mock_chunk_2 = OpenAIChatCompletionChunk(
|
||||
id="chunk-2",
|
||||
created=1,
|
||||
model="foo",
|
||||
object="chat.completion.chunk",
|
||||
choices=[
|
||||
OpenAIChoice(
|
||||
delta=OpenAIChoiceDelta(
|
||||
content="",
|
||||
tool_calls=[
|
||||
OpenAIChoiceDeltaToolCall(
|
||||
id="tc_1",
|
||||
index=1,
|
||||
function=OpenAIChoiceDeltaToolCallFunction(
|
||||
name="power",
|
||||
arguments='{"number": 28, "power": 3}',
|
||||
),
|
||||
)
|
||||
],
|
||||
),
|
||||
finish_reason=None,
|
||||
index=0,
|
||||
)
|
||||
],
|
||||
)
|
||||
mock_chunk_3 = OpenAIChatCompletionChunk(
|
||||
id="chunk-3",
|
||||
created=1,
|
||||
model="foo",
|
||||
object="chat.completion.chunk",
|
||||
choices=[
|
||||
OpenAIChoice(delta=OpenAIChoiceDelta(content="", tool_calls=None), finish_reason="tool_calls", index=0)
|
||||
],
|
||||
)
|
||||
for chunk in [mock_chunk_1, mock_chunk_2, mock_chunk_3]:
|
||||
yield chunk
|
||||
|
||||
chunks = [chunk async for chunk in _process_vllm_chat_completion_stream_response(mock_stream())]
|
||||
assert len(chunks) == 3
|
||||
assert chunks[0].event.event_type.value == "start"
|
||||
assert chunks[1].event.event_type.value == "progress"
|
||||
assert chunks[1].event.delta.type == "tool_call"
|
||||
assert chunks[1].event.delta.parse_status.value == "succeeded"
|
||||
assert chunks[1].event.delta.tool_call.arguments_json == '{"number": 28, "power": 3}'
|
||||
assert chunks[2].event.event_type.value == "complete"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_multiple_tool_calls():
|
||||
async def mock_stream():
|
||||
mock_chunk_1 = OpenAIChatCompletionChunk(
|
||||
id="chunk-1",
|
||||
created=1,
|
||||
model="foo",
|
||||
object="chat.completion.chunk",
|
||||
choices=[
|
||||
OpenAIChoice(
|
||||
delta=OpenAIChoiceDelta(
|
||||
content="",
|
||||
tool_calls=[
|
||||
OpenAIChoiceDeltaToolCall(
|
||||
id="",
|
||||
index=1,
|
||||
function=OpenAIChoiceDeltaToolCallFunction(
|
||||
name="power",
|
||||
arguments='{"number": 28, "power": 3}',
|
||||
),
|
||||
),
|
||||
],
|
||||
),
|
||||
finish_reason=None,
|
||||
index=0,
|
||||
)
|
||||
],
|
||||
)
|
||||
mock_chunk_2 = OpenAIChatCompletionChunk(
|
||||
id="chunk-2",
|
||||
created=1,
|
||||
model="foo",
|
||||
object="chat.completion.chunk",
|
||||
choices=[
|
||||
OpenAIChoice(
|
||||
delta=OpenAIChoiceDelta(
|
||||
content="",
|
||||
tool_calls=[
|
||||
OpenAIChoiceDeltaToolCall(
|
||||
id="",
|
||||
index=2,
|
||||
function=OpenAIChoiceDeltaToolCallFunction(
|
||||
name="multiple",
|
||||
arguments='{"first_number": 4, "second_number": 7}',
|
||||
),
|
||||
),
|
||||
],
|
||||
),
|
||||
finish_reason=None,
|
||||
index=0,
|
||||
)
|
||||
],
|
||||
)
|
||||
mock_chunk_3 = OpenAIChatCompletionChunk(
|
||||
id="chunk-3",
|
||||
created=1,
|
||||
model="foo",
|
||||
object="chat.completion.chunk",
|
||||
choices=[
|
||||
OpenAIChoice(delta=OpenAIChoiceDelta(content="", tool_calls=None), finish_reason="tool_calls", index=0)
|
||||
],
|
||||
)
|
||||
for chunk in [mock_chunk_1, mock_chunk_2, mock_chunk_3]:
|
||||
yield chunk
|
||||
|
||||
chunks = [chunk async for chunk in _process_vllm_chat_completion_stream_response(mock_stream())]
|
||||
assert len(chunks) == 4
|
||||
assert chunks[0].event.event_type.value == "start"
|
||||
assert chunks[1].event.event_type.value == "progress"
|
||||
assert chunks[1].event.delta.type == "tool_call"
|
||||
assert chunks[1].event.delta.parse_status.value == "succeeded"
|
||||
assert chunks[1].event.delta.tool_call.arguments_json == '{"number": 28, "power": 3}'
|
||||
assert chunks[2].event.event_type.value == "progress"
|
||||
assert chunks[2].event.delta.type == "tool_call"
|
||||
assert chunks[2].event.delta.parse_status.value == "succeeded"
|
||||
assert chunks[2].event.delta.tool_call.arguments_json == '{"first_number": 4, "second_number": 7}'
|
||||
assert chunks[3].event.event_type.value == "complete"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
@ -231,7 +393,8 @@ async def test_process_vllm_chat_completion_stream_response_no_choices():
|
|||
yield chunk
|
||||
|
||||
chunks = [chunk async for chunk in _process_vllm_chat_completion_stream_response(mock_stream())]
|
||||
assert len(chunks) == 0
|
||||
assert len(chunks) == 1
|
||||
assert chunks[0].event.event_type.value == "start"
|
||||
|
||||
|
||||
def test_chat_completion_doesnt_block_event_loop(caplog):
|
||||
|
@ -369,7 +532,7 @@ async def test_process_vllm_chat_completion_stream_response_tool_call_args_last_
|
|||
yield chunk
|
||||
|
||||
chunks = [chunk async for chunk in _process_vllm_chat_completion_stream_response(mock_stream())]
|
||||
assert len(chunks) == 2
|
||||
assert len(chunks) == 3
|
||||
assert chunks[-1].event.event_type == ChatCompletionResponseEventType.complete
|
||||
assert chunks[-2].event.delta.type == "tool_call"
|
||||
assert chunks[-2].event.delta.tool_call.tool_name == mock_tool_name
|
||||
|
@ -422,7 +585,7 @@ async def test_process_vllm_chat_completion_stream_response_no_finish_reason():
|
|||
yield chunk
|
||||
|
||||
chunks = [chunk async for chunk in _process_vllm_chat_completion_stream_response(mock_stream())]
|
||||
assert len(chunks) == 2
|
||||
assert len(chunks) == 3
|
||||
assert chunks[-1].event.event_type == ChatCompletionResponseEventType.complete
|
||||
assert chunks[-2].event.delta.type == "tool_call"
|
||||
assert chunks[-2].event.delta.tool_call.tool_name == mock_tool_name
|
||||
|
@ -471,7 +634,7 @@ async def test_process_vllm_chat_completion_stream_response_tool_without_args():
|
|||
yield chunk
|
||||
|
||||
chunks = [chunk async for chunk in _process_vllm_chat_completion_stream_response(mock_stream())]
|
||||
assert len(chunks) == 2
|
||||
assert len(chunks) == 3
|
||||
assert chunks[-1].event.event_type == ChatCompletionResponseEventType.complete
|
||||
assert chunks[-2].event.delta.type == "tool_call"
|
||||
assert chunks[-2].event.delta.tool_call.tool_name == mock_tool_name
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue