From cc9334fdd97fe640585acc38be2f43eb5048fcb6 Mon Sep 17 00:00:00 2001 From: Raihan Afiandi Date: Mon, 25 May 2026 05:18:23 +0000 Subject: [PATCH] fix(providers): kimi_coding requires reasoning_content on assistant tool-call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upstream returns HTTP 400 `thinking is enabled but reasoning_content is missing in assistant tool call message at index N` when an assistant message with tool_calls is replayed in history without a reasoning_content field. Kimi has server-side thinking enabled by default for kimi-k2-turbo-preview, so the field is required even when goclaw doesn't have captured reasoning content to send (e.g. the model emitted a tool_call without any thinking, or the stream chunk that carried it was lost). The existing branch already gates on openAIWireAssistantReasoningContent(model) (kimi/deepseek/o-series) and emits the field only when Thinking != "". Extend so kimi_coding also emits an empty string when Thinking is unset — satisfies Kimi's "must be present" check without inventing reasoning content. Other providers in the allowlist keep today's behavior: omit when empty. Three new tests: - kimi_coding always carries reasoning_content on assistant - kimi_coding preserves real Thinking content when set - non-kimi providers (deepseek) do NOT inject empty reasoning_content Reference: NousResearch/hermes-agent plugins/model-providers/kimi-coding documents the same upstream behavior (thinking enabled by default, reasoning_content roundtrip required). --- .../providers/openai_extra_headers_test.go | 76 +++++++++++++++++++ internal/providers/openai_request.go | 16 +++- 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/internal/providers/openai_extra_headers_test.go b/internal/providers/openai_extra_headers_test.go index 68ef5205..d46593ed 100644 --- a/internal/providers/openai_extra_headers_test.go +++ b/internal/providers/openai_extra_headers_test.go @@ -125,3 +125,79 @@ func TestKimiCoding_TemperatureSentForOtherProviders(t *testing.T) { t.Errorf("temperature = %v, want 0.7", got) } } + +// TestKimiCoding_ReasoningContentAlwaysPresentOnAssistant reproduces upstream +// "thinking is enabled but reasoning_content is missing in assistant tool call +// message" — when an assistant message has tool_calls but no captured Thinking, +// kimi_coding must still carry reasoning_content (empty string is fine). +func TestKimiCoding_ReasoningContentAlwaysPresentOnAssistant(t *testing.T) { + p := NewOpenAIProvider("kimi-coding", "sk", "https://api.kimi.com/coding/v1", "kimi-k2-turbo-preview"). + WithProviderType("kimi_coding") + + body := p.buildRequestBody("kimi-k2-turbo-preview", ChatRequest{ + Messages: []Message{ + {Role: "user", Content: "list pods"}, + {Role: "assistant", ToolCalls: []ToolCall{{ID: "call_1", Name: "exec", Arguments: map[string]any{"cmd": "kubectl get pods"}}}}, + {Role: "tool", Content: "...", ToolCallID: "call_1"}, + }, + }, true) + + msgs, ok := body["messages"].([]map[string]any) + if !ok { + t.Fatalf("messages not []map[string]any: %T", body["messages"]) + } + if len(msgs) != 3 { + t.Fatalf("expected 3 messages, got %d", len(msgs)) + } + // Assistant tool-call message must carry reasoning_content key. + assistant := msgs[1] + rc, present := assistant["reasoning_content"] + if !present { + t.Fatalf("kimi_coding assistant tool-call message must include reasoning_content key; got %v", assistant) + } + if rc != "" { + t.Errorf("reasoning_content = %q, want empty string when Thinking unset", rc) + } +} + +// TestKimiCoding_ReasoningContentPreservedWhenSet ensures the empty-string +// fallback doesn't clobber real captured thinking content. +// (Use a non-trailing assistant message — buildRequestBody strips trailing +// assistant prefills as a safety net for proxy providers.) +func TestKimiCoding_ReasoningContentPreservedWhenSet(t *testing.T) { + p := NewOpenAIProvider("kimi-coding", "sk", "https://api.kimi.com/coding/v1", "kimi-k2-turbo-preview"). + WithProviderType("kimi_coding") + + body := p.buildRequestBody("kimi-k2-turbo-preview", ChatRequest{ + Messages: []Message{ + {Role: "user", Content: "hi"}, + {Role: "assistant", Content: "hello", Thinking: "the user said hi"}, + {Role: "user", Content: "more"}, + }, + }, true) + + msgs := body["messages"].([]map[string]any) + if got := msgs[1]["reasoning_content"]; got != "the user said hi" { + t.Errorf("reasoning_content = %q, want %q", got, "the user said hi") + } +} + +// TestNonKimi_ReasoningContentNotAddedWhenEmpty is the negative control — for +// other providers in the allowlist (e.g. deepseek), an empty Thinking must NOT +// inject an empty reasoning_content key, preserving today's behavior. +func TestNonKimi_ReasoningContentNotAddedWhenEmpty(t *testing.T) { + p := NewOpenAIProvider("deepseek", "sk", "https://api.deepseek.com/v1", "deepseek-chat") + + body := p.buildRequestBody("deepseek-chat", ChatRequest{ + Messages: []Message{ + {Role: "user", Content: "hi"}, + {Role: "assistant", ToolCalls: []ToolCall{{ID: "call_1", Name: "exec", Arguments: map[string]any{}}}}, + {Role: "tool", Content: "...", ToolCallID: "call_1"}, + }, + }, true) + + msgs := body["messages"].([]map[string]any) + if _, present := msgs[1]["reasoning_content"]; present { + t.Error("non-kimi providers must not inject empty reasoning_content; key should be absent") + } +} diff --git a/internal/providers/openai_request.go b/internal/providers/openai_request.go index a2757d21..485b8c84 100644 --- a/internal/providers/openai_request.go +++ b/internal/providers/openai_request.go @@ -58,8 +58,20 @@ func (p *OpenAIProvider) buildRequestBody(model string, req ChatRequest, stream // Echo reasoning_content only for APIs/models that accept it on assistant history. // Together Qwen and many OpenAI-compat gateways reject unknown message fields → HTTP 400. - if m.Thinking != "" && m.Role == "assistant" && openAIWireAssistantReasoningContent(model) { - msg["reasoning_content"] = m.Thinking + // + // Kimi Coding is stricter: when its server-side thinking is on (always-on for + // kimi-k2-turbo-preview), assistant tool-call messages MUST carry + // reasoning_content even if empty — otherwise upstream returns 400 "thinking + // is enabled but reasoning_content is missing in assistant tool call message". + if m.Role == "assistant" && openAIWireAssistantReasoningContent(model) { + switch { + case m.Thinking != "": + msg["reasoning_content"] = m.Thinking + case p.providerType == "kimi_coding": + // Send empty string rather than omit the field — satisfies Kimi's + // "must be present" check without inventing reasoning content. + msg["reasoning_content"] = "" + } } // Include content; omit empty content for assistant messages with tool_calls