Merge pull request 'feat: Add @codebot review-again command for manual PR re-review' (#10) from feature/review-again-command into dev
All checks were successful
Enterprise AI Code Review / ai-review (pull_request) Successful in 19s
All checks were successful
Enterprise AI Code Review / ai-review (pull_request) Successful in 19s
Reviewed-on: #10
This commit was merged in pull request #10.
This commit is contained in:
@@ -37,5 +37,14 @@ jobs:
|
||||
OLLAMA_HOST: ${{ secrets.OLLAMA_HOST }}
|
||||
run: |
|
||||
cd .ai-review/tools/ai-review
|
||||
|
||||
# Check if this is a PR or an issue
|
||||
if [ "${{ gitea.event.issue.pull_request }}" != "" ]; then
|
||||
# This is a PR comment - dispatch as issue_comment event
|
||||
python main.py dispatch ${{ gitea.repository }} issue_comment \
|
||||
'{"action":"created","issue":${{ toJSON(gitea.event.issue) }},"comment":${{ toJSON(gitea.event.comment) }}}'
|
||||
else
|
||||
# This is an issue comment - use the comment command
|
||||
python main.py comment ${{ gitea.repository }} ${{ gitea.event.issue.number }} \
|
||||
"${{ gitea.event.comment.body }}"
|
||||
fi
|
||||
|
||||
45
CLAUDE.md
45
CLAUDE.md
@@ -145,6 +145,7 @@ interaction:
|
||||
- security # Security analysis
|
||||
- summarize # Summarize the issue
|
||||
- triage # Full triage with labeling
|
||||
- review-again # Re-run PR review (PR comments only)
|
||||
|
||||
review:
|
||||
fail_on_severity: HIGH # Fail CI if HIGH severity issues found
|
||||
@@ -309,6 +310,49 @@ pytest tests/test_ai_review.py::TestSecurityScanner -v
|
||||
|
||||
## Common Development Tasks
|
||||
|
||||
### Review-Again Command Implementation
|
||||
|
||||
The `@codebot review-again` command allows manual re-triggering of PR reviews without new commits.
|
||||
|
||||
**Key Features:**
|
||||
- Detects `@codebot review-again` in PR comments (not issue comments)
|
||||
- Compares new review with previous review to show resolved/new issues
|
||||
- Updates existing AI review comment instead of creating duplicates
|
||||
- Updates PR labels based on new severity assessment
|
||||
|
||||
**Implementation Details:**
|
||||
|
||||
1. **PRAgent.can_handle()** - Handles `issue_comment` events on PRs containing "review-again"
|
||||
2. **PRAgent._handle_review_again()** - Main handler that:
|
||||
- Fetches previous review comment
|
||||
- Re-runs full PR review (security scan + AI analysis)
|
||||
- Compares findings using `_compare_reviews()`
|
||||
- Generates diff report with `_format_review_update()`
|
||||
- Updates comment and labels
|
||||
|
||||
3. **Review Comparison** - Uses finding keys (file:line:description) to match issues:
|
||||
- **Resolved**: Issues in previous but not in current review
|
||||
- **New**: Issues in current but not in previous review
|
||||
- **Still Present**: Issues in both reviews
|
||||
- **Severity Changed**: Same issue with different severity
|
||||
|
||||
4. **Workflow Integration** - `.gitea/workflows/ai-comment-reply.yml`:
|
||||
- Detects if comment is on PR or issue
|
||||
- Uses `dispatch` command for PRs to route to PRAgent
|
||||
- Preserves backward compatibility with issue commands
|
||||
|
||||
**Usage:**
|
||||
```bash
|
||||
# In a PR comment:
|
||||
@codebot review-again
|
||||
```
|
||||
|
||||
**Common Use Cases:**
|
||||
- Re-evaluate after explaining false positives in comments
|
||||
- Test new `.ai-review.yml` configuration
|
||||
- Update severity after code clarification
|
||||
- Faster iteration without empty commits
|
||||
|
||||
### Adding a New Command to @codebot
|
||||
|
||||
1. Add command to `config.yml` under `interaction.commands`
|
||||
@@ -323,6 +367,7 @@ Example commands:
|
||||
- `@codebot explain` - Explain the issue
|
||||
- `@codebot suggest` - Suggest solutions
|
||||
- `@codebot setup-labels` - Automatic label setup (built-in, not in config)
|
||||
- `@codebot review-again` - Re-run PR review without new commits (PR comments only)
|
||||
|
||||
### Changing the Bot Name
|
||||
|
||||
|
||||
28
README.md
28
README.md
@@ -169,6 +169,8 @@ python main.py chat owner/repo "Find all API endpoints" --issue 789
|
||||
|
||||
## @codebot Commands
|
||||
|
||||
### Issue Commands
|
||||
|
||||
In any issue comment:
|
||||
|
||||
| Command | Description |
|
||||
@@ -181,6 +183,32 @@ In any issue comment:
|
||||
| `@codebot suggest` | Suggest solutions or next steps |
|
||||
| `@codebot` (any question) | Chat with AI using codebase/web search tools |
|
||||
|
||||
### Pull Request Commands
|
||||
|
||||
In any PR comment:
|
||||
|
||||
| Command | Description |
|
||||
|---------|-------------|
|
||||
| `@codebot review-again` | Re-run AI code review on current PR state without new commits |
|
||||
|
||||
**Features:**
|
||||
- ✅ Shows diff from previous review (resolved/new/changed issues)
|
||||
- 🏷️ Updates labels based on new severity
|
||||
- ⚡ No need for empty commits to trigger review
|
||||
- 🔧 Respects latest `.ai-review.yml` configuration
|
||||
|
||||
**When to use:**
|
||||
- After addressing review feedback in comments
|
||||
- When AI flagged a false positive and you explained it
|
||||
- After updating `.ai-review.yml` security rules
|
||||
- To re-evaluate severity after code clarification
|
||||
|
||||
**Example:**
|
||||
```
|
||||
The hardcoded string at line 45 is a public API URL, not a secret.
|
||||
@codebot review-again
|
||||
```
|
||||
|
||||
**New to OpenRabbit?** Just type `@codebot help` in any issue to see all available commands!
|
||||
|
||||
### Label Setup Command
|
||||
|
||||
@@ -479,6 +479,12 @@ PR reviews run automatically when you open or update a pull request. The bot pro
|
||||
- Security vulnerability scanning
|
||||
- Approval or change-request recommendations
|
||||
|
||||
**Manual re-review:**
|
||||
- `{mention_prefix} review-again` - Re-run AI review on current PR state (in PR comments)
|
||||
- Shows diff from previous review (resolved/new issues)
|
||||
- Updates labels and recommendations
|
||||
- Useful after addressing feedback or updating config
|
||||
|
||||
---
|
||||
|
||||
### Quick Examples
|
||||
|
||||
@@ -83,10 +83,30 @@ class PRAgent(BaseAgent):
|
||||
allowed_events = agent_config.get("events", ["opened", "synchronize"])
|
||||
return action in allowed_events
|
||||
|
||||
# Handle issue comments on PRs (for review-again command)
|
||||
if event_type == "issue_comment":
|
||||
action = event_data.get("action", "")
|
||||
if action == "created":
|
||||
comment_body = event_data.get("comment", {}).get("body", "")
|
||||
mention_prefix = self.config.get("interaction", {}).get(
|
||||
"mention_prefix", "@codebot"
|
||||
)
|
||||
# Only handle if this is a PR and contains review-again command
|
||||
issue = event_data.get("issue", {})
|
||||
is_pr = issue.get("pull_request") is not None
|
||||
has_review_again = (
|
||||
f"{mention_prefix} review-again" in comment_body.lower()
|
||||
)
|
||||
return is_pr and has_review_again
|
||||
|
||||
return False
|
||||
|
||||
def execute(self, context: AgentContext) -> AgentResult:
|
||||
"""Execute the PR review agent."""
|
||||
# Check if this is a review-again command
|
||||
if context.event_type == "issue_comment":
|
||||
return self._handle_review_again(context)
|
||||
|
||||
pr = context.event_data.get("pull_request", {})
|
||||
pr_number = pr.get("number")
|
||||
|
||||
@@ -448,6 +468,307 @@ class PRAgent(BaseAgent):
|
||||
self.logger.warning(f"Failed to get repo labels: {e}")
|
||||
return []
|
||||
|
||||
def _handle_review_again(self, context: AgentContext) -> AgentResult:
|
||||
"""Re-run PR review on current state."""
|
||||
issue = context.event_data.get("issue", {})
|
||||
pr_number = issue.get("number")
|
||||
comment_author = (
|
||||
context.event_data.get("comment", {}).get("user", {}).get("login", "user")
|
||||
)
|
||||
|
||||
self.logger.info(f"Re-reviewing PR #{pr_number} at user request")
|
||||
|
||||
# Get previous review comment
|
||||
previous_comment = self._find_previous_review(
|
||||
context.owner, context.repo, pr_number
|
||||
)
|
||||
previous_findings = []
|
||||
if previous_comment:
|
||||
previous_findings = self._parse_review_comment(previous_comment)
|
||||
|
||||
# Run new review (reuse existing review logic)
|
||||
actions_taken = []
|
||||
|
||||
# Step 1: Get PR diff
|
||||
diff = self._get_diff(context.owner, context.repo, pr_number)
|
||||
if not diff.strip():
|
||||
response = f"@{comment_author}\n\n{self.AI_DISCLAIMER}\n\n**🔄 Re-review Requested**\n\nPR has no changes to review."
|
||||
self.gitea.create_issue_comment(
|
||||
context.owner, context.repo, pr_number, response
|
||||
)
|
||||
return AgentResult(
|
||||
success=True,
|
||||
message="PR has no changes to review",
|
||||
)
|
||||
|
||||
# Step 2: Parse changed files
|
||||
changed_files = self._parse_diff_files(diff)
|
||||
|
||||
# Step 3: Run security scan if enabled
|
||||
security_issues = []
|
||||
agent_config = self.config.get("agents", {}).get("pr", {})
|
||||
if agent_config.get("security_scan", True):
|
||||
security_issues = self._run_security_scan(changed_files, diff)
|
||||
|
||||
# Step 4: Run AI review
|
||||
review_result = self._run_ai_review(diff, context, security_issues)
|
||||
|
||||
# Step 5: Compare with previous review
|
||||
current_findings = self._extract_findings_from_review(review_result)
|
||||
diff_result = self._compare_reviews(previous_findings, current_findings)
|
||||
|
||||
# Step 6: Generate updated review with comparison
|
||||
updated_review = self._format_review_update(
|
||||
review_result, diff_result, comment_author
|
||||
)
|
||||
|
||||
# Step 7: Update existing comment (or create new one)
|
||||
self.upsert_comment(
|
||||
context.owner,
|
||||
context.repo,
|
||||
pr_number,
|
||||
updated_review,
|
||||
marker=self.PR_AI_MARKER,
|
||||
)
|
||||
actions_taken.append("Updated review comment")
|
||||
|
||||
# Step 8: Update PR labels
|
||||
labels_applied = self._apply_review_labels(
|
||||
context.owner, context.repo, pr_number, review_result
|
||||
)
|
||||
if labels_applied:
|
||||
actions_taken.append(f"Updated labels: {labels_applied}")
|
||||
|
||||
return AgentResult(
|
||||
success=True,
|
||||
message=f"Re-reviewed PR #{pr_number}: {review_result.overall_severity} severity",
|
||||
data={
|
||||
"severity": review_result.overall_severity,
|
||||
"approval": review_result.approval,
|
||||
"issues_count": len(review_result.issues),
|
||||
"security_issues_count": len(review_result.security_issues),
|
||||
"resolved_count": len(diff_result.get("resolved", [])),
|
||||
"new_count": len(diff_result.get("new", [])),
|
||||
},
|
||||
actions_taken=actions_taken,
|
||||
)
|
||||
|
||||
def _find_previous_review(
|
||||
self, owner: str, repo: str, pr_number: int
|
||||
) -> str | None:
|
||||
"""Find the previous AI review comment."""
|
||||
comment_id = self.find_ai_comment(
|
||||
owner, repo, pr_number, marker=self.PR_AI_MARKER
|
||||
)
|
||||
if not comment_id:
|
||||
return None
|
||||
|
||||
# Get the comment content
|
||||
comments = self.gitea.list_issue_comments(owner, repo, pr_number)
|
||||
for comment in comments:
|
||||
if comment.get("id") == comment_id:
|
||||
return comment.get("body", "")
|
||||
|
||||
return None
|
||||
|
||||
def _parse_review_comment(self, comment_text: str) -> list[dict]:
|
||||
"""Parse previous review comment to extract findings.
|
||||
|
||||
Returns:
|
||||
List of findings with file, line, severity, description
|
||||
"""
|
||||
findings = []
|
||||
|
||||
if not comment_text:
|
||||
return findings
|
||||
|
||||
# Look for patterns like: **[HIGH]** `src/file.py:45` - Description
|
||||
pattern = r"\*\*\[(\w+)\]\*\*\s+`([^:]+):(\d+)`\s+-\s+(.+?)(?:\n|$)"
|
||||
|
||||
for match in re.finditer(pattern, comment_text):
|
||||
findings.append(
|
||||
{
|
||||
"severity": match.group(1),
|
||||
"file": match.group(2),
|
||||
"line": int(match.group(3)),
|
||||
"description": match.group(4).strip(),
|
||||
}
|
||||
)
|
||||
|
||||
return findings
|
||||
|
||||
def _extract_findings_from_review(self, review: PRReviewResult) -> list[dict]:
|
||||
"""Extract findings from PRReviewResult into comparable format."""
|
||||
findings = []
|
||||
all_issues = review.issues + review.security_issues
|
||||
|
||||
for issue in all_issues:
|
||||
findings.append(
|
||||
{
|
||||
"severity": issue.severity,
|
||||
"file": issue.file,
|
||||
"line": issue.line or 0,
|
||||
"description": issue.description,
|
||||
"category": issue.category,
|
||||
}
|
||||
)
|
||||
|
||||
return findings
|
||||
|
||||
def _finding_key(self, finding: dict) -> str:
|
||||
"""Create unique key for a finding."""
|
||||
file_path = finding.get("file", "unknown")
|
||||
line = finding.get("line", 0)
|
||||
# Use first 50 chars of description for matching
|
||||
desc_key = finding.get("description", "")[:50]
|
||||
return f"{file_path}:{line}:{desc_key}"
|
||||
|
||||
def _compare_reviews(
|
||||
self, previous_findings: list[dict], new_findings: list[dict]
|
||||
) -> dict:
|
||||
"""Compare previous and new review to show what changed.
|
||||
|
||||
Returns:
|
||||
{
|
||||
"resolved": [...], # Issues that disappeared
|
||||
"new": [...], # New issues found
|
||||
"still_present": [...], # Issues that remain
|
||||
"severity_changed": {...} # OLD severity -> NEW severity
|
||||
}
|
||||
"""
|
||||
# Create lookup keys (file:line:description)
|
||||
prev_keys = {self._finding_key(f): f for f in previous_findings}
|
||||
new_keys = {self._finding_key(f): f for f in new_findings}
|
||||
|
||||
resolved = [prev_keys[key] for key in prev_keys if key not in new_keys]
|
||||
|
||||
new = [new_keys[key] for key in new_keys if key not in prev_keys]
|
||||
|
||||
still_present = [new_keys[key] for key in new_keys if key in prev_keys]
|
||||
|
||||
severity_changed = {}
|
||||
for key in prev_keys:
|
||||
if key in new_keys:
|
||||
prev_severity = prev_keys[key].get("severity")
|
||||
new_severity = new_keys[key].get("severity")
|
||||
if prev_severity != new_severity:
|
||||
severity_changed[key] = {
|
||||
"old": prev_severity,
|
||||
"new": new_severity,
|
||||
"finding": new_keys[key],
|
||||
}
|
||||
|
||||
return {
|
||||
"resolved": resolved,
|
||||
"new": new,
|
||||
"still_present": still_present,
|
||||
"severity_changed": severity_changed,
|
||||
}
|
||||
|
||||
def _format_review_update(
|
||||
self, review: PRReviewResult, diff: dict, comment_author: str
|
||||
) -> str:
|
||||
"""Format review with comparison to previous run."""
|
||||
lines = [f"@{comment_author}\n"]
|
||||
lines.append(f"{self.AI_DISCLAIMER}\n")
|
||||
lines.append("**🔄 Re-review Requested**\n")
|
||||
lines.append("---\n")
|
||||
lines.append("## AI Code Review (Updated)\n")
|
||||
|
||||
# Summary of changes
|
||||
prev_total = len(diff["resolved"]) + len(diff["still_present"])
|
||||
curr_total = len(diff["new"]) + len(diff["still_present"])
|
||||
|
||||
if prev_total > 0:
|
||||
lines.append(f"**Previous Review:** {prev_total} issues")
|
||||
lines.append(f"**Current Review:** {curr_total} issues\n")
|
||||
else:
|
||||
lines.append("**First Review** - No previous review found\n")
|
||||
|
||||
# Changes section (only if there was a previous review)
|
||||
if prev_total > 0:
|
||||
lines.append("### Changes from Previous Review\n")
|
||||
|
||||
if diff["resolved"]:
|
||||
lines.append(f"**✅ Resolved ({len(diff['resolved'])}):**")
|
||||
for finding in diff["resolved"][:5]: # Show max 5
|
||||
lines.append(
|
||||
f"- **[{finding['severity']}]** `{finding['file']}:{finding['line']}` - {finding['description']}"
|
||||
)
|
||||
if len(diff["resolved"]) > 5:
|
||||
lines.append(f"- ... and {len(diff['resolved']) - 5} more")
|
||||
lines.append("")
|
||||
|
||||
if diff["new"]:
|
||||
lines.append(f"**⚠️ New Issues ({len(diff['new'])}):**")
|
||||
for finding in diff["new"][:5]:
|
||||
lines.append(
|
||||
f"- **[{finding['severity']}]** `{finding['file']}:{finding['line']}` - {finding['description']}"
|
||||
)
|
||||
if len(diff["new"]) > 5:
|
||||
lines.append(f"- ... and {len(diff['new']) - 5} more")
|
||||
lines.append("")
|
||||
|
||||
if diff["severity_changed"]:
|
||||
lines.append(
|
||||
f"**🔄 Severity Changed ({len(diff['severity_changed'])}):**"
|
||||
)
|
||||
for key, change in list(diff["severity_changed"].items())[:5]:
|
||||
finding = change["finding"]
|
||||
lines.append(
|
||||
f"- `{finding['file']}:{finding['line']}` - {change['old']} → {change['new']}"
|
||||
)
|
||||
lines.append("")
|
||||
|
||||
# Summary table
|
||||
all_issues = review.issues + review.security_issues
|
||||
high = sum(1 for i in all_issues if i.severity == "HIGH")
|
||||
medium = sum(1 for i in all_issues if i.severity == "MEDIUM")
|
||||
low = sum(1 for i in all_issues if i.severity == "LOW")
|
||||
|
||||
lines.append("### Summary\n")
|
||||
lines.append("| Severity | Count |")
|
||||
lines.append("|----------|-------|")
|
||||
lines.append(f"| HIGH | {high} |")
|
||||
lines.append(f"| MEDIUM | {medium} |")
|
||||
lines.append(f"| LOW | {low} |")
|
||||
lines.append("")
|
||||
|
||||
# Security issues section (if any)
|
||||
if review.security_issues:
|
||||
lines.append("### Security Issues\n")
|
||||
for issue in review.security_issues[:5]:
|
||||
loc = (
|
||||
f"`{issue.file}:{issue.line}`" if issue.line else f"`{issue.file}`"
|
||||
)
|
||||
lines.append(f"- **[{issue.severity}]** {loc} - {issue.description}")
|
||||
if len(review.security_issues) > 5:
|
||||
lines.append(f"- ... and {len(review.security_issues) - 5} more")
|
||||
lines.append("")
|
||||
|
||||
# Other issues (limit display)
|
||||
other_issues = [i for i in review.issues if i not in review.security_issues]
|
||||
if other_issues:
|
||||
lines.append("### Review Findings\n")
|
||||
for issue in other_issues[:10]:
|
||||
loc = (
|
||||
f"`{issue.file}:{issue.line}`" if issue.line else f"`{issue.file}`"
|
||||
)
|
||||
lines.append(f"- **[{issue.severity}]** {loc} - {issue.description}")
|
||||
if len(other_issues) > 10:
|
||||
lines.append(f"- ... and {len(other_issues) - 10} more issues")
|
||||
lines.append("")
|
||||
|
||||
# Verdict
|
||||
lines.append("---")
|
||||
lines.append(f"**Overall Severity:** `{review.overall_severity}`")
|
||||
if review.approval:
|
||||
lines.append("**AI Recommendation:** Approved ✅")
|
||||
else:
|
||||
lines.append("**AI Recommendation:** Changes Requested ⚠️")
|
||||
|
||||
return "\n".join(lines)
|
||||
|
||||
labels_to_add = []
|
||||
|
||||
# Add approval/changes required label
|
||||
|
||||
@@ -65,6 +65,7 @@ interaction:
|
||||
- security
|
||||
- summarize
|
||||
- triage
|
||||
- review-again
|
||||
|
||||
# Enterprise settings
|
||||
enterprise:
|
||||
|
||||
Reference in New Issue
Block a user