Skip to content

Python: fix(openai): report the background cause when a tool result is rejected - #7603

Closed
Chinmay V (chinmayv095) wants to merge 3 commits into
microsoft:mainfrom
chinmayv095:fix/background-tool-continuation-diagnostic
Closed

Python: fix(openai): report the background cause when a tool result is rejected#7603
Chinmay V (chinmayv095) wants to merge 3 commits into
microsoft:mainfrom
chinmayv095:fix/background-tool-continuation-diagnostic

Conversation

@chinmayv095

@chinmayv095 Chinmay V (chinmayv095) commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Motivation & Context

When a local tool loop starts with background=True, the Responses API rejects the first tool-result continuation with:

400 No tool call found for function call output with call_id call_<id>.

As established in #7538, the predecessor reaches status="completed" and retrieving it returns the matching function_call — the emitted, submitted, retrieved and error-reported call_id are all identical. The same previous_response_id chaining succeeds when the predecessor is created with background=False. Gieril Lindi (@Laende) confirmed the failure reproduces directly against Azure OpenAI without APIM and against the raw AsyncOpenAI client, so the defect is below Agent Framework and is tracked in Azure/azure-sdk-for-python#46092.

What Agent Framework controls is the diagnostic. Today the failure arrives as the generic service failed to complete the prompt message, which points a reader at the tool result they just submitted rather than at the background predecessor that actually caused the rejection. The call_id in the message is present and correct on both sides, so the text actively misleads.

This implements the third option in the issue, matching Evan Mattson (@moonbox3)'s guidance to consider "a clearer, targeted diagnostic" rather than an automatic full-history replay. No fallback, retry, or continuation-strategy change is included, so stored conversation state, reasoning and tool-call history, and local tool execution counts are all untouched.

Description & Review Guide

  • What are the major changes?

    • _handle_request_error takes the run_options that produced the failure and, for this one case, raises ChatClientException with the cause and the supported alternative instead of the generic message. Both existing behaviours are preserved: the content-filter branch is unchanged, and every other error keeps the generic text.
    • A module-level _is_background_tool_output_pairing_error requires all three signals before the message changes: the pairing error text, a previous_response_id continuation, and a background request. The error body carries no code ('code': None in the issue's payload), so the message fragment is the only available signal; requiring the other two keeps it from firing on a genuinely orphaned foreground tool result, which is a real user error that should keep the generic message.
    • The guidance is phrased as a condition, not a claim. background describes the failing request, not the response named by previous_response_id, and a stateless client cannot know how that predecessor was created — it may have been produced by a different process. A genuinely orphaned call_id on a background continuation therefore reaches this branch too, so the message asserts nothing about the predecessor: it gives the background limitation as a condition the caller can check and names the orphaned-call_id alternative. test_background_tool_output_pairing_error_does_not_assert_predecessor_was_background pins that wording.
    • Per python/AGENTS.md, docs/specs/004-python-function-calling-loop.md gains a Rejected tool-output continuation row in its errors/continuation matrix covering the invariant and all five regression tests.
    • The two retrieve-only call sites deliberately pass no run_options — they carry no request body to attribute a failure to, so they cannot produce this error.
  • What is the impact of these changes?

    • Message-only, on a path that already raised. No public API change: run_options is an optional parameter on an existing private method, appended last. No new exception type, no new configuration.
    • The combination that triggers it fails today with a 400 either way, so no working call changes behaviour.
  • What do you want reviewers to focus on?

    • Whether the wording is the right steer. It names background=False for the tool-calling turn as the supported alternative, since store=False is not an option for background requests. If you would rather it not recommend a specific workaround, or would rather this be a warning at request time instead of a message on the existing failure, I am happy to change it.
    • Whether matching on the message fragment is acceptable given the absent error code. If you would prefer this keyed on something more stable, I can rework it.

Related Issue

Refs #7538

Deliberately not a closing keyword, at Gieril Lindi (@Laende)'s request. This addresses the framework-side diagnostic only, not the chaining defect, so #7538 stays open to track the upstream service fix and the question of a supported continuation strategy.

Verification

Ran against packages/openai:

  • Fail-first: with the tests in place and the source change reverted, the two background tests fail with the generic service failed to complete the prompt message; the two negative-control tests pass both ways by design, since they assert the generic message is kept.
  • Full package suite: main 452 passed / 0 failed / 0 errors, branch 456 passed / 0 failed / 0 errors. The FAILED/ERROR sets are identical (both empty) and the delta is exactly the four new tests.
  • mypy: 13 errors on the branch, and the same 13 on a stashed clean tree — all pre-existing, none in the changed lines.
  • ruff format --diff and ruff check both clean on the two touched files.

Four tests added to tests/openai/test_openai_chat_client.py, all offline:

Test Asserts
test_background_tool_output_pairing_error_reports_background_limitation non-streaming path reports the cause
test_streaming_background_tool_output_pairing_error_reports_background_limitation streaming path reports the same thing
test_foreground_tool_output_pairing_error_keeps_generic_message no background → generic message
test_background_without_previous_response_keeps_generic_message no continuation → generic message

The last two are the ones that prove the gate is narrow rather than catching every pairing error.

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.

…s rejected

A function_call_output chained with previous_response_id is rejected by the
Responses API when the predecessor response was created with background=True,
even though the predecessor contains the matching function_call. The failure
surfaced only as the generic prompt-failure message, which points at the tool
result rather than at the background predecessor that actually caused it.

Report the cause and the supported alternative when all three signals are
present: the pairing error, a previous_response_id continuation, and a
background request. Foreground chaining is unaffected and keeps the generic
message.
Copilot AI balanced review requested due to automatic review settings August 10, 2026 18:20
@agent-framework-automation agent-framework-automation Bot added the python Usage: [Issues, PRs], Target: Python label Aug 10, 2026

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

Adds targeted diagnostics for background Responses API tool-output pairing failures.

Changes:

  • Detects background continuation pairing errors.
  • Raises actionable ChatClientException messages.
  • Adds streaming, non-streaming, and negative-control tests.

Reviewed changes

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

File Description
python/packages/openai/agent_framework_openai/_chat_client.py Adds detection and specialized error handling.
python/packages/openai/tests/openai/test_openai_chat_client.py Adds four diagnostic regression tests.

💡 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_client.py
Comment thread python/packages/openai/agent_framework_openai/_chat_client.py
@Laende

Copy link
Copy Markdown

Thanks for #7603. The clearer diagnostic is useful, but please do not close #7538 as fixed. The PR explicitly adds no fallback or continuation change, so the background tool loop remains broken and currently blocks our workflow. background=False avoids the affected feature rather than fixing it, and we would prefer a supported fix over application-level replay workarounds.

Could the PR drop Fixes #7538 and keep the issue open until the service defect is fixed or Agent Framework provides a supported continuation strategy?

… claim

The run options' background flag describes the failing request, not the
response named by previous_response_id. A genuinely orphaned call_id on a
background continuation reaches the same branch, so the guidance no longer
asserts the predecessor was a background response; it gives the background
limitation as a condition to check and names the orphaned-call_id
alternative. Adds a regression test for that wording and the required
scenario-to-test row in the function-calling loop specification.
@agent-framework-automation agent-framework-automation Bot added the documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs label Aug 11, 2026
@chinmayv095

Copy link
Copy Markdown
Contributor Author

Done — the closing keyword is dropped. The body now reads Refs #7538, so the issue stays open.

Agreed on the framing, and thanks for pushing back on it. This PR is diagnostic-only by design: it does not fix the chaining failure, and background=False avoids the affected path rather than repairing it. The message offers it as a workaround to unblock a caller staring at a 400, not as a resolution — and it now says so conditionally rather than asserting a cause, after a review point that the flag it keys on describes the failing request rather than the predecessor.

The two things you want kept alive both belong on #7538 rather than here: the upstream service defect (Azure/azure-sdk-for-python#46092) and the question of whether Agent Framework should offer a supported continuation strategy. Evan Mattson (@moonbox3)'s caution about automatic full-history replay is on that issue too — stored conversation state, reasoning and tool-call history, and local tools potentially running more than once — so a supported fix needs a design decision that is well outside a message change.

Your direct-endpoint reproduction table is what makes the upstream attribution stand up, so it is cited in the PR body.

@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/openai/agent_framework_openai
   _chat_client.py139811991%341, 354, 731–737, 746–749, 755–759, 767, 811–816, 820–823, 825–827, 834–836, 839, 896, 904, 927, 1144, 1203, 1205, 1207, 1209, 1275, 1289, 1369, 1379, 1384, 1427, 1539–1540, 1555, 1834, 1941, 1946–1947, 2030, 2040, 2067, 2073, 2083, 2089, 2094, 2100, 2105–2106, 2186, 2230, 2233–2236, 2250, 2260–2261, 2273, 2315, 2380, 2397, 2400, 2427–2429, 2468, 2485, 2488, 2550, 2557, 2594–2595, 2630, 2668–2669, 2687–2688, 2860–2861, 2879, 2965–2973, 3151, 3166, 3255–3257, 3267–3268, 3274, 3289, 3422–3423
TOTAL46826436090% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
9470 36 💤 0 ❌ 0 🔥 2m 33s ⏱️

if _is_background_tool_output_pairing_error(ex, run_options):
raise ChatClientException(
maybe_append_azure_endpoint_guidance(
f"{type(self)} service rejected the tool result: {ex} "

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.

Could we reject background=True with local tools before the first Responses request? The new predicate runs only after the background job and local tool execution have already occurred, leaving callers to restart the run after the failed continuation. A preflight check or explicit continuation policy would address the unsupported combination at the layer that creates it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good question, and I looked at whether it can be done at that point. I do not think a check before the first request can be made correct, but there is a narrower one that can, and the gap you are pointing at is real either way.

Why not before the first request. The 400 is raised by the continuation, not by the request that declares the tools, so at the first request none of the failing conditions exist yet. Rejecting background=True + local tools there would reject three things that work today:

  1. A background run whose model never calls a local tool. Declaring a FunctionTool does not mean one gets called. With no function_call there is no continuation and no 400, so these runs complete normally.
  2. A background run chained through a Conversations object. _prepare_options sends conversation for a conv_... id and only sets previous_response_id for a resp_... id, so a conversation-chained background run never builds the request the service rejects.
  3. A background run whose tools are hosted. _prepare_tools_for_openai only converts FunctionTool; web_search, code_interpreter and the rest pass through and execute service-side, so they never produce a function_call_output.

The stronger point is in Gieril Lindi (@Laende)'s own harness table in #7538: a fresh stored background response carrying replayed output and tool output, with no previous_response_id, completed 5/6. So background=True with local tools is not the unsupported combination. function_call_output + previous_response_id + a background predecessor is. A preflight keyed on the first pair would reject a combination the issue itself measured as working.

What is decidable, and where. The narrowest correct preflight is on the continuation request rather than the first one: background and previous_response_id and an input carrying a function_call_output. That is exactly the predicate this PR already evaluates, just checked before the HTTP call instead of after the 400 comes back. It saves the doomed round trip. I am happy to move it there if you want it.

I would gently argue against it, though, for the reason the diagnostic form has going for it: this is a service defect tracked in Azure/azure-sdk-for-python#46092. A local reject is a hard-coded assertion about service behaviour, so when the service starts accepting the pairing, Agent Framework keeps refusing it until someone ships a revert. A message attached to the service's own 400 stops appearing by itself the day the 400 stops.

The part of your comment I cannot answer with a preflight. You are right that the local tool has already run by the time this fires, and no check on the request path changes that. The only place to close it is between the background response arriving with a function_call and Agent Framework executing the tool. The chat client cannot take that decision on its own: at that point it holds a valid completed response, and the caller may well continue by replaying into a fresh response or through a Conversations object, neither of which hits the rejection. Refusing to return the response would break those callers.

That makes it the "explicit continuation policy" half of your comment rather than the preflight half, and it is your design call, not mine. If you want a background-continuation policy on the client, with replay or reject as the non-default options, I am glad to build it, either in this PR or a follow-up.

So: leave it as the diagnostic, move it to a preflight on the continuation request, or take it to a continuation policy. Tell me which and I will push it.

@moonbox3

Copy link
Copy Markdown
Contributor

Thanks for digging into this. I agree the remaining issue is a core continuation-policy decision involving replay and side-effect safety. Since this PR only changes the post-failure diagnostic and does not improve recovery, I think we should leave #7538 open for a core-maintainer-owned design rather than spend more community cycles polishing this workaround. We can revisit the diagnostic as part of that fix.

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

Labels

documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants