Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refine Message model and encapsulate tool calls for AI provider integration #2

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

kushalpoddar
Copy link

@kushalpoddar kushalpoddar commented Nov 27, 2024

Purpose

This PR introduces a new structured messaging system and APIs to support tool calls across multiple AI providers. It aims to unify and streamline the interaction with different AI tools, enhancing the content creation capabilities of an AI suite.

Critical Changes

  • aisuite/framework/message.py: Replaced the existing Message class with a BaseModel derived structure from pydantic, supporting optional fields for tool calls, content, role, and refusal. This enhances type safety and data handling.

    class Function(BaseModel):
        arguments: str
        name: str
    
    class ChatCompletionMessageToolCall(BaseModel):
        id: str
        function: Function
        type: Literal["function"]
    
    class Message(BaseModel):
        content: Optional[str]
        tool_calls: Optional[list[ChatCompletionMessageToolCall]]
        role: Optional[Literal["user", "assistant", "system"]]
        refusal: Optional[str]
  • aisuite/providers/anthropic_provider.py and aisuite/providers/aws_provider.py: Implemented methods for transforming tool calls and responses between OpenAI's format and their respective provider formats (Anthropic, AWS). Detailed handling includes converting tools from generic tool specifications to provider-specific configurations and normalizing responses back into the unified Message format expected by the system.

    class AnthropicProvider(Provider):
        ...
        def chat_completions_create(self, model, messages, **kwargs):
            ...
            # Convert tool results from OpenAI format to Anthropic format
            converted_messages.append(...) # Details of transformation
            ...
            return self.normalize_response(response)
    
    class AwsProvider(Provider):
        ...
        def chat_completions_create(self, model, messages, **kwargs):
            ...
            converted_messages = []
            for message in messages:
                if message_dict["role"] == "tool":
                    bedrock_message = self.transform_tool_result_to_bedrock(message_dict)
                    if bedrock_message:
                        formatted_messages.append(bedrock_message)
            ...
            # Additional logic to handle transformations
  • Added Message import in aisuite/__init__.py and aisuite/framework/__init__.py to enhance framework integration level and ensure all components recognize the Message type throughout the application.

    # In aisuite/__init__.py
    from .framework.message import Message
    
    # In aisuite/framework/__init__.py
    from .message import Message

===== Original PR title and description ============

Original Title: Enhanced Message Structure and Integration with AI Provider APIs for Tool Calls

Original Description:

Purpose

This PR introduces significant enhancements to both the internal message structure and the API integration for handling communication with various AI providers. The main focus is to facilitate the correct mapping and usage of tool calls across different AI services, ensuring uniformity and maximizing compatibility.

Critical Changes

  • aisuite/__init__.py and aisuite/framework/__init__.py:
    Imported Message classes to bolster message handling capabilities.

  • aisuite/framework/choice.py:
    Added optional finish reasons and enriched Message initialization to include new fields like tool_calls which are now distinctly initialized with appropriate defaults to enhance clarity and control over message flow.

  • aisuite/framework/message.py:
    Revamped Message class with Pydantic support for better data validation and structure. Introduced ChatCompletionMessageToolCall and Function as BaseModel subclasses to standardize tool call structures for AI providers.

  • aisuite/providers/anthropic_provider.py:
    Heavily modified response and request handling for the Anthropic provider to accommodate new structured tool calls. Added normalization from Anthropic format to OpenAI’s expected format, increasing interoperability.

  • aisuite/providers/aws_provider.py:
    Adjusted AWS provider to handle message transformations between Bedrock's expected format and the general tool call structure. Also, made changes to include tool configurations if provided.

  • aisuite/providers/groq_provider.py, aisuite/providers/mistral_provider.py, aisuite/providers/openai_provider.py:
    Minor adjustments to each provider, mostly focusing on message transformation consistency.

  • aisuite/utils/tool_manager.py:
    Introduced a ToolManager which handles the registration and execution of tools based on the updated message structures, allowing for dynamic hooking of function calls specific to AI tasks.

Note:

  • Further testing and possibly adjustments may be necessary to fully ensure functionality across all AI providers post-refactor.
  • Verify the comprehensive handling of edge cases in tool transformations, especially with AWS and Groq provider updates.

===== Original PR title and description ============

Original Title: Refactor message structure and tool call API integration for various AI providers

Original Description:

Purpose

The PR aims to enhance the message class compatibility between different AI service providers and standardizes the handling of AI tool calls across providers, ensuring more robust and flexible interactions.

Critical Changes

  • aisuite/framework/message.py:

    • Introduced the Message class utilizing Pydantic for type checking and structure validation. Added new attributes tool_calls, role, and refusal to store tool call information in an API-agnostic format.
    • Extended BaseModel to use type hints ensuring that data conforms to expected schema with relevant types across different providers.
  • aisuite/framework/choice.py:

    • Modified Choice class to include optional attributes finish_reason, allowing the AI to specify why a session or request was completed, leveraging Python's Literal and Optional types for precise state descriptions.
  • aisuite/providers/anthropic_provider.py:

    • Reworked message handling to transform tool call data from the OpenAI format to the Anthropic API format. Added comprehensive transformation of ChatCompletionResponse, including converting response content and handling tool call conversion and normalization processes.
  • aisuite/providers/aws_provider.py:

    • Implemented message format transformations specific to AWS provider requirements, managing both incoming requests to and responses from AWS's AI services. Detailed transformations and print debugging statements added to trace the transformation steps.
  • aisuite/providers/groq_provider.py, aisuite/providers/mistral_provider.py, aisuite/providers/openai_provider.py:

    • Each provider file includes adaptations specific to the respective service, mostly focusing on message transformations adhering to each provider's expected formats.
  • aisuite/utils/tool_manager.py:

    • Developed a ToolManager class to manage registration and execution of tools, handling parameter parsing, execution, and response formatting along with error handling.
  • Additional files like Jupyter notebooks under examples/ and tests in tests/utils/:

    • Serve the purpose of demonstrating and testing the new message structures and tool handling capabilities in practical scenarios.

===== Original PR title and description ============

Original Title: Enhance message structure and adapt tool call conversions across AI providers

Original Description:

Purpose

The changes introduced update the message and tool calling frameworks to support diverse AI providers more effectively. The enhancements include adapting the message structure for different provider APIs, handling tool calls uniquely per provider, and integrating usage details to standardize responses. This ensures compatibility and flexibility within the AI suite's tooling system.

Critical Changes

{

  • aisuite/__init__.py and aisuite/framework/__init__.py: Import the updated Message class, ensuring that all providers utilizing this class can leverage the new structured message format.

  • aisuite/framework/message.py: Transition the Message class to utilize pydantic.BaseModel, making it robust for data validation. The class now optionally includes tool calls, supporting various roles and potential refusal reasons, enhancing the message structure significantly.

  • aisuite/providers/anthropic_provider.py: Adapt message and tool call handling specifically for the Anthropics API. This includes changing kwargs handling, message transformation to match the provider's requirements, and appropriately converting tool call responses, addressing the unique 'stop_reason' and formatting responses to a standardized form.

  • aisuite/providers/aws_provider.py: Implement response normalization and modification of incoming messages (handling tool calls and results) for the AWS Bedrock API. Detailed changes ensure message dicts are correctly formatted for this provider.

  • aisuite/providers/groq_provider.py, aisuite/providers/mistral_provider.py, and aisuite/providers/openai_provider.py: Adjustments reflect the new message handling formats and detailed transformation functions, supporting the structured handling of tool calls and messages across these providers.

  • aisuite/utils/tool_manager.py: Adds a comprehensive suite for tool management, allowing adding and managing tools with simplified or detailed specifications. The implementation focuses on function execution, handling both response and message formation effectively.
    }

===== Original PR title and description ============

Original Title: Integrate advanced message structuring and tool call conversion for various providers

Original Description:

Purpose

The PR is aimed at enhancing message structuring within the aisuite, particularly for handling tool calls across multiple providers like AWS, Anthropic, and more. This ensures that messages are appropriately formatted and converted between different API standards, helping in seamless integration with these external services.

Critical Changes

  • aisuite/framework/message.py: Introduced a new Message class using Pydantic for validating message schemas, including optional fields for content, role, and tool_calls to handle different message types more robustly.
  • aisuite/providers/anthropic_provider.py: Added complex logic to transform tool call data between OpenAI format and Anthropic's expected format, involving detailed conversions both ways and handling tool results separately.
  • aisuite/providers/aws_provider.py: Enhanced the AWS provider to support tool calls by converting message formats suitable for Bedrock's API and processing tool-related responses, aligning them with OpenAI standards.
  • aisuite/utils/tool_manager.py: Tool manager utility class implemented to manage and execute tools based on calls from messages, including a method to dynamically create Pydantic models from functions and handle execution.
    ...

===== Original PR title and description ============

Original Title: Enhance message structuring and add tool call handling across providers

Original Description:

Purpose

The changes are intended to introduce unified message structuring with pydantic models and extend feature support for handling tool calls, ensuring compatibility and streamlining responses for multiple AI model providers. This facilitates a more structured and extensible codebase catering to various AI-powered interactions.

Critical Changes

{

  • In aisuite/framework/__init__.py and aisuite/__init__.py, the Message class is now imported, setting the stage for enhanced message handling through the project using this unified structure.

  • In aisuite/framework/message.py, a significant upgrade has occurred where Message evolved from a basic class to pydantic.BaseModel incorporating optional fields for content, roles, and detailed structures for tool calls such as ChatCompletionMessageToolCall which inherits the runtime checking and documentation benefits of pydantic.

  • aisuite/framework/choice.py now outlines optional finish reasons in tool calls, refining decision capture based on enhanced messaging structures reflecting roles and actions within the workflow.

  • In aisuite/providers/anthropic_provider.py, extensive changes are made to support translation between OpenAI and Anthropic specific tool calling requirements and handling API responses to align with the desired structured message format, utilizing the new Message structure.

  • Enhancements in aisuite/providers/aws_provider.py include better message transformations for AWS-specific formatting and tool usage handling, thereby integrating the extended message and tool handling functionality across various AI model providers.

  • Each provider (Groq, Mistral, AWS, and Anthropic) under aisuite/providers/ now includes transformations and normalization flows to fit the new Message format, demonstrating a commitment to uniformity and extendibility in handling AI messaging responses.

  • The addition of a new file aisuite/utils/tool_manager.py introduces a ToolManager class which manages tool functionalities as callable entities. This development suggests the expansion of tool handling capabilities that are likely to influence future features and integrations in the AI suite.

}

===== Original PR title and description ============

Original Title: Enhance message handling for various providers with new data structures

Original Description:

Purpose

This PR introduces improved support and handling of messages and tool calls across different language models and service providers, increasing interoperability and flexibility in defining and transmitting messages.

Critical Changes

  • aisuite/__init__.py and aisuite/framework/__init__.py:

    • Imported the new Message class to initialize proper message handling.
  • aisuite/framework/message.py:

    • Redefined the Message class as a Pydantic model, enhancing data validation and error handling.
    • Introduced nested models like Function and ChatCompletionMessageToolCall to handle complex data structures for tool calls.
  • aisuite/framework/choice.py:

    • Set finish_reason in Choice class to potentially handle multiple end conditions (like "stop" or "tool_calls") and restructured the Message initialization to align with new attributes.
  • aisuite/providers/anthropic_provider.py and aisuite/providers/aws_provider.py:

    • Major overhaul to convert tool calls and responses between OpenAI and respective format of each provider, AWS and Anthropic.
    • Added normalization functions to transform proprietary API responses into standard formats.
  • aisuite/providers/groq_provider.py, aisuite/providers/mistral_provider.py, and aisuite/providers/openai_provider.py:

    • Adjusted message handling to integrate the new Message structure, ensuring compatibility and consistent data handling across different model providers.
  • aisuite/utils/tool_manager.py:

    • Added a new ToolManager class that manages tool registry and execution, supports dynamic parameter models, and integrates well with JSON-based interoperability.

===== Original PR title and description ============

Original Title: Introduce message handling enhancements using OpenAI and AWS formats

Original Description:

Purpose

This PR introduces systematic enhancements to the handling of messages and tool calling across multiple providers like Anthropics and AWS, aligning with their specific requirements and APIs.

Critical Changes

  • aisuite/framework/message.py: Refactored the Message class to use pydantic.BaseModel for data validation, incorporating support for optional fields and structured tool calls. Major changes include defining a new Function and ChatCompletionMessageToolCall sub-models to format tool calls accurately.
  • aisuite/providers/anthropic_provider.py: Major enhancements adjust tool handling to conform with Anthropics' API, involving modifying tool calls' transforming and normalizing processes. Added comprehensive mapping for function calls and responses between OpenAI and Anthropics format. Additional debug logging is implemented to trace tool call transformations.
  • aisuite/providers/aws_provider.py: Extended support for AWS provider to convert OpenAI formatted tool calls to AWS's expected JSON payload. Modifications ensure correct mapping and handling of tool results and assistant messages for both single and batch requests.

Additional updates across various providers (groq_provider.py, mistral_provider.py, aws_provider.py) improve overall integration and consistency in message format transformation and tool calling processes.

===== Original PR title and description ============

Original Title: Enhanced tool calling and message handling for multiple providers

Original Description:

Purpose

This PR introduces improved tool calling capabilities, richer message object structures, and compatibility updates across various LLM providers to better handle diverse message and tool call formats.

Critical Changes

  • aisuite/__init__.py and aisuite/framework/__init__.py:

    • New imports for Message class to enable enhanced message handling across the suite.
  • aisuite/framework/message.py:

    • Redefinition of Message class using BaseModel for stricter validation and introduces ChatCompletionMessageToolCall class to handle structured tool responses.
  • aisuite/framework/choice.py:

    • Updated Choice class to include optional typing and improved default setup for message instantiation supporting finish_reason attribute for completion state tracking.
  • aisuite/providers/anthropic_provider.py and aisuite/providers/aws_provider.py:

    • Major refactoring to adapt tool call formats from/to OpenAI and native formats of Anthropic and AWS, involving complex JSON transformations and API interactions.
  • aisuite/utils/tool_manager.py:

    • ToolManager class capable of registering tools with dynamic parameter model inference, executing tool calls, and returning standard formatted responses.
  • examples/SimpleToolCalling.ipynb:

    • Jupyter notebook example demonstrating how to utilize the ToolManager with a mock API to handle tool interactions within chat environments.

New Dependencies:

  • Pydantic added for data validation in structured message and tool handling.

===== Original PR title and description ============

Original Title: Rcp/tool calling

Original Description:
None

PR Review Summary Celebratory GIF

Overall Review:

The provided PR adds significant functionality related to tool calling across various AI providers, integrating complex message transformation and normalization logic. It spans across initializing and transforming messages according to the specifications required by APIs like AWS, Anthropic, etc. The modifications involve converting and normalizing tool responses and ensuring these are compatible with the AI models' expected formats. The changes are substantial and have a broad impact on the application, potentially affecting how tool results are processed, displayed, or logged.


Logical Error Analysis

1. [Blocker] In `AnthropicProvider`, `normalize_response` tries to handle different `stop_reason`s including `tool_use`. However, `tool_call_id` and method to retrieve `content` might not reliably parse the expected format every time. This can lead to unhandled exceptions or missing data.

🔒 Security Analysis

2. [Blocker] The methods `transform_tool_call_to_openai` and JSON handling in APIs:
Whenever JSON parsing or serialization from external inputs is involved, ensure there are strict schema validations or error handling to mitigate potential injection attacks or corruption of data formats. The current code necessitates confirmation that content passed to JSON parsers respects expected formats and does not introduce injection risks.

Recommendations

Recommendation #1

To address the potential inconsistencies in parsing tool_call_id and content, it is recommended to add rigorous checks and transformations to ensure the data conforms consistently to the expected format. Consider applying a schema validation or using a more robust method for parsing, especially when handling dynamic JSON content.
For example, you could modify the relevant part in AnthropicProvider as follows:

def normalize_response(self, response):
    normalized_response = ChatCompletionResponse()
    finish_reason_mapping = {
        "end_turn": "stop",
        "max_tokens": "length",
        "tool_use": "tool_calls",
    }
    normalized_response.choices[0].finish_reason = finish_reason_mapping.get(response.stop_reason, "stop")
   
    # Add usage information
    normalized_response.usage = {
        "prompt_tokens": response.usage.input_tokens,
        "completion_tokens": response.usage.output_tokens,
        "total_tokens": response.usage.input_tokens + response.usage.output_tokens,
    }
   
    # Check if the response contains tool usage and validate format
    if response.stop_reason == "tool_use":
        tool_call = next((content for content in response.content if 'type' in content and content.type == "tool_use"), None)
        if tool_call and 'id' in tool_call:
            tool_call_id = tool_call.id
            function = Function(name=tool_call.name, arguments=json.dumps(tool_call.input))
            tool_call_obj = ChatCompletionMessageToolCall(id=tool_call_id, function=function, type="function")
            text_content = next((content.text for content in response.content if 'type' in content and content.type == "text"), "")
            message = Message(content=text_content or None, tool_calls=[tool_call_obj] if tool_call else None, role="assistant", refusal=None)
            normalized_response.choices[0].message = message
        else:
            raise ValueError("Expected 'tool_use' content is missing or malformed in API response.")
    
    return normalized_response

This snippet includes error handling for missing or malformed 'tool_use' data, ensuring the internal data paths are consistent before parsing.

Recommendation #2

Implement rigorous input validation and error handling around JSON operations and external data handling. Make use of secure practices such as using parameterized queries or templates when constructing requests or handling data. For instance:

def transform_tool_call_to_openai(self, response):
    if response.get("stopReason") != "tool_use":
        return None

    try:
        tool_calls = []
        for content in response["output"]["message"]["content"]:
            if "toolUse" in content:
                tool = content["toolUse"]
                tool_calls.append({
                    "type": "function",
                    "id": tool["toolUseId"],
                    "function": {
                        "name": tool["name"],
                        "arguments": json.dumps(tool["input"], cls=SecureJSONEncoder),
                    },
                })
    except json.JSONDecodeError as e:
        raise ValueError(f"JSON parsing error: {str(e)}")

    return {
        "role": "assistant",
        "content": None,
        "tool_calls": tool_calls,
        "refusal": None,
    }

In this code snippet, SecureJSONEncoder could be a custom JSONEncoder that sanitizes inputs to prevent JSON injection attacks. This ensures the integrity and security of data transformations and external interactions.

[Configure settings at: Archie AI - Automated PR Review]

@archie-ai-code-explain-pr-review archie-ai-code-explain-pr-review bot added the enhancement New feature or request label Nov 27, 2024

PR Review Summary Celebratory GIF

Overall Review:

This PR introduces a ToolManager alongside extensive modifications to frameworks and providers to support tool calling capabilities. It affects multiple components of the aisuite, including anthropic, aws, and others, adapting their messaging and response normalization to incorporate tools dynamically. The code is structured to handle conversions between multiple formats and system settings, which is necessary for the expected diverse provider interactions. However, it is critical to ensure security, reliability, and coverage considering these extensive changes.


Logical Error Analysis

1. [Blocker] Conversion Functionality Incompletion:
The conversion method `convert_openai_tools_to_anthropic` only handles "function" type tools but does not address other types that could potentially be part of the toolset.

🔒 Security Analysis

2. [Blocker] Potential Data Exposure:
Classes like `Function` and `Message` in `message.py` and logged responses in service provider classes expose sensitive data that may include IPs, user data, or metadata that could assist attackers.

🧪 Test Coverage Analysis

3. [Consider] Coverage on Exception Handling:
The error paths, especially around malformed incoming data for tools (both in structure and type), are not tested. Tests should be added to simulate and assert behavior when incorrect or malformed data is received.

🌟 Code Quality And Design

4. [Consider] Complexity:
Handling Tool Calls and Mapping - The logic in `AnthropicProvider`, `AwsProvider`, and other provider modules handles transformation between different tool calling formats.

5. [Nit] Design Review:
Module Decoupling - The implementation spreads across multiple modules, enforcing a decoupling that favors single responsibility and easier unit testing.

Recommendations

Recommendation #1

Modify the convert_openai_tools_to_anthropic method to handle all expected tool types, or document and handle unsupported tool types by either logging an error or a warning. For example:

def convert_openai_tools_to_anthropic(openai_tools):
    anthropic_tools = []
    for tool in openai_tools:
        if tool["type"] == "function":
            function = tool["function"]
            anthropic_tool = {
                "name": function["name"],
                "description": function.get("description", ""),
                "input_schema": {
                    "type": "object",
                    "properties": function["parameters"]["properties"],
                    "required": function["parameters"].get("required", []),
                },
            }
            anthropic_tools.append(anthropic_tool)
        else:
            # Log unhandled tool types, or possibly raise an error if necessary
            logging.warning(f"Tool type {tool['type']} is not supported yet")
    return anthropic_tools

Be sure to import the logging module and configure it appropriately in your module setup.

Recommendation #2

Ensure that all logging and data classes do not include or expose sensitive or identifiable information. Use data masking or transformation techniques to obscure real values in logs and ensure that no sensitive data is logged in plain text. Modify the log statements and any debug outputs to mask any potentially sensitive information:

class Function(BaseModel):
    name: str
    arguments: str

    def log_function(self):
        masked_arguments = mask_sensitive_data(self.arguments)  # Assumes existence of this function
        logging.debug(f"Function called: {self.name}, with arguments: {masked_arguments}")

# Replace direct print statements with the above method call:
function_instance.log_function()

Remember to define or implement the mask_sensitive_data function that can handle the specific structure and fields of your data objects.

Recommendation #3

Implement additional test cases that focus on error handling for malformed or incorrect data scenarios to ensure robustness. Here's an example test case using pytest:

import pytest
from module import data_handling_function

def test_data_handling_with_malformed_data():
    malformed_data = "incorrect format data"
    with pytest.raises(ValidationError):
        result = data_handling_function(malformed_data)
        assert result is None, "Function should not handle malformed data without error"

This test checks that handling of malformed data raises a ValidationError, ensuring that the system reacts appropriately to unexpected inputs.

Recommendation andrewyng#4

Abstract the transformations more robustly, potentially using a factory pattern to simplify and manage the complexity of various tool formats. For example:

class ToolTransformerFactory:
    @staticmethod
    def get_transformer(provider_type):
        if provider_type == "Anthropic":
            return AnthropicToolTransformer()
        elif provider_type == "AWS":
            return AWSToolTransformer()
        else:
            raise ValueError("Unsupported provider type")

class AnthropicToolTransformer:
    def transform(self, tool_data):
        # perform transformation logic specific to Anthropic
        pass

class AWSToolTransformer:
    def transform(self, tool_data):
        # perform transformation logic specific to AWS
        pass
# Usage:
transformer = ToolTransformerFactory.get_transformer("Anthropic")
transformed_data = transformer.transform(tool_data)

This pattern will facilitate adding support for new providers or changing existing implementations with minimal impact on other parts of the system.

Recommendation andrewyng#5

Ensure logical grouping and minimal interdependencies of modules. Reviewing module responsibilities and interactions might reveal opportunities to reduce complexity or improve cohesiveness, making it easier to manage and evolve separate parts without unintended side effects. You might consider reevaluating some of the finer-grained separations if they lead to excessive hopping between files or unclear relationships between components.

[Configure settings at: Archie AI - Automated PR Review]

PR Review Summary Celebratory GIF

Overall Review:

This PR introduces extensive modifications across the aisuite, aiming to enhance its ability to interact with AI services by handling tool calls. It touches various aspects of message transformation and normalization, ensuring compatibility across system interfaces. The changes are complex with potentially significant implications for the application’s capability to process external AI tool results, managing these translations across multiple providers like AWS, Anthropic, and others.


Logical Error Analysis

1. [Blocker] In `AnthropicProvider`, `normalize_response` tries to handle different `stop_reason`s including `tool_use`. However, `tool_call_id` and method to retrieve `content` might not reliably parse the expected format every time. This can lead to unhandled exceptions or missing data.

🔒 Security Analysis

2. [Blocker] The methods `transform_tool_call_to_openai` and JSON handling in APIs involve sensitive data parsing which requires strict schema validations or error handling to mitigate potential injection attacks or corruption of data formats.

Recommendations

Recommendation #1

To address the potential inconsistencies in parsing tool_call_id and content, it is recommended to add rigorous checks and transformations to ensure the data conforms consistently to the expected format. Consider applying a schema validation or using a more robust method for parsing, especially when handling dynamic JSON content. Modify the relevant part in AnthropicProvider as follows:

def normalize_response(self, response):
    normalized_response = ChatCompletionResponse()
    finish_reason_mapping = {
        "end_turn": "stop",
        "max_tokens": "length",
        "tool_use": "tool_calls",
    }
    normalized_response.choices[0].finish_reason = finish_reason_mapping.get(response.stop_reason, "stop")
   
    # Add usage information
    normalized_response.usage = {
        "prompt_tokens": response.usage.input_tokens,
        "completion_tokens": response.usage.output_tokens,
        "total_tokens": response.usage.input_tokens + response.usage.output_tokens,
    }
   
    # Check if the response contains tool usage and validate format
    if response.stop_reason == "tool_use":
        tool_call = next((content for content in response.content if 'type' in content and content.type == "tool_use"), None)
        if tool_call and 'id' in tool_call:
            tool_call_id = tool_call.id
            function = Function(name=tool_call.name, arguments=json.dumps(tool_call.input))
            tool_call_obj = ChatCompletionMessageToolCall(id=tool_call_id, function=function, type="function")
            text_content = next((content.text for content in response.content if 'type' in content and content.type == "text"), "")
            message = Message(content=text_content or None, tool_calls=[tool_call_obj] if tool_call else None, role="assistant", refusal=None)
            normalized_response.choices[0].message = message
        else:
            raise ValueError("Expected 'tool_use' content is missing or malformed in API response.")
    
    return normalized_response

This snippet includes error handling for missing or malformed 'tool_use' data, ensuring the internal data paths are consistent before parsing.

Recommendation #2

Implement rigorous input validation and error handling around JSON operations and external data handling. Make use of secure practices such as using parameterized queries or templates when constructing requests or handling data. For instance:

def transform_tool_call_to_openai(self, response):
    if response.get("stopReason") != "tool_use":
        return None

    try:
        tool_calls = []
        for content in response["output"]["message"]["content"]:
            if "toolUse" in content:
                tool = content["toolUse"]
                tool_calls.append({
                    "type": "function",
                    "id": tool["toolUseId"],
                    "function": {
                        "name": tool["name"],
                        "arguments": json.dumps(tool["input"], cls=SecureJSONEncoder),
                    },
                })
    except json.JSONDecodeError as e:
        raise ValueError(f"JSON parsing error: {str(e)}")

    return {
        "role": "assistant",
        "content": None,
        "tool_calls": tool_calls,
        "refusal": None,
    }

In this code snippet, SecureJSONEncoder could be a custom JSONEncoder that sanitizes inputs to prevent JSON injection attacks. This ensures the integrity and security of data transformations and external interactions.

[Configure settings at: Archie AI - Automated PR Review]

Copy link

dev-archie-ai-code-explain-pr bot commented Nov 27, 2024

PR Code Suggestions Summary ✨

CategorySuggestion
Maintainability
Refactor tool handling into a dedicated method to clean up the main method and improve readability.

File: aisuite/providers/aws_provider.py L101-L106

Suggestion: For clarity and maintainability, it is better to separate large blocks of conditional logic into small, well-named private methods. This can be done in the AwsProvider class where multiple if-else blocks handle different message roles.

--- Original
+++ Improved
@@ -1,19 +1 @@
-+        # Handle tools if present in kwargs
-+        tool_config = None
-+        if "tools" in kwargs:
-+            tool_config = {
-+                "tools": [
-+                    {
-+                        "toolSpec": {
-+                            "name": tool["function"]["name"],
-+                            "description": tool["function"].get("description", " "),
-+                            "inputSchema": {"json": tool["function"]["parameters"]},
-+                        }
-+                    }
-+                    for tool in kwargs["tools"]
-+                ]
-+            }
-+            print(f"Tool config: {tool_config}")
-+            print(f"Received tools specification: {kwargs['tools']}")
-+            # Remove tools from kwargs since we're handling it separately
-+            del kwargs["tools"]++    self._configure_tool_handling(kwargs)
Possible issue
Add error handling for JSON parsing to prevent runtime errors due to malformed JSON input.

File: aisuite/providers/aws_provider.py LNone-LNone

Suggestion: For error-prone string manipulations, especially when involving JSON parsing such as in the AWS provider, it's safer to use a try-except block to catch and handle parsing errors.

--- Original
+++ Improved
@@ -1 +1,5 @@
-+                            "input": json.loads(tool_call["function"]["arguments"]),++                            try:
++                                "input": json.loads(tool_call["function"]["arguments"]),
++                            except json.JSONDecodeError as e:
++                                print(f"Failed to parse JSON from the arguments: {e}")
++                                continue
Maintainability
Introduce a validation method for message roles to handle unexpected values gracefully.

File: aisuite/providers/aws_provider.py LNone-LNone

Suggestion: To avoid silent failure and handle unexpected keys in the message dictionary robustly, implement a method to validate and default unknown roles, ensuring proper application flow.

--- Original
+++ Improved
@@ -1,4 +1 @@
-+                        {
-+                            "role": message_dict["role"],
-+                            "content": [{"text": message_dict["content"]}],
-+                        }++                        self._validate_and_default_message_role(message_dict)

Comment on lines +101 to +106
message_dict
)
if bedrock_message:
formatted_messages.append(bedrock_message)
elif message_dict["role"] == "assistant":
# Convert assistant message to Bedrock format

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor tool handling into a dedicated method to clean up the main method and improve readability.

For clarity and maintainability, it is better to separate large blocks of conditional logic into small, well-named private methods. This can be done in the AwsProvider class where multiple if-else blocks handle different message roles.

Suggested change
message_dict
)
if bedrock_message:
formatted_messages.append(bedrock_message)
elif message_dict["role"] == "assistant":
# Convert assistant message to Bedrock format
+ self._configure_tool_handling(kwargs)

@dev-archie-ai-code-explain-pr dev-archie-ai-code-explain-pr bot changed the title Rcp/tool calling Enhanced tool calling and message handling for multiple providers Nov 27, 2024

PR Review Summary Celebratory GIF

Overall Review:

The PR introduces a sophisticated handling mechanism for tool calling across various providers like AWS, Anthropic, and others, along with enhancements in message object structures and compatibility updates. It includes significant refactoring and new implementations in the handling of JSON, external APIs, and dynamic data structures which are pivotal for the functioning of the AI systems in the suite. The changes also involve a new ToolManager, better structured data handling using BaseModel, and extensive JSON data manipulations which could involve security and logic error considerations.


Logical Error Analysis

1. [Blocker] In `AnthropicProvider`, `normalize_response` tries to handle different `stop_reason`s including `tool_use`. However, `tool_call_id` and method to retrieve `content` might not reliably parse the expected format every time. This can lead to unhandled exceptions or missing data.

🔒 Security Analysis

2. [Blocker] The methods `transform_tool_call_to_openai` and JSON handling in APIs involve sensitive data parsing which requires strict schema validations or error handling to mitigate potential injection attacks or corruption of data formats.

Recommendations

Recommendation #1

To address the potential inconsistencies in parsing tool_call_id and content, add rigorous checks and transformations to ensure the data conforms consistently to the expected format. Consider applying a schema validation or using a more robust method for parsing, especially when handling dynamic JSON content. Modify the relevant part in AnthropicProvider like so:

def normalize_response(self, response):
    normalized_response = ChatCompletionResponse()
    finish_reason_mapping = {
        "end_turn": "stop",
        "max_tokens": "length",
        "tool_use": "tool_calls",
    }
    normalized_response.choices[0].finish_reason = finish_reason_mapping.get(response.stop_reason, "stop")
   
    # Add usage information
    normalized_response.usage = {
        "prompt_tokens": response.usage.input_tokens,
        "completion_tokens": response.usage.output_tokens,
        "total_tokens": response.usage.input_tokens + response.usage.output_tokens,
    }
   
    # Check if the response contains tool usage and validate format
    if response.stop_reason == "tool_use":
        tool_call = next((content for content in response.content if 'type' in content and content.type == "tool_use"), None)
        if tool_call and 'id' in tool_call:
            tool_call_id = tool_call.id
            function = Function(name=tool_call.name, arguments=json.dumps(tool_call.input))
            tool_call_obj = ChatCompletionMessageToolCall(id=tool_call_id, function=function, type="function")
            text_content = next((content.text for content in response.content if 'type' in content and content.type == "text"), "")
            message = Message(content=text_content or None, tool_calls=[tool_call_obj] if tool_call else None, role="assistant", refusal=None)
            normalized_response.choices[0].message = message
        else:
            raise ValueError("Expected 'tool_use' content is missing or malformed in API response.")
   
    return normalized_response

This snippet includes error handling for missing or malformed 'tool_use' data, ensuring consistency before parsing.

Recommendation #2

Implement rigorous input validation and error handling around JSON operations and external data handling. Use secure practices such as using parameterized queries or templates when constructing requests or handling data to ensure data integrity and security. For instance:

def transform_tool_call_to_openai(self, response):
    if response.get("stopReason") != "tool_use":
        return None

    try:
        tool_calls = []
        for content in response["output"]["message"]["content"]:
            if "toolUse" in content:
                tool = content["toolUse"]
                tool_calls.append({
                    "type": "function",
                    "id": tool["toolUseId"],
                    "function": {
                        "name": tool["name"],
                        "arguments": json.dumps(tool["input"], cls=SecureJSONEncoder),
                    },
                })
    except json.JSONDecodeError as e:
        raise ValueError(f"JSON parsing error: {str(e)}")

    return {
        "role": "assistant",
        "content": None,
        "tool_calls": tool_calls,
        "refusal": None,
    }

In this code snippet, SecureJSONEncoder could be a custom JSONEncoder that sanitizes inputs to prevent JSON injection attacks. This ensures the integrity and security of data transformations and external interactions.

[Configure settings at: Archie AI - Automated PR Review]

Copy link

dev-archie-ai-code-explain-pr bot commented Nov 27, 2024

PR Code Suggestions Summary ✨

CategorySuggestion
Possible issue
Prevent potential KeyError by using safer dictionary access.

File: aisuite/framework/message.py LNone-LNone

Suggestion: Replace direct dictionary access role with get() for safer access in case the key does not exist in the dictionary.

--- Original
+++ Improved
@@ -1 +1 @@
-elif msg["role"] == "assistant" and "tool_calls" in msg:+elif msg.get("role") == "assistant" and "tool_calls" in msg:
Maintainability
Encapsulate debug print statements for cleaner code and improved maintainability.

File: aisuite/providers/anthropic_provider.py LNone-LNone

Suggestion: Encapsulate all the print statements in a debug function to better manage debugging and clean up code.

--- Original
+++ Improved
@@ -1,2 +1,3 @@
-print(converted_messages)
-print(response)+if debug_enabled:
+    debug_print(converted_messages)
+    debug_print(response)
Security
Add error handling around JSON parsing to manage malformed input better and improve code robustness.

File: aisuite/utils/tool_manager.py LNone-LNone

Suggestion: Add explicit error handling for JSON operations to manage potential exceptions that can arise due to malformatted inputs.

--- Original
+++ Improved
@@ -1 +1,5 @@
-arguments = json.loads(tool_call.function.arguments)+try:
+    arguments = json.loads(tool_call.function.arguments)
+except json.JSONDecodeError as e:
+    log_error(f"Failed to parse JSON: {e}")
+    return
Maintainability
Refactor message transformation to reduce complexity and improve code maintainability.

File: aisuite/providers/aws_provider.py LNone-LNone

Suggestion: Refactor the handling and transformation of messages between OpenAI and AWS to a separate class or function to reduce complexity within the method.

--- Original
+++ Improved
@@ -1,6 +1 @@
-if message_dict["role"] == "tool":
-    bedrock_message = self.transform_tool_result_to_bedrock(
-        message_dict
-    )
-    if bedrock_message:
-        formatted_messages.append(bedrock_message)+formatted_messages += self.handle_message_transformation(message_dict)
Performance
Improve code clarity and performance using list comprehensions for tool transformations.

File: aisuite/providers/anthropic_provider.py LNone-LNone

Suggestion: Use list comprehensions for cleaner code when transforming tool specifications.

--- Original
+++ Improved
@@ -0,0 +1,12 @@
+anthropic_tools = [
+    {
+        "name": tool["function"]["name"],
+        "description": tool["function"]["description"],
+        "input_schema": {
+            "type": "object",
+            "properties": tool["function"]["parameters"]["properties"],
+            "required": tool["function"]["parameters"].get("required", []),
+        },
+    }
+    for tool in openai_tools if tool.get("type") == "function"
+]
Maintainability
Refactor response choice handling into dedicated functions for better code organization and reusability.

File: examples/SimpleToolCalling.ipynb LNone-LNone

Suggestion: Encapsulate response choice handling in functions or method calls for clearer logical boundaries and reusability.

--- Original
+++ Improved
@@ -1,2 +1 @@
-messages.append(response.choices[0].message) # Model's function call message
-messages.append(result_as_message[0])+process_response_and_add_messages(response, result_as_message, messages)

@dev-archie-ai-code-explain-pr dev-archie-ai-code-explain-pr bot changed the title Enhanced tool calling and message handling for multiple providers Introduce message handling enhancements using OpenAI and AWS formats Nov 27, 2024

PR Review Summary Celebratory GIF

Overall Review:

The PR introduces significant changes aimed at standardizing and enhancing message handling and tool calling functionality across various AI providers. While the core intent to align with specific API requirements of services like AWS and Anthropic is beneficial, the complexity of the changes presents multiple areas of concern. This includes potential logical errors in data manipulation logic, security in handling JSON data transformations, and the overall sufficiency of test coverage for new and modified codes. The addition of robust tool-management utilities and modifications in message transformations indicates a well-planned improvement, but the real-world application will need careful validation.


Logical Error Analysis

1. [Blocker] In `AnthropicProvider`, `normalize_response` tries to handle different `stop_reasons` including `tool_use`. However, `tool_call_id` and method to retrieve `content` might not reliably parse the expected format every time. This can lead to unhandled exceptions or missing data.

🔒 Security Analysis

2. [Blocker] The methods `transform_tool_call_to_openai` and JSON handling in APIs involve sensitive data parsing which requires strict schema validations or error handling to mitigate potential injection attacks or corruption of data formats.

Recommendations

Recommendation #1

To address the potential inconsistencies in parsing tool_call_id and content, add rigorous checks and transformations to ensure the data conforms consistently to the expected format. Consider applying a schema validation or using a more robust method for parsing, especially when handling dynamic JSON content. Here's a code modification suggestion for the normalize_response function in the AnthropicProvider:

def normalize_response(self, response):
    normalized_response = ChatCompletionResponse()
    finish_reason_mapping = {
        "end_turn": "stop",
        "max_tokens": "length",
        "tool_use": "tool_calls",
    }
    normalized_response.choices[0].finish_reason = finish_reason_mapping.get(response.stop_reason, "stop")
   
    # Add usage information
    normalized_response.usage = {
        "prompt_tokens": response.usage.input_tokens,
        "completion_tokens": response.usage.output_tokens,
        "total_tokens": response.usage.input_tokens + response.usage.output_tokens,
    }
   
    # Check if the response contains tool usage and validate format
    if response.stop_reason == "tool_use":
        tool_call = next((content for content in response.content if 'type' in content and content.type == "tool_use"), None)
        if tool_call and 'id' in tool_call:
            tool_call_id = tool_call.id
            function = Function(name=tool_call.name, arguments=json.dumps(tool_call.input))
            tool_call_obj = ChatCompletionMessageToolCall(id=tool_call_id, function=function, type="function")
            text_content = next((content.text for content in response.content if 'type' in content and content.type == "text"), "")
            message = Message(content=text_content or None, tool_calls=[tool_call_obj] if tool_call else None, role="assistant", refusal=None)
            normalized_response.choices[0].message = message
        else:
            raise ValueError("Expected 'tool_use' content is missing or malformed in API response.")
   
    return normalized_response

This snippet includes error handling for missing or malformed 'tool_use' data, ensuring the internal data paths are consistent before parsing.

Recommendation #2

Implement rigorous input validation and error handling around JSON operations and external data handling. Use secure practices such as using parameterized queries or templates when constructing requests or handling data to ensure data integrity and security. Here's an example of how you might code this:

def transform_tool_call_to_openai(self, response):
    if response.get("stopReason") != "tool_use":
        return None

    try:
        tool_calls = []
        for content in response["output"]["message"]["content"]:
            if "toolUse" in content:
                tool = content["toolUse"]
                tool_calls.append({
                    "type": "function",
                    "id": tool["toolUseId"],
                    "function": {
                        "name": tool["name"],
                        "arguments": json.dumps(tool["input"], cls=SecureJSONEncoder),
                    },
                })
    except json.JSONDecodeError as e:
        raise ValueError(f"JSON parsing error: {str(e)}")

    return {
        "role": "assistant",
        "content": None,
        "tool_calls": tool_calls,
        "refusal": None,
    }

In this code snippet, SecureJSONEncoder could be a custom JSONEncoder that sanitizes inputs to prevent JSON injection attacks. This ensures the integrity and security of data transformations and external interactions.

[Configure settings at: Archie AI - Automated PR Review]

PR Review Summary Celebratory GIF

Overall Review:

The PR introduces significant enhancements in handling messages and tool calling across various AI providers by standardizing interactions and data formats, particularly for AWS and Anthropics. Changes include refactoring the Message class with pydantic.BaseModel for stronger data validation and enhancing transformation and normalization processes for tool calls. The alterations are broad and impact essential functionalities, including tools management and message coordination. While the alterations aim to accommodate provider-specific nuances, they introduce complexities requiring careful scrutiny to ensure integration smoothness and reliability.


Logical Error Analysis

1. [Blocker] In `AnthropicProvider`, `normalize_response` tries to handle different `stop_reason`s including `tool_use`. However, `tool_call_id` and method to retrieve `content` might not reliably parse the expected format every time. This can lead to unhandled exceptions or missing data.

🔒 Security Analysis

2. [Blocker] The methods `transform_tool_call_to_openai` and JSON handling in APIs involve sensitive data parsing which requires strict schema validations or error handling to mitigate potential injection attacks or corruption of data formats.

🧪 Test Coverage Analysis

3. [Consider] Test coverage appears lacking in scenarios involving erroneous or malicious input data. Given the extensive data handling and transformations, it is crucial to ensure that negative tests check how the system handles unexpected or malformed input.

Recommendations

Recommendation #1

To address the potential inconsistencies in parsing tool_call_id and content, implement rigorous checks and transformations to ensure data conforms to the expected format consistently. Consider applying schema validation or robust parsing especially for handling dynamic JSON content. You can modify the normalize_response function in AnthropicProvider:

def normalize_response(self, response):
    normalized_response = ChatCompletionResponse()
    finish_reason_mapping = {
        "end_turn": "stop",
        "max_tokens": "length",
        "tool_use": "tool_calls",
    }
    normalized_response.choices[0].finish_reason = finish_reason_mapping.get(response.stop_reason, "stop")
   
    # Add usage information
    normalized_response.usage = {
        "prompt_tokens": response.usage.input_tokens,
        "completion_tokens": response.usage.output_tokens,
        "total_tokens": response.usage.input_tokens + response.usage.output_tokens,
    }
   
    # Validate tool usage
    if response.stop_reason == "tool_use":
        tool_call = next((content for content in response.content if 'type' in content and content.type == "tool_use"), None)
        if tool_call and 'id' in tool_call:
            tool_call_id = tool_call.id
            function = Function(name=tool_call.name, arguments=json.dumps(tool_call.input))
            tool_call_obj = ChatCompletionMessageToolCall(id=tool_call_id, function=function, type="function")
            text_content = next((content.text for content in response.content if 'type' in content and content.type == "text"), "")
            message = Message(content=text_content or None, tool_calls=[tool_call_obj] if tool_call else None, role="assistant", refusal=None)
            normalized_response.choices[0].message = message
        else:
            raise ValueError("Expected 'tool_use' content is missing or malformed in API response.")
    
    return normalized_response

This snippet will ensure that the tool_use data is checked and properly parsed, throwing an error if the expected format is not met.

Recommendation #2

Implement rigorous input validation and error handling around JSON operations and external data handling. Use secure practices, such as parameterized queries or templates when constructing requests or handling data. For instance, redesign the method transform_tool_call_to_openai using secure JSON handling practices:

def transform_tool_call_to_openai(self, response):
    if response.get("stopReason") != "tool_use":
        return None

    try:
        tool_calls = []
        for content in response["output"]["message"]["content"]:
            if "toolUse" in content:
                tool = content["toolUse"]
                tool_calls.append({
                    "type": "function",
                    "id": tool["toolUseId"],
                    "function": {
                        "name": tool["name"],
                        "arguments": json.dumps(tool["input"], cls=SecureJSONEncoder),
                    },
                })
    except json.JSONDecodeError as e:
        raise ValueError(f"JSON parsing error: {str(e)}")

    return {
        "role": "assistant",
        "content": None,
        "tool_calls": tool_calls,
        "refusal": None,
    }

In this code snippet, SecureJSONEncoder should be a custom JSONEncoder that sanitizes inputs to prevent JSON injection attacks.

Recommendation #3

Enhance the test suite to cover various scenarios of malformed or incorrect data inputs. Introduce comprehensive tests simulating different types of errors to ensure robust error handling and security. For example, create tests for JSON transformations to simulate malformed JSON:

import pytest
from your_module import some_function_handling_json

def test_malformed_json_handling():
    with pytest.raises(SomeSpecificException):
        malformed_json = '{unclosed_json_object'
        some_function_handling_json(malformed_json)
    # Ensure the function raises an error or handles the malformed JSON gracefully.

This tests how the system behaves under erroneous conditions, thereby enhancing the reliability of data transformations.

[Configure settings at: Archie AI - Automated PR Review]

Copy link

dev-archie-ai-code-explain-pr bot commented Nov 27, 2024

PR Code Suggestions Summary ✨

CategorySuggestion
Enhancement
Utilize enums to enforce constraints on 'role' and type fields, enhancing robustness and maintain type safety.

File: aisuite/framework/message.py L6-L24

Suggestion: Use Enumerations to enforce valid values and enhance type safety in role and type fields.

--- Original
+++ Improved
@@ -1,4 +1,4 @@
 +    content: Optional[str]
 +    tool_calls: Optional[list[ChatCompletionMessageToolCall]]
-+    role: Optional[Literal["user", "assistant", "system"]]
++    role: Optional[Literal["user", "assistant", "system"]] = Field(..., description="Role of the message sender", enum=["user", "assistant", "system"])
 +    refusal: Optional[str]
Maintainability
Reduces code duplication and enhances maintainability by refactoring repetitive conversion code into a separate function.

File: aisuite/providers/anthropic_provider.py L32-L175

Suggestion: Abstract the repetitive conversion logic into separate functions for clarity and reuse.

--- Original
+++ Improved
@@ -1,8 +1 @@
-+        # Convert tool results from OpenAI format to Anthropic format
-+        converted_messages = []
-+        for msg in messages:
-+            if isinstance(msg, dict):
-+                if msg["role"] == "tool":
-+                    # Convert tool result message
-+                    converted_msg = {
-+                        "role": "user",++        converted_messages = self.refactor_conversion_logic(messages)
Performance
Utilizes Python list comprehension for more concise and readable transformation of tools data.

File: aisuite/providers/aws_provider.py LNone-LNone

Suggestion: Incorporate list comprehension for concise and efficient transformations wherever possible.

--- Original
+++ Improved
@@ -1,7 +1 @@
-+        tool_config = None
-+        if "tools" in kwargs:
-+            tool_config = {
-+                "tools": [
-+                    {
-+                        "toolSpec": {
-+                            "name": tool["function"]["name"],++        tool_config = {"tools": [self.transform_tool(tool) for tool in kwargs.get("tools", [])] if "tools" in kwargs else None}
Security
Enforces JSON schema validation for tools input to ensure data integrity and security.

File: aisuite/providers/aws_provider.py LNone-LNone

Suggestion: Add JSON schema validation for tools and input parameters to ensure security and data integrity.

--- Original
+++ Improved
@@ -1,4 +1,5 @@
 +    if "tools" in kwargs:
++        validate_tool_input(kwargs["tools"])  # Validate against a JSON schema
 +        tool_config = {
 +            "tools": [
 +                {
Enhancement
Introduces a decorator for error handling to simplify the function structure and improve consistency.

File: aisuite/providers/aws_provider.py LNone-LNone

Suggestion: Convert separate function calls and structures for error handling into a decorator for consistent error management.

--- Original
+++ Improved
@@ -1,4 +1,6 @@
-+    if "tools" in kwargs:
-+        tool_config = {
-+            "tools": [
-+                {++    @handle_errors
++    def handle_tool_usage(self, kwargs):
++        if "tools" in kwargs:
++            tool_config = {
++                "tools": [
++                    {

Comment on lines 32 to +175
# Handle Message objects
if msg.role == "tool":
converted_msg = {
"role": "user",
"content": [
{
"type": "tool_result",
"tool_use_id": msg.tool_call_id,
"content": msg.content,
}
],
}
converted_messages.append(converted_msg)
elif msg.role == "assistant" and msg.tool_calls:
# Handle Message objects with tool calls
content = []
if msg.content:
content.append({"type": "text", "text": msg.content})
for tool_call in msg.tool_calls:
content.append(
{
"type": "tool_use",
"id": tool_call.id,
"name": tool_call.function.name,
"input": json.loads(tool_call.function.arguments),
}
)
converted_messages.append({"role": "assistant", "content": content})
else:
converted_messages.append(
{"role": msg.role, "content": msg.content}
)

print(converted_messages)
response = self.client.messages.create(
model=model, system=system_message, messages=converted_messages, **kwargs
)
print(response)
return self.normalize_response(response)

def normalize_response(self, response):
"""Normalize the response from the Anthropic API to match OpenAI's response format."""
normalized_response = ChatCompletionResponse()
normalized_response.choices[0].message.content = response.content[0].text

# Map Anthropic stop_reason to OpenAI finish_reason
finish_reason_mapping = {
"end_turn": "stop",
"max_tokens": "length",
"tool_use": "tool_calls",
# Add more mappings as needed
}
normalized_response.choices[0].finish_reason = finish_reason_mapping.get(
response.stop_reason, "stop"
)

# Add usage information
normalized_response.usage = {
"prompt_tokens": response.usage.input_tokens,
"completion_tokens": response.usage.output_tokens,
"total_tokens": response.usage.input_tokens + response.usage.output_tokens,
}

# Check if the response contains tool usage
if response.stop_reason == "tool_use":
# Find the tool_use content
tool_call = next(
(content for content in response.content if content.type == "tool_use"),
None,
)

if tool_call:
function = Function(
name=tool_call.name, arguments=json.dumps(tool_call.input)
)
tool_call_obj = ChatCompletionMessageToolCall(
id=tool_call.id, function=function, type="function"
)
# Get the text content if any
text_content = next(
(
content.text
for content in response.content
if content.type == "text"
),
"",
)

message = Message(
content=text_content or None,
tool_calls=[tool_call_obj] if tool_call else None,
role="assistant",
refusal=None,
)
normalized_response.choices[0].message = message
return normalized_response

# Handle regular text response
message = Message(
content=response.content[0].text,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reduces code duplication and enhances maintainability by refactoring repetitive conversion code into a separate function.

Abstract the repetitive conversion logic into separate functions for clarity and reuse.

Suggested change
if "max_tokens" not in kwargs:
kwargs["max_tokens"] = DEFAULT_MAX_TOKENS
return self.normalize_response(
self.client.messages.create(
model=model, system=system_message, messages=messages, **kwargs
)
# Handle tool calls. Convert from OpenAI tool calls to Anthropic tool calls.
if "tools" in kwargs:
kwargs["tools"] = convert_openai_tools_to_anthropic(kwargs["tools"])
# Convert tool results from OpenAI format to Anthropic format
converted_messages = []
for msg in messages:
if isinstance(msg, dict):
if msg["role"] == "tool":
# Convert tool result message
converted_msg = {
"role": "user",
"content": [
{
"type": "tool_result",
"tool_use_id": msg["tool_call_id"],
"content": msg["content"],
}
],
}
converted_messages.append(converted_msg)
elif msg["role"] == "assistant" and "tool_calls" in msg:
# Handle assistant messages with tool calls
content = []
if msg.get("content"):
content.append({"type": "text", "text": msg["content"]})
for tool_call in msg["tool_calls"]:
content.append(
{
"type": "tool_use",
"id": tool_call["id"],
"name": tool_call["function"]["name"],
"input": json.loads(tool_call["function"]["arguments"]),
}
)
converted_messages.append({"role": "assistant", "content": content})
else:
# Keep other messages as is
converted_messages.append(
{"role": msg["role"], "content": msg["content"]}
)
else:
# Handle Message objects
if msg.role == "tool":
converted_msg = {
"role": "user",
"content": [
{
"type": "tool_result",
"tool_use_id": msg.tool_call_id,
"content": msg.content,
}
],
}
converted_messages.append(converted_msg)
elif msg.role == "assistant" and msg.tool_calls:
# Handle Message objects with tool calls
content = []
if msg.content:
content.append({"type": "text", "text": msg.content})
for tool_call in msg.tool_calls:
content.append(
{
"type": "tool_use",
"id": tool_call.id,
"name": tool_call.function.name,
"input": json.loads(tool_call.function.arguments),
}
)
converted_messages.append({"role": "assistant", "content": content})
else:
converted_messages.append(
{"role": msg.role, "content": msg.content}
)
print(converted_messages)
response = self.client.messages.create(
model=model, system=system_message, messages=converted_messages, **kwargs
)
print(response)
return self.normalize_response(response)
def normalize_response(self, response):
"""Normalize the response from the Anthropic API to match OpenAI's response format."""
normalized_response = ChatCompletionResponse()
normalized_response.choices[0].message.content = response.content[0].text
# Map Anthropic stop_reason to OpenAI finish_reason
finish_reason_mapping = {
"end_turn": "stop",
"max_tokens": "length",
"tool_use": "tool_calls",
# Add more mappings as needed
}
normalized_response.choices[0].finish_reason = finish_reason_mapping.get(
response.stop_reason, "stop"
)
# Add usage information
normalized_response.usage = {
"prompt_tokens": response.usage.input_tokens,
"completion_tokens": response.usage.output_tokens,
"total_tokens": response.usage.input_tokens + response.usage.output_tokens,
}
# Check if the response contains tool usage
if response.stop_reason == "tool_use":
# Find the tool_use content
tool_call = next(
(content for content in response.content if content.type == "tool_use"),
None,
)
if tool_call:
function = Function(
name=tool_call.name, arguments=json.dumps(tool_call.input)
)
tool_call_obj = ChatCompletionMessageToolCall(
id=tool_call.id, function=function, type="function"
)
# Get the text content if any
text_content = next(
(
content.text
for content in response.content
if content.type == "text"
),
"",
)
message = Message(
content=text_content or None,
tool_calls=[tool_call_obj] if tool_call else None,
role="assistant",
refusal=None,
)
normalized_response.choices[0].message = message
return normalized_response
# Handle regular text response
message = Message(
content=response.content[0].text,
+ converted_messages = self.refactor_conversion_logic(messages)

class Message:
def __init__(self):
self.content = None

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Utilize enums to enforce constraints on 'role' and type fields, enhancing robustness and maintain type safety.

Use Enumerations to enforce valid values and enhance type safety in role and type fields.

Suggested change
+ content: Optional[str]
+ tool_calls: Optional[list[ChatCompletionMessageToolCall]]
+ role: Optional[Literal["user", "assistant", "system"]] = Field(..., description="Role of the message sender", enum=["user", "assistant", "system"])
+ refusal: Optional[str]

PR Review Summary Celebratory GIF

Overall Review:

The PR seeks to enhance the message and tool calling functionality across multiple AI service providers like Anthropic and AWS. It introduces significant changes to the handling of message structures and API interactions, adding structured tool management and transformation processes. Notable changes include the refactoring of the Message class using pydantic.BaseModel and extensive modifications in handling APIs' specific requirements, potentially affecting the application's reliability and maintainability. It is critical to scrutinize these changes for potential logical errors, security risks, and their thorough testing given the complexity and broad impact of the modifications involved.


🔒 Security Analysis

1. [Blocker] The methods `transform_tool_call_to_openai` and JSON handling in APIs involve sensitive data parsing which requires strict schema validations or error handling to mitigate potential injection attacks or corruption of data formats.

Logical Error Analysis

2. [Blocker] In `AnthropicProvider`, `normalize_response` tries to handle different `stop_reasons` including `tool_use`. However, `tool_call_id` and method to retrieve `content` might not reliably parse the expected format every time. This can lead to unhandled exceptions or missing data.

Recommendations

Recommendation #1

Implement rigorous input validation and error handling around JSON operations and external data handling. Use secure practices such as using parameterized queries or templates when constructing requests or handling data. For instance:

def transform_tool_call_to_openai(self, response):
    if response.get("stopReason") != "tool_use":
        return None

    try:
        tool_calls = []
        for content in response["output"]["message"]["content"]:
            if "toolUse" in content:
                tool = content["toolUse"]
                tool_calls.append({
                    "type": "function",
                    "id": tool["toolUseId"],
                    "function": {
                        "name": tool["name"],
                        "arguments": json.dumps(tool["input"], cls=SecureJSONEncoder),
                    },
                })
    except json.JSONDecodeError as e:
        raise ValueError(f"JSON parsing error: {str(e)}")

    return {
        "role": "assistant",
        "content": None,
        "tool_calls": tool_calls,
        "refusal": None,
    }

In this code snippet, SecureJSONEncoder could be a custom JSONEncoder that sanitizes inputs or checks them against a schema to prevent JSON injection attacks. This ensures the integrity and security of data transformations and external interactions.

Recommendation #2

To address the potential inconsistencies in parsing tool_call_id and content, implement rigorous checks and transformations to ensure data conforms to the expected format consistently. Consider applying schema validation or more robust parsing especially when handling dynamic JSON content. Example modification:

def normalize_response(self, response):
    normalized_response = ChatCompletionResponse()
    finish_reason_mapping = {
        "end_turn": "stop",
        "max_tokens": "length",
        "tool_use": "tool_calls",
    }
    normalized_response.choices[0].finish_reason = finish_reason_mapping.get(response.stop_reason, "stop")

    # Add usage information
    normalized_response.usage = {
        "prompt_tokens": response.usage.input_tokens,
        "completion_tokens": response.usage.output_tokens,
        "total_tokens": response.usage.input_tokens + response.usage.output_tokens,
    }

    # Validate tool usage
    if response.stop_reason == "tool_use":
        tool_call = next((content for content in response.content if 'type' in content and content.type == "tool_use"), None)
        if tool_call and 'id' in tool_call:
            tool_call_id = tool_call.id
            function = Function(name=tool_call.name, arguments=json.dumps(tool_call.input))
            tool_call_obj = ChatCompletionMessageToolCall(id=tool_call_id, function=function, type="function")
            text_content = next((content.text for content in response.content if 'type' in content and content.type == "text"), "")
            message = Message(content=text_content or None, tool_calls=[tool_call_obj] if tool_call else None, role="assistant", refusal=None)
            normalized_response.choices[0].message = message
        else:
            raise ValueError("Expected 'tool_use' content is missing or malformed in API response.")

    return normalized_response
[Configure settings at: Archie AI - Automated PR Review]

PR Review Summary Celebratory GIF

Overall Review:

The PR introduces profound changes across the AISuite to enhance message and tool calling capabilities for various AI providers, leveraging structured models and APIs specific to providers like AWS and Anthropic. Main enhancements include significant modifications to the message classes, sophisticated JSON data handling, and the incorporation of a ToolManager to abstract and streamline tool calling operations. These adjustments are crucial, given their extensive impact on the tool calling and message management functionalities which are central to the integration flexibility and operational reliability of the AISuite.

Commit logs and file changes suggest iterative development and testing, focusing on incremental integration and ensuring compatibility with external APIs. This approach helps in isolating features for specific providers while maintaining a generalized framework that could be adapted for additional AI service providers as needed.


Logical Error Analysis

1. [Blocker] In `AnthropicProvider`, the method `normalize_response` improperly processes the `tool_call_id` and can lead to unhandled exceptions or missing data if the incoming format does not match expectations. This can destabilize the application's ability to process tool calls effectively.

🔒 Security Analysis

2. [Blocker] The methods `transform_tool_call_to_openai` involve sensitive data parsing which may expose the system to JSON injection attacks unless properly handled with schema validations or stringent input checks.

Recommendations

Recommendation #1

To enhance the robustness of tool_call_id parsing, modify the AnthropicProvider method to handle potential inconsistencies and ensure the data conforms to the expected format. Consider robust error checking or schema validations, especially when dealing with dynamic JSON content. Here's a sample modification:

def normalize_response(self, response):
    normalized_response = ChatCompletionResponse()
    finish_reason_mapping = {
        "end_turn": "stop",
        "max_tokens": "length",
        "tool_use": "tool_calls",
    }
    normalized_response.choices[0].finish_reason = finish_reason_mapping.get(response.stop_reason, "stop")
    
    normalized_response.usage = {
        "prompt_tokens": response.usage.input_tokens,
        "completion_tokens": response.usage.output_tokens,
        "total_tokens": response.usage.input_tokens + response.usage.output_tokens,
    }
    
    if response.stop_reason == "tool_use":
        tool_call = next((content for content in response.content if content.type == "tool_use"), None)
        if tool_call and 'id' in tool_call:
            function = Function(name=tool_call.name, arguments=json.dumps(tool_call.input))
            tool_call_obj = ChatCompletionMessageToolCall(
                id=tool_call['id'], function=function, type="function"
            )
            text_content = next(
                (content.text for content in response.content if content.type == "text"), ""
            )
            message = Message(
                content=text_content or None,
                tool_calls=[tool_call_obj] if tool_call else None,
                role="assistant",
                refusal=None
            )
            normalized_response.choices[0].message = message
        else:
            raise ValueError("Expected 'tool_use' content is missing or malformed in API response.")

    return normalized_response
Recommendation #2

Implement rigorous input validation and sanitization within transform_tool_call_to_openai. It's essential to apply secure practices such as using parameterized queries or prepared statements when constructing queries from JSON content. Here's a revised method example that includes security measures:

def transform_tool_call_to_openai(self, response):
    if response.get("stopReason") != "tool_use":
        return None
    
    try:
        tool_calls = []
        for content in response["output"]["message"]["content"]:
            if "toolUse" in content:
                tool = content["toolUse"]
                tool_calls.append({
                    "type": "function",
                    "id": tool["toolUseId"],
                    "function": {
                        "name": tool["name"],
                        "arguments": json.dumps(tool["input"], cls=SecureJSONEncoder),
                    },
                })
    except json.JSONDecodeError as e:
        raise ValueError(f"JSON parsing error: {str(e)}")
    
    return {
        "role": "assistant",
        "content": None,
        "tool_calls": tool_calls,
        "refusal": None,
    }

In the above snippet, SecureJSONEncoder should be a custom JSONEncoder designed to sanitize and validate inputs to ward off injection threats.

[Configure settings at: Archie AI - Automated PR Review]

@dev-archie-ai-code-explain-pr dev-archie-ai-code-explain-pr bot changed the title Introduce message handling enhancements using OpenAI and AWS formats Enhance message handling for various providers with new data structures Nov 27, 2024
@dev-archie-ai-code-explain-pr dev-archie-ai-code-explain-pr bot changed the title Enhance message handling for various providers with new data structures Enhance message structuring and add tool call handling across providers Nov 27, 2024
Copy link

PR Review Summary Celebratory GIF

Overall Review:

The PR introduces significant enhancements across multiple AI service providers within the aisuite, focusing on structured message handling and tool calling functionality. These changes span introducing new models with Pydantic, implementing a ToolManager, and adjusting APIs to these structures. The PR aims to establish a uniform message structuring system and enhance tool management, which are vital for maintaining the system's flexibility and robustness. The adoption of the pydantic.BaseModel increases type safety and data validation, reinforcing the backend's reliability and maintainability. The changes are extensive and touch on critical components that require careful validation to ensure they perform as expected without introducing regressions or vulnerabilities.


🔒 Security Analysis

1. [Blocker] The methods `transform_tool_call_to_openai` and JSON handling in APIs involve sensitive data parsing which may expose the system to JSON injection attacks unless properly handled with schema validations or stringent input checks.

Logical Error Analysis

2. [Blocker] In `AnthropicProvider`, `normalize_response` tries to handle different `stop_reasons` including `tool_use`. However, `tool_call_id` and method to retrieve `content` might not reliably parse the expected format every time, which can lead to unhandled exceptions or missing data.

Recommendations

Recommendation #1

To mitigate potential security vulnerabilities related to JSON parsing, fortify the data handling process with improved schema validation and thorough error handling. Integrate a custom JSON encoder to sanitize inputs before parsing and ensure functional test coverage:

class SecureJSONEncoder(json.JSONEncoder):
    def default(self, o):
        if isinstance(o, str):
            return o.replace('"', '\"').replace('<', '\<')
        return json.JSONEncoder.default(self, o)

def transform_tool_call_to_openai(self, response):
    if response.get("stopReason") != "tool_use":
        return None

    tool_calls = []
    try:
        for content in response["output"]["message"]["content"]:
            if "toolUse" in content:
                tool = content["toolUse"]
                secure_args = json.dumps(tool["input"], cls=SecureJSONEncoder)
                tool_calls.append({
                    "type": "function",
                    "id": tool["toolUseId"],
                    "function": {
                        "name": tool["name"],
                        "arguments": secure_args,
                    },
                })
    except json.JSONDecodeError as e:
        raise ValueError(f"JSON parsing error: {str(e)}")

    return {
        "role": "assistant",
        "content": None,
        "tool_calls": tool_calls,
        "refusal": None,
    }

Properly structure your error handling and testing to validate defense against common vulnerabilities in JSON processing.

Recommendation #2

Rework the normalize_response function with additional checks and validations to prevent failures due to format discrepancies. Implement a more robust handling of potential missing data:

def normalize_response(self, response):
    normalized_response = ChatCompletionResponse()
    finish_reason_mapping = {
        "end_turn": "stop",
        "max_tokens": "length",
        "tool_use": "tool_calls",
    }
    normalized_response.choices[0].finish_reason = finish_reason_mapping.get(response.stop_reason, "stop")

    normalized_response.usage = {
        "prompt_tokens": response.usage.input_tokens,
        "completion_tokens": response.usage.output_tokens,
        "total_tokens": response.usage.input_tokens + response.usage.output_tokens,
    }

    if response.stop_reason == "tool_use":
        tool_call = next((content for content in response.content if 'type' in content and content.type == "tool_use"), None)
        if tool_call and 'id' in tool_call:
            tool_call_id = tool_call.id
            function = Function(name=tool_call.name, arguments=json.dumps(tool_call.input))
            tool_call_obj = ChatCompletionMessageToolCall(id=tool_call_id, function=function, type="function")
            text_content = next(
                (content.text for content in response.content if 'type' in content and content.type == "text"), ""
            )
            message = Message(
                content=text_content or None,
                tool_calls=[tool_call_obj] if tool_call else None,
                role="assistant",
                refusal=None
            )
            normalized_response.choices[0].message = message
        else:
            raise ValueError("Expected 'tool_use' content is missing or malformed in API response.")

    return normalized_response

This modification ensures that all necessary identifiers and data types are checked before processing, improving the resilience of your application against data inconsistencies and related errors.

[Configure settings at: Archie AI - Automated PR Review]

Copy link

dev-archie-ai-code-explain-pr bot commented Dec 3, 2024

PR Code Suggestions Summary ✨

CategorySuggestion
Maintainability
Improve code readability by using descriptive variable names for complex data structures.

File: aisuite/providers/aws_provider.py L189-L207

Suggestion:

--- Original
+++ Improved
@@ -0,0 +1,13 @@
++    tool_calls = []
++    for content in response["output"]["message"]["content"]:
++        if "toolUse" in content:
++            tool = content["toolUse"]
++            tool_call_details = {
++                "type": "function",
++                "id": tool["toolUseId"],
++                "function": {
++                    "name": tool["name"],
++                    "arguments": json.dumps(tool["input"]),
++                },
++            }
++            tool_calls.append(tool_call_details)

Comment on lines +189 to +207
if response.get("stopReason") != "tool_use":
return None

tool_calls = []
for content in response["output"]["message"]["content"]:
if "toolUse" in content:
tool = content["toolUse"]
tool_calls.append(
{
"type": "function",
"id": tool["toolUseId"],
"function": {
"name": tool["name"],
"arguments": json.dumps(tool["input"]),
},
}
)

if not tool_calls:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improve code readability by using descriptive variable names for complex data structures.

Suggested change
if response.get("stopReason") != "tool_use":
return None
tool_calls = []
for content in response["output"]["message"]["content"]:
if "toolUse" in content:
tool = content["toolUse"]
tool_calls.append(
{
"type": "function",
"id": tool["toolUseId"],
"function": {
"name": tool["name"],
"arguments": json.dumps(tool["input"]),
},
}
)
if not tool_calls:
+ tool_calls = []
+ for content in response["output"]["message"]["content"]:
+ if "toolUse" in content:
+ tool = content["toolUse"]
+ tool_call_details = {
+ "type": "function",
+ "id": tool["toolUseId"],
+ "function": {
+ "name": tool["name"],
+ "arguments": json.dumps(tool["input"]),
+ },
+ }
+ tool_calls.append(tool_call_details)

@dev-archie-ai-code-explain-pr dev-archie-ai-code-explain-pr bot changed the title Enhance message structuring and add tool call handling across providers Integrate advanced message structuring and tool call conversion for various providers Dec 3, 2024
Copy link

PR Review Summary Celebratory GIF

Overall Review:

The presented PR aims to standardize message handling and introduce robust tool management for various AI providers within the aisuite. The proposed changes signify a substantial improvement in message structuring and API interaction uniformity. The adjustments stabilize and adapt the aisuite’s architecture to efficiently incorporate and translate between different provider formats. Refining message interaction through the Message class utilizing Pydantic, and tool management via the ToolManager class, prepares the system for scalable integration and enhanced validation mechanisms. However, extensive revisions like these necessitate vigilant checks for logical coherence, security adherence, and extensive testing to ensure stability and functionality across diverse operating conditions and inputs.


Logical Error Analysis

1. [Blocker] In `AnthropicProvider`, `normalize_response` tries to handle different `stop_reason`s including `tool_use`. However, `tool_call_id` and method to retrieve `content` might not reliably parse the expected format every time. This can lead to unhandled exceptions or missing data.

🔒 Security Analysis

2. [Blocker] The methods `transform_tool_call_to_openai` and JSON handling in APIs involve sensitive data parsing which requires strict schema validations or error handling to mitigate potential injection attacks or corruption of data formats.

3. [Consider] Implementation involves extensive JSON parsing and generation interacting with external API responses, which could pose a security risk for injection attacks or data corruption.

Recommendations

Recommendation #1

To address the potential inconsistencies in parsing tool_call_id and content, please add rigorous checks and transformations in the AnthropicProvider normalize_response method to ensure the data consistently conforms to the expected format. Consider using a conditional logic for validating each field based on its presence before processing it:

def normalize_response(self, response):
    normalized_response = ChatCompletionResponse()
    finish_reason_mapping = {
        "end_turn": "stop",
        "max_tokens": "length",
        "tool_use": "tool_calls",
    }
    normalized_response.choices[0].finish_reason = finish_reason_mapping.get(response.stop_reason, "stop")
   
    # Add usage information
    normalized_response.usage = {
        "prompt_tokens": response.usage.input_tokens,
        "completion_tokens": response.usage.output_tokens,
        "total_tokens": response.usage.input_tokens + response.usage.output_tokens,
    }
   
    # Check if the response contains tool usage and validate format
    if response.stop_reason == "tool_use":
        tool_call = next((content for content in response.content if 'type' in content and content.type == "tool_use"), None)
        if tool_call and 'id' in tool_call:
            tool_call_id = tool_call.id
            function = Function(name=tool_call.name, arguments=json.dumps(tool_call.input))
            tool_call_obj = ChatCompletionMessageToolCall(id=tool_call_id, function=function, type="function")
            text_content = next((content.text for content in response.content if 'type' in content and content.type == "text"), "")
            message = Message(content=text_content or None, tool_calls=[tool_call_obj] if tool_call else None, role="assistant", refusal=None)
            normalized_response.choices[0].message = message
        else:
            raise ValueError("Expected 'tool_use' content is missing or malformed in API response.")
   
    return normalized_response
Recommendation #2

It's vital to implement rigorous input validation and error handling around JSON operations and external data handling. Enhance the security of JSON parsing within the transform_tool_call_to_openai method by implementing a secure JSONEncoder to sanitize input data:

def transform_tool_call_to_openai(self, response):
    if response.get("stopReason") != "tool_use":
        return None

    try:
        tool_calls = []
        for content in response["output"]["message"]["content"]:
            if "toolUse" in content:
                tool = content["toolUse"]
                tool_calls.append({
                    "type": "function",
                    "id": tool["toolUseId"],
                    "function": {
                        "name": tool["name"],
                        "arguments": json.dumps(tool["input"], cls=SecureJSONEncoder),
                    },
                })
    except json.JSONDecodeError as e:
        raise ValueError(f"JSON parsing error: {str(e)}")

    return {
        "role": "assistant",
        "content": None,
        "tool_calls": tool_calls,
        "refusal": None,
    }

This function uses a custom SecureJSONEncoder that should be implemented to sanitize the inputs to prevent JSON injection attacks.

Recommendation #3

Implement strict content validation and utilize secure parsing methods to handle JSON data:

from pydantic import ValidationError

def secure_json_parse(input_data: str):
    try:
        result = json.loads(input_data)
        # Implement additional checks here to validate schema if necessary
        return result
    except json.JSONDecodeError as e:
        raise ValidationError(f"Invalid JSON data: {e}")

# Usage within any data handling context
try:
    safe_data = secure_json_parse(unsafe_data)
except ValidationError as ve:
    logger.error(f"Data validation error: {ve}")
    # Handle the error accordingly

Review and augment the use of Pydantic validators, which enforce all data meets predefined schemas extensively.

[Configure settings at: Archie AI - Automated PR Review]

Copy link

dev-archie-ai-code-explain-pr bot commented Dec 3, 2024

PR Code Suggestions Summary ✨

CategorySuggestion
Maintainability
Refactor repetitive message transformation logic into a helper function within the AWS provider.

File: aisuite/providers/aws_provider.py L70-L94

Suggestion: Abstract repeated logic into helper functions within the AWS provider to make the method more concise and maintainable.

--- Original
+++ Improved
@@ -1,22 +1,5 @@
 +        for message in messages:
 +            message_dict = (
 +                message.model_dump() if hasattr(message, "model_dump") else message
 +            )
-+            if message_dict["role"] != "system":
-+                if message_dict["role"] == "tool":
-+                    bedrock_message = self.transform_tool_result_to_bedrock(
-+                        message_dict
-+                    )
-+                    if bedrock_message:
-+                        formatted_messages.append(bedrock_message)
-+                elif message_dict["role"] == "assistant":
-+                    bedrock_message = self.transform_assistant_to_bedrock(message_dict)
-+                    if bedrock_message:
-+                        formatted_messages.append(bedrock_message)
-+                else:
-+                    formatted_messages.append(
-+                        {
-+                            "role": message_dict["role"],
-+                            "content": [{"text": message_dict["content"]}],
-+                        }
-+                    )++            self.append_formatted_message(message_dict, formatted_messages)

@@ -46,6 +70,7 @@ def __init__(self, **config):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor repetitive message transformation logic into a helper function within the AWS provider.

Abstract repeated logic into helper functions within the AWS provider to make the method more concise and maintainable.

Suggested change
+ for message in messages:
+ message_dict = (
+ message.model_dump() if hasattr(message, "model_dump") else message
+ )
+ self.append_formatted_message(message_dict, formatted_messages)

@dev-archie-ai-code-explain-pr dev-archie-ai-code-explain-pr bot changed the title Integrate advanced message structuring and tool call conversion for various providers Enhance message structure and adapt tool call conversions across AI providers Dec 3, 2024
Copy link

PR Review Summary Celebratory GIF

Overall Review:

The PR introduces extensive modifications aimed at enhancing message structuring and tool management across various AI providers within the aisuite. It centralizes around augmenting the Message class to include robust data validation using Pydantic, implementing a comprehensive ToolManager, and adapting provider-specific message transformations. This aims to standardize message handling, streamline tool calls, and ensure backend integrity. While the PR strives to enhance interoperability and functionality, the complexity and breadth of the changes necessitate a thorough review to ensure accuracy, performance, and security.


Logical Error Analysis

1. [Blocker] The method `normalize_response` in `AnthropicProvider` handles different `stop_reasons`, including `tool_use`. However, reliance on potentially inconsistent `tool_call_id` and method to retrieve `content` may result in unhandled exceptions or missing data when the data is not formatted as expected. This can cause application instability or incorrect tool call data handling.

🔒 Security Analysis

2. [Blocker] The methods `transform_tool_call_to_openai` involve JSON parsing operations that handle potentially sensitive data which, if improperly managed, could expose the system to risks such as data corruption or injection attacks.

Recommendations

Recommendation #1

To address the potential inconsistencies in parsing tool_call_id and content, modify the normalize_response method to include stricter checks and error handling. Here's a proposed adjustment to improve reliability:

def normalize_response(self, response):
    normalized_response = ChatCompletionResponse()
    finish_reason_mapping = {
        "end_turn": "stop",
        "max_tokens": "length",
        "tool_use": "tool_calls",
    }
    normalized_response.choices[0].finish_reason = finish_reason_mapping.get(response.stop_reason, "default_follow_up_action")

    # Validate and parse tool use behavior
    if response.stop_reason == "tool_use":
        tool_call = next((item for item in response.content if item.type == "tool_use"), None)
        if tool_call and 'id' in tool_call:
            tool_id = tool_call.id
            function = Function(
                name=tool_call.name, 
                arguments=json.dumps(to.getExternalStorageDirectory().getJsonfromcall())),
            )
            tool_call_obj = ChatCompletionMessageToolCall(id=tool_id, function=function, type="function")
            text_content = next(
                (item.text for item in response.content if item.type == "text"), 
                ""
            )
            message = Message(
                content=text_content or None,
                tool_calls=[tool_call_obj] if tool_call else None,
                role="assistant",
                refusal=None
            )
            normalized_response.choices[0].message = message
        else:
            raise ValueError("Expected 'tool_use' content is missing or malformed in API response.")
    
    return normalized_response

This modification ensures more rigorous validation and error handling to better accommodate variations in stop_reasons and associated data structures.

Recommendation #2

Implement rigorous input validation and error handling mechanisms around JSON operations within the transform_tool_call_to_openai method to reduce security risks. Consider introducing schema validations and sanitizing inputs prior to processing. Here's a potential implementation:

def transform_tool_call_to_openai(self, response):
    if response.get("stopReason") != "tool_use":
        return None
    
    try:
        tool_calls = []
        for content in response["output"]["message"]["content"]:
            if "toolUse" in content:
                tool = content["toolUse"]
                secure_args = json.dumps(tool["input"], cls=SecureJSONEncoder)
                tool_calls.append({
                    "type": "function",
                    "id": tool["toolUseId"],
                    "function": {
                        "name": tool["name"],
                        "arguments": secure_args,
                    },
                })
    except json.JSONDecodeError as e:
        raise ValueError(f"JSON parsing error: {str(e)}")

    return {
        "role": "assistant",
        "content": None,
        "tool_calls": tool_calls,
        "refusal": None,
    }

Ensure you define or integrate a SecureJSONEncoder that specifically handles sanitization and validation against a defined schema to prevent injection attacks.

[Configure settings at: Archie AI - Automated PR Review]

Copy link

dev-archie-ai-code-explain-pr bot commented Dec 4, 2024

PR Code Suggestions Summary ✨

CategorySuggestion
Possible issue
Add null checks to tool call properties to avoid access errors.

File: aisuite/providers/anthropic_provider.py L133-L136

Suggestion: Handling conversion for tools should include a null check before accessing the tool's name and arguments to prevent the application from crashing.

--- Original
+++ Improved
@@ -1,4 +1,4 @@
 +                                "type": "tool_use",
 +                                "id": tool_call["id"],
-+                                "name": tool_call["function"]["name"],
-+                                "input": json.loads(tool_call["function"]["arguments"]),++                                "name": tool_call.get("function", {}).get("name", ""),
++                                "input": json.loads(tool_call.get("function", {}).get("arguments", "{}")),
Enhancement
Restrict print statements with a debug mode check for cleaner log management.

File: aisuite/providers/aws_provider.py L87-L89

Suggestion: Use a configurable logging level for debug prints could help in managing what is logged in production versus in development environments.

--- Original
+++ Improved
@@ -1,2 +1,3 @@
-+        print(f"Tool config: {tool_config}")
-+        print(f"Received tools specification: {kwargs['tools']}")++        if self.debug:
++            print(f"Tool config: {tool_config}")
++            print(f"Received tools specification: {kwargs['tools']}")

Comment on lines +133 to +136
normalized_response.usage = {
"prompt_tokens": response.usage.input_tokens,
"completion_tokens": response.usage.output_tokens,
"total_tokens": response.usage.input_tokens + response.usage.output_tokens,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add null checks to tool call properties to avoid access errors.

Handling conversion for tools should include a null check before accessing the tool's name and arguments to prevent the application from crashing.

Suggested change
normalized_response.usage = {
"prompt_tokens": response.usage.input_tokens,
"completion_tokens": response.usage.output_tokens,
"total_tokens": response.usage.input_tokens + response.usage.output_tokens,
+ "type": "tool_use",
+ "id": tool_call["id"],
+ "name": tool_call.get("function", {}).get("name", ""),
+ "input": json.loads(tool_call.get("function", {}).get("arguments", "{}")),

@dev-archie-ai-code-explain-pr dev-archie-ai-code-explain-pr bot changed the title Enhance message structure and adapt tool call conversions across AI providers Refactor message structure and tool call API integration for various AI providers Dec 4, 2024
Copy link

PR Review Summary Celebratory GIF

Overall Review:

The PR introduces comprehensive enhancements in message structuring and tool management across several AI service providers, embodying significant improvements and adaptations to the aisuite's messaging and tool handling systems. Changes span from the introduction of a robust Message class leveraging Pydantic for data validation, to the implementation of a ToolManager that facilitates standardized tool interactions. With modifications touching essential aspects such as AI provider-specific message formatting and API response handling, the implications of these changes are substantial, aiming at increased flexibility, reliability, and seamless integration of service providers.


Logical Error Analysis

1. [Blocker] The methods `transform_tool_call_to_openai` in `AwsProvider` don't check correctly for potential mismatches or misconfigurations in tool type handling. This might lead to runtime errors or malfunctioning in processing tool calls.

🔒 Security Analysis

2. [Blocker] The methods `transform_tool_call_to_openai` and JSON handling in APIs involve sensitive data parsing, which requires strict schema validations or error handling to mitigate potential injection attacks or corruption of data formats.

Recommendations

Recommendation #1

Modify the transform_tool_call_to_openai method to include checks and valid type handling to ensure tool types are processed properly or logged if unmatched:

def transform_tool_call_to_openai(self, response):
    if response.get("stopReason") != "tool_use":
        return None

    tool_calls = []
    for content in response["output"]["message"]["content"]:
        if "toolUse" in content and content["toolUse"]["type"] == "function":
            tool = content["toolUse"]
            tool_calls.append({
                "type": "function",
                "id": tool["toolUseId"],
                "function": {
                    "name": tool["name"],
                    "arguments": json.dumps(tool["input"]),
                },
            })
        else:
            logging.warning(f"Unsupported tool type encountered: {content.get('toolUse', {}).get('type')}")
    return {"role": "assistant", "content": None, "tool_calls": tool_calls, "refusal": None}

Ensure to add proper logging configuration if not existing.

Recommendation #2

Enhance security by implementing rigorous input validation and error handling:

def transform_tool_call_to_openai(self, response):
    if response.get("stopReason") != "tool_use":
        return None

    tool_calls = []
    try:
        for content in response["output"]["message"]["content"]:
            if "toolUse" in content:
                tool = content["toolUse"]
                validated_input = json.loads(tool["input"], cls=SecureJSONLoader)  # Custom class to validate JSON input
                tool_calls.append({
                    "type": "function",
                    "id": tool["toolUseId"],
                    "function": {
                        "name": tool["name"],
                        "arguments": json.dumps(validated_input),
                    },
                })
    except json.JSONDecodeError as e:
        logging.error(f"JSON parsing error in tool call: {str(e)}")
        return None

    return {"role": "assistant", "content": None, "tool_calls": tool_calls, "refusal": None}

Create a SecureJSONLoader class responsible for validating and sanitizing JSON input to prevent potential security threats.

[Configure settings at: Archie AI - Automated PR Review]

Copy link

dev-archie-ai-code-explain-pr bot commented Dec 4, 2024

PR Code Suggestions Summary ✨

CategorySuggestion
Enhancement
Streamline and correct tool configuration handling in AWS provider class to remove error space and improve readability.

File: aisuite/providers/aws_provider.py LNone-LNone

Suggestion: Optimize tool configuration handling to reduce potential error spaces and prevent configuration repetition errors.

--- Original
+++ Improved
@@ -1,19 +1,16 @@
 +        # Handle tools if present in kwargs
-+        tool_config = None
 +        if "tools" in kwargs:
 +            tool_config = {
 +                "tools": [
 +                    {
 +                        "toolSpec": {
 +                            "name": tool["function"]["name"],
 +                            "description": tool["function"].get("description", " "),
 +                            "inputSchema": {"json": tool["function"]["parameters"]},
 +                        }
 +                    }
-+                    for tool in kwargs["tools"]
++                    for tool in kwargs.pop("tools")  # removes the tools key and returns its value
 +                ]
 +            }
 +            print(f"Tool config: {tool_config}")
-+            print(f"Received tools specification: {kwargs['tools']}")
-+            # Remove tools from kwargs since we're handling it separately
-+            del kwargs["tools"]++            print(f"Received tools specification: {tool_config['tools']}")

@dev-archie-ai-code-explain-pr dev-archie-ai-code-explain-pr bot changed the title Refactor message structure and tool call API integration for various AI providers Enhanced Message Structure and Integration with AI Provider APIs for Tool Calls Dec 4, 2024
Copy link

PR Review Summary Celebratory GIF

Overall Review:

The PR implements substantial enhancements aimed at upgrading AI provider integrations and message handling within the aisuite by introducing a structured Message class and a ToolManager. These implementations are intended to streamline interactions and data validation across different AI services. While the efforts to standardize the message formatting, tool handling, and improve the overall integration with multiple AI providers are commendable, such changes bring complexities that need thorough scrutiny for potential bugs, security risks, and adequate test coverages. The changes are expansive and touch critical functional components of the application, which requires careful inspection to ensure the introduced functionalities do not disrupt the existing system's operations.


Logical Error Analysis

1. [Blocker] In `AnthropicProvider`, `normalize_response` tries to handle different `stop_reasons` including `tool_use`. However, `tool_call_id` and method to retrieve `content` might not consistently parse the expected format, leading to potential unhandled exceptions or missing data. This is crucial as it can destabilize the application's ability to process tool calls reliably.

🔒 Security Analysis

2. [Blocker] The methods `transform_tool_call_to_openai` and JSON handling in APIs involve sensitive data parsing which requires strict schema validations or error handling to mitigate potential injection attacks or corruption of data formats. Improper handling of JSON parsing can introduce serious security vulnerabilities such as data corruption or injection attacks.

Recommendations

Recommendation #1

To address this issue, add more rigorous validation and error checking in the normalize_response method of AnthropicProvider. Here's a recommended snippet:

def normalize_response(self, response):
    normalized_response = ChatCompletionResponse()
    finish_reason_mapping = {
        "end_turn": "stop",
        "max_tokens": "length",
        "tool_use": "tool_calls",
    }
    normalized_response.choices[0].finish_reason = finish_reason_mapping.get(response.stop_reason, "stop")

    # Validate tool usage
    if response.stop_reason == "tool_use":
        tool_call = next((content for content in response.content if 'type' in content and content.type == "tool_use"), None)
        if tool_call and 'id' in tool_call:
            tool_call_id = tool_call.id
            function = Function(name=tool_call.name, arguments=json.dumps(tool_call.input))
            tool_call_obj = ChatCompletionMessageToolCall(id=tool_call_id, function=function, type="function")
            text_content = next((content.text for content in response.content if 'type' in content and content.type == "text"), "")
            message = Message(content=text_content or None, tool_calls=[tool_call_obj] if tool_call else None, role="assistant", refusal=None)
            normalized_response.choices[0].message = message
        else:
            raise ValueError("Expected 'tool_use' content is missing or malformed in API response.")

    return normalized_response

This modification will make sure each part of the data is properly checked and parsed to prevent failures due to unexpected format changes or missing data.

Recommendation #2

Implement rigorous input validation and error handling around JSON operations within transform_tool_call_to_openai. Make use of secure practices such as parameterized queries or strict schema validations when processing JSON data. Here's a recommended implementation that incorporates a custom JSON encoder:

def transform_tool_call_to_openai(self, response):
    if response.get("stopReason") != "tool_use":
        return None

    tool_calls = []
    try:
        for content in response["output"]["message"]["content"]:
            if "toolUse" in content:
                tool = content["toolUse"]
                tool_calls.append({
                    "type": "function",
                    "id": tool["toolUseId"],
                    "function": {
                        "name": tool["name"],
                        "arguments": json.dumps(tool["input"], cls=SecureJSONEncoder),
                    },
                })
    except json.JSONDecodeError as e:
        raise ValueError(f"JSON parsing error: {str(e)}")

    return {
        "role": "assistant",
        "content": None,
        "tool_calls": tool_calls,
        "refusal": None,
    }

Ensure the SecureJSONEncoder is properly implemented to sanitize and validate inputs effectively to prevent JSON injection.

[Configure settings at: Archie AI - Automated PR Review]

Copy link

dev-archie-ai-code-explain-pr bot commented Dec 4, 2024

PR Code Suggestions Summary ✨

CategorySuggestion
Possible issue
Improves robustness by adding error handling around JSON parsing to prevent crashes from malformed JSON strings.

File: aisuite/providers/anthropic_provider.py LNone-LNone

Suggestion: To ensure optimal and uniform handling of potential erroneous states or exceptions during JSON operations, use try-except blocks strategically where the conversion could fail.

--- Original
+++ Improved
@@ -1 +1,4 @@
-+                                "input": json.loads(tool_call["function"]["arguments"]),+try:
++                                "input": json.loads(tool_call["function"]["arguments"]),
+except json.JSONDecodeError:
++                                "input": {"error": "Invalid JSON format"}
Enhancement
Enhances error handling by capturing and logging exceptions during API calls.

File: aisuite/providers/aws_provider.py LNone-LNone

Suggestion: Implement a unified error handling strategy to manage and log exceptions effectively, which can greatly improve the platform's reliability and ease of debugging.

--- Original
+++ Improved
@@ -1,8 +1,12 @@
-response = self.client.converse(
+try:
+    response = self.client.converse(
 +            modelId=model,  # baseModelId or provisionedModelArn
 +            messages=formatted_messages,
 +            system=system_message,
 +            inferenceConfig=inference_config,
 +            additionalModelRequestFields=additional_model_request_fields,
 +            toolConfig=tool_config,
-+        )++        )
+except Exception as e:
++    print(f"Error during converse API call: {str(e)}")
++    response = {}

@dev-archie-ai-code-explain-pr dev-archie-ai-code-explain-pr bot changed the title Enhanced Message Structure and Integration with AI Provider APIs for Tool Calls Refine Message model and encapsulate tool calls for AI provider integration Dec 4, 2024
Copy link

PR Review Summary Celebratory GIF

Overall Review:

This PR introduces a robust framework for interacting with different AI providers, enhances message structuring using Pydantic models, and centralizes tool management through a new ToolManager class. The changes are aimed at standardizing and improving the interactivity and functional capability of the suite with external tools. While the adaptation and restructuring are extensive, ensuring compatibility with multiple API mappings and handling complex data transformations, these modifications bear risks of introducing logical errors, security vulnerabilities, and might affect the existing functionalities if not properly validated and tested.


🔒 Security Analysis

1. [Blocker] The methods `transform_tool_call_to_openai` involve sensitive data parsing which may expose the system to JSON injection attacks unless properly handled with schema validations or stringent input checks.

Logical Error Analysis

2. [Blocker] In `AnthropicProvider`, `normalize_response` tries to handle different `stop_reasons` including `tool_use`. However, `tool_call_id` and method to retrieve `content` might not reliably parse the expected format every time. This can lead to unhandled exceptions or missing data.

Recommendations

Recommendation #1

To mitigate these security risks, it is recommended to implement rigorous input validations, such as schema checks, and employ JSON parsing with error handling to detect and reject malformed inputs effectively. Example modification for the parsing logic using secure practices could be implemented as follows:

from json import JSONDecodeError

def transform_tool_call_to_openai(self, response):
    if response.get("stopReason") != "tool_use":
        return None

    try:
        tool_calls = []
        for content in response["output"]["message"]["content"]:
            if "toolUse" in content:
                tool = content["toolUse"]
                tool_calls.append({
                    "type": "function",
                    "id": tool["toolUseId"],
                    "function": {
                        "name": tool["name"],
                        "arguments": json.dumps(tool["input"], cls=SecureJSONEncoder),
                    },
                })
    except JSONDecodeError as error:
        raise ValueError(f"Error parsing JSON data: {error}")
    
    return {
        "role": "assistant",
        "content": None,
        "tool_calls": tool_calls,
        "refusal": None,
    }

Ensure defining a SecureJSONEncoder that overrides the default() method to sanitize input data against potential security threats such as injections.

Recommendation #2

It's critical to address this issue by refining the error handling and data extraction logic to prevent application failures. Recommended changes include strict checks on data existence and proper fallbacks in case of missing information:

def normalize_response(self, response):
    normalized_response = ChatCompletionResponse()
    finish_reason_mapping = {
        "end_turn": "stop",
        "max_tokens": "length",
        "tool_use": "tool_calls"
    }
    normalized_response.choices[0].finish_reason = finish_reason_mapping.get(response.stop_reason, "stop")
    
    # Ensure tool usage information is validated and parsed correctly
    if response.stop_reason == "tool_use":
        tool_call = next((item for item in response.content if item.type == "tool_use"), None)
        if tool_call and hasattr(tool_call, 'id'):
            function = Function(name=tool_call.name, arguments=json.dumps(tool_call.input))
            tool_call_obj = ChatCompletionMessageToolCall(
                id=tool_call.id, function=function, type="function")
            text_content = next(
                (item.text for item in response.content if item.type == "text"), 
                ""
            )
            message = Message(
                content=text_content or None,
                tool_calls=[tool_call_obj] if tool_call else None,
                role="assistant",
                refusal=None
            )
            normalized_response.choices[0].message = message
        else:
            raise ValueError("Expected 'tool_use' content is missing or malformed in API response.")
    
    return normalized_response

Implement better validation checks for tool_call and its content existence to ensure consistent response parsing.

[Configure settings at: Archie AI - Automated PR Review]

Copy link

🤔 Analyzing Code for Improvements

Celebratory GIF

Please wait while I analyze the code changes and suggest improvements. This may take a few moments.


This comment will be updated with suggested code improvements shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants