From dbd868679db19bfca1d683be21149fc82040d74c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89mile=20R=C3=A9?= Date: Fri, 29 May 2026 17:02:29 +0200 Subject: [PATCH] Drop sampling params unsupported by the model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The common-pattern enrichment and tracker-mapping agents run on reasoning models such as gpt-5-nano, which reject an explicit temperature and fail the whole request with a 400 ("Unsupported value: 'temperature' does not support 0.1 with this model"). The model registry already records this capability, but nothing consulted it before dispatch, and dated provider snapshots like gpt-5-nano-2025-08-07 did not resolve in the registry. Resolve dated snapshots to their undated base model in registry Lookup, and sanitize each chat completion request in the LLM client by omitting the sampling knobs the target model does not accept (temperature, top_p, frequency/presence penalties, stop). Unknown models are left untouched, so models absent from the registry keep their current behavior. Signed-off-by: Émile Ré --- pkg/llm/llm.go | 71 ++++++++++++++++++++++++ pkg/llm/llm_test.go | 117 +++++++++++++++++++++++++++++++++++++++ pkg/llm/registry.go | 25 ++++++++- pkg/llm/registry_test.go | 17 ++++++ 4 files changed, 229 insertions(+), 1 deletion(-) diff --git a/pkg/llm/llm.go b/pkg/llm/llm.go index f53c99d93..5dc761534 100644 --- a/pkg/llm/llm.go +++ b/pkg/llm/llm.go @@ -17,6 +17,7 @@ package llm import ( "context" "io" + "strings" "time" "go.gearno.de/kit/log" @@ -72,6 +73,8 @@ func NewClient(provider Provider, system string, opts ...Option) *Client { } func (c *Client) ChatCompletion(ctx context.Context, req *ChatCompletionRequest) (*ChatCompletionResponse, error) { + req = sanitizeRequest(ctx, c.logger, req) + ctx, span := startChatSpan(ctx, c.tracer, c.system, req) c.logger.InfoCtx( @@ -115,6 +118,8 @@ func (c *Client) ChatCompletion(ctx context.Context, req *ChatCompletionRequest) } func (c *Client) ChatCompletionStream(ctx context.Context, req *ChatCompletionRequest) (ChatCompletionStream, error) { + req = sanitizeRequest(ctx, c.logger, req) + ctx, span := startChatSpan(ctx, c.tracer, c.system, req) c.logger.InfoCtx( @@ -140,3 +145,69 @@ func (c *Client) ChatCompletionStream(ctx context.Context, req *ChatCompletionRe return newTracedStream(stream, span), nil } + +// sanitizeRequest drops sampling parameters the target model does not +// accept, using the default model registry. Reasoning models (e.g. the +// GPT-5 family) reject an explicit temperature, top_p, or penalties and +// fail the whole request with a 400, so the safest behavior is to omit +// the unsupported knobs rather than propagate a hard error. When the +// model is unknown to the registry the request is left untouched. +func sanitizeRequest(ctx context.Context, logger *log.Logger, req *ChatCompletionRequest) *ChatCompletionRequest { + if req == nil { + return req + } + + model, ok := DefaultRegistry().Lookup(req.Model) + if !ok { + return req + } + + supports := model.Supports + + dropTemperature := req.Temperature != nil && !supports.Temperature + dropTopP := req.TopP != nil && !supports.TopP + dropFrequencyPenalty := req.FrequencyPenalty != nil && !supports.FrequencyPenalty + dropPresencePenalty := req.PresencePenalty != nil && !supports.PresencePenalty + dropStop := len(req.StopSequences) > 0 && !supports.Stop + + if !dropTemperature && !dropTopP && !dropFrequencyPenalty && !dropPresencePenalty && !dropStop { + return req + } + + sanitized := *req + + dropped := make([]string, 0, 5) + if dropTemperature { + sanitized.Temperature = nil + dropped = append(dropped, "temperature") + } + + if dropTopP { + sanitized.TopP = nil + dropped = append(dropped, "top_p") + } + + if dropFrequencyPenalty { + sanitized.FrequencyPenalty = nil + dropped = append(dropped, "frequency_penalty") + } + + if dropPresencePenalty { + sanitized.PresencePenalty = nil + dropped = append(dropped, "presence_penalty") + } + + if dropStop { + sanitized.StopSequences = nil + dropped = append(dropped, "stop") + } + + logger.WarnCtx( + ctx, + "dropping unsupported sampling parameters for model", + log.String("model", req.Model), + log.String("dropped", strings.Join(dropped, ",")), + ) + + return &sanitized +} diff --git a/pkg/llm/llm_test.go b/pkg/llm/llm_test.go index e2861b051..d58accb1c 100644 --- a/pkg/llm/llm_test.go +++ b/pkg/llm/llm_test.go @@ -47,6 +47,24 @@ func (m *mockProvider) ChatCompletionStream(_ context.Context, _ *llm.ChatComple return m.streamResp, m.streamErr } +// capturingProvider records the request it received so tests can assert +// on the parameters the client forwarded to the provider. +type capturingProvider struct { + lastReq *llm.ChatCompletionRequest + chatResp *llm.ChatCompletionResponse + streamResp llm.ChatCompletionStream +} + +func (p *capturingProvider) ChatCompletion(_ context.Context, req *llm.ChatCompletionRequest) (*llm.ChatCompletionResponse, error) { + p.lastReq = req + return p.chatResp, nil +} + +func (p *capturingProvider) ChatCompletionStream(_ context.Context, req *llm.ChatCompletionRequest) (llm.ChatCompletionStream, error) { + p.lastReq = req + return p.streamResp, nil +} + type mockStream struct { events []llm.ChatCompletionStreamEvent idx int @@ -358,6 +376,105 @@ func TestChatCompletion(t *testing.T) { }) } +// --------------------------------------------------------------------------- +// Client — request sanitization +// --------------------------------------------------------------------------- + +func TestChatCompletionSanitizesUnsupportedParameters(t *testing.T) { + t.Parallel() + + temp := 0.1 + topP := 0.9 + freq := 0.5 + pres := 0.5 + + t.Run("drops temperature for reasoning model snapshot", func(t *testing.T) { + t.Parallel() + + provider := &capturingProvider{ + chatResp: &llm.ChatCompletionResponse{ + Model: "gpt-5-nano", + FinishReason: llm.FinishReasonStop, + Message: llm.Message{ + Role: llm.RoleAssistant, + Parts: []llm.Part{llm.TextPart{Text: "ok"}}, + }, + }, + } + + client, _ := newTestClient(provider) + _, err := client.ChatCompletion(context.Background(), &llm.ChatCompletionRequest{ + Model: "gpt-5-nano-2025-08-07", + Messages: []llm.Message{{Role: llm.RoleUser, Parts: []llm.Part{llm.TextPart{Text: "Hi"}}}}, + Temperature: &temp, + TopP: &topP, + FrequencyPenalty: &freq, + PresencePenalty: &pres, + }) + + require.NoError(t, err) + require.NotNil(t, provider.lastReq) + assert.Nil(t, provider.lastReq.Temperature) + assert.Nil(t, provider.lastReq.TopP) + assert.Nil(t, provider.lastReq.FrequencyPenalty) + assert.Nil(t, provider.lastReq.PresencePenalty) + }) + + t.Run("keeps temperature for chat model", func(t *testing.T) { + t.Parallel() + + provider := &capturingProvider{ + chatResp: &llm.ChatCompletionResponse{ + Model: "gpt-4o", + FinishReason: llm.FinishReasonStop, + Message: llm.Message{ + Role: llm.RoleAssistant, + Parts: []llm.Part{llm.TextPart{Text: "ok"}}, + }, + }, + } + + client, _ := newTestClient(provider) + _, err := client.ChatCompletion(context.Background(), &llm.ChatCompletionRequest{ + Model: "gpt-4o", + Messages: []llm.Message{{Role: llm.RoleUser, Parts: []llm.Part{llm.TextPart{Text: "Hi"}}}}, + Temperature: &temp, + }) + + require.NoError(t, err) + require.NotNil(t, provider.lastReq) + require.NotNil(t, provider.lastReq.Temperature) + assert.InEpsilon(t, 0.1, *provider.lastReq.Temperature, 1e-9) + }) + + t.Run("leaves unknown model untouched", func(t *testing.T) { + t.Parallel() + + provider := &capturingProvider{ + chatResp: &llm.ChatCompletionResponse{ + Model: "mystery-model", + FinishReason: llm.FinishReasonStop, + Message: llm.Message{ + Role: llm.RoleAssistant, + Parts: []llm.Part{llm.TextPart{Text: "ok"}}, + }, + }, + } + + client, _ := newTestClient(provider) + _, err := client.ChatCompletion(context.Background(), &llm.ChatCompletionRequest{ + Model: "mystery-model", + Messages: []llm.Message{{Role: llm.RoleUser, Parts: []llm.Part{llm.TextPart{Text: "Hi"}}}}, + Temperature: &temp, + }) + + require.NoError(t, err) + require.NotNil(t, provider.lastReq) + require.NotNil(t, provider.lastReq.Temperature) + assert.InEpsilon(t, 0.1, *provider.lastReq.Temperature, 1e-9) + }) +} + // --------------------------------------------------------------------------- // Client — ChatCompletionStream // --------------------------------------------------------------------------- diff --git a/pkg/llm/registry.go b/pkg/llm/registry.go index 1cc0f23b2..130b8b3b4 100644 --- a/pkg/llm/registry.go +++ b/pkg/llm/registry.go @@ -17,6 +17,7 @@ package llm //go:generate go run go.probo.inc/probo/internal/cmd/genmodels import ( + "regexp" "strings" "sync" ) @@ -81,7 +82,9 @@ func DefaultRegistry() *Registry { // Lookup finds a model by ID. It accepts both provider-prefixed IDs // ("anthropic/claude-opus-4.6") and bare provider IDs ("claude-opus-4-6", -// "gpt-5.4"). Returns false if the model is not in the registry. +// "gpt-5.4"). Dated provider snapshots ("gpt-5-nano-2025-08-07") fall +// back to their undated base model ("gpt-5-nano"). Returns false if the +// model is not in the registry. func (r *Registry) Lookup(modelID string) (ModelDefinition, bool) { if m, ok := r.byID[modelID]; ok { return *m, true @@ -91,6 +94,16 @@ func (r *Registry) Lookup(modelID string) (ModelDefinition, bool) { return *m, true } + if base := stripModelDateSuffix(modelID); base != modelID { + if m, ok := r.byID[base]; ok { + return *m, true + } + + if m, ok := r.byID[normalizeModelID(base)]; ok { + return *m, true + } + } + return ModelDefinition{}, false } @@ -121,3 +134,13 @@ func normalizeModelID(id string) string { return strings.ReplaceAll(id, ".", "-") } + +// modelDateSuffix matches a trailing provider snapshot date such as the +// "-2025-08-07" in "gpt-5-nano-2025-08-07". +var modelDateSuffix = regexp.MustCompile(`-\d{4}-\d{2}-\d{2}$`) + +// stripModelDateSuffix removes a trailing dated-snapshot suffix from a +// model ID, leaving the base model ID unchanged when none is present. +func stripModelDateSuffix(id string) string { + return modelDateSuffix.ReplaceAllString(id, "") +} diff --git a/pkg/llm/registry_test.go b/pkg/llm/registry_test.go index f9a09000c..25049b2bd 100644 --- a/pkg/llm/registry_test.go +++ b/pkg/llm/registry_test.go @@ -73,6 +73,23 @@ func TestRegistry_Lookup(t *testing.T) { }, ) + t.Run( + "dated snapshot falls back to base model", + func(t *testing.T) { + t.Parallel() + + r := llm.NewRegistry(map[string]llm.ModelDefinition{ + "openai/gpt-5-nano": { + Name: "OpenAI: GPT-5 Nano", + }, + }) + + m, ok := r.Lookup("gpt-5-nano-2025-08-07") + require.True(t, ok) + assert.Equal(t, "openai/gpt-5-nano", m.ID) + }, + ) + t.Run( "unknown model returns false", func(t *testing.T) {