Skip to content

Python: Normalize OpenAI tool-call argument envelopes on parse#4741

Open
davidahmann wants to merge 1 commit intomicrosoft:mainfrom
davidahmann:codex/tool-call-envelope-normalization
Open

Python: Normalize OpenAI tool-call argument envelopes on parse#4741
davidahmann wants to merge 1 commit intomicrosoft:mainfrom
davidahmann:codex/tool-call-envelope-normalization

Conversation

@davidahmann
Copy link

Closes #4740

Problem

OpenAI chat and responses parsers return function-call arguments as raw JSON strings, while the other backends already normalize equivalent tool-call payloads to mappings. That backend-dependent envelope shape leaks into downstream handling.

What changed

  • add a helper that normalizes provider function-call arguments to mappings when the payload is a JSON object
  • use that helper in the OpenAI chat and responses parse paths
  • add/update focused tests covering the normalized parse behavior

Validation

  • uv run pytest packages/core/tests/openai/test_openai_chat_client.py::test_parse_tool_calls_from_openai_normalizes_json_object_arguments packages/core/tests/openai/test_openai_responses_client.py::test_response_content_creation_with_function_call packages/core/tests/openai/test_openai_responses_client.py::test_parse_response_from_openai_function_call_includes_status -m "not integration"

@github-actions github-actions bot changed the title Normalize OpenAI tool-call argument envelopes on parse Python: Normalize OpenAI tool-call argument envelopes on parse Mar 17, 2026
@microsoft-github-policy-service

@davidahmann please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"
Contributor License Agreement

Contribution License Agreement

This Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
and conveys certain license rights to Microsoft Corporation and its affiliates (“Microsoft”) for Your
contributions to Microsoft open source projects. This Agreement is effective as of the latest signature
date below.

  1. Definitions.
    “Code” means the computer software code, whether in human-readable or machine-executable form,
    that is delivered by You to Microsoft under this Agreement.
    “Project” means any of the projects owned or managed by Microsoft and offered under a license
    approved by the Open Source Initiative (www.opensource.org).
    “Submit” is the act of uploading, submitting, transmitting, or distributing code or other content to any
    Project, including but not limited to communication on electronic mailing lists, source code control
    systems, and issue tracking systems that are managed by, or on behalf of, the Project for the purpose of
    discussing and improving that Project, but excluding communication that is conspicuously marked or
    otherwise designated in writing by You as “Not a Submission.”
    “Submission” means the Code and any other copyrightable material Submitted by You, including any
    associated comments and documentation.
  2. Your Submission. You must agree to the terms of this Agreement before making a Submission to any
    Project. This Agreement covers any and all Submissions that You, now or in the future (except as
    described in Section 4 below), Submit to any Project.
  3. Originality of Work. You represent that each of Your Submissions is entirely Your original work.
    Should You wish to Submit materials that are not Your original work, You may Submit them separately
    to the Project if You (a) retain all copyright and license information that was in the materials as You
    received them, (b) in the description accompanying Your Submission, include the phrase “Submission
    containing materials of a third party:” followed by the names of the third party and any licenses or other
    restrictions of which You are aware, and (c) follow any other instructions in the Project’s written
    guidelines concerning Submissions.
  4. Your Employer. References to “employer” in this Agreement include Your employer or anyone else
    for whom You are acting in making Your Submission, e.g. as a contractor, vendor, or agent. If Your
    Submission is made in the course of Your work for an employer or Your employer has intellectual
    property rights in Your Submission by contract or applicable law, You must secure permission from Your
    employer to make the Submission before signing this Agreement. In that case, the term “You” in this
    Agreement will refer to You and the employer collectively. If You change employers in the future and
    desire to Submit additional Submissions for the new employer, then You agree to sign a new Agreement
    and secure permission from the new employer before Submitting those Submissions.
  5. Licenses.
  • Copyright License. You grant Microsoft, and those who receive the Submission directly or
    indirectly from Microsoft, a perpetual, worldwide, non-exclusive, royalty-free, irrevocable license in the
    Submission to reproduce, prepare derivative works of, publicly display, publicly perform, and distribute
    the Submission and such derivative works, and to sublicense any or all of the foregoing rights to third
    parties.
  • Patent License. You grant Microsoft, and those who receive the Submission directly or
    indirectly from Microsoft, a perpetual, worldwide, non-exclusive, royalty-free, irrevocable license under
    Your patent claims that are necessarily infringed by the Submission or the combination of the
    Submission with the Project to which it was Submitted to make, have made, use, offer to sell, sell and
    import or otherwise dispose of the Submission alone or with the Project.
  • Other Rights Reserved. Each party reserves all rights not expressly granted in this Agreement.
    No additional licenses or rights whatsoever (including, without limitation, any implied licenses) are
    granted by implication, exhaustion, estoppel or otherwise.
  1. Representations and Warranties. You represent that You are legally entitled to grant the above
    licenses. You represent that each of Your Submissions is entirely Your original work (except as You may
    have disclosed under Section 3). You represent that You have secured permission from Your employer to
    make the Submission in cases where Your Submission is made in the course of Your work for Your
    employer or Your employer has intellectual property rights in Your Submission by contract or applicable
    law. If You are signing this Agreement on behalf of Your employer, You represent and warrant that You
    have the necessary authority to bind the listed employer to the obligations contained in this Agreement.
    You are not expected to provide support for Your Submission, unless You choose to do so. UNLESS
    REQUIRED BY APPLICABLE LAW OR AGREED TO IN WRITING, AND EXCEPT FOR THE WARRANTIES
    EXPRESSLY STATED IN SECTIONS 3, 4, AND 6, THE SUBMISSION PROVIDED UNDER THIS AGREEMENT IS
    PROVIDED WITHOUT WARRANTY OF ANY KIND, INCLUDING, BUT NOT LIMITED TO, ANY WARRANTY OF
    NONINFRINGEMENT, MERCHANTABILITY, OR FITNESS FOR A PARTICULAR PURPOSE.
  2. Notice to Microsoft. You agree to notify Microsoft in writing of any facts or circumstances of which
    You later become aware that would make Your representations in this Agreement inaccurate in any
    respect.
  3. Information about Submissions. You agree that contributions to Projects and information about
    contributions may be maintained indefinitely and disclosed publicly, including Your name and other
    information that You submit with Your Submission.
  4. Governing Law/Jurisdiction. This Agreement is governed by the laws of the State of Washington, and
    the parties consent to exclusive jurisdiction and venue in the federal courts sitting in King County,
    Washington, unless no federal subject matter jurisdiction exists, in which case the parties consent to
    exclusive jurisdiction and venue in the Superior Court of King County, Washington. The parties waive all
    defenses of lack of personal jurisdiction and forum non-conveniens.
  5. Entire Agreement/Assignment. This Agreement is the entire agreement between the parties, and
    supersedes any and all prior agreements, understandings or communications, written or oral, between
    the parties relating to the subject matter hereof. This Agreement may be assigned by Microsoft.

@davidahmann
Copy link
Author

This removes an OpenAI-only envelope mismatch where tool-call arguments arrived as raw JSON strings instead of the mapping shape other backends already emit.

The change stays on the parse boundary: OpenAI chat and responses payloads now normalize JSON-object arguments immediately, and the targeted tests were updated to lock that in.

Validation:

  • uv run pytest packages/core/tests/openai/test_openai_chat_client.py::test_parse_tool_calls_from_openai_normalizes_json_object_arguments packages/core/tests/openai/test_openai_responses_client.py::test_response_content_creation_with_function_call packages/core/tests/openai/test_openai_responses_client.py::test_parse_response_from_openai_function_call_includes_status -m "not integration" (pass)

Inspired by research context: CAISI publishes independent, reproducible AI agent governance research: https://caisi.dev

Copy link
Contributor

@moonbox3 moonbox3 left a comment

Choose a reason for hiding this comment

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

Automated Code Review

Reviewers: 4 | Confidence: 88%

✓ Correctness

The PR introduces normalize_function_call_arguments to eagerly parse JSON-string arguments into dicts at the provider-parsing layer. The helper function itself is correct and well-guarded. The non-streaming paths are properly updated. However, in the chat client, _parse_tool_calls_from_openai is shared by both non-streaming (Choice) and streaming (ChunkChoice) code paths. In streaming, partial JSON argument fragments will fail to parse and remain as strings (correct), but this creates a type inconsistency: non-streaming responses produce dict arguments while streaming accumulation produces str arguments via _add_function_call_content. Additionally, in a rare edge case where a single streaming chunk contains complete valid JSON, normalization would produce a dict, and a subsequent string fragment chunk would trigger TypeError('Incompatible argument types') in _add_function_call_content. No blocking bugs are present in practice since OpenAI streams arguments incrementally, but the inconsistency warrants consideration.

✗ Security Reliability

The new normalize_function_call_arguments function safely parses JSON-string arguments into dicts when receiving tool calls from the OpenAI API. However, the responses client's _prepare_content_for_openai method at line 1166 passes content.arguments directly back to the API without re-serializing to a JSON string. Since arguments can now be a dict after normalization, this will break the round-trip to the OpenAI Responses API, which expects arguments as a JSON string. The chat client already handles this correctly (line 640) with a json.dumps guard. The same issue affects line 1202 for mcp_approval_request payloads. Additionally, one function_call parsing site (line 1478) is inconsistently left un-normalized.

✗ Test Coverage

The diff introduces normalize_function_call_arguments in _types.py to parse JSON-string tool-call arguments into dicts. The function is used in 4 call sites (1 in chat client, 3 in responses client) but has zero unit tests of its own. The integration tests only cover the happy-path JSON-object-string case. There are no tests for: non-JSON strings, None input, already-dict/Mapping input, JSON arrays (should pass through as string), or json.JSONDecodeError fallback. The streaming code path in _parse_chunk_from_openai (line 1918 in responses client) also lacks test coverage for the normalization. Additionally, the mcp_call branches pass item.arguments without normalization — this may be intentional but is inconsistent with the other tool-call types.

✗ Design Approach

The PR introduces normalize_function_call_arguments to eagerly parse JSON-string arguments into dicts at parse time. While the intent is reasonable, there are three significant design problems: (1) normalization is applied to only a subset of from_function_call call sites, leaving others inconsistent; (2) the responses client serialization path at line 1166 passes content.arguments directly to the API payload without the json.dumps guard that the chat client already has, so a round-tripped function call will send a raw dict where OpenAI expects a JSON string; (3) the normalization logic belongs inside Content.from_function_call() (or Content.__init__) rather than scattered at individual call sites—there is already a parse_arguments() method on Content that duplicates this logic on-demand, making the new eagerly-parsed path a parallel but incomplete alternative.

Flagged Issues

  • Serialization regression in _responses_client._prepare_content_for_openai (line 1166): content.arguments is passed directly into the API payload, but after normalization the value may be a dict. The OpenAI Responses API expects a JSON string. The chat client already guards against this at line 640 with json.dumps(content.arguments) if isinstance(content.arguments, Mapping) else content.arguments, but the responses client does not. Multi-turn conversations that serialize a previously parsed function call back to the provider will send malformed payloads.
  • _responses_client.py line 1202: Same serialization issue — content.function_call.arguments (which may now be a dict after normalization) is passed directly in the mcp_approval_request payload without re-serialization to a JSON string.
  • normalize_function_call_arguments has no direct unit tests. It is a pure utility with multiple branches (str→dict, str→passthrough for non-dict JSON, Mapping→dict, None→None, JSONDecodeError→passthrough) that are not exercised by any test.
  • Normalization is applied to only a subset of call sites in _responses_client.py. The case "function_call" path (line 1478), mcp_call (line 1503), and untouched streaming paths (lines 1867, 1880, 2080) remain unnormalized, so content.arguments type will be str or dict depending on which code path produced the Content.
  • The normalization logic is placed at individual call sites rather than inside Content.from_function_call(). Every future provider adapter must independently remember to apply it. Centralizing in Content.from_function_call() (or Content.__init__) would fix all existing and future call sites at once, and the existing Content.parse_arguments() method already performs the same on-demand JSON parsing, making the new helper a parallel but incomplete alternative.

Suggestions

  • In _chat_client.py, _parse_tool_calls_from_openai is shared by both non-streaming (Choice) and streaming (ChunkChoice) paths. Normalizing here means streaming accumulation produces str arguments while non-streaming results become dict. In a rare edge case, if a streaming chunk contains complete valid JSON, normalization converts it to a dict, and a subsequent string-fragment chunk triggers TypeError('Incompatible argument types') in _add_function_call_content. Consider gating normalization on the non-streaming path or normalizing after accumulation.
  • The function_call case at line 1478 and mcp_call branches at lines 1503 and 1880 in _responses_client.py are not wrapped with normalize_function_call_arguments, unlike the other call sites. These should also normalize arguments for consistency.
  • Add parametrized unit tests for normalize_function_call_arguments covering: valid JSON object string, non-JSON string, None, already-a-dict, Mapping subclass, JSON array string, and malformed JSON.
  • Add a test for the streaming path (_parse_chunk_from_openai) in the responses client to verify normalization applies to streamed mcp_approval_request function calls.
  • If eager normalization is truly needed, consider unifying around a single implementation: normalize inside Content.from_function_call(), remove the standalone helper, and reconcile with the existing Content.parse_arguments() method to avoid duplicated JSON-parsing logic.

Automated review by moonbox3's agents

call_id=tool.id if tool.id else "",
name=tool.function.name if tool.function.name else "",
arguments=tool.function.arguments if tool.function.arguments else "",
arguments=normalize_function_call_arguments(tool.function.arguments if tool.function.arguments else ""),
Copy link
Contributor

Choose a reason for hiding this comment

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

This method is called for both non-streaming Choice and streaming ChunkChoice. Normalizing here means streaming chunks with partial JSON stay as strings (fine), but if a chunk contains complete valid JSON it becomes a dict, and a subsequent string chunk causes TypeError('Incompatible argument types') in _add_function_call_content. Consider gating normalization on the non-streaming path (isinstance(choice, Choice)) or applying it after accumulation.

if not isinstance(arguments, str):
return dict(arguments) if isinstance(arguments, Mapping) else arguments

try:
Copy link
Contributor

Choose a reason for hiding this comment

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

The normalization belongs inside Content.from_function_call() rather than as a standalone helper. All current and future call sites automatically benefit, and the existing parse_arguments() method's JSON-parsing logic is no longer duplicated. Applying it only at selected call sites leads to a Content whose arguments type depends on which parsing path produced it.


# region Internal Helper functions for unified Content


Copy link
Contributor

Choose a reason for hiding this comment

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

This utility function has five distinct code paths (None, non-str Mapping, non-str non-Mapping, valid JSON dict string, and non-dict-JSON/invalid-JSON string) but no direct unit tests. Add a parametrized test covering at least: NoneNone, '{"a":1}'{"a":1}, 'not json''not json', '[1,2]''[1,2]', {"a":1} (dict) → {"a":1}, and an OrderedDict → plain dict.

assert contents[0].arguments == {"city": "Seattle"}


def test_usage_content_in_streaming_response(
Copy link
Contributor

Choose a reason for hiding this comment

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

This test only covers the JSON-object-string → dict path. Consider adding cases for non-JSON arguments (e.g., arguments = 'plain text' should pass through as-is) and empty string (arguments = '') to verify the fallback behavior.

@@ -1037,7 +1037,7 @@ def test_response_content_creation_with_function_call() -> None:
function_call = response.messages[0].contents[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the only responses-client test covering function_call argument normalization in the non-streaming path. Consider adding a case where arguments is not valid JSON (e.g., partial/truncated) to confirm passthrough behavior.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: Normalize provider tool-call argument envelopes across chat backends

3 participants