Skip to content

[Bugfix]:Cached_tokens always returns -1 - #229

Open
chasted wants to merge 2 commits into
vllm-project:mainfrom
chasted:main
Open

[Bugfix]:Cached_tokens always returns -1#229
chasted wants to merge 2 commits into
vllm-project:mainfrom
chasted:main

Conversation

@chasted

@chasted chasted commented Sep 1, 2026

Copy link
Copy Markdown

Purpose

In PD disaggregation mode, ensure that "usage.prompt_tokens_details.cached_tokens" preferentially uses the valid value returned by the prefill stage.

If the "cached_tokens" value returned by the decode stage is missing or invalid (e.g., "< 0"), automatically fall back to the prefill value.

Preserve the existing "logprobs" merging logic without introducing regressions.

Add regression tests to ensure the fix remains effective.

Test Plan

This test plan is designed to verify the PD (Prefill/Decode) disaggregation fix for cached_tokens handling.

Objective

Ensure that the final response keeps the valid prefill-side cached_tokens value instead of allowing an invalid decode-side value such as -1 to override it.

Core scenarios to cover

  1. Invalid decode value overrides valid prefill value
  • Prefill: cached_tokens = 50
  • Decode: cached_tokens = -1
  • Expected: final value is 50
  1. Valid decode value is preserved
  • Prefill: cached_tokens = 50
  • Decode: cached_tokens = 12
  • Expected: final value remains 12
  1. Decode value is missing
  • Decode has no cached_tokens
  • Expected: value is filled from prefill
  1. Fallback compatibility
  • Prefill uses input_tokens_details.cached_tokens
  • Expected: logic reads the fallback correctly
  1. Decode usage missing entirely
  • Expected: prefill usage is copied into decode JSON
  1. No prefill usage present
  • Expected: merge is a no-op and no overwrite occurs

Test Result

Result summary

-6 passed
-0 failed

Evidence

The test run output included:

running 6 tests
...
test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 484 filtered out

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aa708696d9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +755 to 757
for (name, value) in resp_headers.iter() {
response_builder = response_builder.header(name, value);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Drop stale Content-Length after serializing discovery responses

In discovery mode, every non-streaming decoder response is now deserialized and serialized before this loop. If that changes the byte count—for example, for pretty-printed JSON or when cached_tokens is added or changes width—the old Content-Length is sent with merged_body, causing clients to receive a truncated/malformed response or a connection error. The direct-worker branch at vllm_pd_router.rs:1537-1540 already strips this header; apply the same filtering or set the new length here.

Useful? React with 👍 / 👎.

Comment thread src/routers/http/vllm_pd_router.rs Outdated
Comment on lines 1510 to 1513
let mut decode_json: Value =
serde_json::from_slice(&decode_body).map_err(|e| PDRouterError::NetworkError {
message: format!("Failed to parse decode response as JSON: {}", e),
})?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve non-JSON decoder error bodies

For a non-streaming decoder response that is not JSON, this parse error escapes process_vllm_two_stage_request and callers convert it to a generic 500 (vllm_pd_router.rs:1996-2002), discarding the decoder's original status/body. This affects supported transparent /v1/rerank requests: its mock worker explicitly returns the plain-text Simulated failure on failure (tests/common/mock_worker.rs:809-824); the discovery helper now has the same unconditional parse at lines 730-731. Normalize only successful JSON responses, or fall back to forwarding the original body.

Useful? React with 👍 / 👎.

…ed_tokens always returns -1 in the end.

Signed-off-by: chasted <jiang1713399@gmail.com>
Signed-off-by: chasted <jiang1713399@gmail.com>
@hsliuustc0106 hsliuustc0106 added the bug Something isn't working label Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants