-
-
Notifications
You must be signed in to change notification settings - Fork 45
feat: implement AI provider architecture with Gemini and Deepseek support #46
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
feat: implement AI provider architecture with Gemini and Deepseek support #46
Conversation
|
/gemini-review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gemini AI Code Reviewer Comments
| @@ -0,0 +1,34 @@ | |||
| # AI Provider API Keys (Required) | |||
| GEMINI_API_KEY=your_gemini_api_key_here | |||
| DEEPSEEK_API_KEY=your_deepseek_api_key_here | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider removing the default API keys. While this file is named .env.example, users might copy it directly to .env and accidentally commit the placeholder keys. It might be better to leave the values blank and add a comment indicating that the user needs to provide their own keys.
| GEMINI_API_KEY=your_gemini_api_key_here | ||
| DEEPSEEK_API_KEY=your_deepseek_api_key_here | ||
| OPENAI_API_KEY=your_openai_api_key_here # Optional | ||
| ANTHROPIC_API_KEY=your_claude_api_key_here # Optional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a comment explaining the purpose of the ANTHROPIC_API_KEY to clarify when it is needed. It's mentioned as optional, but the context of its use isn't immediately obvious from this file alone.
| ANTHROPIC_API_KEY=your_claude_api_key_here # Optional | ||
|
|
||
| # Provider Configuration | ||
| ENABLED_AI_PROVIDERS=gemini,deepseek |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ENABLED_AI_PROVIDERS and AI_PROVIDER_FALLBACK_CHAIN variables should be documented more thoroughly (either here or in the main documentation). It's not immediately obvious what the difference is between these and how they interact. Specifically, what happens if a provider is in the fallback chain but not in the enabled providers?
| # Gemini Settings | ||
| GEMINI_MODEL=gemini-2.0-flash-001 | ||
| GEMINI_TEMPERATURE=0.8 | ||
| GEMINI_TOP_P=0.95 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a comment to DEEPSEEK_TEMPERATURE indicating the valid range (e.g., 0.0 to 1.0) to prevent unexpected behavior if an invalid value is used.
|
|
||
| # Claude Settings (Optional) | ||
| CLAUDE_MODEL=claude-3-opus-20240229 | ||
| CLAUDE_TEMPERATURE=0.7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the API keys, consider removing the placeholder value for GITHUB_TOKEN. It's a security risk if someone accidentally commits this default value.
| response_text = response.text.strip() | ||
| if response_text.startswith('```json'): | ||
| response_text = response_text[7:] | ||
| if response_text.endswith('```'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code removes 'json' and '' from the response text. It would be more robust to use regular expressions to handle variations in whitespace or casing (e.g., 'JSON', ' json '). This will ensure that the parsing works correctly even if the AI's output format isn't perfectly consistent.
| data = json.loads(response_text) | ||
| if "reviews" in data and isinstance(data["reviews"], list): | ||
| return [ | ||
| review for review in data["reviews"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding logging with more context when a JSONDecodeError occurs. Include the full response_text in the log (if it's not too large) to help diagnose the issue. Also, consider adding a metric to track the rate of JSON decoding errors.
| def configure(self) -> None: | ||
| """Configure the AI provider with necessary credentials and settings.""" | ||
| pass | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This block of code duplicates the functionality of fetching the repository and pull request details. It exists in get_pr_details. Consider refactoring to avoid redundant API calls and improve efficiency.
Specifically, the lines:
repo = gh.get_repo(repo_name)
pr = repo.get_pull(pull_number)can be removed, as this information is likely already available or can be passed from the get_pr_details function. Duplicating these calls can lead to unnecessary API rate limit consumption and slower execution.
|
|
||
| @abstractmethod | ||
| def get_name(self) -> str: | ||
| """Get the name of the AI provider.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider handling potential errors when making the API request, such as network issues or rate limiting. Add a try-except block to catch requests.exceptions.RequestException and provide a more informative error message, potentially including instructions on how to resolve the issue (e.g., checking network connectivity or waiting for rate limit reset).
| @abstractmethod | ||
| def generate_review(self, prompt: str) -> List[Dict[str, Any]]: | ||
| """Generate code review from the given prompt.""" | ||
| pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider to move the api_url definition before the headers definition.
Add Deepseek AI Provider Support
Changes Made
Key Features
/deepseek-reviewfor Deepseek-specific reviewsConfiguration