From 08d289c3d9234e14bbe80391ae0483df94bd9449 Mon Sep 17 00:00:00 2001 From: flyworker Date: Sat, 5 Sep 2026 16:45:27 +0000 Subject: [PATCH] fix: stop the registry callbacks erasing context_length and local_model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An explicit context_length in models.json was read at startup and then wiped. loadModelMappings unmarshals models.json into modelMappings correctly, but the registry's added/updated callbacks rebuild each entry from a field list that omitted ContextLength and LocalModel — and they fire after that load, and again on every hot reload. So the override survived only until the registry reported the model back, which is immediately. The consequence reaches the marketplace. ModelContext reads the override from modelMappings alone, so a backend that publishes no max_model_len of its own — llama.cpp, Ollama, an OpenAI-compatible proxy — declared nothing at all, and the server fell back to the catalog value for a window the operator had stated explicitly. On this node Qwen/Qwen3.8-27B carries context_length 65536 in models.json and was reporting "not reported". LocalModel was dropped the same way but does not bite: every forwarding path asks the registry first and only falls back to the mapping, so the name rewrite kept working. It was one refactor away from mattering. Both callbacks now go through one constructor that copies every field the registry holds, so a field added to RegisteredModel cannot be silently lost in the mirror again. --- internal/computing/inference_service.go | 43 +++++++------ internal/computing/mapping_mirror_test.go | 74 +++++++++++++++++++++++ 2 files changed, 99 insertions(+), 18 deletions(-) create mode 100644 internal/computing/mapping_mirror_test.go diff --git a/internal/computing/inference_service.go b/internal/computing/inference_service.go index de94c36..fe08682 100644 --- a/internal/computing/inference_service.go +++ b/internal/computing/inference_service.go @@ -44,6 +44,29 @@ var streamingHttpClient = &http.Client{ }, } +// mappingFor mirrors a registered model into the legacy mapping table. +// +// Every field the registry holds has to be copied. These callbacks fire after +// models.json is first read and again on every hot reload, so anything omitted +// here is not merely missing — it silently overwrites a value that was loaded +// correctly moments earlier. context_length was dropped this way: an operator +// setting it in models.json saw it read at startup and then erased, so the +// window went out as undetermined and the marketplace fell back to the catalog +// value for a backend that publishes no max_model_len of its own. +func mappingFor(model *RegisteredModel) ModelMapping { + return ModelMapping{ + Container: model.Container, + Endpoint: model.Endpoint, + GPUMemory: model.GPUMemory, + Category: model.Category, + LocalModel: model.LocalModel, + Format: model.Format, + Quantization: model.Quantization, + APIKey: model.APIKey, + ContextLength: model.ContextLength, + } +} + // ModelMapping represents a model-to-endpoint mapping from models.json type ModelMapping struct { Container string `json:"container"` @@ -146,15 +169,7 @@ func NewInferenceService(nodeID, cpPath string) *InferenceService { registry.SetCallbacks( func(model *RegisteredModel) { // On model added - s.modelMappings[model.ID] = ModelMapping{ - Container: model.Container, - Endpoint: model.Endpoint, - GPUMemory: model.GPUMemory, - Category: model.Category, - Format: model.Format, - Quantization: model.Quantization, - APIKey: model.APIKey, - } + s.modelMappings[model.ID] = mappingFor(model) s.updateClientModels() }, func(modelID string) { @@ -164,15 +179,7 @@ func NewInferenceService(nodeID, cpPath string) *InferenceService { }, func(model *RegisteredModel) { // On model updated - s.modelMappings[model.ID] = ModelMapping{ - Container: model.Container, - Endpoint: model.Endpoint, - GPUMemory: model.GPUMemory, - Category: model.Category, - Format: model.Format, - Quantization: model.Quantization, - APIKey: model.APIKey, - } + s.modelMappings[model.ID] = mappingFor(model) }, ) diff --git a/internal/computing/mapping_mirror_test.go b/internal/computing/mapping_mirror_test.go new file mode 100644 index 0000000..6223bdf --- /dev/null +++ b/internal/computing/mapping_mirror_test.go @@ -0,0 +1,74 @@ +package computing + +import "testing" + +// The registry callbacks overwrite whatever models.json loaded, so a field +// omitted from the mirror does not merely go missing — it erases a value that +// was read correctly moments earlier. context_length was lost exactly that way. +func TestMappingForCopiesEveryField(t *testing.T) { + model := &RegisteredModel{ + ID: "org/model", + Container: "c", + Endpoint: "http://backend:8000", + GPUMemory: 16000, + Category: "text-generation", + LocalModel: "model-local-name", + Format: "awq", + Quantization: "w4a16", + APIKey: "sk-local", + ContextLength: 65536, + } + + got := mappingFor(model) + + for _, tc := range []struct { + field string + got any + want any + }{ + {"Container", got.Container, model.Container}, + {"Endpoint", got.Endpoint, model.Endpoint}, + {"GPUMemory", got.GPUMemory, model.GPUMemory}, + {"Category", got.Category, model.Category}, + {"LocalModel", got.LocalModel, model.LocalModel}, + {"Format", got.Format, model.Format}, + {"Quantization", got.Quantization, model.Quantization}, + {"APIKey", got.APIKey, model.APIKey}, + {"ContextLength", got.ContextLength, model.ContextLength}, + } { + if tc.got != tc.want { + t.Errorf("%s = %v, want %v", tc.field, tc.got, tc.want) + } + } +} + +// The end-to-end consequence: an explicit override in models.json must reach +// the declaration as an override, not be erased into "unknown". +func TestModelsJSONOverrideSurvivesRegistryCallback(t *testing.T) { + s := newContextService(map[string]ModelMapping{ + "org/proxied": {Endpoint: "http://proxy", ContextLength: 65536}, + }) + // The backend publishes nothing, as llama.cpp and Ollama do not. + s.healthChecker.recordDetectedContext("org/proxied", nil) + + // Simulate the registry reporting the same model back, which is what + // overwrote the mapping before. + s.modelMappings["org/proxied"] = mappingFor(&RegisteredModel{ + ID: "org/proxied", + Endpoint: "http://proxy", + ContextLength: 65536, + }) + + info := s.ModelContext("org/proxied") + if info.Length != 65536 { + t.Errorf("length = %d, want the operator's 65536", info.Length) + } + if info.Source != ContextSourceOverride { + t.Errorf("source = %q, want %q", info.Source, ContextSourceOverride) + } + + declared := s.resolveModelContexts() + if declared["org/proxied"].Length != 65536 { + t.Errorf("declaration = %+v, want the override to be declared", declared["org/proxied"]) + } +}