Skip to content

[Bugfix][Responses API] Reuse streamed item ids in final harmony response - #59859

Merged
sfeng33 merged 2 commits into
vllm-project:mainfrom
he-yufeng:fix/harmony-completed-id-stability
Oct 4, 2026
Merged

sfeng33 merged 2 commits into
vllm-project:mainfrom
he-yufeng:fix/harmony-completed-id-stability

Conversation

@he-yufeng

@he-yufeng he-yufeng commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Fixes the harmony (gpt-oss) part of #59834; #59307 fixed the non-harmony path. When /v1/responses streams, response.completed rebuilds its output from the harmony messages with harmony_to_response_output. That rebuild mints a new item id and function call_id, which no longer match the ones sent in response.output_item.added/done, so strict clients abort.

  • _process_harmony_streaming_events records, for each completed harmony message, the items sent in its output_item.done events (HarmonyContext.streamed_items_by_message).
  • responses_full_generator matches each message to its own record by object identity. reuse_streamed_item_ids then copies id/call_id onto that message's rebuilt items by type. Content and incomplete status still come from the rebuild. Items that were never streamed (zero-delta) keep a new id.
  • Streamed harmony reasoning items now use the rs_ prefix instead of msg_, matching the non-streaming path and the non-harmony path.

Matching per message matters. A built-in python call streams as code_interpreter_call but is rebuilt as reasoning. If ids were matched across messages, that call would take the streamed id of the next reasoning item.

Testing

vllm serve openai/gpt-oss-20b --no-enable-prefix-caching (1x H200), then the request from the issue:

curl -sN localhost:8000/v1/responses -H 'Content-Type: application/json' -d '{
  "model": "openai/gpt-oss-20b", "stream": true,
  "input": "Call the get_weather tool for Paris. You must call the tool now.",
  "tools": [{"type": "function", "name": "get_weather", "parameters": {"type": "object",
    "properties": {"city": {"type": "string"}}, "required": ["city"]}}]
}' | sed -n 's/^data: //p' | jq -c '
  if .type == "response.output_item.done" then ["done", .item.type, .item.id, .item.call_id]
  elif .type == "response.completed" then .response.output[] | ["completed", .type, .id, .call_id]
  else empty end'

main (5f30fc7): completed changes every id.

["done","reasoning","msg_832f7cd8a3888df4",null]
["done","function_call","fc_8019379c1a695c28","call_b07a9991be290d4c"]
["completed","reasoning","rs_b5c9be54d08ca702",null]
["completed","function_call","fc_bc041d3276faf22e","call_bc041d3276faf22e"]

This PR: completed keeps the streamed ids.

["done","reasoning","rs_a972e97aaf1f0eb1",null]
["done","function_call","fc_be95d8cf99657077","call_84160cffacc40ff2"]
["completed","reasoning","rs_a972e97aaf1f0eb1",null]
["completed","function_call","fc_be95d8cf99657077","call_84160cffacc40ff2"]

I ran the same check, 3 times each, on a forced tool call, a reasoning + text answer, and a max_output_tokens=40 cut-off (incomplete). Results: main had id mismatches in 9/9 runs, and this PR in 0/9.

Other tests:

  • tests/entrypoints/openai/responses/test_serving_responses.py: 64 passed. The 2 new tests fail on main.
  • tests/entrypoints/openai/responses/test_harmony.py (gpt-oss-20b e2e): 37 passed, 2 skipped, 1 xpassed on both main and this PR.

No change to generated content: only item ids change, so no eval is needed.

Pull Request Checklist
  • I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).

  • AI assistance was used during the creation of this PR.

  • Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.

  • Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.

  • Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.

  • Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

…onse

When streaming with gpt-oss (harmony), responses_full_generator rebuilds
the output items from the parsed harmony messages and mints fresh ids, so
the response.completed event disagrees with the streamed
output_item.added/done events. The simple path already reuses the streamed
items (vllm-project#59307); do the equivalent for harmony by collecting the done items
during streaming and overlaying their ids onto the rebuilt output, matched
by (type, name) in order. Items without a streamed counterpart (zero-delta
items whose done event is suppressed) keep their fresh id and stay in the
output.

Fixes vllm-project#59834

Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added frontend gpt-oss Related to GPT-OSS models bug Something isn't working labels Oct 3, 2026
Replace the cross-message (type, name) scan with per-message pairing.
The harmony stream now records the items sent in output_item.done for
each completed message, and the final response reuses their id/call_id
only for the rebuilt items of that same message. A built-in python call
(streamed as code_interpreter_call, rebuilt as reasoning) no longer
takes the streamed id of the next reasoning item.

Streamed harmony reasoning items now use the rs_ id prefix, matching
the non-streaming and non-harmony paths.

Signed-off-by: sfeng33 <4florafeng@gmail.com>

@sfeng33 sfeng33 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @he-yufeng for tracking down the harmony path. I pushed a commit that switches the id overlay from a (type, name) scan across all messages to per-message pairing. The scan can give an item another item's id: a built-in python call streams as code_interpreter_call but is rebuilt as reasoning, so it took the streamed id of the next reasoning item. Each completed harmony message now records its own output_item.done items, and only that message's rebuilt items reuse those ids.

@sfeng33

sfeng33 commented Oct 4, 2026

Copy link
Copy Markdown
Member

/ci run --allow-stale

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #92773 for commit 1d595d05d080.

⚠️ This PR is 6 commits behind upstream main. Running CI at your own risk because --allow-stale was requested; outdated CI configuration may cause failures. Before merging, merge or rebase onto the latest main, then rerun /ci run on the latest PR commit.

@sfeng33
sfeng33 enabled auto-merge (squash) October 4, 2026 01:10
@github-actions github-actions Bot added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 4, 2026
@vllm-agent

Copy link
Copy Markdown
Collaborator

CI selector (shadow): 3 test steps (3 jobs) instead of 38 (51 jobs)

Shadow mode: this changes nothing about what CI runs. It shows what the evidence-based selector would pick for this PR, next to today's rules. How it works.

Feedback welcome: reply here if it would skip a step this change needs, or runs something unrelated.

steps (jobs) Today's rules Selector Would skip Would add
NVIDIA, CPU and others 38 (51) 3 (3) 35 (48) 0 (0)
AMD mirrors 37 (45) 3 (3) 34 (42) 0 (0)
Selector would run (3)
  • entrypoints-integration-multimodal
  • entrypoints-integration-responses-api
  • entrypoints-unit-tests
Would skip (today's rules run them) (35)
  • basic-correctness ×2
  • basic-correctness-cpu-offload
  • basic-correctness-cumem
  • basic-correctness-prefetch-offload
  • basic-correctness-sleep-mode
  • basic-models-test-other-cpu
  • basic-models-tests-initialization
  • basic-models-tests-other
  • benchmarks-cli-test
  • cpu-language-generation-and-pooling-model-tests ×3
  • entrypoints-integration-api-server ×4
  • entrypoints-integration-api-server-generate
  • entrypoints-integration-api-server-openai-chat_completion
  • entrypoints-integration-api-server-openai-completion
  • entrypoints-integration-llm
  • entrypoints-integration-pooling
  • entrypoints-integration-speech_to_text
  • examples
  • kernels-fla-ops-test-b200
  • kernels-mhc-test-b200
  • kernels-root-misc-test-b200
  • language-models-tests-granite-l4-compatibility
  • language-models-tests-hybrid ×2
  • language-models-tests-standard
  • multi-modal-models-standard-1-qwen2
  • multi-modal-models-standard-2-qwen3-gemma
  • multi-modal-models-standard-3-llava-qwen2-vl
  • multi-modal-models-standard-4-other-whisper
  • multi-modal-processor ×4
  • multi-modal-processor-cpu ×4
  • openai-api-correctness
  • rust-frontend-core-correctness
  • rust-frontend-openai-coverage
  • rust-frontend-serve-admin-coverage
  • rust-frontend-tool-use
Would add (today's rules do not run them) (0)

none

AMD mirrors: would skip (34)
  • basic-correctness ×2
  • basic-correctness-cpu-offload
  • basic-correctness-cumem
  • basic-correctness-prefetch-offload
  • basic-correctness-sleep-mode
  • basic-models-tests-initialization
  • basic-models-tests-other
  • benchmarks-cli-test
  • entrypoints-integration-api-server ×4
  • entrypoints-integration-api-server-generate
  • entrypoints-integration-api-server-openai-chat_completion
  • entrypoints-integration-api-server-openai-completion
  • entrypoints-integration-llm
  • entrypoints-integration-pooling
  • entrypoints-integration-speech_to_text
  • examples
  • kernels-fla-ops-test-b200
  • kernels-mhc-test-b200
  • kernels-root-misc-test-b200
  • language-models-tests-granite-l4-compatibility
  • language-models-tests-hybrid ×2
  • language-models-tests-standard
  • multi-modal-models-standard-1-qwen2
  • multi-modal-models-standard-2-qwen3-gemma
  • multi-modal-models-standard-3-llava-qwen2-vl
  • multi-modal-models-standard-4-other-whisper
  • multi-modal-processor ×4
  • openai-api-correctness
  • platform-tests
  • pytorch-compilation-passes-unit-tests
  • rust-frontend-core-correctness
  • rust-frontend-openai-coverage
  • rust-frontend-serve-admin-coverage
  • rust-frontend-tool-use
AMD mirrors: would add (0)

none

5 changed files · base 5f30fc7031 · head 1d595d05d0 · Python record: build 92769 at 84bcbc6264 · kernel record: table 84bcbc6 (build 92769), map 84bcbc6 · not counted: 10 build steps, 5 A100 steps the generator no longer emits, 13 optional steps the selector would also run

@sfeng33
sfeng33 merged commit 6f75cc7 into vllm-project:main Oct 4, 2026
101 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working frontend gpt-oss Related to GPT-OSS models ready ONLY add when PR is ready to merge/full CI is needed

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants