Python: fix background=True + tools infinite-retrieve loop#5462
Open
he-yufeng wants to merge 1 commit intomicrosoft:mainfrom
Open
Python: fix background=True + tools infinite-retrieve loop#5462he-yufeng wants to merge 1 commit intomicrosoft:mainfrom
he-yufeng wants to merge 1 commit intomicrosoft:mainfrom
Conversation
| # of POSTing tool results (issue #5394). | ||
| if chat_response.continuation_token is None and isinstance(options, dict): | ||
| options.pop("continuation_token", None) | ||
| options.pop("background", None) |
Member
There was a problem hiding this comment.
I think it should still ask for a background response, but you are right in that the continuation_token needs to be reset
… tool loop Fixes microsoft#5394. When `background=True` is combined with local function tools, `FunctionInvocationLayer` calls `_inner_get_response(options=mutable_options)` repeatedly with the same dict reference across loop iterations. Once the first poll retrieves a completed background response, `continuation_token` stays in `mutable_options`, so every subsequent iteration takes the `continuation_token is not None` branch and `GET`s the same completed response instead of `POST`ing the tool results. The loop exits after `max_iterations` with empty text and the model never sees any tool output. After the retrieve, if the returned `ChatResponse.continuation_token` is `None` (the background response is no longer in progress), pop `continuation_token` and `background` from the shared options dict in place. The next loop iteration then falls through to the normal `responses.create`/`parse` path and posts tool results. The diagnosis and a verified runtime monkeypatch are in the issue; this is the same fix moved in-tree.
1ed0f6a to
8bfa009
Compare
Author
|
@eavanvalkenburg good catch — kept |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5394.
The bug
When a caller combines
background=Truewith local function tools, the agent gets stuck retrieving the same completed response on every tool-loop iteration and tool results are never POSTed. Aftermax_iterationsthe loop exits with empty text.Root cause
FunctionInvocationLayer.get_response()builds a singlemutable_options = dict(options)dict and threads it through every iteration of the tool loop viasuper_get_response(options=mutable_options).RawOpenAIChatClient._inner_get_responsereadscontinuation_tokenfrom that dict and, when present, takes theresponses.retrieve(response_id)branch.After the very first retrieve returns a completed background response,
continuation_tokenis still sitting inmutable_options. Every subsequent iteration therefore re-enters the retrieve branch and hits the same completed response again — never theresponses.create/parsepath that would POST the tool results.The HTTP trace from the issue confirms it: 1 POST followed by ~40 GETs of the same
response_id, and no tool-result POSTs.Fix
In
_inner_get_response, after the non-streaming retrieve completes, check whether the returnedChatResponsestill carries acontinuation_token. If it doesn't (i.e. the background operation is no longer in progress), popcontinuation_tokenandbackgroundfrom the caller's options dict in place.FunctionInvocationLayerpasses the same dict reference across iterations, so the mutation makes the next iteration fall through to_prepare_request→responses.create(...)and the tool results flow normally.This is the same change the reporter (@Laende) verified as a runtime monkeypatch, lifted into
_chat_client.py.Scope
responses.retrieve(stream=True)) yields chunks inside an async generator and isn't exercised by the tool loop in the same way; leaving that out until someone sees a real reproduction.optionsargument that's already owned and re-used by the caller.Test plan
background=Truecauses infinite tool-call loop, tool-result submissions inherit background mode #5394 (agent withbackground=True+ function tools): after this fix the HTTP trace should match the monkeypatched trace in the issue — 1 POST, a few GETs while the background response is in progress, then the tool-result POSTs that let the model produce a final answer.ruff check/ruff formatgreen locally; happy to add a regression test if a reviewer points me at the right mock scaffolding forclient.responses.retrieve.