From 2f2d0460b527ed27a391538c03f2703fd528e5cd Mon Sep 17 00:00:00 2001 From: aceforeverd Date: Mon, 9 Nov 2026 20:51:34 +0800 Subject: [PATCH 1/7] feat(compose): answer a variant's requires.inputs from the site profile A catalog variant may declare requires.inputs: chart values it needs and cannot know. The site profile answers them in features.inputs, keyed by chart path, with model.pathTemplate's placeholders ({{hf}}, {{org}}, {{name}}, {{model}}). compose sets each answer in the derived layer, so the deployer's overrides win over it, and refuses a plan whose merged values leave an input unset or answer it with a value of the wrong type or pattern. swissd re-checks the catalog's rules on load. The API and plan format do not change. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011bWVCSybBZ1QmsL9gjrqxW --- internal/catalog/entry.go | 2 + internal/catalog/inputs.go | 152 ++++++++++++++++++++++++++++++++ internal/catalog/inputs_test.go | 92 +++++++++++++++++++ internal/catalog/load.go | 3 + internal/compose/compose.go | 50 +++++++++++ internal/compose/inputs_test.go | 97 ++++++++++++++++++++ internal/site/features_test.go | 60 +++++++++++++ internal/site/profile.go | 79 +++++++++++++++-- 8 files changed, 530 insertions(+), 5 deletions(-) create mode 100644 internal/catalog/inputs.go create mode 100644 internal/catalog/inputs_test.go create mode 100644 internal/compose/inputs_test.go create mode 100644 internal/site/features_test.go diff --git a/internal/catalog/entry.go b/internal/catalog/entry.go index f08eabd..130d137 100644 --- a/internal/catalog/entry.go +++ b/internal/catalog/entry.go @@ -85,6 +85,8 @@ type Requires struct { GPUProduct []string `yaml:"gpuProduct,omitempty" json:"gpuProduct,omitempty"` MinGPUMemoryGiB float64 `yaml:"minGpuMemoryGiB,omitempty" json:"minGpuMemoryGiB,omitempty"` RDMA bool `yaml:"rdma,omitempty" json:"rdma,omitempty"` + // Inputs are chart values the variant leaves to the deploy. + Inputs []Input `yaml:"inputs,omitempty" json:"inputs,omitempty"` } const VendorNvidia = "nvidia" diff --git a/internal/catalog/inputs.go b/internal/catalog/inputs.go new file mode 100644 index 0000000..c114d69 --- /dev/null +++ b/internal/catalog/inputs.go @@ -0,0 +1,152 @@ +package catalog + +import ( + "fmt" + "math" + "regexp" + "strings" + + "github.com/modelsphere/swiss/internal/values" +) + +// Input is one requires.inputs entry. Its rules repeat the catalog's +// hack/lib/inputs.js: the catalog is untrusted where it is used. +type Input struct { + // Path is the chart value, dotted: storageKVOffload.mooncake.master. + Path string `yaml:"path" json:"path"` + Title string `yaml:"title" json:"title"` + Description string `yaml:"description,omitempty" json:"description,omitempty"` + // Type is string, integer or boolean. Empty is string. + Type string `yaml:"type,omitempty" json:"type,omitempty"` + // Pattern is what a whole string answer matches. + Pattern string `yaml:"pattern,omitempty" json:"pattern,omitempty"` + // Example is the form's placeholder and what catalog CI renders with; never deployed. + Example any `yaml:"example" json:"example"` +} + +const ( + InputString = "string" + InputInteger = "integer" + InputBoolean = "boolean" +) + +var inputPath = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_-]*(\.[A-Za-z_][A-Za-z0-9_-]*)*$`) + +// ValidInputPath is shared with the site profile, which keys its answers by these paths. +func ValidInputPath(p string) bool { return inputPath.MatchString(p) } + +func (i Input) TypeOrDefault() string { + if i.Type == "" { + return InputString + } + return i.Type +} + +// Check reports why v does not answer i. A whole float64 is an integer: JSON decodes numbers so. +func (i Input) Check(v any) error { + switch t := i.TypeOrDefault(); t { + case InputString: + s, ok := v.(string) + if !ok { + return fmt.Errorf("%v is not a string", v) + } + if i.Pattern == "" { + return nil + } + re, err := i.pattern() + if err != nil { + return err + } + if !re.MatchString(s) { + return fmt.Errorf("%q does not match %s", s, i.Pattern) + } + case InputInteger: + if !isInteger(v) { + return fmt.Errorf("%v is not an integer", v) + } + case InputBoolean: + if _, ok := v.(bool); !ok { + return fmt.Errorf("%v is not a boolean", v) + } + default: + return fmt.Errorf("type %q is not string, integer or boolean", t) + } + return nil +} + +func (i Input) pattern() (*regexp.Regexp, error) { + re, err := regexp.Compile(`^(?:` + i.Pattern + `)$`) + if err != nil { + return nil, fmt.Errorf("pattern %s does not compile: %w", i.Pattern, err) + } + return re, nil +} + +func isInteger(v any) bool { + switch n := v.(type) { + case int, int64, uint64: + return true + case float64: + return n == math.Trunc(n) && !math.IsInf(n, 0) + } + return false +} + +// checkInputs: values leaves every input unset, since an input with a default is not one. +func checkInputs(inputs []Input, vals values.Tree) error { + declared := map[string]bool{} + for _, in := range inputs { + where := "requires.inputs " + in.Path + switch { + case !ValidInputPath(in.Path): + return fmt.Errorf("requires.inputs: path %q is not a dotted chart path", in.Path) + case in.Path == "model.localPath": + return fmt.Errorf("%s: swiss answers it from the site's model.pathTemplate, so it is never an input", where) + case declared[in.Path]: + return fmt.Errorf("%s: declared twice", where) + case strings.TrimSpace(in.Title) == "": + return fmt.Errorf("%s: title is required", where) + case in.Pattern != "" && in.TypeOrDefault() != InputString: + return fmt.Errorf("%s: pattern is only for a string input, and this one is %s", where, in.TypeOrDefault()) + } + declared[in.Path] = true + if at := setAt(vals, strings.Split(in.Path, ".")); at != "" { + return fmt.Errorf("%s: values.%s sets it; values leaves an input unset, for the deploy to answer", where, at) + } + if in.Pattern != "" { + if _, err := in.pattern(); err != nil { + return fmt.Errorf("%s: %w", where, err) + } + } + if in.Example == nil { + return fmt.Errorf("%s: example is required", where) + } + if err := in.Check(in.Example); err != nil { + return fmt.Errorf("%s: example %w", where, err) + } + } + return nil +} + +// setAt is where vals already says something about keys: the value, under it, or a scalar above it. +func setAt(vals values.Tree, keys []string) string { + node := map[string]any(vals) + for i, k := range keys { + next, ok := node[k] + if !ok { + return "" + } + if i == len(keys)-1 { + return strings.Join(keys, ".") + } + switch m := next.(type) { + case map[string]any: + node = m + case values.Tree: + node = m + default: + return strings.Join(keys[:i+1], ".") + } + } + return "" +} diff --git a/internal/catalog/inputs_test.go b/internal/catalog/inputs_test.go new file mode 100644 index 0000000..725e203 --- /dev/null +++ b/internal/catalog/inputs_test.go @@ -0,0 +1,92 @@ +package catalog + +import ( + "context" + "fmt" + "strings" + "testing" + + "github.com/modelsphere/swiss/internal/values" +) + +var mooncakeMaster = Input{ + Path: "storageKVOffload.mooncake.master", + Title: "Mooncake master", + Pattern: `[^\s:/]+:[0-9]+`, + Example: "mooncake-master.example:50051", +} + +func inputVariant(inputs []Input, vals values.Tree) Variant { + return Variant{ + ID: "a", + Engine: "sglang", + Chart: Chart{Name: "sglang", Version: "0.9.32"}, + Requires: Requires{GPUs: 8, Inputs: inputs}, + Values: vals, + } +} + +// The same rules as the catalog's validator (hack/lib/inputs.js), because the +// catalog is untrusted where it is used. +func TestAnInputIsLeftToTheDeploy(t *testing.T) { + ok := inputVariant([]Input{mooncakeMaster}, values.Tree{"storageKVOffload": map[string]any{"enabled": true, "mooncake": map[string]any{"protocol": "rdma"}}}) + if err := ok.validate(); err != nil { + t.Fatal(err) + } + one := func(in Input) []Input { return []Input{in} } + at := func(in Input, edit func(*Input)) Input { edit(&in); return in } + for name, c := range map[string]struct { + v Variant + want string + }{ + "values sets it": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": ""}}}), "values.storageKVOffload.mooncake.master sets it"}, + "a non-map above it": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": "off"}}), "values.storageKVOffload.mooncake sets it"}, + "declared twice": {inputVariant([]Input{mooncakeMaster, mooncakeMaster}, nil), "declared twice"}, + "the model path": {inputVariant(one(Input{Path: "model.localPath", Title: "p", Example: "/m"}), nil), "never an input"}, + "not a path": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Path = "a..b" })), nil), "not a dotted chart path"}, + "no title": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Title = " " })), nil), "title is required"}, + "no example": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Example = nil })), nil), "example is required"}, + "example off pattern": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Example = "no-port" })), nil), `example "no-port" does not match`}, + "pattern compiles": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Pattern = "(" })), nil), "does not compile"}, + "pattern on integer": {inputVariant(one(Input{Path: "a.b", Title: "t", Type: InputInteger, Pattern: "[0-9]+", Example: 1}), nil), "only for a string input"}, + "example of a type": {inputVariant(one(Input{Path: "a.b", Title: "t", Type: InputInteger, Example: "8"}), nil), "8 is not an integer"}, + "unknown type": {inputVariant(one(Input{Path: "a.b", Title: "t", Type: "list", Example: "x"}), nil), `type "list" is not`}, + } { + if err := c.v.validate(); err == nil || !strings.Contains(err.Error(), c.want) { + t.Errorf("%s: want an error containing %q, got %v", name, c.want, err) + } + } +} + +// A number read from JSON is a float64: a whole one answers an integer input. +func TestAnIntegerInputTakesAWholeNumberOfAnyKind(t *testing.T) { + port := Input{Path: "a.port", Title: "Port", Type: InputInteger, Example: 1} + for _, v := range []any{8080, int64(8080), float64(8080)} { + if err := port.Check(v); err != nil { + t.Errorf("%T %v: %v", v, v, err) + } + } + if err := port.Check(80.5); err == nil { + t.Error("80.5 answered an integer input") + } +} + +// requires.inputs is a known key in both places a v2 variant is read strictly: +// its file, and its entry in the index. +func TestV2InputsAreReadFromTheFileAndTheIndex(t *testing.T) { + raw := []byte("apiVersion: catalog.swiss/v2\nkind: Variant\nmodel: m\nid: a\nversion: 1.0.0\nengine: sglang\nchart: {name: sglang, version: \"0.9.32\"}\n" + + "requires:\n gpus: 1\n inputs:\n - {path: storageKVOffload.mooncake.master, title: Mooncake master, example: \"h:1\"}\n") + f := memFetcher{"models/m/a.yaml": raw} + f["index-v2.json"] = []byte(fmt.Sprintf(`{"apiVersion":"catalog.swiss/v2","count":1,"models":[{"name":"m","source":{"hf":"org/m"},"variants":[{"id":"a","version":"1.0.0","path":"models/m/a.yaml","digest":%q,"engine":"sglang","chart":{"name":"sglang","version":"0.9.32"},"requires":{"gpus":1,"inputs":[{"path":"storageKVOffload.mooncake.master","title":"Mooncake master","example":"h:1"}]}}]}]}`, contentRef(raw))) + c, err := OpenFetcher(context.Background(), f) + if err != nil { + t.Fatal(err) + } + r, err := c.Resolve(context.Background(), "m", "", "") + if err != nil { + t.Fatal(err) + } + if in := r.Variant.Requires.Inputs; len(in) != 1 || in[0].Path != "storageKVOffload.mooncake.master" || in[0].Example != "h:1" { + t.Errorf("inputs = %+v", in) + } +} diff --git a/internal/catalog/load.go b/internal/catalog/load.go index b67fcbc..921a6a3 100644 --- a/internal/catalog/load.go +++ b/internal/catalog/load.go @@ -352,5 +352,8 @@ func (v Variant) validate() error { return fmt.Errorf("%s: values.lws.size (%v) != requires.nodes (%d)", where, size, v.Requires.NodesOrDefault()) } } + if err := checkInputs(v.Requires.Inputs, v.Values); err != nil { + return fmt.Errorf("%s: %w", where, err) + } return nil } diff --git a/internal/compose/compose.go b/internal/compose/compose.go index ee4fda8..592b1e0 100644 --- a/internal/compose/compose.go +++ b/internal/compose/compose.go @@ -119,6 +119,9 @@ func Compose(in Input) (*plan.Plan, error) { if err := applyDefaults(siteVals, derivedVals, base, in); err != nil { return nil, err } + if err := applyInputs(derivedVals, base, in); err != nil { + return nil, err + } if res, ok := values.Get(derivedVals, "model.gpuResource"); ok { if err := checkGPUResourceChart(in.Resolved.Variant.Chart.Name, chartVersion, res.(string)); err != nil { return nil, err @@ -169,6 +172,9 @@ func Compose(in Input) (*plan.Plan, error) { CreateNamespace: in.CreateNamespace, Layers: layers, } + if err := checkInputs(in.Resolved.Variant.Requires.Inputs, p.Values()); err != nil { + return nil, err + } if err := p.ComputeHash(); err != nil { return nil, err } @@ -379,6 +385,50 @@ func applyDefaults(site, derived, base values.Tree, in Input) error { return nil } +// applyInputs puts the site's answers in the derived layer, under the form's overrides. +func applyInputs(derived, base values.Tree, in Input) error { + for _, input := range in.Resolved.Variant.Requires.Inputs { + answer, ok, err := in.Profile.InputAnswer(input.Path, in.Resolved.Source.HF, in.Resolved.Model) + if err != nil { + return err + } + if !ok { + continue + } + if _, set := values.Get(base, input.Path); set { + continue + } + if err := values.Set(derived, input.Path, answer); err != nil { + return err + } + } + return nil +} + +// checkInputs reads the composed values, so an answer from any layer counts. "" is no +// answer: it is what variants wrote before they could declare an input. +func checkInputs(inputs []catalog.Input, composed values.Tree) error { + var missing []string + for _, input := range inputs { + v, ok := values.Get(composed, input.Path) + if !ok || v == nil || v == "" { + missing = append(missing, fmt.Sprintf("%s (%s)", input.Path, input.Title)) + continue + } + if err := input.Check(v); err != nil { + return fmt.Errorf("input %s (%s): %w", input.Path, input.Title, err) + } + } + switch len(missing) { + case 0: + return nil + case 1: + return fmt.Errorf("input %s is not set: set it in the deploy's overrides, or in the site profile's features.inputs", missing[0]) + default: + return fmt.Errorf("inputs %s are not set: set them in the deploy's overrides, or in the site profile's features.inputs", strings.Join(missing, ", ")) + } +} + func checkGPUResourceChart(name, version, res string) error { since, ok := GPUResourceSince[name] if !ok { diff --git a/internal/compose/inputs_test.go b/internal/compose/inputs_test.go new file mode 100644 index 0000000..2576cb1 --- /dev/null +++ b/internal/compose/inputs_test.go @@ -0,0 +1,97 @@ +package compose + +import ( + "strings" + "testing" + + "github.com/modelsphere/swiss/internal/catalog" + "github.com/modelsphere/swiss/internal/values" +) + +const masterPath = "storageKVOffload.mooncake.master" + +func inputsInput() Input { + in := testInput() + in.Resolved.Variant.Requires.Inputs = []catalog.Input{{ + Path: masterPath, + Title: "Mooncake master", + Pattern: `[^\s:/]+:[0-9]+`, + Example: "mooncake-master.example:50051", + }} + in.Resolved.Variant.Values["storageKVOffload"] = map[string]any{"enabled": true, "mooncake": map[string]any{"protocol": "rdma"}} + return in +} + +// Level one: the site answers, in the derived layer, beside the variant's own +// values under the same section. +func TestTheSiteAnswersAnInputInTheDerivedLayer(t *testing.T) { + in := inputsInput() + in.Profile.Features.Inputs = map[string]any{masterPath: "mc-{{model}}.svc:50051"} + p, err := Compose(in) + if err != nil { + t.Fatal(err) + } + if got, _ := values.Get(p.Values(), masterPath); got != "mc-glm-5.3.svc:50051" { + t.Errorf("%s = %v", masterPath, got) + } + if got := p.LayerOf(masterPath); got != values.LayerDerived { + t.Errorf("answered by %s, want derived", got) + } + if got, _ := values.Get(p.Values(), "storageKVOffload.mooncake.protocol"); got != "rdma" { + t.Errorf("the variant's own mooncake values were lost: protocol = %v", got) + } +} + +// Level two: the deployer overrides any answer, and needs no site answer to. +func TestTheDeployOverridesTheSitesAnswer(t *testing.T) { + for name, site := range map[string]map[string]any{ + "over the site's answer": {masterPath: "site.svc:50051"}, + "with no site answer": nil, + } { + in := inputsInput() + in.Profile.Features.Inputs = site + in.Overrides = values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": "mine.svc:50051"}}} + p, err := Compose(in) + if err != nil { + t.Fatalf("%s: %v", name, err) + } + if got, _ := values.Get(p.Values(), masterPath); got != "mine.svc:50051" || p.LayerOf(masterPath) != values.LayerForm { + t.Errorf("%s: %s = %v from %s, want the deploy's", name, masterPath, got, p.LayerOf(masterPath)) + } + } +} + +func TestAnUnansweredInputRefusesThePlan(t *testing.T) { + in := inputsInput() + _, err := Compose(in) + if err == nil || !strings.Contains(err.Error(), "input storageKVOffload.mooncake.master (Mooncake master) is not set") { + t.Fatalf("want a refusal naming the input, got %v", err) + } + // The empty string a variant wrote before it could declare an input is no answer either. + in.Overrides = values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": ""}}} + if _, err := Compose(in); err == nil || !strings.Contains(err.Error(), "is not set") { + t.Fatalf("an empty answer was taken: %v", err) + } +} + +func TestAnAnswerTheInputDoesNotTakeRefusesThePlan(t *testing.T) { + in := inputsInput() + in.Profile.Features.Inputs = map[string]any{masterPath: "no-port"} + if _, err := Compose(in); err == nil || !strings.Contains(err.Error(), `"no-port" does not match`) { + t.Fatalf("want a refusal quoting the answer, got %v", err) + } +} + +// An answer reaches only a variant that declares its path: unlike extra, a +// site answer never lands in a deploy that did not ask. +func TestASiteAnswerReachesOnlyAVariantThatDeclaresIt(t *testing.T) { + in := testInput() + in.Profile.Features.Inputs = map[string]any{masterPath: "site.svc:50051"} + p, err := Compose(in) + if err != nil { + t.Fatal(err) + } + if _, ok := values.Get(p.Values(), masterPath); ok { + t.Error("a site answer landed in a variant that declares no input") + } +} diff --git a/internal/site/features_test.go b/internal/site/features_test.go new file mode 100644 index 0000000..3f34d71 --- /dev/null +++ b/internal/site/features_test.go @@ -0,0 +1,60 @@ +package site + +import ( + "strings" + "testing" +) + +const featuresHead = "name: prod\nmodel:\n pathTemplate: /models/{{name}}\nnodes:\n gpus: []\n" + +func TestInputAnswersFillTheModelPlaceholders(t *testing.T) { + p, err := Parse([]byte(featuresHead+`features: + inputs: + storageKVOffload.mooncake.master: mooncake-master.svc:50051 + engineCache.hostPath: /data/cache/{{model}}/{{org}} + service.port: 8050 + shm.enabled: true +`), "test") + if err != nil { + t.Fatal(err) + } + for path, want := range map[string]any{ + "storageKVOffload.mooncake.master": "mooncake-master.svc:50051", + "engineCache.hostPath": "/data/cache/glm-5.3/zai-org", + "service.port": 8050, + "shm.enabled": true, + } { + got, ok, err := p.InputAnswer(path, "zai-org/GLM-5.3", "glm-5.3") + if err != nil || !ok || got != want { + t.Errorf("%s = %v (ok %v, err %v), want %v", path, got, ok, err, want) + } + } + if _, ok, _ := p.InputAnswer("not.answered", "zai-org/GLM-5.3", "glm-5.3"); ok { + t.Error("an unanswered path has an answer") + } +} + +// A model with no weights has no repo id for a template to name. +func TestATemplateNamingTheRepoNeedsASource(t *testing.T) { + p := Profile{Features: Features{Inputs: map[string]any{"a.b": "/data/{{name}}", "c.d": "/data/{{model}}"}}} + if _, _, err := p.InputAnswer("a.b", "", "mock"); err == nil || !strings.Contains(err.Error(), "no source.hf") { + t.Errorf("want a refusal naming source.hf, got %v", err) + } + if got, _, err := p.InputAnswer("c.d", "", "mock"); err != nil || got != "/data/mock" { + t.Errorf("{{model}} needs no source: %v, %v", got, err) + } +} + +func TestAnAnswerThatCouldNeverBeUsedIsRefused(t *testing.T) { + for name, c := range map[string]struct{ yaml, want string }{ + "unknown placeholder": {"features:\n inputs:\n a.b: /x/{{release}}\n", "unknown placeholder"}, + "not a scalar": {"features:\n inputs:\n a.b: {host: h}\n", "string, integer or boolean"}, + "no value": {"features:\n inputs:\n a.b:\n", "string, integer or boolean"}, + "not a path": {"features:\n inputs:\n a..b: x\n", "not a dotted chart path"}, + "extra answers too": {"extra:\n a:\n b: y\nfeatures:\n inputs:\n a.b: x\n", "extra sets the same path"}, + } { + if _, err := Parse([]byte(featuresHead+c.yaml), "test"); err == nil || !strings.Contains(err.Error(), c.want) { + t.Errorf("%s: want an error containing %q, got %v", name, c.want, err) + } + } +} diff --git a/internal/site/profile.go b/internal/site/profile.go index df56d7d..6046206 100644 --- a/internal/site/profile.go +++ b/internal/site/profile.go @@ -7,7 +7,9 @@ package site import ( "fmt" + "maps" "os" + "slices" "strings" "time" @@ -76,6 +78,9 @@ type Profile struct { // and composes above the catalog. Defaults Defaults `yaml:"defaults,omitempty" json:"defaults"` + // Features answers a variant's requires. Unrelated to Defaults.Features. + Features Features `yaml:"features,omitempty" json:"features"` + // Sites are the other clusters' swissd instances, for the nav switcher. // // In the profile rather than swissd's config because it is a fact about the @@ -202,6 +207,52 @@ func (f FeatureDefaults) Resolve() map[string]bool { } } +type Features struct { + // Inputs answer requires.inputs by chart path, only for a variant that + // declares the path -- unlike extra. Strings take pathTemplate's placeholders. + Inputs map[string]any `yaml:"inputs,omitempty" json:"inputs,omitempty"` +} + +// check refuses an answer that could never be used, or a second answer from extra. +func (f Features) check(extra values.Tree) error { + for _, path := range slices.Sorted(maps.Keys(f.Inputs)) { + where := fmt.Sprintf("features.inputs[%q]", path) + if !catalog.ValidInputPath(path) { + return fmt.Errorf("%s: not a dotted chart path", where) + } + switch v := f.Inputs[path].(type) { + case string: + if _, err := fill(v, "org/name", "model"); err != nil { + return fmt.Errorf("%s: %w", where, err) + } + case bool, int, int64, uint64, float64: + default: + return fmt.Errorf("%s: an answer is a string, integer or boolean, not %T", where, v) + } + if _, set := values.Get(extra, path); set { + return fmt.Errorf("%s: extra sets the same path; give the one answer here", where) + } + } + return nil +} + +// InputAnswer is the site's answer to an input, placeholders filled for this model. +func (p Profile) InputAnswer(path, hf, model string) (answer any, ok bool, err error) { + v, ok := p.Features.Inputs[path] + if !ok { + return nil, false, nil + } + s, isString := v.(string) + if !isString { + return v, true, nil + } + out, err := fill(s, hf, model) + if err != nil { + return nil, true, fmt.Errorf("features.inputs[%q]: %w", path, err) + } + return out, true, nil +} + // DefaultSLOAddress is the in-cluster SLO API when the profile names none. const DefaultSLOAddress = "http://slo-api.llm-scaler.svc:80" @@ -507,6 +558,9 @@ func Parse(raw []byte, origin string) (*Profile, error) { if err := p.Nodes.checkGPUs(); err != nil { return nil, fmt.Errorf("%s: %w", origin, err) } + if err := p.Features.check(p.Extra); err != nil { + return nil, fmt.Errorf("%s: %w", origin, err) + } return &p, nil } @@ -587,14 +641,29 @@ func (p Profile) LocalPath(hf, model string) (string, error) { if v, ok := p.Model.Overrides[model]; ok { return v, nil } - org, name, ok := strings.Cut(hf, "/") - if !ok { + if _, _, ok := strings.Cut(hf, "/"); !ok { return "", fmt.Errorf("source.hf %q is not org/name", hf) } - r := strings.NewReplacer("{{hf}}", hf, "{{org}}", org, "{{name}}", name, "{{model}}", model) - out := r.Replace(p.Model.PathTemplate) + out, err := fill(p.Model.PathTemplate, hf, model) + if err != nil { + return "", fmt.Errorf("model.pathTemplate %w", err) + } + return out, nil +} + +// fill replaces {{hf}} (the repo id), {{org}} and {{name}} (its halves) and {{model}} (the entry name). +func fill(tmpl, hf, model string) (string, error) { + if hf == "" { + for _, ph := range []string{"{{hf}}", "{{org}}", "{{name}}"} { + if strings.Contains(tmpl, ph) { + return "", fmt.Errorf("%q uses %s, and the model has no source.hf", tmpl, ph) + } + } + } + org, name, _ := strings.Cut(hf, "/") + out := strings.NewReplacer("{{hf}}", hf, "{{org}}", org, "{{name}}", name, "{{model}}", model).Replace(tmpl) if strings.Contains(out, "{{") { - return "", fmt.Errorf("model.pathTemplate %q has an unknown placeholder", p.Model.PathTemplate) + return "", fmt.Errorf("%q has an unknown placeholder", tmpl) } return out, nil } From 4a986449c553270db6aa501dbd736cc10bb4c644 Mon Sep 17 00:00:00 2001 From: aceforeverd Date: Tue, 10 Nov 2026 10:45:12 +0800 Subject: [PATCH 2/7] feat(compose): a variant's default answers an input, under the site's A value the variant's values gives an input's path is its default: swissd checks it on load against the input's type and pattern instead of refusing it. The site's features.inputs answer now merges over that default (it yields only to the site layer), and the deploy's overrides over both. Compose refuses only an input left with no answer from any layer; "" is none. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011bWVCSybBZ1QmsL9gjrqxW --- internal/catalog/entry.go | 2 +- internal/catalog/inputs.go | 29 ++++++++++++++++--------- internal/catalog/inputs_test.go | 19 +++++++++++------ internal/compose/compose.go | 15 +++++++------ internal/compose/inputs_test.go | 38 ++++++++++++++++++++++++++++++++- 5 files changed, 78 insertions(+), 25 deletions(-) diff --git a/internal/catalog/entry.go b/internal/catalog/entry.go index 130d137..3771d42 100644 --- a/internal/catalog/entry.go +++ b/internal/catalog/entry.go @@ -85,7 +85,7 @@ type Requires struct { GPUProduct []string `yaml:"gpuProduct,omitempty" json:"gpuProduct,omitempty"` MinGPUMemoryGiB float64 `yaml:"minGpuMemoryGiB,omitempty" json:"minGpuMemoryGiB,omitempty"` RDMA bool `yaml:"rdma,omitempty" json:"rdma,omitempty"` - // Inputs are chart values the variant leaves to the deploy. + // Inputs are chart values the deploy is asked for. values may give one a default. Inputs []Input `yaml:"inputs,omitempty" json:"inputs,omitempty"` } diff --git a/internal/catalog/inputs.go b/internal/catalog/inputs.go index c114d69..652f548 100644 --- a/internal/catalog/inputs.go +++ b/internal/catalog/inputs.go @@ -92,7 +92,10 @@ func isInteger(v any) bool { return false } -// checkInputs: values leaves every input unset, since an input with a default is not one. +// Unanswered is "" as well as nil: "" is what variants wrote for a value they could not know. +func Unanswered(v any) bool { return v == nil || v == "" } + +// checkInputs: a value values gives an input is its default, so it must answer it. func checkInputs(inputs []Input, vals values.Tree) error { declared := map[string]bool{} for _, in := range inputs { @@ -110,9 +113,6 @@ func checkInputs(inputs []Input, vals values.Tree) error { return fmt.Errorf("%s: pattern is only for a string input, and this one is %s", where, in.TypeOrDefault()) } declared[in.Path] = true - if at := setAt(vals, strings.Split(in.Path, ".")); at != "" { - return fmt.Errorf("%s: values.%s sets it; values leaves an input unset, for the deploy to answer", where, at) - } if in.Pattern != "" { if _, err := in.pattern(); err != nil { return fmt.Errorf("%s: %w", where, err) @@ -124,20 +124,29 @@ func checkInputs(inputs []Input, vals values.Tree) error { if err := in.Check(in.Example); err != nil { return fmt.Errorf("%s: example %w", where, err) } + def, blocked := valueAt(vals, strings.Split(in.Path, ".")) + if blocked != "" { + return fmt.Errorf("%s: values.%s is not a map, so it cannot hold this input", where, blocked) + } + if !Unanswered(def) { + if err := in.Check(def); err != nil { + return fmt.Errorf("%s: default %w", where, err) + } + } } return nil } -// setAt is where vals already says something about keys: the value, under it, or a scalar above it. -func setAt(vals values.Tree, keys []string) string { +// valueAt is what vals holds at keys, or the prefix where a non-map is in the way. +func valueAt(vals values.Tree, keys []string) (any, string) { node := map[string]any(vals) for i, k := range keys { next, ok := node[k] if !ok { - return "" + return nil, "" } if i == len(keys)-1 { - return strings.Join(keys, ".") + return next, "" } switch m := next.(type) { case map[string]any: @@ -145,8 +154,8 @@ func setAt(vals values.Tree, keys []string) string { case values.Tree: node = m default: - return strings.Join(keys[:i+1], ".") + return nil, strings.Join(keys[:i+1], ".") } } - return "" + return nil, "" } diff --git a/internal/catalog/inputs_test.go b/internal/catalog/inputs_test.go index 725e203..6456076 100644 --- a/internal/catalog/inputs_test.go +++ b/internal/catalog/inputs_test.go @@ -28,10 +28,16 @@ func inputVariant(inputs []Input, vals values.Tree) Variant { // The same rules as the catalog's validator (hack/lib/inputs.js), because the // catalog is untrusted where it is used. -func TestAnInputIsLeftToTheDeploy(t *testing.T) { - ok := inputVariant([]Input{mooncakeMaster}, values.Tree{"storageKVOffload": map[string]any{"enabled": true, "mooncake": map[string]any{"protocol": "rdma"}}}) - if err := ok.validate(); err != nil { - t.Fatal(err) +func TestAnInputsDefaultAndExampleAreAnswers(t *testing.T) { + for _, def := range []any{nil, "", "mc.svc:50051"} { + mooncake := map[string]any{"protocol": "rdma"} + if def != nil { + mooncake["master"] = def + } + ok := inputVariant([]Input{mooncakeMaster}, values.Tree{"storageKVOffload": map[string]any{"enabled": true, "mooncake": mooncake}}) + if err := ok.validate(); err != nil { + t.Errorf("default %q: %v", def, err) + } } one := func(in Input) []Input { return []Input{in} } at := func(in Input, edit func(*Input)) Input { edit(&in); return in } @@ -39,8 +45,9 @@ func TestAnInputIsLeftToTheDeploy(t *testing.T) { v Variant want string }{ - "values sets it": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": ""}}}), "values.storageKVOffload.mooncake.master sets it"}, - "a non-map above it": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": "off"}}), "values.storageKVOffload.mooncake sets it"}, + "default off pattern": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": "no-port"}}}), `default "no-port" does not match`}, + "default of a type": {inputVariant(one(Input{Path: "a.b", Title: "t", Type: InputInteger, Example: 1}), values.Tree{"a": map[string]any{"b": "8"}}), "default 8 is not an integer"}, + "a non-map above it": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": "off"}}), "values.storageKVOffload.mooncake is not a map"}, "declared twice": {inputVariant([]Input{mooncakeMaster, mooncakeMaster}, nil), "declared twice"}, "the model path": {inputVariant(one(Input{Path: "model.localPath", Title: "p", Example: "/m"}), nil), "never an input"}, "not a path": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Path = "a..b" })), nil), "not a dotted chart path"}, diff --git a/internal/compose/compose.go b/internal/compose/compose.go index 592b1e0..aa30773 100644 --- a/internal/compose/compose.go +++ b/internal/compose/compose.go @@ -119,7 +119,7 @@ func Compose(in Input) (*plan.Plan, error) { if err := applyDefaults(siteVals, derivedVals, base, in); err != nil { return nil, err } - if err := applyInputs(derivedVals, base, in); err != nil { + if err := applyInputs(derivedVals, siteVals, in); err != nil { return nil, err } if res, ok := values.Get(derivedVals, "model.gpuResource"); ok { @@ -385,8 +385,9 @@ func applyDefaults(site, derived, base values.Tree, in Input) error { return nil } -// applyInputs puts the site's answers in the derived layer, under the form's overrides. -func applyInputs(derived, base values.Tree, in Input) error { +// applyInputs puts the site's answers in the derived layer: over the variant's +// defaults, unlike the other derived values, and under the form's overrides. +func applyInputs(derived, site values.Tree, in Input) error { for _, input := range in.Resolved.Variant.Requires.Inputs { answer, ok, err := in.Profile.InputAnswer(input.Path, in.Resolved.Source.HF, in.Resolved.Model) if err != nil { @@ -395,7 +396,7 @@ func applyInputs(derived, base values.Tree, in Input) error { if !ok { continue } - if _, set := values.Get(base, input.Path); set { + if _, set := values.Get(site, input.Path); set { continue } if err := values.Set(derived, input.Path, answer); err != nil { @@ -405,13 +406,13 @@ func applyInputs(derived, base values.Tree, in Input) error { return nil } -// checkInputs reads the composed values, so an answer from any layer counts. "" is no -// answer: it is what variants wrote before they could declare an input. +// checkInputs reads the composed values, so an answer from any layer counts, the +// variant's default among them. func checkInputs(inputs []catalog.Input, composed values.Tree) error { var missing []string for _, input := range inputs { v, ok := values.Get(composed, input.Path) - if !ok || v == nil || v == "" { + if !ok || catalog.Unanswered(v) { missing = append(missing, fmt.Sprintf("%s (%s)", input.Path, input.Title)) continue } diff --git a/internal/compose/inputs_test.go b/internal/compose/inputs_test.go index 2576cb1..f10dd81 100644 --- a/internal/compose/inputs_test.go +++ b/internal/compose/inputs_test.go @@ -67,11 +67,47 @@ func TestAnUnansweredInputRefusesThePlan(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "input storageKVOffload.mooncake.master (Mooncake master) is not set") { t.Fatalf("want a refusal naming the input, got %v", err) } - // The empty string a variant wrote before it could declare an input is no answer either. + // "" is no answer, from the deploy or as the variant's default. in.Overrides = values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": ""}}} if _, err := Compose(in); err == nil || !strings.Contains(err.Error(), "is not set") { t.Fatalf("an empty answer was taken: %v", err) } + in = withDefault(inputsInput(), "") + if _, err := Compose(in); err == nil || !strings.Contains(err.Error(), "is not set") { + t.Fatalf("an empty default was taken: %v", err) + } +} + +func withDefault(in Input, def any) Input { + in.Resolved.Variant.Values["storageKVOffload"].(map[string]any)["mooncake"].(map[string]any)["master"] = def + return in +} + +// A variant's default answers an input nobody else does; the site's answer +// beats it, and the deploy's beats both. +func TestTheVariantsDefaultIsTheLowestAnswer(t *testing.T) { + for name, c := range map[string]struct { + site, form map[string]any + want string + layer string + }{ + "the default alone": {want: "tuned.svc:50051", layer: values.LayerCatalog}, + "the site over it": {site: map[string]any{masterPath: "site.svc:50051"}, want: "site.svc:50051", layer: values.LayerDerived}, + "the deploy over the two": {site: map[string]any{masterPath: "site.svc:50051"}, form: map[string]any{"master": "mine.svc:50051"}, want: "mine.svc:50051", layer: values.LayerForm}, + } { + in := withDefault(inputsInput(), "tuned.svc:50051") + in.Profile.Features.Inputs = c.site + if c.form != nil { + in.Overrides = values.Tree{"storageKVOffload": map[string]any{"mooncake": c.form}} + } + p, err := Compose(in) + if err != nil { + t.Fatalf("%s: %v", name, err) + } + if got, _ := values.Get(p.Values(), masterPath); got != c.want || p.LayerOf(masterPath) != c.layer { + t.Errorf("%s: %s = %v from %s, want %s from %s", name, masterPath, got, p.LayerOf(masterPath), c.want, c.layer) + } + } } func TestAnAnswerTheInputDoesNotTakeRefusesThePlan(t *testing.T) { From 01df142e8f0575b33e10a022d7db3d4e45e5c9cb Mon Sep 17 00:00:00 2001 From: aceforeverd Date: Tue, 10 Nov 2026 10:59:05 +0800 Subject: [PATCH 3/7] feat(catalog): every input is required; a blank default is refused A value values gives an input's path is its default and must pass the input's type and pattern, "" and null included. Compose checks "" like any answer; only an absent or null value is "not set". Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011bWVCSybBZ1QmsL9gjrqxW --- internal/catalog/entry.go | 2 +- internal/catalog/inputs.go | 17 +++++++---------- internal/catalog/inputs_test.go | 13 +++++++------ internal/compose/compose.go | 2 +- internal/compose/inputs_test.go | 8 ++------ 5 files changed, 18 insertions(+), 24 deletions(-) diff --git a/internal/catalog/entry.go b/internal/catalog/entry.go index 3771d42..801110c 100644 --- a/internal/catalog/entry.go +++ b/internal/catalog/entry.go @@ -85,7 +85,7 @@ type Requires struct { GPUProduct []string `yaml:"gpuProduct,omitempty" json:"gpuProduct,omitempty"` MinGPUMemoryGiB float64 `yaml:"minGpuMemoryGiB,omitempty" json:"minGpuMemoryGiB,omitempty"` RDMA bool `yaml:"rdma,omitempty" json:"rdma,omitempty"` - // Inputs are chart values the deploy is asked for. values may give one a default. + // Inputs are chart values every deploy must answer. values may give one a default. Inputs []Input `yaml:"inputs,omitempty" json:"inputs,omitempty"` } diff --git a/internal/catalog/inputs.go b/internal/catalog/inputs.go index 652f548..a6bd7e8 100644 --- a/internal/catalog/inputs.go +++ b/internal/catalog/inputs.go @@ -92,9 +92,6 @@ func isInteger(v any) bool { return false } -// Unanswered is "" as well as nil: "" is what variants wrote for a value they could not know. -func Unanswered(v any) bool { return v == nil || v == "" } - // checkInputs: a value values gives an input is its default, so it must answer it. func checkInputs(inputs []Input, vals values.Tree) error { declared := map[string]bool{} @@ -124,11 +121,11 @@ func checkInputs(inputs []Input, vals values.Tree) error { if err := in.Check(in.Example); err != nil { return fmt.Errorf("%s: example %w", where, err) } - def, blocked := valueAt(vals, strings.Split(in.Path, ".")) + def, ok, blocked := valueAt(vals, strings.Split(in.Path, ".")) if blocked != "" { return fmt.Errorf("%s: values.%s is not a map, so it cannot hold this input", where, blocked) } - if !Unanswered(def) { + if ok { if err := in.Check(def); err != nil { return fmt.Errorf("%s: default %w", where, err) } @@ -138,15 +135,15 @@ func checkInputs(inputs []Input, vals values.Tree) error { } // valueAt is what vals holds at keys, or the prefix where a non-map is in the way. -func valueAt(vals values.Tree, keys []string) (any, string) { +func valueAt(vals values.Tree, keys []string) (v any, ok bool, blocked string) { node := map[string]any(vals) for i, k := range keys { next, ok := node[k] if !ok { - return nil, "" + return nil, false, "" } if i == len(keys)-1 { - return next, "" + return next, true, "" } switch m := next.(type) { case map[string]any: @@ -154,8 +151,8 @@ func valueAt(vals values.Tree, keys []string) (any, string) { case values.Tree: node = m default: - return nil, strings.Join(keys[:i+1], ".") + return nil, false, strings.Join(keys[:i+1], ".") } } - return nil, "" + return nil, false, "" } diff --git a/internal/catalog/inputs_test.go b/internal/catalog/inputs_test.go index 6456076..1a6c6ca 100644 --- a/internal/catalog/inputs_test.go +++ b/internal/catalog/inputs_test.go @@ -29,14 +29,13 @@ func inputVariant(inputs []Input, vals values.Tree) Variant { // The same rules as the catalog's validator (hack/lib/inputs.js), because the // catalog is untrusted where it is used. func TestAnInputsDefaultAndExampleAreAnswers(t *testing.T) { - for _, def := range []any{nil, "", "mc.svc:50051"} { - mooncake := map[string]any{"protocol": "rdma"} - if def != nil { - mooncake["master"] = def - } + for name, mooncake := range map[string]map[string]any{ + "no default": {"protocol": "rdma"}, + "with default": {"protocol": "rdma", "master": "mc.svc:50051"}, + } { ok := inputVariant([]Input{mooncakeMaster}, values.Tree{"storageKVOffload": map[string]any{"enabled": true, "mooncake": mooncake}}) if err := ok.validate(); err != nil { - t.Errorf("default %q: %v", def, err) + t.Errorf("%s: %v", name, err) } } one := func(in Input) []Input { return []Input{in} } @@ -46,6 +45,8 @@ func TestAnInputsDefaultAndExampleAreAnswers(t *testing.T) { want string }{ "default off pattern": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": "no-port"}}}), `default "no-port" does not match`}, + "a blank default": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": ""}}}), `default "" does not match`}, + "a null default": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": nil}}}), "default is not a string"}, "default of a type": {inputVariant(one(Input{Path: "a.b", Title: "t", Type: InputInteger, Example: 1}), values.Tree{"a": map[string]any{"b": "8"}}), "default 8 is not an integer"}, "a non-map above it": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": "off"}}), "values.storageKVOffload.mooncake is not a map"}, "declared twice": {inputVariant([]Input{mooncakeMaster, mooncakeMaster}, nil), "declared twice"}, diff --git a/internal/compose/compose.go b/internal/compose/compose.go index aa30773..08ef45e 100644 --- a/internal/compose/compose.go +++ b/internal/compose/compose.go @@ -412,7 +412,7 @@ func checkInputs(inputs []catalog.Input, composed values.Tree) error { var missing []string for _, input := range inputs { v, ok := values.Get(composed, input.Path) - if !ok || catalog.Unanswered(v) { + if !ok || v == nil { missing = append(missing, fmt.Sprintf("%s (%s)", input.Path, input.Title)) continue } diff --git a/internal/compose/inputs_test.go b/internal/compose/inputs_test.go index f10dd81..813b81c 100644 --- a/internal/compose/inputs_test.go +++ b/internal/compose/inputs_test.go @@ -67,15 +67,11 @@ func TestAnUnansweredInputRefusesThePlan(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "input storageKVOffload.mooncake.master (Mooncake master) is not set") { t.Fatalf("want a refusal naming the input, got %v", err) } - // "" is no answer, from the deploy or as the variant's default. + // "" is checked like any answer, and fails the pattern. in.Overrides = values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": ""}}} - if _, err := Compose(in); err == nil || !strings.Contains(err.Error(), "is not set") { + if _, err := Compose(in); err == nil || !strings.Contains(err.Error(), `"" does not match`) { t.Fatalf("an empty answer was taken: %v", err) } - in = withDefault(inputsInput(), "") - if _, err := Compose(in); err == nil || !strings.Contains(err.Error(), "is not set") { - t.Fatalf("an empty default was taken: %v", err) - } } func withDefault(in Input, def any) Input { From 0bcfb1d7aed0e62f3d5a8c48763b39d30cac7317 Mon Sep 17 00:00:00 2001 From: aceforeverd Date: Tue, 10 Nov 2026 11:08:59 +0800 Subject: [PATCH 4/7] feat(compose): select an input's answer, the site's else its default catalog.Input gains Default. Compose puts each input's answer in the derived layer: the site profile's features.inputs, else the input's default; the deploy's overrides go over it. swissd checks the default against the input's type and pattern on load, and again refuses a variant whose values set an input's path. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011bWVCSybBZ1QmsL9gjrqxW --- internal/catalog/entry.go | 2 +- internal/catalog/inputs.go | 27 ++++++++++++++------------- internal/catalog/inputs_test.go | 27 +++++++++++++-------------- internal/compose/compose.go | 10 ++++++---- internal/compose/inputs_test.go | 8 ++++---- 5 files changed, 38 insertions(+), 36 deletions(-) diff --git a/internal/catalog/entry.go b/internal/catalog/entry.go index 801110c..9db8aee 100644 --- a/internal/catalog/entry.go +++ b/internal/catalog/entry.go @@ -85,7 +85,7 @@ type Requires struct { GPUProduct []string `yaml:"gpuProduct,omitempty" json:"gpuProduct,omitempty"` MinGPUMemoryGiB float64 `yaml:"minGpuMemoryGiB,omitempty" json:"minGpuMemoryGiB,omitempty"` RDMA bool `yaml:"rdma,omitempty" json:"rdma,omitempty"` - // Inputs are chart values every deploy must answer. values may give one a default. + // Inputs are chart values every deploy must answer. Inputs []Input `yaml:"inputs,omitempty" json:"inputs,omitempty"` } diff --git a/internal/catalog/inputs.go b/internal/catalog/inputs.go index a6bd7e8..00f96ce 100644 --- a/internal/catalog/inputs.go +++ b/internal/catalog/inputs.go @@ -22,6 +22,8 @@ type Input struct { Pattern string `yaml:"pattern,omitempty" json:"pattern,omitempty"` // Example is the form's placeholder and what catalog CI renders with; never deployed. Example any `yaml:"example" json:"example"` + // Default answers the input when the site profile does not. + Default any `yaml:"default,omitempty" json:"default,omitempty"` } const ( @@ -92,7 +94,7 @@ func isInteger(v any) bool { return false } -// checkInputs: a value values gives an input is its default, so it must answer it. +// checkInputs: values leaves every input to compose, which answers it in the derived layer. func checkInputs(inputs []Input, vals values.Tree) error { declared := map[string]bool{} for _, in := range inputs { @@ -110,6 +112,9 @@ func checkInputs(inputs []Input, vals values.Tree) error { return fmt.Errorf("%s: pattern is only for a string input, and this one is %s", where, in.TypeOrDefault()) } declared[in.Path] = true + if at := setAt(vals, strings.Split(in.Path, ".")); at != "" { + return fmt.Errorf("%s: values.%s sets it; give the value as the input's default", where, at) + } if in.Pattern != "" { if _, err := in.pattern(); err != nil { return fmt.Errorf("%s: %w", where, err) @@ -121,12 +126,8 @@ func checkInputs(inputs []Input, vals values.Tree) error { if err := in.Check(in.Example); err != nil { return fmt.Errorf("%s: example %w", where, err) } - def, ok, blocked := valueAt(vals, strings.Split(in.Path, ".")) - if blocked != "" { - return fmt.Errorf("%s: values.%s is not a map, so it cannot hold this input", where, blocked) - } - if ok { - if err := in.Check(def); err != nil { + if in.Default != nil { + if err := in.Check(in.Default); err != nil { return fmt.Errorf("%s: default %w", where, err) } } @@ -134,16 +135,16 @@ func checkInputs(inputs []Input, vals values.Tree) error { return nil } -// valueAt is what vals holds at keys, or the prefix where a non-map is in the way. -func valueAt(vals values.Tree, keys []string) (v any, ok bool, blocked string) { +// setAt is where vals already says something about keys: the value, under it, or a scalar above it. +func setAt(vals values.Tree, keys []string) string { node := map[string]any(vals) for i, k := range keys { next, ok := node[k] if !ok { - return nil, false, "" + return "" } if i == len(keys)-1 { - return next, true, "" + return strings.Join(keys, ".") } switch m := next.(type) { case map[string]any: @@ -151,8 +152,8 @@ func valueAt(vals values.Tree, keys []string) (v any, ok bool, blocked string) { case values.Tree: node = m default: - return nil, false, strings.Join(keys[:i+1], ".") + return strings.Join(keys[:i+1], ".") } } - return nil, false, "" + return "" } diff --git a/internal/catalog/inputs_test.go b/internal/catalog/inputs_test.go index 1a6c6ca..f874b91 100644 --- a/internal/catalog/inputs_test.go +++ b/internal/catalog/inputs_test.go @@ -29,13 +29,12 @@ func inputVariant(inputs []Input, vals values.Tree) Variant { // The same rules as the catalog's validator (hack/lib/inputs.js), because the // catalog is untrusted where it is used. func TestAnInputsDefaultAndExampleAreAnswers(t *testing.T) { - for name, mooncake := range map[string]map[string]any{ - "no default": {"protocol": "rdma"}, - "with default": {"protocol": "rdma", "master": "mc.svc:50051"}, - } { - ok := inputVariant([]Input{mooncakeMaster}, values.Tree{"storageKVOffload": map[string]any{"enabled": true, "mooncake": mooncake}}) + withDefault := mooncakeMaster + withDefault.Default = "mc.svc:50051" + for _, in := range []Input{mooncakeMaster, withDefault} { + ok := inputVariant([]Input{in}, values.Tree{"storageKVOffload": map[string]any{"enabled": true, "mooncake": map[string]any{"protocol": "rdma"}}}) if err := ok.validate(); err != nil { - t.Errorf("%s: %v", name, err) + t.Errorf("default %v: %v", in.Default, err) } } one := func(in Input) []Input { return []Input{in} } @@ -44,11 +43,11 @@ func TestAnInputsDefaultAndExampleAreAnswers(t *testing.T) { v Variant want string }{ - "default off pattern": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": "no-port"}}}), `default "no-port" does not match`}, - "a blank default": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": ""}}}), `default "" does not match`}, - "a null default": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": nil}}}), "default is not a string"}, - "default of a type": {inputVariant(one(Input{Path: "a.b", Title: "t", Type: InputInteger, Example: 1}), values.Tree{"a": map[string]any{"b": "8"}}), "default 8 is not an integer"}, - "a non-map above it": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": "off"}}), "values.storageKVOffload.mooncake is not a map"}, + "values sets it": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": "mc.svc:50051"}}}), "values.storageKVOffload.mooncake.master sets it; give the value as the input's default"}, + "a non-map above it": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": "off"}}), "values.storageKVOffload.mooncake sets it"}, + "default off pattern": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Default = "no-port" })), nil), `default "no-port" does not match`}, + "a blank default": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Default = "" })), nil), `default "" does not match`}, + "default of a type": {inputVariant(one(Input{Path: "a.b", Title: "t", Type: InputInteger, Example: 1, Default: "8"}), nil), "default 8 is not an integer"}, "declared twice": {inputVariant([]Input{mooncakeMaster, mooncakeMaster}, nil), "declared twice"}, "the model path": {inputVariant(one(Input{Path: "model.localPath", Title: "p", Example: "/m"}), nil), "never an input"}, "not a path": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Path = "a..b" })), nil), "not a dotted chart path"}, @@ -83,9 +82,9 @@ func TestAnIntegerInputTakesAWholeNumberOfAnyKind(t *testing.T) { // its file, and its entry in the index. func TestV2InputsAreReadFromTheFileAndTheIndex(t *testing.T) { raw := []byte("apiVersion: catalog.swiss/v2\nkind: Variant\nmodel: m\nid: a\nversion: 1.0.0\nengine: sglang\nchart: {name: sglang, version: \"0.9.32\"}\n" + - "requires:\n gpus: 1\n inputs:\n - {path: storageKVOffload.mooncake.master, title: Mooncake master, example: \"h:1\"}\n") + "requires:\n gpus: 1\n inputs:\n - {path: storageKVOffload.mooncake.master, title: Mooncake master, example: \"h:1\", default: \"d:1\"}\n") f := memFetcher{"models/m/a.yaml": raw} - f["index-v2.json"] = []byte(fmt.Sprintf(`{"apiVersion":"catalog.swiss/v2","count":1,"models":[{"name":"m","source":{"hf":"org/m"},"variants":[{"id":"a","version":"1.0.0","path":"models/m/a.yaml","digest":%q,"engine":"sglang","chart":{"name":"sglang","version":"0.9.32"},"requires":{"gpus":1,"inputs":[{"path":"storageKVOffload.mooncake.master","title":"Mooncake master","example":"h:1"}]}}]}]}`, contentRef(raw))) + f["index-v2.json"] = []byte(fmt.Sprintf(`{"apiVersion":"catalog.swiss/v2","count":1,"models":[{"name":"m","source":{"hf":"org/m"},"variants":[{"id":"a","version":"1.0.0","path":"models/m/a.yaml","digest":%q,"engine":"sglang","chart":{"name":"sglang","version":"0.9.32"},"requires":{"gpus":1,"inputs":[{"path":"storageKVOffload.mooncake.master","title":"Mooncake master","example":"h:1","default":"d:1"}]}}]}]}`, contentRef(raw))) c, err := OpenFetcher(context.Background(), f) if err != nil { t.Fatal(err) @@ -94,7 +93,7 @@ func TestV2InputsAreReadFromTheFileAndTheIndex(t *testing.T) { if err != nil { t.Fatal(err) } - if in := r.Variant.Requires.Inputs; len(in) != 1 || in[0].Path != "storageKVOffload.mooncake.master" || in[0].Example != "h:1" { + if in := r.Variant.Requires.Inputs; len(in) != 1 || in[0].Path != "storageKVOffload.mooncake.master" || in[0].Example != "h:1" || in[0].Default != "d:1" { t.Errorf("inputs = %+v", in) } } diff --git a/internal/compose/compose.go b/internal/compose/compose.go index 08ef45e..427a937 100644 --- a/internal/compose/compose.go +++ b/internal/compose/compose.go @@ -385,14 +385,17 @@ func applyDefaults(site, derived, base values.Tree, in Input) error { return nil } -// applyInputs puts the site's answers in the derived layer: over the variant's -// defaults, unlike the other derived values, and under the form's overrides. +// applyInputs puts each input's answer in the derived layer, under the form's +// overrides: the site's, else the input's default. func applyInputs(derived, site values.Tree, in Input) error { for _, input := range in.Resolved.Variant.Requires.Inputs { answer, ok, err := in.Profile.InputAnswer(input.Path, in.Resolved.Source.HF, in.Resolved.Model) if err != nil { return err } + if !ok { + answer, ok = input.Default, input.Default != nil + } if !ok { continue } @@ -406,8 +409,7 @@ func applyInputs(derived, site values.Tree, in Input) error { return nil } -// checkInputs reads the composed values, so an answer from any layer counts, the -// variant's default among them. +// checkInputs reads the composed values, so an answer from any layer counts. func checkInputs(inputs []catalog.Input, composed values.Tree) error { var missing []string for _, input := range inputs { diff --git a/internal/compose/inputs_test.go b/internal/compose/inputs_test.go index 813b81c..8e14f3c 100644 --- a/internal/compose/inputs_test.go +++ b/internal/compose/inputs_test.go @@ -75,19 +75,19 @@ func TestAnUnansweredInputRefusesThePlan(t *testing.T) { } func withDefault(in Input, def any) Input { - in.Resolved.Variant.Values["storageKVOffload"].(map[string]any)["mooncake"].(map[string]any)["master"] = def + in.Resolved.Variant.Requires.Inputs[0].Default = def return in } -// A variant's default answers an input nobody else does; the site's answer -// beats it, and the deploy's beats both. +// The input's default answers it in the derived layer when the site does not; +// the deploy's answer beats both. func TestTheVariantsDefaultIsTheLowestAnswer(t *testing.T) { for name, c := range map[string]struct { site, form map[string]any want string layer string }{ - "the default alone": {want: "tuned.svc:50051", layer: values.LayerCatalog}, + "the default alone": {want: "tuned.svc:50051", layer: values.LayerDerived}, "the site over it": {site: map[string]any{masterPath: "site.svc:50051"}, want: "site.svc:50051", layer: values.LayerDerived}, "the deploy over the two": {site: map[string]any{masterPath: "site.svc:50051"}, form: map[string]any{"master": "mine.svc:50051"}, want: "mine.svc:50051", layer: values.LayerForm}, } { From d1b366dbbb17175e838b6528d45c1536e95140f4 Mon Sep 17 00:00:00 2001 From: aceforeverd Date: Tue, 10 Nov 2026 11:28:01 +0800 Subject: [PATCH 5/7] feat(catalog): inputs beside requires, and at the site profile's top level A variant's inputs move from requires.inputs to the variant's top level, and the site profile's answers from features.inputs to a top-level inputs map. The v2 index no longer carries inputs. No rule yet keeps an input off a path swiss derives from requires; the catalog schema can add one. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011bWVCSybBZ1QmsL9gjrqxW --- internal/catalog/entry.go | 22 +++++++-------- internal/catalog/inputs.go | 6 ++--- internal/catalog/inputs_test.go | 15 ++++++----- internal/catalog/load.go | 2 +- internal/catalog/variant_v2.go | 3 ++- internal/compose/compose.go | 8 +++--- internal/compose/inputs_test.go | 14 +++++----- .../site/{features_test.go => inputs_test.go} | 27 +++++++++---------- internal/site/profile.go | 27 ++++++++----------- 9 files changed, 60 insertions(+), 64 deletions(-) rename internal/site/{features_test.go => inputs_test.go} (57%) diff --git a/internal/catalog/entry.go b/internal/catalog/entry.go index 9db8aee..047c013 100644 --- a/internal/catalog/entry.go +++ b/internal/catalog/entry.go @@ -49,15 +49,17 @@ type Source struct { // together, which is why they are one object rather than independent form // fields: nothing should be able to select --tp-size=8 and a 2-GPU node. type Variant struct { - ID string `yaml:"id" json:"id"` - Description string `yaml:"description,omitempty" json:"description,omitempty"` - Link string `yaml:"link,omitempty" json:"link,omitempty"` - Default bool `yaml:"default,omitempty" json:"default,omitempty"` - Engine string `yaml:"engine" json:"engine"` - Chart Chart `yaml:"chart" json:"chart"` - Image *Image `yaml:"image,omitempty" json:"image,omitempty"` - Requires Requires `yaml:"requires" json:"requires"` - Values values.Tree `yaml:"values,omitempty" json:"values,omitempty"` + ID string `yaml:"id" json:"id"` + Description string `yaml:"description,omitempty" json:"description,omitempty"` + Link string `yaml:"link,omitempty" json:"link,omitempty"` + Default bool `yaml:"default,omitempty" json:"default,omitempty"` + Engine string `yaml:"engine" json:"engine"` + Chart Chart `yaml:"chart" json:"chart"` + Image *Image `yaml:"image,omitempty" json:"image,omitempty"` + Requires Requires `yaml:"requires" json:"requires"` + // Inputs are chart values every deploy must answer. + Inputs []Input `yaml:"inputs,omitempty" json:"inputs,omitempty"` + Values values.Tree `yaml:"values,omitempty" json:"values,omitempty"` } // Chart carries a name and a version, never a repository: a public catalog @@ -85,8 +87,6 @@ type Requires struct { GPUProduct []string `yaml:"gpuProduct,omitempty" json:"gpuProduct,omitempty"` MinGPUMemoryGiB float64 `yaml:"minGpuMemoryGiB,omitempty" json:"minGpuMemoryGiB,omitempty"` RDMA bool `yaml:"rdma,omitempty" json:"rdma,omitempty"` - // Inputs are chart values every deploy must answer. - Inputs []Input `yaml:"inputs,omitempty" json:"inputs,omitempty"` } const VendorNvidia = "nvidia" diff --git a/internal/catalog/inputs.go b/internal/catalog/inputs.go index 00f96ce..389aab0 100644 --- a/internal/catalog/inputs.go +++ b/internal/catalog/inputs.go @@ -9,7 +9,7 @@ import ( "github.com/modelsphere/swiss/internal/values" ) -// Input is one requires.inputs entry. Its rules repeat the catalog's +// Input is one inputs entry. Its rules repeat the catalog's // hack/lib/inputs.js: the catalog is untrusted where it is used. type Input struct { // Path is the chart value, dotted: storageKVOffload.mooncake.master. @@ -98,10 +98,10 @@ func isInteger(v any) bool { func checkInputs(inputs []Input, vals values.Tree) error { declared := map[string]bool{} for _, in := range inputs { - where := "requires.inputs " + in.Path + where := "inputs " + in.Path switch { case !ValidInputPath(in.Path): - return fmt.Errorf("requires.inputs: path %q is not a dotted chart path", in.Path) + return fmt.Errorf("inputs: path %q is not a dotted chart path", in.Path) case in.Path == "model.localPath": return fmt.Errorf("%s: swiss answers it from the site's model.pathTemplate, so it is never an input", where) case declared[in.Path]: diff --git a/internal/catalog/inputs_test.go b/internal/catalog/inputs_test.go index f874b91..486a725 100644 --- a/internal/catalog/inputs_test.go +++ b/internal/catalog/inputs_test.go @@ -21,7 +21,8 @@ func inputVariant(inputs []Input, vals values.Tree) Variant { ID: "a", Engine: "sglang", Chart: Chart{Name: "sglang", Version: "0.9.32"}, - Requires: Requires{GPUs: 8, Inputs: inputs}, + Requires: Requires{GPUs: 8}, + Inputs: inputs, Values: vals, } } @@ -78,13 +79,13 @@ func TestAnIntegerInputTakesAWholeNumberOfAnyKind(t *testing.T) { } } -// requires.inputs is a known key in both places a v2 variant is read strictly: -// its file, and its entry in the index. -func TestV2InputsAreReadFromTheFileAndTheIndex(t *testing.T) { +// inputs is a known key of a v2 variant file, read strictly. The index carries +// no inputs: nothing lists or picks a variant by them. +func TestV2InputsAreReadFromTheFile(t *testing.T) { raw := []byte("apiVersion: catalog.swiss/v2\nkind: Variant\nmodel: m\nid: a\nversion: 1.0.0\nengine: sglang\nchart: {name: sglang, version: \"0.9.32\"}\n" + - "requires:\n gpus: 1\n inputs:\n - {path: storageKVOffload.mooncake.master, title: Mooncake master, example: \"h:1\", default: \"d:1\"}\n") + "requires:\n gpus: 1\ninputs:\n - {path: storageKVOffload.mooncake.master, title: Mooncake master, example: \"h:1\", default: \"d:1\"}\n") f := memFetcher{"models/m/a.yaml": raw} - f["index-v2.json"] = []byte(fmt.Sprintf(`{"apiVersion":"catalog.swiss/v2","count":1,"models":[{"name":"m","source":{"hf":"org/m"},"variants":[{"id":"a","version":"1.0.0","path":"models/m/a.yaml","digest":%q,"engine":"sglang","chart":{"name":"sglang","version":"0.9.32"},"requires":{"gpus":1,"inputs":[{"path":"storageKVOffload.mooncake.master","title":"Mooncake master","example":"h:1","default":"d:1"}]}}]}]}`, contentRef(raw))) + f["index-v2.json"] = []byte(fmt.Sprintf(`{"apiVersion":"catalog.swiss/v2","count":1,"models":[{"name":"m","source":{"hf":"org/m"},"variants":[{"id":"a","version":"1.0.0","path":"models/m/a.yaml","digest":%q,"engine":"sglang","chart":{"name":"sglang","version":"0.9.32"},"requires":{"gpus":1}}]}]}`, contentRef(raw))) c, err := OpenFetcher(context.Background(), f) if err != nil { t.Fatal(err) @@ -93,7 +94,7 @@ func TestV2InputsAreReadFromTheFileAndTheIndex(t *testing.T) { if err != nil { t.Fatal(err) } - if in := r.Variant.Requires.Inputs; len(in) != 1 || in[0].Path != "storageKVOffload.mooncake.master" || in[0].Example != "h:1" || in[0].Default != "d:1" { + if in := r.Variant.Inputs; len(in) != 1 || in[0].Path != "storageKVOffload.mooncake.master" || in[0].Example != "h:1" || in[0].Default != "d:1" { t.Errorf("inputs = %+v", in) } } diff --git a/internal/catalog/load.go b/internal/catalog/load.go index 921a6a3..f5d02be 100644 --- a/internal/catalog/load.go +++ b/internal/catalog/load.go @@ -352,7 +352,7 @@ func (v Variant) validate() error { return fmt.Errorf("%s: values.lws.size (%v) != requires.nodes (%d)", where, size, v.Requires.NodesOrDefault()) } } - if err := checkInputs(v.Requires.Inputs, v.Values); err != nil { + if err := checkInputs(v.Inputs, v.Values); err != nil { return fmt.Errorf("%s: %w", where, err) } return nil diff --git a/internal/catalog/variant_v2.go b/internal/catalog/variant_v2.go index 22d1ba4..377b3a3 100644 --- a/internal/catalog/variant_v2.go +++ b/internal/catalog/variant_v2.go @@ -23,6 +23,7 @@ type VariantV2 struct { Chart Chart `yaml:"chart" json:"chart"` Image *Image `yaml:"image,omitempty" json:"image,omitempty"` Requires Requires `yaml:"requires" json:"requires"` + Inputs []Input `yaml:"inputs,omitempty" json:"inputs,omitempty"` Values values.Tree `yaml:"values,omitempty" json:"values,omitempty"` // Deprecated comes before deleting a file; a string is the reason. Deprecated any `yaml:"deprecated,omitempty" json:"deprecated,omitempty"` @@ -57,6 +58,6 @@ func ParseVariantV2(raw []byte) (*VariantV2, error) { func (d *VariantV2) variant(isDefault bool) Variant { return Variant{ ID: d.ID, Description: d.Description, Link: d.Link, Default: isDefault, - Engine: d.Engine, Chart: d.Chart, Image: d.Image, Requires: d.Requires, Values: d.Values, + Engine: d.Engine, Chart: d.Chart, Image: d.Image, Requires: d.Requires, Inputs: d.Inputs, Values: d.Values, } } diff --git a/internal/compose/compose.go b/internal/compose/compose.go index 427a937..b732cd3 100644 --- a/internal/compose/compose.go +++ b/internal/compose/compose.go @@ -172,7 +172,7 @@ func Compose(in Input) (*plan.Plan, error) { CreateNamespace: in.CreateNamespace, Layers: layers, } - if err := checkInputs(in.Resolved.Variant.Requires.Inputs, p.Values()); err != nil { + if err := checkInputs(in.Resolved.Variant.Inputs, p.Values()); err != nil { return nil, err } if err := p.ComputeHash(); err != nil { @@ -388,7 +388,7 @@ func applyDefaults(site, derived, base values.Tree, in Input) error { // applyInputs puts each input's answer in the derived layer, under the form's // overrides: the site's, else the input's default. func applyInputs(derived, site values.Tree, in Input) error { - for _, input := range in.Resolved.Variant.Requires.Inputs { + for _, input := range in.Resolved.Variant.Inputs { answer, ok, err := in.Profile.InputAnswer(input.Path, in.Resolved.Source.HF, in.Resolved.Model) if err != nil { return err @@ -426,9 +426,9 @@ func checkInputs(inputs []catalog.Input, composed values.Tree) error { case 0: return nil case 1: - return fmt.Errorf("input %s is not set: set it in the deploy's overrides, or in the site profile's features.inputs", missing[0]) + return fmt.Errorf("input %s is not set: set it in the deploy's overrides, or in the site profile's inputs", missing[0]) default: - return fmt.Errorf("inputs %s are not set: set them in the deploy's overrides, or in the site profile's features.inputs", strings.Join(missing, ", ")) + return fmt.Errorf("inputs %s are not set: set them in the deploy's overrides, or in the site profile's inputs", strings.Join(missing, ", ")) } } diff --git a/internal/compose/inputs_test.go b/internal/compose/inputs_test.go index 8e14f3c..d2ddbbc 100644 --- a/internal/compose/inputs_test.go +++ b/internal/compose/inputs_test.go @@ -12,7 +12,7 @@ const masterPath = "storageKVOffload.mooncake.master" func inputsInput() Input { in := testInput() - in.Resolved.Variant.Requires.Inputs = []catalog.Input{{ + in.Resolved.Variant.Inputs = []catalog.Input{{ Path: masterPath, Title: "Mooncake master", Pattern: `[^\s:/]+:[0-9]+`, @@ -26,7 +26,7 @@ func inputsInput() Input { // values under the same section. func TestTheSiteAnswersAnInputInTheDerivedLayer(t *testing.T) { in := inputsInput() - in.Profile.Features.Inputs = map[string]any{masterPath: "mc-{{model}}.svc:50051"} + in.Profile.Inputs = map[string]any{masterPath: "mc-{{model}}.svc:50051"} p, err := Compose(in) if err != nil { t.Fatal(err) @@ -49,7 +49,7 @@ func TestTheDeployOverridesTheSitesAnswer(t *testing.T) { "with no site answer": nil, } { in := inputsInput() - in.Profile.Features.Inputs = site + in.Profile.Inputs = site in.Overrides = values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": "mine.svc:50051"}}} p, err := Compose(in) if err != nil { @@ -75,7 +75,7 @@ func TestAnUnansweredInputRefusesThePlan(t *testing.T) { } func withDefault(in Input, def any) Input { - in.Resolved.Variant.Requires.Inputs[0].Default = def + in.Resolved.Variant.Inputs[0].Default = def return in } @@ -92,7 +92,7 @@ func TestTheVariantsDefaultIsTheLowestAnswer(t *testing.T) { "the deploy over the two": {site: map[string]any{masterPath: "site.svc:50051"}, form: map[string]any{"master": "mine.svc:50051"}, want: "mine.svc:50051", layer: values.LayerForm}, } { in := withDefault(inputsInput(), "tuned.svc:50051") - in.Profile.Features.Inputs = c.site + in.Profile.Inputs = c.site if c.form != nil { in.Overrides = values.Tree{"storageKVOffload": map[string]any{"mooncake": c.form}} } @@ -108,7 +108,7 @@ func TestTheVariantsDefaultIsTheLowestAnswer(t *testing.T) { func TestAnAnswerTheInputDoesNotTakeRefusesThePlan(t *testing.T) { in := inputsInput() - in.Profile.Features.Inputs = map[string]any{masterPath: "no-port"} + in.Profile.Inputs = map[string]any{masterPath: "no-port"} if _, err := Compose(in); err == nil || !strings.Contains(err.Error(), `"no-port" does not match`) { t.Fatalf("want a refusal quoting the answer, got %v", err) } @@ -118,7 +118,7 @@ func TestAnAnswerTheInputDoesNotTakeRefusesThePlan(t *testing.T) { // site answer never lands in a deploy that did not ask. func TestASiteAnswerReachesOnlyAVariantThatDeclaresIt(t *testing.T) { in := testInput() - in.Profile.Features.Inputs = map[string]any{masterPath: "site.svc:50051"} + in.Profile.Inputs = map[string]any{masterPath: "site.svc:50051"} p, err := Compose(in) if err != nil { t.Fatal(err) diff --git a/internal/site/features_test.go b/internal/site/inputs_test.go similarity index 57% rename from internal/site/features_test.go rename to internal/site/inputs_test.go index 3f34d71..1629ce4 100644 --- a/internal/site/features_test.go +++ b/internal/site/inputs_test.go @@ -5,15 +5,14 @@ import ( "testing" ) -const featuresHead = "name: prod\nmodel:\n pathTemplate: /models/{{name}}\nnodes:\n gpus: []\n" +const inputsHead = "name: prod\nmodel:\n pathTemplate: /models/{{name}}\nnodes:\n gpus: []\n" func TestInputAnswersFillTheModelPlaceholders(t *testing.T) { - p, err := Parse([]byte(featuresHead+`features: - inputs: - storageKVOffload.mooncake.master: mooncake-master.svc:50051 - engineCache.hostPath: /data/cache/{{model}}/{{org}} - service.port: 8050 - shm.enabled: true + p, err := Parse([]byte(inputsHead+`inputs: + storageKVOffload.mooncake.master: mooncake-master.svc:50051 + engineCache.hostPath: /data/cache/{{model}}/{{org}} + service.port: 8050 + shm.enabled: true `), "test") if err != nil { t.Fatal(err) @@ -36,7 +35,7 @@ func TestInputAnswersFillTheModelPlaceholders(t *testing.T) { // A model with no weights has no repo id for a template to name. func TestATemplateNamingTheRepoNeedsASource(t *testing.T) { - p := Profile{Features: Features{Inputs: map[string]any{"a.b": "/data/{{name}}", "c.d": "/data/{{model}}"}}} + p := Profile{Inputs: map[string]any{"a.b": "/data/{{name}}", "c.d": "/data/{{model}}"}} if _, _, err := p.InputAnswer("a.b", "", "mock"); err == nil || !strings.Contains(err.Error(), "no source.hf") { t.Errorf("want a refusal naming source.hf, got %v", err) } @@ -47,13 +46,13 @@ func TestATemplateNamingTheRepoNeedsASource(t *testing.T) { func TestAnAnswerThatCouldNeverBeUsedIsRefused(t *testing.T) { for name, c := range map[string]struct{ yaml, want string }{ - "unknown placeholder": {"features:\n inputs:\n a.b: /x/{{release}}\n", "unknown placeholder"}, - "not a scalar": {"features:\n inputs:\n a.b: {host: h}\n", "string, integer or boolean"}, - "no value": {"features:\n inputs:\n a.b:\n", "string, integer or boolean"}, - "not a path": {"features:\n inputs:\n a..b: x\n", "not a dotted chart path"}, - "extra answers too": {"extra:\n a:\n b: y\nfeatures:\n inputs:\n a.b: x\n", "extra sets the same path"}, + "unknown placeholder": {"inputs:\n a.b: /x/{{release}}\n", "unknown placeholder"}, + "not a scalar": {"inputs:\n a.b: {host: h}\n", "string, integer or boolean"}, + "no value": {"inputs:\n a.b:\n", "string, integer or boolean"}, + "not a path": {"inputs:\n a..b: x\n", "not a dotted chart path"}, + "extra answers too": {"extra:\n a:\n b: y\ninputs:\n a.b: x\n", "extra sets the same path"}, } { - if _, err := Parse([]byte(featuresHead+c.yaml), "test"); err == nil || !strings.Contains(err.Error(), c.want) { + if _, err := Parse([]byte(inputsHead+c.yaml), "test"); err == nil || !strings.Contains(err.Error(), c.want) { t.Errorf("%s: want an error containing %q, got %v", name, c.want, err) } } diff --git a/internal/site/profile.go b/internal/site/profile.go index 6046206..0c9f91f 100644 --- a/internal/site/profile.go +++ b/internal/site/profile.go @@ -78,8 +78,9 @@ type Profile struct { // and composes above the catalog. Defaults Defaults `yaml:"defaults,omitempty" json:"defaults"` - // Features answers a variant's requires. Unrelated to Defaults.Features. - Features Features `yaml:"features,omitempty" json:"features"` + // Inputs answer a variant's inputs by chart path, only for a variant that + // declares the path -- unlike extra. Strings take pathTemplate's placeholders. + Inputs map[string]any `yaml:"inputs,omitempty" json:"inputs,omitempty"` // Sites are the other clusters' swissd instances, for the nav switcher. // @@ -207,20 +208,14 @@ func (f FeatureDefaults) Resolve() map[string]bool { } } -type Features struct { - // Inputs answer requires.inputs by chart path, only for a variant that - // declares the path -- unlike extra. Strings take pathTemplate's placeholders. - Inputs map[string]any `yaml:"inputs,omitempty" json:"inputs,omitempty"` -} - -// check refuses an answer that could never be used, or a second answer from extra. -func (f Features) check(extra values.Tree) error { - for _, path := range slices.Sorted(maps.Keys(f.Inputs)) { - where := fmt.Sprintf("features.inputs[%q]", path) +// checkInputs refuses an answer that could never be used, or a second answer from extra. +func checkInputs(inputs map[string]any, extra values.Tree) error { + for _, path := range slices.Sorted(maps.Keys(inputs)) { + where := fmt.Sprintf("inputs[%q]", path) if !catalog.ValidInputPath(path) { return fmt.Errorf("%s: not a dotted chart path", where) } - switch v := f.Inputs[path].(type) { + switch v := inputs[path].(type) { case string: if _, err := fill(v, "org/name", "model"); err != nil { return fmt.Errorf("%s: %w", where, err) @@ -238,7 +233,7 @@ func (f Features) check(extra values.Tree) error { // InputAnswer is the site's answer to an input, placeholders filled for this model. func (p Profile) InputAnswer(path, hf, model string) (answer any, ok bool, err error) { - v, ok := p.Features.Inputs[path] + v, ok := p.Inputs[path] if !ok { return nil, false, nil } @@ -248,7 +243,7 @@ func (p Profile) InputAnswer(path, hf, model string) (answer any, ok bool, err e } out, err := fill(s, hf, model) if err != nil { - return nil, true, fmt.Errorf("features.inputs[%q]: %w", path, err) + return nil, true, fmt.Errorf("inputs[%q]: %w", path, err) } return out, true, nil } @@ -558,7 +553,7 @@ func Parse(raw []byte, origin string) (*Profile, error) { if err := p.Nodes.checkGPUs(); err != nil { return nil, fmt.Errorf("%s: %w", origin, err) } - if err := p.Features.check(p.Extra); err != nil { + if err := checkInputs(p.Inputs, p.Extra); err != nil { return nil, fmt.Errorf("%s: %w", origin, err) } return &p, nil From cd480738542cef26a9c56e4645ec71d6ef6356d1 Mon Sep 17 00:00:00 2001 From: aceforeverd Date: Tue, 10 Nov 2026 11:45:53 +0800 Subject: [PATCH 6/7] feat(catalog): read an input's chart value from inputs[].key catalog.Input.Path becomes Key, and ValidInputPath ValidInputKey; the site profile's inputs are keyed the same way. --- internal/catalog/inputs.go | 24 +++++++++++------------ internal/catalog/inputs_test.go | 20 +++++++++---------- internal/compose/compose.go | 12 ++++++------ internal/compose/inputs_test.go | 34 ++++++++++++++++----------------- internal/site/inputs_test.go | 12 ++++++------ internal/site/profile.go | 24 +++++++++++------------ 6 files changed, 63 insertions(+), 63 deletions(-) diff --git a/internal/catalog/inputs.go b/internal/catalog/inputs.go index 389aab0..ea9e7ec 100644 --- a/internal/catalog/inputs.go +++ b/internal/catalog/inputs.go @@ -12,8 +12,8 @@ import ( // Input is one inputs entry. Its rules repeat the catalog's // hack/lib/inputs.js: the catalog is untrusted where it is used. type Input struct { - // Path is the chart value, dotted: storageKVOffload.mooncake.master. - Path string `yaml:"path" json:"path"` + // Key is the chart value's dotted key, as helm --set takes it: storageKVOffload.mooncake.master. + Key string `yaml:"key" json:"key"` Title string `yaml:"title" json:"title"` Description string `yaml:"description,omitempty" json:"description,omitempty"` // Type is string, integer or boolean. Empty is string. @@ -32,10 +32,10 @@ const ( InputBoolean = "boolean" ) -var inputPath = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_-]*(\.[A-Za-z_][A-Za-z0-9_-]*)*$`) +var inputKey = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_-]*(\.[A-Za-z_][A-Za-z0-9_-]*)*$`) -// ValidInputPath is shared with the site profile, which keys its answers by these paths. -func ValidInputPath(p string) bool { return inputPath.MatchString(p) } +// ValidInputKey is shared with the site profile, which keys its answers the same way. +func ValidInputKey(k string) bool { return inputKey.MatchString(k) } func (i Input) TypeOrDefault() string { if i.Type == "" { @@ -98,21 +98,21 @@ func isInteger(v any) bool { func checkInputs(inputs []Input, vals values.Tree) error { declared := map[string]bool{} for _, in := range inputs { - where := "inputs " + in.Path + where := "inputs " + in.Key switch { - case !ValidInputPath(in.Path): - return fmt.Errorf("inputs: path %q is not a dotted chart path", in.Path) - case in.Path == "model.localPath": + case !ValidInputKey(in.Key): + return fmt.Errorf("inputs: key %q is not a dotted chart key", in.Key) + case in.Key == "model.localPath": return fmt.Errorf("%s: swiss answers it from the site's model.pathTemplate, so it is never an input", where) - case declared[in.Path]: + case declared[in.Key]: return fmt.Errorf("%s: declared twice", where) case strings.TrimSpace(in.Title) == "": return fmt.Errorf("%s: title is required", where) case in.Pattern != "" && in.TypeOrDefault() != InputString: return fmt.Errorf("%s: pattern is only for a string input, and this one is %s", where, in.TypeOrDefault()) } - declared[in.Path] = true - if at := setAt(vals, strings.Split(in.Path, ".")); at != "" { + declared[in.Key] = true + if at := setAt(vals, strings.Split(in.Key, ".")); at != "" { return fmt.Errorf("%s: values.%s sets it; give the value as the input's default", where, at) } if in.Pattern != "" { diff --git a/internal/catalog/inputs_test.go b/internal/catalog/inputs_test.go index 486a725..018a62b 100644 --- a/internal/catalog/inputs_test.go +++ b/internal/catalog/inputs_test.go @@ -10,7 +10,7 @@ import ( ) var mooncakeMaster = Input{ - Path: "storageKVOffload.mooncake.master", + Key: "storageKVOffload.mooncake.master", Title: "Mooncake master", Pattern: `[^\s:/]+:[0-9]+`, Example: "mooncake-master.example:50051", @@ -48,17 +48,17 @@ func TestAnInputsDefaultAndExampleAreAnswers(t *testing.T) { "a non-map above it": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": "off"}}), "values.storageKVOffload.mooncake sets it"}, "default off pattern": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Default = "no-port" })), nil), `default "no-port" does not match`}, "a blank default": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Default = "" })), nil), `default "" does not match`}, - "default of a type": {inputVariant(one(Input{Path: "a.b", Title: "t", Type: InputInteger, Example: 1, Default: "8"}), nil), "default 8 is not an integer"}, + "default of a type": {inputVariant(one(Input{Key: "a.b", Title: "t", Type: InputInteger, Example: 1, Default: "8"}), nil), "default 8 is not an integer"}, "declared twice": {inputVariant([]Input{mooncakeMaster, mooncakeMaster}, nil), "declared twice"}, - "the model path": {inputVariant(one(Input{Path: "model.localPath", Title: "p", Example: "/m"}), nil), "never an input"}, - "not a path": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Path = "a..b" })), nil), "not a dotted chart path"}, + "the model path": {inputVariant(one(Input{Key: "model.localPath", Title: "p", Example: "/m"}), nil), "never an input"}, + "not a key": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Key = "a..b" })), nil), "not a dotted chart key"}, "no title": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Title = " " })), nil), "title is required"}, "no example": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Example = nil })), nil), "example is required"}, "example off pattern": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Example = "no-port" })), nil), `example "no-port" does not match`}, "pattern compiles": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Pattern = "(" })), nil), "does not compile"}, - "pattern on integer": {inputVariant(one(Input{Path: "a.b", Title: "t", Type: InputInteger, Pattern: "[0-9]+", Example: 1}), nil), "only for a string input"}, - "example of a type": {inputVariant(one(Input{Path: "a.b", Title: "t", Type: InputInteger, Example: "8"}), nil), "8 is not an integer"}, - "unknown type": {inputVariant(one(Input{Path: "a.b", Title: "t", Type: "list", Example: "x"}), nil), `type "list" is not`}, + "pattern on integer": {inputVariant(one(Input{Key: "a.b", Title: "t", Type: InputInteger, Pattern: "[0-9]+", Example: 1}), nil), "only for a string input"}, + "example of a type": {inputVariant(one(Input{Key: "a.b", Title: "t", Type: InputInteger, Example: "8"}), nil), "8 is not an integer"}, + "unknown type": {inputVariant(one(Input{Key: "a.b", Title: "t", Type: "list", Example: "x"}), nil), `type "list" is not`}, } { if err := c.v.validate(); err == nil || !strings.Contains(err.Error(), c.want) { t.Errorf("%s: want an error containing %q, got %v", name, c.want, err) @@ -68,7 +68,7 @@ func TestAnInputsDefaultAndExampleAreAnswers(t *testing.T) { // A number read from JSON is a float64: a whole one answers an integer input. func TestAnIntegerInputTakesAWholeNumberOfAnyKind(t *testing.T) { - port := Input{Path: "a.port", Title: "Port", Type: InputInteger, Example: 1} + port := Input{Key: "a.port", Title: "Port", Type: InputInteger, Example: 1} for _, v := range []any{8080, int64(8080), float64(8080)} { if err := port.Check(v); err != nil { t.Errorf("%T %v: %v", v, v, err) @@ -83,7 +83,7 @@ func TestAnIntegerInputTakesAWholeNumberOfAnyKind(t *testing.T) { // no inputs: nothing lists or picks a variant by them. func TestV2InputsAreReadFromTheFile(t *testing.T) { raw := []byte("apiVersion: catalog.swiss/v2\nkind: Variant\nmodel: m\nid: a\nversion: 1.0.0\nengine: sglang\nchart: {name: sglang, version: \"0.9.32\"}\n" + - "requires:\n gpus: 1\ninputs:\n - {path: storageKVOffload.mooncake.master, title: Mooncake master, example: \"h:1\", default: \"d:1\"}\n") + "requires:\n gpus: 1\ninputs:\n - {key: storageKVOffload.mooncake.master, title: Mooncake master, example: \"h:1\", default: \"d:1\"}\n") f := memFetcher{"models/m/a.yaml": raw} f["index-v2.json"] = []byte(fmt.Sprintf(`{"apiVersion":"catalog.swiss/v2","count":1,"models":[{"name":"m","source":{"hf":"org/m"},"variants":[{"id":"a","version":"1.0.0","path":"models/m/a.yaml","digest":%q,"engine":"sglang","chart":{"name":"sglang","version":"0.9.32"},"requires":{"gpus":1}}]}]}`, contentRef(raw))) c, err := OpenFetcher(context.Background(), f) @@ -94,7 +94,7 @@ func TestV2InputsAreReadFromTheFile(t *testing.T) { if err != nil { t.Fatal(err) } - if in := r.Variant.Inputs; len(in) != 1 || in[0].Path != "storageKVOffload.mooncake.master" || in[0].Example != "h:1" || in[0].Default != "d:1" { + if in := r.Variant.Inputs; len(in) != 1 || in[0].Key != "storageKVOffload.mooncake.master" || in[0].Example != "h:1" || in[0].Default != "d:1" { t.Errorf("inputs = %+v", in) } } diff --git a/internal/compose/compose.go b/internal/compose/compose.go index b732cd3..4b855d5 100644 --- a/internal/compose/compose.go +++ b/internal/compose/compose.go @@ -389,7 +389,7 @@ func applyDefaults(site, derived, base values.Tree, in Input) error { // overrides: the site's, else the input's default. func applyInputs(derived, site values.Tree, in Input) error { for _, input := range in.Resolved.Variant.Inputs { - answer, ok, err := in.Profile.InputAnswer(input.Path, in.Resolved.Source.HF, in.Resolved.Model) + answer, ok, err := in.Profile.InputAnswer(input.Key, in.Resolved.Source.HF, in.Resolved.Model) if err != nil { return err } @@ -399,10 +399,10 @@ func applyInputs(derived, site values.Tree, in Input) error { if !ok { continue } - if _, set := values.Get(site, input.Path); set { + if _, set := values.Get(site, input.Key); set { continue } - if err := values.Set(derived, input.Path, answer); err != nil { + if err := values.Set(derived, input.Key, answer); err != nil { return err } } @@ -413,13 +413,13 @@ func applyInputs(derived, site values.Tree, in Input) error { func checkInputs(inputs []catalog.Input, composed values.Tree) error { var missing []string for _, input := range inputs { - v, ok := values.Get(composed, input.Path) + v, ok := values.Get(composed, input.Key) if !ok || v == nil { - missing = append(missing, fmt.Sprintf("%s (%s)", input.Path, input.Title)) + missing = append(missing, fmt.Sprintf("%s (%s)", input.Key, input.Title)) continue } if err := input.Check(v); err != nil { - return fmt.Errorf("input %s (%s): %w", input.Path, input.Title, err) + return fmt.Errorf("input %s (%s): %w", input.Key, input.Title, err) } } switch len(missing) { diff --git a/internal/compose/inputs_test.go b/internal/compose/inputs_test.go index d2ddbbc..322143c 100644 --- a/internal/compose/inputs_test.go +++ b/internal/compose/inputs_test.go @@ -8,12 +8,12 @@ import ( "github.com/modelsphere/swiss/internal/values" ) -const masterPath = "storageKVOffload.mooncake.master" +const masterKey = "storageKVOffload.mooncake.master" func inputsInput() Input { in := testInput() in.Resolved.Variant.Inputs = []catalog.Input{{ - Path: masterPath, + Key: masterKey, Title: "Mooncake master", Pattern: `[^\s:/]+:[0-9]+`, Example: "mooncake-master.example:50051", @@ -26,15 +26,15 @@ func inputsInput() Input { // values under the same section. func TestTheSiteAnswersAnInputInTheDerivedLayer(t *testing.T) { in := inputsInput() - in.Profile.Inputs = map[string]any{masterPath: "mc-{{model}}.svc:50051"} + in.Profile.Inputs = map[string]any{masterKey: "mc-{{model}}.svc:50051"} p, err := Compose(in) if err != nil { t.Fatal(err) } - if got, _ := values.Get(p.Values(), masterPath); got != "mc-glm-5.3.svc:50051" { - t.Errorf("%s = %v", masterPath, got) + if got, _ := values.Get(p.Values(), masterKey); got != "mc-glm-5.3.svc:50051" { + t.Errorf("%s = %v", masterKey, got) } - if got := p.LayerOf(masterPath); got != values.LayerDerived { + if got := p.LayerOf(masterKey); got != values.LayerDerived { t.Errorf("answered by %s, want derived", got) } if got, _ := values.Get(p.Values(), "storageKVOffload.mooncake.protocol"); got != "rdma" { @@ -45,7 +45,7 @@ func TestTheSiteAnswersAnInputInTheDerivedLayer(t *testing.T) { // Level two: the deployer overrides any answer, and needs no site answer to. func TestTheDeployOverridesTheSitesAnswer(t *testing.T) { for name, site := range map[string]map[string]any{ - "over the site's answer": {masterPath: "site.svc:50051"}, + "over the site's answer": {masterKey: "site.svc:50051"}, "with no site answer": nil, } { in := inputsInput() @@ -55,8 +55,8 @@ func TestTheDeployOverridesTheSitesAnswer(t *testing.T) { if err != nil { t.Fatalf("%s: %v", name, err) } - if got, _ := values.Get(p.Values(), masterPath); got != "mine.svc:50051" || p.LayerOf(masterPath) != values.LayerForm { - t.Errorf("%s: %s = %v from %s, want the deploy's", name, masterPath, got, p.LayerOf(masterPath)) + if got, _ := values.Get(p.Values(), masterKey); got != "mine.svc:50051" || p.LayerOf(masterKey) != values.LayerForm { + t.Errorf("%s: %s = %v from %s, want the deploy's", name, masterKey, got, p.LayerOf(masterKey)) } } } @@ -88,8 +88,8 @@ func TestTheVariantsDefaultIsTheLowestAnswer(t *testing.T) { layer string }{ "the default alone": {want: "tuned.svc:50051", layer: values.LayerDerived}, - "the site over it": {site: map[string]any{masterPath: "site.svc:50051"}, want: "site.svc:50051", layer: values.LayerDerived}, - "the deploy over the two": {site: map[string]any{masterPath: "site.svc:50051"}, form: map[string]any{"master": "mine.svc:50051"}, want: "mine.svc:50051", layer: values.LayerForm}, + "the site over it": {site: map[string]any{masterKey: "site.svc:50051"}, want: "site.svc:50051", layer: values.LayerDerived}, + "the deploy over the two": {site: map[string]any{masterKey: "site.svc:50051"}, form: map[string]any{"master": "mine.svc:50051"}, want: "mine.svc:50051", layer: values.LayerForm}, } { in := withDefault(inputsInput(), "tuned.svc:50051") in.Profile.Inputs = c.site @@ -100,30 +100,30 @@ func TestTheVariantsDefaultIsTheLowestAnswer(t *testing.T) { if err != nil { t.Fatalf("%s: %v", name, err) } - if got, _ := values.Get(p.Values(), masterPath); got != c.want || p.LayerOf(masterPath) != c.layer { - t.Errorf("%s: %s = %v from %s, want %s from %s", name, masterPath, got, p.LayerOf(masterPath), c.want, c.layer) + if got, _ := values.Get(p.Values(), masterKey); got != c.want || p.LayerOf(masterKey) != c.layer { + t.Errorf("%s: %s = %v from %s, want %s from %s", name, masterKey, got, p.LayerOf(masterKey), c.want, c.layer) } } } func TestAnAnswerTheInputDoesNotTakeRefusesThePlan(t *testing.T) { in := inputsInput() - in.Profile.Inputs = map[string]any{masterPath: "no-port"} + in.Profile.Inputs = map[string]any{masterKey: "no-port"} if _, err := Compose(in); err == nil || !strings.Contains(err.Error(), `"no-port" does not match`) { t.Fatalf("want a refusal quoting the answer, got %v", err) } } -// An answer reaches only a variant that declares its path: unlike extra, a +// An answer reaches only a variant that declares its key: unlike extra, a // site answer never lands in a deploy that did not ask. func TestASiteAnswerReachesOnlyAVariantThatDeclaresIt(t *testing.T) { in := testInput() - in.Profile.Inputs = map[string]any{masterPath: "site.svc:50051"} + in.Profile.Inputs = map[string]any{masterKey: "site.svc:50051"} p, err := Compose(in) if err != nil { t.Fatal(err) } - if _, ok := values.Get(p.Values(), masterPath); ok { + if _, ok := values.Get(p.Values(), masterKey); ok { t.Error("a site answer landed in a variant that declares no input") } } diff --git a/internal/site/inputs_test.go b/internal/site/inputs_test.go index 1629ce4..5f5b502 100644 --- a/internal/site/inputs_test.go +++ b/internal/site/inputs_test.go @@ -17,19 +17,19 @@ func TestInputAnswersFillTheModelPlaceholders(t *testing.T) { if err != nil { t.Fatal(err) } - for path, want := range map[string]any{ + for key, want := range map[string]any{ "storageKVOffload.mooncake.master": "mooncake-master.svc:50051", "engineCache.hostPath": "/data/cache/glm-5.3/zai-org", "service.port": 8050, "shm.enabled": true, } { - got, ok, err := p.InputAnswer(path, "zai-org/GLM-5.3", "glm-5.3") + got, ok, err := p.InputAnswer(key, "zai-org/GLM-5.3", "glm-5.3") if err != nil || !ok || got != want { - t.Errorf("%s = %v (ok %v, err %v), want %v", path, got, ok, err, want) + t.Errorf("%s = %v (ok %v, err %v), want %v", key, got, ok, err, want) } } if _, ok, _ := p.InputAnswer("not.answered", "zai-org/GLM-5.3", "glm-5.3"); ok { - t.Error("an unanswered path has an answer") + t.Error("an unanswered key has an answer") } } @@ -49,8 +49,8 @@ func TestAnAnswerThatCouldNeverBeUsedIsRefused(t *testing.T) { "unknown placeholder": {"inputs:\n a.b: /x/{{release}}\n", "unknown placeholder"}, "not a scalar": {"inputs:\n a.b: {host: h}\n", "string, integer or boolean"}, "no value": {"inputs:\n a.b:\n", "string, integer or boolean"}, - "not a path": {"inputs:\n a..b: x\n", "not a dotted chart path"}, - "extra answers too": {"extra:\n a:\n b: y\ninputs:\n a.b: x\n", "extra sets the same path"}, + "not a key": {"inputs:\n a..b: x\n", "not a dotted chart key"}, + "extra answers too": {"extra:\n a:\n b: y\ninputs:\n a.b: x\n", "extra sets the same key"}, } { if _, err := Parse([]byte(inputsHead+c.yaml), "test"); err == nil || !strings.Contains(err.Error(), c.want) { t.Errorf("%s: want an error containing %q, got %v", name, c.want, err) diff --git a/internal/site/profile.go b/internal/site/profile.go index 0c9f91f..9abece5 100644 --- a/internal/site/profile.go +++ b/internal/site/profile.go @@ -78,8 +78,8 @@ type Profile struct { // and composes above the catalog. Defaults Defaults `yaml:"defaults,omitempty" json:"defaults"` - // Inputs answer a variant's inputs by chart path, only for a variant that - // declares the path -- unlike extra. Strings take pathTemplate's placeholders. + // Inputs answer a variant's inputs by key, only for a variant that declares + // the key -- unlike extra. Strings take pathTemplate's placeholders. Inputs map[string]any `yaml:"inputs,omitempty" json:"inputs,omitempty"` // Sites are the other clusters' swissd instances, for the nav switcher. @@ -210,12 +210,12 @@ func (f FeatureDefaults) Resolve() map[string]bool { // checkInputs refuses an answer that could never be used, or a second answer from extra. func checkInputs(inputs map[string]any, extra values.Tree) error { - for _, path := range slices.Sorted(maps.Keys(inputs)) { - where := fmt.Sprintf("inputs[%q]", path) - if !catalog.ValidInputPath(path) { - return fmt.Errorf("%s: not a dotted chart path", where) + for _, key := range slices.Sorted(maps.Keys(inputs)) { + where := fmt.Sprintf("inputs[%q]", key) + if !catalog.ValidInputKey(key) { + return fmt.Errorf("%s: not a dotted chart key", where) } - switch v := inputs[path].(type) { + switch v := inputs[key].(type) { case string: if _, err := fill(v, "org/name", "model"); err != nil { return fmt.Errorf("%s: %w", where, err) @@ -224,16 +224,16 @@ func checkInputs(inputs map[string]any, extra values.Tree) error { default: return fmt.Errorf("%s: an answer is a string, integer or boolean, not %T", where, v) } - if _, set := values.Get(extra, path); set { - return fmt.Errorf("%s: extra sets the same path; give the one answer here", where) + if _, set := values.Get(extra, key); set { + return fmt.Errorf("%s: extra sets the same key; give the one answer here", where) } } return nil } // InputAnswer is the site's answer to an input, placeholders filled for this model. -func (p Profile) InputAnswer(path, hf, model string) (answer any, ok bool, err error) { - v, ok := p.Inputs[path] +func (p Profile) InputAnswer(key, hf, model string) (answer any, ok bool, err error) { + v, ok := p.Inputs[key] if !ok { return nil, false, nil } @@ -243,7 +243,7 @@ func (p Profile) InputAnswer(path, hf, model string) (answer any, ok bool, err e } out, err := fill(s, hf, model) if err != nil { - return nil, true, fmt.Errorf("inputs[%q]: %w", path, err) + return nil, true, fmt.Errorf("inputs[%q]: %w", key, err) } return out, true, nil } From 6b63b9f615965800eda8ad2403743fe47cfeddc8 Mon Sep 17 00:00:00 2001 From: aceforeverd Date: Tue, 10 Nov 2026 12:02:49 +0800 Subject: [PATCH 7/7] docs(api): inputs in the site profile and the resolved variant SiteProfile gains inputs, answers by input key; ResolvedVariant gains inputs, with an Input schema; POST /api/plans says how they are answered and that a plan left without an answer is 400. --- docs/openapi.yaml | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 1d34431..b794545 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -748,6 +748,10 @@ paths: the form layer, model and variant, and only what the request names moves. With `reuseSource: true`, it instead reads the live LLMService plan and merges arbitrary form overrides and optional edit values, without resolving a catalog. + + Each of the variant's `inputs` is answered in the `derived` layer by the + site profile's `inputs`, else the input's `default`; `overrides` go over + it. A plan that leaves one without a valid answer is 400. operationId: composePlan requestBody: required: true @@ -1788,6 +1792,17 @@ components: extra: type: object description: Values every release on this site gets, as the site layer. + inputs: + type: object + description: | + Answers to variants' `inputs`, by each input's key. Unlike `extra`, an + answer reaches only a variant that declares the key, in the `derived` + layer. A string may use `model.pathTemplate`'s placeholders + (`{{hf}}`, `{{org}}`, `{{name}}`, `{{model}}`). A key `extra` also sets is + refused. + additionalProperties: { type: [string, number, boolean] } + examples: + - storageKVOffload.mooncake.master: mooncake-master.svc:50051 Secrets: type: object @@ -1825,6 +1840,19 @@ components: minGpuMemoryGiB: { type: number } rdma: { type: boolean } + Input: + type: object + description: A chart value every deploy must answer. Answered in the `derived` layer by the site profile's `inputs`, else `default`; a deploy's overrides go over it. + required: [key, title, example] + properties: + key: { type: string, description: "The chart value's dotted key.", examples: [storageKVOffload.mooncake.master] } + title: { type: string } + description: { type: string } + type: { type: string, enum: [string, integer, boolean], default: string } + pattern: { type: string, description: "A string answer matches it, whole." } + example: { type: [string, integer, boolean], description: "The form's placeholder; never deployed." } + default: { type: [string, integer, boolean], description: "The answer when the site profile has none." } + ModelVariant: type: object required: [id, version, digest, engine, chart, requires] @@ -1907,6 +1935,7 @@ components: tag: { type: string } digest: { type: string } requires: { $ref: "#/components/schemas/Requires" } + inputs: { type: array, items: { $ref: "#/components/schemas/Input" } } values: { type: object, description: "The catalog layer's chart values." } version: { type: string } digest: { type: string, description: "sha256 of the file this version pins." }