[pull] master from mudler:master

core/backend/options.go

@@ -202,18 +202,18 @@ func ModelOptions(c config.ModelConfig, so *config.ApplicationConfig, opts ...mo
 		model.WithContext(so.Context),
 		model.WithModelID(c.ModelID()),
 	}
-	// Prefer the revision stamped when the configuration was loaded. c has since
-	// been merged with this request's prediction parameters (temperature, top_p,
-	// stop, ...), and hashing it here would produce a different revision for
-	// every distinct request body — which the controller reads as a config
-	// change and rejects as stale. Recomputing is the fallback for a config that
-	// never passed through the loader.
+	// Use the revision stamped when the configuration was parsed, and only
+	// that. By this point c has been merged with the request's prediction
+	// parameters and had SetDefaults applied, so hashing it here would produce
+	// a revision that depends on the request body and on whether the model file
+	// parsed, which the controller reads as a config change and rejects. Every
+	// config the loader hands out is stamped; an unstamped one was synthesized
+	// elsewhere and is routed without a revision rather than with a wrong one.
 	if revision := c.PersistedConfigRevision(); revision != "" {
 		defOpts = append(defOpts, model.WithConfigRevision(revision))
-	} else if revision, err := config.ModelConfigRevision(&c); err == nil {
-		defOpts = append(defOpts, model.WithConfigRevision(revision))
 	} else {
-		xlog.Warn("Failed to compute model configuration revision", "model", c.ModelID(), "error", err)
+		xlog.Warn("Model configuration carries no revision stamp; routing without one",
+			"model", c.ModelID())
 	}
 	managedPrimary := len(c.Artifacts) > 0 && c.Artifacts[0].Resolved != nil
 	if managedPrimary {

core/config/model_config.go

@@ -1864,7 +1864,7 @@ func (c *ModelConfig) PersistedConfigRevision() string {
 // persisted. It is computed from the receiver as-is, so callers must invoke it
 // only on a configuration that has not been merged with request overrides.
 func (c *ModelConfig) StampPersistedConfigRevision() error {
-	revision, err := ModelConfigRevision(c)
+	revision, err := modelConfigRevision(c)
 	if err != nil {
 		return err
 	}

core/config/model_config_loader.go

@@ -965,3 +965,38 @@ func hasAnyMappingKey(mapping *yaml.Node, keys ...string) bool {
 func nonemptyScalar(node *yaml.Node) bool {
 	return node != nil && node.Kind == yaml.ScalarNode && node.Tag == "!!str" && strings.TrimSpace(node.Value) != ""
 }
+
+// RevisionFor returns the config revision for modelName: the one an inference
+// request for that model will carry.
+//
+// This is the only way to obtain a revision outside this package. Every
+// publisher must use it, so that what is published and what is checked are
+// the same value by construction rather than by two implementations happening
+// to agree. Hashing a ModelConfig directly is not available to callers, because
+// a config that has been through SetDefaults or the request middleware hashes
+// to something no request will ever present.
+func (bcl *ModelConfigLoader) RevisionFor(modelName string, appConfig *ApplicationConfig) (string, error) {
+	cfg, err := bcl.LoadModelConfigFileByNameDefaultOptions(modelName, appConfig)
+	if err != nil {
+		return "", fmt.Errorf("resolving config revision for %q: %w", modelName, err)
+	}
+	return stampedRevision(cfg, modelName)
+}
+
+// RevisionForPath is RevisionFor for callers that hold loader options and a
+// models path rather than an ApplicationConfig.
+func (bcl *ModelConfigLoader) RevisionForPath(modelName, modelPath string, opts ...ConfigLoaderOption) (string, error) {
+	cfg, err := bcl.LoadModelConfigFileByName(modelName, modelPath, opts...)
+	if err != nil {
+		return "", fmt.Errorf("resolving config revision for %q: %w", modelName, err)
+	}
+	return stampedRevision(cfg, modelName)
+}
+
+func stampedRevision(cfg *ModelConfig, modelName string) (string, error) {
+	revision := cfg.PersistedConfigRevision()
+	if revision == "" {
+		return "", fmt.Errorf("no config revision stamped for %q", modelName)
+	}
+	return revision, nil
+}

core/config/model_config_revision.go

@@ -10,10 +10,17 @@ import (
 	"google.golang.org/protobuf/proto"
 )
 
-// ModelConfigRevision returns a stable revision of the persisted semantic
+// modelConfigRevision returns a stable revision of the persisted semantic
 // configuration. ModelConfig's JSON tags exclude runtime-derived state and
 // source bookkeeping, while encoding/json orders map keys deterministically.
-func ModelConfigRevision(cfg *ModelConfig) (string, error) {
+//
+// Deliberately unexported. It must only ever be called on a configuration as
+// parsed from disk, before SetDefaults folds in the GGUF guess, the hardware
+// defaults and app-level options. Callers outside this package cannot tell
+// which they hold, and every time one hashed a defaulted or request-merged
+// config it published a revision no inference request would carry, which makes
+// the model unroutable. Use ModelConfigLoader.RevisionFor instead.
+func modelConfigRevision(cfg *ModelConfig) (string, error) {
 	if cfg == nil {
 		return "", errors.New("model config is nil")
 	}

core/config/model_config_revision_stability_test.go

@@ -51,9 +51,8 @@ template:
 		Expect(loader.LoadModelConfigsFromPath(dir, appConfig.ToConfigLoaderOptions()...)).To(Succeed())
 		cfg, ok := loader.GetModelConfig("example")
 		Expect(ok).To(BeTrue())
-		revision, err := config.ModelConfigRevision(&cfg)
-		Expect(err).ToNot(HaveOccurred())
-		return revision
+		Expect(cfg.PersistedConfigRevision()).ToNot(BeEmpty())
+		return cfg.PersistedConfigRevision()
 	}
 
 	It("does not change when the same file is loaded repeatedly", func() {

core/config/model_config_revision_test.go

@@ -19,10 +19,11 @@ var _ = Describe("Model configuration revisions", func() {
 		return cfg
 	}
 
+	// The raw hash is unexported on purpose, so these specs exercise it the way
+	// every caller now must: by stamping the parsed config.
 	revision := func(cfg *config.ModelConfig) string {
-		value, err := config.ModelConfigRevision(cfg)
-		Expect(err).NotTo(HaveOccurred())
-		return value
+		Expect(cfg.StampPersistedConfigRevision()).To(Succeed())
+		return cfg.PersistedConfigRevision()
 	}
 
 	It("is stable across equivalent YAML formatting and map order", func() {

core/http/endpoints/localai/edit_model_test.go

@@ -294,9 +294,9 @@ var _ = Describe("Edit Model test", func() {
 			Expect(client.published[0]).To(Equal(messaging.CacheInvalidateEvent{
 				Element: "old", Op: "delete", ConfigRevision: modeladmin.DeletedModelConfigRevision("old"),
 			}))
-			newConfig, ok := loader.GetModelConfig("new")
+			_, ok := loader.GetModelConfig("new")
 			Expect(ok).To(BeTrue())
-			newRevision, err := config.ModelConfigRevision(&newConfig)
+			newRevision, err := loader.RevisionForPath("new", tempDir)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(client.published[1]).To(Equal(messaging.CacheInvalidateEvent{
 				Element: "new", Op: "install", ConfigRevision: newRevision,
@@ -313,9 +313,9 @@ var _ = Describe("Edit Model test", func() {
 			}
 			_, oldOnPeer := peerLoader.GetModelConfig("old")
 			Expect(oldOnPeer).To(BeFalse())
-			peerConfig, newOnPeer := peerLoader.GetModelConfig("new")
+			_, newOnPeer := peerLoader.GetModelConfig("new")
 			Expect(newOnPeer).To(BeTrue())
-			peerRevision, err := config.ModelConfigRevision(&peerConfig)
+			peerRevision, err := peerLoader.RevisionForPath("new", tempDir)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(peerRevision).To(Equal(newRevision))
 			Expect(peerLifecycle.batches).To(Equal([][]modeladmin.ModelRevisionTransition{

core/services/modeladmin/config.go

@@ -185,13 +185,9 @@ func (s *ConfigService) patchConfig(ctx context.Context, name string, patch map[
 		// because SetDefaults runs again on the request path and is not
 		// idempotent for every model, and the edit would leave the model
 		// unroutable.
-		resolved, err := s.Loader.LoadModelConfigFileByNameDefaultOptions(updated.Name, s.AppConfig)
+		revision, err := s.Loader.RevisionFor(updated.Name, s.AppConfig)
 		if err != nil {
-			return fmt.Errorf("resolve config revision: %w", err)
-		}
-		revision := resolved.PersistedConfigRevision()
-		if revision == "" {
-			return fmt.Errorf("no config revision stamped for %q", updated.Name)
+			return err
 		}
 		_ = s.Loader.Preload(s.modelsPath())
 		pending, err := s.applyRevision(ctx, name, updated.Name, revision, updated.IsDisabled())
@@ -351,13 +347,12 @@ func (s *ConfigService) editYAML(ctx context.Context, name string, body []byte)
 		if err := s.Loader.LoadModelConfigsFromPath(modelsPath, s.AppConfig.ToConfigLoaderOptions()...); err != nil {
 			return fmt.Errorf("reload configs: %w", err)
 		}
-		loaded, ok := s.Loader.GetModelConfig(req.Name)
-		if !ok {
+		if _, ok := s.Loader.GetModelConfig(req.Name); !ok {
 			return fmt.Errorf("reload configs: model %q missing", req.Name)
 		}
-		revision, err := config.ModelConfigRevision(&loaded)
+		revision, err := s.Loader.RevisionFor(req.Name, s.AppConfig)
 		if err != nil {
-			return fmt.Errorf("compute config revision: %w", err)
+			return err
 		}
 		if err := s.Loader.Preload(modelsPath); err != nil {
 			return fmt.Errorf("preload after edit: %w", err)

core/services/modeladmin/remote_sync.go

@@ -49,9 +49,9 @@ func applyRemoteChange(ctx context.Context, cl *config.ModelConfigLoader, models
 			disabled := true
 			if exists {
 				var err error
-				revision, err = config.ModelConfigRevision(&cfg)
+				revision, err = authoritative.RevisionForPath(name, modelsPath, opts...)
 				if err != nil {
-					return fmt.Errorf("compute authoritative model config revision for %q: %w", name, err)
+					return fmt.Errorf("resolve authoritative model config revision for %q: %w", name, err)
 				}
 				disabled = cfg.IsDisabled()
 			}
@@ -83,15 +83,9 @@ func changedConfigNames(current, snapshot map[string]config.ModelConfig, named s
 			changed[name] = struct{}{}
 			continue
 		}
-		previousRevision, err := config.ModelConfigRevision(&previous)
-		if err != nil {
-			return nil, fmt.Errorf("compute current model config revision for %q: %w", name, err)
-		}
-		revision, err := config.ModelConfigRevision(&cfg)
-		if err != nil {
-			return nil, fmt.Errorf("compute authoritative model config revision for %q: %w", name, err)
-		}
-		if previousRevision != revision {
+		// Both sides come from a loader, so both carry the revision stamped
+		// when their file was parsed. Comparing the stamps compares the files.
+		if previous.PersistedConfigRevision() != cfg.PersistedConfigRevision() {
 			changed[name] = struct{}{}
 		}
 	}

core/services/modeladmin/remote_sync_test.go

@@ -58,9 +58,9 @@ var _ = Describe("ApplyRemoteChange", func() {
 		Expect(ApplyRemoteChange(context.Background(), loader, dir, evt, lifecycle)).To(Succeed())
 		Expect(ApplyRemoteChange(context.Background(), loader, dir, evt, lifecycle)).To(Succeed())
 		Expect(lifecycle.calls).To(HaveLen(2))
-		loaded, ok := loader.GetModelConfig("peer-alias")
+		_, ok := loader.GetModelConfig("peer-alias")
 		Expect(ok).To(BeTrue())
-		revision, err := config.ModelConfigRevision(&loaded)
+		revision, err := loader.RevisionForPath("peer-alias", dir)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(lifecycle.calls[0].revision).To(Equal(revision))
 		Expect(lifecycle.calls[1].revision).To(Equal(revision))
@@ -84,7 +84,7 @@ var _ = Describe("ApplyRemoteChange", func() {
 		loaded, ok := loader.GetModelConfig("peer-alias")
 		Expect(ok).To(BeTrue())
 		Expect(loaded.ContextSize).To(HaveValue(Equal(10000)))
-		revision, err := config.ModelConfigRevision(&loaded)
+		revision, err := loader.RevisionForPath(loaded.Name, dir)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(lifecycle.calls).To(HaveLen(3))
 		Expect(lifecycle.calls[1].revision).To(Equal(revision))
@@ -100,7 +100,7 @@ var _ = Describe("ApplyRemoteChange", func() {
 
 		loaded, ok := loader.GetModelConfig("reinstalled")
 		Expect(ok).To(BeTrue())
-		revision, err := config.ModelConfigRevision(&loaded)
+		revision, err := loader.RevisionForPath(loaded.Name, dir)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(lifecycle.calls).To(HaveLen(1))
 		Expect(lifecycle.calls[0].revision).To(Equal(revision))
@@ -172,7 +172,7 @@ var _ = Describe("ApplyRemoteChange", func() {
 		Expect(loaded.ContextSize).To(HaveValue(Equal(10000)))
 		_, ok = loader.GetModelConfig("deleted")
 		Expect(ok).To(BeFalse())
-		changedRevision, err := config.ModelConfigRevision(&loaded)
+		changedRevision, err := loader.RevisionForPath(loaded.Name, dir)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(lifecycle.calls).To(ConsistOf(
 			revisionLifecycleCall{oldName: "changed", newName: "changed", revision: changedRevision},
@@ -228,7 +228,7 @@ var _ = Describe("ApplyRemoteChange", func() {
 		loaded, ok := loader.GetModelConfig("ordered")
 		Expect(ok).To(BeTrue())
 		Expect(loaded.ContextSize).To(HaveValue(Equal(10000)))
-		revision, err := config.ModelConfigRevision(&loaded)
+		revision, err := loader.RevisionForPath(loaded.Name, dir)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(lifecycle.revisions()).To(HaveLen(2))
 		Expect(lifecycle.revisions()[1]).To(Equal(revision))
@@ -263,7 +263,7 @@ var _ = Describe("ApplyRemoteChange", func() {
 		Expect(ok).To(BeTrue())
 		Expect(loaded.ContextSize).To(HaveValue(Equal(10000)))
 		Expect(readMap(filepath.Join(dir, "ordered.yaml"))).To(HaveKeyWithValue("context_size", 10000))
-		revision, err := config.ModelConfigRevision(&loaded)
+		revision, err := loader.RevisionForPath(loaded.Name, dir)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(lifecycle.revisions()).To(HaveLen(2))
 		Expect(lifecycle.revisions()[1]).To(Equal(revision))

core/services/modeladmin/revision_agreement_test.go

@@ -0,0 +1,102 @@
+package modeladmin
+
+import (
+	"os"
+	"path/filepath"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	"github.com/mudler/LocalAI/core/config"
+	"github.com/mudler/LocalAI/pkg/system"
+)
+
+// A model's revision is published by administration and checked against on
+// every inference request. Those were computed by different code, and each time
+// they drifted the model became unroutable until someone deleted the row by
+// hand: the request path resolves through the loader, while publishers hashed
+// whatever ModelConfig they were holding, which by then had SetDefaults applied.
+//
+// There is now one resolver, ModelConfigLoader.RevisionFor, and the raw hash is
+// unexported so a new publisher cannot reintroduce the split. This pins the
+// property that mattered: whatever a publisher writes is what a request brings.
+var _ = Describe("Published and requested revisions agree", func() {
+	var (
+		dir       string
+		appConfig *config.ApplicationConfig
+		loader    *config.ModelConfigLoader
+	)
+
+	// Several shapes, because the divergence only ever showed up on configs
+	// rich enough for SetDefaults to change something: a model file to guess
+	// from, several derived usecases, explicit options.
+	models := map[string]string{
+		"plain":       "name: plain\nbackend: llama-cpp\nparameters:\n    model: plain.gguf\n",
+		"multimodal":  "name: multimodal\nbackend: llama-cpp\ncontext_size: 50000\nknown_usecases:\n    - chat\nmmproj: mm/mmproj.gguf\noptions:\n    - use_jinja:true\n    - parallel:2\nparameters:\n    model: mm/model.gguf\n",
+		"auto-ctx":    "name: auto-ctx\nbackend: llama-cpp\ncontext_size: -1\nparameters:\n    model: auto.gguf\n",
+		"no-backend":  "name: no-backend\nparameters:\n    model: bare.gguf\n",
+		"with-thread": "name: with-thread\nbackend: llama-cpp\nthreads: 3\nparameters:\n    model: t.gguf\n",
+	}
+
+	BeforeEach(func() {
+		dir = GinkgoT().TempDir()
+		for name, body := range models {
+			Expect(os.WriteFile(filepath.Join(dir, name+".yaml"), []byte(body), 0o600)).To(Succeed())
+		}
+		appConfig = config.NewApplicationConfig()
+		appConfig.SystemState = &system.SystemState{Model: system.Model{ModelsPath: dir}}
+		appConfig.Threads = 8
+		loader = config.NewModelConfigLoader(dir)
+		Expect(loader.LoadModelConfigsFromPath(dir, appConfig.ToConfigLoaderOptions()...)).To(Succeed())
+	})
+
+	// requestRevision mirrors what core/backend.ModelOptions forwards to the
+	// router: the stamp on the config the request pipeline resolved.
+	requestRevision := func(name string) string {
+		cfg, err := loader.LoadModelConfigFileByNameDefaultOptions(name, appConfig)
+		Expect(err).ToNot(HaveOccurred())
+		return cfg.PersistedConfigRevision()
+	}
+
+	It("resolves the same revision a request will carry, for every model shape", func() {
+		for name := range models {
+			published, err := loader.RevisionFor(name, appConfig)
+			Expect(err).ToNot(HaveOccurred(), "model %s", name)
+			Expect(published).To(Equal(requestRevision(name)), "model %s: publisher and request disagree", name)
+		}
+	})
+
+	It("resolves the same revision through the path-based form", func() {
+		for name := range models {
+			byAppConfig, err := loader.RevisionFor(name, appConfig)
+			Expect(err).ToNot(HaveOccurred())
+			byPath, err := loader.RevisionForPath(name, dir, appConfig.ToConfigLoaderOptions()...)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(byPath).To(Equal(byAppConfig), "model %s", name)
+		}
+	})
+
+	It("does not move when the app-level defaults change", func() {
+		before := map[string]string{}
+		for name := range models {
+			r, err := loader.RevisionFor(name, appConfig)
+			Expect(err).ToNot(HaveOccurred())
+			before[name] = r
+		}
+
+		other := config.NewApplicationConfig()
+		other.SystemState = &system.SystemState{Model: system.Model{ModelsPath: dir}}
+		other.Threads = 1
+		other.F16 = true
+		other.ContextSize = 4096
+		fresh := config.NewModelConfigLoader(dir)
+		Expect(fresh.LoadModelConfigsFromPath(dir, other.ToConfigLoaderOptions()...)).To(Succeed())
+
+		for name := range models {
+			r, err := fresh.RevisionFor(name, other)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(r).To(Equal(before[name]),
+				"model %s: changing an app-level setting must not make every model unroutable", name)
+		}
+	})
+})

core/services/modeladmin/revision_resync.go

@@ -87,13 +87,9 @@ func ResyncModelConfigRevisions(ctx context.Context, loader *config.ModelConfigL
 		// (it re-runs the GGUF guess and hardware defaults), so hashing the
 		// stored config yields a value no request will ever carry, and
 		// publishing it would wedge the model this resync exists to unwedge.
-		resolved, err := loader.LoadModelConfigFileByNameDefaultOptions(cfg.Name, appConfig)
+		want, err := loader.RevisionFor(cfg.Name, appConfig)
 		if err != nil {
-			return fmt.Errorf("resolve config for %q: %w", cfg.Name, err)
-		}
-		want := resolved.PersistedConfigRevision()
-		if want == "" {
-			return fmt.Errorf("no config revision stamped for %q", cfg.Name)
+			return err
 		}
 
 		stored, err := store.GetModelConfigRevision(ctx, cfg.Name)