mirror of
https://github.com/coder/coder.git
synced 2026-09-22 05:05:20 +08:00
fix(coderd/x/chatd): retry quickgen without temperature when model rejects it (#27120)
This commit is contained in:
+63
-26
@@ -5,6 +5,7 @@ import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"slices"
|
||||
"strings"
|
||||
"time"
|
||||
@@ -54,9 +55,57 @@ const titleGenerationPrompt = "Write a short title for the user's message. " +
|
||||
// quickgenTemperature keeps title and status-label output stable
|
||||
// across repeated runs over the same input. Fantasy providers drop
|
||||
// this with a call warning for models that reject it (OpenAI
|
||||
// reasoning models, Anthropic thinking models).
|
||||
// reasoning models, Anthropic thinking models), but only for model
|
||||
// names they recognize. generateQuickgenObject handles models that
|
||||
// reject the parameter at the API instead.
|
||||
const quickgenTemperature = 0.0
|
||||
|
||||
// generateQuickgenObject generates a structured object with provider
|
||||
// retries and the pinned quickgen temperature. Model aliases served
|
||||
// through gateways such as AI Bridge are not recognized by fantasy's
|
||||
// per-model parameter stripping and can reject temperature with a
|
||||
// bad-request error, so the call is retried without temperature when
|
||||
// the model rejects it.
|
||||
func generateQuickgenObject[T any](
|
||||
ctx context.Context,
|
||||
model fantasy.LanguageModel,
|
||||
call fantasy.ObjectCall,
|
||||
) (*fantasy.ObjectResult[T], error) {
|
||||
call.Temperature = ptr.Ref(quickgenTemperature)
|
||||
var result *fantasy.ObjectResult[T]
|
||||
err := chatretry.Retry(ctx, func(retryCtx context.Context) error {
|
||||
var genErr error
|
||||
result, genErr = object.Generate[T](retryCtx, model, call)
|
||||
if call.Temperature != nil && isTemperatureRejectedError(genErr) {
|
||||
// The model rejects the temperature parameter. Drop it
|
||||
// for this and any later retry attempts.
|
||||
call.Temperature = nil
|
||||
result, genErr = object.Generate[T](retryCtx, model, call)
|
||||
}
|
||||
return genErr
|
||||
}, nil)
|
||||
return result, err
|
||||
}
|
||||
|
||||
// isTemperatureRejectedError reports whether a provider rejected the
|
||||
// request because the model does not accept the temperature parameter,
|
||||
// for example Anthropic's "`temperature` is deprecated for this model."
|
||||
// or OpenAI's "Unsupported parameter: 'temperature' is not supported
|
||||
// with this model.". Quickgen only sends a valid temperature value, so
|
||||
// any bad-request response mentioning temperature means the model
|
||||
// rejects the parameter itself.
|
||||
func isTemperatureRejectedError(err error) bool {
|
||||
var providerErr *fantasy.ProviderError
|
||||
if !errors.As(err, &providerErr) {
|
||||
return false
|
||||
}
|
||||
if providerErr.StatusCode != http.StatusBadRequest {
|
||||
return false
|
||||
}
|
||||
text := strings.ToLower(providerErr.Error() + " " + string(providerErr.ResponseBody))
|
||||
return strings.Contains(text, "temperature")
|
||||
}
|
||||
|
||||
const (
|
||||
// maxConversationContextRunes caps the conversation sample in manual
|
||||
// title prompts to avoid exceeding model context windows.
|
||||
@@ -555,19 +604,13 @@ func generateStructuredTitleWithUsage(
|
||||
}
|
||||
|
||||
var maxOutputTokens int64 = 256
|
||||
var result *fantasy.ObjectResult[generatedTitle]
|
||||
err := chatretry.Retry(ctx, func(retryCtx context.Context) error {
|
||||
var genErr error
|
||||
result, genErr = object.Generate[generatedTitle](retryCtx, model, fantasy.ObjectCall{
|
||||
Prompt: prompt,
|
||||
SchemaName: "propose_title",
|
||||
SchemaDescription: "Propose a short chat title.",
|
||||
MaxOutputTokens: &maxOutputTokens,
|
||||
Temperature: ptr.Ref(quickgenTemperature),
|
||||
ProviderOptions: providerOptions,
|
||||
})
|
||||
return genErr
|
||||
}, nil)
|
||||
result, err := generateQuickgenObject[generatedTitle](ctx, model, fantasy.ObjectCall{
|
||||
Prompt: prompt,
|
||||
SchemaName: "propose_title",
|
||||
SchemaDescription: "Propose a short chat title.",
|
||||
MaxOutputTokens: &maxOutputTokens,
|
||||
ProviderOptions: providerOptions,
|
||||
})
|
||||
if err != nil {
|
||||
var usage fantasy.Usage
|
||||
var noObjErr *fantasy.NoObjectGeneratedError
|
||||
@@ -1023,18 +1066,12 @@ func generateStructuredTurnStatusLabel(
|
||||
}
|
||||
|
||||
var maxOutputTokens int64 = 64
|
||||
var result *fantasy.ObjectResult[generatedTurnStatusLabel]
|
||||
err := chatretry.Retry(ctx, func(retryCtx context.Context) error {
|
||||
var genErr error
|
||||
result, genErr = object.Generate[generatedTurnStatusLabel](retryCtx, model, fantasy.ObjectCall{
|
||||
Prompt: prompt,
|
||||
SchemaName: "propose_turn_status_label",
|
||||
SchemaDescription: "Propose a compact chat status label.",
|
||||
MaxOutputTokens: &maxOutputTokens,
|
||||
Temperature: ptr.Ref(quickgenTemperature),
|
||||
})
|
||||
return genErr
|
||||
}, nil)
|
||||
result, err := generateQuickgenObject[generatedTurnStatusLabel](ctx, model, fantasy.ObjectCall{
|
||||
Prompt: prompt,
|
||||
SchemaName: "propose_turn_status_label",
|
||||
SchemaDescription: "Propose a compact chat status label.",
|
||||
MaxOutputTokens: &maxOutputTokens,
|
||||
})
|
||||
if err != nil {
|
||||
return "", xerrors.Errorf("generate structured turn status label: %w", err)
|
||||
}
|
||||
|
||||
@@ -17,6 +17,7 @@ import (
|
||||
"github.com/sqlc-dev/pqtype"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.uber.org/mock/gomock"
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"cdr.dev/slog/v3/sloggers/slogtest"
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
@@ -901,6 +902,48 @@ func TestGenerateStructuredTitleWithUsage_OpenAICompatibleRequiredToolChoice(t *
|
||||
"title generation should pin temperature for repeatable output")
|
||||
}
|
||||
|
||||
// newTemperatureRejectedError mirrors the bad-request error returned
|
||||
// through AI Bridge by models that do not accept the temperature
|
||||
// parameter.
|
||||
func newTemperatureRejectedError() *fantasy.ProviderError {
|
||||
return &fantasy.ProviderError{
|
||||
Title: "bad request",
|
||||
Message: `POST "http://coder-aibridge/v1/messages": 400 Bad Request ` +
|
||||
`{"error":{"message":"` + "`temperature`" + ` is deprecated for this model.",` +
|
||||
`"type":"invalid_request_error"},"request_id":"","type":"error"}`,
|
||||
StatusCode: http.StatusBadRequest,
|
||||
}
|
||||
}
|
||||
|
||||
func TestGenerateStructuredTitleWithUsage_DropsRejectedTemperature(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var sawTemperature []bool
|
||||
model := &chattest.FakeModel{
|
||||
GenerateObjectFn: func(_ context.Context, call fantasy.ObjectCall) (*fantasy.ObjectResponse, error) {
|
||||
sawTemperature = append(sawTemperature, call.Temperature != nil)
|
||||
if call.Temperature != nil {
|
||||
return nil, newTemperatureRejectedError()
|
||||
}
|
||||
return &fantasy.ObjectResponse{
|
||||
Object: map[string]any{"title": "Failed workspace logs"},
|
||||
}, nil
|
||||
},
|
||||
}
|
||||
|
||||
title, _, err := generateStructuredTitleWithUsage(
|
||||
t.Context(),
|
||||
model,
|
||||
nil,
|
||||
titleGenerationPrompt,
|
||||
"summarize failed workspace build logs",
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "Failed workspace logs", title)
|
||||
require.Equal(t, []bool{true, false}, sawTemperature,
|
||||
"generation should retry without temperature after the model rejects it")
|
||||
}
|
||||
|
||||
func newOpenAICompatStructuredOutputServer(
|
||||
t *testing.T,
|
||||
toolName string,
|
||||
@@ -1013,6 +1056,50 @@ func TestGenerateStructuredTurnStatusLabel(t *testing.T) {
|
||||
"status-label generation should pin temperature for repeatable output")
|
||||
})
|
||||
|
||||
t.Run("drops temperature when model rejects it", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var sawTemperature []bool
|
||||
model := &chattest.FakeModel{
|
||||
GenerateObjectFn: func(_ context.Context, call fantasy.ObjectCall) (*fantasy.ObjectResponse, error) {
|
||||
sawTemperature = append(sawTemperature, call.Temperature != nil)
|
||||
if call.Temperature != nil {
|
||||
return nil, newTemperatureRejectedError()
|
||||
}
|
||||
return &fantasy.ObjectResponse{
|
||||
Object: map[string]any{"label": "Submitted PR"},
|
||||
}, nil
|
||||
},
|
||||
}
|
||||
|
||||
label, err := generateStructuredTurnStatusLabel(t.Context(), model, turnStatusLabelPrompt, "done")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "Submitted PR", label)
|
||||
require.Equal(t, []bool{true, false}, sawTemperature,
|
||||
"generation should retry without temperature after the model rejects it")
|
||||
})
|
||||
|
||||
t.Run("surfaces unrelated bad request errors", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var calls int
|
||||
model := &chattest.FakeModel{
|
||||
GenerateObjectFn: func(_ context.Context, _ fantasy.ObjectCall) (*fantasy.ObjectResponse, error) {
|
||||
calls++
|
||||
return nil, &fantasy.ProviderError{
|
||||
Title: "bad request",
|
||||
Message: "tools.0.custom.input_schema: JSON schema is invalid",
|
||||
StatusCode: http.StatusBadRequest,
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
_, err := generateStructuredTurnStatusLabel(t.Context(), model, turnStatusLabelPrompt, "done")
|
||||
require.ErrorContains(t, err, "JSON schema is invalid")
|
||||
require.Equal(t, 1, calls,
|
||||
"bad requests unrelated to temperature should not trigger a second attempt")
|
||||
})
|
||||
|
||||
t.Run("rejects narrative label", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -1037,6 +1124,72 @@ func TestGenerateStructuredTurnStatusLabel(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
func TestIsTemperatureRejectedError(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
err error
|
||||
want bool
|
||||
}{
|
||||
{
|
||||
name: "nil error",
|
||||
err: nil,
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "plain error mentioning temperature",
|
||||
err: xerrors.New("temperature is deprecated for this model"),
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "bad request rejecting temperature",
|
||||
err: newTemperatureRejectedError(),
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "wrapped bad request rejecting temperature",
|
||||
err: xerrors.Errorf("tool-based generation failed: %w", newTemperatureRejectedError()),
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "bad request with temperature only in response body",
|
||||
err: &fantasy.ProviderError{
|
||||
Title: "bad request",
|
||||
Message: "provider request failed",
|
||||
StatusCode: http.StatusBadRequest,
|
||||
ResponseBody: []byte(`{"error":{"message":"Unsupported parameter: 'temperature' is not supported with this model."}}`),
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "bad request unrelated to temperature",
|
||||
err: &fantasy.ProviderError{
|
||||
Title: "bad request",
|
||||
Message: "tools.0.custom.input_schema: JSON schema is invalid",
|
||||
StatusCode: http.StatusBadRequest,
|
||||
},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "server error mentioning temperature",
|
||||
err: &fantasy.ProviderError{
|
||||
Title: "internal server error",
|
||||
Message: "temperature processing failed",
|
||||
StatusCode: http.StatusInternalServerError,
|
||||
},
|
||||
want: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
require.Equal(t, tt.want, isTemperatureRejectedError(tt.err))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func mustChatMessage(
|
||||
t *testing.T,
|
||||
role database.ChatMessageRole,
|
||||
|
||||
Reference in New Issue
Block a user