Skip to content

Commit 69c3ccb

Browse files
vMaroonCopilot
andauthored
fix(sidecar): cap min_tokens on the prefill leg for chat completions (llm-d#2041)
* fix(sidecar): cap min_tokens on the prefill leg for chat completions The prefill leg caps max_tokens at 1 to suppress output, but min_tokens was staged only for the generate API. On the chat completions APIs a client-supplied min_tokens above 1 reached the prefiller next to max_tokens=1, which vLLM rejects because min_tokens may not exceed max_tokens, so the request failed with a 400 before decode ran. Stage min_tokens with the other chat completions token limits, and do the same in the MoRI-IO concurrent-dispatch path, which builds its prefill body separately. The saved-value buffer now sizes off the field list, which a fixed two would have overflowed. Signed-off-by: Maroon Ayoub <mayoub@redhat.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Maroon Ayoub <Maroonay@gmail.com> --------- Signed-off-by: Maroon Ayoub <mayoub@redhat.com> Signed-off-by: Maroon Ayoub <Maroonay@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 55d381f commit 69c3ccb

3 files changed

Lines changed: 96 additions & 7 deletions

File tree

pkg/sidecar/proxy/connector_nixlv2.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ func (s *Server) handleNIXLV2(w http.ResponseWriter, r *http.Request, prefillPod
115115
present bool
116116
}
117117
tokenMap, createdSamplingParams := tokenLimitMap(completionRequest, apiType)
118-
var savedTokenValues [2]savedField
118+
savedTokenValues := make([]savedField, len(tokenLimitFields))
119119
for i, field := range tokenLimitFields {
120120
if v, ok := tokenMap[field]; ok {
121121
savedTokenValues[i] = savedField{field: field, val: v, present: true}
@@ -324,7 +324,7 @@ retryLoop:
324324
completionRequest[requestFieldStreamOptions] = streamOptionsValue
325325
}
326326

327-
for i := range savedTokenValues[:len(tokenLimitFields)] {
327+
for i := range savedTokenValues {
328328
sv := &savedTokenValues[i]
329329
delete(tokenMap, sv.field)
330330
if sv.present {
@@ -459,6 +459,7 @@ func (s *Server) runNIXLProtocolV2WriteParallel(
459459
maxTokensValue, maxTokensOk := completionRequest[requestFieldMaxTokens]
460460
maxCompletionTokensValue, maxCompletionTokensOk := completionRequest[requestFieldMaxCompletionTokens]
461461
maxOutputTokensValue, maxOutputTokensOk := completionRequest[requestFieldMaxOutputTokens]
462+
minTokensValue, minTokensOk := completionRequest[requestFieldMinTokens]
462463

463464
// Pin both legs to the same DP rank (kv_transfer_params + HTTP header).
464465
dpRank := pickDPRank(uuidStr, s.config.MoRIIODPSize)
@@ -503,6 +504,7 @@ func (s *Server) runNIXLProtocolV2WriteParallel(
503504
completionRequest[requestFieldMaxTokens] = 1
504505
completionRequest[requestFieldMaxCompletionTokens] = 1
505506
completionRequest[requestFieldMaxOutputTokens] = 1
507+
completionRequest[requestFieldMinTokens] = 1
506508

507509
pbody, err := json.Marshal(completionRequest)
508510
if err != nil {
@@ -513,7 +515,7 @@ func (s *Server) runNIXLProtocolV2WriteParallel(
513515
}
514516

515517
// ---------- Build decode body ----------
516-
// Restore the client's streaming flags and max-token caps.
518+
// Restore the client's streaming flags and token-limit fields.
517519
delete(completionRequest, requestFieldStream)
518520
if streamOk {
519521
completionRequest[requestFieldStream] = streamValue
@@ -533,6 +535,10 @@ func (s *Server) runNIXLProtocolV2WriteParallel(
533535
if maxOutputTokensOk {
534536
completionRequest[requestFieldMaxOutputTokens] = maxOutputTokensValue
535537
}
538+
delete(completionRequest, requestFieldMinTokens)
539+
if minTokensOk {
540+
completionRequest[requestFieldMinTokens] = minTokensValue
541+
}
536542

537543
// Synthesise decode-leg kv_transfer_params that the serial path would
538544
// otherwise read from the prefill response. do_remote_prefill must be true:

pkg/sidecar/proxy/connector_nixlv2_test.go

Lines changed: 86 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,60 @@ var _ = Describe("NIXL Connector (v2)", func() {
249249
Expect(responseBody).To(ContainSubstring("data: [DONE]"))
250250
})
251251

252+
sendChatCompletionsRequestWithBody := func(proxyBaseAddr, body string) {
253+
req, err := http.NewRequest(http.MethodPost, proxyBaseAddr+ChatCompletionsPath, bytes.NewReader([]byte(body)))
254+
Expect(err).ToNot(HaveOccurred())
255+
req.Header.Add(routing.PrefillEndpointHeader, testInfo.prefillBackend.URL[len("http://"):])
256+
257+
rp, err := http.DefaultClient.Do(req)
258+
Expect(err).ToNot(HaveOccurred())
259+
defer rp.Body.Close()
260+
261+
responseBody, err := io.ReadAll(rp.Body)
262+
Expect(err).ToNot(HaveOccurred())
263+
Expect(rp.StatusCode).To(Equal(http.StatusOK), string(responseBody))
264+
}
265+
266+
It("should cap token limits in prefill and restore originals in decode", func() {
267+
proxyBaseAddr := startProxy()
268+
269+
sendChatCompletionsRequestWithBody(proxyBaseAddr, `{
270+
"model": "Qwen/Qwen2-0.5B",
271+
"messages": [
272+
{"role": "user", "content": "Hello"}
273+
],
274+
"max_tokens": 100,
275+
"min_tokens": 5
276+
}`)
277+
278+
prefillReq := testInfo.prefillHandler.CompletionRequests[0]
279+
Expect(prefillReq).To(HaveKeyWithValue(requestFieldMaxTokens, BeNumerically("==", 1)))
280+
Expect(prefillReq).To(HaveKeyWithValue(requestFieldMinTokens, BeNumerically("==", 1)))
281+
282+
decodeReq := testInfo.decodeHandler.CompletionRequests[0]
283+
Expect(decodeReq).To(HaveKeyWithValue(requestFieldMaxTokens, BeNumerically("==", 100)))
284+
Expect(decodeReq).To(HaveKeyWithValue(requestFieldMinTokens, BeNumerically("==", 5)))
285+
})
286+
287+
It("should cap prefill and drop the caps in decode when the request omits them", func() {
288+
proxyBaseAddr := startProxy()
289+
290+
sendChatCompletionsRequestWithBody(proxyBaseAddr, `{
291+
"model": "Qwen/Qwen2-0.5B",
292+
"messages": [
293+
{"role": "user", "content": "Hello"}
294+
]
295+
}`)
296+
297+
prefillReq := testInfo.prefillHandler.CompletionRequests[0]
298+
Expect(prefillReq).To(HaveKeyWithValue(requestFieldMaxTokens, BeNumerically("==", 1)))
299+
Expect(prefillReq).To(HaveKeyWithValue(requestFieldMinTokens, BeNumerically("==", 1)))
300+
301+
decodeReq := testInfo.decodeHandler.CompletionRequests[0]
302+
Expect(decodeReq).ToNot(HaveKey(requestFieldMaxTokens))
303+
Expect(decodeReq).ToNot(HaveKey(requestFieldMinTokens))
304+
})
305+
252306
// Messages API tests — verify /v1/messages routes through the disaggregation
253307
// handler with the same token-limit fields as chat completions.
254308

@@ -1018,6 +1072,30 @@ var _ = Describe("NIXL Connector (v2)", func() {
10181072
Expect(pRank).To(And(BeNumerically(">=", 0), BeNumerically("<", 16)))
10191073
})
10201074

1075+
// The concurrent-dispatch path stages its own prefill body rather than going
1076+
// through the serial path's token-limit handling, so it caps min_tokens too.
1077+
It("concurrent WRITE-mode dispatch caps min_tokens in prefill and restores it in decode", func() {
1078+
env := startMoRIProxy(func(c *Config) {
1079+
c.MoRIIOParallelDispatch = true
1080+
})
1081+
env.sendBody(`{
1082+
"model": "Qwen/Qwen2-0.5B",
1083+
"messages": [
1084+
{"role": "user", "content": "Hello"}
1085+
],
1086+
"max_tokens": 100,
1087+
"min_tokens": 5
1088+
}`)
1089+
1090+
prefillReq := env.prefillHandler.GetCompletionRequests()[0]
1091+
Expect(prefillReq).To(HaveKeyWithValue(requestFieldMaxTokens, BeNumerically("==", 1)))
1092+
Expect(prefillReq).To(HaveKeyWithValue(requestFieldMinTokens, BeNumerically("==", 1)))
1093+
1094+
decodeReq := env.decodeHandler.GetCompletionRequests()[0]
1095+
Expect(decodeReq).To(HaveKeyWithValue(requestFieldMaxTokens, BeNumerically("==", 100)))
1096+
Expect(decodeReq).To(HaveKeyWithValue(requestFieldMinTokens, BeNumerically("==", 5)))
1097+
})
1098+
10211099
// 1P1D DP=8, serial dispatch: the prefill leg sets the DP-rank header and
10221100
// the decode leg's kv_transfer_params are backfilled with the same rank.
10231101
It("serial WRITE-mode DP=8 pins prefill and decode HTTP legs to the same DP rank", func() {
@@ -1142,15 +1220,20 @@ func startMoRIProxy(mutate func(cfg *Config)) *moriProxyEnv {
11421220
// sequential in the serial path) by the time this returns, so the captured
11431221
// requests / headers are safe to read afterwards.
11441222
func (env *moriProxyEnv) send() {
1145-
req, err := http.NewRequest(http.MethodPost, env.baseAddr+ChatCompletionsPath, strings.NewReader(chatCompletionsRequestBody))
1223+
env.sendBody(chatCompletionsRequestBody)
1224+
}
1225+
1226+
// sendBody sends a caller-supplied request body.
1227+
func (env *moriProxyEnv) sendBody(body string) {
1228+
req, err := http.NewRequest(http.MethodPost, env.baseAddr+ChatCompletionsPath, strings.NewReader(body))
11461229
Expect(err).ToNot(HaveOccurred())
11471230
req.Header.Add(routing.PrefillEndpointHeader, env.prefillBackend.URL[len("http://"):])
11481231

11491232
rp, err := http.DefaultClient.Do(req)
11501233
Expect(err).ToNot(HaveOccurred())
11511234
defer rp.Body.Close()
1152-
body, _ := io.ReadAll(rp.Body) //nolint:errcheck
1153-
Expect(rp.StatusCode).To(Equal(http.StatusOK), string(body))
1235+
responseBody, _ := io.ReadAll(rp.Body) //nolint:errcheck
1236+
Expect(rp.StatusCode).To(Equal(http.StatusOK), string(responseBody))
11541237
}
11551238

11561239
// kvParams returns the kv_transfer_params map of the i-th request captured by h.

pkg/sidecar/proxy/proxy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ func (a APIType) String() string {
136136
// JSON request field names used for token limits in prefill/decode staging.
137137
// Do not mutate these slices.
138138
var (
139-
chatCompletionTokenLimitFields = []string{requestFieldMaxTokens, requestFieldMaxCompletionTokens}
139+
chatCompletionTokenLimitFields = []string{requestFieldMaxTokens, requestFieldMaxCompletionTokens, requestFieldMinTokens}
140140
responsesStyleTokenLimitFields = []string{requestFieldMaxOutputTokens}
141141
generateStyleTokenLimitFields = []string{requestFieldMaxTokens, requestFieldMinTokens}
142142
)

0 commit comments

Comments
 (0)