Merge branch 'main' of https://github.com/MervinPraison/PraisonAI

MervinPraison · MervinPraison · commit faaff9f27d18 · 2025-07-15T15:04:19.000+01:00
diff --git a/src/praisonai-agents/praisonaiagents/llm/llm.py b/src/praisonai-agents/praisonaiagents/llm/llm.py
@@ -2,6 +2,7 @@
 import os
 import warnings
 import re
+import inspect
 from typing import Any, Dict, List, Optional, Union, Literal, Callable
 from pydantic import BaseModel
 import time
@@ -16,7 +17,6 @@
     ReflectionOutput,
     execute_sync_callback,
 )
-from .model_capabilities import is_gemini_internal_tool
 from rich.console import Console
 from rich.live import Live
 
@@ -380,6 +380,65 @@ def _parse_tool_call_arguments(self, tool_call: Dict, is_ollama: bool = False) -
             
         return function_name, arguments, tool_call_id
 
+    def _validate_and_filter_ollama_arguments(self, function_name: str, arguments: Dict[str, Any], available_tools: List) -> Dict[str, Any]:
+        """
+        Validate and filter tool call arguments for Ollama provider.
+        
+        Ollama sometimes generates tool calls with mixed parameters where arguments
+        from different functions are combined. This method validates arguments against
+        the actual function signature and removes invalid parameters.
+        
+        Args:
+            function_name: Name of the function to call
+            arguments: Arguments provided in the tool call
+            available_tools: List of available tool functions
+            
+        Returns:
+            Filtered arguments dictionary with only valid parameters
+        """
+        if not available_tools:
+            logging.debug(f"[OLLAMA_FIX] No available tools provided for validation")
+            return arguments
+            
+        # Find the target function
+        target_function = None
+        for tool in available_tools:
+            tool_name = getattr(tool, '__name__', str(tool))
+            if tool_name == function_name:
+                target_function = tool
+                break
+                
+        if not target_function:
+            logging.debug(f"[OLLAMA_FIX] Function {function_name} not found in available tools")
+            return arguments
+            
+        try:
+            # Get function signature
+            sig = inspect.signature(target_function)
+            valid_params = set(sig.parameters.keys())
+            
+            # Filter arguments to only include valid parameters
+            filtered_args = {}
+            invalid_params = []
+            
+            for param_name, param_value in arguments.items():
+                if param_name in valid_params:
+                    filtered_args[param_name] = param_value
+                else:
+                    invalid_params.append(param_name)
+                    
+            if invalid_params:
+                logging.debug(f"[OLLAMA_FIX] Function {function_name} received invalid parameters: {invalid_params}")
+                logging.debug(f"[OLLAMA_FIX] Valid parameters for {function_name}: {list(valid_params)}")
+                logging.debug(f"[OLLAMA_FIX] Original arguments: {arguments}")
+                logging.debug(f"[OLLAMA_FIX] Filtered arguments: {filtered_args}")
+                
+            return filtered_args
+            
+        except Exception as e:
+            logging.debug(f"[OLLAMA_FIX] Error validating arguments for {function_name}: {e}")
+            return arguments
+
     def _needs_system_message_skip(self) -> bool:
         """Check if this model requires skipping system messages"""
         if not self.model:
@@ -591,10 +650,14 @@ def _format_tools_for_litellm(self, tools: Optional[List[Any]]) -> Optional[List
                 if tool_def:
                     formatted_tools.append(tool_def)
             # Handle Gemini internal tools (e.g., {"googleSearch": {}}, {"urlContext": {}}, {"codeExecution": {}})
-            elif is_gemini_internal_tool(tool):
+            elif isinstance(tool, dict) and len(tool) == 1:
                 tool_name = next(iter(tool.keys()))
-                logging.debug(f"Using Gemini internal tool: {tool_name}")
-                formatted_tools.append(tool)
+                gemini_internal_tools = {'googleSearch', 'urlContext', 'codeExecution'}
+                if tool_name in gemini_internal_tools:
+                    logging.debug(f"Using Gemini internal tool: {tool_name}")
+                    formatted_tools.append(tool)
+                else:
+                    logging.debug(f"Skipping unknown tool: {tool_name}")
             else:
                 logging.debug(f"Skipping tool of unsupported type: {type(tool)}")
                 
@@ -959,6 +1022,10 @@ def get_response(
                             is_ollama = self._is_ollama_provider()
                             function_name, arguments, tool_call_id = self._extract_tool_call_info(tool_call, is_ollama)
 
+                            # Validate and filter arguments for Ollama provider
+                            if is_ollama and tools:
+                                arguments = self._validate_and_filter_ollama_arguments(function_name, arguments, tools)
+
                             logging.debug(f"[TOOL_EXEC_DEBUG] About to execute tool {function_name} with args: {arguments}")
                             tool_result = execute_tool_fn(function_name, arguments)
                             logging.debug(f"[TOOL_EXEC_DEBUG] Tool execution result: {tool_result}")
@@ -1610,6 +1677,10 @@ async def get_response_async(
                         is_ollama = self._is_ollama_provider()
                         function_name, arguments, tool_call_id = self._extract_tool_call_info(tool_call, is_ollama)
 
+                        # Validate and filter arguments for Ollama provider
+                        if is_ollama and tools:
+                            arguments = self._validate_and_filter_ollama_arguments(function_name, arguments, tools)
+
                         tool_result = await execute_tool_fn(function_name, arguments)
                         tool_results.append(tool_result)  # Store the result
 
diff --git a/src/praisonai-agents/praisonaiagents/llm/openai_client.py b/src/praisonai-agents/praisonaiagents/llm/openai_client.py
@@ -18,7 +18,6 @@
 from rich.console import Console
 from rich.live import Live
 import inspect
-from .model_capabilities import is_gemini_internal_tool
 
 # Constants
 LOCAL_SERVER_API_KEY_PLACEHOLDER = "not-needed"
@@ -407,10 +406,14 @@ def format_tools(self, tools: Optional[List[Any]]) -> Optional[List[Dict]]:
                 if tool_def:
                     formatted_tools.append(tool_def)
             # Handle Gemini internal tools (e.g., {"googleSearch": {}}, {"urlContext": {}}, {"codeExecution": {}})
-            elif is_gemini_internal_tool(tool):
+            elif isinstance(tool, dict) and len(tool) == 1:
                 tool_name = next(iter(tool.keys()))
-                logging.debug(f"Using Gemini internal tool: {tool_name}")
-                formatted_tools.append(tool)
+                gemini_internal_tools = {'googleSearch', 'urlContext', 'codeExecution'}
+                if tool_name in gemini_internal_tools:
+                    logging.debug(f"Using Gemini internal tool: {tool_name}")
+                    formatted_tools.append(tool)
+                else:
+                    logging.debug(f"Skipping unknown tool: {tool_name}")
             else:
                 logging.debug(f"Skipping tool of unsupported type: {type(tool)}")
                 
diff --git a/src/praisonai-agents/tests/test_ollama_sequential_fix.py b/src/praisonai-agents/tests/test_ollama_sequential_fix.py
@@ -0,0 +1,129 @@
+#!/usr/bin/env python3
+"""
+Test script to verify Ollama sequential tool calling argument mixing fix.
+
+This test validates that the parameter validation and filtering fix correctly handles
+the case where Ollama generates tool calls with mixed parameters from different functions.
+"""
+
+import logging
+from praisonaiagents.llm.llm import LLM
+
+# Enable debug logging
+logging.basicConfig(level=logging.DEBUG)
+
+# Test functions matching the issue description
+def get_stock_price(company_name: str) -> str:
+    """
+    Get the stock price of a company
+    
+    Args:
+        company_name (str): The name of the company
+        
+    Returns:
+        str: The stock price of the company
+    """
+    return f"The stock price of {company_name} is 100"
+
+def multiply(a: int, b: int) -> int:
+    """
+    Multiply two numbers
+    """
+    return a * b
+
+def test_ollama_argument_validation():
+    """
+    Test the Ollama argument validation and filtering functionality.
+    """
+    print("Testing Ollama argument validation and filtering...")
+    
+    llm = LLM(model="ollama/llama3.2")
+    tools = [get_stock_price, multiply]
+    
+    # Test case 1: Valid arguments (should pass through unchanged)
+    print("\n1. Testing valid arguments:")
+    valid_args = {"a": 100, "b": 2}
+    filtered_args = llm._validate_and_filter_ollama_arguments("multiply", valid_args, tools)
+    print(f"Original: {valid_args}")
+    print(f"Filtered: {filtered_args}")
+    assert filtered_args == valid_args, "Valid arguments should pass through unchanged"
+    print("✅ Valid arguments test passed")
+    
+    # Test case 2: Mixed arguments (the actual issue from #918)
+    print("\n2. Testing mixed arguments (the main issue):")
+    mixed_args = {"a": "get_stock_price", "company_name": "Google", "b": "2"}
+    filtered_args = llm._validate_and_filter_ollama_arguments("multiply", mixed_args, tools)
+    expected_filtered = {"a": "get_stock_price", "b": "2"}  # Should remove 'company_name'
+    print(f"Original: {mixed_args}")
+    print(f"Filtered: {filtered_args}")
+    print(f"Expected: {expected_filtered}")
+    assert filtered_args == expected_filtered, f"Expected {expected_filtered}, got {filtered_args}"
+    print("✅ Mixed arguments filtering test passed")
+    
+    # Test case 3: All invalid arguments
+    print("\n3. Testing all invalid arguments:")
+    invalid_args = {"invalid_param1": "value1", "invalid_param2": "value2"}
+    filtered_args = llm._validate_and_filter_ollama_arguments("multiply", invalid_args, tools)
+    expected_empty = {}
+    print(f"Original: {invalid_args}")
+    print(f"Filtered: {filtered_args}")
+    assert filtered_args == expected_empty, "All invalid arguments should be filtered out"
+    print("✅ Invalid arguments filtering test passed")
+    
+    # Test case 4: Function not found in tools
+    print("\n4. Testing function not found:")
+    some_args = {"param": "value"}
+    filtered_args = llm._validate_and_filter_ollama_arguments("nonexistent_function", some_args, tools)
+    print(f"Original: {some_args}")
+    print(f"Filtered: {filtered_args}")
+    assert filtered_args == some_args, "Arguments should pass through if function not found"
+    print("✅ Function not found test passed")
+    
+    # Test case 5: Empty tools list
+    print("\n5. Testing empty tools list:")
+    some_args = {"param": "value"}
+    filtered_args = llm._validate_and_filter_ollama_arguments("multiply", some_args, [])
+    print(f"Original: {some_args}")
+    print(f"Filtered: {filtered_args}")
+    assert filtered_args == some_args, "Arguments should pass through if no tools provided"
+    print("✅ Empty tools test passed")
+    
+    print("\n🎉 All Ollama argument validation tests passed!")
+    return True
+
+def test_provider_detection():
+    """
+    Test the Ollama provider detection functionality.
+    """
+    print("\nTesting Ollama provider detection...")
+    
+    # Test Ollama provider detection
+    ollama_llm = LLM(model="ollama/llama3.2")
+    assert ollama_llm._is_ollama_provider(), "Should detect ollama/ prefix"
+    print("✅ Ollama prefix detection works")
+    
+    # Test non-Ollama provider
+    openai_llm = LLM(model="gpt-4o-mini")
+    assert not openai_llm._is_ollama_provider(), "Should not detect OpenAI as Ollama"
+    print("✅ Non-Ollama provider detection works")
+    
+    print("✅ Provider detection tests passed!")
+    return True
+
+if __name__ == "__main__":
+    print("Running Ollama sequential tool calling fix tests...")
+    print("=" * 60)
+    
+    # Run tests
+    try:
+        test_provider_detection()
+        test_ollama_argument_validation()
+        
+        print("\n" + "=" * 60)
+        print("🎉 ALL TESTS PASSED!")
+        print("The Ollama sequential tool calling argument mixing issue has been fixed!")
+        
+    except Exception as e:
+        print(f"\n❌ TEST FAILED: {e}")
+        import traceback
+        traceback.print_exc()
diff --git a/test_gemini_tools.py b/test_gemini_tools.py
@@ -0,0 +1,40 @@
+#!/usr/bin/env python3
+"""Test script to verify the refactored Gemini tools logic."""
+
+import logging
+import sys
+import os
+
+# Setup logging to see debug messages
+logging.basicConfig(level=logging.DEBUG, format='%(levelname)s: %(message)s')
+
+# Add the src directory to the Python path
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), 'src', 'praisonai-agents'))
+
+try:
+    from praisonaiagents.llm.llm import LLM
+    from praisonaiagents.llm.openai_client import OpenAIClient
+    
+    print("Testing LLM tool formatting...")
+    llm = LLM(model="gpt-4o-mini")
+    tools = [
+        {'googleSearch': {}},  # Valid Gemini tool
+        {'urlContext': {}},    # Valid Gemini tool  
+        {'codeExecution': {}}, # Valid Gemini tool
+        {'unknown': {}}        # Invalid tool - should be skipped
+    ]
+    
+    formatted = llm._format_tools_for_litellm(tools)
+    print(f"LLM formatted tools ({len(formatted)} tools):", formatted)
+    
+    print("\nTesting OpenAI client tool formatting...")
+    client = OpenAIClient(api_key="not-needed")
+    formatted = client.format_tools(tools)
+    print(f"OpenAI client formatted tools ({len(formatted)} tools):", formatted)
+    
+    print("\nTest completed successfully!")
+    
+except Exception as e:
+    import traceback
+    print(f"Error: {e}")
+    traceback.print_exc()
