[Chore] Add E2E test for 'required' tool_choice with streaming #24579
Conversation
Signed-off-by: tlipoca9 <[email protected]> Signed-off-by: tlipoca9 <[email protected]>
![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.
tlipoca9
requested review from
DarkLight1337,
NickLucche,
robertgshaw2-redhat and
simon-mo
as code owners
Signed-off-by: tlipoca9 <[email protected]>
Signed-off-by: tlipoca9 <[email protected]>
Signed-off-by: tlipoca9 <[email protected]>
|
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 <[email protected]>
Signed-off-by: tlipoca9 <[email protected]>
|
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: tlipoca9 <[email protected]>
Signed-off-by: tlipoca9 <[email protected]>
Signed-off-by: tlipoca9 <[email protected]>
2 participants
Purpose
Fixes #24509
Description
The issue was that delta_text was being added twice when tool_choice was set to 'required'.