diff --git a/apps/backend/prompts/github/pr_ai_triage.md b/apps/backend/prompts/github/pr_ai_triage.md index f13cf415..96e33435 100644 --- a/apps/backend/prompts/github/pr_ai_triage.md +++ b/apps/backend/prompts/github/pr_ai_triage.md @@ -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 diff --git a/apps/backend/prompts/github/pr_followup.md b/apps/backend/prompts/github/pr_followup.md index 423463f0..75aba5ba 100644 --- a/apps/backend/prompts/github/pr_followup.md +++ b/apps/backend/prompts/github/pr_followup.md @@ -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 { diff --git a/apps/backend/prompts/github/pr_followup_comment_agent.md b/apps/backend/prompts/github/pr_followup_comment_agent.md index 7c45ac59..370b9740 100644 --- a/apps/backend/prompts/github/pr_followup_comment_agent.md +++ b/apps/backend/prompts/github/pr_followup_comment_agent.md @@ -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 diff --git a/apps/backend/runners/github/models.py b/apps/backend/runners/github/models.py index d4c4a90c..8919b7d9 100644 --- a/apps/backend/runners/github/models.py +++ b/apps/backend/runners/github/models.py @@ -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): diff --git a/apps/backend/runners/github/services/followup_reviewer.py b/apps/backend/runners/github/services/followup_reviewer.py index a0474fe9..4ceb16ff 100644 --- a/apps/backend/runners/github/services/followup_reviewer.py +++ b/apps/backend/runners/github/services/followup_reviewer.py @@ -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 ] ) diff --git a/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py b/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py index eef98688..d9175efa 100644 --- a/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py +++ b/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py @@ -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} diff --git a/apps/backend/runners/github/services/pr_review_engine.py b/apps/backend/runners/github/services/pr_review_engine.py index f68a8ec3..7130330d 100644 --- a/apps/backend/runners/github/services/pr_review_engine.py +++ b/apps/backend/runners/github/services/pr_review_engine.py @@ -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: diff --git a/apps/backend/runners/github/services/prompt_manager.py b/apps/backend/runners/github/services/prompt_manager.py index a1e71c6a..882a8fe2 100644 --- a/apps/backend/runners/github/services/prompt_manager.py +++ b/apps/backend/runners/github/services/prompt_manager.py @@ -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" } diff --git a/apps/backend/runners/github/services/pydantic_models.py b/apps/backend/runners/github/services/pydantic_models.py index 42903dbf..9e3346de 100644 --- a/apps/backend/runners/github/services/pydantic_models.py +++ b/apps/backend/runners/github/services/pydantic_models.py @@ -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(