Stop streaming if LiteLLM provide a finish_reason #1350
Conversation
…nue to send empty messages)
|
Forgot to make style, sorry ! |
src/smolagents/models.py
Outdated
| if event.choices[0].delta is None: | ||
| if not getattr(event.choices[0], "finish_reason", None): | ||
| if event.choices[0].delta is None or event.choices[0].delta.content is None: | ||
| if getattr(event.choices[0], "finish_reason", None): |
There was a problem hiding this comment.
I remember having raised my concerns about a related issue before: #1236 (comment)
I'm wondering if event.choices[0].delta can be None or it is always a class instance.
There was a problem hiding this comment.
Do you know if LiteLLM can return None event.choices[0].delta when streaming?
There was a problem hiding this comment.
I kept if event.choices[0].delta is None in case LiteLLM returns no Delta instance but not sure it is possible
There was a problem hiding this comment.
LiteLLM always return a Delta object because of this block. I will remove the check event.choices[0].delta is None
src/smolagents/models.py
Outdated
| if event.choices[0].delta is None or event.choices[0].delta.content is None: | ||
| if getattr(event.choices[0], "finish_reason", None): | ||
| break # Stop streaming if a finish reason is provided | ||
| else: | ||
| raise ValueError(f"No content or tool calls in event: {event}") | ||
| else: | ||
| yield ChatMessageStreamDelta( |
There was a problem hiding this comment.
And what about simplifying the logic even further?
| if event.choices[0].delta is None or event.choices[0].delta.content is None: | |
| if getattr(event.choices[0], "finish_reason", None): | |
| break # Stop streaming if a finish reason is provided | |
| else: | |
| raise ValueError(f"No content or tool calls in event: {event}") | |
| else: | |
| yield ChatMessageStreamDelta( | |
| if event.choices[0].delta and event.choices[0].delta.content: | |
| yield ChatMessageStreamDelta( |
There was a problem hiding this comment.
Nice one, added that to last commit along with removing the check of event.choices[0].delta
Issue #1349.
Sometimes LiteLLM send a last message with finish_reason when generation is done.
example :
The code in generate_stream only check is the key delta is None. But if finish_reason is provided, there might be a Delta object provided under the key delta, but with a content to None. In that case we should not append the None to the generation and we should just stop generation.