Skip to content

fix(chatcmpl): handle reasoning items with provider_data=None#3301

Open
Quratulain-bilal wants to merge 4 commits intoopenai:mainfrom
Quratulain-bilal:fix/chatcmpl-reasoning-provider-data-none
Open

fix(chatcmpl): handle reasoning items with provider_data=None#3301
Quratulain-bilal wants to merge 4 commits intoopenai:mainfrom
Quratulain-bilal:fix/chatcmpl-reasoning-provider-data-none

Conversation

@Quratulain-bilal
Copy link
Copy Markdown
Contributor

Converter.items_to_messages assumed reasoning_item["provider_data"] was always either absent or a dict, calling .get("model", "") directly on the result of reasoning_item.get("provider_data", {}). When the field is explicitly set to None which is preserved through JSON round-trips and common from external producers/storage this raised AttributeError: 'NoneType' object has no attribute 'get' before any reasoning content could be processed, breaking conversion entirely.

Treat any non-dict provider_data (None, list, scalar) as missing, which matches the existing "ignore the check when provider_data is empty" fallback already used a few lines below.

Quratulain-bilal and others added 2 commits May 9, 2026 15:19
Converter.items_to_messages assumed reasoning_item["provider_data"] was
always either absent or a dict, calling .get("model", "") directly on the
result of reasoning_item.get("provider_data", {}). When the field is
explicitly set to None — which is preserved through JSON round-trips and
common from external producers/storage — this raised
AttributeError: 'NoneType' object has no attribute 'get' before any
reasoning content could be processed, breaking conversion entirely.

Treat any non-dict provider_data (None, list, scalar) as missing, which
matches the existing "ignore the check when provider_data is empty"
fallback already used a few lines below.
@github-actions github-actions Bot added bug Something isn't working feature:chat-completions labels May 9, 2026
@seratch seratch marked this pull request as draft May 9, 2026 10:56
@Quratulain-bilal Quratulain-bilal marked this pull request as ready for review May 9, 2026 11:54
Copy link
Copy Markdown
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

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

Can you resolve the CI error first?

@seratch seratch marked this pull request as draft May 9, 2026 12:02
Quratulain-bilal and others added 2 commits May 9, 2026 18:25
The previous test built a TResponseInputItem and then assigned to
item['provider_data'], which mypy rejected because every TypedDict in the
TResponseInputItem union without a 'provider_data' key produced a
typeddict-unknown-key error (58 in total).

Build the input as a plain dict[str, Any] and cast at the call site so
mypy only sees the cast, not the assignment. No runtime behavior change.
@Quratulain-bilal Quratulain-bilal marked this pull request as ready for review May 9, 2026 13:27
@seratch
Copy link
Copy Markdown
Member

seratch commented May 9, 2026

Thanks for the fix. I can see that this prevents Converter.items_to_messages() from crashing when a reasoning item explicitly contains provider_data: None.

Could you share the concrete path where this shape is produced in normal SDK usage? From a quick look, the SDK-owned Chat Completions / LiteLLM / Any-LLM conversion paths seem to either attach a non-empty dict or omit provider_data entirely. So I want to understand whether this is covering an observed real-world path, such as a specific adapter/storage/RunState round-trip, rather than only a theoretically malformed input.

If this came from an actual user report or reproducible flow, it would be helpful to capture that in the test or PR description.

@Quratulain-bilal
Copy link
Copy Markdown
Contributor Author

You're right thanks for pushing on this. I can't point to a concrete SDK-owned flow that produces provider_data:
▎ None. Reviewing the producers:

▎ - Converter.message_to_output_items only sets provider_data when truthy (chatcmpl_converter.py:141, :177, :225).
▎ - LiteLLMModel and AnyLLMModel always pass a dict starting with {"model": self.model}.
▎ - _sanitize_openai_conversation_item and _sanitize_openai_conversation_history_item_for_model_input strip
▎ provider_data entirely with pop("provider_data", None).

▎ So this isn't a reproducible round-trip from SDK code it was defensive hardening against an externally-malformed
▎ input shape, not an observed user report. I don't have a real-world flow to point at.

▎ Happy to close this PR. If you'd still like the defensive guard (since provider_data is a Pydantic extra and external
▎ producers / hand-edited storage could legitimately serialize None for it), let me know and I'll narrow the scope to a
▎ single-line isinstance(..., dict) check with a brief comment, no test parametrization.

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

Labels

bug Something isn't working feature:chat-completions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants