fix(providers): narrow responses routing to gpt-5
Keep automatic /responses routing limited to gpt-5 models so OpenAI-compatible endpoints continue to use chat/completions by default, and document the shared request-body conversion path used with openai_responses_common.
This commit is contained in:
parent
021a184ab3
commit
c1d0b37675
6 changed files with 105 additions and 38 deletions
|
|
@ -15,7 +15,6 @@ import (
|
||||||
anthropicmessages "github.com/sipeed/picoclaw/pkg/providers/anthropic_messages"
|
anthropicmessages "github.com/sipeed/picoclaw/pkg/providers/anthropic_messages"
|
||||||
"github.com/sipeed/picoclaw/pkg/providers/azure"
|
"github.com/sipeed/picoclaw/pkg/providers/azure"
|
||||||
"github.com/sipeed/picoclaw/pkg/providers/bedrock"
|
"github.com/sipeed/picoclaw/pkg/providers/bedrock"
|
||||||
"github.com/sipeed/picoclaw/pkg/providers/openai_compat"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// createClaudeAuthProvider creates a Claude provider using OAuth credentials from auth store.
|
// createClaudeAuthProvider creates a Claude provider using OAuth credentials from auth store.
|
||||||
|
|
@ -99,7 +98,6 @@ func CreateProviderFromConfig(cfg *config.ModelConfig) (LLMProvider, string, err
|
||||||
cfg.MaxTokensField,
|
cfg.MaxTokensField,
|
||||||
cfg.RequestTimeout,
|
cfg.RequestTimeout,
|
||||||
cfg.ExtraBody,
|
cfg.ExtraBody,
|
||||||
openai_compat.WithResponsesPreferred(),
|
|
||||||
), modelID, nil
|
), modelID, nil
|
||||||
|
|
||||||
case "azure", "azure-openai":
|
case "azure", "azure-openai":
|
||||||
|
|
|
||||||
|
|
@ -107,7 +107,7 @@ func TestCreateProviderFromConfig_OpenAI(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestCreateProviderFromConfig_OpenAIUsesResponsesFirst(t *testing.T) {
|
func TestCreateProviderFromConfig_OpenAIGPT5UsesResponsesFirst(t *testing.T) {
|
||||||
var paths []string
|
var paths []string
|
||||||
|
|
||||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
|
@ -116,10 +116,19 @@ func TestCreateProviderFromConfig_OpenAIUsesResponsesFirst(t *testing.T) {
|
||||||
switch r.URL.Path {
|
switch r.URL.Path {
|
||||||
case "/responses":
|
case "/responses":
|
||||||
w.Header().Set("Content-Type", "application/json")
|
w.Header().Set("Content-Type", "application/json")
|
||||||
_, _ = w.Write([]byte(`{"status":"completed","output":[{"type":"message","content":[{"type":"output_text","text":"from responses"}]}]}`))
|
_, _ = w.Write([]byte(`{
|
||||||
|
"status": "completed",
|
||||||
|
"output": [
|
||||||
|
{"type": "message", "content": [{"type": "output_text", "text": "from responses"}]}
|
||||||
|
]
|
||||||
|
}`))
|
||||||
case "/chat/completions":
|
case "/chat/completions":
|
||||||
w.Header().Set("Content-Type", "application/json")
|
w.Header().Set("Content-Type", "application/json")
|
||||||
_, _ = w.Write([]byte(`{"choices":[{"message":{"content":"from chat completions"},"finish_reason":"stop"}]}`))
|
_, _ = w.Write([]byte(`{
|
||||||
|
"choices": [
|
||||||
|
{"message": {"content": "from chat completions"}, "finish_reason": "stop"}
|
||||||
|
]
|
||||||
|
}`))
|
||||||
default:
|
default:
|
||||||
http.Error(w, "not found", http.StatusNotFound)
|
http.Error(w, "not found", http.StatusNotFound)
|
||||||
}
|
}
|
||||||
|
|
@ -128,7 +137,7 @@ func TestCreateProviderFromConfig_OpenAIUsesResponsesFirst(t *testing.T) {
|
||||||
|
|
||||||
cfg := &config.ModelConfig{
|
cfg := &config.ModelConfig{
|
||||||
ModelName: "test-openai",
|
ModelName: "test-openai",
|
||||||
Model: "openai/gpt-4o",
|
Model: "openai/gpt-5.2",
|
||||||
APIKeys: config.SimpleSecureStrings("test-key"),
|
APIKeys: config.SimpleSecureStrings("test-key"),
|
||||||
APIBase: server.URL,
|
APIBase: server.URL,
|
||||||
}
|
}
|
||||||
|
|
@ -137,6 +146,9 @@ func TestCreateProviderFromConfig_OpenAIUsesResponsesFirst(t *testing.T) {
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("CreateProviderFromConfig() error = %v", err)
|
t.Fatalf("CreateProviderFromConfig() error = %v", err)
|
||||||
}
|
}
|
||||||
|
if modelID != "gpt-5.2" {
|
||||||
|
t.Fatalf("modelID = %q, want %q", modelID, "gpt-5.2")
|
||||||
|
}
|
||||||
|
|
||||||
out, err := provider.Chat(
|
out, err := provider.Chat(
|
||||||
t.Context(),
|
t.Context(),
|
||||||
|
|
|
||||||
|
|
@ -33,11 +33,12 @@ func NewHTTPProviderWithMaxTokensFieldAndRequestTimeout(
|
||||||
extraBody map[string]any,
|
extraBody map[string]any,
|
||||||
opts ...openai_compat.Option,
|
opts ...openai_compat.Option,
|
||||||
) *HTTPProvider {
|
) *HTTPProvider {
|
||||||
providerOpts := []openai_compat.Option{
|
providerOpts := make([]openai_compat.Option, 0, 3+len(opts))
|
||||||
|
providerOpts = append(providerOpts,
|
||||||
openai_compat.WithMaxTokensField(maxTokensField),
|
openai_compat.WithMaxTokensField(maxTokensField),
|
||||||
openai_compat.WithRequestTimeout(time.Duration(requestTimeoutSeconds)*time.Second),
|
openai_compat.WithRequestTimeout(time.Duration(requestTimeoutSeconds)*time.Second),
|
||||||
openai_compat.WithExtraBody(extraBody),
|
openai_compat.WithExtraBody(extraBody),
|
||||||
}
|
)
|
||||||
providerOpts = append(providerOpts, opts...)
|
providerOpts = append(providerOpts, opts...)
|
||||||
|
|
||||||
return &HTTPProvider{
|
return &HTTPProvider{
|
||||||
|
|
|
||||||
|
|
@ -16,6 +16,7 @@ import (
|
||||||
|
|
||||||
"github.com/openai/openai-go/v3"
|
"github.com/openai/openai-go/v3"
|
||||||
"github.com/openai/openai-go/v3/responses"
|
"github.com/openai/openai-go/v3/responses"
|
||||||
|
|
||||||
"github.com/sipeed/picoclaw/pkg/providers/common"
|
"github.com/sipeed/picoclaw/pkg/providers/common"
|
||||||
orc "github.com/sipeed/picoclaw/pkg/providers/openai_responses_common"
|
orc "github.com/sipeed/picoclaw/pkg/providers/openai_responses_common"
|
||||||
"github.com/sipeed/picoclaw/pkg/providers/protocoltypes"
|
"github.com/sipeed/picoclaw/pkg/providers/protocoltypes"
|
||||||
|
|
@ -39,7 +40,6 @@ type Provider struct {
|
||||||
maxTokensField string // Field name for max tokens (e.g., "max_completion_tokens" for o1/glm models)
|
maxTokensField string // Field name for max tokens (e.g., "max_completion_tokens" for o1/glm models)
|
||||||
httpClient *http.Client
|
httpClient *http.Client
|
||||||
extraBody map[string]any // Additional fields to inject into request body
|
extraBody map[string]any // Additional fields to inject into request body
|
||||||
preferResponses bool // Prefer /responses for OpenAI-native configs.
|
|
||||||
}
|
}
|
||||||
|
|
||||||
type Option func(*Provider)
|
type Option func(*Provider)
|
||||||
|
|
@ -60,12 +60,6 @@ func WithRequestTimeout(timeout time.Duration) Option {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func WithResponsesPreferred() Option {
|
|
||||||
return func(p *Provider) {
|
|
||||||
p.preferResponses = true
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
func WithExtraBody(extraBody map[string]any) Option {
|
func WithExtraBody(extraBody map[string]any) Option {
|
||||||
return func(p *Provider) {
|
return func(p *Provider) {
|
||||||
p.extraBody = extraBody
|
p.extraBody = extraBody
|
||||||
|
|
@ -173,13 +167,14 @@ func requestTemperature(model string, options map[string]any) (float64, bool) {
|
||||||
return temperature, true
|
return temperature, true
|
||||||
}
|
}
|
||||||
|
|
||||||
func shouldPreferResponses(rawModel, normalizedModel string, preferOpenAIModels bool) bool {
|
func shouldPreferResponses(rawModel, normalizedModel string) bool {
|
||||||
rawModel = strings.ToLower(strings.TrimSpace(rawModel))
|
rawModel = strings.ToLower(strings.TrimSpace(rawModel))
|
||||||
normalizedModel = strings.ToLower(strings.TrimSpace(normalizedModel))
|
normalizedModel = strings.ToLower(strings.TrimSpace(normalizedModel))
|
||||||
|
|
||||||
return preferOpenAIModels ||
|
// Keep the automatic route conservative: only gpt-5 models are forced
|
||||||
strings.HasPrefix(rawModel, "gpt-5") ||
|
// onto /responses, and all other model families stay on chat/completions
|
||||||
strings.HasPrefix(normalizedModel, "gpt-5")
|
// unless they are explicitly routed elsewhere by the caller.
|
||||||
|
return strings.HasPrefix(rawModel, "gpt-5") || strings.HasPrefix(normalizedModel, "gpt-5")
|
||||||
}
|
}
|
||||||
|
|
||||||
func hasReasoningContentHistory(messages []Message) bool {
|
func hasReasoningContentHistory(messages []Message) bool {
|
||||||
|
|
@ -233,6 +228,8 @@ func (p *Provider) buildResponsesRequestBody(
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Marshal through the SDK type first so we keep its validation/defaulting for
|
||||||
|
// Responses API fields, then convert back to a generic map to merge extraBody.
|
||||||
jsonData, err := json.Marshal(requestBody)
|
jsonData, err := json.Marshal(requestBody)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("failed to marshal responses request: %w", err)
|
return nil, fmt.Errorf("failed to marshal responses request: %w", err)
|
||||||
|
|
@ -262,7 +259,7 @@ func (p *Provider) Chat(
|
||||||
}
|
}
|
||||||
|
|
||||||
normalizedModel := normalizeModel(model, p.apiBase)
|
normalizedModel := normalizeModel(model, p.apiBase)
|
||||||
if shouldPreferResponses(model, normalizedModel, p.preferResponses) && !hasReasoningContentHistory(messages) {
|
if shouldPreferResponses(model, normalizedModel) && !hasReasoningContentHistory(messages) {
|
||||||
out, err := p.chatResponses(ctx, messages, tools, normalizedModel, options)
|
out, err := p.chatResponses(ctx, messages, tools, normalizedModel, options)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
return out, nil
|
return out, nil
|
||||||
|
|
@ -270,11 +267,19 @@ func (p *Provider) Chat(
|
||||||
if ctx.Err() != nil {
|
if ctx.Err() != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
log.Printf("openai_compat: /responses failed for %q, falling back to /chat/completions: %v", normalizedModel, err)
|
log.Printf(
|
||||||
|
"openai_compat: /responses failed for %q, falling back to /chat/completions: %v",
|
||||||
|
normalizedModel,
|
||||||
|
err,
|
||||||
|
)
|
||||||
|
|
||||||
fallbackOut, fallbackErr := p.chatCompletions(ctx, messages, tools, normalizedModel, options)
|
fallbackOut, fallbackErr := p.chatCompletions(ctx, messages, tools, normalizedModel, options)
|
||||||
if fallbackErr != nil {
|
if fallbackErr != nil {
|
||||||
return nil, fmt.Errorf("responses request failed; fallback chat/completions failed: %w", errors.Join(err, fallbackErr))
|
joinedErr := errors.Join(err, fallbackErr)
|
||||||
|
return nil, fmt.Errorf(
|
||||||
|
"responses request failed; fallback chat/completions failed: %w",
|
||||||
|
joinedErr,
|
||||||
|
)
|
||||||
}
|
}
|
||||||
return fallbackOut, nil
|
return fallbackOut, nil
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -17,7 +17,7 @@ import (
|
||||||
"github.com/sipeed/picoclaw/pkg/providers/protocoltypes"
|
"github.com/sipeed/picoclaw/pkg/providers/protocoltypes"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestProviderChat_PrefersResponsesWhenConfigured(t *testing.T) {
|
func TestProviderChat_PrefersResponsesForGPT5Models(t *testing.T) {
|
||||||
var paths []string
|
var paths []string
|
||||||
var responsesBody map[string]any
|
var responsesBody map[string]any
|
||||||
|
|
||||||
|
|
@ -64,12 +64,12 @@ func TestProviderChat_PrefersResponsesWhenConfigured(t *testing.T) {
|
||||||
}))
|
}))
|
||||||
defer server.Close()
|
defer server.Close()
|
||||||
|
|
||||||
p := NewProvider("key", server.URL, "", WithResponsesPreferred())
|
p := NewProvider("key", server.URL, "")
|
||||||
out, err := p.Chat(
|
out, err := p.Chat(
|
||||||
t.Context(),
|
t.Context(),
|
||||||
[]Message{{Role: "user", Content: "hi"}},
|
[]Message{{Role: "user", Content: "hi"}},
|
||||||
nil,
|
nil,
|
||||||
"gpt-4o",
|
"gpt-5.2",
|
||||||
map[string]any{"max_tokens": 256},
|
map[string]any{"max_tokens": 256},
|
||||||
)
|
)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|
@ -82,8 +82,8 @@ func TestProviderChat_PrefersResponsesWhenConfigured(t *testing.T) {
|
||||||
if !reflect.DeepEqual(paths, []string{"/responses"}) {
|
if !reflect.DeepEqual(paths, []string{"/responses"}) {
|
||||||
t.Fatalf("paths = %v, want [/responses]", paths)
|
t.Fatalf("paths = %v, want [/responses]", paths)
|
||||||
}
|
}
|
||||||
if responsesBody["model"] != "gpt-4o" {
|
if responsesBody["model"] != "gpt-5.2" {
|
||||||
t.Fatalf("model = %v, want gpt-4o", responsesBody["model"])
|
t.Fatalf("model = %v, want gpt-5.2", responsesBody["model"])
|
||||||
}
|
}
|
||||||
if _, ok := responsesBody["input"]; !ok {
|
if _, ok := responsesBody["input"]; !ok {
|
||||||
t.Fatalf("expected responses request body to contain input")
|
t.Fatalf("expected responses request body to contain input")
|
||||||
|
|
@ -124,7 +124,7 @@ func TestProviderChat_ResponsesBodyUsesSharedTranslatorSemantics(t *testing.T) {
|
||||||
}))
|
}))
|
||||||
defer server.Close()
|
defer server.Close()
|
||||||
|
|
||||||
p := NewProvider("key", server.URL, "", WithResponsesPreferred())
|
p := NewProvider("key", server.URL, "")
|
||||||
_, err := p.Chat(
|
_, err := p.Chat(
|
||||||
t.Context(),
|
t.Context(),
|
||||||
[]Message{
|
[]Message{
|
||||||
|
|
@ -132,7 +132,7 @@ func TestProviderChat_ResponsesBodyUsesSharedTranslatorSemantics(t *testing.T) {
|
||||||
{Role: "user", Content: "Transcribe this", Media: []string{"data:audio/wav;base64,AAAA"}},
|
{Role: "user", Content: "Transcribe this", Media: []string{"data:audio/wav;base64,AAAA"}},
|
||||||
},
|
},
|
||||||
nil,
|
nil,
|
||||||
"gpt-4o",
|
"gpt-5.2",
|
||||||
nil,
|
nil,
|
||||||
)
|
)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|
@ -154,6 +154,49 @@ func TestProviderChat_ResponsesBodyUsesSharedTranslatorSemantics(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestProviderChat_UsesChatCompletionsForNonGPT5Models(t *testing.T) {
|
||||||
|
var paths []string
|
||||||
|
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
paths = append(paths, r.URL.Path)
|
||||||
|
|
||||||
|
switch r.URL.Path {
|
||||||
|
case "/chat/completions":
|
||||||
|
resp := map[string]any{
|
||||||
|
"choices": []map[string]any{{
|
||||||
|
"message": map[string]any{"content": "from chat completions"},
|
||||||
|
"finish_reason": "stop",
|
||||||
|
}},
|
||||||
|
}
|
||||||
|
w.Header().Set("Content-Type", "application/json")
|
||||||
|
json.NewEncoder(w).Encode(resp)
|
||||||
|
case "/responses":
|
||||||
|
http.Error(w, "responses should not be used", http.StatusBadRequest)
|
||||||
|
default:
|
||||||
|
http.Error(w, "not found", http.StatusNotFound)
|
||||||
|
}
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
p := NewProvider("key", server.URL, "")
|
||||||
|
out, err := p.Chat(
|
||||||
|
t.Context(),
|
||||||
|
[]Message{{Role: "user", Content: "hi"}},
|
||||||
|
nil,
|
||||||
|
"Qwen3.5-35B-A3B",
|
||||||
|
nil,
|
||||||
|
)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Chat() error = %v", err)
|
||||||
|
}
|
||||||
|
if out.Content != "from chat completions" {
|
||||||
|
t.Fatalf("Content = %q, want %q", out.Content, "from chat completions")
|
||||||
|
}
|
||||||
|
if !reflect.DeepEqual(paths, []string{"/chat/completions"}) {
|
||||||
|
t.Fatalf("paths = %v, want [/chat/completions]", paths)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestProviderChat_FallsBackToChatCompletionsWhenResponsesFails(t *testing.T) {
|
func TestProviderChat_FallsBackToChatCompletionsWhenResponsesFails(t *testing.T) {
|
||||||
var paths []string
|
var paths []string
|
||||||
|
|
||||||
|
|
@ -616,7 +659,9 @@ func TestProviderChat_ParsesRefusalFromResponses(t *testing.T) {
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestParseResponsesResponse_FailedStatusUsesServerMessage(t *testing.T) {
|
func TestParseResponsesResponse_FailedStatusUsesServerMessage(t *testing.T) {
|
||||||
_, err := parseResponsesResponse(strings.NewReader(`{"status":" failed ","error":{"message":"responses failed"}}`))
|
_, err := parseResponsesResponse(
|
||||||
|
strings.NewReader(`{"status":" failed ","error":{"message":"responses failed"}}`),
|
||||||
|
)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
t.Fatal("expected error, got nil")
|
t.Fatal("expected error, got nil")
|
||||||
}
|
}
|
||||||
|
|
@ -626,7 +671,11 @@ func TestParseResponsesResponse_FailedStatusUsesServerMessage(t *testing.T) {
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestParseResponsesResponse_UsesNormalizedIncompleteStatus(t *testing.T) {
|
func TestParseResponsesResponse_UsesNormalizedIncompleteStatus(t *testing.T) {
|
||||||
out, err := parseResponsesResponse(strings.NewReader(`{"status":" incomplete ","output":[{"type":"message","content":[{"type":"output_text","text":"partial answer"}]}],"incomplete_details":{"reason":"content_filter"}}`))
|
out, err := parseResponsesResponse(
|
||||||
|
strings.NewReader(
|
||||||
|
`{"status":" incomplete ","output":[{"type":"message","content":[{"type":"output_text","text":"partial answer"}]}],"incomplete_details":{"reason":"content_filter"}}`,
|
||||||
|
),
|
||||||
|
)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("parseResponsesResponse() error = %v", err)
|
t.Fatalf("parseResponsesResponse() error = %v", err)
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -360,7 +360,9 @@ func parseResponseEnvelope(apiResp *responseEnvelope) (*protocoltypes.LLMRespons
|
||||||
finishReason = "tool_calls"
|
finishReason = "tool_calls"
|
||||||
} else if status == "incomplete" {
|
} else if status == "incomplete" {
|
||||||
finishReason = "length"
|
finishReason = "length"
|
||||||
if apiResp.IncompleteDetails != nil && apiResp.IncompleteDetails.Reason != "" && apiResp.IncompleteDetails.Reason != "max_output_tokens" {
|
if apiResp.IncompleteDetails != nil &&
|
||||||
|
apiResp.IncompleteDetails.Reason != "" &&
|
||||||
|
apiResp.IncompleteDetails.Reason != "max_output_tokens" {
|
||||||
finishReason = apiResp.IncompleteDetails.Reason
|
finishReason = apiResp.IncompleteDetails.Reason
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue