diff --git a/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py b/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py index d17beea337..ce73464a27 100644 --- a/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py +++ b/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py @@ -1289,12 +1289,30 @@ async def review(self, context: PRContext) -> PRReviewResult: f"{len(filtered_findings)} filtered" ) - # No confidence routing - validation is binary via finding-validator - unique_findings = validated_findings - logger.info(f"[PRReview] Final findings: {len(unique_findings)} validated") + # Separate active findings (drive verdict) from dismissed (shown in UI only) + active_findings = [] + dismissed_findings = [] + for f in validated_findings: + if f.validation_status == "dismissed_false_positive": + dismissed_findings.append(f) + else: + active_findings.append(f) + safe_print( + f"[ParallelOrchestrator] Final: {len(active_findings)} active, " + f"{len(dismissed_findings)} disputed by validator", + flush=True, + ) + logger.info( + f"[PRReview] Final findings: {len(active_findings)} active, " + f"{len(dismissed_findings)} disputed" + ) + + # All findings (active + dismissed) go in the result for UI display + all_review_findings = validated_findings logger.info( - f"[ParallelOrchestrator] Review complete: {len(unique_findings)} findings" + f"[ParallelOrchestrator] Review complete: {len(all_review_findings)} findings " + f"({len(active_findings)} active, {len(dismissed_findings)} disputed)" ) # Fetch CI status for verdict consideration @@ -1304,9 +1322,9 @@ async def review(self, context: PRContext) -> PRReviewResult: f"{ci_status.get('failing', 0)} failing, {ci_status.get('pending', 0)} pending" ) - # Generate verdict (includes merge conflict check, branch-behind check, and CI status) + # Generate verdict from ACTIVE findings only (dismissed don't affect verdict) verdict, verdict_reasoning, blockers = self._generate_verdict( - unique_findings, + active_findings, has_merge_conflicts=context.has_merge_conflicts, merge_state_status=context.merge_state_status, ci_status=ci_status, @@ -1317,7 +1335,7 @@ async def review(self, context: PRContext) -> PRReviewResult: verdict=verdict, verdict_reasoning=verdict_reasoning, blockers=blockers, - findings=unique_findings, + findings=all_review_findings, agents_invoked=agents_invoked, ) @@ -1362,7 +1380,7 @@ async def review(self, context: PRContext) -> PRReviewResult: pr_number=context.pr_number, repo=self.config.repo, success=True, - findings=unique_findings, + findings=all_review_findings, summary=summary, overall_status=overall_status, verdict=verdict, @@ -1937,12 +1955,38 @@ async def _validate_findings( validated_findings.append(finding) elif validation.validation_status == "dismissed_false_positive": - # Dismiss - do not include - dismissed_count += 1 - logger.info( - f"[PRReview] Dismissed {finding.id} as false positive: " - f"{validation.explanation[:100]}" - ) + # Protect cross-validated findings from dismissal — + # if multiple specialists independently found the same issue, + # a single validator should not override that consensus + if finding.cross_validated: + finding.validation_status = "confirmed_valid" + finding.validation_evidence = validation.code_evidence + finding.validation_explanation = ( + f"[Auto-kept: cross-validated by {len(finding.source_agents)} agents] " + f"{validation.explanation}" + ) + validated_findings.append(finding) + safe_print( + f"[FindingValidator] Kept cross-validated finding '{finding.title}' " + f"despite dismissal (agents={finding.source_agents})", + flush=True, + ) + else: + # Keep finding but mark as dismissed (user can see it in UI) + finding.validation_status = "dismissed_false_positive" + finding.validation_evidence = validation.code_evidence + finding.validation_explanation = validation.explanation + validated_findings.append(finding) + dismissed_count += 1 + safe_print( + f"[FindingValidator] Disputed '{finding.title}': " + f"{validation.explanation} (file={finding.file}:{finding.line})", + flush=True, + ) + logger.info( + f"[PRReview] Disputed {finding.id}: " + f"{validation.explanation[:200]}" + ) elif validation.validation_status == "needs_human_review": # Keep but flag @@ -2127,11 +2171,16 @@ def _generate_summary( sev = f.severity.value emoji = severity_emoji.get(sev, "⚪") + is_disputed = f.validation_status == "dismissed_false_positive" + # Finding header with location line_range = f"L{f.line}" if f.end_line and f.end_line != f.line: line_range = f"L{f.line}-L{f.end_line}" - lines.append(f"#### {emoji} [{sev.upper()}] {f.title}") + if is_disputed: + lines.append(f"#### ⚪ [DISPUTED] ~~{f.title}~~") + else: + lines.append(f"#### {emoji} [{sev.upper()}] {f.title}") lines.append(f"**File:** `{f.file}` ({line_range})") # Cross-validation badge @@ -2161,6 +2210,7 @@ def _generate_summary( status_label = { "confirmed_valid": "Confirmed", "needs_human_review": "Needs human review", + "dismissed_false_positive": "Disputed by validator", }.get(f.validation_status, f.validation_status) lines.append("") lines.append(f"**Validation:** {status_label}") @@ -2182,18 +2232,27 @@ def _generate_summary( lines.append("") - # Findings count summary + # Findings count summary (exclude dismissed from active count) + active_count = 0 + dismissed_count = 0 by_severity: dict[str, int] = {} for f in findings: + if f.validation_status == "dismissed_false_positive": + dismissed_count += 1 + continue + active_count += 1 sev = f.severity.value by_severity[sev] = by_severity.get(sev, 0) + 1 summary_parts = [] for sev in ["critical", "high", "medium", "low"]: if sev in by_severity: summary_parts.append(f"{by_severity[sev]} {sev}") - lines.append( - f"**Total:** {len(findings)} finding(s) ({', '.join(summary_parts)})" + count_text = ( + f"**Total:** {active_count} finding(s) ({', '.join(summary_parts)})" ) + if dismissed_count > 0: + count_text += f" + {dismissed_count} disputed" + lines.append(count_text) lines.append("") lines.append("---") diff --git a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts index 61200666dc..1f63b6c4ab 100644 --- a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts @@ -268,6 +268,10 @@ export interface PRReviewFinding { endLine?: number; suggestedFix?: string; fixable: boolean; + validationStatus?: "confirmed_valid" | "dismissed_false_positive" | "needs_human_review" | null; + validationExplanation?: string; + sourceAgents?: string[]; + crossValidated?: boolean; } /** @@ -1341,6 +1345,10 @@ function getReviewResult(project: Project, prNumber: number): PRReviewResult | n endLine: f.end_line, suggestedFix: f.suggested_fix, fixable: f.fixable ?? false, + validationStatus: f.validation_status ?? null, + validationExplanation: f.validation_explanation ?? undefined, + sourceAgents: f.source_agents ?? [], + crossValidated: f.cross_validated ?? false, })) ?? [], summary: data.summary ?? "", overallStatus: data.overall_status ?? "comment", diff --git a/apps/frontend/src/preload/api/modules/github-api.ts b/apps/frontend/src/preload/api/modules/github-api.ts index c27da235b3..bca2a8f129 100644 --- a/apps/frontend/src/preload/api/modules/github-api.ts +++ b/apps/frontend/src/preload/api/modules/github-api.ts @@ -376,6 +376,10 @@ export interface PRReviewFinding { endLine?: number; suggestedFix?: string; fixable: boolean; + validationStatus?: 'confirmed_valid' | 'dismissed_false_positive' | 'needs_human_review' | null; + validationExplanation?: string; + sourceAgents?: string[]; + crossValidated?: boolean; } /** diff --git a/apps/frontend/src/renderer/components/github-prs/components/FindingItem.tsx b/apps/frontend/src/renderer/components/github-prs/components/FindingItem.tsx index f3be2e4243..47dcde99c5 100644 --- a/apps/frontend/src/renderer/components/github-prs/components/FindingItem.tsx +++ b/apps/frontend/src/renderer/components/github-prs/components/FindingItem.tsx @@ -14,6 +14,7 @@ interface FindingItemProps { finding: PRReviewFinding; selected: boolean; posted?: boolean; + disputed?: boolean; onToggle: () => void; } @@ -33,7 +34,7 @@ function getCategoryTranslationKey(category: string): string { return categoryMap[category.toLowerCase()] || category; } -export function FindingItem({ finding, selected, posted = false, onToggle }: FindingItemProps) { +export function FindingItem({ finding, selected, posted = false, disputed = false, onToggle }: FindingItemProps) { const { t } = useTranslation('common'); const CategoryIcon = getCategoryIcon(finding.category); @@ -45,8 +46,9 @@ export function FindingItem({ finding, selected, posted = false, onToggle }: Fin
{/* Finding Header */} @@ -72,6 +74,16 @@ export function FindingItem({ finding, selected, posted = false, onToggle }: Fin {t('prReview.posted')} )} + {disputed && ( + + {t('prReview.disputed')} + + )} + {finding.crossValidated && finding.sourceAgents && finding.sourceAgents.length > 1 && ( + + {t('prReview.crossValidatedBy', { count: finding.sourceAgents.length })} + + )} {finding.title} @@ -79,6 +91,11 @@ export function FindingItem({ finding, selected, posted = false, onToggle }: Fin

{finding.description}

+ {disputed && finding.validationExplanation && ( +

+ {finding.validationExplanation} +

+ )}
{finding.file}:{finding.line} diff --git a/apps/frontend/src/renderer/components/github-prs/components/FindingsSummary.tsx b/apps/frontend/src/renderer/components/github-prs/components/FindingsSummary.tsx index 2259b35aad..eb28a0e06e 100644 --- a/apps/frontend/src/renderer/components/github-prs/components/FindingsSummary.tsx +++ b/apps/frontend/src/renderer/components/github-prs/components/FindingsSummary.tsx @@ -9,9 +9,10 @@ import type { PRReviewFinding } from '../hooks/useGitHubPRs'; interface FindingsSummaryProps { findings: PRReviewFinding[]; selectedCount: number; + disputedCount?: number; } -export function FindingsSummary({ findings, selectedCount }: FindingsSummaryProps) { +export function FindingsSummary({ findings, selectedCount, disputedCount = 0 }: FindingsSummaryProps) { const { t } = useTranslation('common'); // Count findings by severity @@ -46,6 +47,11 @@ export function FindingsSummary({ findings, selectedCount }: FindingsSummaryProp {counts.low} {t('prReview.severity.low')} )} + {disputedCount > 0 && ( + + {disputedCount} {t('prReview.disputed')} + + )}
{t('prReview.selectedOfTotal', { selected: selectedCount, total: counts.total })} diff --git a/apps/frontend/src/renderer/components/github-prs/components/ReviewFindings.tsx b/apps/frontend/src/renderer/components/github-prs/components/ReviewFindings.tsx index cd5e335669..8a9bba4003 100644 --- a/apps/frontend/src/renderer/components/github-prs/components/ReviewFindings.tsx +++ b/apps/frontend/src/renderer/components/github-prs/components/ReviewFindings.tsx @@ -7,6 +7,7 @@ * - Quick select actions (Critical/High, All, None) * - Collapsible sections for less important findings * - Visual summary of finding counts + * - Disputed findings shown in a separate collapsible section */ import { useState, useMemo } from 'react'; @@ -16,6 +17,9 @@ import { CheckSquare, Square, Send, + ChevronDown, + ChevronRight, + ShieldQuestion, } from 'lucide-react'; import { useTranslation } from 'react-i18next'; import { Button } from '../../ui/button'; @@ -47,6 +51,7 @@ export function ReviewFindings({ const [expandedSections, setExpandedSections] = useState>( new Set(['critical', 'high']) // Critical and High expanded by default ); + const [disputedExpanded, setDisputedExpanded] = useState(false); // Filter out posted findings - only show unposted findings for selection const unpostedFindings = useMemo(() => @@ -54,10 +59,24 @@ export function ReviewFindings({ [findings, postedIds] ); + // Split unposted findings into active vs disputed (single pass) + const { activeFindings, disputedFindings } = useMemo(() => { + const active: PRReviewFinding[] = []; + const disputed: PRReviewFinding[] = []; + for (const finding of unpostedFindings) { + if (finding.validationStatus === 'dismissed_false_positive') { + disputed.push(finding); + } else { + active.push(finding); + } + } + return { activeFindings: active, disputedFindings: disputed }; + }, [unpostedFindings]); + // Check if all findings are posted const allFindingsPosted = findings.length > 0 && unpostedFindings.length === 0; - // Group unposted findings by severity (only show findings that haven't been posted) + // Group ACTIVE unposted findings by severity (disputed go in their own section) const groupedFindings = useMemo(() => { const groups: Record = { critical: [], @@ -66,7 +85,7 @@ export function ReviewFindings({ low: [], }; - for (const finding of unpostedFindings) { + for (const finding of activeFindings) { const severity = finding.severity as SeverityGroup; if (groups[severity]) { groups[severity].push(finding); @@ -74,20 +93,20 @@ export function ReviewFindings({ } return groups; - }, [unpostedFindings]); + }, [activeFindings]); - // Count by severity (unposted findings only) + // Count by severity (active findings only) const counts = useMemo(() => ({ critical: groupedFindings.critical.length, high: groupedFindings.high.length, medium: groupedFindings.medium.length, low: groupedFindings.low.length, - total: unpostedFindings.length, + total: activeFindings.length, important: groupedFindings.critical.length + groupedFindings.high.length, posted: postedIds.size, - }), [groupedFindings, unpostedFindings.length, postedIds.size]); + }), [groupedFindings, activeFindings.length, postedIds.size]); - // Selection hooks - use unposted findings only + // Selection hooks - use ACTIVE unposted findings only (Select All excludes disputed) const { toggleFinding, selectAll, @@ -95,7 +114,7 @@ export function ReviewFindings({ selectImportant, toggleSeverityGroup, } = useFindingSelection({ - findings: unpostedFindings, + findings: activeFindings, selectedIds, onSelectionChange, groupedFindings, @@ -114,6 +133,12 @@ export function ReviewFindings({ }); }; + // Count only active findings that are selected (excludes disputed from count) + const selectedActiveCount = useMemo( + () => activeFindings.filter(f => selectedIds.has(f.id)).length, + [activeFindings, selectedIds] + ); + // When all findings have been posted, show a success message instead of the selection UI if (allFindingsPosted) { return ( @@ -131,10 +156,11 @@ export function ReviewFindings({ return (
- {/* Summary Stats Bar - show unposted findings only */} + {/* Summary Stats Bar - show active findings + disputed count */} {/* Quick Select Actions */} @@ -170,7 +196,7 @@ export function ReviewFindings({
- {/* Grouped Findings (unposted only) */} + {/* Grouped Findings (active only) */}
{SEVERITY_ORDER.map((severity) => { const group = groupedFindings[severity]; @@ -220,6 +246,48 @@ export function ReviewFindings({ })}
+ {/* Disputed Findings Section */} + {disputedFindings.length > 0 && ( +
+ {/* Disputed Header */} + + + {/* Disputed Content */} + {disputedExpanded && ( +
+

+ {t('prReview.disputedSectionHint')} +

+ {disputedFindings.map((finding) => ( + toggleFinding(finding.id)} + /> + ))} +
+ )} +
+ )} + {/* Empty State - no findings at all */} {findings.length === 0 && (
diff --git a/apps/frontend/src/renderer/components/github-prs/hooks/useFindingSelection.ts b/apps/frontend/src/renderer/components/github-prs/hooks/useFindingSelection.ts index 1b14eb0ca4..b06ea440ae 100644 --- a/apps/frontend/src/renderer/components/github-prs/hooks/useFindingSelection.ts +++ b/apps/frontend/src/renderer/components/github-prs/hooks/useFindingSelection.ts @@ -30,21 +30,31 @@ export function useFindingSelection({ onSelectionChange(next); }, [selectedIds, onSelectionChange]); - // Select all findings + // Select all findings (preserving any disputed selections not in active findings) const selectAll = useCallback(() => { - onSelectionChange(new Set(findings.map(f => f.id))); - }, [findings, onSelectionChange]); + const activeIds = new Set(findings.map(f => f.id)); + // Preserve selections for disputed findings (IDs not in active findings list) + for (const id of selectedIds) { + if (!findings.some(f => f.id === id)) activeIds.add(id); + } + onSelectionChange(activeIds); + }, [findings, selectedIds, onSelectionChange]); // Clear all selections const selectNone = useCallback(() => { onSelectionChange(new Set()); }, [onSelectionChange]); - // Select only critical and high severity findings + // Select only critical and high severity findings (preserving disputed selections) const selectImportant = useCallback(() => { const important = [...groupedFindings.critical, ...groupedFindings.high]; - onSelectionChange(new Set(important.map(f => f.id))); - }, [groupedFindings, onSelectionChange]); + const importantIds = new Set(important.map(f => f.id)); + // Preserve selections for disputed findings (IDs not in active findings list) + for (const id of selectedIds) { + if (!findings.some(f => f.id === id)) importantIds.add(id); + } + onSelectionChange(importantIds); + }, [groupedFindings, findings, selectedIds, onSelectionChange]); // Toggle entire severity group selection const toggleSeverityGroup = useCallback((severity: SeverityGroup) => { diff --git a/apps/frontend/src/shared/i18n/locales/en/common.json b/apps/frontend/src/shared/i18n/locales/en/common.json index 8078a129c5..f504232db0 100644 --- a/apps/frontend/src/shared/i18n/locales/en/common.json +++ b/apps/frontend/src/shared/i18n/locales/en/common.json @@ -402,6 +402,10 @@ "verifyChanges": "Verify Changes", "verifyAnyway": "Verify", "runFollowupAnyway": "Run follow-up verification even though no files overlap", + "disputed": "Disputed", + "disputedByValidator": "Disputed by Validator ({{count}})", + "crossValidatedBy": "Confirmed by {{count}} agents", + "disputedSectionHint": "These findings were reported by specialists but disputed by the validator. You can still select and post them.", "logs": { "agentActivity": "Agent Activity", "showMore": "Show {{count}} more", diff --git a/apps/frontend/src/shared/i18n/locales/fr/common.json b/apps/frontend/src/shared/i18n/locales/fr/common.json index d2c452b756..b1ec2c6f8a 100644 --- a/apps/frontend/src/shared/i18n/locales/fr/common.json +++ b/apps/frontend/src/shared/i18n/locales/fr/common.json @@ -402,6 +402,10 @@ "blockedStatusMessageTitle": "## 🤖 Auto Claude PR Review", "blockedStatusMessageFooter": "*This review identified blockers that must be resolved before merge. Generated by Auto Claude.*", "failedPostBlockedStatus": "Échec de la publication du statut", + "disputed": "Contesté", + "disputedByValidator": "Contesté par le validateur ({{count}})", + "crossValidatedBy": "Confirmé par {{count}} agents", + "disputedSectionHint": "Ces résultats ont été signalés par les spécialistes mais contestés par le validateur. Vous pouvez toujours les sélectionner et les publier.", "logs": { "agentActivity": "Activité des agents", "showMore": "Afficher {{count}} de plus",