Conversation
This commit introduces support for OpenAI and Anthropic as LLM providers, enhances model selection flexibility, and refactors core components. Key changes include: - Added `OpenAIProvider` and `AnthropicProvider` to integrate with their respective APIs. - Updated `pyproject.toml` with `openai` and `anthropic` optional dependencies. - Enhanced `conf.py` to allow custom model strings in the format "provider/model-name" via the `get_provider_and_model_name` function. - Updated the `ModelName` enum to replace `openai_gpt_3_5_turbo` with `openai_gpt_5`, and updated Anthropic models to `claude-4-sonet` and `claude-4.5-sonet`. - Refactored the `get_provider` factory function in `llm_provider.py` to use a more maintainable dictionary mapping. - Updated all `README.md` files to document the new providers, API keys, and the custom model name feature.
Summary of ChangesHello @leo-gan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request successfully adds support for new LLM providers like OpenAI and Anthropic, and refactors the model handling to be more flexible. The code is generally well-structured. However, I've identified a critical issue in the Anthropic provider implementation related to streaming, a high-severity issue where a new parameter is not being passed, and a few medium-severity issues to improve consistency and user experience. Please see the detailed comments for suggestions.
| if isinstance(max_output_tokens, int) and max_output_tokens > 0 | ||
| else DEFAULT_CHARACTERS_TO_ANONYMIZE // 4, | ||
| messages=[{"role": "user", "content": prompt}], | ||
| stream=True, |
There was a problem hiding this comment.
The Anthropic API is called with stream=True, but the response is handled as a non-streaming response. When streaming, client.messages.create returns a Stream object that needs to be iterated over to assemble the full response. The current implementation (response.content[0].text) will fail because a Stream object does not have a .content attribute.
To fix this, you should either implement the logic to handle a streaming response or, more simply, set stream=False to receive the complete message object at once.
| stream=True, | |
| stream=False, |
| raw_text = provider.call(prompt, model_enum.value) | ||
| provider_name, actual_model_name = get_provider_and_model_name(model_name) | ||
| provider = get_provider(provider_name) | ||
| raw_text = provider.call(prompt, actual_model_name) |
There was a problem hiding this comment.
The LLMProvider.call method was updated to accept a max_output_tokens parameter, but it's not being passed here. This makes the feature non-functional for providers like Anthropic that rely on it, as they will always use a default fallback. You should estimate a reasonable maximum for output tokens based on the input text length and pass it to the provider. A common heuristic is a fraction of the input length, for example len(text) // 4.
| raw_text = provider.call(prompt, actual_model_name) | |
| max_output_tokens = len(text) // 4 | |
| raw_text = provider.call(prompt, actual_model_name, max_output_tokens=max_output_tokens) |
| provider_name, _ = get_provider_and_model_name(model_name) | ||
| if provider_name == "google": | ||
| if "gemini" in model_name and not os.getenv("GOOGLE_API_KEY"): | ||
| logging.error( | ||
| "Error: GOOGLE_API_KEY not found. Please set it in the .env file." | ||
| ) | ||
| sys.exit(1) |
There was a problem hiding this comment.
The API key check is only implemented for Google models. To improve user experience and provide early feedback for misconfigurations, you should add checks for the other providers that require API keys (OpenAI, Anthropic, OpenRouter, Hugging Face). This can be generalized using a map of providers to their required environment variable names, making it easier to maintain.
provider_name, _ = get_provider_and_model_name(model_name)
api_key_env_vars = {
"google": "GOOGLE_API_KEY",
"openai": "OPENAI_API_KEY",
"anthropic": "ANTHROPIC_API_KEY",
"openrouter": "OPENROUTER_API_KEY",
"huggingface": "HUGGING_FACE_TOKEN",
}
if provider_name in api_key_env_vars:
key_name = api_key_env_vars[provider_name]
if not os.getenv(key_name):
logging.error(f"Error: {key_name} not found. Please set it in the .env file.")
sys.exit(1)| completion = client.chat.completions.create( | ||
| model=model_name, messages=[{"role": "user", "content": prompt}] | ||
| ) |
There was a problem hiding this comment.
The call method receives max_output_tokens, but it's not being used when calling the OpenAI API. The OpenAI API supports a max_tokens parameter, which you should use to limit the response size. This improves consistency with other providers like Anthropic and can help control costs and prevent unexpectedly large responses.
| completion = client.chat.completions.create( | |
| model=model_name, messages=[{"role": "user", "content": prompt}] | |
| ) | |
| completion = client.chat.completions.create( | |
| model=model_name, | |
| messages=[{"role": "user", "content": prompt}], | |
| max_tokens=max_output_tokens, | |
| ) |
No description provided.