Skip to content

[Rust Frontend] Report every invalid logit_bias key like Python - #32

Closed
richard-for-bugen[bot] wants to merge 2 commits into
mainfrom
richard/parity-fix-logit-bias-all-invalid-keys
Closed

richard-for-bugen[bot] wants to merge 2 commits into
mainfrom
richard/parity-fix-logit-bias-all-invalid-keys

Conversation

@richard-for-bugen

@richard-for-bugen richard-for-bugen Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Motivation

Python. SamplingParams.from_optional (vllm/sampling_params.py) converts the OpenAI logit_bias keys to integer token IDs. When any key fails, it collects every bad key and raises one VLLMValidationError (400, param="logit_bias") naming all of them: logit_bias contains key(s) that cannot be converted to integer token IDs: ['bad1', 'bad2']. tests/entrypoints/openai/chat_completion/test_chat_logit_bias_validation.py::test_chat_logit_bias_multiple_non_integer_keys pins this: it sends {"bad1": 50.0, "bad2": 20.0} and asserts that both keys appear in the 400.

Rust today. convert_logit_bias (rust/src/server/src/utils.rs), used by both chat and completions lowering, stops at the first key that fails to parse. The request map is a HashMap, so which key gets reported is arbitrary, and the others are never mentioned. That Python test fails under the Rust frontend.

This PR. convert_logit_bias collects every key that is not a u32 and returns one 400 with param = "logit_bias", sorting the keys so the message is stable: logit_bias contains key(s) that are not valid token IDs: ["bad1", "bad2"]. Token IDs must be non-negative integers.

Ownership and layering. The string keys only exist in the OpenAI JSON wire format. gRPC already carries a map<uint32, float>, and vllm-text takes HashMap<u32, f32>. The HTTP DTO lowering in vllm-server is therefore the only consumer of string keys, and the fix stays there. No new helper: the existing function now collects errors instead of short-circuiting.

Intentional difference. A negative key such as "-1" passes Python's int() and is then rejected by the out-of-vocabulary check with a different message. Rust's token IDs are u32, so Rust reports "-1" together with the other invalid keys. Both return a 400 with param = "logit_bias"; this PR keeps that existing difference.

Item key Category Python change Consumer trace Rust action
chat+completions.logit_bias_report_all_invalid_keys 4 (behavior) SamplingParams.from_optional logit_bias conversion vllm/sampling_params.py from_optional, reached from the chat and completions to_sampling_params fix in convert_logit_bias

The second commit (10640e2, per review) drops -k "not multiple" from the Rust OpenAI Coverage step, so test_chat_logit_bias_validation.py::test_chat_logit_bias_multiple_non_integer_keys runs in Rust CI with this fix. With this branch's vllm-rs on one GB200 GPU, all four tests of the file pass.

Validation

  • Unit test utils::tests::convert_logit_bias_reports_all_invalid_keys uses keys bad2, 7, bad1 and -1, and snapshots the error, which names -1, bad1 and bad2 in sorted order. Red-green check: with the old function restored, the test fails because the error names only '-1', the first key the map yields. convert_logit_bias_converts_valid_keys covers the success path of the rewritten loop.
  • CI command set from .buildkite/scripts/run-rust-frontend-cargo-ci.sh, run in a per-worktree target dir:
    • cargo fmt --manifest-path rust/Cargo.toml --all -- --check
    • cargo sort --workspace --check rust
    • cargo deny --manifest-path rust/Cargo.toml --config rust/deny.toml check bans
    • cargo clippy --manifest-path rust/Cargo.toml --workspace --all-targets --all-features --locked -- -D warnings
    • cargo nextest run --manifest-path rust/Cargo.toml --workspace --all-features --locked --no-fail-fast
    • Result: all green; nextest 2296 passed, 13 skipped.

Codex review

Independent Codex review (gpt-6.1-sol, high effort): approve, with no findings and no nits. Codex confirmed both HTTP lowering paths (chat and completions, including their render endpoints) as the only consumers of string keys. It reproduced the red-green result itself, and an in-process HTTP probe of chat and completions, streaming and non-streaming, failed on origin/main and passed with this change (400, param="logit_bias", every key in sorted order).

Python's SamplingParams collects every logit_bias key that is not an
integer and names all of them in one 400. Rust stopped at the first bad
key in HashMap order, so the reported key was arbitrary and the others
were hidden. Collect them all and sort them for a stable message.

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 BugenZhao left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Unblocks test_chat_logit_bias_validation.py::test_chat_logit_bias_multiple_non_integer_keys for the Rust-frontend e2e CI once this merges (tracked in the e2e coverage ledger).

Please either unblock in this PR or add to the run PR now

…inst the Rust frontend

test_chat_logit_bias_multiple_non_integer_keys asserts that the 400 names
every invalid key, which the Rust frontend now does.

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>
@richard-for-bugen

Copy link
Copy Markdown
Author

Unblocked in this PR (10640e2): the Rust OpenAI Coverage step now runs test_chat_logit_bias_validation.py without -k "not multiple". With this branch's vllm-rs on one GB200 GPU (Slurm job 38739), all four tests pass, including test_chat_logit_bias_multiple_non_integer_keys. The e2e run PR (#30) no longer adds a comment on that line, so the two do not conflict.

@richard-for-bugen

Copy link
Copy Markdown
Author

Promoted as vllm-project#63313.

@richard-for-bugen

Copy link
Copy Markdown
Author

Merged upstream as vllm-project#63313.

@richard-for-bugen
richard-for-bugen Bot deleted the richard/parity-fix-logit-bias-all-invalid-keys branch October 25, 2026 19:18
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