Skip to content

Commit 13a6ed7

Browse files
authored
fix: thinking models with tools returning empty content (reasoning-only retry loop) (#9290)
When clients like Nextcloud or Home Assistant send requests with tools to thinking models (e.g. Gemma 4 with <|channel>thought tags), the response was empty despite the backend producing valid content. Root cause: the C++ autoparser puts clean content in both the raw Response and ChatDeltas. The Go-side PrependThinkingTokenIfNeeded then prepends the thinking start token to the already-clean content, causing ExtractReasoning to classify the entire response as unclosed reasoning. This made cbRawResult empty, triggering a retry loop that never succeeds. Two fixes: - inference.go: check ChatDeltas for content/tool_calls regardless of whether Response is empty, so skipCallerRetry fires correctly - chat.go: when ChatDeltas have content but no tool calls, use that content directly instead of falling back to the empty cbRawResult
1 parent 85be4ff commit 13a6ed7

6 files changed

Lines changed: 151 additions & 8 deletions

File tree

core/http/endpoints/openai/chat.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1045,6 +1045,13 @@ func ChatEndpoint(cl *config.ModelConfigLoader, ml *model.ModelLoader, evaluator
10451045
funcResults = deltaToolCalls
10461046
textContentToReturn = functions.ContentFromChatDeltas(chatDeltas)
10471047
cbReasoning = functions.ReasoningFromChatDeltas(chatDeltas)
1048+
} else if deltaContent := functions.ContentFromChatDeltas(chatDeltas); len(chatDeltas) > 0 && deltaContent != "" {
1049+
// ChatDeltas have content but no tool calls — model answered without using tools.
1050+
// This happens with thinking models (e.g. Gemma 4) where the Go-side reasoning
1051+
// extraction misclassifies clean content as reasoning, leaving cbRawResult empty.
1052+
xlog.Debug("[ChatDeltas] non-SSE: using C++ autoparser content (no tool calls)", "content_len", len(deltaContent))
1053+
textContentToReturn = deltaContent
1054+
cbReasoning = functions.ReasoningFromChatDeltas(chatDeltas)
10481055
} else {
10491056
// Fallback: parse tool calls from raw text
10501057
xlog.Debug("[ChatDeltas] non-SSE: no chat deltas, falling back to Go-side text parsing")
@@ -1067,7 +1074,13 @@ func ChatEndpoint(cl *config.ModelConfigLoader, ml *model.ModelLoader, evaluator
10671074

10681075
switch {
10691076
case noActionsToRun:
1070-
qResult, qErr := handleQuestion(config, funcResults, cbRawResult, predInput)
1077+
// Use textContentToReturn if available (e.g. from ChatDeltas),
1078+
// otherwise fall back to cbRawResult for legacy Go-side parsing.
1079+
questionInput := cbRawResult
1080+
if textContentToReturn != "" {
1081+
questionInput = textContentToReturn
1082+
}
1083+
qResult, qErr := handleQuestion(config, funcResults, questionInput, predInput)
10711084
if qErr != nil {
10721085
xlog.Error("error handling question", "error", qErr)
10731086
}

core/http/endpoints/openai/inference.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -143,12 +143,15 @@ func ComputeChoices(
143143
cb(finetunedResponse, &result)
144144

145145
// Caller-driven retry (tool parsing, reasoning-only, etc.).
146-
// When the C++ autoparser is active, it clears the raw response
147-
// and delivers data via ChatDeltas. If the response is empty but
148-
// ChatDeltas contain actionable data, skip the caller retry —
149-
// the autoparser already parsed the response successfully.
146+
// When the C++ autoparser is active, it may deliver parsed data
147+
// via ChatDeltas while also keeping the raw response. If ChatDeltas
148+
// contain actionable data (content or tool calls), skip the caller
149+
// retry — the autoparser already parsed the response successfully.
150+
// Note: we check ChatDeltas regardless of whether Response is empty,
151+
// because thinking models (e.g. Gemma 4) produce a non-empty Response
152+
// that the Go-side reasoning extraction can misclassify as reasoning-only.
150153
skipCallerRetry := false
151-
if strings.TrimSpace(prediction.Response) == "" && len(prediction.ChatDeltas) > 0 {
154+
if len(prediction.ChatDeltas) > 0 {
152155
for _, d := range prediction.ChatDeltas {
153156
if d.Content != "" || len(d.ToolCalls) > 0 {
154157
skipCallerRetry = true

core/http/endpoints/openai/inference_test.go

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,11 +242,13 @@ var _ = Describe("ComputeChoices", func() {
242242
})
243243

244244
Context("chat deltas from latest attempt", func() {
245-
It("should return chat deltas from the last attempt only", func() {
245+
It("should return chat deltas from the last attempt when retry is allowed", func() {
246+
// When the first attempt has only reasoning (no content/tool calls),
247+
// the caller-driven retry proceeds and we get deltas from the last attempt.
246248
mockInference([]backend.LLMResponse{
247249
{
248250
Response: "retry-me",
249-
ChatDeltas: []*pb.ChatDelta{{Content: "old"}},
251+
ChatDeltas: []*pb.ChatDelta{{ReasoningContent: "thinking..."}},
250252
},
251253
{
252254
Response: "final",
@@ -266,6 +268,40 @@ var _ = Describe("ComputeChoices", func() {
266268
Expect(deltas).To(HaveLen(1))
267269
Expect(deltas[0].Content).To(Equal("new"))
268270
})
271+
272+
It("should keep first attempt deltas when ChatDeltas have content (skip retry)", func() {
273+
// When the first attempt has content in ChatDeltas, skipCallerRetry
274+
// prevents the retry — the autoparser already parsed successfully.
275+
mockInference([]backend.LLMResponse{
276+
{
277+
Response: "autoparser-content",
278+
ChatDeltas: []*pb.ChatDelta{{Content: "first-content"}},
279+
},
280+
{
281+
Response: "should-not-reach",
282+
ChatDeltas: []*pb.ChatDelta{{Content: "second-content"}},
283+
},
284+
})
285+
286+
retryRequested := false
287+
_, _, deltas, err := ComputeChoices(
288+
makeReq(), "test", cfg, nil, appCfg, nil,
289+
func(s string, c *[]schema.Choice) {
290+
*c = append(*c, schema.Choice{Text: s})
291+
},
292+
nil,
293+
func(attempt int) bool {
294+
retryRequested = true
295+
return true
296+
},
297+
)
298+
Expect(err).ToNot(HaveOccurred())
299+
Expect(retryRequested).To(BeFalse(),
300+
"shouldRetry should not be called when ChatDeltas have content")
301+
Expect(deltas).To(HaveLen(1))
302+
Expect(deltas[0].Content).To(Equal("first-content"))
303+
})
304+
269305
})
270306

271307
Context("result choices cleared on retry", func() {

tests/e2e/e2e_suite_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,31 @@ var _ = BeforeSuite(func() {
121121
Expect(err).ToNot(HaveOccurred())
122122
Expect(os.WriteFile(autoparserPath, autoparserYAML, 0644)).To(Succeed())
123123

124+
// Create model config for thinking model + autoparser tests.
125+
// The chat template ends with <|channel>thought to simulate Gemma 4 thinking models.
126+
// This triggers DetectThinkingStartToken and PrependThinkingTokenIfNeeded in the
127+
// reasoning extraction path, reproducing a bug where clean content from the C++
128+
// autoparser gets misclassified as unclosed reasoning.
129+
thinkingAutoparserConfig := map[string]any{
130+
"name": "mock-model-thinking-autoparser",
131+
"backend": "mock-backend",
132+
"parameters": map[string]any{
133+
"model": "mock-model.bin",
134+
},
135+
"template": map[string]any{
136+
"chat": "{{.Input}}\n<|turn>model\n<|channel>thought\n<channel|>",
137+
},
138+
"function": map[string]any{
139+
"grammar": map[string]any{
140+
"disable": true,
141+
},
142+
},
143+
}
144+
thinkingAutoparserPath := filepath.Join(modelsPath, "mock-model-thinking-autoparser.yaml")
145+
thinkingAutoparserYAML, err := yaml.Marshal(thinkingAutoparserConfig)
146+
Expect(err).ToNot(HaveOccurred())
147+
Expect(os.WriteFile(thinkingAutoparserPath, thinkingAutoparserYAML, 0644)).To(Succeed())
148+
124149
// Start mock MCP server and create MCP-enabled model config
125150
mcpServerURL, mcpServerShutdown = startMockMCPServer()
126151
mcpConfig := mcpModelConfig(mcpServerURL)

tests/e2e/mock-backend/main.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,26 @@ func (m *MockBackend) Predict(ctx context.Context, in *pb.PredictOptions) (*pb.R
9595
}, nil
9696
}
9797

98+
// Simulate Gemma 4 / thinking model with C++ autoparser:
99+
// - Message contains the clean content (autoparser extracts it from OAI choices[0].message.content)
100+
// - ChatDeltas contain both reasoning and content separately
101+
// This reproduces the bug where Go-side PrependThinkingTokenIfNeeded
102+
// incorrectly prepends a thinking start token to the clean content,
103+
// causing the entire response to be classified as unclosed reasoning.
104+
if strings.Contains(in.Prompt, "AUTOPARSER_THINKING_CONTENT") {
105+
return &pb.Reply{
106+
Message: []byte("I am a helpful AI assistant designed to assist you with a wide range of tasks."),
107+
Tokens: 20,
108+
PromptTokens: 50,
109+
ChatDeltas: []*pb.ChatDelta{
110+
{
111+
ReasoningContent: "The user is asking a simple introductory question. I should respond directly.",
112+
Content: "I am a helpful AI assistant designed to assist you with a wide range of tasks.",
113+
},
114+
},
115+
}, nil
116+
}
117+
98118
var response string
99119
toolName := mockToolNameFromRequest(in)
100120
if toolName != "" && !promptHasToolResults(in.Prompt) {

tests/e2e/mock_backend_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,5 +462,51 @@ var _ = Describe("Mock Backend E2E Tests", Label("MockBackend"), func() {
462462
Expect(fn["name"]).To(Equal("search_collections"))
463463
})
464464
})
465+
466+
// Regression test: thinking model (Gemma 4-style) with tools, where the
467+
// model responds with content only (no tool calls). The C++ autoparser
468+
// puts clean content in Message AND reasoning+content in ChatDeltas.
469+
// Bug: Go-side PrependThinkingTokenIfNeeded prepends <|channel>thought
470+
// to the clean content, causing it to be classified as unclosed reasoning,
471+
// leading to "Backend produced reasoning without actionable content, retrying".
472+
Context("Non-streaming thinking model with tools and ChatDelta content (no tool calls)", func() {
473+
It("should return content without retrying", func() {
474+
body := `{
475+
"model": "mock-model-thinking-autoparser",
476+
"messages": [{"role": "user", "content": "AUTOPARSER_THINKING_CONTENT"}],
477+
"tools": [{"type": "function", "function": {"name": "search_collections", "description": "Search documents", "parameters": {"type": "object", "properties": {"query": {"type": "string"}}, "required": ["query"]}}}]
478+
}`
479+
req, err := http.NewRequest("POST", apiURL+"/chat/completions", strings.NewReader(body))
480+
Expect(err).ToNot(HaveOccurred())
481+
req.Header.Set("Content-Type", "application/json")
482+
483+
httpClient := &http.Client{Timeout: 60 * time.Second}
484+
resp, err := httpClient.Do(req)
485+
Expect(err).ToNot(HaveOccurred())
486+
defer resp.Body.Close()
487+
Expect(resp.StatusCode).To(Equal(200))
488+
489+
data, err := io.ReadAll(resp.Body)
490+
Expect(err).ToNot(HaveOccurred())
491+
492+
var result map[string]any
493+
Expect(json.Unmarshal(data, &result)).To(Succeed())
494+
495+
choices, ok := result["choices"].([]any)
496+
Expect(ok).To(BeTrue(), "Expected choices array, got: %s", string(data))
497+
Expect(choices).To(HaveLen(1))
498+
499+
choice := choices[0].(map[string]any)
500+
msg, _ := choice["message"].(map[string]any)
501+
Expect(msg).ToNot(BeNil())
502+
503+
content, _ := msg["content"].(string)
504+
Expect(content).ToNot(BeEmpty(),
505+
"Expected non-empty content in thinking model response with tools, "+
506+
"but got empty content. Full response: %s", string(data))
507+
Expect(content).To(ContainSubstring("helpful AI assistant"),
508+
"Expected content to contain the model's response text, got: %s", content)
509+
})
510+
})
465511
})
466512
})

0 commit comments

Comments
 (0)