Skip to content

fix: stop feeding prior-turn reasoning back into the Responses gateway tool loop#122

Open
ashwing wants to merge 1 commit into
vllm-project:mainfrom
ashwing:fix/messages-reasoning-carryforward
Open

fix: stop feeding prior-turn reasoning back into the Responses gateway tool loop#122
ashwing wants to merge 1 commit into
vllm-project:mainfrom
ashwing:fix/messages-reasoning-carryforward

Conversation

@ashwing

@ashwing ashwing commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes #120.

In a multi-round gateway tool loop (/v1/responses, HTTP or WebSocket), raw <think>...</think> reasoning markup leaks into the client's visible output_text channel — reasoning also streams correctly on its own reasoning_text channel, so it effectively appears twice, once as markup inside the answer.

This affects any client driving the Responses gateway loop with gateway-owned tools (web_search / MCP) over more than one round — Codex, or any /v1/responses caller. It's not client-specific; the trigger is the model's reasoning parser, not the caller. (Note: the /v1/messages path is currently a transparent proxy and does not go through this loop, so it is not affected.)

Root cause: the loop's Continue branch (executor/engine.rs) calls append_output_items_to_input(&mut ctx.enriched_request.input, &current_output), which fed each round's reasoning item back into the next inference round (OutputItem::to_input_item maps Reasoning → InputItem::Reasoning). Reasoning parsers (e.g. Qwen3) then re-emit that prior-turn reasoning wrapped in <think> on the visible text channel.

Reasoning is ephemeral to the turn that produced it — there's no reason to feed it back into subsequent inference rounds. This scopes the fix to the loop carry-forward: append_output_items_to_input now skips Reasoning, carrying forward only messages and tool calls.

Deliberately scoped: to_input_item is also used by the previous_response_id rehydration path (storage/types/item.rs::into_input_items), where reasoning should be preserved. That path is untouched — its test_into_input_items_includes_reasoning still passes. The client-visible/persisted output path (public_output_itemspayload.output) is also untouched, so reasoning is still shown to the client; only what's fed back into inference changes.

Test Plan

  • New unit test carry_forward_drops_reasoning_keeps_messages_and_calls: feeds reasoning + message + function_call through append_output_items_to_input and asserts reasoning is dropped while message/function_call carry forward. Verified it fails on the pre-fix code and passes with the fix.
  • cargo test -p agentic-server-core — 174/174 lib tests pass, including test_into_input_items_includes_reasoning (rehydration still preserves reasoning — no regression).
  • cargo fmt -- --check clean; cargo clippy -p agentic-server-core --all-targets -- -D warnings clean.
  • Root cause localized live (gateway → vLLM Qwen3 on GPU) with layered vLLM-direct tests: vLLM is clean unless a prior reasoning item is present in the input history (single-turn: clean; round-2 with function_call + output but no reasoning: clean; round-2 with reasoning fed back: leaks; round-2 with reasoning excluded: clean — this fix). The committed unit test is the regression guard.

The multi-turn gateway tool loop's Continue branch fed each round's full
output — including the reasoning item — back into the next inference round
via append_output_items_to_input. Reasoning parsers (e.g. Qwen3) then
re-emit that content wrapped in <think> tags on the visible output_text
channel, leaking model reasoning into the answer the client sees.

Reasoning is ephemeral to the turn that produced it; carry forward only
messages and tool calls. Scoped to the loop carry-forward — the
previous_response_id rehydration path (into_input_items) still preserves
reasoning, as its test asserts.

Fixes vllm-project#120

Signed-off-by: Ashwin Giridharan <girida@amazon.com>
@ashwing ashwing changed the title fix: stop feeding prior-turn reasoning back into the gateway tool loop fix: stop feeding prior-turn reasoning back into the Responses gateway tool loop Jul 15, 2026
@ashwing
ashwing marked this pull request as ready for review July 15, 2026 05:14
@jiahuei

jiahuei commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Although current models may only require reasoning for the current turn, it is likely that future models will require prior reasoning turns as well, since that is the progression of frontier closed-sourced models.

  • Claude Opus 4.5 and later Opus models keep all prior thinking blocks. Source
  • OpenAI now has reasoning.context value of all_turns that renders available, compatible reasoning items from earlier turns into the next sample. Source

@ashwing

ashwing commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

@jiahuei good point — blanket-dropping breaks preserve-reasoning models (Opus 4.5, OpenAI all_turns). The leak is actually the plaintext reasoning content being fed back, not the item itself — to_input_item clones the whole ReasoningOutput incl. content into the next inference input, and that's what Qwen3 re-emits as <think>. Validated on vLLM (streaming, Qwen3):

round-2 input <think> leak
reasoning with full content 3/3
reasoning, content stripped (id kept) 0/3
reasoning excluded 0/3

So the fix is: keep the item, drop the plaintext content, keep encrypted_content+id — leak-proof and still round-trips signed reasoning. Same to_input_item also runs on the previous_response_id rehydration path, so the leak exists there too; fixing it at that seam covers both.

cc @maralbahari — this moves into the continuation layer you worked on. Thoughts?

@jiahuei

jiahuei commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

I propose to convert this into a feature PR to support reasoning.context parameter, along with the necessary tests.

context: optional, values: "auto" or "current_turn" or "all_turns"
Controls which reasoning items are rendered back to the model on later turns.

When it is set to current_turn, then we can drop prior reasoning turns when sending back the items to upstream.

@maralbahari

Copy link
Copy Markdown
Collaborator

I propose to convert this into a feature PR to support reasoning.context parameter, along with the necessary tests.

context: optional, values: "auto" or "current_turn" or "all_turns" Controls which reasoning items are rendered back to the model on later turns.

When it is set to current_turn, then we can drop prior reasoning turns when sending back the items to upstream.

I agree with this proposal. @ashwing I think it would be best to stick to the api contract.

Copy link
Copy Markdown
Collaborator

i agree with using reasoning.context, but there’s one distinction i don’t think the current discussion covers: current_turn is not the same as “drop reasoning from every prior inference round.” This function runs inside one Responses request’s gateway tool loop, so the reasoning being filtered here belongs to the active turn. The OpenAI contract says current_turn keeps active-turn reasoning available, and the function-calling guidance recommends preserving all reasoning/call/output items since the last user message. It should only exclude reasoning from earlier API turns during rehydration; same-turn reasoning should survive this loop. See https://developers.openai.com/api/docs/guides/reasoning#keeping-reasoning-items-in-context and https://developers.openai.com/api/docs/guides/reasoning#preserve-reasoning-across-calls.

there’s also an upstream gap: agentic-api currently drops the reasoning object instead of forwarding and returning it, while current vLLM main accepts the object but does not apply context during input rendering; its normal Responses renderer also rejects encrypted_content. So adding the enum, or keeping only encrypted content, won’t fix this end to end. I think this needs either corresponding vLLM support or explicit gateway-side rendering semantics, with separate tests for same-turn tool continuation, previous_response_id + current_turn, and all_turns.

@ashwing

ashwing commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

Agreed on reasoning.context as the contract, and @franciscojavierarceo you're right on the same-turn distinction — this loop is one Responses request, so the reasoning it carries is active-turn and current_turn says to keep it.

I dug into where context actually needs to be applied. Tested on vLLM 0.25.1 (Qwen3):

  • vLLM accepts and enum-validates reasoning.context (auto/current_turn/all_turns → 200; bogus → 400), but doesn't apply it on input: with a prior reasoning item, <think> leaks identically for current_turn, all_turns, and no field (3/3 each). So passing context through is a no-op today.
  • vLLM rejects encrypted_content on input (400 "Encrypted content is not supported."), so we can't rely on the encrypted path upstream yet.
  • On our side, RequestPayload has no reasoning field, so step one regardless is parsing/forwarding/returning the object.

The leak is fundamentally a vLLM parser behavior — fed-back reasoning gets re-verbalized as <think> on the visible channel. So I'd propose fixing it at the source: land the context handling in vLLM's Responses input renderer (the seam is entrypoints/openai/responses/harmony.py, which renders reasoning items to input, and the reject/render policy at utils.py:273) so current_turn keeps same-turn reasoning and drops earlier-turn without re-verbalizing, and the gateway just forwards the field. That fixes it for every client of the model, not just our path, and keeps the gateway a clean pass-through. Happy to take that vLLM PR.

Alternative if we want the leak closed before that ships: an interim gateway-side strip of same-turn reasoning text (scoped to plaintext only — no encrypted_content), which stops the leak now and self-deactivates once vLLM handles context, so removing it later is trivial. The cross-turn/rehydration handling is the same either way.

Either works for me — I lean on the vLLM fix since it's the real root cause. Your thoughts?

@ashwing

ashwing commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

@jiahuei @franciscojavierarceo @maralbahari Thoughts?

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.

[SSE Completion - Issue 2] Prior-turn reasoning leaks <think> into the visible answer

4 participants