diff --git a/AGENTS.md b/AGENTS.md index 57027473d7b..79631a6fe78 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -56,3 +56,31 @@ go build -o test-output ./cmd/server && rm test-output # Verify compile (REQUIRE - Use logrus structured logging; avoid leaking secrets/tokens in logs - Avoid panics in HTTP handlers; prefer logged errors and meaningful HTTP status codes - Timeouts are allowed only during credential acquisition; after an upstream connection is established, do not set timeouts for any subsequent network behavior. Intentional exceptions that must remain allowed are the Codex websocket liveness deadlines in `internal/runtime/executor/codex_websockets_executor.go`, the wsrelay session deadlines in `internal/wsrelay/session.go`, the management APICall timeout in `internal/api/handlers/management/api_tools.go`, and the `cmd/fetch_antigravity_models` utility timeouts + +### [Testing] Baseline full-suite failures can be unrelated to the current patch +Detailed description: +Running `go test ./...` on `fix/preserve-reasoning-content` surfaced existing failures outside the Responses reasoning follow-up work: +- `internal/registry`: `TestCodexFreeModelsExcludeGPT55` +- `internal/runtime/executor`: `TestEnsureAccessToken_WarmTokenLoadsCreditsHint` +- `internal/runtime/executor`: `TestUpdateAntigravityCreditsBalance_LoadCodeAssistUserAgent` +These failures can block "green full suite" expectations even when the modified package under review is passing. + +Impact scope: +AI agents reviewing or preparing commits for narrowly scoped translator/request fixes may incorrectly assume their patch caused unrelated red tests, delaying or broadening the change unnecessarily. + +Suggested solutions: +- Record both the full-suite result and the package-scoped result when reporting verification. +- For Responses reasoning fixes, verify at minimum `go test ./internal/translator/openai/openai/responses` and `go build -o test-output ./cmd/server`. +- Treat unrelated full-suite failures as baseline noise unless the diff touches the failing package. + +### [Change Scope] Do not mix unverified local executor refactors into Responses-only fixes +Detailed description: +The working tree may contain extra local edits under `internal/runtime/executor/` that are not required for a Responses translator issue. In this session, `internal/runtime/executor/reasoning_preserve.go` included a separate strategy change that rebuilds the entire `messages` array after patching reasoning fields. That implementation detail is broader than the Responses follow-up fix and needs its own dedicated validation before inclusion. + +Impact scope: +If an agent stages all modified files blindly, a small Responses bugfix commit can accidentally absorb executor behavior changes that were not part of the same root cause or acceptance scope. + +Suggested solutions: +- Stage only files directly tied to the issue being fixed. +- When executor-side reasoning preservation logic changes independently, add focused tests for the specific reconstruction strategy before committing it. +- Call out excluded local files explicitly in the handoff or commit summary. diff --git a/internal/runtime/executor/openai_compat_executor.go b/internal/runtime/executor/openai_compat_executor.go index 82fc9e97d8d..25162febfab 100644 --- a/internal/runtime/executor/openai_compat_executor.go +++ b/internal/runtime/executor/openai_compat_executor.go @@ -89,11 +89,10 @@ func (e *OpenAICompatExecutor) Execute(ctx context.Context, auth *cliproxyauth.A to = sdktranslator.FromString("openai-response") endpoint = "/responses/compact" } - originalPayloadSource := req.Payload + originalPayload := req.Payload if len(opts.OriginalRequest) > 0 { - originalPayloadSource = opts.OriginalRequest + originalPayload = opts.OriginalRequest } - originalPayload := originalPayloadSource originalTranslated := sdktranslator.TranslateRequest(from, to, baseModel, originalPayload, opts.Stream) translated := sdktranslator.TranslateRequest(from, to, baseModel, req.Payload, opts.Stream) @@ -105,6 +104,12 @@ func (e *OpenAICompatExecutor) Execute(ctx context.Context, auth *cliproxyauth.A requestedModel := helps.PayloadRequestedModel(opts, req.Model) requestPath := helps.PayloadRequestPath(opts) translated = helps.ApplyPayloadConfigWithRoot(e.cfg, baseModel, to.String(), "", translated, originalTranslated, requestedModel, requestPath) + + translated, err = preserveReasoningContent(originalTranslated, translated) + if err != nil { + return resp, err + } + if opts.Alt == "responses/compact" { if updated, errDelete := sjson.DeleteBytes(translated, "stream"); errDelete == nil { translated = updated @@ -193,11 +198,15 @@ func (e *OpenAICompatExecutor) ExecuteStream(ctx context.Context, auth *cliproxy from := opts.SourceFormat to := sdktranslator.FromString("openai") - originalPayloadSource := req.Payload + endpoint := "/chat/completions" + if opts.Alt == "responses/compact" { + to = sdktranslator.FromString("openai-response") + endpoint = "/responses/compact" + } + originalPayload := req.Payload if len(opts.OriginalRequest) > 0 { - originalPayloadSource = opts.OriginalRequest + originalPayload = opts.OriginalRequest } - originalPayload := originalPayloadSource originalTranslated := sdktranslator.TranslateRequest(from, to, baseModel, originalPayload, true) translated := sdktranslator.TranslateRequest(from, to, baseModel, req.Payload, true) @@ -210,11 +219,16 @@ func (e *OpenAICompatExecutor) ExecuteStream(ctx context.Context, auth *cliproxy requestPath := helps.PayloadRequestPath(opts) translated = helps.ApplyPayloadConfigWithRoot(e.cfg, baseModel, to.String(), "", translated, originalTranslated, requestedModel, requestPath) + translated, err = preserveReasoningContent(originalTranslated, translated) + if err != nil { + return nil, err + } + // Request usage data in the final streaming chunk so that token statistics // are captured even when the upstream is an OpenAI-compatible provider. translated, _ = sjson.SetBytes(translated, "stream_options.include_usage", true) - url := strings.TrimSuffix(baseURL, "/") + "/chat/completions" + url := strings.TrimSuffix(baseURL, "/") + endpoint httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(translated)) if err != nil { return nil, err diff --git a/internal/runtime/executor/reasoning_preserve.go b/internal/runtime/executor/reasoning_preserve.go new file mode 100644 index 00000000000..16f39bc32a8 --- /dev/null +++ b/internal/runtime/executor/reasoning_preserve.go @@ -0,0 +1,96 @@ +package executor + +import ( + "fmt" + "strings" + + "github.com/tidwall/gjson" + "github.com/tidwall/sjson" +) + +// preserveReasoningContent ensures assistant messages in the translated OpenAI-format +// payload retain reasoning_content from the original source payload. +// +// DeepSeek and other providers that support thinking mode require reasoning_content +// to be passed back verbatim in multi-turn conversations. Without this, the API returns +// a 400 error: "The reasoning_content in the thinking mode must be passed back to the API." +// +// Matching strategy: instead of requiring identical message counts (which breaks when +// translation inserts/splits messages like Claude tool_result → tool role), we match +// assistant messages by their ordinal position within the assistant-only sequence. +// This is robust because translation never reorders or drops assistant messages — +// it only inserts non-assistant messages (tool, system) around them. +// +// When the translated payload already carries reasoning_content at a given assistant +// ordinal (e.g. from a payload override or from translation), that value is preserved — +// the user or translator has explicitly set it and their intent takes precedence. +// Only when reasoning_content is missing do we fall back to the original value. +// +// Error contract: on sjson.SetBytes failure, the function discards any partial writes +// and returns the unmodified translated input along with the error, so the caller never +// receives a partially-patched payload. +func preserveReasoningContent(original, translated []byte) ([]byte, error) { + if len(original) == 0 || len(translated) == 0 { + return translated, nil + } + if !gjson.ValidBytes(original) || !gjson.ValidBytes(translated) { + return translated, nil + } + + origMsgs := gjson.GetBytes(original, "messages") + if !origMsgs.Exists() || !origMsgs.IsArray() { + return translated, nil + } + origMsgArr := origMsgs.Array() + + transMsgs := gjson.GetBytes(translated, "messages") + if !transMsgs.Exists() || !transMsgs.IsArray() { + return translated, nil + } + transMsgArr := transMsgs.Array() + + origReasoning := collectAssistantReasoning(origMsgArr) + if len(origReasoning) == 0 { + return translated, nil + } + + out := translated + assistantOrdinal := 0 + for i, msg := range transMsgArr { + if strings.TrimSpace(msg.Get("role").String()) != "assistant" { + continue + } + + origText, origOK := origReasoning[assistantOrdinal] + transRC := msg.Get("reasoning_content") + if origOK && !transRC.Exists() { + path := fmt.Sprintf("messages.%d.reasoning_content", i) + next, err := sjson.SetBytes(out, path, origText) + if err != nil { + return translated, fmt.Errorf("preserveReasoningContent: failed to set reasoning_content at index %d: %w", i, err) + } + out = next + } + assistantOrdinal++ + } + + return out, nil +} + +// collectAssistantReasoning extracts reasoning_content from assistant messages, +// keyed by their ordinal position in the assistant-only sequence (0, 1, 2, ...). +// Empty-string reasoning_content is preserved because DeepSeek requires it. +func collectAssistantReasoning(messages []gjson.Result) map[int]string { + reasoning := make(map[int]string) + ordinal := 0 + for _, msg := range messages { + if strings.TrimSpace(msg.Get("role").String()) != "assistant" { + continue + } + if rc := msg.Get("reasoning_content"); rc.Exists() { + reasoning[ordinal] = rc.String() + } + ordinal++ + } + return reasoning +} diff --git a/internal/runtime/executor/reasoning_preserve_test.go b/internal/runtime/executor/reasoning_preserve_test.go new file mode 100644 index 00000000000..4a021b27db4 --- /dev/null +++ b/internal/runtime/executor/reasoning_preserve_test.go @@ -0,0 +1,396 @@ +package executor + +import ( + "testing" + + "github.com/tidwall/gjson" +) + +func TestPreserveReasoningContent_PreservesEmptyStringReasoning(t *testing.T) { + original := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer","reasoning_content":""}, + {"role":"user","content":"follow up"} + ] + }`) + translated := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer"}, + {"role":"user","content":"follow up"} + ] + }`) + + out, err := preserveReasoningContent(original, translated) + if err != nil { + t.Fatalf("preserveReasoningContent() error = %v", err) + } + + rc := gjson.GetBytes(out, "messages.1.reasoning_content") + if !rc.Exists() { + t.Fatalf("messages.1.reasoning_content should exist (even if empty)") + } + if rc.String() != "" { + t.Fatalf("messages.1.reasoning_content = %q, want empty string", rc.String()) + } +} + +func TestPreserveReasoningContent_DoesNotInheritReasoningForMissingMessages(t *testing.T) { + original := []byte(`{ + "messages":[ + {"role":"user","content":"list files"}, + {"role":"assistant","content":"I'll list the files","reasoning_content":"let me check the directory"}, + {"role":"tool","tool_call_id":"call_1","content":"[file1.txt, file2.txt]"}, + {"role":"assistant","content":"Here are the files"} + ] + }`) + translated := []byte(`{ + "messages":[ + {"role":"user","content":"list files"}, + {"role":"assistant","content":"I'll list the files"}, + {"role":"tool","tool_call_id":"call_1","content":"[file1.txt, file2.txt]"}, + {"role":"assistant","content":"Here are the files"} + ] + }`) + + out, err := preserveReasoningContent(original, translated) + if err != nil { + t.Fatalf("preserveReasoningContent() error = %v", err) + } + + rc1 := gjson.GetBytes(out, "messages.1.reasoning_content").String() + if rc1 != "let me check the directory" { + t.Fatalf("messages.1.reasoning_content = %q, want %q", rc1, "let me check the directory") + } + + if gjson.GetBytes(out, "messages.3.reasoning_content").Exists() { + t.Fatalf("messages.3.reasoning_content should not exist when original had none") + } +} + +func TestPreserveReasoningContent_NoOpWhenNoOriginalReasoning(t *testing.T) { + original := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer"} + ] + }`) + translated := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer"} + ] + }`) + + out, err := preserveReasoningContent(original, translated) + if err != nil { + t.Fatalf("preserveReasoningContent() error = %v", err) + } + + if gjson.GetBytes(out, "messages.1.reasoning_content").Exists() { + t.Fatalf("messages.1.reasoning_content should not exist when original has none") + } +} + +func TestPreserveReasoningContent_IgnoresNonAssistantMessages(t *testing.T) { + original := []byte(`{ + "messages":[ + {"role":"system","content":"you are helpful"}, + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer","reasoning_content":"thinking..."}, + {"role":"tool","tool_call_id":"call_1","content":"data"} + ] + }`) + translated := []byte(`{ + "messages":[ + {"role":"system","content":"you are helpful"}, + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer"}, + {"role":"tool","tool_call_id":"call_1","content":"data"} + ] + }`) + + out, err := preserveReasoningContent(original, translated) + if err != nil { + t.Fatalf("preserveReasoningContent() error = %v", err) + } + + if gjson.GetBytes(out, "messages.0.reasoning_content").Exists() { + t.Fatalf("system message should not get reasoning_content") + } + if gjson.GetBytes(out, "messages.1.reasoning_content").Exists() { + t.Fatalf("user message should not get reasoning_content") + } + if !gjson.GetBytes(out, "messages.2.reasoning_content").Exists() { + t.Fatalf("assistant message should have reasoning_content preserved") + } + if gjson.GetBytes(out, "messages.3.reasoning_content").Exists() { + t.Fatalf("tool message should not get reasoning_content") + } +} + +func TestPreserveReasoningContent_KeepsExistingNonEmptyReasoning(t *testing.T) { + original := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer","reasoning_content":"let me think..."} + ] + }`) + translated := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer"} + ] + }`) + + out, err := preserveReasoningContent(original, translated) + if err != nil { + t.Fatalf("preserveReasoningContent() error = %v", err) + } + + got := gjson.GetBytes(out, "messages.1.reasoning_content").String() + if got != "let me think..." { + t.Fatalf("messages.1.reasoning_content = %q, want %q", got, "let me think...") + } +} + +func TestPreserveReasoningContent_KeepsTranslatedReasoningWhenOriginalLacksIt(t *testing.T) { + original := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer"} + ] + }`) + translated := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer","reasoning_content":"from upstream"} + ] + }`) + + out, err := preserveReasoningContent(original, translated) + if err != nil { + t.Fatalf("preserveReasoningContent() error = %v", err) + } + + got := gjson.GetBytes(out, "messages.1.reasoning_content").String() + if got != "from upstream" { + t.Fatalf("messages.1.reasoning_content = %q, want %q", got, "from upstream") + } +} + +func TestPreserveReasoningContent_OrdinalMatchingAcrossMessageCountMismatch(t *testing.T) { + original := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer","reasoning_content":"thinking..."} + ] + }`) + translated := []byte(`{ + "messages":[ + {"role":"system","content":"you are helpful"}, + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer"} + ] + }`) + + out, err := preserveReasoningContent(original, translated) + if err != nil { + t.Fatalf("preserveReasoningContent() error = %v", err) + } + + rc := gjson.GetBytes(out, "messages.2.reasoning_content") + if !rc.Exists() { + t.Fatalf("assistant message (ordinal 0) should have reasoning_content preserved despite message count mismatch") + } + if rc.String() != "thinking..." { + t.Fatalf("messages.2.reasoning_content = %q, want %q", rc.String(), "thinking...") + } +} + +func TestPreserveReasoningContent_OrdinalMatchingMultipleAssistants(t *testing.T) { + original := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer1","reasoning_content":"think1"}, + {"role":"user","content":"more"}, + {"role":"assistant","content":"answer2","reasoning_content":"think2"} + ] + }`) + translated := []byte(`{ + "messages":[ + {"role":"system","content":"system"}, + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer1"}, + {"role":"user","content":"more"}, + {"role":"assistant","content":"answer2"} + ] + }`) + + out, err := preserveReasoningContent(original, translated) + if err != nil { + t.Fatalf("preserveReasoningContent() error = %v", err) + } + + rc1 := gjson.GetBytes(out, "messages.2.reasoning_content") + if !rc1.Exists() || rc1.String() != "think1" { + t.Fatalf("first assistant (ordinal 0): got %q, want %q", rc1.String(), "think1") + } + + rc2 := gjson.GetBytes(out, "messages.4.reasoning_content") + if !rc2.Exists() || rc2.String() != "think2" { + t.Fatalf("second assistant (ordinal 1): got %q, want %q", rc2.String(), "think2") + } +} + +func TestPreserveReasoningContent_OrdinalMatchingWithToolCalls(t *testing.T) { + original := []byte(`{ + "messages":[ + {"role":"user","content":"list files"}, + {"role":"assistant","content":"I'll check","reasoning_content":"need to ls","tool_calls":[{"id":"c1","type":"function","function":{"name":"ls","arguments":"{}"}}]}, + {"role":"tool","tool_call_id":"c1","content":"file1.txt"}, + {"role":"assistant","content":"Here are the files","reasoning_content":"got the list"} + ] + }`) + translated := []byte(`{ + "messages":[ + {"role":"user","content":"list files"}, + {"role":"assistant","content":"I'll check","tool_calls":[{"id":"c1","type":"function","function":{"name":"ls","arguments":"{}"}}]}, + {"role":"tool","tool_call_id":"c1","content":"file1.txt"}, + {"role":"assistant","content":"Here are the files"} + ] + }`) + + out, err := preserveReasoningContent(original, translated) + if err != nil { + t.Fatalf("preserveReasoningContent() error = %v", err) + } + + rc1 := gjson.GetBytes(out, "messages.1.reasoning_content").String() + if rc1 != "need to ls" { + t.Fatalf("first assistant reasoning = %q, want %q", rc1, "need to ls") + } + + rc2 := gjson.GetBytes(out, "messages.3.reasoning_content").String() + if rc2 != "got the list" { + t.Fatalf("second assistant reasoning = %q, want %q", rc2, "got the list") + } +} + +func TestPreserveReasoningContent_PartialAssistantReasoning(t *testing.T) { + original := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer1","reasoning_content":"thinking..."}, + {"role":"user","content":"more"}, + {"role":"assistant","content":"answer2"} + ] + }`) + translated := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer1"}, + {"role":"user","content":"more"}, + {"role":"assistant","content":"answer2"} + ] + }`) + + out, err := preserveReasoningContent(original, translated) + if err != nil { + t.Fatalf("preserveReasoningContent() error = %v", err) + } + + rc1 := gjson.GetBytes(out, "messages.1.reasoning_content").String() + if rc1 != "thinking..." { + t.Fatalf("first assistant reasoning = %q, want %q", rc1, "thinking...") + } + + if gjson.GetBytes(out, "messages.3.reasoning_content").Exists() { + t.Fatalf("second assistant should not have reasoning_content when original had none") + } +} + +func TestPreserveReasoningContent_OriginalWinsOverTranslated(t *testing.T) { + original := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer","reasoning_content":"original reasoning"} + ] + }`) + translated := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer"} + ] + }`) + + out, err := preserveReasoningContent(original, translated) + if err != nil { + t.Fatalf("preserveReasoningContent() error = %v", err) + } + + got := gjson.GetBytes(out, "messages.1.reasoning_content").String() + if got != "original reasoning" { + t.Fatalf("original must fill in when translated lacks reasoning_content: got %q, want %q", got, "original reasoning") + } +} + +func TestPreserveReasoningContent_TranslatedOverrideTakesPrecedence(t *testing.T) { + original := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer","reasoning_content":"original reasoning"} + ] + }`) + translated := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer","reasoning_content":"override reasoning"} + ] + }`) + + out, err := preserveReasoningContent(original, translated) + if err != nil { + t.Fatalf("preserveReasoningContent() error = %v", err) + } + + got := gjson.GetBytes(out, "messages.1.reasoning_content").String() + if got != "override reasoning" { + t.Fatalf("translated override must take precedence over original: got %q, want %q", got, "override reasoning") + } +} + +func TestPreserveReasoningContent_FilterRemovalIsRespected(t *testing.T) { + original := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer","reasoning_content":"original reasoning"}, + {"role":"user","content":"follow up"}, + {"role":"assistant","content":"answer2","reasoning_content":"original reasoning2"} + ] + }`) + translated := []byte(`{ + "messages":[ + {"role":"user","content":"hello"}, + {"role":"assistant","content":"answer"}, + {"role":"user","content":"follow up"}, + {"role":"assistant","content":"answer2","reasoning_content":"filtered replacement"} + ] + }`) + + out, err := preserveReasoningContent(original, translated) + if err != nil { + t.Fatalf("preserveReasoningContent() error = %v", err) + } + + rc1 := gjson.GetBytes(out, "messages.1.reasoning_content").String() + if rc1 != "original reasoning" { + t.Fatalf("first assistant: original fills in when translated lacks reasoning_content: got %q, want %q", rc1, "original reasoning") + } + + rc2 := gjson.GetBytes(out, "messages.3.reasoning_content").String() + if rc2 != "filtered replacement" { + t.Fatalf("second assistant: translated override takes precedence: got %q, want %q", rc2, "filtered replacement") + } +} diff --git a/internal/translator/openai/openai/responses/openai_openai-responses_request.go b/internal/translator/openai/openai/responses/openai_openai-responses_request.go index 15acf7cdb4f..7cd6264ddba 100644 --- a/internal/translator/openai/openai/responses/openai_openai-responses_request.go +++ b/internal/translator/openai/openai/responses/openai_openai-responses_request.go @@ -74,6 +74,8 @@ func ConvertOpenAIResponsesRequestToOpenAIChatCompletions(modelName string, inpu pendingToolCallIDs := make([]string, 0) awaitingToolOutputs := make(map[string]struct{}) deferredMessages := make([][]byte, 0) + pendingReasoningContent := "" + pendingReasoningContentSet := false flushPendingToolCalls := func() { if len(pendingToolCalls) == 0 { @@ -81,6 +83,11 @@ func ConvertOpenAIResponsesRequestToOpenAIChatCompletions(modelName string, inpu } assistantMessage := []byte(`{"role":"assistant","tool_calls":[]}`) assistantMessage, _ = sjson.SetBytes(assistantMessage, "tool_calls", pendingToolCalls) + if pendingReasoningContentSet { + assistantMessage, _ = sjson.SetBytes(assistantMessage, "reasoning_content", pendingReasoningContent) + pendingReasoningContent = "" + pendingReasoningContentSet = false + } out, _ = sjson.SetRawBytes(out, "messages.-1", assistantMessage) for _, id := range pendingToolCallIDs { if strings.TrimSpace(id) == "" { @@ -170,6 +177,16 @@ func ConvertOpenAIResponsesRequestToOpenAIChatCompletions(modelName string, inpu message, _ = sjson.SetBytes(message, "content", content.String()) } + if role == "assistant" { + if rc := item.Get("reasoning_content"); rc.Exists() { + message, _ = sjson.SetBytes(message, "reasoning_content", rc.String()) + } else if pendingReasoningContentSet { + message, _ = sjson.SetBytes(message, "reasoning_content", pendingReasoningContent) + pendingReasoningContent = "" + pendingReasoningContentSet = false + } + } + appendRegularMessage(message) case "function_call": @@ -213,6 +230,26 @@ func ConvertOpenAIResponsesRequestToOpenAIChatCompletions(modelName string, inpu if len(awaitingToolOutputs) == 0 && len(deferredMessages) > 0 { flushDeferredMessages() } + + case "reasoning": + if summary := item.Get("summary"); summary.Exists() && summary.IsArray() { + var textParts []string + hasSummaryText := false + summary.ForEach(func(_, s gjson.Result) bool { + if t := s.Get("text"); t.Exists() { + hasSummaryText = true + textParts = append(textParts, t.String()) + } + return true + }) + if hasSummaryText { + pendingReasoningContent = strings.Join(textParts, "") + pendingReasoningContentSet = true + } + } else if ec := item.Get("encrypted_content"); ec.Exists() { + pendingReasoningContent = ec.String() + pendingReasoningContentSet = true + } } } diff --git a/internal/translator/openai/openai/responses/openai_openai-responses_request_test.go b/internal/translator/openai/openai/responses/openai_openai-responses_request_test.go index 9dd0e288b2c..e0d86638e8b 100644 --- a/internal/translator/openai/openai/responses/openai_openai-responses_request_test.go +++ b/internal/translator/openai/openai/responses/openai_openai-responses_request_test.go @@ -122,3 +122,200 @@ func TestConvertOpenAIResponsesRequestToOpenAIChatCompletions_DefersMessageUntil t.Fatalf("messages.3.content = %q, want %q", got, "next") } } + +func TestConvertOpenAIResponsesRequestToOpenAIChatCompletions_ReasoningContentPreserved(t *testing.T) { + raw := []byte(`{ + "input": [ + {"type":"message","role":"user","content":[{"type":"input_text","text":"hello"}]}, + {"type":"message","role":"assistant","content":[{"type":"output_text","text":"answer"}],"reasoning_content":"thinking step by step"}, + {"type":"message","role":"user","content":[{"type":"input_text","text":"follow up"}]} + ] + }`) + + out := ConvertOpenAIResponsesRequestToOpenAIChatCompletions("deepseek-r1", raw, true) + + rc := gjson.GetBytes(out, "messages.1.reasoning_content") + if !rc.Exists() { + t.Fatalf("messages.1.reasoning_content should exist") + } + if rc.String() != "thinking step by step" { + t.Fatalf("messages.1.reasoning_content = %q, want %q", rc.String(), "thinking step by step") + } + + if gjson.GetBytes(out, "messages.0.reasoning_content").Exists() { + t.Fatalf("user message should not have reasoning_content") + } +} + +func TestConvertOpenAIResponsesRequestToOpenAIChatCompletions_ReasoningContentOnlyOnAssistant(t *testing.T) { + raw := []byte(`{ + "input": [ + {"type":"message","role":"user","content":[{"type":"input_text","text":"hello"}],"reasoning_content":"should not transfer"}, + {"type":"message","role":"assistant","content":[{"type":"output_text","text":"answer"}]} + ] + }`) + + out := ConvertOpenAIResponsesRequestToOpenAIChatCompletions("deepseek-r1", raw, false) + + if gjson.GetBytes(out, "messages.0.reasoning_content").Exists() { + t.Fatalf("user message should not have reasoning_content even if original had it") + } + if gjson.GetBytes(out, "messages.1.reasoning_content").Exists() { + t.Fatalf("assistant message should not have reasoning_content when original had none") + } +} + +func TestConvertOpenAIResponsesRequestToOpenAIChatCompletions_ReasoningItemSummary(t *testing.T) { + raw := []byte(`{ + "input": [ + {"type":"message","role":"user","content":[{"type":"input_text","text":"hello"}]}, + {"type":"reasoning","id":"rs_abc","summary":[{"type":"summary_text","text":"thinking step by step"}]}, + {"type":"message","role":"assistant","content":[{"type":"output_text","text":"answer"}]}, + {"type":"message","role":"user","content":[{"type":"input_text","text":"follow up"}]} + ] + }`) + + out := ConvertOpenAIResponsesRequestToOpenAIChatCompletions("deepseek-r1", raw, true) + + rc := gjson.GetBytes(out, "messages.1.reasoning_content") + if !rc.Exists() { + t.Fatalf("messages.1.reasoning_content should exist from reasoning item summary") + } + if rc.String() != "thinking step by step" { + t.Fatalf("messages.1.reasoning_content = %q, want %q", rc.String(), "thinking step by step") + } + + if gjson.GetBytes(out, "messages.0.reasoning_content").Exists() { + t.Fatalf("user message should not have reasoning_content") + } +} + +func TestConvertOpenAIResponsesRequestToOpenAIChatCompletions_ReasoningItemEncryptedContent(t *testing.T) { + raw := []byte(`{ + "input": [ + {"type":"message","role":"user","content":[{"type":"input_text","text":"hello"}]}, + {"type":"reasoning","id":"rs_abc","encrypted_content":"encrypted_reasoning_data"}, + {"type":"message","role":"assistant","content":[{"type":"output_text","text":"answer"}]} + ] + }`) + + out := ConvertOpenAIResponsesRequestToOpenAIChatCompletions("deepseek-r1", raw, false) + + rc := gjson.GetBytes(out, "messages.1.reasoning_content") + if !rc.Exists() { + t.Fatalf("messages.1.reasoning_content should exist from reasoning item encrypted_content") + } + if rc.String() != "encrypted_reasoning_data" { + t.Fatalf("messages.1.reasoning_content = %q, want %q", rc.String(), "encrypted_reasoning_data") + } +} + +func TestConvertOpenAIResponsesRequestToOpenAIChatCompletions_ReasoningItemSummaryMultipleParts(t *testing.T) { + raw := []byte(`{ + "input": [ + {"type":"message","role":"user","content":[{"type":"input_text","text":"hello"}]}, + {"type":"reasoning","id":"rs_abc","summary":[{"type":"summary_text","text":"part1"},{"type":"summary_text","text":"part2"}]}, + {"type":"message","role":"assistant","content":[{"type":"output_text","text":"answer"}]} + ] + }`) + + out := ConvertOpenAIResponsesRequestToOpenAIChatCompletions("deepseek-r1", raw, true) + + rc := gjson.GetBytes(out, "messages.1.reasoning_content") + if !rc.Exists() { + t.Fatalf("messages.1.reasoning_content should exist from reasoning item summary") + } + if rc.String() != "part1part2" { + t.Fatalf("messages.1.reasoning_content = %q, want %q", rc.String(), "part1part2") + } +} + +func TestConvertOpenAIResponsesRequestToOpenAIChatCompletions_ReasoningBeforeFunctionCall(t *testing.T) { + raw := []byte(`{ + "input": [ + {"type":"message","role":"user","content":[{"type":"input_text","text":"hello"}]}, + {"type":"reasoning","id":"rs_abc","summary":[{"type":"summary_text","text":"I need to call a tool"}]}, + {"type":"function_call","call_id":"call_1","name":"search","arguments":"{\"q\":\"test\"}"}, + {"type":"function_call_output","call_id":"call_1","output":"result"} + ] + }`) + + out := ConvertOpenAIResponsesRequestToOpenAIChatCompletions("deepseek-r1", raw, true) + t.Logf("output json:\n%s", prettyJSONForTest(out)) + + rc := gjson.GetBytes(out, "messages.1.reasoning_content") + if !rc.Exists() { + t.Fatalf("messages.1.reasoning_content should exist when reasoning precedes function_call") + } + if rc.String() != "I need to call a tool" { + t.Fatalf("messages.1.reasoning_content = %q, want %q", rc.String(), "I need to call a tool") + } + + if got := gjson.GetBytes(out, "messages.1.role").String(); got != "assistant" { + t.Fatalf("messages.1.role = %q, want %q", got, "assistant") + } + if got := len(gjson.GetBytes(out, "messages.1.tool_calls").Array()); got != 1 { + t.Fatalf("messages.1.tool_calls length = %d, want %d", got, 1) + } +} + +func TestConvertOpenAIResponsesRequestToOpenAIChatCompletions_ReasoningBeforeFunctionCallEncrypted(t *testing.T) { + raw := []byte(`{ + "input": [ + {"type":"message","role":"user","content":[{"type":"input_text","text":"hello"}]}, + {"type":"reasoning","id":"rs_abc","encrypted_content":"enc_data"}, + {"type":"function_call","call_id":"call_1","name":"search","arguments":"{\"q\":\"test\"}"}, + {"type":"function_call_output","call_id":"call_1","output":"result"} + ] + }`) + + out := ConvertOpenAIResponsesRequestToOpenAIChatCompletions("kimi-k2.6", raw, false) + t.Logf("output json:\n%s", prettyJSONForTest(out)) + + rc := gjson.GetBytes(out, "messages.1.reasoning_content") + if !rc.Exists() { + t.Fatalf("messages.1.reasoning_content should exist from encrypted_content when reasoning precedes function_call") + } + if rc.String() != "enc_data" { + t.Fatalf("messages.1.reasoning_content = %q, want %q", rc.String(), "enc_data") + } +} + +func TestConvertOpenAIResponsesRequestToOpenAIChatCompletions_ReasoningItemEmptySummaryText(t *testing.T) { + raw := []byte(`{ + "input": [ + {"type":"message","role":"user","content":[{"type":"input_text","text":"hello"}]}, + {"type":"reasoning","id":"rs_empty","summary":[{"type":"summary_text","text":""}]}, + {"type":"message","role":"assistant","content":[{"type":"output_text","text":"answer"}]} + ] + }`) + + out := ConvertOpenAIResponsesRequestToOpenAIChatCompletions("deepseek-r1", raw, false) + + rc := gjson.GetBytes(out, "messages.1.reasoning_content") + if !rc.Exists() { + t.Fatalf("messages.1.reasoning_content should exist for empty summary text") + } + if rc.String() != "" { + t.Fatalf("messages.1.reasoning_content = %q, want empty string", rc.String()) + } +} + +func TestConvertOpenAIResponsesRequestToOpenAIChatCompletions_ReasoningItemFallsBackToMessageRC(t *testing.T) { + raw := []byte(`{ + "input": [ + {"type":"message","role":"user","content":[{"type":"input_text","text":"hello"}]}, + {"type":"message","role":"assistant","content":[{"type":"output_text","text":"answer"}],"reasoning_content":"from message rc"} + ] + }`) + + out := ConvertOpenAIResponsesRequestToOpenAIChatCompletions("deepseek-r1", raw, true) + + rc := gjson.GetBytes(out, "messages.1.reasoning_content") + if !rc.Exists() { + t.Fatalf("messages.1.reasoning_content should exist from message reasoning_content") + } + if rc.String() != "from message rc" { + t.Fatalf("messages.1.reasoning_content = %q, want %q", rc.String(), "from message rc") + } +} diff --git a/internal/translator/openai/openai/responses/openai_openai-responses_response.go b/internal/translator/openai/openai/responses/openai_openai-responses_response.go index 8895b684452..bc50a6e2fa7 100644 --- a/internal/translator/openai/openai/responses/openai_openai-responses_response.go +++ b/internal/translator/openai/openai/responses/openai_openai-responses_response.go @@ -717,9 +717,10 @@ func ConvertOpenAIChatCompletionsResponseToOpenAIResponsesNonStream(_ context.Co // Build output list from choices[...] outputsWrapper := []byte(`{"arr":[]}`) - // Detect and capture reasoning content if present - rcText := gjson.GetBytes(rawJSON, "choices.0.message.reasoning_content").String() - includeReasoning := rcText != "" + // Detect and capture reasoning content if present. + rcNode := gjson.GetBytes(rawJSON, "choices.0.message.reasoning_content") + rcText := rcNode.String() + includeReasoning := rcNode.Exists() if !includeReasoning && len(requestRawJSON) > 0 { includeReasoning = gjson.GetBytes(requestRawJSON, "reasoning").Exists() } @@ -731,7 +732,7 @@ func ConvertOpenAIChatCompletionsResponseToOpenAIResponsesNonStream(_ context.Co // Prefer summary_text from reasoning_content; encrypted_content is optional reasoningItem := []byte(`{"id":"","type":"reasoning","encrypted_content":"","summary":[]}`) reasoningItem, _ = sjson.SetBytes(reasoningItem, "id", fmt.Sprintf("rs_%s", rid)) - if rcText != "" { + if rcNode.Exists() { reasoningItem, _ = sjson.SetBytes(reasoningItem, "summary.0.type", "summary_text") reasoningItem, _ = sjson.SetBytes(reasoningItem, "summary.0.text", rcText) } diff --git a/internal/translator/openai/openai/responses/openai_openai-responses_response_test.go b/internal/translator/openai/openai/responses/openai_openai-responses_response_test.go index cafcacb7280..c4680687519 100644 --- a/internal/translator/openai/openai/responses/openai_openai-responses_response_test.go +++ b/internal/translator/openai/openai/responses/openai_openai-responses_response_test.go @@ -421,3 +421,55 @@ func TestConvertOpenAIChatCompletionsResponseToOpenAIResponses_FunctionCallDoneA t.Fatalf("unexpected completed function_call order: %v", completedOrder) } } + +func TestConvertOpenAIChatCompletionsResponseToOpenAIResponsesNonStream_PreservesEmptyReasoningContent(t *testing.T) { + request := []byte(`{"model":"deepseek-r1","reasoning":{"effort":"medium"}}`) + raw := []byte(`{ + "id":"resp_empty_reasoning", + "object":"chat.completion", + "created":1773896263, + "model":"deepseek-r1", + "choices":[ + { + "index":0, + "message":{ + "role":"assistant", + "content":"answer", + "reasoning_content":"" + }, + "finish_reason":"stop" + } + ], + "usage":{"prompt_tokens":1,"completion_tokens":1,"total_tokens":2} + }`) + + out := ConvertOpenAIChatCompletionsResponseToOpenAIResponsesNonStream(context.Background(), "deepseek-r1", request, request, raw, nil) + + output := gjson.GetBytes(out, "output") + if !output.Exists() || !output.IsArray() { + t.Fatalf("output should be an array") + } + if got := len(output.Array()); got != 2 { + t.Fatalf("output length = %d, want %d", got, 2) + } + + reasoning := output.Array()[0] + if got := reasoning.Get("type").String(); got != "reasoning" { + t.Fatalf("output[0].type = %q, want %q", got, "reasoning") + } + summary := reasoning.Get("summary") + if !summary.Exists() || !summary.IsArray() || len(summary.Array()) != 1 { + t.Fatalf("reasoning summary should contain one empty summary_text item") + } + if got := summary.Get("0.type").String(); got != "summary_text" { + t.Fatalf("summary[0].type = %q, want %q", got, "summary_text") + } + if got := summary.Get("0.text").String(); got != "" { + t.Fatalf("summary[0].text = %q, want empty string", got) + } + + message := output.Array()[1] + if got := message.Get("type").String(); got != "message" { + t.Fatalf("output[1].type = %q, want %q", got, "message") + } +}