add time sensitive AI review logic (#1137)
This commit is contained in:
@@ -55,6 +55,19 @@ Use when:
|
||||
- Suggestions that would add complexity without clear benefit
|
||||
- Comment is technically correct but practically irrelevant
|
||||
|
||||
### ADDRESSED
|
||||
The AI found a **valid issue that was subsequently fixed** by the contributor.
|
||||
|
||||
Use when:
|
||||
- AI correctly identified an issue at the time of its comment
|
||||
- A later commit explicitly fixed the issue the AI flagged
|
||||
- The issue no longer exists in the current code BECAUSE of a fix
|
||||
- Commit messages reference the AI's feedback (e.g., "Fixed typo per Gemini review")
|
||||
|
||||
**CRITICAL: Do NOT use FALSE_POSITIVE when an issue was valid but has been fixed!**
|
||||
- If Gemini said "typo: CLADE should be CLAUDE" and a later commit fixed it → ADDRESSED (not false_positive)
|
||||
- The AI was RIGHT when it made the comment - the fix came later
|
||||
|
||||
### FALSE_POSITIVE
|
||||
The AI is **wrong** about this.
|
||||
|
||||
@@ -64,6 +77,30 @@ Use when:
|
||||
- AI suggested a fix that would introduce bugs
|
||||
- AI missed context that makes the "issue" not an issue
|
||||
- AI duplicated another tool's comment
|
||||
- The issue NEVER existed (even at the time of the AI comment)
|
||||
|
||||
## CRITICAL: Timeline Awareness
|
||||
|
||||
**You MUST consider the timeline when evaluating AI comments.**
|
||||
|
||||
AI tools comment at specific points in time. The code you see now may be DIFFERENT from what the AI saw when it made the comment.
|
||||
|
||||
**Timeline Analysis Process:**
|
||||
1. **Check the AI comment timestamp** - When did the AI make this comment?
|
||||
2. **Check the commit timeline** - Were there commits AFTER the AI comment?
|
||||
3. **Check commit messages** - Do any commits mention fixing the AI's concern?
|
||||
4. **Compare states** - Did the issue exist when the AI commented, but get fixed later?
|
||||
|
||||
**Common Mistake to Avoid:**
|
||||
- You see: Code currently shows `CLAUDE_CLI_PATH` (correct)
|
||||
- AI comment says: "Typo: CLADE_CLI_PATH should be CLAUDE_CLI_PATH"
|
||||
- WRONG conclusion: "The AI is wrong, there's no typo" → FALSE_POSITIVE
|
||||
- CORRECT conclusion: "The typo existed when AI commented, then was fixed" → ADDRESSED
|
||||
|
||||
**How to determine ADDRESSED vs FALSE_POSITIVE:**
|
||||
- If the issue NEVER existed (AI hallucinated) → FALSE_POSITIVE
|
||||
- If the issue DID exist but was FIXED by a later commit → ADDRESSED
|
||||
- Check commit messages for evidence: "fix typo", "address review feedback", etc.
|
||||
|
||||
## Evaluation Framework
|
||||
|
||||
@@ -126,6 +163,14 @@ Return a JSON array with your triage verdict for each AI comment:
|
||||
"verdict": "false_positive",
|
||||
"reasoning": "The import IS used - it's a type import used in the function signature on line 45. The AI's static analysis missed the type-only usage.",
|
||||
"response_comment": "False positive - this import is used for TypeScript type annotations (line 45). The import is correctly present."
|
||||
},
|
||||
{
|
||||
"comment_id": 12345682,
|
||||
"tool_name": "Gemini Code Assist",
|
||||
"original_summary": "Typo: CLADE_CLI_PATH should be CLAUDE_CLI_PATH",
|
||||
"verdict": "addressed",
|
||||
"reasoning": "Gemini correctly identified a typo in the initial commit (c933e36f). The contributor fixed this in commit 6b1d3d3 just 7 minutes later. The issue was real and is now resolved.",
|
||||
"response_comment": "Good catch! This typo was fixed in commit 6b1d3d3. Thanks for flagging it."
|
||||
}
|
||||
]
|
||||
```
|
||||
@@ -135,7 +180,7 @@ Return a JSON array with your triage verdict for each AI comment:
|
||||
- **comment_id**: The GitHub comment ID (for posting replies)
|
||||
- **tool_name**: Which AI tool made the comment (CodeRabbit, Cursor, Greptile, etc.)
|
||||
- **original_summary**: Brief summary of what the AI flagged (max 100 chars)
|
||||
- **verdict**: `critical` | `important` | `nice_to_have` | `trivial` | `false_positive`
|
||||
- **verdict**: `critical` | `important` | `nice_to_have` | `trivial` | `addressed` | `false_positive`
|
||||
- **reasoning**: Your analysis of why you agree/disagree (2-3 sentences)
|
||||
- **response_comment**: The reply to post on GitHub (concise, helpful, professional)
|
||||
|
||||
@@ -147,6 +192,7 @@ Return a JSON array with your triage verdict for each AI comment:
|
||||
- **IMPORTANT**: "Valid point. [Brief reasoning]. Recommend addressing but not blocking."
|
||||
- **NICE_TO_HAVE**: "Valid suggestion. [Context]. Optional improvement."
|
||||
- **TRIVIAL**: "Style preference. [Why it doesn't apply]. No change needed."
|
||||
- **ADDRESSED**: "Good catch! This was fixed in commit [SHA]. Thanks for flagging it."
|
||||
- **FALSE_POSITIVE**: "False positive - [brief explanation of why the AI is wrong]."
|
||||
|
||||
**Avoid:**
|
||||
@@ -154,6 +200,7 @@ Return a JSON array with your triage verdict for each AI comment:
|
||||
- Condescending tone toward either the AI or the developer
|
||||
- Vague verdicts without reasoning
|
||||
- Simply agreeing/disagreeing without explanation
|
||||
- Calling valid-but-fixed issues "false positives" (use ADDRESSED instead)
|
||||
|
||||
## Important Notes
|
||||
|
||||
|
||||
@@ -88,10 +88,17 @@ Check contributor and AI bot comments for:
|
||||
- Concerns raised ("I'm worried about performance here")
|
||||
|
||||
**AI bot suggestions:**
|
||||
- CodeRabbit, Copilot, or other AI feedback
|
||||
- CodeRabbit, Copilot, Gemini Code Assist, or other AI feedback
|
||||
- Security warnings from automated scanners
|
||||
- Suggestions that align with your findings
|
||||
|
||||
**IMPORTANT - Timeline Awareness for AI Comments:**
|
||||
AI tools comment at specific points in time. When evaluating AI bot comments:
|
||||
- Check the comment timestamp vs commit timestamps
|
||||
- If an AI flagged an issue that was LATER FIXED by a commit, the AI was RIGHT (not a false positive)
|
||||
- If an AI comment seems wrong but the code is now correct, check if a recent commit fixed it
|
||||
- Don't dismiss valid AI feedback just because the fix already happened - acknowledge the issue was caught and fixed
|
||||
|
||||
For important unaddressed comments, create a finding:
|
||||
```json
|
||||
{
|
||||
|
||||
@@ -71,11 +71,17 @@ Common AI reviewers you'll encounter:
|
||||
- Performance optimizations
|
||||
- Documentation improvements
|
||||
|
||||
### Addressed (Acknowledge)
|
||||
- Valid issue that was fixed in a later commit
|
||||
- AI correctly identified the problem, contributor fixed it
|
||||
- The issue no longer exists BECAUSE of a fix
|
||||
- **Use this instead of False Positive when the AI was RIGHT but the fix already happened**
|
||||
|
||||
### False Positive (Dismiss)
|
||||
- Incorrect analysis
|
||||
- Incorrect analysis (AI was WRONG - issue never existed)
|
||||
- Not applicable to this context
|
||||
- Already addressed
|
||||
- Stylistic preferences
|
||||
- **Do NOT use for valid issues that were fixed - use Addressed instead**
|
||||
|
||||
## Output Format
|
||||
|
||||
@@ -161,6 +167,21 @@ Common bot patterns:
|
||||
- Known bot names: dependabot, renovate, snyk-bot, sonarcloud
|
||||
- Automated review format (structured markdown)
|
||||
|
||||
## CRITICAL: Timeline Awareness
|
||||
|
||||
**AI tools comment at specific points in time. The code may have changed since their comments.**
|
||||
|
||||
When evaluating AI tool comments:
|
||||
1. **Check when the AI commented** - Look at the timestamp
|
||||
2. **Check when commits were made** - Were there commits AFTER the AI comment?
|
||||
3. **Check if commits fixed the issue** - Did the contributor address the AI's feedback?
|
||||
|
||||
**Common Mistake to Avoid:**
|
||||
- AI says "Line 45 has a bug" at 2:00 PM
|
||||
- Contributor fixes it in a commit at 2:30 PM
|
||||
- You see the fixed code and think "AI was wrong, there's no bug"
|
||||
- WRONG! The AI was RIGHT - the fix came later → Use **Addressed**, not False Positive
|
||||
|
||||
## Important Notes
|
||||
|
||||
1. **Humans first**: Prioritize human feedback over AI suggestions
|
||||
@@ -168,6 +189,7 @@ Common bot patterns:
|
||||
3. **Don't duplicate**: If an issue is already in previous findings, reference it
|
||||
4. **Be constructive**: Extract actionable items, not just concerns
|
||||
5. **Verify AI findings**: AI tools can be wrong - assess validity
|
||||
6. **Timeline matters**: A valid finding that was later fixed is ADDRESSED, not a false positive
|
||||
|
||||
## Sample Workflow
|
||||
|
||||
|
||||
@@ -84,6 +84,7 @@ class AICommentVerdict(str, Enum):
|
||||
NICE_TO_HAVE = "nice_to_have" # Optional improvement
|
||||
TRIVIAL = "trivial" # Can be ignored
|
||||
FALSE_POSITIVE = "false_positive" # AI was wrong
|
||||
ADDRESSED = "addressed" # Valid issue that was fixed in a subsequent commit
|
||||
|
||||
|
||||
class TriageCategory(str, Enum):
|
||||
|
||||
@@ -600,25 +600,26 @@ class FollowupReviewer:
|
||||
]
|
||||
)
|
||||
|
||||
# Format commits
|
||||
# Format commits with timestamps (for timeline correlation with AI comments)
|
||||
commits_text = "\n".join(
|
||||
[
|
||||
f"- {c.get('sha', '')[:8]}: {c.get('commit', {}).get('message', '').split(chr(10))[0]}"
|
||||
f"- {c.get('sha', '')[:8]} ({c.get('commit', {}).get('author', {}).get('date', 'unknown')}): {c.get('commit', {}).get('message', '').split(chr(10))[0]}"
|
||||
for c in context.commits_since_review
|
||||
]
|
||||
)
|
||||
|
||||
# Format comments
|
||||
# Format contributor comments with timestamps
|
||||
contributor_comments_text = "\n".join(
|
||||
[
|
||||
f"- @{c.get('user', {}).get('login', 'unknown')}: {c.get('body', '')[:200]}"
|
||||
f"- @{c.get('user', {}).get('login', 'unknown')} ({c.get('created_at', 'unknown')}): {c.get('body', '')[:200]}"
|
||||
for c in context.contributor_comments_since_review
|
||||
]
|
||||
)
|
||||
|
||||
# Format AI comments with timestamps for timeline awareness
|
||||
ai_comments_text = "\n".join(
|
||||
[
|
||||
f"- @{c.get('user', {}).get('login', 'unknown')}: {c.get('body', '')[:200]}"
|
||||
f"- @{c.get('user', {}).get('login', 'unknown')} ({c.get('created_at', 'unknown')}): {c.get('body', '')[:200]}"
|
||||
for c in context.ai_bot_comments_since_review
|
||||
]
|
||||
)
|
||||
|
||||
@@ -275,19 +275,35 @@ class ParallelOrchestratorReviewer:
|
||||
if len(diff_content) > MAX_DIFF_CHARS:
|
||||
diff_content = diff_content[:MAX_DIFF_CHARS] + "\n\n... (diff truncated)"
|
||||
|
||||
# Build AI comments context if present
|
||||
# Build AI comments context if present (with timestamps for timeline awareness)
|
||||
ai_comments_section = ""
|
||||
if context.ai_bot_comments:
|
||||
ai_comments_list = []
|
||||
for comment in context.ai_bot_comments[:20]:
|
||||
ai_comments_list.append(
|
||||
f"- **{comment.tool_name}** on {comment.file or 'general'}: "
|
||||
f"- **{comment.tool_name}** ({comment.created_at}) on {comment.file or 'general'}: "
|
||||
f"{comment.body[:200]}..."
|
||||
)
|
||||
ai_comments_section = f"""
|
||||
### AI Review Comments (need triage)
|
||||
Found {len(context.ai_bot_comments)} comments from AI tools:
|
||||
Found {len(context.ai_bot_comments)} comments from AI tools.
|
||||
**IMPORTANT: Check timestamps! If a later commit fixed an AI-flagged issue, use ADDRESSED verdict (not FALSE_POSITIVE).**
|
||||
|
||||
{chr(10).join(ai_comments_list)}
|
||||
"""
|
||||
|
||||
# Build commits timeline section (important for AI triage)
|
||||
commits_section = ""
|
||||
if context.commits:
|
||||
commits_list = []
|
||||
for commit in context.commits:
|
||||
sha = commit.get("oid", "")[:8]
|
||||
message = commit.get("messageHeadline", "")
|
||||
committed_at = commit.get("committedDate", "")
|
||||
commits_list.append(f"- `{sha}` ({committed_at}): {message}")
|
||||
commits_section = f"""
|
||||
### Commit Timeline
|
||||
{chr(10).join(commits_list)}
|
||||
"""
|
||||
|
||||
pr_context = f"""
|
||||
@@ -307,7 +323,7 @@ Found {len(context.ai_bot_comments)} comments from AI tools:
|
||||
|
||||
### All Changed Files
|
||||
{chr(10).join(files_list)}
|
||||
{ai_comments_section}
|
||||
{commits_section}{ai_comments_section}
|
||||
### Code Changes
|
||||
```diff
|
||||
{diff_content}
|
||||
|
||||
@@ -580,12 +580,18 @@ class PRReviewEngine:
|
||||
"",
|
||||
f"Found {len(context.ai_bot_comments)} comments from AI code review tools:",
|
||||
"",
|
||||
"**IMPORTANT: Check the timeline! AI comments were made at specific times.",
|
||||
"If a later commit fixed the issue the AI flagged, use ADDRESSED (not FALSE_POSITIVE).**",
|
||||
"",
|
||||
]
|
||||
|
||||
for i, comment in enumerate(context.ai_bot_comments, 1):
|
||||
lines.append(f"### Comment {i}: {comment.tool_name}")
|
||||
lines.append(f"- **Comment ID**: {comment.comment_id}")
|
||||
lines.append(f"- **Author**: {comment.author}")
|
||||
lines.append(
|
||||
f"- **Commented At**: {comment.created_at}"
|
||||
) # Include timestamp
|
||||
lines.append(f"- **File**: {comment.file or 'General'}")
|
||||
if comment.line:
|
||||
lines.append(f"- **Line**: {comment.line}")
|
||||
@@ -594,6 +600,21 @@ class PRReviewEngine:
|
||||
lines.append(comment.body)
|
||||
lines.append("")
|
||||
|
||||
# Add commit timeline for reference
|
||||
if context.commits:
|
||||
lines.append("## Commit Timeline (for reference)")
|
||||
lines.append("")
|
||||
lines.append(
|
||||
"Use this to determine if issues were fixed AFTER AI comments:"
|
||||
)
|
||||
lines.append("")
|
||||
for commit in context.commits:
|
||||
sha = commit.get("oid", "")[:8]
|
||||
message = commit.get("messageHeadline", "")
|
||||
committed_at = commit.get("committedDate", "")
|
||||
lines.append(f"- `{sha}` ({committed_at}): {message}")
|
||||
lines.append("")
|
||||
|
||||
return "\n".join(lines)
|
||||
|
||||
def _build_review_context(self, context: PRContext) -> str:
|
||||
|
||||
@@ -270,12 +270,19 @@ Output JSON array of structural issues:
|
||||
ReviewPass.AI_COMMENT_TRIAGE: """
|
||||
You are triaging comments from other AI code review tools (CodeRabbit, Gemini Code Assist, Cursor, Greptile, etc).
|
||||
|
||||
**CRITICAL: TIMELINE AWARENESS**
|
||||
AI comments were made at specific points in time. The current code may have FIXED issues that AI tools correctly identified.
|
||||
- If an AI flagged an issue that was LATER FIXED by a commit, use ADDRESSED (not FALSE_POSITIVE)
|
||||
- FALSE_POSITIVE means the AI was WRONG - the issue never existed
|
||||
- ADDRESSED means the AI was RIGHT - the issue existed but was fixed
|
||||
|
||||
For each AI comment, determine:
|
||||
- CRITICAL: Genuine issue that must be addressed before merge
|
||||
- IMPORTANT: Valid issue that should be addressed
|
||||
- NICE_TO_HAVE: Valid but optional improvement
|
||||
- TRIVIAL: Style preference, can be ignored
|
||||
- FALSE_POSITIVE: The AI is wrong about this
|
||||
- ADDRESSED: Valid issue that was fixed in a subsequent commit
|
||||
- FALSE_POSITIVE: The AI is wrong about this (issue never existed)
|
||||
|
||||
Output JSON array:
|
||||
```json
|
||||
@@ -284,7 +291,7 @@ Output JSON array:
|
||||
"comment_id": 12345678,
|
||||
"tool_name": "CodeRabbit",
|
||||
"original_summary": "Brief summary of what AI flagged (max 100 chars)",
|
||||
"verdict": "critical|important|nice_to_have|trivial|false_positive",
|
||||
"verdict": "critical|important|nice_to_have|trivial|addressed|false_positive",
|
||||
"reasoning": "2-3 sentence explanation of your verdict",
|
||||
"response_comment": "Concise reply to post on GitHub"
|
||||
}
|
||||
|
||||
@@ -111,7 +111,12 @@ class AICommentTriage(BaseModel):
|
||||
description="AI tool name (CodeRabbit, Cursor, Greptile, etc.)"
|
||||
)
|
||||
verdict: Literal[
|
||||
"critical", "important", "nice_to_have", "trivial", "false_positive"
|
||||
"critical",
|
||||
"important",
|
||||
"nice_to_have",
|
||||
"trivial",
|
||||
"addressed",
|
||||
"false_positive",
|
||||
] = Field(description="Verdict on the comment")
|
||||
reasoning: str = Field(description="Why this verdict was chosen")
|
||||
response_comment: str | None = Field(
|
||||
|
||||
Reference in New Issue
Block a user