Merge pull request #557 from MervinPraison/claude/issue-471-20250530_151237

fix(ollama): resolve tool-call issues with local Ollama models

Author: MervinPraison

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

@@ -864,7 +864,11 @@ def chat(self, prompt, temperature=0.2, tools=None, output_json=None, output_pyd
         if self._using_custom_llm:
             try:
                 # Special handling for MCP tools when using provider/model format
-                tool_param = self.tools if tools is None else tools
+                # Fix: Handle empty tools list properly - use self.tools if tools is None or empty
+                if tools is None or (isinstance(tools, list) and len(tools) == 0):
+                    tool_param = self.tools
+                else:
+                    tool_param = tools
 
                 # Convert MCP tool objects to OpenAI format if needed
                 if tool_param is not None:

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

@@ -205,6 +205,58 @@ def __init__(
             }
             logging.debug(f"LLM instance initialized with: {json.dumps(debug_info, indent=2, default=str)}")
 
+    def _is_ollama_provider(self) -> bool:
+        """Detect if this is an Ollama provider regardless of naming convention"""
+        if not self.model:
+            return False
+        
+        # Direct ollama/ prefix
+        if self.model.startswith("ollama/"):
+            return True
+            
+        # Check environment variables for Ollama base URL
+        base_url = os.getenv("OPENAI_BASE_URL", "")
+        api_base = os.getenv("OPENAI_API_BASE", "")
+        
+        # Common Ollama endpoints
+        ollama_endpoints = ["localhost:11434", "127.0.0.1:11434", ":11434"]
+        
+        return any(endpoint in base_url or endpoint in api_base for endpoint in ollama_endpoints)
+
+    def _parse_tool_call_arguments(self, tool_call: Dict, is_ollama: bool = False) -> tuple:
+        """
+        Safely parse tool call arguments with proper error handling
+        
+        Returns:
+            tuple: (function_name, arguments, tool_call_id)
+        """
+        try:
+            if is_ollama:
+                # Special handling for Ollama provider which may have different structure
+                if "function" in tool_call and isinstance(tool_call["function"], dict):
+                    function_name = tool_call["function"]["name"]
+                    arguments = json.loads(tool_call["function"]["arguments"])
+                else:
+                    # Try alternative format that Ollama might return
+                    function_name = tool_call.get("name", "unknown_function")
+                    arguments_str = tool_call.get("arguments", "{}")
+                    arguments = json.loads(arguments_str) if arguments_str else {}
+                tool_call_id = tool_call.get("id", f"tool_{id(tool_call)}")
+            else:
+                # Standard format for other providers with error handling
+                function_name = tool_call["function"]["name"]
+                arguments_str = tool_call["function"]["arguments"]
+                arguments = json.loads(arguments_str) if arguments_str else {}
+                tool_call_id = tool_call["id"]
+                
+        except (KeyError, json.JSONDecodeError, TypeError) as e:
+            logging.error(f"Error parsing tool call arguments: {e}")
+            function_name = tool_call.get("name", "unknown_function")
+            arguments = {}
+            tool_call_id = tool_call.get("id", f"tool_{id(tool_call)}")
+            
+        return function_name, arguments, tool_call_id
+
     def _needs_system_message_skip(self) -> bool:
         """Check if this model requires skipping system messages"""
         if not self.model:
@@ -486,32 +538,19 @@ def get_response(
                         for tool_call in tool_calls:
                             # Handle both object and dict access patterns
                             if isinstance(tool_call, dict):
-                                # Special handling for Ollama provider which may have a different structure
-                                if self.model and self.model.startswith("ollama/"):
-                                    try:
-                                        # Try standard format first
-                                        if "function" in tool_call and isinstance(tool_call["function"], dict):
-                                            function_name = tool_call["function"]["name"]
-                                            arguments = json.loads(tool_call["function"]["arguments"])
-                                        else:
-                                            # Try alternative format that Ollama might return
-                                            function_name = tool_call.get("name", "unknown_function")
-                                            arguments = json.loads(tool_call.get("arguments", "{}"))
-                                        tool_call_id = tool_call.get("id", f"tool_{id(tool_call)}")
-                                    except Exception as e:
-                                        logging.error(f"Error processing Ollama tool call: {e}")
-                                        function_name = "unknown_function"
-                                        arguments = {}
-                                        tool_call_id = f"tool_{id(tool_call)}"
-                                else:
-                                    # Standard format for other providers
-                                    function_name = tool_call["function"]["name"]
-                                    arguments = json.loads(tool_call["function"]["arguments"])
-                                    tool_call_id = tool_call["id"]
+                                is_ollama = self._is_ollama_provider()
+                                function_name, arguments, tool_call_id = self._parse_tool_call_arguments(tool_call, is_ollama)
                             else:
-                                function_name = tool_call.function.name
-                                arguments = json.loads(tool_call.function.arguments)
-                                tool_call_id = tool_call.id
+                                # Handle object-style tool calls
+                                try:
+                                    function_name = tool_call.function.name
+                                    arguments = json.loads(tool_call.function.arguments) if tool_call.function.arguments else {}
+                                    tool_call_id = tool_call.id
+                                except (json.JSONDecodeError, AttributeError) as e:
+                                    logging.error(f"Error parsing object-style tool call: {e}")
+                                    function_name = "unknown_function"
+                                    arguments = {}
+                                    tool_call_id = f"tool_{id(tool_call)}"
 
                             logging.debug(f"[TOOL_EXEC_DEBUG] About to execute tool {function_name} with args: {arguments}")
                             tool_result = execute_tool_fn(function_name, arguments)
@@ -1083,32 +1122,19 @@ async def get_response_async(
                     for tool_call in tool_calls:
                         # Handle both object and dict access patterns
                         if isinstance(tool_call, dict):
-                            # Special handling for Ollama provider which may have a different structure
-                            if self.model and self.model.startswith("ollama/"):
-                                try:
-                                    # Try standard format first
-                                    if "function" in tool_call and isinstance(tool_call["function"], dict):
-                                        function_name = tool_call["function"]["name"]
-                                        arguments = json.loads(tool_call["function"]["arguments"])
-                                    else:
-                                        # Try alternative format that Ollama might return
-                                        function_name = tool_call.get("name", "unknown_function")
-                                        arguments = json.loads(tool_call.get("arguments", "{}"))
-                                    tool_call_id = tool_call.get("id", f"tool_{id(tool_call)}")
-                                except Exception as e:
-                                    logging.error(f"Error processing Ollama tool call: {e}")
-                                    function_name = "unknown_function"
-                                    arguments = {}
-                                    tool_call_id = f"tool_{id(tool_call)}"
-                            else:
-                                # Standard format for other providers
-                                function_name = tool_call["function"]["name"]
-                                arguments = json.loads(tool_call["function"]["arguments"])
-                                tool_call_id = tool_call["id"]
+                            is_ollama = self._is_ollama_provider()
+                            function_name, arguments, tool_call_id = self._parse_tool_call_arguments(tool_call, is_ollama)
                         else:
-                            function_name = tool_call.function.name
-                            arguments = json.loads(tool_call.function.arguments)
-                            tool_call_id = tool_call.id
+                            # Handle object-style tool calls
+                            try:
+                                function_name = tool_call.function.name
+                                arguments = json.loads(tool_call.function.arguments) if tool_call.function.arguments else {}
+                                tool_call_id = tool_call.id
+                            except (json.JSONDecodeError, AttributeError) as e:
+                                logging.error(f"Error parsing object-style tool call: {e}")
+                                function_name = "unknown_function"
+                                arguments = {}
+                                tool_call_id = f"tool_{id(tool_call)}"
 
                         tool_result = await execute_tool_fn(function_name, arguments)
 
@@ -1129,7 +1155,7 @@ async def get_response_async(
                     response_text = ""
 
                     # Special handling for Ollama models that don't automatically process tool results
-                    if self.model and self.model.startswith("ollama/") and tool_result:
+                    if self._is_ollama_provider() and tool_result:
                         # For Ollama models, we need to explicitly ask the model to process the tool results
                         # First, check if the response is just a JSON tool call
                         try:

src/praisonai/tests/unit/test_ollama_fix.py

@@ -0,0 +1,132 @@
+#!/usr/bin/env python3
+"""
+Test script to verify Ollama tool-call fixes
+"""
+import sys
+import os
+
+# Add the source directory to Python path
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..', '..', 'src', 'praisonai-agents'))
+
+from praisonaiagents.llm.llm import LLM
+from praisonaiagents.agent.agent import Agent
+
+def test_ollama_provider_detection():
+    """Test the new Ollama provider detection logic"""
+    print("Testing Ollama provider detection...")
+    
+    # Test 1: Direct ollama/ prefix
+    llm1 = LLM(model="ollama/qwen3:32b")
+    assert llm1._is_ollama_provider() == True, "Should detect ollama/ prefix"
+    print("✓ Direct ollama/ prefix detection works")
+    
+    # Test 2: Environment variable detection
+    os.environ["OPENAI_BASE_URL"] = "http://localhost:11434/v1"
+    llm2 = LLM(model="qwen3:32b")
+    assert llm2._is_ollama_provider() == True, "Should detect via env var"
+    print("✓ Environment variable detection works")
+    
+    # Test 3: Non-Ollama model
+    os.environ.pop("OPENAI_BASE_URL", None)
+    llm3 = LLM(model="gpt-4o-mini")
+    assert llm3._is_ollama_provider() == False, "Should not detect non-Ollama"
+    print("✓ Non-Ollama detection works")
+
+def test_tool_call_parsing():
+    """Test the new tool call parsing logic"""
+    print("\nTesting tool call parsing...")
+    
+    llm = LLM(model="ollama/qwen3:32b")
+    
+    # Test 1: Standard format
+    tool_call_std = {
+        "id": "call_123",
+        "function": {
+            "name": "hello_world",
+            "arguments": '{"name": "test"}'
+        }
+    }
+    name, args, id = llm._parse_tool_call_arguments(tool_call_std, is_ollama=True)
+    assert name == "hello_world", f"Expected 'hello_world', got '{name}'"
+    assert args == {"name": "test"}, f"Expected {{'name': 'test'}}, got {args}"
+    print("✓ Standard format parsing works")
+    
+    # Test 2: Ollama alternative format
+    tool_call_alt = {
+        "name": "hello_world",
+        "arguments": '{"name": "test"}'
+    }
+    name, args, id = llm._parse_tool_call_arguments(tool_call_alt, is_ollama=True)
+    assert name == "hello_world", f"Expected 'hello_world', got '{name}'"
+    assert args == {"name": "test"}, f"Expected {{'name': 'test'}}, got {args}"
+    print("✓ Alternative format parsing works")
+    
+    # Test 3: Error handling - malformed JSON
+    tool_call_bad = {
+        "function": {
+            "name": "hello_world",
+            "arguments": 'invalid json'
+        }
+    }
+    name, args, id = llm._parse_tool_call_arguments(tool_call_bad, is_ollama=False)
+    assert name == "hello_world", f"Expected 'hello_world', got '{name}'"
+    assert args == {}, f"Expected empty dict, got {args}"
+    print("✓ Error handling works")
+
+def test_agent_tool_parameter_logic():
+    """Test the fixed tool parameter logic in Agent"""
+    print("\nTesting agent tool parameter logic...")
+    
+    def dummy_tool():
+        """Dummy tool for testing"""
+        return "test"
+    
+    # Test 1: None tools should use agent tools
+    agent = Agent(name="test", tools=[dummy_tool])
+    # Simulate the fixed logic
+    tools = None
+    if tools is None or (isinstance(tools, list) and len(tools) == 0):
+        tool_param = agent.tools
+    else:
+        tool_param = tools
+    
+    assert tool_param == [dummy_tool], "Should use agent tools when tools=None"
+    print("✓ None tools handling works")
+    
+    # Test 2: Empty list should use agent tools  
+    tools = []
+    if tools is None or (isinstance(tools, list) and len(tools) == 0):
+        tool_param = agent.tools
+    else:
+        tool_param = tools
+        
+    assert tool_param == [dummy_tool], "Should use agent tools when tools=[]"
+    print("✓ Empty list handling works")
+    
+    # Test 3: Non-empty list should use provided tools
+    custom_tools = [lambda: "custom"]
+    tools = custom_tools
+    if tools is None or (isinstance(tools, list) and len(tools) == 0):
+        tool_param = agent.tools
+    else:
+        tool_param = tools
+        
+    assert tool_param == custom_tools, "Should use provided tools when not empty"
+    print("✓ Non-empty list handling works")
+
+if __name__ == "__main__":
+    print("Running Ollama tool-call fix tests...\n")
+    
+    try:
+        test_ollama_provider_detection()
+        test_tool_call_parsing()
+        test_agent_tool_parameter_logic()
+        
+        print("\n🎉 All tests passed! The Ollama tool-call fixes are working correctly.")
+        sys.exit(0)
+        
+    except Exception as e:
+        print(f"\n❌ Test failed: {e}")
+        import traceback
+        traceback.print_exc()
+        sys.exit(1)