Visitar URL original
fix(integrations): replay OpenAI Responses reasoning items between tool calls by Bensonluo · Pull Request #8112 · google/adk-python · GitHub
Skip to content

fix(integrations): replay OpenAI Responses reasoning items between tool calls - #8112

Open
Bensonluo wants to merge 2 commits into
google:mainfrom
Bensonluo:fix/openai-responses-reasoning-replay
Open

Bensonluo wants to merge 2 commits into
google:mainfrom
Bensonluo:fix/openai-responses-reasoning-replay

Conversation

@Bensonluo

Copy link
Copy Markdown

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

Problem

Reasoning models on the Responses API return their reasoning as separate output items, and OpenAI's function-calling guide requires clients to pass those items back with the tool call outputs. OpenAIResponsesLlm never did, so in every tool loop the model restarted its reasoning from scratch after each tool call, and with store: false the encrypted reasoning state was lost entirely. Two gaps combined:

  • Inbound, _reasoning_parts stored the item's encrypted_content on each thought part's thought_signature, but the reasoning item id was recorded only in the response-level custom_metadata, which the request builder never reads.
  • Outbound, _content_to_response_input_items skipped every thought part on replay (its docstring: without real item IDs, synthetic ones are rejected), and previous_response_id is never set for this model — the flow only fills previous_interaction_id for the Gemini interactions API — so continuity was neither replayed nor chained.

Solution

  • Inbound: _reasoning_parts now stores a JSON envelope {"adk_openai_reasoning": {"id", "encrypted_content"}} on the first thought part of each captured reasoning item (at most one envelope per item, so a rebuilt input never contains duplicate reasoning ids). The envelope is built only when the item has an id: the Responses API requires reasoning input items to reference a real item, so id-less captures keep today's behavior instead of risking a 400. The same slot already carried the encrypted payload, so session persistence and the streaming paths (response.completed and the output_item.done accumulator branch) are covered without further changes.
  • Outbound: _content_to_response_input_items rebuilds a {type: "reasoning", id, summary, encrypted_content} input item at the part's original position, in front of the function call it came with, whenever a part carries the envelope. Everything without an envelope — synthetic thoughts, plain summaries, pre-envelope sessions that stored raw encrypted bytes, other providers' signatures — takes the pre-existing skip path unchanged, preserving the guard's original rationale.
  • previous_response_id handling is untouched; when ADK later wires previous_interaction_id for this model, it composes independently.

Known limitations:

  • A reasoning item with several summary segments replays with the first segment as its summary; the encrypted payload carries the full reasoning state, so model continuity does not depend on the remaining text.
  • Sessions persisted before this change hold raw encrypted bytes with no id in thought_signature; those parts stay skipped (no real id to reference).
  • If a caller manually sets LlmRequest.previous_interaction_id (no ADK flow does for this model today), replayed reasoning items are still included in input alongside previous_response_id.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

8 new tests in tests/unittests/integrations/openai/test_openai_responses_llm.py: envelope capture (first-part-only across multiple summary segments, id-less items capture nothing), outbound replay (captured item rebuilt in front of its function call, encrypted-only carrier, id-only stored-item reference, envelope without id stays skipped), a two-turn test through generate_content_async with a mocked client asserting the second request's input carries the reasoning item in position, and a streaming test covering the output_item.done accumulator path. The two pre-existing tests pinning the skip behavior for synthetic parts pass unchanged; the three inbound assertions were updated from raw signature bytes to the envelope payload.

pytest summaries:

  • tests/unittests/integrations/openai/: 299 passed.
  • tox (py311/py312/py313/py314, full suite): 24517 / 24509 / 24509 / 24511 passed. 13 tests fail in every environment — the same network-path tools tests (SSRF/DNS pinning in test_rest_api_tool.py and test_load_web_page.py, plus one MCP google-auth transport test) that fail identically on pristine main in the same virtualenvs (A/B-verified when preparing fix(models): replay OpenRouter reasoning_details in LiteLlm histories #8111); zero failures in integrations/.

Manual End-to-End (E2E) Tests:

  • Red→green: with the source change reverted, the two-turn test fails exactly as reported (the second request's input contains no reasoning item); restored, it passes.
  • To verify against the live endpoint (needs an OpenAI key with a reasoning model, not available locally): run the contributing/samples/models/openai_responses_reasoning agent with a multi-step tool prompt and capture the second request (subclass OpenAIResponsesLlm and log [i.get("type") for i in kwargs["input"]] from _get_response_create_kwargs); with this change each request after a tool call includes the earlier reasoning items in their original positions.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

Sibling fix #8111 addresses the same class of loss in the LiteLlm adapter (OpenRouter reasoning_details); this PR only changes the OpenAI Responses integration.

…ol calls

OpenAIResponsesLlm never sent a reasoning model's reasoning back to
OpenAI: `_content_to_response_input_items` skipped every thought part on
replay (the reasoning item ids were only recorded at response level, and
synthetic ids are rejected), and `previous_response_id` is never set for
this model -- the flow only fills `previous_interaction_id` for the
Gemini interactions API. In every tool loop the model restarted its
reasoning from scratch after each tool call, and with `store: false` the
encrypted reasoning state was lost entirely.

Capture the reasoning item id and encrypted_content in a JSON envelope
on the first thought part of each captured item (thought_signature, the
same slot that already carried the encrypted payload; one envelope per
item, so a rebuilt input never repeats a reasoning id). Rebuild the
`reasoning` input item at the part's original position, in front of the
function call it came with. Parts without a captured envelope --
synthetic thoughts, pre-envelope sessions, other providers' signatures
-- keep the existing skip path unchanged.

Fixes google#8110
A captured reasoning item was replayed whenever it had an item id,
which can send items the Responses API rejects: after the agent
switched models, after a history edit removed the follow-up the
item preceded, or with store disabled and no encrypted payload.
Guard replay on three conditions:

- Same model: the replay envelope now also records the
  response-level model, and a captured item is replayed only for
  the same model family (a trailing -YYYY-MM-DD snapshot suffix
  is ignored, so gpt-5 and gpt-5-2025-08-07 are one family).
  Envelopes without a recorded model -- captured before this
  change, or by a backend that does not report one -- still
  replay, so existing sessions are unaffected.
- Not orphaned: a post-pass over the assembled request input
  drops a replayed reasoning item whose first following
  non-reasoning item is not the function call or assistant
  message it precedes (for example after a history edit removed
  the follow-up, or when a user turn comes next). Consecutive
  reasoning items are one block anchored by whatever follows
  them.
- Resolvable with store=false: the server keeps no reasoning
  state, so an id-only item is dropped with a warning pointing
  at include=['reasoning.encrypted_content']; items that carry
  encrypted_content, and the default store setting, are
  unaffected.

The envelope's new model field is additive: readers of the
previous revision ignore it, and envelopes without it replay as
before.
@Bensonluo

Copy link
Copy Markdown
Author

Thanks @surajksharma07 -- all three points are addressed in abf1e24:

  1. Same-model replay: the capture envelope now records the response-level model next to the item id and encrypted_content, and replay skips an item captured for a different model family (a trailing date-snapshot suffix is ignored, so gpt-5 ↔ gpt-5-2025-08-07 still replay). Envelopes without a recorded model -- sessions captured before this change, or a backend that does not report one -- keep replaying, so nothing regresses for existing sessions.
    Tests: test_replay_skips_reasoning_captured_for_different_model_family, test_replay_keeps_reasoning_for_same_model_alias_and_snapshot, test_replay_keeps_reasoning_without_recorded_model.

  2. Orphaned reasoning items: _get_response_input runs a post-pass over the fully assembled input and drops a reasoning item whose first following non-reasoning item is not the function call or assistant message it precedes -- this covers cross-Content orphans after a history edit and a reasoning item followed by a user turn. Consecutive reasoning items are treated as one block anchored by the shared follow-up.
    Tests: test_get_response_input_drops_orphaned_reasoning_after_history_edit, test_get_response_input_drops_reasoning_item_before_user_turn, test_get_response_input_keeps_consecutive_reasoning_before_call.

  3. store=false: an id-only replayed item cannot be resolved server-side, so it is dropped with a warning that points at include=['reasoning.encrypted_content']; items that carry encrypted_content and the default store setting are unaffected.
    Tests: test_store_false_drops_id_only_reasoning_item, test_store_default_keeps_id_only_reasoning_item.

Verified RED→GREEN: with the production changes stashed the 8 new tests fail as expected; restored, the test file is 96/96 green and the openai integration suite is 307/307. pyink and ruff clean.

Replaying the raw item wholesale (preserving phase, etc.) is left as a follow-up per your suggestion.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpenAIResponsesLlm drops reasoning items between tool calls (never replayed or chained)

2 participants