Skip to content

Add support for OpenRouter as an embedding provider#2452

Closed
devin-ai-integration[bot] wants to merge 3 commits intomainfrom
devin/1742799804-fix-issue-2451
Closed

Add support for OpenRouter as an embedding provider#2452
devin-ai-integration[bot] wants to merge 3 commits intomainfrom
devin/1742799804-fix-issue-2451

Conversation

@devin-ai-integration
Copy link
Contributor

Add support for OpenRouter as an embedding provider

Fixes #2451

Description

This PR adds support for OpenRouter as an embedding provider in CrewAI. Users can now configure their agents to use OpenRouter for embeddings.

Usage Example

agent = Agent(
    config=config,
    verbose=True,
    knowledge_sources=[text_source],
    llm=llm,
    embedder={
        "provider": "openrouter",
        "config": {
            "model": "your-model",
            "api_key": "your-api-key",
        },
    },
)

Testing

  • Added a unit test for the OpenRouter embedding provider
  • Ran existing tests to ensure nothing was broken

Link to Devin run: https://app.devin.ai/sessions/90f238089f664da2ba9a4bd79c061e3e
Requested by: Joe Moura (joao@crewai.com)

Co-Authored-By: Joe Moura <joao@crewai.com>
@devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add "(aside)" to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Co-Authored-By: Joe Moura <joao@crewai.com>
@joaomdmoura
Copy link
Collaborator

Disclaimer: This review was made by a crew of AI Agents.

Code Review Comment for PR #2452

Overview

This pull request introduces OpenRouter as an embedding provider by extending the EmbeddingConfigurator class, alongside the necessary updates to configuration support and corresponding test cases. Below, I outline the positive aspects, identified issues, and recommendations for further improvements.

Positive Aspects

  • Clean Integration: The integration of the OpenRouter provider is well executed, maintaining the architecture of existing configuration mapping.
  • Consistent Implementation: The addition follows a consistent pattern with other providers, demonstrating adherence to prior coding practices.
  • Environment Variable Fallback: The fallback mechanism for the API key enhances flexibility and usability for end-users.

Issues and Recommendations

1. Missing Type Hints

The _configure_openrouter method currently lacks type hints, which affects consistency with Python's typing practices. Adding type annotations will improve code readability and maintainability.

@staticmethod
def _configure_openrouter(
    config: Dict[str, Any],
    model_name: str
) -> OpenAIEmbeddingFunction:

2. Error Handling Improvement

Incorporating error handling for cases where the API key is absent is crucial. A clear error message will assist users in troubleshooting issues effectively.

@staticmethod
def _configure_openrouter(config: Dict[str, Any], model_name: str) -> OpenAIEmbeddingFunction:
    api_key = config.get("api_key") or os.getenv("OPENROUTER_API_KEY")
    if not api_key:
        raise ValueError("OpenRouter API key must be provided either in config or OPENROUTER_API_KEY environment variable")

3. Documentation

Adding a docstring to the _configure_openrouter method will clarify its purpose, parameters, and return types, which is essential for maintainability.

@staticmethod
def _configure_openrouter(config: Dict[str, Any], model_name: str) -> OpenAIEmbeddingFunction:
    """
    Configure OpenRouter embedding provider.
    
    Args:
        config (Dict[str, Any]): Configuration dictionary containing the API key and optional settings.
        model_name (str): Name of the embedding model to use.
        
    Returns:
        OpenAIEmbeddingFunction: Configured OpenRouter embedding function.
        
    Raises:
        ValueError: If the API key is not provided in the config or environment.
    """

4. Test Coverage Enhancement

To ensure thorough validation of the OpenRouter functionality, additional test cases should be created to cover scenarios involving environment variables.

def test_openrouter_embedder_configuration_with_env_var():
    configurator = EmbeddingConfigurator()
    mock_openai_embedding = MagicMock()
    
    # Test with API key from environment variable
    with patch.dict(os.environ, {"OPENROUTER_API_KEY": "env-key"}), \
         patch("chromadb.utils.embedding_functions.openai_embedding_function.OpenAIEmbeddingFunction",
               return_value=mock_openai_embedding) as mock_embedder:
        
        embedder_config = {
            "provider": "openrouter",
            "config": {
                "model": "test-model",
            },
        }
        
        result = configurator.configure_embedder(embedder_config)
        
        assert result == mock_openai_embedding
        mock_embedder.assert_called_once_with(
            api_key="env-key",
            api_base="https://openrouter.ai/api/v1",
            model_name="test-model",
        )

5. Error Case Testing

An additional test case should address situations with missing API keys to confirm that relevant exceptions are raised appropriately.

def test_openrouter_embedder_configuration_missing_api_key():
    configurator = EmbeddingConfigurator()
    
    embedder_config = {
        "provider": "openrouter",
        "config": {
            "model": "test-model",
        },
    }
    
    with pytest.raises(ValueError, match="OpenRouter API key must be provided"):
        configurator.configure_embedder(embedder_config)

Historical Context

It would be beneficial to reference previous PRs that involved similar integration efforts, as they can provide a framework for best practices and lessons learned. A search for related PRs that modified similar configurations or functionality can shed light on consistency across the codebase and further solidify the integration of OpenRouter.

If you would like, I can perform a search for related pull requests to provide more context and references for future development.

Summary of Recommendations

  1. Add type hints to the OpenRouter configuration method for clarity and consistency.
  2. Implement robust error handling for cases where the API key is not present.
  3. Enhance method documentation to clarify function usage and expected behavior.
  4. Expand test coverage to include various configuration scenarios and error handling tests.
  5. Maintain alignment with existing implementation patterns for easier future modifications.

Implementing these recommendations will significantly increase code reliability, maintainability, and usability while ensuring comprehensive validation of the embedding provider integration. Thank you for your contributions, and I look forward to seeing these enhancements!

…, and tests

Co-Authored-By: Joe Moura <joao@crewai.com>
@lucasgomide
Copy link
Contributor

OpenRouter does not support embedding models

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants

Comments