Repository navigation
[Responses] Normalize OpenResponses input items to match openai-python SDK requirements - #35508
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a normalization step for the Responses API input items, specifically addressing scenarios where assistant messages and reasoning items might omit id and status fields. The changes ensure that these items are accepted by populating sensible default values while preserving any existing id or status values provided by clients. New test cases have been added to validate this behavior, ensuring robustness in multi-turn agent scenarios.
chaunceyjiang
left a comment
chaunceyjiang
left a comment
There was a problem hiding this comment.
Can you write an e2e test? Mainly to align with the OpenAI spec.
chaunceyjiang
added
the
ready
anencore94
changed the title
d1bc512 to
285c05b
Compare
|
Hi @anencore94, the pre-commit checks have failed. Please run: uv pip install pre-commit
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
1 similar comment
|
Hi @anencore94, the pre-commit checks have failed. Please run: uv pip install pre-commit
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
91d4c32 to
c6c4529
Compare
Additional issues discovered during E2E testingWhile adding E2E tests ( 1.
|
c6c4529 to
7e34164
Compare
|
Rebased on latest main, resolved merge conflicts, and fixed the failing |
|
Gentle ping — all CI checks are green (47/47 pass). Would appreciate a re-review when you get a chance. @qandrew @DarkLight1337 @chaunceyjiang |
chaunceyjiang
left a comment
chaunceyjiang
left a comment
There was a problem hiding this comment.
Thanks~
@qandrew PTAL.
|
PTAL 🙇 @chaunceyjiang @qandrew |
|
@anencore94 Thank you for your contribution. I was also hitting the same error with Codex <-> vLLM settings and was looking for a way to fix it. I tested this branch and confirmed that Codex multi-turn conversations now work well. While testing, I noticed a related improvement that might be worth considering, especially around basic skill/tool-call cases. Codex client sent a I do not know why there was an empty content though. That item appeared right before a tool call: So this looks like a tool-calling assistant turn where the assistant produced no visible text, only a tool call. That is a valid shape to tolerate. vLLM crashed because vllm/entrypoints/openai/responses/utils.py:214 assumed every item.content[0].textWhen So I added a fallback in diff --git a/vllm/entrypoints/openai/responses/utils.py b/vllm/entrypoints/openai/responses/utils.py
index 0988c5358..7bab1e683 100644
--- a/vllm/entrypoints/openai/responses/utils.py
+++ b/vllm/entrypoints/openai/responses/utils.py
@@ -211,6 +211,11 @@ def _construct_single_message_from_response_item(
"reasoning": reasoning,
}
elif isinstance(item, ResponseOutputMessage):
+ if not item.content:
+ return {
+ "role": "assistant",
+ "content": "",
+ }
return {
"role": "assistant",
"content": item.content[0].text,This may be out of scope for the current PR, so I’m happy to open a separate issue by myself if that would be preferable. ++) 
|
|
i have same problem. when i use codex desktop with own qwen3.6, if i choose any skill example Skill Installer bababab, it return like this . and qwen log put: after that, all ask will be failed, qwen always log indexError. please help me to deal it. thanks. |
|
This pull request has merge conflicts that must be resolved before it can be |
|
Resolved merge conflicts with the latest PR-targeted local tests all green (93 passed):
Pre-commit hooks (ruff-check, ruff-format, mypy, etc.) all pass on changed files. PTAL when you have a chance @chaunceyjiang @qandrew @DarkLight1337. Thanks! |
|
@TheDuyIT good catch, and apologies for the very slow reply — you were right. The dict-form Also rebased onto the latest
@chaunceyjiang this should also settle your earlier question: the PR still routes everything through I'll re-push shortly with the sign-off trailer restored on the squashed commit (DCO is red purely because of that, not the content). PTAL when either of you has a chance — this was blocking Responses-API adoption for at least one other team in this thread. |
70c46fa to
01ca842
Compare
…n SDK requirements
Requests that replay a prior OpenResponses turn as input fail validation
because items produced by the SDK omit fields that vLLM's models require:
assistant messages arrive without `id`/`status`, reasoning items without a
`summary` default, and `output_text` parts without `annotations`. The result
is a ValidationError on otherwise well-formed conversation history.
Extract the per-item defaulting into a `_normalize_openresponses_input_item`
helper and route `input_item_parsing` through it, so the entry point stays the
same. Also handle the dict-form `reasoning` and `assistant` items that the
TypedDict union parsing can produce (materializing Pydantic
`ValidatorIterator` fields before indexing), and stop
`_construct_message_from_response_item` raising `IndexError` when an assistant
turn carries no content because it only emitted a tool call.
The dict-form branches follow the same previous-assistant-message merge
contract as their model-form counterparts, so a reasoning item followed by an
assistant message merges into one turn rather than emitting two consecutive
assistant turns.
`type` is optional in the OpenResponses schema, so the helper stamps back the
type it keyed off before filling defaults. Without that, a typeless assistant
item gains `id`/`status` and then matches `ItemReference` in the input union,
collapsing to `{"id": ...}` and silently dropping the assistant turn — where
unpatched vLLM had rejected the same payload outright.
Signed-off-by: Jaeyeon Kim <anencore94@gmail.com>
01ca842 to
17660e9
Compare
|
Rebased onto current
That is worse than the bug this PR fixes. Unpatched The fix is one line — stamp back the type the helper already decided — plus four regression tests in I also corrected the commit message, which named @chaunceyjiang @TheDuyIT — no change to the behaviour you each reviewed; this only stops the normalization path from silently discarding a turn. |
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title names the main change: normalizing OpenResponses input items so incomplete assistant and reasoning history is accepted. The objectives describe the same goal, including the reuse of input_it… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Description check | |
The pull request description directly explains the Responses API history normalization changes, empty assistant content handling, related validation behavior, tests, and known testing limits. |
Full details: Title check
-
Explanation
The title names the main change: normalizing OpenResponses input items so incomplete assistant and reasoning history is accepted. The objectives describe the same goal, including the reuse of input_item_parsing and the dict and typed-item history handling in the diff. The title is long at 87 characters, but it is specific and on topic.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Assisted-by: OpenAI Codex Signed-off-by: Jaeyeon Kim <anencore94@gmail.com>
|
/ci run |
|
✅ Triggered Buildkite CI #97292 for commit ℹ️ This PR is 17 commits behind upstream |
|
@coderabbitai full review |
✅ Action performed
|
|
@chaunceyjiang could you re-review commit The full CodeRabbit review on this head found no actionable correctness issues; it still reports a docstring-coverage warning (55.56% against its 80% threshold). The implementation reuses |
chaunceyjiang
left a comment
chaunceyjiang
left a comment
There was a problem hiding this comment.
Thanks! I'll take a look at this PR in a couple of days.
|
@chaunceyjiang following up on Could you take another look and, if this resolves your concerns, approve the revision for merge? The 182 local CPU tests pass. Local history-replay E2E coverage remains unrun, as recorded in the PR body |
chaunceyjiang
left a comment
chaunceyjiang
left a comment
There was a problem hiding this comment.
make sense.
Thanks~
Overview
Accept incomplete assistant and reasoning history in Responses API requests without losing turns or crashing on empty assistant content. Related to #33089 and openai/codex#12669.
Claims
input_item_parsingvalidator, preserving supplied metadata and non-assistant inputs.Validation
At 35b420c, after the CI bot merged current main, these CPU tests passed on macOS/Python 3.12:
Buildkite #97292 completed successfully on this head; GitHub reports 69 passing checks and three skipped. The existing history replay E2E tests in
test_simple.pywere not run locally; the aggregate CI result does not imply every test ran. No model-quality or latency improvement is claimed. The full CodeRabbit review on this head found no actionable correctness issues, with a remaining docstring-coverage warning (55.56% against its 80% threshold).Details
The validator normalizes raw dictionaries before SDK object construction. The conversion changes handle typed or dict-form history later, including empty assistant content. This follows the review suggestion to reuse
input_item_parsing; it does not add a parallel entry point. The empty-content and dict/model merge-policy reports in this discussion are addressed in the same history-conversion path.The latest main integration retained upstream tool-schema validation. This continues the existing PR and its issue scope rather than opening a competing implementation. AI assistance was used for implementation, upstream integration, tests and this description.
Pull Request Checklist
/pr-checklistskill.