add time sensitive AI review logic (#1137)

This commit is contained in:
Andy
2026-01-15 18:17:47 +01:00
committed by GitHub
parent 45060ca3e9
commit 5fb7574b7b
9 changed files with 143 additions and 16 deletions
+48 -1
View File
@@ -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
+8 -1
View File
@@ -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
+1
View File
@@ -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(