From 19dcaad6cfdf30880bd9bf503e4b09a22fac3aa1 Mon Sep 17 00:00:00 2001
From: Henry Park <henrypark133@gmail.com>
Date: Fri, 27 Mar 2026 16:46:26 -0700
Subject: [PATCH] Address malformed tool recovery review comments

---
 src/llm/reasoning.rs              | 68 +++++++++++++++++++++++++++++--
 src/worker/autonomous_recovery.rs |  7 ++--
 2 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/src/llm/reasoning.rs b/src/llm/reasoning.rs
index 5f8a45a2..a083e0db 100644
--- a/src/llm/reasoning.rs
+++ b/src/llm/reasoning.rs
@@ -766,9 +766,7 @@ Respond in JSON format:
                 });
             }
 
-            let content = response
-                .content
-                .unwrap_or_else(|| "I'm not sure how to respond to that.".to_string());
+            let content = response.content.unwrap_or_default();
 
             // Some models (e.g. GLM-4.7) emit tool calls as XML tags in content
             // instead of using the structured tool_calls field. Try to recover
@@ -3176,6 +3174,70 @@ That's my plan."#;
         }
     }
 
+    #[tokio::test]
+    async fn test_respond_with_tools_flags_empty_tool_completion_when_content_is_none() {
+        use crate::llm::{FinishReason, LlmProvider, ToolCompletionRequest, ToolCompletionResponse};
+        use async_trait::async_trait;
+        use rust_decimal::Decimal;
+
+        struct NoneContentToolLlm;
+
+        #[async_trait]
+        impl LlmProvider for NoneContentToolLlm {
+            fn model_name(&self) -> &str {
+                "none-content-tool-llm"
+            }
+
+            fn cost_per_token(&self) -> (Decimal, Decimal) {
+                (Decimal::ZERO, Decimal::ZERO)
+            }
+
+            async fn complete(
+                &self,
+                _request: crate::llm::CompletionRequest,
+            ) -> Result<crate::llm::CompletionResponse, crate::llm::LlmError> {
+                unreachable!("tool-mode test should not call complete()")
+            }
+
+            async fn complete_with_tools(
+                &self,
+                _request: ToolCompletionRequest,
+            ) -> Result<ToolCompletionResponse, crate::llm::LlmError> {
+                Ok(ToolCompletionResponse {
+                    content: None,
+                    tool_calls: Vec::new(),
+                    input_tokens: 0,
+                    output_tokens: 0,
+                    finish_reason: FinishReason::Stop,
+                    cache_read_input_tokens: 0,
+                    cache_creation_input_tokens: 0,
+                })
+            }
+        }
+
+        let reasoning = Reasoning::new(Arc::new(NoneContentToolLlm));
+
+        let context = ReasoningContext::new()
+            .with_message(ChatMessage::user("list tools"))
+            .with_tools(vec![ToolDefinition {
+                name: "tool_list".to_string(),
+                description: "Lists tools".to_string(),
+                parameters: serde_json::json!({}),
+            }]);
+
+        let output = reasoning.respond_with_tools(&context).await.unwrap();
+        let metadata = output.metadata;
+        match output.result {
+            RespondResult::Text(text) => {
+                assert_eq!(text, "I'm not sure how to respond to that.");
+                assert_eq!(metadata.anomaly, Some(ResponseAnomaly::EmptyToolCompletion));
+            }
+            RespondResult::ToolCalls { .. } => {
+                panic!("Expected fallback text, not tool calls");
+            }
+        }
+    }
+
     #[tokio::test]
     async fn test_plan_truncates_tool_tags_before_json() {
         use crate::testing::StubLlm;
diff --git a/src/worker/autonomous_recovery.rs b/src/worker/autonomous_recovery.rs
index 2db2f0c7..4b4b4cc1 100644
--- a/src/worker/autonomous_recovery.rs
+++ b/src/worker/autonomous_recovery.rs
@@ -11,11 +11,11 @@ Do not call any more tools in the next reply.\n\
 Instead, provide a concise final status based only on work already completed.\n\
 If the job is complete, say so explicitly. If not, explain what blocked you.";
 
-pub(crate) const EMPTY_TOOL_COMPLETION_FAILURE: &str = "Execution failed: the selected model repeatedly returned empty or malformed tool-completion responses and is not reliable for autonomous tool use.";
+pub(crate) const EMPTY_TOOL_COMPLETION_FAILURE: &str = "the selected model repeatedly returned empty or malformed tool-completion responses and is not reliable for autonomous tool use.";
 
 #[derive(Debug, Default, Clone, Copy)]
 pub(crate) struct AutonomousRecoveryState {
-    consecutive_empty_tool_completions: u8,
+    consecutive_empty_tool_completions: usize,
     force_text_recovery_pending: bool,
     force_text_recovery_active: bool,
 }
@@ -46,7 +46,8 @@ impl AutonomousRecoveryState {
     ) -> AutonomousRecoveryAction {
         match metadata.anomaly {
             Some(ResponseAnomaly::EmptyToolCompletion) => {
-                self.consecutive_empty_tool_completions += 1;
+                self.consecutive_empty_tool_completions =
+                    self.consecutive_empty_tool_completions.saturating_add(1);
                 self.force_text_recovery_active = false;
                 match self.consecutive_empty_tool_completions {
                     1 => AutonomousRecoveryAction::ToolModeNudge,