fix(sambanova): emit empty delta on usage-only final stream chunk - #22337
fix(sambanova): emit empty delta on usage-only final stream chunk#22337ebarkhordar wants to merge 1 commit into
Conversation
SambaNovaCloud enables stream_options={'include_usage': True} by default,
so the stream ends with a usage-only chunk where choices == []. That chunk
takes the else branch, which never reassigns content_delta, so the final
yield re-emits the previous chunk's token as its delta. This breaks the
streaming invariant that ''.join(deltas) equals the final message.content.
Set content_delta to '' on the usage-only chunk. Adds a regression test
that stubs the streaming request and asserts the concatenated deltas equal
the final content.
dab83c2 to
a17bb50
Compare
|
No rush on this. CI is green and the branch still applies cleanly on main, so it is ready whenever it fits your review queue. Happy to add a regression test for the usage-only final chunk if that would help. |
|
Correcting a line in my note above: I said CI is green, and that was wrong. The only check reporting on this PR is What I do have is local: the added regression test fails on main and passes with the fix. The branch does still apply cleanly on main. |
|
Closing this to keep my open PR list at a size I can actually shepherd. Nothing here is The change still applies if it is useful later, and I am happy to reopen and rebase it |
Description
SambaNovaCloudsetsstream_options={"include_usage": True}by default (base.py:201-204), so a streaming response ends with a usage-only chunk wherechoices == []. Instream_chat, that final chunk takes theelsebranch (base.py:553-561), which buildsadditional_kwargsbut never reassignscontent_delta. The trailingyieldthen re-emits the previous chunk's token as its delta:So
"".join(r.delta for r in client.stream_chat(...))no longer equals the finalmessage.content: any consumer that rebuilds the text from deltas (the usual streaming pattern) gets the last token duplicated.message.contentitself stays correct, only the re-emitteddeltais wrong.The invariant a streaming generator should hold is that the concatenation of the
deltas equals the finalmessage.content, and a usage-only chunk carries no new text, so its delta must be"". This is what the OpenAI-compatible reference implementations do.There is no tracking issue; this was found by reading the code and reproduced as described below.
Fix
Set
content_delta = ""in theelse(empty-choices) branch so the usage-only chunk yields an empty delta. One line plus a comment; no behavior changes for the text chunks or formessage.content.Version Bump?
0.6.0to0.6.1)Type of Change
How Has This Been Tested?
Added
test_stream_chat_usage_chunk_emits_empty_delta. It stubs_handle_streaming_requestto yield a text chunk followed by the usage-only chunk (choices == [], plususage/model/system_fingerprint/created) that the provider sends wheninclude_usage=True, then asserts"".join(deltas) == final content == "Hello"and that the last delta is"".Verified in a clean Docker container (
python:3.11) against currentmain:assert 'HelloHello' == 'Hello'.tests/test_llms_sambanovasystems.pyfile is green (the live-API tests are skipped without a key).ruff check,ruff format --check, andcodespellpass on the two changed files.Not verified: I did not call the real SambaNova Cloud endpoint (no API key). The test reproduces the usage-chunk shape via a stub, using the exact fields the existing
elsebranch already reads.AI assistance: this change was prepared with AI assistance. I reproduced the bug on
main, validated the fix with the Docker differential and repo lint above, and reviewed the diff line by line.