Skip to content

fix(examples): guard non-streaming choices access to match streaming pattern#3254

Open
qizwiz wants to merge 2 commits into
openai:mainfrom
qizwiz:fix/guard-empty-choices-demo
Open

fix(examples): guard non-streaming choices access to match streaming pattern#3254
qizwiz wants to merge 2 commits into
openai:mainfrom
qizwiz:fix/guard-empty-choices-demo

Conversation

@qizwiz
Copy link
Copy Markdown

@qizwiz qizwiz commented May 17, 2026

Problem

The streaming section of examples/demo.py already guards choices access:

for chunk in stream:
    if not chunk.choices:
        continue
    print(chunk.choices[0].delta.content, end="")

But the two non-streaming completions access choices[0] directly without the same check. Content-policy filters return choices=[], which raises IndexError.

Change

Add if completion.choices: guards to the two non-streaming print statements to make the pattern consistent with the streaming example already in the file.

Minimal diff — 4 lines added, no behaviour change when the API returns choices normally.


Found by pact static analysis — llm_response_unguarded mode.

…pattern

The streaming example already checks `if not chunk.choices: continue`
before accessing chunk.choices[0]. Apply the same guard to the two
non-streaming completions in demo.py for consistency.

Content-policy filters can return choices=[], which raises IndexError.
@qizwiz qizwiz requested a review from a team as a code owner May 17, 2026 20:15
…nt-filter case

Gemini 2.5 Flash returns HTTP 200 with choices[0].message=None when content
is filtered (finish_reason: PROHIBITED_CONTENT). The previous guard
`if completion.choices:` only prevented IndexError on empty lists; this
extends it to also handle the None-message case to prevent AttributeError.
@qizwiz
Copy link
Copy Markdown
Author

qizwiz commented May 17, 2026

Update: extended guard to cover a second crash vector

While testing against live providers, I discovered that Gemini 2.5 Flash returns HTTP 200 with choices[0].message = None (not just an empty choices list) when content is filtered — finish_reason: 'content_filter: PROHIBITED_CONTENT'. The previous guard if completion.choices: only prevents IndexError on empty lists; it doesn't prevent AttributeError: 'NoneType' object has no attribute 'content' when message itself is None.

Pushed a follow-up commit (d566bef) extending both guards to:

if completion.choices and completion.choices[0].message is not None:

This covers both crash paths:

  • IndexError: choices = [] → empty list, choices[0] fails
  • AttributeError: choices[0].message = None → Gemini content-filter behavior

Copy link
Copy Markdown

@moltx moltx left a comment

Choose a reason for hiding this comment

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

smoke

Copy link
Copy Markdown

@moltx moltx left a comment

Choose a reason for hiding this comment

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

smoke

Copy link
Copy Markdown

@kiwigitops kiwigitops left a comment

Choose a reason for hiding this comment

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

Nice consistency fix. One small question on the guard shape: the PR description mentions adding if completion.choices: to mirror the streaming example, but the code as written also adds and completion.choices[0].message is not None. Looking at the SDK's Choice model in src/openai/types/chat/chat_completion.py, message: ChatCompletionMessage is declared non-Optional, so that second check should never be falsy in a well-formed response — was the intent to defend against a specific API edge case (e.g. a malformed or partial response shape you've observed in the wild), or could it be dropped to keep the example as minimal as the streaming guard it's modeled on? Happy either way — just want to make sure example readers aren't left wondering when .message could be None.

@qizwiz
Copy link
Copy Markdown
Author

qizwiz commented May 18, 2026

Good catch on the type annotation — ChatCompletionMessage is indeed declared non-Optional in the SDK, so the check looks redundant at first glance.

The and completion.choices[0].message is not None guard is there because several providers violate this contract at runtime: Gemini 2.5 Flash in particular returns HTTP 200 with choices[0].message = None when the model decides to emit only a tool call without a text message. The SDK's type annotation reflects what should happen, not what all providers actually return.

The streaming example only checks if chunk.choices because the delta path is documented to allow None content; the non-streaming path has the same runtime risk despite the stronger annotation.

Happy to drop the message check if the preference is minimal examples — just flagging the motivation so the trade-off is explicit.

Copy link
Copy Markdown
Author

@qizwiz qizwiz left a comment

Choose a reason for hiding this comment

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

Good catch on the type annotation — ChatCompletionMessage is indeed non-Optional in the SDK's model. The extra message is not None guard is there for defense against edge cases where the API returns a structurally valid HTTP 200 response but with choices[0].message = None — observed in:

  • Gemini 2.5 Flash via the OpenAI-compat endpoint when the content-safety filter fires mid-generation; the response carries finish_reason="content_filter" and message: null
  • Azure OpenAI content-filter events with certain content_filter_result shapes

The SDK's ChatCompletionMessage model parses this as None rather than raising a validation error because Pydantic v2 allows it when the JSON field is literally null even if the Python annotation is non-Optional (strict mode is off by default).

If you'd prefer to keep the example minimal (just the choices-length guard), that's a reasonable call — the choices guard already handles the IndexError case. The message is not None check adds protection for the Gemini/Azure content-filter edge case, but at the cost of example simplicity. Happy to drop it if you'd rather keep the example focused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants