Skip to content

Python: Align OpenAI parser conversions - #7964

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 3 commits into
microsoft:mainfrom
eavanvalkenburg:eavanvalkenburg-openai-parser-alignment
Sep 1, 2026
Merged

Python: Align OpenAI parser conversions#7964
Eduard van Valkenburg (eavanvalkenburg) merged 3 commits into
microsoft:mainfrom
eavanvalkenburg:eavanvalkenburg-openai-parser-alignment

Conversation

@eavanvalkenburg

Copy link
Copy Markdown
Member

Motivation & Context

The Python OpenAI clients still had a few conversion paths that conflated valid falsey values with missing values, emitted Agent Framework's internal content representation into Chat Completions requests, or assumed embedding responses arrived in input order. These mismatches could lose provider data, produce invalid request content, or associate an embedding with the wrong input.

Description & Review Guide

  • What are the major changes? Preserve SDK-supported falsey Responses function outputs, omit unsupported Agent Framework content from Chat Completions payloads, retain explicitly reported zero-valued optional usage details, and order embedding response items by their indexes.
  • What is the impact of these changes? OpenAI request and response conversion is more faithful while remaining compatible with the OpenAI 2.x wire types. Unsupported content is omitted instead of being sent as a schema-invalid dictionary.
  • What do you want reviewers to focus on? Please focus on the narrow conversion behavior changes and their regression coverage, especially the unsupported-content omission semantics.

Related Issue

N/A — follow-up from the Responses conversion consistency sweep

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Aligns Python OpenAI conversions with provider wire formats and response metadata.

Changes:

  • Preserves supported falsey outputs and zero-valued usage details.
  • Omits unsupported Chat Completions content.
  • Orders embeddings by provider indexes and adds regression tests.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
_chat_client.py Updates function-result output handling.
_chat_completion_client.py Filters unsupported content and preserves zero usage values.
_embedding_client.py Sorts embeddings by response index.
test_openai_chat_client.py Tests falsey function outputs.
test_openai_chat_completion_client.py Tests omission and usage conversions.
test_openai_embedding_client.py Tests indexed embedding ordering.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread python/packages/openai/tests/openai/test_openai_chat_completion_client.py Outdated
Comment thread python/packages/openai/agent_framework_openai/_chat_client.py Outdated
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/openai/agent_framework_openai
   _chat_client.py143311791%361, 374, 728–734, 743–746, 752–756, 764, 808–813, 817–820, 822–824, 831–833, 836, 914, 922, 945, 1162, 1221, 1223, 1225, 1227, 1293, 1307, 1387, 1397, 1402, 1445, 1557–1558, 1573, 1852, 1964, 1969–1970, 2020, 2024–2025, 2064, 2074, 2101, 2107, 2117, 2123, 2128, 2134, 2139–2140, 2220, 2264, 2267–2270, 2284, 2294–2295, 2307, 2414, 2431, 2434, 2461–2463, 2502, 2519, 2522, 2584, 2591, 2628–2629, 2670, 2708–2709, 2727–2728, 2900–2901, 2919, 2953, 3026–3029, 3213, 3228, 3317–3319, 3329–3330, 3336, 3351, 3484–3485
   _chat_completion_client.py4541696%543, 637, 644–645, 649, 674, 849, 1007, 1075, 1092, 1114, 1122, 1148, 1161, 1194, 1560
   _embedding_client.py81297%240, 288
TOTAL48418450090% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
9865 36 💤 0 ❌ 0 🔥 2m 45s ⏱️

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MAF Automated Review — Iteration 1

Result: Findings reported
Scope: full PR (1 commit(s)): be809084dc76
Model: gpt-5.6-sol

Overview

The PR improves conversion fidelity by preserving SDK-supported falsey outputs, retaining explicit zero usage details, omitting unsupported Chat Completions content, and ordering embeddings by provider index, with focused regression coverage for each path. The remaining compatibility risk is that the broader falsey-value check also passes arbitrary deserialized values through to an OpenAI field that accepts only strings or rich-content lists.

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 medium) across 1 file. Details are attached to the affected lines below.

Affected areas: python/packages/openai/agent_framework_openai/_chat_client.py

Comment thread python/packages/openai/agent_framework_openai/_chat_client.py Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MAF Automated Review — Iteration 2

Result: No findings
Scope: 1 net-new commit(s): dd765cd2e6c6
Model: gpt-5.6-sol

Overview

This incremental update addresses the prior function-output finding by preserving supported string and list values while serializing other non-None values with the same json.dumps(default=str) pattern used by the framework's canonical function-result constructor. The added tests cover the previously problematic falsey values False, 0, and {} as well as preserving "" and []. No Critical, High, or Medium issue remains in the supplied incremental diff.

Reviewed the supplied incremental change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.

Comment thread python/packages/openai/agent_framework_openai/_chat_client.py Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: aad873ba-4f58-4be2-ad55-720795157704
Merged via the queue into microsoft:main with commit b037924 Sep 1, 2026
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants