Skip to content

Conversation

@bbrowning
Copy link
Collaborator

What does this PR do?

This gracefully handles the case where the vLLM server responded to a completion request with no choices, which can happen in certain vLLM error situations. Previously, we'd error out with a stack trace about a list index out of range. Now, we just log a warning to the user and move past any chunks with an empty choices list.

A specific example of the type of stack trace this fixes:

  File "/app/llama-stack-source/llama_stack/providers/remote/inference/vllm/vllm.py", line 170, in _process_vllm_chat_completion_stream_response
    choice = chunk.choices[0]
             ~~~~~~~~~~~~~^^^
IndexError: list index out of range

Now, instead of erroring out with that stack trace, we log a warning that vLLM failed to generate any completions and alert the user to check the vLLM server logs for details.

This is related to #1277 and addresses the stack trace shown in that issue, although does not in and of itself change the functional behavior of vLLM tool calling.

Test Plan

As part of this fix, I added new unit tests to trigger this same error and verify it no longer happens. That is
test_process_vllm_chat_completion_stream_response_no_choices in the new tests/unit/providers/inference/test_remote_vllm.py. I also added a couple of more tests to trigger and verify the last couple of remote vllm provider bug fixes - specifically a test for #1236 (builtin tool calling) and #1325 (vLLM <= v0.6.3).

This required fixing the signature of
_process_vllm_chat_completion_stream_response to accept the actual type of chunks it was getting passed - specifically changing from our openai_compat OpenAICompatCompletionResponse to openai.types.chat.chat_completion_chunk.ChatCompletionChunk. It was not actually getting passed OpenAICompatCompletionResponse objects before, and was using attributes that didn't exist on those objects. So, the signature now matches the type of object it's actually passed.

Run these new unit tests like this:

pytest tests/unit/providers/inference/test_remote_vllm.py

Additionally, I ensured the existing test_text_inference.py tests passed via:

VLLM_URL="http://localhost:8000/v1" \
INFERENCE_MODEL="meta-llama/Llama-3.2-3B-Instruct" \
LLAMA_STACK_CONFIG=remote-vllm \
python -m pytest -v tests/integration/inference/test_text_inference.py \
--inference-model "meta-llama/Llama-3.2-3B-Instruct" \
--vision-inference-model ""

This gracefully handles the case where the vLLM server responded to a
completion request with no choices, which can happen in certain vLLM
error situations. Previously, we'd error out with a stack trace about
a list index out of range. Now, we just log a warning to the user and
move past any chunks with an empty choices list.

A specific example of the type of stack trace this fixes:

```
  File "/app/llama-stack-source/llama_stack/providers/remote/inference/vllm/vllm.py", line 170, in _process_vllm_chat_completion_stream_response
    choice = chunk.choices[0]
             ~~~~~~~~~~~~~^^^
IndexError: list index out of range
```

Now, instead of erroring out with that stack trace, we log a warning
that vLLM failed to generate any completions and alert the user to
check the vLLM server logs for details.

This is related to llamastack#1277 and addresses the stack trace shown in that
issue, although does not in and of itself change the functional
behavior of vLLM tool calling.

As part of this fix, I added new unit tests to trigger this same error
and verify it no longer happens. That is
`test_process_vllm_chat_completion_stream_response_no_choices` in the
new `tests/unit/providers/inference/test_remote_vllm.py`. I also added
a couple of more tests to trigger and verify the last couple of remote
vllm provider bug fixes - specifically a test for llamastack#1236 (builtin tool
calling) and llamastack#1325 (vLLM <= v0.6.3).

This required fixing the signature of
`_process_vllm_chat_completion_stream_response` to accept the actual
type of chunks it was getting passed - specifically changing from
our openai_compat `OpenAICompatCompletionResponse` to
`openai.types.chat.chat_completion_chunk.ChatCompletionChunk`. It was
not actually getting passed `OpenAICompatCompletionResponse` objects
before, and was using attributes that didn't exist on those
objects. So, the signature now matches the type of object it's
actually passed.

Run these new unit tests like this:

```
pytest tests/unit/providers/inference/test_remote_vllm.py
```

Additionally, I ensured the existing `test_text_inference.py` tests
passed via:

```
VLLM_URL="http://localhost:8000/v1" INFERENCE_MODEL="meta-llama/Llama-3.2-3B-Instruct" LLAMA_STACK_CONFIG=remote-vllm python -m pytest -v tests/integration/inference/test_text_inference.py --inference-model "meta-llama/Llama-3.2-3B-Instruct" --vision-inference-model ""
```

Signed-off-by: Ben Browning <bbrownin@redhat.com>
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Mar 5, 2025
Copy link
Collaborator

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@terrytangyuan terrytangyuan merged commit 9c4074e into llamastack:main Mar 5, 2025
3 checks passed
@bbrowning bbrowning deleted the remote-vllm-no-choices branch March 5, 2025 20:16
tool_call_buf = UnparseableToolCall()
async for chunk in stream:
if not chunk.choices:
log.warning("vLLM failed to generation any completions - check the vLLM server logs for an error.")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/generation/generate/?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants