Skip to content

Conversation

@hungitytng99
Copy link

@hungitytng99 hungitytng99 commented Mar 3, 2025

Add Deepseek AI Provider Support

Changes Made

  • Implemented multi-provider architecture in review_code_gemini.py
  • Added Deepseek AI integration alongside existing Gemini support
  • Extended action.yml with new provider configurations
  • Updated documentation with Deepseek usage instructions

Key Features

  • Support for multiple AI code review providers
  • Easy switching between Gemini and Deepseek using AI_PROVIDER setting
  • Maintained backward compatibility with existing Gemini setup
  • New command /deepseek-review for Deepseek-specific reviews

Configuration

- uses: truongnh1992/gemini-ai-code-reviewer@main
  with:
    GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
    AI_PROVIDER: deepseek
    DEEPSEEK_API_KEY: ${{ secrets.DEEPSEEK_API_KEY }}
    DEEPSEEK_MODEL: deepseek-chat # Optional
    EXCLUDE: "*.md,*.txt,package-lock.json,*.yml,*.yaml"

@hungitytng99
Copy link
Author

/gemini-review

Copy link

@github-actions github-actions bot left a 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
Copy link

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
Copy link

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
Copy link

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
Copy link

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
Copy link

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('```'):
Copy link

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"]
Copy link

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

Copy link

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."""
Copy link

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
Copy link

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant