Skip to content

Commit f33b811

Browse files
committed
fix(lib): only raise finish-reason errors when there is parseable input
`parse_chat_completion` raised `LengthFinishReasonError` / `ContentFilterFinishReasonError` for any choice whose `finish_reason` was `length` / `content_filter`, regardless of whether structured-output parsing was requested. This is inconsistent with the streaming accumulator (`ChatCompletionStreamState._accumulate_chunk`), which only raises these when `has_parseable_input` is true. As a result a plain `client.chat.completions.stream(...)` (no `response_format` and no parseable tools) that stops at the token limit would iterate to completion without raising, but then raise `LengthFinishReasonError` from `get_final_completion()` — even though `client.chat.completions.create()` never raises for the same response and the length/content-filter restriction is documented only for `.parse()` (structured outputs). Guard both raises with `has_parseable_input`, mirroring the accumulator, so the errors are only raised when there is actually something to parse.
1 parent 0c09a3f commit f33b811

2 files changed

Lines changed: 60 additions & 2 deletions

File tree

src/openai/lib/_parsing/_completions.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,20 @@ def parse_chat_completion(
9494
else:
9595
input_tools = []
9696

97+
# `length` / `content_filter` finish reasons only prevent us from producing a valid
98+
# parsed result when there is actually something to parse. For a plain completion
99+
# (no `response_format` and no parseable tools) there is nothing to parse, so we
100+
# mirror the streaming accumulator (`ChatCompletionStreamState`), which guards these
101+
# errors with `has_parseable_input`, and leave the completion untouched — matching
102+
# `chat.completions.create()`.
103+
raise_on_incomplete = has_parseable_input(response_format=response_format, input_tools=input_tools)
104+
97105
choices: list[ParsedChoice[ResponseFormatT]] = []
98106
for choice in chat_completion.choices:
99-
if choice.finish_reason == "length":
107+
if raise_on_incomplete and choice.finish_reason == "length":
100108
raise LengthFinishReasonError(completion=chat_completion)
101109

102-
if choice.finish_reason == "content_filter":
110+
if raise_on_incomplete and choice.finish_reason == "content_filter":
103111
raise ContentFilterFinishReasonError()
104112

105113
message = choice.message

tests/lib/chat/test_completions_streaming.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
ParsedChatCompletionSnapshot,
3131
)
3232
from openai.lib._parsing._completions import ResponseFormatT
33+
from openai.types.chat.chat_completion_chunk import Choice as ChunkChoice, ChoiceDelta
3334

3435
from ..utils import print_obj
3536
from ...conftest import base_url
@@ -1069,6 +1070,55 @@ def streamer(client: OpenAI) -> Iterator[ChatCompletionChunk]:
10691070
)
10701071

10711072

1073+
def _chunk(delta: ChoiceDelta, finish_reason: str | None) -> ChatCompletionChunk:
1074+
return ChatCompletionChunk.construct(
1075+
id="chatcmpl-test",
1076+
object="chat.completion.chunk",
1077+
created=0,
1078+
model="gpt-4o-2024-08-06",
1079+
choices=[ChunkChoice.construct(index=0, delta=delta, finish_reason=finish_reason)],
1080+
)
1081+
1082+
1083+
def _content_chunk(text: str) -> ChatCompletionChunk:
1084+
return _chunk(ChoiceDelta.construct(role="assistant", content=text), finish_reason=None)
1085+
1086+
1087+
def _finish_chunk(finish_reason: str) -> ChatCompletionChunk:
1088+
return _chunk(ChoiceDelta.construct(), finish_reason=finish_reason)
1089+
1090+
1091+
@pytest.mark.parametrize("finish_reason", ["length", "content_filter"])
1092+
def test_non_parse_stream_terminal_finish_reason_does_not_raise(finish_reason: str) -> None:
1093+
# A plain stream (no `response_format` and no parseable tools) has nothing to parse,
1094+
# so a `length` / `content_filter` finish reason must not raise from
1095+
# `get_final_completion()` — matching `chat.completions.create()` and the
1096+
# streaming accumulator, which already suppresses these for non-parse streams.
1097+
state: ChatCompletionStreamState[None] = ChatCompletionStreamState()
1098+
1099+
# accumulating the chunks must not raise
1100+
state.handle_chunk(_content_chunk("partial answer that got cut o"))
1101+
state.handle_chunk(_finish_chunk(finish_reason))
1102+
1103+
completion = state.get_final_completion()
1104+
assert completion.choices[0].finish_reason == finish_reason
1105+
assert completion.choices[0].message.content == "partial answer that got cut o"
1106+
assert completion.choices[0].message.parsed is None
1107+
1108+
1109+
def test_parse_stream_length_finish_still_raises() -> None:
1110+
# When a `response_format` is given there *is* something to parse, so the terminal
1111+
# `length` finish reason must still raise (unchanged behavior).
1112+
class Location(BaseModel):
1113+
city: str
1114+
1115+
state: ChatCompletionStreamState[Location] = ChatCompletionStreamState(response_format=Location)
1116+
state.handle_chunk(_content_chunk('{"city":"San Francisc'))
1117+
1118+
with pytest.raises(openai.LengthFinishReasonError):
1119+
state.handle_chunk(_finish_chunk("length"))
1120+
1121+
10721122
@pytest.mark.parametrize("sync", [True, False], ids=["sync", "async"])
10731123
def test_stream_method_in_sync(sync: bool, client: OpenAI, async_client: AsyncOpenAI) -> None:
10741124
checking_client: OpenAI | AsyncOpenAI = client if sync else async_client

0 commit comments

Comments
 (0)