refactor: eliminate code duplication and improve test quality in Ollama fix

- Extract duplicated Ollama tool summary logic into _generate_ollama_tool_summary() helper method
- Replace magic number 10 with OLLAMA_MIN_RESPONSE_LENGTH constant
- Fix test_tool_summary_generation() to call production code instead of reimplementing logic
- Add comprehensive test scenarios for different response types and provider types
- Maintain backward compatibility and fix functionality

Co-authored-by: Mervin Praison <[email protected]>

Author: github-actions[bot]

src/praisonai-agents/praisonaiagents/llm/llm.py

@@ -302,6 +302,42 @@ def _is_ollama_provider(self) -> bool:
 
         return False
 
+    def _generate_ollama_tool_summary(self, tool_results: List[Any], response_text: str) -> Optional[str]:
+        """
+        Generate a summary from tool results for Ollama to prevent infinite loops.
+        
+        This prevents infinite loops where Ollama provides an empty response after a
+        tool call, expecting the user to prompt for a summary.
+
+        Args:
+            tool_results: The list of results from tool execution.
+            response_text: The text response from the LLM.
+
+        Returns:
+            A summary string if conditions are met, otherwise None.
+        """
+        # Constant for minimal response length check
+        OLLAMA_MIN_RESPONSE_LENGTH = 10
+        
+        # Only generate summary for Ollama with tool results
+        if not (self._is_ollama_provider() and tool_results):
+            return None
+
+        # If response is substantial, no summary needed
+        if response_text and len(response_text.strip()) > OLLAMA_MIN_RESPONSE_LENGTH:
+            return None
+            
+        # Build tool summary efficiently
+        summary_lines = ["Based on the tool execution results:"]
+        for i, result in enumerate(tool_results):
+            if isinstance(result, dict) and 'result' in result:
+                function_name = result.get('function_name', 'Tool')
+                summary_lines.append(f"- {function_name}: {result['result']}")
+            else:
+                summary_lines.append(f"- Tool {i+1}: {result}")
+        
+        return "\n".join(summary_lines)
+
     def _format_ollama_tool_result_message(self, function_name: str, tool_result: Any) -> Dict[str, str]:
         """
         Format tool result message for Ollama provider.
@@ -1072,21 +1108,11 @@ def get_response(
                             final_response_text = response_text.strip()
                             break
 
-                        # Special handling for Ollama: if we have tool results but empty/minimal response,
-                        # generate a summary based on tool results to prevent infinite loops
-                        if self._is_ollama_provider() and tool_results and len(tool_results) > 0:
-                            # Create a summary of tool results for Ollama
-                            tool_summary = "Based on the tool execution results:\n"
-                            for i, result in enumerate(tool_results):
-                                if isinstance(result, dict) and 'result' in result:
-                                    tool_summary += f"- {result.get('function_name', 'Tool')}: {result['result']}\n"
-                                else:
-                                    tool_summary += f"- Tool {i+1}: {result}\n"
-                            
-                            # If response is empty or minimal, use tool summary as final answer
-                            if not response_text or len(response_text.strip()) <= 10:
-                                final_response_text = tool_summary.strip()
-                                break
+                        # Special handling for Ollama to prevent infinite loops
+                        tool_summary = self._generate_ollama_tool_summary(tool_results, response_text)
+                        if tool_summary:
+                            final_response_text = tool_summary
+                            break
 
                         # Otherwise, continue the loop to check if more tools are needed
                         iteration_count += 1
@@ -1831,21 +1857,11 @@ async def get_response_async(
                         final_response_text = response_text.strip()
                         break
 
-                    # Special handling for Ollama: if we have tool results but empty/minimal response,
-                    # generate a summary based on tool results to prevent infinite loops
-                    if self._is_ollama_provider() and tool_results and len(tool_results) > 0:
-                        # Create a summary of tool results for Ollama
-                        tool_summary = "Based on the tool execution results:\n"
-                        for i, result in enumerate(tool_results):
-                            if isinstance(result, dict) and 'result' in result:
-                                tool_summary += f"- {result.get('function_name', 'Tool')}: {result['result']}\n"
-                            else:
-                                tool_summary += f"- Tool {i+1}: {result}\n"
-                        
-                        # If response is empty or minimal, use tool summary as final answer
-                        if not response_text or len(response_text.strip()) <= 10:
-                            final_response_text = tool_summary.strip()
-                            break
+                    # Special handling for Ollama to prevent infinite loops
+                    tool_summary = self._generate_ollama_tool_summary(tool_results, response_text)
+                    if tool_summary:
+                        final_response_text = tool_summary
+                        break
 
                     # Continue the loop to check if more tools are needed
                     iteration_count += 1

test_ollama_fix.py

@@ -57,34 +57,58 @@ def test_ollama_provider_detection():
         return False
 
 def test_tool_summary_generation():
-    """Test that tool results summary generation works correctly."""
+    """Test that tool results summary generation works correctly by calling production code."""
     try:
+        from praisonaiagents.llm.llm import LLM
+        
+        # Create an Ollama LLM instance
+        ollama_llm = LLM(model="ollama/test")
+        
         # Mock tool results like what would be generated
         tool_results = [
-            "The stock price of Google is 100",
+            "The stock price of Google is 100", 
             200
         ]
 
-        # Simulate the summary generation logic
-        tool_summary = "Based on the tool execution results:\n"
-        for i, result in enumerate(tool_results):
-            if isinstance(result, dict) and 'result' in result:
-                tool_summary += f"- {result.get('function_name', 'Tool')}: {result['result']}\n"
-            else:
-                tool_summary += f"- Tool {i+1}: {result}\n"
-        
+        # Test with empty response (should generate summary)
+        summary = ollama_llm._generate_ollama_tool_summary(tool_results, "")
         expected_summary = "Based on the tool execution results:\n- Tool 1: The stock price of Google is 100\n- Tool 2: 200"
 
-        if tool_summary.strip() == expected_summary:
-            print("✅ Tool summary generation works correctly")
-            print(f"Generated summary: {repr(tool_summary.strip())}")
-            return True
+        if summary == expected_summary:
+            print("✅ Tool summary generation (empty response) works correctly")
         else:
-            print("❌ Tool summary generation failed")
+            print("❌ Tool summary generation (empty response) failed")
             print(f"Expected: {repr(expected_summary)}")
-            print(f"Got: {repr(tool_summary.strip())}")
+            print(f"Got: {repr(summary)}")
+            return False
+        
+        # Test with minimal response (should generate summary)
+        summary_minimal = ollama_llm._generate_ollama_tool_summary(tool_results, "ok")
+        if summary_minimal == expected_summary:
+            print("✅ Tool summary generation (minimal response) works correctly")
+        else:
+            print("❌ Tool summary generation (minimal response) failed")
+            return False
+        
+        # Test with substantial response (should NOT generate summary)
+        summary_substantial = ollama_llm._generate_ollama_tool_summary(tool_results, "This is a detailed response with more than 10 characters")
+        if summary_substantial is None:
+            print("✅ Tool summary generation correctly skips substantial responses")
+        else:
+            print("❌ Tool summary generation incorrectly generated summary for substantial response")
+            return False
+        
+        # Test with non-Ollama model (should NOT generate summary)
+        non_ollama_llm = LLM(model="gpt-4o-mini")
+        summary_non_ollama = non_ollama_llm._generate_ollama_tool_summary(tool_results, "")
+        if summary_non_ollama is None:
+            print("✅ Tool summary generation correctly skips non-Ollama models")
+        else:
+            print("❌ Tool summary generation incorrectly generated summary for non-Ollama model")
             return False
 
+        return True
+        
     except Exception as e:
         print(f"❌ Tool summary generation test failed: {e}")
         return False