Python: fix: use getattr for non-OpenAI provider response compatibility#6270
Python: fix: use getattr for non-OpenAI provider response compatibility#6270Oxygen56 wants to merge 3 commits into
Conversation
Fixes microsoft#6234 Fixes microsoft#6235 Use getattr with None fallback for system_fingerprint and output attributes to prevent AttributeError when non-OpenAI providers return response objects without these fields.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR hardens OpenAI response parsing by making attribute access resilient to optional/missing fields in different response shapes.
Changes:
- Use
getattr(..., default)when readingsystem_fingerprintfrom chat and streaming chat responses. - Use
getattr(..., [])when iteratingresponse.outputto avoid attribute errors when the field is absent.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| python/packages/openai/agent_framework_openai/_chat_completion_client.py | Makes metadata extraction tolerant to missing system_fingerprint. |
| python/packages/openai/agent_framework_openai/_chat_client.py | Prevents failures when response.output is not present by defaulting to an empty list. |
Python Test Coverage Report •
Python Unit Test Overview
|
|||||||||||||||||||||||||||||||||||
|
@Oxygen56 there are checks failing, please check, and ideally run them locally first. |
Fixes microsoft#6235 Use getattr with None fallback for the output attribute, and assign to a typed list variable before the match statement to help pyright narrow the response item types correctly.
|
Thanks for the review @eavanvalkenburg! Fixed the pyright error — the |
|
@Oxygen56 both checks and tests now failing, please have a look |
…variable Fixes microsoft#6235 Rename outputs to response_outputs on line 1974 to avoid mypy error about conflicting variable names in the match statement's case blocks. Also use list[Any] for explicit generic type annotation.
|
@eavanvalkenburg sorry about that! Fixed the mypy error — the |
|
@Oxygen56 pyright failed... thanks for keeping at it! |
Summary
system_fingerprint(fixes Python: [Bug]: OpenAIChatCompletionClient fails with 'str' object has no attribute 'system_fingerprint' when provider returns non-OpenAI conformant response #6234)outputattribute (fixes Python: [Bug]: OpenAIChatClient fails with 'str' object has no attribute 'output' when provider returns non-OpenAI conformant/structured response #6235)getattr(x, attr, None)pattern for defensive accessFixes #6234
Fixes #6235
Test Plan