[Bugfix][Frontend] Truncate pooling prompts before padding them - #54364
Conversation
`_validate_tokens` padded to the full input length before applying `truncate_prompt_tokens`. With `truncation_side="left"` the truncation then kept only the pad tokens that padding had just appended, so the request succeeded and the model embedded a prompt with no content in it. Run truncation first, matching the Transformers pipeline these parameters are named after. Signed-off-by: hotragn <hotragn.pettugani_2024@woxsen.edu.in>
DarkLight1337
added
the
ready
|
@claude review |
|
✅ @Hotragn, CI is now available for this PR.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟣
vllm/entrypoints/pooling/scoring/io_processor.py— The diff reorders truncate-then-pad in TokenizeParams._validate_tokens, but the duplicate pad/truncate logic here for token_type_ids (used for cross-encoder query/doc segment boundaries) still pads before truncating. For scoring/rerank requests combiningpadding+ lefttruncate_prompt_tokens, prompt_token_ids and token_type_ids are now computed with different orders, so the segment boundary passed to the model via compress_token_type_ids/compressed_token_type_ids no longer matches the actual (now-correct) token positions -- previously both were wrong the same way and stayed aligned; now they silently diverge, corrupting cross-encoder scores. …Extended reasoning...
…Fix: make _apply_post_tokenization_to_token_type_ids apply truncation before padding, mirroring the corrected order in TokenizeParams._validate_tokens.
io_processor.py:541 calls tok_params.apply_post_tokenization (params.py, new order: truncate then pad) on prompt_token_ids. io_processor.py:543-549 then calls _apply_post_tokenization_to_token_type_ids (still pad-then-truncate, lines 58-83) on the parallel token_type_ids array using the same tok_params. Example: query+doc real content 50 tokens, pad_prompt_tokens=-1 (pad to max_input_tokens=100), truncate_prompt_tokens=5, truncation_side=left. prompt_token_ids: truncate to last 5 real tokens, then pad to 100 -> last 5 are real content, rest pad. token_type_ids: pad first to 100 (appending copies of the last real type id, e.g. 1), then left-truncate to last 5 -> all become the pad-appended value (all 1s). compress_token_type_ids then returns first_one=0 for the token_type array, meaning the reconstructed torch.arange(len(prompt_token_ids)) >= 0 marks every one of the 100 final tokens (including the 5 real ones and the doc-side…
Verification: Severity: pre-existing (real divergence introduced, but the affected scenario already produced wrong output on base by the same route — not merge-blocking). The candidate's core technical claim is correct. After the diff,
TokenizeParams._validate_tokensruns truncate-then-pad (params.py:469-474), but the parallel handler for cross-encoder segment ids,… | pre-existing severity — real, reachable…
|
PTAL at @claude 's comment |
`_apply_post_tokenization_to_token_type_ids` padded before truncating, so it no longer matched `TokenizeParams._validate_tokens`. The two arrays are parallel and the compressed boundary is what tells the model where the document starts, so a scoring request combining `padding` with left `truncate_prompt_tokens` reported an empty query segment. Signed-off-by: hotragn <hotragn.pettugani_2024@woxsen.edu.in>
|
Thanks — I checked that finding and it is correct, so I've pushed a fix (
That matters because
10 is right: left-truncating to 40 drops the first 10 query tokens, so 10 query The fix reorders the helper to truncate-then-pad, matching Test result (ubuntu-24.04 x86_64 / Python 3.12.3, Against On this branch (ac5d07e):
Happy to split the |
|
/ci run |
|
✅ Triggered Buildkite CI #86210 for commit |
|
Build 86210 on It is not this diffThis PR only swaps the order of What actually failedThe generated body includes
No fuzzing needed to reproduce -- a plain space has the same effect: curl -X POST http://localhost:8000/v1/chat/completions \
-H 'Content-Type: application/json' \
-d '{"model": "<model>", "messages": [{"role": "user", "content": "hi"}], "bad_words": [" "]}'It is already being fixed elsewhere -- #53433, #47841 and #49898 all target this On retryingThe |
…-project#54364) Signed-off-by: hotragn <hotragn.pettugani_2024@woxsen.edu.in>
…-project#54364) Signed-off-by: hotragn <hotragn.pettugani_2024@woxsen.edu.in>
…-project#54364) Signed-off-by: hotragn <hotragn.pettugani_2024@woxsen.edu.in> (cherry picked from commit 687db59)
…-project#54364) Signed-off-by: hotragn <hotragn.pettugani_2024@woxsen.edu.in>
3 participants
Purpose
TokenizeParams._validate_tokensruns its validators in this order:Padding runs before truncation.
_token_paddingappends pad tokens up tomax_input_tokens, and_token_truncationwithtruncation_side="left"thenkeeps only the last N tokens — which are precisely the pad tokens that padding
just appended. The prompt's actual content is discarded, the request succeeds,
and the model embeds nothing but padding. No error, no warning.
paddingis documented as "using the same names as the Transformers tokenizer",but the Transformers pipeline truncates and then pads
(
pad_truncation).Reachability
paddingbecame settable over HTTP in #51157 (merged 2026-08-27).PoolingBasicRequestMixin(vllm/entrypoints/pooling/base/protocol.py:41-63)exposes
truncate_prompt_tokens,paddingandtruncation_sideon the samerequest model, and routes
paddingthroughtok_params.with_kwargs(padding=self.padding)→pad_prompt_tokens = max_length(
vllm/renderers/params.py:273-277). All three are therefore settable on onerequest:
With
max_model_len=512: the tokenizer returns ~3 ids →_token_paddingextends to 512 →
_token_truncation(left) returnstokens[-8:]→ 8 padtokens, zero content tokens →
_token_len_checkpasses → 200 OK with anembedding of pure padding.
truncation_sidedoes not have to be sent explicitly:_token_truncationfallsback to
tokenizer.truncation_side(params.py:428-430), so any tokenizerwhose own default is
"left"hits this with justpadding+truncate_prompt_tokens.This is also why it is easy to miss — with
truncation_side="right"the resultcoincidentally matches Transformers, so the bug only shows on the left side.
Approach
Run
_token_truncationbefore_token_padding, matching the Transformerspipeline the parameters are named after. Truncation then applies to real
content, and padding fills out the remainder.
The reordering is safe with respect to
_token_len_check, which still runslast:
_token_paddingpads to at mostmax_input_tokens, and__post_init__already rejects
truncate_prompt_tokens > max_input_tokens(
params.py:239-251), so neither step can push the sequence past the limit.Test Plan
Two tests added to the existing
TestRenderPromptclass:test_padding_with_left_truncation_keeps_the_prompt— the regression. Thesentinel is
set(token_ids) != {pad_id}: it asserts the prompt is not madeentirely of pad tokens, which is what proves the degenerate value reached
the consumer, rather than asserting that some exception was raised (nothing
raises here — that is the point).
DummyTokenizer.pad_token_idis 99999,deliberately outside the range of ids
encodereturns, so a pad token cannever be confused with a real one.
test_padding_without_truncation_is_unchanged— pins the behaviour ofpadding on its own, so the reorder cannot silently change the case [Bugfix][Frontend] Let pooling requests set padding #51157
added.
The existing
tests/entrypoints/pooling/embed/test_online.py::test_paddingonly checks
usage.prompt_tokensgrows and never combinespaddingwithtruncate_prompt_tokens, so this interaction was untested.Test Result
Run on ubuntu-24.04 x86_64 / Python 3.12.3, installed with
VLLM_USE_PRECOMPILED=1 uv pip install -e . --torch-backend=auto.Before — the new tests against
vllm/renderers/params.pyfrommain(4fc943b). The prompt is 100% pad tokens:
After — this branch, whole file:
Every pre-existing test in the file — including all four truncation-side tests —
passes unchanged, which is the evidence that the reorder does not alter
truncation-only or padding-only behaviour.
pre-commithooks applicable to the two changed files —ruff-check,ruff-format,typos,check-spdx-header— pass.Not fixed here
_token_paddingusestokenizer.pad_token_idunguarded, so a tokenizer with noconfigured pad token would splice
Noneintoprompt_token_ids. I could notidentify a commonly served pooling model whose tokenizer reports
pad_token_id is None, so I have left it alone rather than add a guard for apath I cannot demonstrate. Happy to add it if maintainers want it covered.
AI assistance was used to research and draft this change. I have reviewed every
changed line and run the tests above.
Follow-up commit
ac5d07e1a— keeptoken_type_idsalignedvllm/entrypoints/pooling/scoring/io_processor.pykeeps a second copy of thepad/truncate logic for the
token_type_idsarray that runs parallel toprompt_token_idson cross-encoder scoring requests(
_apply_post_tokenization_to_token_type_ids). It also padded beforetruncating, so once
_validate_tokenswas reordered the two arrays no longerdescribed the same positions.
compress_token_type_idsreduces that array to one index — where the documentsegment begins — and the model applies it to
prompt_token_ids. With a20-token query + 30-token document,
paddingto 100,truncate_prompt_tokens=40and
truncation_side="left":len(prompt_token_ids)len(token_type_ids)ac5d07e1aac5d07e1a10 is correct: left-truncating to 40 drops the first 10 query tokens, so 10
query tokens survive and the document starts at index 10. A boundary of 0 tells
the cross-encoder the query segment is empty.
tests/entrypoints/pooling/scoring/test_io_processor_unit.pyasserts both thematching lengths and the boundary. The boundary assertion is the load-bearing
one — the length check alone passes on
main, where both arrays are wrong inthe same way (
assert 0 == 10is the failure onmain).