Skip to content

Commit 9aa3867

Browse files
committed
fix: normalize null and missing fields in streaming chat deltas
The API sends null where OpenAI leaves a field out, so an explicit null and a missing field behaved differently on the streaming delta, and tool call fragments stayed raw dicts (#160). Declare role and tool_calls on a chat delta subclass so both parse to None and fragments become typed models, and carry the fragment index on a streaming only subclass so non streaming message dumps are untouched. Related to #160
1 parent cc9f253 commit 9aa3867

2 files changed

Lines changed: 262 additions & 1 deletion

File tree

src/together/types/chat_completions.py

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,12 +185,53 @@ class ChatCompletionResponse(BaseModel):
185185
usage: UsageData | None = None
186186

187187

188+
class ChatCompletionDeltaToolCalls(ToolCalls):
189+
"""One tool call fragment inside a streaming delta.
190+
191+
Streaming splits a single tool call across several chunks, so every
192+
fragment carries an ``index`` naming the call it belongs to. The
193+
non-streaming :class:`ToolCalls` has no such field, so the index is
194+
declared on a streaming-only subclass. Putting it on the shared class
195+
instead would add an ``index`` key to non-streaming
196+
:class:`ChatCompletionMessage` dumps, which is why the subclass exists.
197+
"""
198+
199+
index: int | None = None
200+
201+
202+
class ChatCompletionDeltaContent(DeltaContent):
203+
"""Streaming delta for chat completion chunks.
204+
205+
The API returns an explicit ``null`` for ``choices[n].delta.tool_calls`` on
206+
text-only chunks, and for ``function.name`` / ``function.arguments`` inside
207+
tool-call fragments, where the OpenAI streaming format either omits the
208+
field or sends an empty string, never ``null``
209+
(https://github.com/togethercomputer/together-python/issues/160).
210+
211+
Declaring these as typed optional fields makes ``null`` and *missing* parse
212+
identically (to ``None``) and validates the items into
213+
:class:`ChatCompletionDeltaToolCalls`, matching the non-streaming
214+
:class:`ChatCompletionMessage`, so ``model_dump(exclude_none=True)``
215+
produces OpenAI-shaped deltas with the nulls omitted.
216+
217+
``role`` is declared for the same reason. It is sent on the first chunk of
218+
a response and left out of later ones, so while it was undeclared a present
219+
role and an absent one behaved differently for callers. It is typed as
220+
``str`` rather than :class:`MessageRole` on purpose: chunks are parsed one
221+
at a time inside the streaming generator, so an unrecognised role value
222+
would otherwise raise part way through and end the stream.
223+
"""
224+
225+
role: str | None = None
226+
tool_calls: List[ChatCompletionDeltaToolCalls] | None = None
227+
228+
188229
class ChatCompletionChoicesChunk(BaseModel):
189230
index: int | None = None
190231
logprobs: float | None = None
191232
seed: int | None = None
192233
finish_reason: FinishReason | None = None
193-
delta: DeltaContent | None = None
234+
delta: ChatCompletionDeltaContent | None = None
194235

195236

196237
class ChatCompletionChunk(BaseModel):
Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
1+
"""Regression tests for https://github.com/togethercomputer/together-python/issues/160
2+
3+
The Together API returns an explicit ``null`` where the OpenAI streaming format
4+
either leaves the field out or sends an empty string, in three known places:
5+
6+
1. ``choices[n].delta.tool_calls`` (text-only chunks, left out by OpenAI)
7+
2. ``choices[n].delta.tool_calls[n].function.arguments`` (first tool-call chunk,
8+
where only the name is given; OpenAI sends an empty string here so that
9+
consumers can concatenate every fragment without a special case)
10+
3. ``choices[n].delta.tool_calls[n].function.name`` (continuation chunks that
11+
stream the JSON arguments incrementally, left out by OpenAI)
12+
13+
``choices[n].delta.role`` has the same shape of problem from the other
14+
direction: it is sent on the first chunk and left out of the rest, so while it
15+
was undeclared a present role and an absent one behaved differently.
16+
17+
These tests pin down that the parsed models normalize ``null`` to be
18+
indistinguishable from a missing field, so OpenAI-compatible consumers do not
19+
need Together-specific special cases.
20+
"""
21+
22+
from together.types import ChatCompletionChunk, ChatCompletionResponse
23+
from together.types.chat_completions import FunctionCall, ToolCalls
24+
25+
26+
def _chunk(delta: dict) -> ChatCompletionChunk:
27+
"""Build a chunk the way the SDK does: ChatCompletionChunk(**line.data)."""
28+
return ChatCompletionChunk(
29+
**{
30+
"id": "884581f24f0cfdd0-SJC",
31+
"object": "chat.completion.chunk",
32+
"created": 1725561260,
33+
"model": "meta-llama/Meta-Llama-3.1-8B-Instruct-Turbo",
34+
"choices": [{"index": 0, "delta": delta, "finish_reason": None}],
35+
}
36+
)
37+
38+
39+
# Wire payloads as observed in issue #160
40+
TEXT_ONLY_DELTA_WITH_NULL = {
41+
"role": "assistant",
42+
"content": "Hello",
43+
"tool_calls": None,
44+
}
45+
TEXT_ONLY_DELTA_OMITTED = {"role": "assistant", "content": "Hello"}
46+
FIRST_TOOL_CALL_DELTA = {
47+
"role": "assistant",
48+
"content": None,
49+
"tool_calls": [
50+
{
51+
"index": 0,
52+
"id": "call_f7g2h8i9j0",
53+
"type": "function",
54+
"function": {"name": "get_current_weather", "arguments": None},
55+
}
56+
],
57+
}
58+
CONTINUATION_TOOL_CALL_DELTA = {
59+
"tool_calls": [
60+
{
61+
"index": 0,
62+
"function": {"name": None, "arguments": '{"location": "San Fra'},
63+
}
64+
]
65+
}
66+
67+
68+
def _has_no_none_values(obj: object) -> bool:
69+
if obj is None:
70+
return False
71+
if isinstance(obj, dict):
72+
return all(_has_no_none_values(v) for v in obj.values())
73+
if isinstance(obj, list):
74+
return all(_has_no_none_values(v) for v in obj)
75+
return True
76+
77+
78+
def test_null_tool_calls_parses_like_omitted_tool_calls() -> None:
79+
"""`tool_calls: null` (text-only chunks) must behave exactly like a
80+
missing `tool_calls` field: attribute exists and is None in both cases."""
81+
with_null = _chunk(TEXT_ONLY_DELTA_WITH_NULL).choices[0].delta
82+
omitted = _chunk(TEXT_ONLY_DELTA_OMITTED).choices[0].delta
83+
84+
assert with_null is not None and omitted is not None
85+
assert with_null.tool_calls is None
86+
assert omitted.tool_calls is None # was AttributeError before the fix
87+
assert with_null.content == omitted.content == "Hello"
88+
89+
90+
def test_tool_call_delta_items_are_typed_models() -> None:
91+
"""Streaming tool-call fragments parse into the same ToolCalls/FunctionCall
92+
models used by the non-streaming ChatCompletionMessage."""
93+
delta = _chunk(FIRST_TOOL_CALL_DELTA).choices[0].delta
94+
assert delta is not None and delta.tool_calls is not None
95+
96+
(tool_call,) = delta.tool_calls
97+
assert isinstance(tool_call, ToolCalls)
98+
assert isinstance(tool_call.function, FunctionCall)
99+
assert tool_call.id == "call_f7g2h8i9j0"
100+
assert tool_call.type == "function"
101+
assert tool_call.function.name == "get_current_weather"
102+
# null arguments on the first chunk normalizes to None (absent)
103+
assert tool_call.function.arguments is None
104+
105+
106+
def test_null_function_name_on_continuation_chunks() -> None:
107+
"""`function.name: null` on argument-continuation chunks normalizes to
108+
None while the incremental arguments fragment is preserved verbatim."""
109+
delta = _chunk(CONTINUATION_TOOL_CALL_DELTA).choices[0].delta
110+
assert delta is not None and delta.tool_calls is not None
111+
112+
(tool_call,) = delta.tool_calls
113+
assert tool_call.function is not None
114+
assert tool_call.function.name is None
115+
assert tool_call.function.arguments == '{"location": "San Fra'
116+
117+
118+
def test_exclude_none_dump_produces_openai_shaped_deltas() -> None:
119+
"""model_dump(exclude_none=True) must omit every API-provided null,
120+
including the ones nested inside tool_calls[n].function."""
121+
for wire_delta in (
122+
TEXT_ONLY_DELTA_WITH_NULL,
123+
TEXT_ONLY_DELTA_OMITTED,
124+
FIRST_TOOL_CALL_DELTA,
125+
CONTINUATION_TOOL_CALL_DELTA,
126+
):
127+
delta = _chunk(wire_delta).choices[0].delta
128+
assert delta is not None
129+
dumped = delta.model_dump(exclude_none=True)
130+
assert _has_no_none_values(dumped), f"None survived in {dumped!r}"
131+
132+
text_only = _chunk(TEXT_ONLY_DELTA_WITH_NULL).choices[0].delta
133+
assert text_only is not None
134+
assert "tool_calls" not in text_only.model_dump(exclude_none=True)
135+
136+
137+
def test_role_parses_the_same_whether_sent_or_left_out() -> None:
138+
"""`delta.role` is sent on the first chunk and left out of later ones, so
139+
both must give the same attribute rather than one raising AttributeError."""
140+
first = _chunk(TEXT_ONLY_DELTA_WITH_NULL).choices[0].delta
141+
later = _chunk(CONTINUATION_TOOL_CALL_DELTA).choices[0].delta
142+
143+
assert first is not None and later is not None
144+
assert first.role == "assistant"
145+
assert later.role is None # was AttributeError before the fix
146+
# An unrecognised role must not end the stream, which is why the field is
147+
# typed as str rather than the MessageRole enum.
148+
assert _chunk({"role": "some_future_role"}).choices[0].delta.role == (
149+
"some_future_role"
150+
)
151+
152+
153+
def test_tool_call_index_is_typed_on_the_streaming_model_only() -> None:
154+
"""Streaming splits one tool call across chunks, so `index` says which call
155+
a fragment belongs to.
156+
157+
It is declared on a streaming-only subclass. Declaring it on the shared
158+
ToolCalls instead would add an `index` key to non-streaming message dumps,
159+
so this pins the separation in both directions.
160+
"""
161+
delta = _chunk(FIRST_TOOL_CALL_DELTA).choices[0].delta
162+
assert delta is not None and delta.tool_calls is not None
163+
164+
(tool_call,) = delta.tool_calls
165+
assert isinstance(tool_call, ToolCalls)
166+
assert tool_call.index == 0
167+
assert "index" in type(tool_call).model_fields
168+
assert "index" not in ToolCalls.model_fields
169+
assert delta.model_dump(exclude_none=True)["tool_calls"][0]["index"] == 0
170+
171+
172+
def test_undeclared_wire_fields_are_still_preserved() -> None:
173+
"""Fields the SDK does not declare must keep flowing through, as they did
174+
before the fix (extra="allow"), so new API fields are never dropped."""
175+
delta = (
176+
_chunk({"role": "assistant", "content": "Hi", "future_field": 7})
177+
.choices[0]
178+
.delta
179+
)
180+
assert delta is not None
181+
assert delta.future_field == 7 # type: ignore[attr-defined]
182+
assert delta.model_dump(exclude_none=True)["future_field"] == 7
183+
184+
185+
def test_non_streaming_tool_calls_unchanged() -> None:
186+
"""Non-streaming responses keep parsing tool_calls into typed models."""
187+
response = ChatCompletionResponse(
188+
**{
189+
"id": "884581f24f0cfdd0-SJC",
190+
"object": "chat.completion",
191+
"created": 1725561260,
192+
"model": "meta-llama/Meta-Llama-3.1-8B-Instruct-Turbo",
193+
"choices": [
194+
{
195+
"index": 0,
196+
"finish_reason": "tool_calls",
197+
"message": {
198+
"role": "assistant",
199+
"content": None,
200+
"tool_calls": [
201+
{
202+
"id": "call_f7g2h8i9j0",
203+
"type": "function",
204+
"function": {
205+
"name": "get_current_weather",
206+
"arguments": '{"location": "San Francisco, CA"}',
207+
},
208+
}
209+
],
210+
},
211+
}
212+
],
213+
}
214+
)
215+
assert response.choices is not None
216+
message = response.choices[0].message
217+
assert message is not None and message.tool_calls is not None
218+
assert isinstance(message.tool_calls[0], ToolCalls)
219+
assert message.tool_calls[0].function is not None
220+
assert message.tool_calls[0].function.name == "get_current_weather"

0 commit comments

Comments
 (0)