diff --git a/apps/backend/runners/github/orchestrator.py b/apps/backend/runners/github/orchestrator.py index e3b797f7..b98fb223 100644 --- a/apps/backend/runners/github/orchestrator.py +++ b/apps/backend/runners/github/orchestrator.py @@ -450,6 +450,7 @@ class GitHubOrchestrator: structural_issues=structural_issues, ai_triages=ai_triages, risk_assessment=risk_assessment, + ci_status=ci_status, ) # Get HEAD SHA for follow-up review tracking @@ -1026,6 +1027,7 @@ class GitHubOrchestrator: structural_issues: list[StructuralIssue], ai_triages: list[AICommentTriage], risk_assessment: dict, + ci_status: dict | None = None, ) -> str: """Generate enhanced summary with verdict, risk, and actionable next steps.""" verdict_emoji = { @@ -1035,8 +1037,19 @@ class GitHubOrchestrator: MergeVerdict.BLOCKED: "🔴", } + # Generate bottom line for quick scanning + bottom_line = self._generate_bottom_line( + verdict=verdict, + ci_status=ci_status, + blockers=blockers, + findings=findings, + ) + lines = [ f"### Merge Verdict: {verdict_emoji.get(verdict, '⚪')} {verdict.value.upper().replace('_', ' ')}", + "", + f"> {bottom_line}", + "", verdict_reasoning, "", "### Risk Assessment", @@ -1103,6 +1116,70 @@ class GitHubOrchestrator: return "\n".join(lines) + def _generate_bottom_line( + self, + verdict: MergeVerdict, + ci_status: dict | None, + blockers: list[str], + findings: list[PRReviewFinding], + ) -> str: + """Generate a one-line summary for quick scanning at the top of the review.""" + # Check CI status + ci = ci_status or {} + pending_ci = ci.get("pending", 0) + failing_ci = ci.get("failing", 0) + awaiting_approval = ci.get("awaiting_approval", 0) + + # Count blocking findings and issues + blocking_findings = [ + f for f in findings if f.severity.value in ("critical", "high", "medium") + ] + code_blockers = [ + b for b in blockers if "CI" not in b and "Merge Conflict" not in b + ] + has_merge_conflicts = any("Merge Conflict" in b for b in blockers) + + # Determine the bottom line based on verdict and context + if verdict == MergeVerdict.READY_TO_MERGE: + return ( + "**✅ Ready to merge** - All checks passing, no blocking issues found." + ) + + elif verdict == MergeVerdict.BLOCKED: + if has_merge_conflicts: + return "**🔴 Blocked** - Merge conflicts must be resolved before merge." + elif failing_ci > 0: + return f"**🔴 Blocked** - {failing_ci} CI check(s) failing. Fix CI before merge." + elif awaiting_approval > 0: + return "**🔴 Blocked** - Awaiting maintainer approval for fork PR workflow." + elif blocking_findings: + return f"**🔴 Blocked** - {len(blocking_findings)} critical/high/medium issue(s) must be fixed." + else: + return "**🔴 Blocked** - Critical issues must be resolved before merge." + + elif verdict == MergeVerdict.NEEDS_REVISION: + # Key insight: distinguish "waiting on CI" from "needs code fixes" + # Check code issues FIRST before checking pending CI + if blocking_findings: + return f"**🟠 Needs revision** - {len(blocking_findings)} issue(s) require attention." + elif code_blockers: + return f"**🟠 Needs revision** - {len(code_blockers)} structural/other issue(s) require attention." + elif pending_ci > 0: + # Only show "Ready once CI passes" when no code issues exist + return f"**⏳ Ready once CI passes** - {pending_ci} check(s) pending, no blocking code issues." + else: + return "**🟠 Needs revision** - See details below." + + elif verdict == MergeVerdict.MERGE_WITH_CHANGES: + if pending_ci > 0: + return ( + "**🟡 Can merge once CI passes** - Minor suggestions, no blockers." + ) + else: + return "**🟡 Can merge** - Minor suggestions noted, no blockers." + + return "**📝 Review complete** - See details below." + def _format_review_body(self, result: PRReviewResult) -> str: """Format the review body for posting to GitHub.""" return result.summary diff --git a/apps/backend/runners/github/services/parallel_followup_reviewer.py b/apps/backend/runners/github/services/parallel_followup_reviewer.py index 9b05a481..65409b92 100644 --- a/apps/backend/runners/github/services/parallel_followup_reviewer.py +++ b/apps/backend/runners/github/services/parallel_followup_reviewer.py @@ -627,6 +627,7 @@ The SDK will run invoked agents in parallel automatically. dismissed_false_positive_count=dismissed_count, confirmed_valid_count=confirmed_count, needs_human_review_count=needs_human_count, + ci_status=context.ci_status, ) # Map verdict to overall_status @@ -977,13 +978,15 @@ The SDK will run invoked agents in parallel automatically. dismissed_false_positive_count: int = 0, confirmed_valid_count: int = 0, needs_human_review_count: int = 0, + ci_status: dict | None = None, ) -> str: """Generate a human-readable summary of the follow-up review.""" + # Use same emojis as orchestrator.py for consistency status_emoji = { MergeVerdict.READY_TO_MERGE: "✅", - MergeVerdict.MERGE_WITH_CHANGES: "⚠️", - MergeVerdict.NEEDS_REVISION: "🔄", - MergeVerdict.BLOCKED: "🚫", + MergeVerdict.MERGE_WITH_CHANGES: "🟡", + MergeVerdict.NEEDS_REVISION: "🟠", + MergeVerdict.BLOCKED: "🔴", } emoji = status_emoji.get(verdict, "📝") @@ -991,6 +994,15 @@ The SDK will run invoked agents in parallel automatically. ", ".join(agents_invoked) if agents_invoked else "orchestrator only" ) + # Generate a prominent bottom-line summary for quick scanning + bottom_line = self._generate_bottom_line( + verdict=verdict, + ci_status=ci_status, + unresolved_count=unresolved_count, + new_count=new_count, + blockers=blockers, + ) + # Build validation section if there are validation results validation_section = "" if ( @@ -1016,6 +1028,8 @@ The SDK will run invoked agents in parallel automatically. summary = f"""## {emoji} Follow-up Review: {verdict.value.replace("_", " ").title()} +> {bottom_line} + ### Resolution Status - ✅ **Resolved**: {resolved_count} previous findings addressed - ❌ **Unresolved**: {unresolved_count} previous findings remain @@ -1031,3 +1045,65 @@ Agents invoked: {agents_str} *This is an AI-generated follow-up review using parallel specialist analysis with finding validation.* """ return summary + + def _generate_bottom_line( + self, + verdict: MergeVerdict, + ci_status: dict | None, + unresolved_count: int, + new_count: int, + blockers: list[str], + ) -> str: + """Generate a one-line summary for quick scanning at the top of the review.""" + # Check CI status + ci = ci_status or {} + pending_ci = ci.get("pending", 0) + failing_ci = ci.get("failing", 0) + awaiting_approval = ci.get("awaiting_approval", 0) + + # Count blocking issues (excluding CI-related ones) + code_blockers = [ + b for b in blockers if "CI" not in b and "Merge Conflict" not in b + ] + has_merge_conflicts = any("Merge Conflict" in b for b in blockers) + + # Determine the bottom line based on verdict and context + if verdict == MergeVerdict.READY_TO_MERGE: + return "**✅ Ready to merge** - All checks passing and findings addressed." + + elif verdict == MergeVerdict.BLOCKED: + if has_merge_conflicts: + return "**🔴 Blocked** - Merge conflicts must be resolved before merge." + elif failing_ci > 0: + return f"**🔴 Blocked** - {failing_ci} CI check(s) failing. Fix CI before merge." + elif awaiting_approval > 0: + return "**🔴 Blocked** - Awaiting maintainer approval for fork PR workflow." + elif code_blockers: + return f"**🔴 Blocked** - {len(code_blockers)} blocking issue(s) require fixes." + else: + return "**🔴 Blocked** - Critical issues must be resolved before merge." + + elif verdict == MergeVerdict.NEEDS_REVISION: + # Key insight: distinguish "waiting on CI" from "needs code fixes" + # Check code issues FIRST before checking pending CI + if unresolved_count > 0: + return f"**🟠 Needs revision** - {unresolved_count} unresolved finding(s) from previous review." + elif code_blockers: + return f"**🟠 Needs revision** - {len(code_blockers)} blocking issue(s) require fixes." + elif new_count > 0: + return f"**🟠 Needs revision** - {new_count} new issue(s) found in recent changes." + elif pending_ci > 0: + # Only show "Ready once CI passes" when no code issues exist + return f"**⏳ Ready once CI passes** - {pending_ci} check(s) pending, all findings addressed." + else: + return "**🟠 Needs revision** - See details below." + + elif verdict == MergeVerdict.MERGE_WITH_CHANGES: + if pending_ci > 0: + return ( + "**🟡 Can merge once CI passes** - Minor suggestions, no blockers." + ) + else: + return "**🟡 Can merge** - Minor suggestions noted, no blockers." + + return "**📝 Review complete** - See details below."