Skip to content

[Rust Frontend] Reject stream_options with null or unknown keys without streaming like Python - #61

Closed
richard-for-bugen[bot] wants to merge 1 commit into
mainfrom
richard/parity-fix-stream-options-null-keys
Closed

richard-for-bugen[bot] wants to merge 1 commit into
mainfrom
richard/parity-fix-stream-options-null-keys

Conversation

@richard-for-bugen

Copy link
Copy Markdown

Overview

Python's chat and completions validate_stream_options check the raw JSON body: if data.get("stream_options") and not data.get("stream"). With streaming off, any stream_options object that has a key is a 400, even when every value is null or the key is one Python does not model; only {} and null pass. vllm-project#62643 made Rust accept {} like Python, but it decides emptiness on the deserialized options, so {"include_usage": null} and {"include_obfuscation": true} (an OpenAI option neither frontend models) count as empty and are accepted.

The OpenAI API agrees with Python on the all-null case: with stream=false, a live probe returns 400 "The 'stream_options' parameter is only allowed when 'stream' is enabled." for {}, {"include_usage": null} and {"include_usage": false}, and 200 for an absent or null stream_options. This PR keeps Python's acceptance of {} and rejects the rest like both.

stream=false and stream_options Python Rust before Rust after
absent or null 200 200 200
{} 200 200 200
{"include_usage": false} 400 400 400
{"include_usage": null} 400 200 400
{"include_obfuscation": true} 400 200 400

/inference/v1/generate is unchanged: Python's generate route has no stream_options check.

Ownership and layering

The rule is about the raw request object, so it is captured where that object is parsed: StreamOptions (the shared HTTP DTO in routes/openai/utils/types.rs) records at deserialization whether the object had any key, and is_empty reports that. The three existing consumers of is_empty (chat validate_request_compat, chat validate_chat_cross_parameters, completions validate_request_compat) are unchanged. The custom Deserialize reads the object into a JSON map once and derives the two typed fields from it, the same shape as the existing ChatMessage deserializer.

Validation

Commands from .buildkite/scripts/run-rust-frontend-cargo-ci.sh (style-clippy and test), run from the repo root with a per-worktree target dir:

cargo fmt --manifest-path rust/Cargo.toml --all -- --check   # ok
cargo sort --workspace --check rust                          # ok
cargo deny --manifest-path rust/Cargo.toml --config rust/deny.toml check bans   # ok
cargo clippy --manifest-path rust/Cargo.toml --workspace --all-targets --all-features --locked -- -D warnings   # ok
cargo nextest run --manifest-path rust/Cargo.toml --workspace --all-features --locked --no-fail-fast
# 2320 tests run: 2320 passed, 13 skipped
  • Chat and completions validate_request_compat_rejects_stream_options_without_streaming gain {"include_usage": null} and {"include_obfuscation": true}; the chat test runs the request through both chat checks. Both tests fail with the StreamOptions change reverted.
  • validate_request_compat_accepts_empty_stream_options_without_streaming (chat and completions) still pins {}.
  • validate_request_compat_accepts_stream_options_with_streaming (chat and completions) pins that the same null and unknown-key objects pass when stream=true.
  • Two chat convert tests built StreamOptions as struct literals; they now parse it from JSON, since emptiness is a property of the request object.

Python test pinning the behavior: tests/entrypoints/openai/chat_completion/test_chat.py::test_chat_completion_stream_options (not yet in Rust CI; the file has other open Rust gaps).

A wrongly typed option, such as {"include_usage": "yes"}, still fails body deserialization; the typed fields are parsed by a derived Deserialize from the same map.

Codex review (gpt-6.1-sol, high): approve with nits. It confirmed the gap against the Python request models and a temporary HTTP probe (base returns 200 for stream=false with {"include_usage": null}; all 60 probe combinations across chat, completions, both render routes and generate pass at the head).

  • P3, cover the new objects with streaming on, so a deserializer that rejected them outright could not pass: fixed with the accepts_stream_options_with_streaming tests. They parse the whole request from JSON, which is where such a deserializer would fail, so they stay at the validator level instead of the HTTP route tests.

Not a duplicate: no open upstream PR changes this check; vllm-project#61275 adds a new Python option and does not touch the validation.

…ut streaming like Python

Python's chat and completions `validate_stream_options` test the raw JSON
(`if data.get("stream_options") and not data.get("stream")`), so with
streaming off they reject any `stream_options` object with a key, including
`{"include_usage": null}` and keys Python does not model, and accept only
`{}` and `null`. Rust checked the deserialized options, so an all-null or
unknown-key object counted as empty and was accepted.

`StreamOptions` now records at deserialization whether the object had any
key, and `is_empty` reports that.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: richard-for-bugen[bot] <342439978+richard-for-bugen[bot]@users.noreply.github.com>
@BugenZhao

Copy link
Copy Markdown
Owner

It’s ugly to distinguish between absent and explicit NULL…

@BugenZhao BugenZhao closed this Oct 27, 2026
@richard-for-bugen
richard-for-bugen Bot deleted the richard/parity-fix-stream-options-null-keys branch October 27, 2026 05:58
richard-for-bugen Bot added a commit that referenced this pull request Nov 5, 2026
…ust frontend

vllm-project#65625 (n > 1) fixed the last Rust gaps in test_chat.py, so
run it in a new 35 GB step (zephyr-7b-beta, as in the Python
chat_completion step), with three relaxations for differences both
frontends accept:

- the last streamed chunk may carry only the finish_reason, so
  test_chat_streaming no longer requires content on it (the joined text is
  still compared with the non-streaming output), and the continuous-usage
  check lets that chunk repeat the completion-token count, like the
  usage-only chunk;
- the Rust frontend treats an all-null stream_options like an omitted one
  without streaming (decided on #61), so that one case is
  skipped under it;
- test_long_seed also accepts the Rust deserialization message for an
  out-of-range seed (the 400 is still asserted).

test_invocations is deselected: it calls the SageMaker /invocations route,
which the Rust frontend does not serve.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: richard-for-bugen[bot] <342439978+richard-for-bugen[bot]@users.noreply.github.com>
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.

1 participant