fix(gallery): persist inference defaults where the loader reads them (#11232)

The recommended sampling parameters for a model family were applied at
install and then never took effect. Two things went wrong on the way to
disk.

They were written as top level keys. ModelConfig embeds PredictionOptions
under the "parameters" yaml key, so temperature, top_p, top_k, min_p,
repeat_penalty and presence_penalty are only read from there. At the top
level they parse without error and are then ignored for the life of the
model.

They were also merged in after the YAML had already been marshalled. The
only re-marshal sat behind the artifact binding, which an entry carrying
files: never reaches, so for those entries the defaults were computed and
then dropped before anything was written.

Neither failure was visible in normal use. ApplyInferenceDefaults runs
again at load time and fills the same values from the same table, so the
model ends up tuned correctly while the file on disk pins nothing. It
surfaces when someone edits one of those values expecting it to win, or
when a family is absent from inference_defaults.json and there is nothing
to refill from.

Both install paths are covered: an entry carrying files:, and one that
binds a primary artifact instead.

The empty base spec asserted that the authored parameters block landed
verbatim. It now checks the authored keys individually, because the family
defaults are merged into that same block.

Assisted-by: Claude:claude-opus-5

Signed-off-by: Dimitris Karakasilis <dimitris@karakasilis.me>

core/gallery/empty_base_install_test.go

@@ -236,8 +236,15 @@ var _ = Describe("InstallModelFromGallery with an empty base config", func() {
 		Expect(install(e.Name, gallery.GalleryModel{})).To(Succeed())
 		cfg := installedConfig(e.Name)
 		Expect(cfg["name"]).To(Equal(e.Name))
-		// The catalog's own overrides, verbatim, laid over the empty base.
-		Expect(cfg["parameters"]).To(Equal(e.Overrides["parameters"]))
+		// The catalog's own overrides, laid over the empty base. parameters is
+		// checked key by key rather than as a whole map: the install also merges
+		// the model family's inference defaults into it, and what matters here is
+		// that the authored keys survive that.
+		authored, ok := e.Overrides["parameters"].(map[string]any)
+		Expect(ok).To(BeTrue())
+		for key, want := range authored {
+			Expect(cfg["parameters"]).To(HaveKeyWithValue(key, want))
+		}
 		Expect(cfg["known_usecases"]).To(Equal(e.Overrides["known_usecases"]))
 	})
 })

core/gallery/inference_defaults_install_test.go

@@ -0,0 +1,189 @@
+package gallery_test
+
+import (
+	"context"
+	"fmt"
+	"maps"
+	"os"
+	"path/filepath"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"gopkg.in/yaml.v3"
+
+	"github.com/mudler/LocalAI/core/config"
+	"github.com/mudler/LocalAI/core/gallery"
+	"github.com/mudler/LocalAI/pkg/modelartifacts"
+	"github.com/mudler/LocalAI/pkg/system"
+)
+
+// The recommended sampling parameters for a model family are applied at install
+// and persisted into the model YAML. Persisting them is only worth anything if
+// they are written where the loader reads them back: PredictionOptions is nested
+// under "parameters" in ModelConfig, so a top level "temperature" key parses
+// without error and is then ignored for the life of the model.
+//
+// The expected values are read from the family table rather than written out
+// here, so that retuning a family stays a one file change.
+//
+// Nothing here reaches the network.
+var _ = Describe("Inference defaults persisted at install", func() {
+	var tempdir string
+	var galleries []config.Gallery
+	var systemState *system.SystemState
+	// The gallery listing is cached on the name and URL pair, so every spec
+	// needs a gallery of its own or it reads the previous spec's catalog.
+	galleryRevision := 0
+
+	// The name has to contain a pattern from inference_defaults.json, otherwise
+	// no defaults are applied and every assertion below passes vacuously.
+	const modelName = "qwen3.5-install-defaults"
+
+	newGallery := func(entries ...gallery.GalleryModel) {
+		out, err := yaml.Marshal(entries)
+		Expect(err).ToNot(HaveOccurred())
+		name := fmt.Sprintf("inference-defaults-%d", galleryRevision)
+		galleryRevision++
+		galleryPath := filepath.Join(tempdir, name+".yaml")
+		Expect(os.WriteFile(galleryPath, out, 0600)).To(Succeed())
+		galleries = []config.Gallery{{Name: name, URL: "file://" + galleryPath}}
+	}
+
+	install := func(name string) error {
+		return gallery.InstallModelFromGallery(
+			context.TODO(), galleries, []config.Gallery{}, systemState, nil,
+			name, gallery.GalleryModel{}, func(string, string, string, float64) {}, false, false, false)
+	}
+
+	installedConfig := func(name string) map[string]any {
+		dat, err := os.ReadFile(filepath.Join(tempdir, name+".yaml"))
+		Expect(err).ToNot(HaveOccurred())
+		content := map[string]any{}
+		Expect(yaml.Unmarshal(dat, &content)).To(Succeed())
+		return content
+	}
+
+	// Seeding the weights keeps the install off the network: the downloader
+	// treats an already-present destination with no declared sha256 as fetched.
+	// extra goes into parameters:, so a spec can pin a value the defaults would
+	// otherwise supply.
+	seedGallery := func(extra map[string]any) {
+		Expect(os.WriteFile(filepath.Join(tempdir, "weights.gguf"), []byte("weights"), 0600)).To(Succeed())
+
+		params := map[string]any{"model": "weights.gguf"}
+		maps.Copy(params, extra)
+
+		e := gallery.GalleryModel{Overrides: map[string]any{
+			"backend":    "llama-cpp",
+			"parameters": params,
+		}}
+		e.Name = modelName
+		e.AdditionalFiles = []gallery.File{{Filename: "weights.gguf", URI: "https://example.com/weights.gguf"}}
+		newGallery(e)
+	}
+
+	// Guards the fixture itself. If the name stops matching a family the specs
+	// below would still pass while asserting nothing at all.
+	expectedFamily := func() map[string]float64 {
+		family := config.MatchModelFamily(modelName)
+		Expect(family).ToNot(BeEmpty(), "fixture name no longer matches a family in inference_defaults.json")
+		return family
+	}
+
+	BeforeEach(func() {
+		var err error
+		tempdir, err = os.MkdirTemp("", "inference-defaults-install")
+		Expect(err).ToNot(HaveOccurred())
+		DeferCleanup(func() { Expect(os.RemoveAll(tempdir)).To(Succeed()) })
+
+		systemState, err = system.GetSystemState(system.WithModelPath(tempdir))
+		Expect(err).ToNot(HaveOccurred())
+	})
+
+	It("writes them under parameters, where the loader reads them back", func() {
+		family := expectedFamily()
+		seedGallery(nil)
+
+		Expect(install(modelName)).To(Succeed())
+
+		params, ok := installedConfig(modelName)["parameters"].(map[string]any)
+		Expect(ok).To(BeTrue(), "parameters should be a map")
+
+		for key, want := range family {
+			Expect(params).To(HaveKey(key))
+			Expect(params[key]).To(BeNumerically("==", want), "parameters.%s", key)
+		}
+	})
+
+	It("does not leave them at the top level, where they are ignored", func() {
+		family := expectedFamily()
+		seedGallery(nil)
+
+		Expect(install(modelName)).To(Succeed())
+
+		cfg := installedConfig(modelName)
+		for key := range family {
+			Expect(cfg).ToNot(HaveKey(key), "%s at the top level is never read", key)
+		}
+	})
+
+	It("leaves a value the entry already sets alone", func() {
+		family := expectedFamily()
+		Expect(family).To(HaveKey("temperature"))
+		Expect(family["temperature"]).ToNot(BeNumerically("==", 0.05), "pick a value the family does not use")
+
+		seedGallery(map[string]any{"temperature": 0.05})
+
+		Expect(install(modelName)).To(Succeed())
+
+		params, ok := installedConfig(modelName)["parameters"].(map[string]any)
+		Expect(ok).To(BeTrue(), "parameters should be a map")
+		Expect(params["temperature"]).To(BeNumerically("==", 0.05))
+	})
+
+	// An entry that binds a primary artifact carries no files: of its own, so it
+	// takes the other branch of the install and none of the specs above reach it.
+	// It is also the one branch that already re-marshalled, which is why the
+	// defaults did land on disk there, at the top level where nothing reads them.
+	It("writes them under parameters on the artifact binding path too", func() {
+		family := expectedFamily()
+
+		definition := &gallery.ModelConfig{ConfigFile: `
+backend: transformers
+artifacts:
+  - name: model
+    target: model
+    source:
+      type: huggingface
+      repo: owner/repo
+parameters:
+  model: owner/repo
+`}
+		// Standing in for the materializer keeps the install off the network.
+		materializer := &fakeArtifactMaterializer{result: modelartifacts.Result{
+			Spec: modelartifacts.Spec{
+				Name: "model", Target: "model",
+				Source: modelartifacts.Source{Type: "huggingface", Repo: "owner/repo", Revision: "main"},
+				Resolved: &modelartifacts.Resolved{
+					Endpoint: "https://huggingface.co",
+					Revision: "0123456789abcdef0123456789abcdef01234567",
+					CacheKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
+				},
+			},
+			RelativePath: ".artifacts/huggingface/0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef/snapshot",
+		}}
+
+		_, err := gallery.InstallModel(context.TODO(), systemState, modelName, definition, nil, nil, false,
+			gallery.WithArtifactMaterializer(materializer))
+		Expect(err).ToNot(HaveOccurred())
+
+		cfg := installedConfig(modelName)
+		params, ok := cfg["parameters"].(map[string]any)
+		Expect(ok).To(BeTrue(), "parameters should be a map")
+		for key, want := range family {
+			Expect(params).To(HaveKey(key))
+			Expect(params[key]).To(BeNumerically("==", want), "parameters.%s", key)
+			Expect(cfg).ToNot(HaveKey(key), "%s at the top level is never read", key)
+		}
+	})
+})

core/gallery/models.go

@@ -622,35 +622,51 @@ func InstallModel(ctx context.Context, systemState *system.SystemState, nameOver
 		lconfig.ApplyInferenceDefaults(&modelConfig, name, modelConfig.Model)
 
 		// Merge inference defaults into configMap so they are persisted without losing unknown fields.
-		if modelConfig.Temperature != nil {
-			if _, exists := configMap["temperature"]; !exists {
-				configMap["temperature"] = *modelConfig.Temperature
+		// They belong under "parameters": ModelConfig embeds PredictionOptions with
+		// that yaml key, so a top level "temperature" parses without error and is
+		// then ignored for the life of the model.
+		params, mergeable := configMap["parameters"].(map[string]any)
+		if configMap["parameters"] == nil {
+			params, mergeable = map[string]any{}, true
+		}
+		if mergeable {
+			// An entry that sets one of these keeps its own value. ApplyInferenceDefaults
+			// already skipped those fields; this keeps the write side symmetric.
+			setDefault := func(key string, value any) {
+				if _, exists := params[key]; !exists {
+					params[key] = value
+				}
+			}
+			if modelConfig.Temperature != nil {
+				setDefault("temperature", *modelConfig.Temperature)
+			}
+			if modelConfig.TopP != nil {
+				setDefault("top_p", *modelConfig.TopP)
+			}
+			if modelConfig.TopK != nil {
+				setDefault("top_k", *modelConfig.TopK)
+			}
+			if modelConfig.MinP != nil {
+				setDefault("min_p", *modelConfig.MinP)
+			}
+			if modelConfig.RepeatPenalty != 0 {
+				setDefault("repeat_penalty", modelConfig.RepeatPenalty)
+			}
+			if modelConfig.PresencePenalty != 0 {
+				setDefault("presence_penalty", modelConfig.PresencePenalty)
+			}
+			if len(params) > 0 {
+				configMap["parameters"] = params
 			}
 		}
-		if modelConfig.TopP != nil {
-			if _, exists := configMap["top_p"]; !exists {
-				configMap["top_p"] = *modelConfig.TopP
-			}
-		}
-		if modelConfig.TopK != nil {
-			if _, exists := configMap["top_k"]; !exists {
-				configMap["top_k"] = *modelConfig.TopK
-			}
-		}
-		if modelConfig.MinP != nil {
-			if _, exists := configMap["min_p"]; !exists {
-				configMap["min_p"] = *modelConfig.MinP
-			}
-		}
-		if modelConfig.RepeatPenalty != 0 {
-			if _, exists := configMap["repeat_penalty"]; !exists {
-				configMap["repeat_penalty"] = modelConfig.RepeatPenalty
-			}
-		}
-		if modelConfig.PresencePenalty != 0 {
-			if _, exists := configMap["presence_penalty"]; !exists {
-				configMap["presence_penalty"] = modelConfig.PresencePenalty
-			}
+
+		// The marshal above predates this merge, and the only other re-marshal is
+		// behind the artifact binding below, which an entry carrying files: never
+		// reaches. Without this the defaults are computed and then dropped on the
+		// way to disk.
+		updatedConfigYAML, err = yaml.Marshal(configMap)
+		if err != nil {
+			return nil, fmt.Errorf("failed to marshal config with inference defaults: %v", err)
 		}
 
 		if valid, err := modelConfig.Validate(); !valid {