Skip to content
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
4d27912
refactor: unify API type handling across sidecar and coordinator
revit13 Sep 8, 2026
7fb3187
refactor: address simplify review findings
revit13 Sep 8, 2026
bf634a3
fix(request): cap max_output_tokens on Responses prefill legs
revit13 Sep 8, 2026
c9473b1
refactor(sidecar): cap the NIXLv2 prefill leg on a copy
revit13 Sep 8, 2026
121b1a2
refactor(sidecar): copy request bodies with maps.Clone
revit13 Sep 8, 2026
fdaa9a9
fix(sidecar): refuse request bodies that are not JSON objects
revit13 Sep 8, 2026
1738b06
test(request): cover a malformed sampling_params on the generate cap
revit13 Sep 8, 2026
e55885e
Add tests and simplify.
revit13 Sep 9, 2026
0545789
Merge origin/main into parser3
revit13 Sep 9, 2026
ba07e18
Merge branch 'main' into parser3
revit13 Sep 9, 2026
0f22ecc
Merge branch 'main' into parser3
revit13 Sep 9, 2026
359483f
Address review comments.
revit13 Sep 10, 2026
703e90c
Merge branch 'main' into parser3
revit13 Sep 10, 2026
320c820
Address review comments.
revit13 Sep 10, 2026
2bde31a
Merge branch 'main' into parser3
revit13 Sep 10, 2026
511fb6f
Default unrecognized API types to chat completions
revit13 Sep 10, 2026
09a1ab7
Clean up API type comments and tighten min_tokens prefill test
revit13 Sep 10, 2026
3313219
Remove LookupAPIType and use DetectAPIType in coordinator steps
revit13 Sep 10, 2026
54dd89f
State token-limit invariants once and reference them elsewhere
revit13 Sep 10, 2026
7b27bbf
Address review comments on API types and path constants
revit13 Sep 10, 2026
47692b1
Merge branch 'main' into parser3
revit13 Sep 10, 2026
de329aa
Address review comments on the EC encoder request
revit13 Sep 10, 2026
12bb81d
Use "request" instead of "leg" in comments and test names
revit13 Sep 11, 2026
27403ff
Merge branch 'main' into parser3
revit13 Sep 11, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 135 additions & 0 deletions pkg/common/request/apitype.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
/*
Copyright 2026 The llm-d Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package request

import (
"fmt"
"maps"
"strings"
)

// Inference API paths served by the sidecar and the coordinator.
const (
PathChatCompletions = "/v1/chat/completions"
PathCompletions = "/v1/completions"
PathResponses = "/v1/responses"
PathMessages = "/v1/messages"
PathGenerate = "/inference/v1/generate"
)

// APIType is the inference API a request speaks. It selects the JSON field

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd write just "APIType is the inference API a request was sent. "

// names a request carries and the path a synthesized request is sent to.
type APIType int

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two things worth a second look here, not blocking:

  1. APIType is a bare int with no validating constructor, and the "degrades to chat completions" rule is repeated independently in Path, tokenLimitFields, tokenLimitMap, and DetectAPIType rather than centralized. There's also no exhaustive lint rule enabled for this package, so a 5th APIType constant added later would compile cleanly and silently degrade in every one of these switches except prefill.go, which treats an unhandled value as a hard error. Worth enabling exhaustive for this package if the closed-set invariant is meant to be enforced rather than conventional.
    (We can return to it later)

  2. The PR description's test plan says unknown/out-of-range values "fall back to generate." They actually fall back to chat completions (confirmed by this file and by apitype_test.go:56,74). Worth fixing the PR description so reviewers aren't misled about which API type is the actual default.


const (
// APITypeChatCompletions is the Chat Completions API (/v1/chat/completions)
// and the Anthropic Messages API (/v1/messages), which share its field names.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

in the original code it was

// APITypeChatCompletions is the Chat Completions API (/v1/chat/completions, /v1/completions)"

Chat completions and completion processes are the same, but I'w define separate entries, and of course not for messages

we can define a separate entry for /v1/messages

APITypeChatCompletions APIType = iota
// APITypeCompletions is the legacy Completions API (/v1/completions).
APITypeCompletions
// APITypeResponses is the Responses API (/v1/responses).
APITypeResponses
// APITypeGenerate is vLLM's token-in generate API (/inference/v1/generate).
APITypeGenerate
)

// String implements fmt.Stringer so structured logs show readable API names.
func (a APIType) String() string {
switch a {
case APITypeChatCompletions:
return "chat_completions"
case APITypeCompletions:
return "completions"
case APITypeResponses:
return "responses"
case APITypeGenerate:
return "generate"
default:
return fmt.Sprintf("APIType(%d)", int(a))
}
}

// Path returns the canonical request path for the API. APITypeChatCompletions
// maps to PathChatCompletions; PathMessages shares its field names but is not
// a synthesis target.
func (a APIType) Path() string {
switch a {
case APITypeChatCompletions:
return PathChatCompletions
case APITypeCompletions:
return PathCompletions
case APITypeResponses:
return PathResponses
default:
return PathGenerate
}
}

// DetectAPIType classifies a request path. An unrecognized path maps to
// APITypeGenerate: callers that route only known paths never reach the
// fallback, and a path the router does not register is not a client fault.
func DetectAPIType(path string) APIType {
switch {
case strings.Contains(path, PathChatCompletions):
return APITypeChatCompletions
case strings.Contains(path, PathCompletions):
return APITypeCompletions
case strings.Contains(path, PathResponses):
return APITypeResponses
case strings.Contains(path, PathMessages):
return APITypeChatCompletions
default:
return APITypeGenerate
}
}

// JSON request field names that cap output tokens, by API. The Completions and
// generate APIs share a list: neither defines max_completion_tokens, so capping
// it would put a field on the wire that a strict server is free to reject.
var (
chatCompletionTokenLimitFields = []string{FieldMaxTokens, FieldMaxCompletionTokens}
maxTokensOnlyTokenLimitFields = []string{FieldMaxTokens}
responsesTokenLimitFields = []string{FieldMaxOutputTokens}
)

// TokenLimitFields returns the output token cap field names the API uses.
// The returned slices are shared package-level vars; callers must not mutate them.
func (a APIType) TokenLimitFields() []string {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TokenLimitFields and TokenLimitMap (L:126) are exported but only called from CapSingleToken in this package (tokens.go). Can these be lowercase? Path, String, and DetectAPIType have real external callers and should stay exported, but these two don't need to be exported.

switch a {
case APITypeCompletions, APITypeGenerate:
return maxTokensOnlyTokenLimitFields
case APITypeResponses:
return responsesTokenLimitFields
default:
return chatCompletionTokenLimitFields
Comment thread
roytman marked this conversation as resolved.
}
}

// TokenLimitMap returns the map inside body that holds the token limit fields:
// sampling_params for the generate API, body itself otherwise. The generate map
// is always replaced with one body owns, so a caller that writes into the result
// never reaches a nested map the body was cloned from.
func (a APIType) TokenLimitMap(body map[string]any) map[string]any {
if a != APITypeGenerate {
return body
}
sp, _ := body[FieldSamplingParams].(map[string]any)
owned := make(map[string]any, len(sp)+1)
maps.Copy(owned, sp)
body[FieldSamplingParams] = owned
return owned
}
131 changes: 131 additions & 0 deletions pkg/common/request/apitype_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
/*
Copyright 2026 The llm-d Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package request

import (
"reflect"
"testing"
)

func TestAPIType_StringAndPath(t *testing.T) {
cases := map[APIType]struct{ name, path string }{
APITypeChatCompletions: {"chat_completions", PathChatCompletions},
APITypeCompletions: {"completions", PathCompletions},
APITypeResponses: {"responses", PathResponses},
APITypeGenerate: {"generate", PathGenerate},
APIType(7): {"APIType(7)", PathGenerate},
}
for apiType, want := range cases {
if got := apiType.String(); got != want.name {
t.Errorf("APIType(%d).String() = %q, want %q", int(apiType), got, want.name)
}
if got := apiType.Path(); got != want.path {
t.Errorf("APIType(%d).Path() = %q, want %q", int(apiType), got, want.path)
}
}
}

func TestDetectAPIType(t *testing.T) {
tests := []struct {
name string
path string
want APIType
}{
{name: "chat completions", path: PathChatCompletions, want: APITypeChatCompletions},
{name: "completions", path: PathCompletions, want: APITypeCompletions},
{name: "responses", path: PathResponses, want: APITypeResponses},
{name: "messages shares chat completions fields", path: PathMessages, want: APITypeChatCompletions},
{name: "generate", path: PathGenerate, want: APITypeGenerate},
{name: "prefixed chat completions", path: "/prefix" + PathChatCompletions, want: APITypeChatCompletions},
{name: "prefixed completions", path: "/prefix" + PathCompletions, want: APITypeCompletions},
{name: "unknown path falls back to generate", path: "/v1/embeddings", want: APITypeGenerate},
{name: "empty path falls back to generate", path: "", want: APITypeGenerate},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := DetectAPIType(tt.path); got != tt.want {
t.Errorf("DetectAPIType(%q) = %v, want %v", tt.path, got, tt.want)
}
})
}
}

func TestAPIType_TokenLimitFields(t *testing.T) {
cases := map[APIType][]string{
APITypeChatCompletions: {FieldMaxTokens, FieldMaxCompletionTokens},
APITypeCompletions: {FieldMaxTokens},
APITypeResponses: {FieldMaxOutputTokens},
APITypeGenerate: {FieldMaxTokens},
}
for apiType, want := range cases {
if got := apiType.TokenLimitFields(); !reflect.DeepEqual(got, want) {
t.Errorf("APIType(%d).TokenLimitFields() = %v, want %v", int(apiType), got, want)
}
}
}

func TestAPIType_TokenLimitMap(t *testing.T) {
t.Run("non-generate returns the body itself", func(t *testing.T) {
body := map[string]any{"model": "m"}
got := APITypeChatCompletions.TokenLimitMap(body)
if !reflect.DeepEqual(got, body) {
t.Errorf("got %v, want the body %v", got, body)
}
if _, ok := body[FieldSamplingParams]; ok {
t.Error("sampling_params was added to a non-generate body")
}
})

// A generate body may share sampling_params with the body it was cloned from,
// so the caller gets a copy the body owns and the original stays intact.
t.Run("generate copies an existing sampling_params", func(t *testing.T) {
sp := map[string]any{FieldMaxTokens: 100}
body := map[string]any{FieldSamplingParams: sp}

got := APITypeGenerate.TokenLimitMap(body)

if !reflect.DeepEqual(got, sp) {
t.Errorf("got %v, want the entries of %v", got, sp)
}
got[FieldMaxTokens] = 1
if sp[FieldMaxTokens] != 100 {
t.Errorf("caller's sampling_params was written through: %v", sp)
}
if body[FieldSamplingParams].(map[string]any)[FieldMaxTokens] != 1 {
t.Errorf("body sampling_params = %v, want the returned map", body[FieldSamplingParams])
}
})

for name, value := range map[string]any{"absent": nil, "not a map": "not-a-map"} {
t.Run("generate replaces a sampling_params that is "+name, func(t *testing.T) {
body := map[string]any{"model": "m"}
if value != nil {
body[FieldSamplingParams] = value
}

got := APITypeGenerate.TokenLimitMap(body)

if len(got) != 0 {
t.Errorf("got %v, want an empty map", got)
}
got[FieldMaxTokens] = 1
if sp, ok := body[FieldSamplingParams].(map[string]any); !ok || sp[FieldMaxTokens] != 1 {
t.Errorf("body sampling_params = %v, want the returned map", body[FieldSamplingParams])
}
})
}
}
49 changes: 28 additions & 21 deletions pkg/common/request/tokens.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,33 @@ limitations under the License.

package request

// CapMaxTokensField caps target's max_tokens to 1 and strips min_tokens.
// min_tokens is stripped rather than clamped: it defaults to 0 in vLLM, so
// removing it keeps min_tokens <= max_tokens=1 without raising the floor
// above the cap (vLLM's SamplingParams rejects min_tokens > max_tokens).
func CapMaxTokensField(target map[string]any) {
target[FieldMaxTokens] = 1
delete(target, FieldMinTokens)
}

// PrimeSingleTokenRequest mutates target in place into a synthetic,
// non-streaming, single-output-token chat-completions or completions
// request. max_completion_tokens is unconditionally capped to 1 alongside
// max_tokens: vLLM and SGLang both accept the two fields together
// (max_completion_tokens takes precedence over max_tokens when present),
// so setting both guarantees the cap regardless of which field the serving
// engine consults.
func PrimeSingleTokenRequest(target map[string]any) {
CapMaxTokensField(target)
target[FieldMaxCompletionTokens] = 1
// CapSingleToken rewrites body into a synthetic, non-streaming,
// single-output-token request for a prefill or encode leg. It returns the map
// the caps were written into, which is where the generate API also expects
// transfer params, so a caller adding them needs no second lookup.
//
// The caps to rewrite come from APIType.TokenLimitFields, so each API's output
// caps are named in one place. min_tokens is a floor rather than a cap, so it is
// stripped instead of capped: it defaults to 0 in vLLM, so removing it keeps
// min_tokens <= max_tokens=1 without raising the floor above the cap (vLLM's
// SamplingParams rejects min_tokens > max_tokens).
//
// Chat completions lists both max_tokens and max_completion_tokens: vLLM and
// SGLang accept the two together and prefer max_completion_tokens, so capping
// both bounds the leg regardless of which field the engine consults.
//
// body is rewritten in place, so the caller passes its own copy. A one-level
// copy is enough: the generate API caps inside sampling_params, and
// TokenLimitMap replaces that nested map rather than writing through it, so a
// body that still shares it with the decode leg keeps the client's limits.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This restates the chat-completions dual-field rationale from apitype.go:100-102 and the TokenLimitMap replacement contract from apitype.go:122-125. Could this live in one place? The min_tokens-as-floor paragraph above (lines 25-28) is the only part here that isn't said elsewhere.

func CapSingleToken(body map[string]any, apiType APIType) map[string]any {
limits := apiType.TokenLimitMap(body)
for _, field := range apiType.TokenLimitFields() {
limits[field] = 1
}
delete(limits, FieldMinTokens)

target[FieldStream] = false
delete(target, FieldStreamOptions)
body[FieldStream] = false
delete(body, FieldStreamOptions)
return limits
}
Loading
Loading