fix: stop feeding prior-turn reasoning back into the Responses gateway tool loop#122
fix: stop feeding prior-turn reasoning back into the Responses gateway tool loop#122ashwing wants to merge 1 commit into
Conversation
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>
|
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. |
|
@jiahuei good point — blanket-dropping breaks preserve-reasoning models (Opus 4.5, OpenAI
So the fix is: keep the item, drop the plaintext cc @maralbahari — this moves into the continuation layer you worked on. Thoughts? |
|
I propose to convert this into a feature PR to support
When it is set to |
I agree with this proposal. @ashwing I think it would be best to stick to the api contract. |
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 visibleoutput_textchannel — reasoning also streams correctly on its ownreasoning_textchannel, 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/responsescaller. It's not client-specific; the trigger is the model's reasoning parser, not the caller. (Note: the/v1/messagespath is currently a transparent proxy and does not go through this loop, so it is not affected.)Root cause: the loop's
Continuebranch (executor/engine.rs) callsappend_output_items_to_input(&mut ctx.enriched_request.input, ¤t_output), which fed each round's reasoning item back into the next inference round (OutputItem::to_input_itemmapsReasoning → 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_inputnow skipsReasoning, carrying forward only messages and tool calls.Deliberately scoped:
to_input_itemis also used by theprevious_response_idrehydration path (storage/types/item.rs::into_input_items), where reasoning should be preserved. That path is untouched — itstest_into_input_items_includes_reasoningstill passes. The client-visible/persisted output path (public_output_items→payload.output) is also untouched, so reasoning is still shown to the client; only what's fed back into inference changes.Test Plan
carry_forward_drops_reasoning_keeps_messages_and_calls: feeds reasoning + message + function_call throughappend_output_items_to_inputand 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, includingtest_into_input_items_includes_reasoning(rehydration still preserves reasoning — no regression).cargo fmt -- --checkclean;cargo clippy -p agentic-server-core --all-targets -- -D warningsclean.reasoningitem 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.