Skip to content

Python: fix: parse Responses function_call_output so hosted tool results reach transports - #8078

Draft
Manjunath Janardhan (manjunathshiva) wants to merge 4 commits into
microsoft:mainfrom
manjunathshiva:python-openai-parse-function-call-output-8068
Draft

Python: fix: parse Responses function_call_output so hosted tool results reach transports#8078
Manjunath Janardhan (manjunathshiva) wants to merge 4 commits into
microsoft:mainfrom
manjunathshiva:python-openai-parse-function-call-output-8068

Conversation

@manjunathshiva

Copy link
Copy Markdown
Contributor

Motivation & Context

A Foundry hosted agent using a Foundry Toolbox dispatches its knowledge-base tool through the toolbox's generic call_tool wrapper. The Toolbox executes the inner tool server-side and ResponsesHostServer serializes the result as a standard function_call_output Responses item. None of the three parse dispatch sites in agent_framework_openai handled that item type, so it fell into case _: / the end of the elif chain, was logged as Unparsed event of type: ... at debug level, and discarded.

No Content.from_function_result(...) was produced, so agent_framework_ag_ui emitted TOOL_CALL_END with no matching TOOL_CALL_RESULT and its declaration-only fallback took over (_agent_run.py:3100-3108). The model still received the real output — only the client lost the structured result. The same tool called directly is an mcp_call and works, which is what isolates this to the client parser.

Description & Review Guide

  • What are the major changes?

    A function_call_output branch at all three dispatch sites — the non-streaming _parse_response_from_openai and both streaming response.output_item.added / .done handlers — sharing one _parse_function_call_output_content helper and one _function_call_output_has_result gate, so the three item-type lists cannot drift apart on this type again.

    output is str | list[ResponseInputText|Image|File], so it is normalized through the existing _stringify_mcp_output rather than JSON-encoding provider models. That helper's name is MCP-flavoured but its logic is generic; happy to rename it if you would prefer (1 caller, 2 dedicated tests).

    The two streaming handlers emit from whichever event first carries a populated output, recording the item id in a per-request set so the other cannot emit a second result. Keyed on the item id rather than call_id, which the function-calling loop contract says must not be assumed unique forever.

    Two follow-up commits are separated deliberately so they can be read on their own: forwarding the new parameter through the two RawFoundryChatClient / RawFoundryAgentChatClient overrides, and rejecting a blank call_id so no unpairable result is emitted.

  • What is the impact of these changes?

    Hosted-toolbox tool results now reach transports as function_result content, so AG-UI emits the full TOOL_CALL_STARTTOOL_CALL_ENDTOOL_CALL_RESULT lifecycle. No AG-UI change was needed: _emit_tool_result already handles function_result.

    Reviewed against docs/specs/004-python-function-calling-loop.md, which covers provider serialization of function calls and results. No result is orphaned or duplicated, and the streaming and non-streaming paths agree. I have not edited the spec — its checklist asks that the matrix name a regression test for each affected scenario, and I would rather you decide whether this warrants a row than edit a cross-package contract in a bug fix. Glad to add one.

    Verified against a live Foundry Responses endpoint in addition to the unit tests. Two things that measurement settled:

    1. Every output item fires both .added and .done (confirmed for function_call, message, reasoning). Emitting from both handlers without the seen-id set would have produced duplicate results; emitting from only one would have been a guess about which event carries the payload.
    2. A stored function_call_output is re-sent inline on the next turn under previous_response_id, and the service accepts it. Since the outbound serializer emits only call_id / type / output and no item id, a server-generated result is indistinguishable on the wire from a locally-executed one, so this needs no outbound companion change.
  • What do you want reviewers to focus on?

    Three things I could not settle myself, all raised deliberately rather than left for you to find:

    1. The parse signature as an extension point. Adding one per-request state field meant touching a creation site, three call sites, and two subclass overrides — I broke Foundry streaming and caught it on live traffic. I followed the existing mechanism: seen_reasoning_delta_item_ids was added the same way and both Foundry overrides already carry it. If you would rather this state travelled in a per-request context object, that is a refactor of a spec-004 method and I would do it separately rather than inside a bug fix.
    2. Whether _parse_chunk_from_openai counts as public API. RawOpenAIChatClient is exported and its docstring demonstrates subclassing, so an out-of-tree override with the old signature would break the same way Foundry did. I read the leading underscore as private and did not label this a breaking change — tell me if you disagree and I will relabel.
    3. packages/foundry is missing from spec 004's minimum validation commands (docs/specs/004-python-function-calling-loop.md), even though it subclasses the OpenAI Responses client the spec governs. That omission is exactly why my first sweep missed the regression above; uv run poe test -P foundry reproduces it. Worth adding for the next contributor.

    Separately, and not proposed here: the case _: default at all three sites drops any unknown item type at debug severity, which is why the reporter hit three such types in one session (function_call_output plus two SharePoint preview ones). Silently discarding a tool result is a correctness event rather than a diagnostic one. I deliberately kept this PR to the reproduced type — the SharePoint types are preview and I have no repro, so guessing their shape risks a wrong parser. If useful I will open a separate issue proposing either a shared dispatch table across the three sites or a warning for unknown *_output items.

Related Issue

Fixes #8068

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.

…each transports

A hosted tool that executes server-side -- for example a Foundry Toolbox
dispatching through its generic `call_tool` wrapper -- returns its result as a
standalone `function_call_output` Responses item rather than on the originating
call item. None of the three parse dispatch sites handled that item type, so it
fell through to the `Unparsed ...` debug log and was discarded. No
`Content.from_function_result` was produced, and AG-UI consequently emitted
TOOL_CALL_END with no matching TOOL_CALL_RESULT, falling back to treating the
call as declaration-only. The model still received the real output, so only the
client lost the structured result.

Add a `function_call_output` branch to all three sites -- the non-streaming
`_parse_response_from_openai` and both streaming `response.output_item.added` /
`.done` handlers -- sharing one `_parse_function_call_output_content` helper so
the lists cannot drift again. `output` is a string or a list of input-content
parts, so it is normalized through the existing `_stringify_mcp_output` rather
than JSON-encoding provider models.

The streaming handlers emit from whichever event first carries a populated
`output` and record the item id in a per-request set, so the other event cannot
produce a second result. Keyed on the item id rather than `call_id`, which the
function-calling loop contract says must not be assumed unique forever.

Reviewed against docs/specs/004-python-function-calling-loop.md, which covers
provider serialization of function calls and results: no result is orphaned or
duplicated, and the streaming and non-streaming paths agree.

Fixes microsoft#8068
…parse overrides

`RawFoundryChatClient` and `RawFoundryAgentChatClient` override
`_parse_chunk_from_openai` to intercept oauth_consent items and then delegate to
`RawOpenAIChatClient`. Both had the pre-change signature, so once the base
started passing `seen_function_call_output_ids` every Foundry streaming call
raised `TypeError: _parse_chunk_from_openai() got an unexpected keyword
argument`.

Accept and forward the new parameter in both overrides, and update the two
delegation assertions that pin the forwarded argument list.

Caught against a live Foundry Responses endpoint; `poe test -P foundry` also
reproduces it, but that package is not in the validation command list in
docs/specs/004-python-function-calling-loop.md even though it subclasses the
OpenAI Responses client.
Review follow-up. `Content.from_function_result` does not validate `call_id`, so a
`function_call_output` item carrying a blank one produced an orphaned result:
transports drop it (`_emit_tool_result` returns early on a falsy `call_id`) and
the outbound serializer would re-send it as an unpairable
`function_call_output` input item on the next turn. These items are synthesized
by the hosting layer, so a blank `call_id` is a realistic host-side defect
rather than a theoretical one, and the function-calling loop contract requires
that no result becomes orphaned.

Extract the emission gate into `_function_call_output_has_result` so the
populated-output and pairable-call_id checks are shared by all three dispatch
sites instead of being repeated at each one.

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.

🟡 Changes recommended

Supported older SDK versions can crash, and rich output parts are not serialized into usable results.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds parsing for hosted function_call_output items so tool results reach downstream transports.

Changes:

  • Parses streaming and non-streaming function outputs.
  • Deduplicates streaming results by item ID.
  • Propagates parser state through Foundry clients and adds tests.
File summaries
File Description
python/packages/openai/agent_framework_openai/_chat_client.py Implements output parsing and deduplication.
python/packages/openai/tests/openai/test_openai_chat_client.py Tests parsing scenarios.
python/packages/foundry/agent_framework_foundry/_chat_client.py Forwards deduplication state.
python/packages/foundry/agent_framework_foundry/_agent.py Forwards deduplication state.
python/packages/foundry/tests/foundry/test_foundry_chat_client.py Updates delegation assertion.
python/packages/foundry/tests/foundry/test_foundry_agent.py Updates delegation assertion.
Review details

Suppressed comments (1)

python/packages/openai/agent_framework_openai/_chat_client.py:2692

  • For list output, the SDK supplies ResponseInputText/ResponseInputImage/ResponseInputFile model instances, not the dictionaries used in the new test. Text happens to work via .text, but image/file parts fall through to json.dumps(..., default=str), producing quoted Pydantic reprs (and concatenating multiple reprs) rather than preserving usable rich output. Map these provider parts to Content items, or serialize the full list with _serialize_provider_payload while retaining its boundaries before creating the function result.
        return Content.from_function_result(
            call_id=item.call_id,
            result=self._stringify_mcp_output(item.output),
            additional_properties=additional_properties,
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Balanced

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

Comment thread python/packages/openai/agent_framework_openai/_chat_client.py Outdated
… SDK floor

Address review on two counts.

`name` is not on `ResponseFunctionToolCallOutputItem` in openai 2.25.0, the
declared floor -- that version ships only call_id/id/output/status/type. Reading
it as an attribute raised `AttributeError` out of the shared parse helper, which
all three dispatch sites call, so on any supported SDK below the release that
added the field the whole response parse failed rather than merely dropping the
result. Read it with `getattr`. The other attributes touched here
(type/status/id/call_id/output) are all present on the floor, and the two module
helpers already used `getattr`.

`output` may also be a list of input-content parts. Passing those provider
models straight to `_stringify_mcp_output` fell through to
`json.dumps(..., default=str)` and embedded a Python repr in the result text sent
back to the model -- e.g. `"ResponseInputImage(detail='auto', ...)"`. Dump each
part first so text extraction still works and non-text parts serialize as
readable JSON.

Both paths are now regression-tested, including a stub item shaped like the
2.25.0 field set.
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

2 participants