From 26d274ac79f44997cf57e5941f53b228d66386aa Mon Sep 17 00:00:00 2001 From: Henry Park Date: Fri, 6 Mar 2026 00:29:30 -0800 Subject: [PATCH] fix(llm): fix reasoning model response parsing bugs (#564) (#580) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three related fixes for reasoning model artifacts (GLM-4/5, DeepSeek R1, Qwen3): 1. reasoning_content no longer leaks into tool-call assistant messages in nearai_chat — only used as fallback for final text responses. 2. plan() and evaluate_success() now apply clean_response() before JSON parsing, preventing tag prefixes from breaking plan/eval. 3. Unclosed before no longer discards the answer — the strict discard path now extracts content first. 8 regression tests added. Co-authored-by: Claude Opus 4.6 (1M context) Co-authored-by: Illia Polosukhin --- src/llm/nearai_chat.rs | 120 +++++++++++++++++++++++++++++++++++++++-- src/llm/reasoning.rs | 69 ++++++++++++++++++++++-- 2 files changed, 183 insertions(+), 6 deletions(-) diff --git a/src/llm/nearai_chat.rs b/src/llm/nearai_chat.rs index 50895ecd..626c4d5c 100644 --- a/src/llm/nearai_chat.rs +++ b/src/llm/nearai_chat.rs @@ -522,9 +522,6 @@ impl LlmProvider for NearAiChatProvider { reason: "No choices in response".to_string(), })?; - // Fall back to reasoning_content when content is null (e.g. GLM-5 - // returns its answer in reasoning_content instead of content). - let content = choice.message.content.or(choice.message.reasoning_content); let tool_calls: Vec = choice .message .tool_calls @@ -541,6 +538,18 @@ impl LlmProvider for NearAiChatProvider { }) .collect(); + // Fall back to reasoning_content when content is null (e.g. GLM-5 + // returns its answer in reasoning_content instead of content), but + // only for final text responses. Tool-call responses often have + // content: null + reasoning_content filled with chain-of-thought; + // leaking that into conversation history inflates context and + // confuses the model. + let content = if tool_calls.is_empty() { + choice.message.content.or(choice.message.reasoning_content) + } else { + choice.message.content + }; + let finish_reason = match choice.finish_reason.as_deref() { Some("stop") => FinishReason::Stop, Some("length") => FinishReason::Length, @@ -1285,4 +1294,109 @@ mod tests { assert_eq!(input, default_in); assert_eq!(output, default_out); } + + /// Regression: reasoning_content must NOT leak into tool-call responses. + #[test] + fn test_reasoning_content_not_leaked_into_tool_call_response() { + let response: ChatCompletionResponse = serde_json::from_value(serde_json::json!({ + "id": "chatcmpl-test", + "choices": [{ + "message": { + "role": "assistant", + "content": null, + "reasoning_content": "Let me think about which tool to call...", + "tool_calls": [{ + "id": "call_abc123", + "type": "function", + "function": { + "name": "search", + "arguments": "{\"query\":\"test\"}" + } + }] + }, + "finish_reason": "tool_calls" + }], + "usage": { "prompt_tokens": 100, "completion_tokens": 50 } + })) + .unwrap(); + + let choice = response.choices.into_iter().next().unwrap(); + let tool_calls: Vec = choice + .message + .tool_calls + .unwrap_or_default() + .into_iter() + .map(|tc| { + let arguments = serde_json::from_str(&tc.function.arguments) + .unwrap_or(serde_json::Value::Object(Default::default())); + ToolCall { + id: tc.id, + name: tc.function.name, + arguments, + } + }) + .collect(); + + let content = if tool_calls.is_empty() { + choice.message.content.or(choice.message.reasoning_content) + } else { + choice.message.content + }; + + assert!( + content.is_none(), + "reasoning_content should NOT leak into tool-call responses, got: {:?}", + content + ); + assert_eq!(tool_calls.len(), 1); + assert_eq!(tool_calls[0].name, "search"); + } + + /// Regression: reasoning_content SHOULD be used as fallback for text responses. + #[test] + fn test_reasoning_content_used_for_text_response() { + let response: ChatCompletionResponse = serde_json::from_value(serde_json::json!({ + "id": "chatcmpl-test", + "choices": [{ + "message": { + "role": "assistant", + "content": null, + "reasoning_content": "The answer is 42." + }, + "finish_reason": "stop" + }], + "usage": { "prompt_tokens": 50, "completion_tokens": 20 } + })) + .unwrap(); + + let choice = response.choices.into_iter().next().unwrap(); + let tool_calls: Vec = choice + .message + .tool_calls + .unwrap_or_default() + .into_iter() + .map(|tc| { + let arguments = serde_json::from_str(&tc.function.arguments) + .unwrap_or(serde_json::Value::Object(Default::default())); + ToolCall { + id: tc.id, + name: tc.function.name, + arguments, + } + }) + .collect(); + + let content = if tool_calls.is_empty() { + choice.message.content.or(choice.message.reasoning_content) + } else { + choice.message.content + }; + + assert_eq!( + content, + Some("The answer is 42.".to_string()), + "reasoning_content should be used as fallback for text responses" + ); + assert!(tool_calls.is_empty()); + } } diff --git a/src/llm/reasoning.rs b/src/llm/reasoning.rs index faf9047d..0afa10d9 100644 --- a/src/llm/reasoning.rs +++ b/src/llm/reasoning.rs @@ -335,8 +335,9 @@ impl Reasoning { let response = self.llm.complete(request).await?; - // Parse the plan from the response - self.parse_plan(&response.content) + // Clean reasoning model artifacts before parsing JSON + let cleaned = clean_response(&response.content); + self.parse_plan(&cleaned) } /// Select the best tool for the current situation. @@ -429,7 +430,9 @@ Respond in JSON format: let response = self.llm.complete(request).await?; - self.parse_evaluation(&response.content) + // Clean reasoning model artifacts before parsing JSON + let cleaned = clean_response(&response.content); + self.parse_evaluation(&cleaned) } /// Generate a response to a user message. @@ -1292,8 +1295,15 @@ fn strip_thinking_tags_regex(text: &str, code_regions: &[CodeRegion]) -> String } // Strict mode: if still inside an unclosed thinking tag, discard trailing text + // BUT preserve any block embedded in the discarded region if !in_thinking { result.push_str(&text[last_index..]); + } else { + let trailing = &text[last_index..]; + let trailing_regions = find_code_regions(trailing); + if let Some(final_content) = extract_final_content(trailing, &trailing_regions) { + result.push_str(&final_content); + } } result @@ -1918,6 +1928,59 @@ That's my plan."#; assert_eq!(calls[0].name, "tool_list"); } + // ---- plan/evaluate bypass clean_response (Bug #564-2) ---- + + #[test] + fn test_clean_response_strips_think_before_json_plan() { + let raw = r#"I need to plan the steps carefully...{"steps": [{"description": "Step 1", "tool": "search", "expected_outcome": "results"}], "reasoning": "Simple plan"}"#; + let cleaned = clean_response(raw); + // After cleaning, the JSON should be parseable + let json_str = extract_json(&cleaned).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(json_str).unwrap(); + assert!(parsed.get("steps").is_some()); + } + + #[test] + fn test_clean_response_strips_think_before_json_evaluation() { + let raw = r#"Let me evaluate whether this was successful...{"success": true, "confidence": 0.95, "reasoning": "Task completed", "issues": [], "suggestions": []}"#; + let cleaned = clean_response(raw); + let json_str = extract_json(&cleaned).unwrap(); + let eval: SuccessEvaluation = serde_json::from_str(json_str).unwrap(); + assert!(eval.success); + assert_eq!(eval.confidence, 0.95); + } + + // ---- Unclosed think before final (Bug #564-3) ---- + + #[test] + fn test_unclosed_think_before_final() { + assert_eq!( + clean_response("reasoning no close tag actual answer"), + "actual answer" + ); + } + + #[test] + fn test_unclosed_thinking_before_final() { + assert_eq!( + clean_response("long reasoning... the real answer"), + "the real answer" + ); + } + + #[test] + fn test_unclosed_think_before_final_with_prefix() { + assert_eq!( + clean_response("Hello reasoning world"), + "Hello world" + ); + } + + #[test] + fn test_unclosed_think_no_final_still_discards() { + assert_eq!(clean_response("Hello this never closes"), "Hello"); + } + #[test] fn test_recover_bracket_format_tool_call() { let tools = make_tools(&["http"]);