Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/openai/lib/_parsing/_completions.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,20 @@ def parse_chat_completion(
else:
input_tools = []

# `length` / `content_filter` finish reasons only prevent us from producing a valid
# parsed result when there is actually something to parse. For a plain completion
# (no `response_format` and no parseable tools) there is nothing to parse, so we
# mirror the streaming accumulator (`ChatCompletionStreamState`), which guards these
# errors with `has_parseable_input`, and leave the completion untouched — matching
# `chat.completions.create()`.
raise_on_incomplete = has_parseable_input(response_format=response_format, input_tools=input_tools)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve finish errors for one-shot tool iterables

When tools is a generator (which the public Iterable annotation permits), both sync and async chat.completions.parse() first consume it in _validate_input_tools (completions.py:179 and completions.py:1790) and then pass the exhausted iterator here. Consequently input_tools becomes empty and raise_on_incomplete is false, so a request that supplied a strict parseable tool but finishes with length or content_filter now silently returns an incomplete completion instead of raising. Materialize the tools before validation or otherwise preserve their parseability state.

Useful? React with 👍 / 👎.


choices: list[ParsedChoice[ResponseFormatT]] = []
for choice in chat_completion.choices:
if choice.finish_reason == "length":
if raise_on_incomplete and choice.finish_reason == "length":
raise LengthFinishReasonError(completion=chat_completion)

if choice.finish_reason == "content_filter":
if raise_on_incomplete and choice.finish_reason == "content_filter":
raise ContentFilterFinishReasonError()

message = choice.message
Expand Down
50 changes: 50 additions & 0 deletions tests/lib/chat/test_completions_streaming.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
ParsedChatCompletionSnapshot,
)
from openai.lib._parsing._completions import ResponseFormatT
from openai.types.chat.chat_completion_chunk import Choice as ChunkChoice, ChoiceDelta

from ..utils import print_obj
from ...conftest import base_url
Expand Down Expand Up @@ -1069,6 +1070,55 @@ def streamer(client: OpenAI) -> Iterator[ChatCompletionChunk]:
)


def _chunk(delta: ChoiceDelta, finish_reason: str | None) -> ChatCompletionChunk:
return ChatCompletionChunk.construct(
id="chatcmpl-test",
object="chat.completion.chunk",
created=0,
model="gpt-4o-2024-08-06",
choices=[ChunkChoice.construct(index=0, delta=delta, finish_reason=finish_reason)],
)


def _content_chunk(text: str) -> ChatCompletionChunk:
return _chunk(ChoiceDelta.construct(role="assistant", content=text), finish_reason=None)


def _finish_chunk(finish_reason: str) -> ChatCompletionChunk:
return _chunk(ChoiceDelta.construct(), finish_reason=finish_reason)


@pytest.mark.parametrize("finish_reason", ["length", "content_filter"])
def test_non_parse_stream_terminal_finish_reason_does_not_raise(finish_reason: str) -> None:
# A plain stream (no `response_format` and no parseable tools) has nothing to parse,
# so a `length` / `content_filter` finish reason must not raise from
# `get_final_completion()` — matching `chat.completions.create()` and the
# streaming accumulator, which already suppresses these for non-parse streams.
state: ChatCompletionStreamState[None] = ChatCompletionStreamState()

# accumulating the chunks must not raise
state.handle_chunk(_content_chunk("partial answer that got cut o"))
state.handle_chunk(_finish_chunk(finish_reason))

completion = state.get_final_completion()
assert completion.choices[0].finish_reason == finish_reason
assert completion.choices[0].message.content == "partial answer that got cut o"
assert completion.choices[0].message.parsed is None


def test_parse_stream_length_finish_still_raises() -> None:
# When a `response_format` is given there *is* something to parse, so the terminal
# `length` finish reason must still raise (unchanged behavior).
class Location(BaseModel):
city: str

state: ChatCompletionStreamState[Location] = ChatCompletionStreamState(response_format=Location)
state.handle_chunk(_content_chunk('{"city":"San Francisc'))

with pytest.raises(openai.LengthFinishReasonError):
state.handle_chunk(_finish_chunk("length"))


@pytest.mark.parametrize("sync", [True, False], ids=["sync", "async"])
def test_stream_method_in_sync(sync: bool, client: OpenAI, async_client: AsyncOpenAI) -> None:
checking_client: OpenAI | AsyncOpenAI = client if sync else async_client
Expand Down