[Chore] Add E2E test for 'required' tool_choice with streaming#24579
[Chore] Add E2E test for 'required' tool_choice with streaming#24579tlipoca9 wants to merge 12 commits intovllm-project:mainfrom
Conversation
Signed-off-by: tlipoca9 <tlipoca9@gmail.com> Signed-off-by: tlipoca9 <160737620+tlipoca9@users.noreply.github.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a bug where delta_text was being added twice when tool_choice was set to 'required' during streaming chat completions. The change correctly removes a redundant update to previous_texts that was causing this duplication. The fix is sound and also improves code clarity by handling the update of previous_texts in a single, consistent location at the end of the loop.
chaunceyjiang
left a comment
chaunceyjiang
left a comment
There was a problem hiding this comment.
Can you add an e2e test?
tlipoca9
requested review from
DarkLight1337,
NickLucche,
robertgshaw2-redhat and
simon-mo
as code owners
Signed-off-by: tlipoca9 <tlipoca9@gmail.com>
|
There is already a PR fixing this issue. |
| assert serving_completion.chat_template == CHAT_TEMPLATE | ||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_serving_chat_tool_choice_required_streaming(): |
There was a problem hiding this comment.
E2E is also what we need, and we can keep moving forward.
There was a problem hiding this comment.
Thanks, I'll try to get this done.
tlipoca9
changed the title
Signed-off-by: tlipoca9 <tlipoca9@gmail.com>
|
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: tlipoca9 <tlipoca9@gmail.com>
|
This pull request has been automatically marked as stale because it has not had any activity within 90 days. It will be automatically closed if no further activity occurs within 30 days. Leave a comment if you feel this pull request should remain open. Thank you! |
|
This pull request has been automatically closed due to inactivity. Please feel free to reopen if you intend to continue working on it. Thank you! |
2 participants
Purpose
Fixes #24509
Description
The issue was that delta_text was being added twice when tool_choice was set to 'required'.