Skip to content

Commit ca846ed

Browse files
committed
fix: validate context size range and update backend configuration handling
1 parent 8499b3d commit ca846ed

4 files changed

Lines changed: 120 additions & 7 deletions

File tree

cmd/cli/commands/compose.go

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"encoding/json"
55
"errors"
66
"fmt"
7+
"math"
78
"net"
89
"slices"
910
"strconv"
@@ -85,14 +86,20 @@ func newUpCommand() *cobra.Command {
8586
}
8687

8788
for _, model := range models {
88-
size := int32(ctxSize)
89+
backendConfig := inference.BackendConfiguration{
90+
Speculative: speculativeConfig,
91+
}
92+
if cmd.Flags().Changed("context-size") {
93+
if ctxSize > math.MaxInt32 || ctxSize < math.MinInt32 {
94+
return fmt.Errorf("context-size %d is out of range (must be between %d and %d)", ctxSize, math.MinInt32, math.MaxInt32)
95+
}
96+
size := int32(ctxSize)
97+
backendConfig.ContextSize = &size
98+
}
8999
if err := desktopClient.ConfigureBackend(scheduling.ConfigureRequest{
90-
Model: model,
91-
BackendConfiguration: inference.BackendConfiguration{
92-
ContextSize: &size,
93-
Speculative: speculativeConfig,
94-
},
95-
RawRuntimeFlags: rawRuntimeFlags,
100+
Model: model,
101+
BackendConfiguration: backendConfig,
102+
RawRuntimeFlags: rawRuntimeFlags,
96103
}); err != nil {
97104
configErrFmtString := "failed to configure backend for model %s with context-size %d and runtime-flags %s"
98105
_ = sendErrorf(configErrFmtString+": %v", model, ctxSize, rawRuntimeFlags, err)

cmd/cli/commands/compose_test.go

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,104 @@
11
package commands
22

33
import (
4+
"fmt"
5+
"math"
46
"testing"
57

68
"github.com/docker/model-runner/pkg/inference"
79
"github.com/stretchr/testify/assert"
810
"github.com/stretchr/testify/require"
911
)
1012

13+
// TestUpCommandContextSizeFlagBehavior verifies that the --context-size flag on
14+
// the compose up command is not "changed" by default (i.e. nil ContextSize
15+
// should be sent when the flag is absent) and is marked as changed after an
16+
// explicit value is provided.
17+
func TestUpCommandContextSizeFlagBehavior(t *testing.T) {
18+
t.Run("context-size flag not changed by default", func(t *testing.T) {
19+
cmd := newUpCommand()
20+
// Parse with just the required --model flag — no --context-size.
21+
err := cmd.ParseFlags([]string{"--model", "mymodel"})
22+
require.NoError(t, err)
23+
// The flag must NOT be marked as changed so that ContextSize is omitted
24+
// from the configure request (i.e. remains nil).
25+
assert.False(t, cmd.Flags().Changed("context-size"),
26+
"context-size must not be Changed when the flag is absent")
27+
})
28+
29+
t.Run("context-size flag changed after explicit value", func(t *testing.T) {
30+
cmd := newUpCommand()
31+
err := cmd.ParseFlags([]string{"--model", "mymodel", "--context-size", "4096"})
32+
require.NoError(t, err)
33+
assert.True(t, cmd.Flags().Changed("context-size"),
34+
"context-size must be Changed when explicitly provided")
35+
})
36+
37+
t.Run("context-size flag changed with unlimited value -1", func(t *testing.T) {
38+
cmd := newUpCommand()
39+
err := cmd.ParseFlags([]string{"--model", "mymodel", "--context-size", "-1"})
40+
require.NoError(t, err)
41+
assert.True(t, cmd.Flags().Changed("context-size"),
42+
"context-size must be Changed when explicitly set to -1 (unlimited)")
43+
})
44+
45+
t.Run("ContextSize is nil in BackendConfiguration when flag not set", func(t *testing.T) {
46+
cmd := newUpCommand()
47+
require.NoError(t, cmd.ParseFlags([]string{"--model", "mymodel"}))
48+
// Simulate the logic in compose.go RunE: only add ContextSize when Changed.
49+
backendConfig := inference.BackendConfiguration{}
50+
if cmd.Flags().Changed("context-size") {
51+
size := int32(-1) // default value
52+
backendConfig.ContextSize = &size
53+
}
54+
assert.Nil(t, backendConfig.ContextSize,
55+
"ContextSize must be nil in BackendConfiguration when --context-size is not provided")
56+
})
57+
58+
t.Run("ContextSize is non-nil in BackendConfiguration when flag is set", func(t *testing.T) {
59+
cmd := newUpCommand()
60+
require.NoError(t, cmd.ParseFlags([]string{"--model", "mymodel", "--context-size", "64000"}))
61+
ctxSize, err := cmd.Flags().GetInt64("context-size")
62+
require.NoError(t, err)
63+
backendConfig := inference.BackendConfiguration{}
64+
if cmd.Flags().Changed("context-size") {
65+
size := int32(ctxSize)
66+
backendConfig.ContextSize = &size
67+
}
68+
require.NotNil(t, backendConfig.ContextSize,
69+
"ContextSize must be non-nil when --context-size is provided")
70+
assert.Equal(t, int32(64000), *backendConfig.ContextSize)
71+
})
72+
73+
t.Run("context-size above int32 max is out of range", func(t *testing.T) {
74+
tooBig := int64(math.MaxInt32) + 1
75+
cmd := newUpCommand()
76+
require.NoError(t, cmd.ParseFlags([]string{"--model", "mymodel", "--context-size", fmt.Sprintf("%d", tooBig)}))
77+
ctxSize, err := cmd.Flags().GetInt64("context-size")
78+
require.NoError(t, err)
79+
require.True(t, cmd.Flags().Changed("context-size"))
80+
// Simulate the range check from compose.go RunE.
81+
if ctxSize > math.MaxInt32 || ctxSize < math.MinInt32 {
82+
// Expected: would return an error in RunE.
83+
return
84+
}
85+
t.Fatal("expected out-of-range check to trigger for value above MaxInt32")
86+
})
87+
88+
t.Run("context-size below int32 min is out of range", func(t *testing.T) {
89+
tooSmall := int64(math.MinInt32) - 1
90+
cmd := newUpCommand()
91+
require.NoError(t, cmd.ParseFlags([]string{"--model", "mymodel", "--context-size", fmt.Sprintf("%d", tooSmall)}))
92+
ctxSize, err := cmd.Flags().GetInt64("context-size")
93+
require.NoError(t, err)
94+
require.True(t, cmd.Flags().Changed("context-size"))
95+
if ctxSize > math.MaxInt32 || ctxSize < math.MinInt32 {
96+
return
97+
}
98+
t.Fatal("expected out-of-range check to trigger for value below MinInt32")
99+
})
100+
}
101+
11102
func TestParseBackendMode(t *testing.T) {
12103
tests := []struct {
13104
name string

pkg/inference/backends/sglang/sglang_config.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ func GetContextLength(modelCfg types.ModelConfig, backendCfg *inference.BackendC
7272
return backendCfg.ContextSize
7373
}
7474
// Fallback to model config (set at packaging time via docker model package --context-size)
75+
if modelCfg == nil {
76+
return nil
77+
}
7578
if cs := modelCfg.GetContextSize(); cs != nil && *cs > 0 {
7679
return cs
7780
}

pkg/inference/backends/sglang/sglang_config_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,18 @@ func TestGetContextLength(t *testing.T) {
265265
},
266266
expectedValue: nil,
267267
},
268+
{
269+
name: "nil model config with backend config",
270+
modelCfg: nil,
271+
backendCfg: &inference.BackendConfiguration{ContextSize: int32ptr(4096)},
272+
expectedValue: int32ptr(4096),
273+
},
274+
{
275+
name: "nil model config without backend config",
276+
modelCfg: nil,
277+
backendCfg: nil,
278+
expectedValue: nil,
279+
},
268280
}
269281

270282
for _, tt := range tests {

0 commit comments

Comments
 (0)