diff --git a/frontend b/frontend index a2a7f408..b2d8460a 160000 --- a/frontend +++ b/frontend @@ -1 +1 @@ -Subproject commit a2a7f408ea448913080bf4bc06493e45cc39ca34 +Subproject commit b2d8460a6af5fe3b2d0f79e672522e9cc6f436ab diff --git a/java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/dto/request/ai/AiRequestPreviousIssueDTO.java b/java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/dto/request/ai/AiRequestPreviousIssueDTO.java index 18c86685..8d166455 100644 --- a/java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/dto/request/ai/AiRequestPreviousIssueDTO.java +++ b/java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/dto/request/ai/AiRequestPreviousIssueDTO.java @@ -7,14 +7,15 @@ public record AiRequestPreviousIssueDTO( String id, String type, // security|quality|performance|style String severity, // critical|high|medium|low|info - String title, + String reason, String suggestedFixDescription, + String suggestedFixDiff, String file, Integer line, String branch, String pullRequestId, String status, // open|resolved|ignored - String issueCategory + String category ) { public static AiRequestPreviousIssueDTO fromEntity(CodeAnalysisIssue issue) { String categoryStr = issue.getIssueCategory() != null @@ -23,9 +24,10 @@ public static AiRequestPreviousIssueDTO fromEntity(CodeAnalysisIssue issue) { return new AiRequestPreviousIssueDTO( String.valueOf(issue.getId()), categoryStr, - issue.getSeverity() != null ? issue.getSeverity().name().toLowerCase() : null, + issue.getSeverity() != null ? issue.getSeverity().name() : null, issue.getReason(), issue.getSuggestedFixDescription(), + issue.getSuggestedFixDiff(), issue.getFilePath(), issue.getLineNumber(), issue.getAnalysis() == null ? null : issue.getAnalysis().getBranchName(), diff --git a/java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/processor/analysis/BranchAnalysisProcessor.java b/java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/processor/analysis/BranchAnalysisProcessor.java index 128785a5..7f51d813 100644 --- a/java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/processor/analysis/BranchAnalysisProcessor.java +++ b/java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/processor/analysis/BranchAnalysisProcessor.java @@ -372,7 +372,7 @@ private void mapCodeAnalysisIssuesToBranch(Set changedFiles, Branch bran if (existing.isPresent()) { bc = existing.get(); bc.setSeverity(issue.getSeverity()); - branchIssueRepository.save(bc); + branchIssueRepository.saveAndFlush(bc); } else { bc = new BranchIssue(); bc.setBranch(branch); @@ -380,7 +380,7 @@ private void mapCodeAnalysisIssuesToBranch(Set changedFiles, Branch bran bc.setResolved(issue.isResolved()); bc.setSeverity(issue.getSeverity()); bc.setFirstDetectedPrNumber(issue.getAnalysis() != null ? issue.getAnalysis().getPrNumber() : null); - branchIssueRepository.save(bc); + branchIssueRepository.saveAndFlush(bc); } } } diff --git a/java-ecosystem/libs/core/src/main/resources/db/migration/1.1.0/V1.1.0__add_info_severity_to_constraint.sql b/java-ecosystem/libs/core/src/main/resources/db/migration/1.1.0/V1.1.0__add_info_severity_to_constraint.sql new file mode 100644 index 00000000..1b2a9e0d --- /dev/null +++ b/java-ecosystem/libs/core/src/main/resources/db/migration/1.1.0/V1.1.0__add_info_severity_to_constraint.sql @@ -0,0 +1,9 @@ +-- Add INFO to the severity check constraint on code_analysis_issue table +-- This is needed because INFO was added to IssueSeverity enum but the constraint wasn't updated + +-- Drop the existing constraint and recreate with INFO included +ALTER TABLE code_analysis_issue DROP CONSTRAINT IF EXISTS code_analysis_issue_severity_check; + +ALTER TABLE code_analysis_issue +ADD CONSTRAINT code_analysis_issue_severity_check +CHECK (severity IN ('HIGH', 'MEDIUM', 'LOW', 'INFO', 'RESOLVED')); diff --git a/java-ecosystem/libs/vcs-client/src/main/java/org/rostilos/codecrow/vcsclient/bitbucket/cloud/actions/CommentOnBitbucketCloudAction.java b/java-ecosystem/libs/vcs-client/src/main/java/org/rostilos/codecrow/vcsclient/bitbucket/cloud/actions/CommentOnBitbucketCloudAction.java index 7c5a6f85..37550cfb 100644 --- a/java-ecosystem/libs/vcs-client/src/main/java/org/rostilos/codecrow/vcsclient/bitbucket/cloud/actions/CommentOnBitbucketCloudAction.java +++ b/java-ecosystem/libs/vcs-client/src/main/java/org/rostilos/codecrow/vcsclient/bitbucket/cloud/actions/CommentOnBitbucketCloudAction.java @@ -33,6 +33,15 @@ public CommentOnBitbucketCloudAction( } public void postSummaryResult(String textContent) throws IOException { + postSummaryResultWithId(textContent); + } + + /** + * Post a summary result comment and return the comment ID. + * @param textContent The markdown content to post + * @return The ID of the created comment + */ + public String postSummaryResultWithId(String textContent) throws IOException { String workspace = vcsRepoInfo.getRepoWorkspace(); String repoSlug = vcsRepoInfo.getRepoSlug(); @@ -65,6 +74,15 @@ public void postSummaryResult(String textContent) throws IOException { try (Response response = authorizedOkHttpClient.newCall(req).execute()) { validate(response); + // Parse response to get comment ID + if (response.body() != null) { + String responseBody = response.body().string(); + JsonNode jsonNode = objectMapper.readTree(responseBody); + if (jsonNode.has("id")) { + return String.valueOf(jsonNode.get("id").asInt()); + } + } + return null; } } diff --git a/java-ecosystem/libs/vcs-client/src/main/java/org/rostilos/codecrow/vcsclient/bitbucket/model/report/formatters/MarkdownAnalysisFormatter.java b/java-ecosystem/libs/vcs-client/src/main/java/org/rostilos/codecrow/vcsclient/bitbucket/model/report/formatters/MarkdownAnalysisFormatter.java index de81b7f8..00dc7be2 100644 --- a/java-ecosystem/libs/vcs-client/src/main/java/org/rostilos/codecrow/vcsclient/bitbucket/model/report/formatters/MarkdownAnalysisFormatter.java +++ b/java-ecosystem/libs/vcs-client/src/main/java/org/rostilos/codecrow/vcsclient/bitbucket/model/report/formatters/MarkdownAnalysisFormatter.java @@ -30,12 +30,14 @@ public class MarkdownAnalysisFormatter implements AnalysisFormatter { ); private final boolean useGitHubSpoilers; + private final boolean includeDetailedIssues; /** - * Default constructor - no spoilers (for Bitbucket) + * Default constructor - no spoilers (for Bitbucket), no detailed issues in summary */ public MarkdownAnalysisFormatter() { this.useGitHubSpoilers = false; + this.includeDetailedIssues = false; } /** @@ -44,6 +46,17 @@ public MarkdownAnalysisFormatter() { */ public MarkdownAnalysisFormatter(boolean useGitHubSpoilers) { this.useGitHubSpoilers = useGitHubSpoilers; + this.includeDetailedIssues = false; + } + + /** + * Full constructor with all options + * @param useGitHubSpoilers true for GitHub (uses details/summary), false for Bitbucket + * @param includeDetailedIssues true to include detailed issues in summary, false for separate comment + */ + public MarkdownAnalysisFormatter(boolean useGitHubSpoilers, boolean includeDetailedIssues) { + this.useGitHubSpoilers = useGitHubSpoilers; + this.includeDetailedIssues = includeDetailedIssues; } @Override @@ -104,15 +117,19 @@ public String format(AnalysisSummary summary) { md.append("\n"); - md.append("### Detailed Issues\n\n"); + // Only include detailed issues if explicitly requested + if (includeDetailedIssues) { + md.append("### Detailed Issues\n\n"); - appendIssuesBySevertiy(md, summary.getIssues(), IssueSeverity.HIGH, EMOJI_HIGH, "High Severity Issues"); - appendIssuesBySevertiy(md, summary.getIssues(), IssueSeverity.MEDIUM, EMOJI_MEDIUM, "Medium Severity Issues"); - appendIssuesBySevertiy(md, summary.getIssues(), IssueSeverity.LOW, EMOJI_LOW, "Low Severity Issues"); - appendIssuesBySevertiy(md, summary.getIssues(), IssueSeverity.INFO, EMOJI_INFO, "Informational Notes"); + appendIssuesBySevertiy(md, summary.getIssues(), IssueSeverity.HIGH, EMOJI_HIGH, "High Severity Issues"); + appendIssuesBySevertiy(md, summary.getIssues(), IssueSeverity.MEDIUM, EMOJI_MEDIUM, "Medium Severity Issues"); + appendIssuesBySevertiy(md, summary.getIssues(), IssueSeverity.LOW, EMOJI_LOW, "Low Severity Issues"); + appendIssuesBySevertiy(md, summary.getIssues(), IssueSeverity.INFO, EMOJI_INFO, "Informational Notes"); + } } - if (!summary.getFileIssueCount().isEmpty()) { + // Only include files affected if detailed issues are included + if (includeDetailedIssues && !summary.getFileIssueCount().isEmpty()) { md.append("### Files Affected\n"); summary.getFileIssueCount().entrySet().stream() .sorted((a, b) -> b.getValue().compareTo(a.getValue())) @@ -144,6 +161,58 @@ public String format(AnalysisSummary summary) { return md.toString(); } + /** + * Format only the detailed issues section for posting as a separate comment. + * For GitHub, wraps all issues in a single collapsible spoiler. + * @param summary The analysis summary containing issues + * @return Markdown-formatted string with detailed issues, or empty string if no issues + */ + public String formatDetailedIssues(AnalysisSummary summary) { + if (summary.getIssues() == null || summary.getIssues().isEmpty()) { + return ""; + } + + StringBuilder md = new StringBuilder(); + + int totalIssues = summary.getIssues().size(); + + if (useGitHubSpoilers) { + // GitHub: wrap ALL issues in a single collapsible section + md.append("
\n"); + md.append(String.format("📋 Detailed Issues (%d)\n\n", totalIssues)); + } else { + // Bitbucket: regular header (no spoiler support) + md.append("## 📋 Detailed Issues\n\n"); + } + + appendIssuesBySevertiy(md, summary.getIssues(), IssueSeverity.HIGH, EMOJI_HIGH, "High Severity Issues"); + appendIssuesBySevertiy(md, summary.getIssues(), IssueSeverity.MEDIUM, EMOJI_MEDIUM, "Medium Severity Issues"); + appendIssuesBySevertiy(md, summary.getIssues(), IssueSeverity.LOW, EMOJI_LOW, "Low Severity Issues"); + appendIssuesBySevertiy(md, summary.getIssues(), IssueSeverity.INFO, EMOJI_INFO, "Informational Notes"); + + if (!summary.getFileIssueCount().isEmpty()) { + md.append("### Files Affected\n"); + summary.getFileIssueCount().entrySet().stream() + .sorted((a, b) -> b.getValue().compareTo(a.getValue())) + .limit(10) + .forEach(entry -> { + String fileName = getShortFileName(entry.getKey()); + md.append(String.format("- **%s**: %d issue%s\n", + fileName, + entry.getValue(), + entry.getValue() == 1 ? "" : "s")); + }); + md.append("\n"); + } + + if (useGitHubSpoilers) { + // Close the details tag for GitHub + md.append("
\n"); + } + + return md.toString(); + } + private void appendIssuesBySevertiy(StringBuilder md, List issues, IssueSeverity severity, String emoji, String title) { List severityIssues = issues.stream() diff --git a/java-ecosystem/libs/vcs-client/src/main/java/org/rostilos/codecrow/vcsclient/bitbucket/service/ReportGenerator.java b/java-ecosystem/libs/vcs-client/src/main/java/org/rostilos/codecrow/vcsclient/bitbucket/service/ReportGenerator.java index b88951cb..62602be6 100644 --- a/java-ecosystem/libs/vcs-client/src/main/java/org/rostilos/codecrow/vcsclient/bitbucket/service/ReportGenerator.java +++ b/java-ecosystem/libs/vcs-client/src/main/java/org/rostilos/codecrow/vcsclient/bitbucket/service/ReportGenerator.java @@ -213,6 +213,22 @@ public String createMarkdownSummary(CodeAnalysis analysis, AnalysisSummary summa } } + /** + * Creates markdown-formatted detailed issues for posting as a separate comment. + * + * @param summary The analysis summary + * @param useGitHubSpoilers true for GitHub (uses details/summary for collapsible fixes), false for Bitbucket + * @return Markdown-formatted string with detailed issues, or empty string if no issues + */ + public String createDetailedIssuesMarkdown(AnalysisSummary summary, boolean useGitHubSpoilers) { + try { + return new MarkdownAnalysisFormatter(useGitHubSpoilers).formatDetailedIssues(summary); + } catch (Exception e) { + log.error("Error creating detailed issues markdown: {}", e.getMessage(), e); + return ""; + } + } + /** * Creates a plain text summary for pull request comments * diff --git a/java-ecosystem/libs/vcs-client/src/main/java/org/rostilos/codecrow/vcsclient/github/actions/CommentOnPullRequestAction.java b/java-ecosystem/libs/vcs-client/src/main/java/org/rostilos/codecrow/vcsclient/github/actions/CommentOnPullRequestAction.java index 54e58eed..b7efaf41 100644 --- a/java-ecosystem/libs/vcs-client/src/main/java/org/rostilos/codecrow/vcsclient/github/actions/CommentOnPullRequestAction.java +++ b/java-ecosystem/libs/vcs-client/src/main/java/org/rostilos/codecrow/vcsclient/github/actions/CommentOnPullRequestAction.java @@ -200,4 +200,66 @@ public void deletePreviousComments(String owner, String repo, int prNumber, Stri } } } + + /** + * Create a Pull Request Review with a summary body and optional inline comments. + * This creates a review that appears in the "Conversation" tab with connected comments. + * + * @param owner Repository owner + * @param repo Repository name + * @param pullRequestNumber PR number + * @param commitId The SHA of the commit to review + * @param body The main review body/summary (appears as the review comment) + * @param event Review event: COMMENT, APPROVE, REQUEST_CHANGES + * @param comments List of inline comments, each with: path, line, body + * @return The review ID + */ + public String createPullRequestReview(String owner, String repo, int pullRequestNumber, + String commitId, String body, String event, + List> comments) throws IOException { + String apiUrl = String.format("%s/repos/%s/%s/pulls/%d/reviews", + GitHubConfig.API_BASE, owner, repo, pullRequestNumber); + + Map payload = new HashMap<>(); + payload.put("commit_id", commitId); + payload.put("body", body); + payload.put("event", event); // COMMENT, APPROVE, or REQUEST_CHANGES + + if (comments != null && !comments.isEmpty()) { + payload.put("comments", comments); + } + + Request req = new Request.Builder() + .url(apiUrl) + .header("Accept", "application/vnd.github+json") + .header("X-GitHub-Api-Version", "2022-11-28") + .post(RequestBody.create(objectMapper.writeValueAsString(payload), JSON)) + .build(); + + log.debug("Creating PR review on GitHub: {}", apiUrl); + + try (Response resp = authorizedOkHttpClient.newCall(req).execute()) { + if (!resp.isSuccessful()) { + String respBody = resp.body() != null ? resp.body().string() : ""; + String msg = String.format("Failed to create PR review: %d - %s", resp.code(), respBody); + log.warn(msg); + throw new IOException(msg); + } + + String respBody = resp.body() != null ? resp.body().string() : ""; + Map responseMap = objectMapper.readValue(respBody, new TypeReference>() {}); + Number id = (Number) responseMap.get("id"); + log.info("Created PR review {} on PR #{}", id, pullRequestNumber); + return id != null ? String.valueOf(id.longValue()) : null; + } + } + + /** + * Create a simplified PR review with just summary and detailed issues body. + * Both will appear as connected review comments. + */ + public String createReviewWithSummary(String owner, String repo, int pullRequestNumber, + String commitId, String summaryBody) throws IOException { + return createPullRequestReview(owner, repo, pullRequestNumber, commitId, summaryBody, "COMMENT", null); + } } diff --git a/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/service/bitbucket/BitbucketReportingService.java b/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/service/bitbucket/BitbucketReportingService.java index a88f0099..1e93ca89 100644 --- a/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/service/bitbucket/BitbucketReportingService.java +++ b/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/service/bitbucket/BitbucketReportingService.java @@ -33,6 +33,7 @@ public class BitbucketReportingService implements VcsReportingService { private static final Logger log = LoggerFactory.getLogger(BitbucketReportingService.class); private static final String CODECROW_REVIEW_MARKER = ""; + private static final String CODECROW_ISSUES_MARKER = ""; private final ReportGenerator reportGenerator; private final VcsClientProvider vcsClientProvider; @@ -121,6 +122,7 @@ public void postAnalysisResults( AnalysisSummary summary = reportGenerator.createAnalysisSummary(codeAnalysis, platformPrEntityId); String markdownSummary = reportGenerator.createMarkdownSummary(codeAnalysis, summary); + String detailedIssuesMarkdown = reportGenerator.createDetailedIssuesMarkdown(summary, false); CodeInsightsReport report = reportGenerator.createCodeInsightsReport( summary, codeAnalysis @@ -136,14 +138,21 @@ public void postAnalysisResults( vcsRepoInfo.getVcsConnection() ); - postOrUpdateComment(httpClient, vcsRepoInfo, pullRequestNumber, markdownSummary, placeholderCommentId); + // Post summary comment (or update placeholder) + String summaryCommentId = postOrUpdateComment(httpClient, vcsRepoInfo, pullRequestNumber, markdownSummary, placeholderCommentId); + + // Post detailed issues as a separate comment reply if there are issues + if (detailedIssuesMarkdown != null && !detailedIssuesMarkdown.isEmpty() && summaryCommentId != null) { + postDetailedIssuesReply(httpClient, vcsRepoInfo, pullRequestNumber, summaryCommentId, detailedIssuesMarkdown); + } + postReport(httpClient, vcsRepoInfo, codeAnalysis.getCommitHash(), report); postAnnotations(httpClient, vcsRepoInfo, codeAnalysis.getCommitHash(), annotations); log.info("Successfully posted analysis results to Bitbucket"); } - private void postOrUpdateComment( + private String postOrUpdateComment( OkHttpClient httpClient, VcsRepoInfo vcsRepoInfo, Long pullRequestNumber, @@ -165,8 +174,34 @@ private void postOrUpdateComment( String fullContent = markdownSummary + "\n\n" + CODECROW_REVIEW_MARKER; commentAction.updateComment(placeholderCommentId, fullContent); log.debug("Updated placeholder comment {} with analysis results", placeholderCommentId); + return placeholderCommentId; } else { - commentAction.postSummaryResult(markdownSummary); + return commentAction.postSummaryResultWithId(markdownSummary); + } + } + + private void postDetailedIssuesReply( + OkHttpClient httpClient, + VcsRepoInfo vcsRepoInfo, + Long pullRequestNumber, + String parentCommentId, + String detailedIssuesMarkdown + ) throws IOException { + try { + log.debug("Posting detailed issues as reply to comment {} on PR {}", parentCommentId, pullRequestNumber); + + CommentOnBitbucketCloudAction commentAction = new CommentOnBitbucketCloudAction( + httpClient, + vcsRepoInfo, + pullRequestNumber + ); + + String content = detailedIssuesMarkdown + "\n\n" + CODECROW_ISSUES_MARKER; + commentAction.postCommentReply(parentCommentId, content); + + log.debug("Posted detailed issues reply to PR {}", pullRequestNumber); + } catch (Exception e) { + log.warn("Failed to post detailed issues as reply, will be included in annotations: {}", e.getMessage()); } } diff --git a/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/service/github/GitHubReportingService.java b/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/service/github/GitHubReportingService.java index 3dfc7f42..5925aa76 100644 --- a/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/service/github/GitHubReportingService.java +++ b/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/service/github/GitHubReportingService.java @@ -30,7 +30,6 @@ public class GitHubReportingService implements VcsReportingService { /** * Marker text used to identify CodeCrow comments for deletion. - * This should be unique enough to not match user comments. */ private static final String CODECROW_COMMENT_MARKER = ""; @@ -100,6 +99,14 @@ public void postAnalysisResults( AnalysisSummary summary = reportGenerator.createAnalysisSummary(codeAnalysis, platformPrEntityId); // Use GitHub-specific markdown with collapsible spoilers for suggested fixes String markdownSummary = reportGenerator.createMarkdownSummary(codeAnalysis, summary, true); + String detailedIssuesMarkdown = reportGenerator.createDetailedIssuesMarkdown(summary, true); + + // GitHub doesn't support threaded replies for issue comments like Bitbucket does. + // So we combine summary and detailed issues into ONE comment. + String fullComment = markdownSummary; + if (detailedIssuesMarkdown != null && !detailedIssuesMarkdown.isEmpty()) { + fullComment = markdownSummary + "\n\n---\n\n" + detailedIssuesMarkdown; + } VcsRepoInfo vcsRepoInfo = getVcsRepoInfo(project); @@ -107,62 +114,76 @@ public void postAnalysisResults( vcsRepoInfo.getVcsConnection() ); - // Post or update PR comment with detailed analysis - postOrUpdateComment(httpClient, vcsRepoInfo, pullRequestNumber, markdownSummary, placeholderCommentId); + // Post or update comment with full content (summary + issues) + if (placeholderCommentId != null) { + updatePlaceholderComment(httpClient, vcsRepoInfo, pullRequestNumber, fullComment, placeholderCommentId); + } else { + postSummaryComment(httpClient, vcsRepoInfo, pullRequestNumber, fullComment); + } // Create Check Run for the commit createCheckRun(httpClient, vcsRepoInfo, codeAnalysis, summary); log.info("Successfully posted analysis results to GitHub"); } - - private void postOrUpdateComment( + + /** + * Post summary as a regular comment. + */ + private void postSummaryComment( OkHttpClient httpClient, VcsRepoInfo vcsRepoInfo, Long pullRequestNumber, - String markdownSummary, - String placeholderCommentId + String summaryMarkdown ) throws IOException { - - log.debug("Posting/updating summary comment to PR {} (placeholderCommentId={})", - pullRequestNumber, placeholderCommentId); - CommentOnPullRequestAction commentAction = new CommentOnPullRequestAction(httpClient); - if (placeholderCommentId != null) { - // Update the placeholder comment with the analysis results - String markedComment = CODECROW_COMMENT_MARKER + "\n" + markdownSummary; - commentAction.updateComment( - vcsRepoInfo.getRepoWorkspace(), - vcsRepoInfo.getRepoSlug(), - Long.parseLong(placeholderCommentId), - markedComment - ); - log.debug("Updated placeholder comment {} with analysis results", placeholderCommentId); - } else { - // Delete previous CodeCrow comments before posting new one - try { - commentAction.deletePreviousComments( - vcsRepoInfo.getRepoWorkspace(), - vcsRepoInfo.getRepoSlug(), - pullRequestNumber.intValue(), - CODECROW_COMMENT_MARKER - ); - log.debug("Deleted previous CodeCrow comments from PR {}", pullRequestNumber); - } catch (Exception e) { - log.warn("Failed to delete previous comments: {}", e.getMessage()); - } - - // Add marker to the comment for future identification - String markedComment = CODECROW_COMMENT_MARKER + "\n" + markdownSummary; - - commentAction.postComment( + // Delete previous CodeCrow summary comments first + try { + commentAction.deletePreviousComments( vcsRepoInfo.getRepoWorkspace(), vcsRepoInfo.getRepoSlug(), pullRequestNumber.intValue(), - markedComment + CODECROW_COMMENT_MARKER ); + } catch (Exception e) { + log.warn("Failed to delete previous summary comments: {}", e.getMessage()); } + + String markedBody = CODECROW_COMMENT_MARKER + "\n" + summaryMarkdown; + + commentAction.postComment( + vcsRepoInfo.getRepoWorkspace(), + vcsRepoInfo.getRepoSlug(), + pullRequestNumber.intValue(), + markedBody + ); + + log.debug("Posted summary comment to PR {}", pullRequestNumber); + } + + /** + * Update placeholder comment with summary. + */ + private void updatePlaceholderComment( + OkHttpClient httpClient, + VcsRepoInfo vcsRepoInfo, + Long pullRequestNumber, + String summaryMarkdown, + String placeholderCommentId + ) throws IOException { + CommentOnPullRequestAction commentAction = new CommentOnPullRequestAction(httpClient); + + String markedBody = CODECROW_COMMENT_MARKER + "\n" + summaryMarkdown; + + commentAction.updateComment( + vcsRepoInfo.getRepoWorkspace(), + vcsRepoInfo.getRepoSlug(), + Long.parseLong(placeholderCommentId), + markedBody + ); + + log.debug("Updated placeholder comment {} with summary", placeholderCommentId); } private void createCheckRun( diff --git a/python-ecosystem/mcp-client/inspect_mcp_agent.py b/python-ecosystem/mcp-client/inspect_mcp_agent.py new file mode 100644 index 00000000..99fb51fa --- /dev/null +++ b/python-ecosystem/mcp-client/inspect_mcp_agent.py @@ -0,0 +1,18 @@ + +import inspect +import mcp_use.agent +from mcp_use.agent import MCPAgent + +print("MCPAgent location:", mcp_use.agent.__file__) +print("\nMCPAgent.__init__ signature:") +print(inspect.signature(MCPAgent.__init__)) + +print("\nMCPAgent.stream signature:") +print(inspect.signature(MCPAgent.stream)) + +print("\nMCPAgent.stream source (start):") +try: + src = inspect.getsource(MCPAgent.stream) + print(src[:500]) +except Exception as e: + print("Could not get source:", e) diff --git a/python-ecosystem/mcp-client/main.py b/python-ecosystem/mcp-client/main.py index 360e85c5..abb28d6f 100644 --- a/python-ecosystem/mcp-client/main.py +++ b/python-ecosystem/mcp-client/main.py @@ -15,6 +15,7 @@ import sys import logging import warnings +import threading from server.stdin_handler import StdinHandler @@ -43,21 +44,23 @@ class FilteredStderr: def __init__(self, original_stderr): self.original_stderr = original_stderr self.buffer = "" - self.suppress_next_lines = 0 + self._lock = threading.Lock() + self._suppress_next_lines = 0 def write(self, text): - # Check if this is the start of a JSONRPC parsing error - if "Failed to parse JSONRPC message from server" in text: - self.suppress_next_lines = 15 # Suppress the next ~15 lines (traceback) - return - - # If we're suppressing lines, decrement counter - if self.suppress_next_lines > 0: - self.suppress_next_lines -= 1 - return - - # Otherwise, write to original stderr - self.original_stderr.write(text) + with self._lock: + # Check if this is the start of a JSONRPC parsing error + if "Failed to parse JSONRPC message from server" in text: + self._suppress_next_lines = 15 # Suppress the next ~15 lines (traceback) + return + + # If we're suppressing lines, decrement counter + if self._suppress_next_lines > 0: + self._suppress_next_lines -= 1 + return + + # Otherwise, write to original stderr + self.original_stderr.write(text) def flush(self): self.original_stderr.flush() diff --git a/python-ecosystem/mcp-client/model/models.py b/python-ecosystem/mcp-client/model/models.py index 0b46faba..153de871 100644 --- a/python-ecosystem/mcp-client/model/models.py +++ b/python-ecosystem/mcp-client/model/models.py @@ -25,19 +25,27 @@ class AnalysisMode(str, Enum): class IssueDTO(BaseModel): + """ + Maps to Java's AiRequestPreviousIssueDTO. + Fields match exactly what Java sends for previousCodeAnalysisIssues. + """ id: Optional[str] = None type: Optional[str] = None # security|quality|performance|style category: Optional[str] = None # SECURITY|PERFORMANCE|CODE_QUALITY|BUG_RISK|STYLE|DOCUMENTATION|BEST_PRACTICES|ERROR_HANDLING|TESTING|ARCHITECTURE - severity: Optional[str] = None # critical|high|medium|low|info - title: Optional[str] = None - description: Optional[str] = None # Typically holds the suggestedFix + severity: Optional[str] = None # HIGH|MEDIUM|LOW|INFO + reason: Optional[str] = None # Issue description/title (from Java) + suggestedFixDescription: Optional[str] = None # Suggested fix text (from Java) + suggestedFixDiff: Optional[str] = None # Diff for suggested fix (from Java) file: Optional[str] = None line: Optional[int] = None - column: Optional[int] = None - rule: Optional[str] = None branch: Optional[str] = None pullRequestId: Optional[str] = None status: Optional[str] = None # open|resolved|ignored + # Legacy fields for backwards compatibility + title: Optional[str] = None # Legacy - use reason instead + description: Optional[str] = None # Legacy - use suggestedFixDescription instead + column: Optional[int] = None + rule: Optional[str] = None createdAt: Optional[datetime] = None resolvedAt: Optional[datetime] = None resolvedBy: Optional[str] = None @@ -82,7 +90,6 @@ class ReviewResponseDto(BaseModel): error: Optional[str] = None exception: Optional[str] = None - class SummarizeRequestDto(BaseModel): """Request model for PR summarization command.""" projectId: int @@ -104,7 +111,6 @@ class SummarizeRequestDto(BaseModel): maxAllowedTokens: Optional[int] = None vcsProvider: Optional[str] = Field(default=None, description="VCS provider type (github, bitbucket_cloud)") - class SummarizeResponseDto(BaseModel): """Response model for PR summarization command.""" summary: Optional[str] = None @@ -112,7 +118,6 @@ class SummarizeResponseDto(BaseModel): diagramType: Optional[str] = Field(default="MERMAID", description="MERMAID or ASCII") error: Optional[str] = None - class AskRequestDto(BaseModel): """Request model for ask command.""" projectId: int @@ -135,13 +140,11 @@ class AskRequestDto(BaseModel): analysisContext: Optional[str] = Field(default=None, description="Existing analysis data for context") issueReferences: Optional[List[str]] = Field(default_factory=list, description="Issue IDs referenced in the question") - class AskResponseDto(BaseModel): """Response model for ask command.""" answer: Optional[str] = None error: Optional[str] = None - # ==================== Output Schemas for MCP Agent ==================== # These Pydantic models are used with MCPAgent's output_schema parameter # to ensure structured JSON output from the LLM. @@ -150,7 +153,7 @@ class CodeReviewIssue(BaseModel): """Schema for a single code review issue.""" # Optional issue identifier (preserve DB/client-side ids for reconciliation) id: Optional[str] = Field(default=None, description="Optional issue id to link to existing issues") - severity: str = Field(description="Issue severity: HIGH, MEDIUM, or LOW") + severity: str = Field(description="Issue severity: HIGH, MEDIUM, LOW, or INFO") category: str = Field(description="Issue category: SECURITY, PERFORMANCE, CODE_QUALITY, BUG_RISK, STYLE, DOCUMENTATION, BEST_PRACTICES, ERROR_HANDLING, TESTING, or ARCHITECTURE") file: str = Field(description="File path where the issue is located") line: str = Field(description="Line number or range (e.g., '42' or '42-45')") @@ -158,6 +161,9 @@ class CodeReviewIssue(BaseModel): suggestedFixDescription: str = Field(description="Description of the suggested fix") suggestedFixDiff: Optional[str] = Field(default=None, description="Optional unified diff format patch for the fix") isResolved: bool = Field(default=False, description="Whether this issue from previous analysis is resolved") + # Additional fields preserved from previous issues during reconciliation + visibility: Optional[str] = Field(default=None, description="Issue visibility status") + codeSnippet: Optional[str] = Field(default=None, description="Code snippet associated with the issue") class CodeReviewOutput(BaseModel): @@ -175,4 +181,97 @@ class SummarizeOutput(BaseModel): class AskOutput(BaseModel): """Schema for ask command output.""" - answer: str = Field(description="Well-formatted markdown answer to the user's question") \ No newline at end of file + answer: str = Field(description="Well-formatted markdown answer to the user's question") + + +# ==================== Multi-Stage Review Models ==================== + +class FileReviewOutput(BaseModel): + """Stage 1 Output: Single file review result.""" + file: str + analysis_summary: str + issues: List[CodeReviewIssue] = Field(default_factory=list) + confidence: str = Field(description="Confidence level (HIGH/MEDIUM/LOW)") + note: str = Field(default="", description="Optional analysis note") + + + +class FileReviewBatchOutput(BaseModel): + """Stage 1 Output: Batch of file reviews.""" + reviews: List[FileReviewOutput] = Field(description="List of review results for the files in the batch") + + + +class ReviewFile(BaseModel): + """File details for review planning.""" + path: str + focus_areas: List[str] = Field(default_factory=list, description="Specific areas to focus on (SECURITY, ARCHITECTURE, etc.)") + risk_level: str = Field(description="CRITICAL, HIGH, MEDIUM, or LOW") + estimated_issues: Optional[int] = Field(default=0) + + +class FileGroup(BaseModel): + """Group of files to be reviewed together.""" + group_id: str + priority: str = Field(description="CRITICAL, HIGH, MEDIUM, LOW") + rationale: str + files: List[ReviewFile] + + +class FileToSkip(BaseModel): + """File skipped from deep review.""" + path: str + reason: str + + +class ReviewPlan(BaseModel): + """Stage 0 Output: Plan for the review scanning.""" + analysis_summary: str + file_groups: List[FileGroup] + files_to_skip: List[FileToSkip] = Field(default_factory=list) + cross_file_concerns: List[str] = Field(default_factory=list, description="Hypotheses to verify in Stage 2") + + +class CrossFileIssue(BaseModel): + """Issue spanning multiple files (Stage 2).""" + id: str + severity: str + category: str + title: str + affected_files: List[str] + description: str + evidence: str + business_impact: str + suggestion: str + + +class DataFlowConcern(BaseModel): + """Stage 2: Data flow gap analysis.""" + flow: str + gap: str + files_involved: List[str] + severity: str + + +class ImmutabilityCheck(BaseModel): + """Stage 2: Immutability usage check.""" + rule: str + check_pass: bool = Field(alias="check_pass") + evidence: str + + +class DatabaseIntegrityCheck(BaseModel): + """Stage 2: DB integrity check.""" + concerns: List[str] + findings: List[str] + + +class CrossFileAnalysisResult(BaseModel): + """Stage 2 Output: Cross-file architectural analysis.""" + pr_risk_level: str + cross_file_issues: List[CrossFileIssue] + data_flow_concerns: List[DataFlowConcern] = Field(default_factory=list) + immutability_enforcement: Optional[ImmutabilityCheck] = None + database_integrity: Optional[DatabaseIntegrityCheck] = None + pr_recommendation: str + confidence: str \ No newline at end of file diff --git a/python-ecosystem/mcp-client/server/web_server.py b/python-ecosystem/mcp-client/server/web_server.py index 96cac914..37a9f9f7 100644 --- a/python-ecosystem/mcp-client/server/web_server.py +++ b/python-ecosystem/mcp-client/server/web_server.py @@ -300,7 +300,7 @@ def run_http_server(host: str = "0.0.0.0", port: int = 8000): """Run the FastAPI application.""" app = create_app() import uvicorn - uvicorn.run(app, host=host, port=port, log_level="info") + uvicorn.run(app, host=host, port=port, log_level="info", timeout_keep_alive=300) if __name__ == "__main__": diff --git a/python-ecosystem/mcp-client/service/command_service.py b/python-ecosystem/mcp-client/service/command_service.py index a07a8aea..9de4898b 100644 --- a/python-ecosystem/mcp-client/service/command_service.py +++ b/python-ecosystem/mcp-client/service/command_service.py @@ -64,13 +64,8 @@ async def process_summarize( # Build configuration jvm_props = self._build_jvm_props_for_summarize(request) - - # DEBUG: Log what we're sending - logger.info(f"DEBUG SUMMARIZE REQUEST: oAuthClient={request.oAuthClient}, oAuthSecret={request.oAuthSecret[:10] if request.oAuthSecret else None}..., accessToken={request.accessToken}") - logger.info(f"DEBUG JVM PROPS: {jvm_props}") - + config = MCPConfigBuilder.build_config(jar_path, jvm_props) - logger.info(f"DEBUG MCP CONFIG: {config}") # Create MCP client and LLM self._emit_event(event_callback, { @@ -319,10 +314,11 @@ async def _fetch_rag_context_for_ask( }) # Use the question as the query for RAG - rag_response = await self.rag_client.query( + rag_response = await self.rag_client.semantic_search( workspace=request.projectWorkspace, project=request.projectNamespace, query=request.question, + branch="main", # AskRequestDto doesn't have branch, default to main top_k=8 ) diff --git a/python-ecosystem/mcp-client/service/llm_reranker.py b/python-ecosystem/mcp-client/service/llm_reranker.py index cc0519c3..39bdf302 100644 --- a/python-ecosystem/mcp-client/service/llm_reranker.py +++ b/python-ecosystem/mcp-client/service/llm_reranker.py @@ -178,7 +178,25 @@ async def _llm_rerank( # Call LLM response = await self.llm_client.ainvoke(prompt) - response_text = response.content if hasattr(response, 'content') else str(response) + + # Handle different response types + if hasattr(response, 'content'): + content = response.content + # Handle case where content is a list (e.g., from some LLM providers) + if isinstance(content, list): + # Extract text from list elements + response_text = "" + for item in content: + if isinstance(item, str): + response_text += item + elif isinstance(item, dict) and 'text' in item: + response_text += item['text'] + elif hasattr(item, 'text'): + response_text += item.text + else: + response_text = str(content) + else: + response_text = str(response) # Parse response try: diff --git a/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py b/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py new file mode 100644 index 00000000..12876ce7 --- /dev/null +++ b/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py @@ -0,0 +1,1056 @@ +import logging +import asyncio +import json +from typing import Dict, Any, List, Optional, Callable + +from model.models import ( + ReviewRequestDto, + ReviewPlan, + CodeReviewOutput, + CodeReviewIssue, + ReviewFile, + FileGroup, + CrossFileAnalysisResult, + FileReviewBatchOutput +) +from utils.prompts.prompt_builder import PromptBuilder +from utils.diff_processor import ProcessedDiff, DiffProcessor +from mcp_use import MCPAgent + +logger = logging.getLogger(__name__) + + +def extract_llm_response_text(response: Any) -> str: + """ + Extract text content from LLM response, handling different response formats. + Some LLM providers return content as a list of objects instead of a string. + """ + if hasattr(response, 'content'): + content = response.content + if isinstance(content, list): + # Handle list content (e.g., from Gemini or other providers) + text_parts = [] + for item in content: + if isinstance(item, str): + text_parts.append(item) + elif isinstance(item, dict): + if 'text' in item: + text_parts.append(item['text']) + elif 'content' in item: + text_parts.append(item['content']) + elif hasattr(item, 'text'): + text_parts.append(item.text) + return "".join(text_parts) + return str(content) + return str(response) + + +# Prevent duplicate logs from mcp_use +mcp_logger = logging.getLogger("mcp_use") +mcp_logger.propagate = False +if not mcp_logger.handlers: + handler = logging.StreamHandler() + formatter = logging.Formatter('%(asctime)s - %(name)s - %(levelname)s - %(message)s') + handler.setFormatter(formatter) + mcp_logger.addHandler(handler) + +class RecursiveMCPAgent(MCPAgent): + """ + Subclass of MCPAgent that enforces a higher recursion limit on the internal agent executor. + """ + def __init__(self, *args, recursion_limit: int = 50, **kwargs): + self._custom_recursion_limit = recursion_limit + super().__init__(*args, **kwargs) + + async def stream(self, *args, **kwargs): + """ + Override stream to ensure recursion_limit is applied. + """ + # Ensure the executor exists + if self._agent_executor is None: + await self.initialize() + + # Patch the executor's astream if not already patched + executor = self._agent_executor + if executor and not getattr(executor, "_is_patched_recursion", False): + original_astream = executor.astream + limit = self._custom_recursion_limit + + async def patched_astream(input_data, config=None, **astream_kwargs): + if config is None: + config = {} + config["recursion_limit"] = limit + async for chunk in original_astream(input_data, config=config, **astream_kwargs): + yield chunk + + executor.astream = patched_astream + executor._is_patched_recursion = True + logger.info(f"RecursiveMCPAgent: Patched recursion limit to {limit}") + + # Call parent stream + async for item in super().stream(*args, **kwargs): + yield item + +class MultiStageReviewOrchestrator: + """ + Orchestrates the 4-stage AI code review pipeline: + Stage 0: Planning & Prioritization + Stage 1: Parallel File Review + Stage 2: Cross-File & Architectural Analysis + Stage 3: Aggregation & Final Report + """ + + def __init__( + self, + llm, + mcp_client, + rag_client=None, + event_callback: Optional[Callable[[Dict], None]] = None + ): + self.llm = llm + self.client = mcp_client + self.rag_client = rag_client + self.event_callback = event_callback + self.max_parallel_stage_1 = 5 # Limit parallel execution to avoid rate limits + + async def execute_branch_analysis(self, prompt: str) -> Dict[str, Any]: + """ + Execute a single-pass branch analysis using the provided prompt. + """ + self._emit_status("branch_analysis_started", "Starting Branch Analysis & Reconciliation...") + + agent = RecursiveMCPAgent( + llm=self.llm, + client=self.client, + additional_instructions=PromptBuilder.get_additional_instructions() + ) + + + try: + final_text = "" + # Branch analysis expects standard CodeReviewOutput + async for item in agent.stream(prompt, max_steps=15, output_schema=CodeReviewOutput): + if isinstance(item, CodeReviewOutput): + # Convert to dict format expected by service + issues = [i.model_dump() for i in item.issues] if item.issues else [] + return { + "issues": issues, + "comment": item.comment or "Branch analysis completed." + } + + if isinstance(item, str): + final_text = item + + # If stream finished without object, try parsing text + if final_text: + data = await self._parse_response(final_text, CodeReviewOutput) + issues = [i.model_dump() for i in data.issues] if data.issues else [] + return { + "issues": issues, + "comment": data.comment or "Branch analysis completed." + } + + return {"issues": [], "comment": "No issues found."} + + except Exception as e: + logger.error(f"Branch analysis failed: {e}", exc_info=True) + self._emit_error(str(e)) + raise + + async def orchestrate_review( + self, + request: ReviewRequestDto, + rag_context: Optional[Dict[str, Any]] = None, + processed_diff: Optional[ProcessedDiff] = None + ) -> Dict[str, Any]: + """ + Main entry point for the multi-stage review. + Supports both FULL (initial review) and INCREMENTAL (follow-up review) modes. + The same pipeline is used, but with incremental-aware prompts and issue reconciliation. + """ + # Determine if this is an incremental review + is_incremental = ( + request.analysisMode == "INCREMENTAL" + and request.deltaDiff + ) + + if is_incremental: + logger.info(f"INCREMENTAL mode: reviewing delta diff, {len(request.previousCodeAnalysisIssues or [])} previous issues to reconcile") + else: + logger.info("FULL mode: initial PR review") + + try: + # === STAGE 0: Planning === + self._emit_status("stage_0_started", "Stage 0: Planning & Prioritization...") + review_plan = await self._execute_stage_0_planning(request, is_incremental) + + # Validate and fix the plan to ensure all files are included + review_plan = self._ensure_all_files_planned(review_plan, request.changedFiles or []) + self._emit_progress(10, "Stage 0 Complete: Review plan created") + + # === STAGE 1: File Reviews === + self._emit_status("stage_1_started", f"Stage 1: Analyzing {self._count_files(review_plan)} files...") + file_issues = await self._execute_stage_1_file_reviews( + request, review_plan, rag_context, processed_diff, is_incremental + ) + self._emit_progress(60, f"Stage 1 Complete: {len(file_issues)} issues found across files") + + # === STAGE 1.5: Issue Reconciliation (INCREMENTAL only) === + if is_incremental and request.previousCodeAnalysisIssues: + self._emit_status("reconciliation_started", "Reconciling previous issues...") + file_issues = await self._reconcile_previous_issues( + request, file_issues, processed_diff + ) + self._emit_progress(70, f"Reconciliation Complete: {len(file_issues)} total issues after reconciliation") + + # === STAGE 2: Cross-File Analysis === + self._emit_status("stage_2_started", "Stage 2: Analyzing cross-file patterns...") + cross_file_results = await self._execute_stage_2_cross_file(request, file_issues, review_plan) + self._emit_progress(85, "Stage 2 Complete: Cross-file analysis finished") + + # === STAGE 3: Aggregation === + self._emit_status("stage_3_started", "Stage 3: Generating final report...") + final_report = await self._execute_stage_3_aggregation( + request, review_plan, file_issues, cross_file_results, is_incremental + ) + self._emit_progress(100, "Stage 3 Complete: Report generated") + + # Return structure compatible with existing response expected by frontend/controller + return { + "comment": final_report, + "issues": [issue.model_dump() for issue in file_issues], + } + + except Exception as e: + logger.error(f"Multi-stage review failed: {e}", exc_info=True) + self._emit_error(str(e)) + raise + + async def _reconcile_previous_issues( + self, + request: ReviewRequestDto, + new_issues: List[CodeReviewIssue], + processed_diff: Optional[ProcessedDiff] = None + ) -> List[CodeReviewIssue]: + """ + Reconcile previous issues with new findings in incremental mode. + - Mark resolved issues as isResolved=true + - Update line numbers for persisting issues + - Merge with new issues found in delta diff + """ + if not request.previousCodeAnalysisIssues: + return new_issues + + logger.info(f"Reconciling {len(request.previousCodeAnalysisIssues)} previous issues with {len(new_issues)} new issues") + + # Get the delta diff content to check what files/lines changed + delta_diff = request.deltaDiff or "" + + # Build a set of files that changed in the delta + changed_files_in_delta = set() + if processed_diff: + for f in processed_diff.files: + changed_files_in_delta.add(f.path) + + reconciled_issues = list(new_issues) # Start with new issues + + # Process each previous issue + for prev_issue in request.previousCodeAnalysisIssues: + # Convert to dict if needed + if hasattr(prev_issue, 'model_dump'): + prev_data = prev_issue.model_dump() + else: + prev_data = prev_issue if isinstance(prev_issue, dict) else vars(prev_issue) + + # Debug log to verify field mapping + logger.debug(f"Previous issue data: reason={prev_data.get('reason')}, " + f"suggestedFixDescription={prev_data.get('suggestedFixDescription')}, " + f"suggestedFixDiff={prev_data.get('suggestedFixDiff')[:50] if prev_data.get('suggestedFixDiff') else None}") + + file_path = prev_data.get('file', prev_data.get('filePath', '')) + issue_id = prev_data.get('id') + + # Check if this issue was already found in new issues (by file+line or ID) + already_reported = False + for new_issue in new_issues: + new_data = new_issue.model_dump() if hasattr(new_issue, 'model_dump') else new_issue + if (new_data.get('file') == file_path and + str(new_data.get('line')) == str(prev_data.get('line', prev_data.get('lineNumber')))): + already_reported = True + break + if issue_id and new_data.get('id') == issue_id: + already_reported = True + break + + if already_reported: + continue # Already in new issues, skip + + # Check if the file was modified in delta diff + file_in_delta = any(file_path.endswith(f) or f.endswith(file_path) for f in changed_files_in_delta) + + # If file wasn't touched in delta, issue persists unchanged + # If file was touched, we need to check if the specific line was modified + is_resolved = False + if file_in_delta: + # Simple heuristic: if file changed and we didn't re-report this issue, + # it might be resolved. But we should be conservative here. + # For now, we'll re-report it as persisting unless LLM marked it resolved + pass + + # Preserve all original issue data - just pass through as CodeReviewIssue + # Field mapping from Java DTO: + # reason (or title for legacy) -> reason + # severity (uppercase) -> severity + # category (or issueCategory) -> category + # file -> file + # line -> line + # suggestedFixDescription -> suggestedFixDescription + # suggestedFixDiff -> suggestedFixDiff + persisting_issue = CodeReviewIssue( + id=str(issue_id) if issue_id else None, + severity=(prev_data.get('severity') or prev_data.get('issueSeverity') or 'MEDIUM').upper(), + category=prev_data.get('category') or prev_data.get('issueCategory') or prev_data.get('type') or 'CODE_QUALITY', + file=file_path or prev_data.get('file') or prev_data.get('filePath') or 'unknown', + line=str(prev_data.get('line') or prev_data.get('lineNumber') or '1'), + reason=prev_data.get('reason') or prev_data.get('title') or prev_data.get('description') or '', + suggestedFixDescription=prev_data.get('suggestedFixDescription') or prev_data.get('suggestedFix') or '', + suggestedFixDiff=prev_data.get('suggestedFixDiff') or None, + isResolved=is_resolved, + visibility=prev_data.get('visibility'), + codeSnippet=prev_data.get('codeSnippet') + ) + reconciled_issues.append(persisting_issue) + + logger.info(f"Reconciliation complete: {len(reconciled_issues)} total issues") + return reconciled_issues + + def _issue_matches_files(self, issue: Any, file_paths: List[str]) -> bool: + """Check if an issue is related to any of the given file paths.""" + if hasattr(issue, 'model_dump'): + issue_data = issue.model_dump() + elif isinstance(issue, dict): + issue_data = issue + else: + issue_data = vars(issue) if hasattr(issue, '__dict__') else {} + + issue_file = issue_data.get('file', issue_data.get('filePath', '')) + + for fp in file_paths: + if issue_file == fp or issue_file.endswith('/' + fp) or fp.endswith('/' + issue_file): + return True + # Also check basename match + if issue_file.split('/')[-1] == fp.split('/')[-1]: + return True + return False + + def _format_previous_issues_for_batch(self, issues: List[Any]) -> str: + """Format previous issues for inclusion in batch prompt.""" + if not issues: + return "" + + lines = ["=== PREVIOUS ISSUES IN THESE FILES (check if resolved) ==="] + for issue in issues: + if hasattr(issue, 'model_dump'): + data = issue.model_dump() + elif isinstance(issue, dict): + data = issue + else: + data = vars(issue) if hasattr(issue, '__dict__') else {} + + issue_id = data.get('id', 'unknown') + severity = data.get('severity', 'MEDIUM') + file_path = data.get('file', data.get('filePath', 'unknown')) + line = data.get('line', data.get('lineNumber', '?')) + reason = data.get('reason', data.get('description', 'No description')) + + lines.append(f"[ID:{issue_id}] {severity} @ {file_path}:{line}") + lines.append(f" Issue: {reason}") + lines.append("") + + lines.append("Mark these as 'isResolved: true' if fixed in the diff above.") + lines.append("=== END PREVIOUS ISSUES ===") + return "\n".join(lines) + + def _get_diff_snippets_for_batch( + self, + all_diff_snippets: List[str], + batch_file_paths: List[str] + ) -> List[str]: + """ + Filter diff snippets to only include those relevant to the batch files. + + The diffSnippets from ReviewRequestDto are extracted by Java DiffParser.extractDiffSnippets() + which creates snippets in format that may include file paths in diff headers. + We filter to get only snippets relevant to the current batch for targeted RAG queries. + """ + if not all_diff_snippets or not batch_file_paths: + return [] + + batch_snippets = [] + batch_file_names = {path.split('/')[-1] for path in batch_file_paths} + + for snippet in all_diff_snippets: + # Check if snippet relates to any file in the batch + # Diff snippets typically contain file paths in headers like "diff --git a/path b/path" + # or "--- a/path" or "+++ b/path" + snippet_lower = snippet.lower() + + for file_path in batch_file_paths: + if file_path.lower() in snippet_lower: + batch_snippets.append(snippet) + break + else: + # Also check by filename only (for cases where paths differ) + for file_name in batch_file_names: + if file_name.lower() in snippet_lower: + batch_snippets.append(snippet) + break + + logger.debug(f"Filtered {len(batch_snippets)} diff snippets for batch from {len(all_diff_snippets)} total") + return batch_snippets + + async def _execute_stage_0_planning(self, request: ReviewRequestDto, is_incremental: bool = False) -> ReviewPlan: + """ + Stage 0: Analyze metadata and generate a review plan. + Uses structured output for reliable JSON parsing. + """ + # Prepare context for prompt + changed_files_summary = [] + if request.changedFiles: + for f in request.changedFiles: + changed_files_summary.append({ + "path": f, + "type": "MODIFIED", + "lines_added": "?", + "lines_deleted": "?" + }) + + prompt = PromptBuilder.build_stage_0_planning_prompt( + repo_slug=request.projectVcsRepoSlug, + pr_id=str(request.pullRequestId), + pr_title=request.prTitle or "", + author="Unknown", + branch_name="source-branch", + target_branch=request.targetBranchName or "main", + commit_hash=request.commitHash or "HEAD", + changed_files_json=json.dumps(changed_files_summary, indent=2) + ) + + # Stage 0 uses direct LLM call (no tools needed - all metadata is provided) + try: + structured_llm = self.llm.with_structured_output(ReviewPlan) + result = await structured_llm.ainvoke(prompt) + if result: + logger.info("Stage 0 planning completed with structured output") + return result + except Exception as e: + logger.warning(f"Structured output failed for Stage 0: {e}") + + # Fallback to manual parsing + try: + response = await self.llm.ainvoke(prompt) + content = extract_llm_response_text(response) + return await self._parse_response(content, ReviewPlan) + except Exception as e: + logger.error(f"Stage 0 planning failed: {e}") + raise ValueError(f"Stage 0 planning failed: {e}") + + def _chunk_files(self, file_groups: List[Any], max_files_per_batch: int = 5) -> List[List[Dict[str, Any]]]: + """Flatten file groups and chunk into batches.""" + all_files = [] + for group in file_groups: + for f in group.files: + # Attach priority context for the review + all_files.append({ + "file": f, + "priority": group.priority + }) + + return [all_files[i:i + max_files_per_batch] for i in range(0, len(all_files), max_files_per_batch)] + + async def _execute_stage_1_file_reviews( + self, + request: ReviewRequestDto, + plan: ReviewPlan, + rag_context: Optional[Dict[str, Any]] = None, + processed_diff: Optional[ProcessedDiff] = None, + is_incremental: bool = False + ) -> List[CodeReviewIssue]: + """ + Stage 1: Execute batch file reviews with per-batch RAG context. + """ + # Use smaller batches (3 files max) for better RAG relevance and review quality + batches = self._chunk_files(plan.file_groups, max_files_per_batch=3) + + total_files = sum(len(batch) for batch in batches) + logger.info(f"Stage 1: Processing {total_files} files in {len(batches)} batches (max 3 files/batch)") + + # Process batches with controlled parallelism + all_issues = [] + batch_results = [] + + # Process in waves to avoid rate limits + for wave_start in range(0, len(batches), self.max_parallel_stage_1): + wave_end = min(wave_start + self.max_parallel_stage_1, len(batches)) + wave_batches = batches[wave_start:wave_end] + + logger.info(f"Stage 1: Processing wave {wave_start // self.max_parallel_stage_1 + 1}, " + f"batches {wave_start + 1}-{wave_end} of {len(batches)}") + + tasks = [] + for batch_idx, batch in enumerate(wave_batches, start=wave_start + 1): + batch_paths = [item["file"].path for item in batch] + logger.debug(f"Batch {batch_idx}: {batch_paths}") + tasks.append(self._review_file_batch(request, batch, processed_diff, is_incremental)) + + results = await asyncio.gather(*tasks, return_exceptions=True) + + for idx, res in enumerate(results): + batch_num = wave_start + idx + 1 + if isinstance(res, Exception): + logger.error(f"Error reviewing batch {batch_num}: {res}") + elif res: + logger.info(f"Batch {batch_num} completed: {len(res)} issues found") + all_issues.extend(res) + else: + logger.info(f"Batch {batch_num} completed: no issues found") + + # Update progress + progress = 10 + int((wave_end / len(batches)) * 50) + self._emit_progress(progress, f"Stage 1: Reviewed {wave_end}/{len(batches)} batches") + + logger.info(f"Stage 1 Complete: {len(all_issues)} issues found across {total_files} files") + return all_issues + + async def _review_file_batch( + self, + request: ReviewRequestDto, + batch_items: List[Dict[str, Any]], + processed_diff: Optional[ProcessedDiff] = None, + is_incremental: bool = False + ) -> List[CodeReviewIssue]: + """ + Review a batch of files in a single LLM call with per-batch RAG context. + In incremental mode, uses delta diff and focuses on new changes only. + """ + batch_files_data = [] + batch_file_paths = [] + project_rules = "1. No hardcoded secrets.\n2. Use dependency injection.\n3. Verify all inputs." + + # For incremental mode, use deltaDiff instead of full diff + diff_source = None + if is_incremental and request.deltaDiff: + # Parse delta diff to extract per-file diffs + diff_source = DiffProcessor().process(request.deltaDiff) if request.deltaDiff else None + else: + diff_source = processed_diff + + # Collect file paths and diffs for this batch + for item in batch_items: + file_info = item["file"] + batch_file_paths.append(file_info.path) + + # Extract diff from the appropriate source (delta for incremental, full for initial) + file_diff = "" + if diff_source: + for f in diff_source.files: + if f.path == file_info.path or f.path.endswith("/" + file_info.path): + file_diff = f.content + break + + batch_files_data.append({ + "path": file_info.path, + "type": "MODIFIED", + "focus_areas": file_info.focus_areas, + "old_code": "", + "diff": file_diff or "(Diff unavailable)", + "is_incremental": is_incremental # Pass mode to prompt builder + }) + + # Filter pre-computed diff snippets for files in this batch (for RAG query) + # The diffSnippets from ReviewRequestDto are already properly extracted by Java DiffParser + batch_diff_snippets = self._get_diff_snippets_for_batch( + request.diffSnippets or [], + batch_file_paths + ) + + # Fetch per-batch RAG context (targeted to these specific files) + rag_context_text = "" + if self.rag_client and self.rag_client.enabled: + try: + rag_response = await self.rag_client.get_pr_context( + workspace=request.projectWorkspace, + project=request.projectVcsRepoSlug, + branch=request.targetBranchName, + changed_files=batch_file_paths, + diff_snippets=batch_diff_snippets, + pr_title=request.prTitle, + top_k=5, # Focused context for this batch + min_relevance_score=0.75 # Higher threshold for per-batch + ) + # Pass ALL changed files from the PR to filter out stale context + # RAG returns context from main branch, so files being modified in PR are stale + rag_context_text = self._format_rag_context( + rag_response.get("context", {}), + set(batch_file_paths), + pr_changed_files=request.changedFiles + ) + logger.debug(f"Batch RAG context retrieved for {batch_file_paths}") + except Exception as e: + logger.warning(f"Failed to fetch per-batch RAG context: {e}") + + # For incremental mode, filter previous issues relevant to this batch + previous_issues_for_batch = "" + if is_incremental and request.previousCodeAnalysisIssues: + relevant_prev_issues = [ + issue for issue in request.previousCodeAnalysisIssues + if self._issue_matches_files(issue, batch_file_paths) + ] + if relevant_prev_issues: + previous_issues_for_batch = self._format_previous_issues_for_batch(relevant_prev_issues) + + # Build ONE prompt for the batch + prompt = PromptBuilder.build_stage_1_batch_prompt( + files=batch_files_data, + priority=batch_items[0]["priority"] if batch_items else "MEDIUM", + project_rules=project_rules, + rag_context=rag_context_text, + is_incremental=is_incremental, + previous_issues=previous_issues_for_batch + ) + + # Stage 1 uses direct LLM call (no tools needed - diff is already provided) + try: + # Try structured output first + structured_llm = self.llm.with_structured_output(FileReviewBatchOutput) + result = await structured_llm.ainvoke(prompt) + if result: + all_batch_issues = [] + for review in result.reviews: + all_batch_issues.extend(review.issues) + return all_batch_issues + except Exception as e: + logger.warning(f"Structured output failed for Stage 1 batch: {e}") + + # Fallback to manual parsing + try: + response = await self.llm.ainvoke(prompt) + content = extract_llm_response_text(response) + data = await self._parse_response(content, FileReviewBatchOutput) + all_batch_issues = [] + for review in data.reviews: + all_batch_issues.extend(review.issues) + return all_batch_issues + except Exception as parse_err: + logger.error(f"Batch review failed: {parse_err}") + return [] + + return [] + + async def _execute_stage_2_cross_file( + self, + request: ReviewRequestDto, + stage_1_issues: List[CodeReviewIssue], + plan: ReviewPlan + ) -> CrossFileAnalysisResult: + """ + Stage 2: Cross-file analysis. + """ + # Serialize Stage 1 findings + issues_json = json.dumps([i.model_dump() for i in stage_1_issues], indent=2) + + prompt = PromptBuilder.build_stage_2_cross_file_prompt( + repo_slug=request.projectVcsRepoSlug, + pr_title=request.prTitle or "", + commit_hash=request.commitHash or "HEAD", + stage_1_findings_json=issues_json, + architecture_context="(Architecture context from MCP or knowledge base)", + migrations="(Migration scripts found in PR)", + cross_file_concerns=plan.cross_file_concerns + ) + + # Stage 2 uses direct LLM call (no tools needed - all data is provided from Stage 1) + try: + structured_llm = self.llm.with_structured_output(CrossFileAnalysisResult) + result = await structured_llm.ainvoke(prompt) + if result: + logger.info("Stage 2 cross-file analysis completed with structured output") + return result + except Exception as e: + logger.warning(f"Structured output failed for Stage 2: {e}") + + # Fallback to manual parsing + try: + response = await self.llm.ainvoke(prompt) + content = extract_llm_response_text(response) + return await self._parse_response(content, CrossFileAnalysisResult) + except Exception as e: + logger.error(f"Stage 2 cross-file analysis failed: {e}") + raise + + async def _execute_stage_3_aggregation( + self, + request: ReviewRequestDto, + plan: ReviewPlan, + stage_1_issues: List[CodeReviewIssue], + stage_2_results: CrossFileAnalysisResult, + is_incremental: bool = False + ) -> str: + """ + Stage 3: Generate Markdown report. + In incremental mode, includes summary of resolved vs new issues. + """ + stage_1_json = json.dumps([i.model_dump() for i in stage_1_issues], indent=2) + stage_2_json = stage_2_results.model_dump_json(indent=2) + plan_json = plan.model_dump_json(indent=2) + + # Add incremental context to aggregation + incremental_context = "" + if is_incremental: + resolved_count = sum(1 for i in stage_1_issues if i.isResolved) + new_count = len(stage_1_issues) - resolved_count + previous_count = len(request.previousCodeAnalysisIssues or []) + incremental_context = f""" +## INCREMENTAL REVIEW SUMMARY +- Previous issues from last review: {previous_count} +- Issues resolved in this update: {resolved_count} +- New issues found in delta: {new_count} +- Total issues after reconciliation: {len(stage_1_issues)} +""" + + prompt = PromptBuilder.build_stage_3_aggregation_prompt( + repo_slug=request.projectVcsRepoSlug, + pr_id=str(request.pullRequestId), + author="Unknown", + pr_title=request.prTitle or "", + total_files=len(request.changedFiles or []), + additions=0, # Need accurate stats + deletions=0, + stage_0_plan=plan_json, + stage_1_issues_json=stage_1_json, + stage_2_findings_json=stage_2_json, + recommendation=stage_2_results.pr_recommendation + ) + + response = await self.llm.ainvoke(prompt) + return extract_llm_response_text(response) + + async def _parse_response(self, content: str, model_class: Any, retries: int = 2) -> Any: + """ + Robustly parse JSON response into a Pydantic model with retries. + Falls back to manual parsing if structured output wasn't used. + """ + last_error = None + + # Initial cleaning attempt + try: + cleaned = self._clean_json_text(content) + logger.debug(f"Cleaned JSON for {model_class.__name__} (first 500 chars): {cleaned[:500]}") + data = json.loads(cleaned) + return model_class(**data) + except Exception as e: + last_error = e + logger.warning(f"Initial parse failed for {model_class.__name__}: {e}") + logger.debug(f"Raw content (first 1000 chars): {content[:1000]}") + + # Retry with structured output if available + try: + logger.info(f"Attempting structured output retry for {model_class.__name__}") + structured_llm = self.llm.with_structured_output(model_class) + result = await structured_llm.ainvoke( + f"Parse and return this as valid {model_class.__name__}:\n{content[:4000]}" + ) + if result: + logger.info(f"Structured output retry succeeded for {model_class.__name__}") + return result + except Exception as e: + logger.warning(f"Structured output retry failed: {e}") + last_error = e + + # Final fallback: LLM repair loop + for attempt in range(retries): + try: + logger.info(f"Repairing JSON for {model_class.__name__}, attempt {attempt+1}") + repaired = await self._repair_json_with_llm( + content, + str(last_error), + model_class.model_json_schema() + ) + cleaned = self._clean_json_text(repaired) + logger.debug(f"Repaired JSON attempt {attempt+1} (first 500 chars): {cleaned[:500]}") + data = json.loads(cleaned) + return model_class(**data) + except Exception as e: + last_error = e + logger.warning(f"Retry {attempt+1} failed: {e}") + + raise ValueError(f"Failed to parse {model_class.__name__} after retries: {last_error}") + + async def _repair_json_with_llm(self, broken_json: str, error: str, schema: Any) -> str: + """ + Ask LLM to repair malformed JSON. + """ + # Truncate the broken JSON to avoid token limits but show enough context + truncated_json = broken_json[:3000] if len(broken_json) > 3000 else broken_json + + prompt = f"""You are a JSON repair expert. +The following JSON failed to parse/validate: +Error: {error} + +Broken JSON: +{truncated_json} + +Required Schema (the output MUST be a JSON object, not an array): +{json.dumps(schema, indent=2)} + +CRITICAL INSTRUCTIONS: +1. Return ONLY the fixed valid JSON object +2. The response MUST start with {{ and end with }} +3. All property names MUST be enclosed in double quotes +4. No markdown code blocks (no ```) +5. No explanatory text before or after the JSON +6. Ensure all required fields from the schema are present + +Output the corrected JSON object now:""" + response = await self.llm.ainvoke(prompt) + return extract_llm_response_text(response) + + def _clean_json_text(self, text: str) -> str: + """ + Clean markdown and extraneous text from JSON. + """ + text = text.strip() + + # Remove markdown code blocks + if text.startswith("```"): + lines = text.split("\n") + # Skip the opening ``` line (with or without language identifier) + lines = lines[1:] + # Remove trailing ``` if present + if lines and lines[-1].strip() == "```": + lines = lines[:-1] + text = "\n".join(lines).strip() + + # Also handle case where ``` appears mid-text + if "```json" in text: + start_idx = text.find("```json") + end_idx = text.find("```", start_idx + 7) + if end_idx != -1: + text = text[start_idx + 7:end_idx].strip() + else: + text = text[start_idx + 7:].strip() + elif "```" in text: + # Generic code block without language + start_idx = text.find("```") + remaining = text[start_idx + 3:] + end_idx = remaining.find("```") + if end_idx != -1: + text = remaining[:end_idx].strip() + else: + text = remaining.strip() + + # Find JSON object boundaries + obj_start = text.find("{") + obj_end = text.rfind("}") + arr_start = text.find("[") + arr_end = text.rfind("]") + + # Determine if we have an object or array (whichever comes first) + if obj_start != -1 and obj_end != -1: + if arr_start == -1 or obj_start < arr_start: + # Object comes first or no array + text = text[obj_start:obj_end+1] + elif arr_start < obj_start and arr_end != -1: + # Array comes first - but we need an object for Pydantic + # Check if the object is nested inside the array or separate + if obj_end > arr_end: + # Object extends beyond array - likely the object we want + text = text[obj_start:obj_end+1] + else: + # Try to use the object anyway + text = text[obj_start:obj_end+1] + elif arr_start != -1 and arr_end != -1 and obj_start == -1: + # Only array found - log warning as Pydantic models expect objects + logger.warning(f"JSON cleaning found array instead of object, this may fail parsing") + text = text[arr_start:arr_end+1] + + return text + + def _format_rag_context( + self, + rag_context: Optional[Dict[str, Any]], + relevant_files: Optional[set] = None, + pr_changed_files: Optional[List[str]] = None + ) -> str: + """ + Format RAG context into a readable string for the prompt. + Optionally filter to chunks relevant to specific files. + + Args: + rag_context: RAG response with code chunks + relevant_files: Files in current batch to prioritize + pr_changed_files: ALL files modified in the PR - chunks from these files + are marked as potentially stale (from main branch, not PR branch) + """ + if not rag_context: + return "" + + # Handle both "chunks" and "relevant_code" keys (RAG API uses "relevant_code") + chunks = rag_context.get("relevant_code", []) or rag_context.get("chunks", []) + if not chunks: + return "" + + # Normalize PR changed files for comparison + pr_changed_set = set() + if pr_changed_files: + for f in pr_changed_files: + pr_changed_set.add(f) + # Also add just the filename for matching + if "/" in f: + pr_changed_set.add(f.rsplit("/", 1)[-1]) + + formatted_parts = [] + included_count = 0 + for chunk in chunks: + if included_count >= 10: # Limit to top 10 chunks for focused context + break + + # Extract metadata + metadata = chunk.get("metadata", {}) + path = metadata.get("path", chunk.get("path", "unknown")) + chunk_type = metadata.get("type", chunk.get("type", "code")) + score = chunk.get("score", chunk.get("relevance_score", 0)) + + # Check if this chunk is from a file being modified in the PR + is_from_modified_file = False + if pr_changed_set: + path_filename = path.rsplit("/", 1)[-1] if "/" in path else path + is_from_modified_file = ( + path in pr_changed_set or + path_filename in pr_changed_set or + any(path.endswith(f) or f.endswith(path) for f in pr_changed_set) + ) + + # Skip chunks from files being modified in the PR - they're stale + if is_from_modified_file: + logger.debug(f"Skipping RAG chunk from modified file: {path}") + continue + + # Optionally filter by relevance to batch files + if relevant_files: + # Include if the chunk's path relates to any batch file + is_relevant = any( + path in f or f in path or + path.rsplit("/", 1)[-1] == f.rsplit("/", 1)[-1] + for f in relevant_files + ) + # Also include high-scoring chunks regardless + if not is_relevant and score < 0.85: + continue + + text = chunk.get("text", chunk.get("content", "")) # Truncate long chunks + if not text: + continue + + included_count += 1 + formatted_parts.append( + f"### Related Code #{included_count} (relevance: {score:.2f})\n" + f"File: {path}\n" + f"Type: {chunk_type}\n" + f"```\n{text}\n```\n" + ) + + if not formatted_parts: + return "" + + return "\n".join(formatted_parts) + + def _emit_status(self, state: str, message: str): + if self.event_callback: + self.event_callback({ + "type": "status", + "state": state, + "message": message + }) + + def _emit_progress(self, percent: int, message: str): + if self.event_callback: + self.event_callback({ + "type": "progress", + "percent": percent, + "message": message + }) + + def _emit_error(self, message: str): + if self.event_callback: + self.event_callback({ + "type": "error", + "message": message + }) + + def _count_files(self, plan: ReviewPlan) -> int: + count = 0 + for group in plan.file_groups: + count += len(group.files) + return count + + def _ensure_all_files_planned(self, plan: ReviewPlan, all_changed_files: List[str]) -> ReviewPlan: + """ + Ensure all changed files are included in the review plan. + If LLM missed files, add them to a LOW priority group. + """ + # Collect files already in the plan + planned_files = set() + for group in plan.file_groups: + for f in group.files: + planned_files.add(f.path) + + # Also count skipped files + skipped_files = set() + for skip in plan.files_to_skip: + skipped_files.add(skip.path) + + # Find missing files + all_files_set = set(all_changed_files) + missing_files = all_files_set - planned_files - skipped_files + + if missing_files: + logger.warning( + f"Stage 0 plan missing {len(missing_files)} files out of {len(all_changed_files)}. " + f"Adding to LOW priority group." + ) + + # Create ReviewFile objects for missing files + missing_review_files = [] + for path in missing_files: + missing_review_files.append(ReviewFile( + path=path, + focus_areas=["GENERAL"], + risk_level="LOW", + estimated_issues=0 + )) + + # Add a new group for missing files or append to existing LOW group + low_group_found = False + for group in plan.file_groups: + if group.priority == "LOW": + group.files.extend(missing_review_files) + low_group_found = True + break + + if not low_group_found: + plan.file_groups.append(FileGroup( + group_id="GROUP_MISSING_FILES", + priority="LOW", + rationale="Files not categorized by planner - added automatically", + files=missing_review_files + )) + + logger.info(f"Plan now includes {self._count_files(plan)} files for review") + else: + logger.info( + f"Stage 0 plan complete: {len(planned_files)} files to review, " + f"{len(skipped_files)} files skipped" + ) + + return plan diff --git a/python-ecosystem/mcp-client/service/pooled_review_service.py b/python-ecosystem/mcp-client/service/pooled_review_service.py index 25d03e8b..dce735bb 100644 --- a/python-ecosystem/mcp-client/service/pooled_review_service.py +++ b/python-ecosystem/mcp-client/service/pooled_review_service.py @@ -23,7 +23,7 @@ from utils.mcp_pool import get_mcp_pool, McpProcessPool from model.models import ReviewRequestDto from llm.llm_factory import LLMFactory -from utils.prompt_builder import PromptBuilder +from utils.prompts.prompt_builder import PromptBuilder from utils.response_parser import ResponseParser from service.rag_client import RagClient @@ -247,7 +247,7 @@ def _create_llm(self, request): max_tokens=request.maxAllowedTokens ) - +#TODO: Implement pooling logic # Example usage in web server async def create_pooled_service(): """Create and initialize a pooled review service.""" diff --git a/python-ecosystem/mcp-client/service/rag_client.py b/python-ecosystem/mcp-client/service/rag_client.py index d5d4e978..de85951a 100644 --- a/python-ecosystem/mcp-client/service/rag_client.py +++ b/python-ecosystem/mcp-client/service/rag_client.py @@ -16,6 +16,9 @@ class RagClient: """Client for interacting with the RAG Pipeline API.""" + + # Shared HTTP client for connection pooling + _shared_client: Optional[httpx.AsyncClient] = None def __init__(self, base_url: Optional[str] = None, enabled: Optional[bool] = None): """ @@ -33,6 +36,21 @@ def __init__(self, base_url: Optional[str] = None, enabled: Optional[bool] = Non logger.info(f"RAG client initialized: {self.base_url}") else: logger.info("RAG client disabled") + + async def _get_client(self) -> httpx.AsyncClient: + """Get or create a shared HTTP client for connection pooling.""" + if RagClient._shared_client is None or RagClient._shared_client.is_closed: + RagClient._shared_client = httpx.AsyncClient( + timeout=self.timeout, + limits=httpx.Limits(max_connections=10, max_keepalive_connections=5) + ) + return RagClient._shared_client + + async def close(self): + """Close the shared HTTP client.""" + if RagClient._shared_client is not None and not RagClient._shared_client.is_closed: + await RagClient._shared_client.aclose() + RagClient._shared_client = None async def get_pr_context( self, @@ -96,20 +114,20 @@ async def get_pr_context( "min_relevance_score": min_relevance_score } - async with httpx.AsyncClient(timeout=self.timeout) as client: - response = await client.post( - f"{self.base_url}/query/pr-context", - json=payload - ) - response.raise_for_status() - result = response.json() - - # Log timing and result stats - elapsed_ms = (datetime.now() - start_time).total_seconds() * 1000 - chunk_count = len(result.get("context", {}).get("relevant_code", [])) - logger.info(f"RAG query completed in {elapsed_ms:.2f}ms, retrieved {chunk_count} chunks") - - return result + client = await self._get_client() + response = await client.post( + f"{self.base_url}/query/pr-context", + json=payload + ) + response.raise_for_status() + result = response.json() + + # Log timing and result stats + elapsed_ms = (datetime.now() - start_time).total_seconds() * 1000 + chunk_count = len(result.get("context", {}).get("relevant_code", [])) + logger.info(f"RAG query completed in {elapsed_ms:.2f}ms, retrieved {chunk_count} chunks") + + return result except httpx.HTTPError as e: logger.warning(f"Failed to retrieve PR context from RAG: {e}") @@ -155,13 +173,13 @@ async def semantic_search( if filter_language: payload["filter_language"] = filter_language - async with httpx.AsyncClient(timeout=self.timeout) as client: - response = await client.post( - f"{self.base_url}/query/search", - json=payload - ) - response.raise_for_status() - return response.json() + client = await self._get_client() + response = await client.post( + f"{self.base_url}/query/search", + json=payload + ) + response.raise_for_status() + return response.json() except httpx.HTTPError as e: logger.warning(f"Semantic search failed: {e}") @@ -181,9 +199,9 @@ async def is_healthy(self) -> bool: return False try: - async with httpx.AsyncClient(timeout=5.0) as client: - response = await client.get(f"{self.base_url}/health") - return response.status_code == 200 + client = await self._get_client() + response = await client.get(f"{self.base_url}/health") + return response.status_code == 200 except Exception as e: logger.warning(f"RAG health check failed: {e}") return False diff --git a/python-ecosystem/mcp-client/service/review_service.py b/python-ecosystem/mcp-client/service/review_service.py index baeef56a..ef9bb770 100644 --- a/python-ecosystem/mcp-client/service/review_service.py +++ b/python-ecosystem/mcp-client/service/review_service.py @@ -4,25 +4,20 @@ from datetime import datetime from typing import Dict, Any, Optional, Callable from dotenv import load_dotenv -from mcp_use import MCPAgent, MCPClient -from langchain_core.agents import AgentAction +from mcp_use import MCPClient -from model.models import ReviewRequestDto, CodeReviewOutput, CodeReviewIssue, AnalysisMode +from model.models import ReviewRequestDto from utils.mcp_config import MCPConfigBuilder from llm.llm_factory import LLMFactory -from utils.prompt_builder import PromptBuilder +from utils.prompts.prompt_builder import PromptBuilder from utils.response_parser import ResponseParser -from service.rag_client import RagClient, RAG_MIN_RELEVANCE_SCORE, RAG_DEFAULT_TOP_K +from service.rag_client import RagClient, RAG_DEFAULT_TOP_K from service.llm_reranker import LLMReranker -from service.issue_post_processor import IssuePostProcessor, post_process_analysis_result -from utils.context_builder import ( - ContextBuilder, ContextBudget, RagReranker, - RAGMetrics, SmartChunker, get_rag_cache -) -from utils.file_classifier import FileClassifier, FilePriority -from utils.prompt_logger import PromptLogger -from utils.diff_processor import DiffProcessor, ProcessedDiff, format_diff_for_prompt -from utils.error_sanitizer import sanitize_error_for_display, create_user_friendly_error +from service.issue_post_processor import post_process_analysis_result +from utils.context_builder import (RAGMetrics, get_rag_cache) +from utils.diff_processor import DiffProcessor +from utils.error_sanitizer import create_user_friendly_error +from service.multi_stage_orchestrator import MultiStageReviewOrchestrator logger = logging.getLogger(__name__) @@ -69,32 +64,6 @@ async def process_review_request( event_callback=event_callback ) - async def process_review_request_with_local_repo( - self, - request: ReviewRequestDto, - repo_path: str, - max_allowed_tokens: Optional[int] = None, - event_callback: Optional[Callable[[Dict], None]] = None - ) -> Dict[str, Any]: - """ - Process a review request using a local repository directory. - - Args: - request: The review request data - repo_path: Path to the local repository - max_allowed_tokens: Optional token limit - event_callback: Optional callback to receive progress events - - Returns: - Dict with "result" key containing the analysis result or error - """ - return await self._process_review( - request=request, - repo_path=repo_path, - max_allowed_tokens=max_allowed_tokens, - event_callback=event_callback - ) - async def _process_review( self, request: ReviewRequestDto, @@ -155,11 +124,9 @@ async def _process_review( # Fetch RAG context if enabled rag_context = await self._fetch_rag_context(request, event_callback) - # Build prompt - different depending on whether we have rawDiff - pr_metadata = self._build_pr_metadata(request) - + # Build processed_diff if rawDiff is available to optimize Stage 1 + processed_diff = None if has_raw_diff: - # Process and embed diff directly in prompt diff_processor = DiffProcessor() processed_diff = diff_processor.process(request.rawDiff) @@ -174,16 +141,6 @@ async def _process_review( "type": "warning", "message": processed_diff.truncation_reason }) - - prompt = self._build_prompt_with_diff( - request=request, - pr_metadata=pr_metadata, - processed_diff=processed_diff, - rag_context=rag_context - ) - else: - # Standard prompt - agent will fetch diff via MCP tool - prompt = self._build_prompt(request, pr_metadata, rag_context) self._emit_event(event_callback, { "type": "status", @@ -191,15 +148,40 @@ async def _process_review( "message": "MCP server ready, starting analysis" }) - # Execute review with MCP agent - always use agent for tool access - result = await self._execute_review_with_streaming( + + # Use the new pipeline + self._emit_event(event_callback, { + "type": "status", + "state": "multi_stage_started", + "message": "Starting Multi-Stage Review Pipeline" + }) + + # This replaces the monolithic _execute_review_with_streaming call + orchestrator = MultiStageReviewOrchestrator( llm=llm, - client=client, - prompt=prompt, - event_callback=event_callback, - request=request + mcp_client=client, + rag_client=self.rag_client, + event_callback=event_callback ) + # Check for Branch Analysis / Reconciliation mode + if request.analysisType == "BRANCH_ANALYSIS": + logger.info("Executing Branch Analysis & Reconciliation mode") + # Build specific prompt for branch analysis + pr_metadata = self._build_pr_metadata(request) + prompt = PromptBuilder.build_branch_review_prompt_with_branch_issues_data(pr_metadata) + + result = await orchestrator.execute_branch_analysis(prompt) + else: + # Execute review with Multi-Stage Orchestrator + # Standard PR Review + result = await orchestrator.orchestrate_review( + request=request, + rag_context=rag_context, + processed_diff=processed_diff + ) + + # Post-process issues to fix line numbers and merge duplicates if result and 'issues' in result: self._emit_event(event_callback, { @@ -253,481 +235,6 @@ async def _process_review( }) return {"result": error_response} - async def _execute_review_with_streaming( - self, - llm, - client: MCPClient, - prompt: str, - event_callback: Optional[Callable[[Dict], None]], - request: Optional[ReviewRequestDto] = None - ) -> Dict[str, Any]: - """ - Execute the code review using MCP agent with real streaming output. - - This method uses the agent's stream() method to capture intermediate outputs - (tool calls, observations) and forwards them via event_callback in real-time. - Uses output_schema for structured JSON output. - """ - additional_instructions = PromptBuilder.get_additional_instructions() - max_steps = 120 - - # Create agent - agent = MCPAgent( - llm=llm, - client=client, - max_steps=max_steps, - additional_instructions=additional_instructions, - ) - - try: - self._emit_event(event_callback, { - "type": "progress", - "step": 0, - "max_steps": max_steps, - "message": "Agent execution started" - }) - - step_count = 0 - final_result = None - - # Use streaming with output_schema for structured JSON output - async for item in agent.stream( - prompt, - max_steps=max_steps, - output_schema=CodeReviewOutput - ): - # item can be: - # - tuple[AgentAction, str]: (action, observation) for tool calls - # - str: intermediate text - # - CodeReviewOutput: final structured output - - if isinstance(item, tuple) and len(item) == 2: - # Tool call with observation - action, observation = item - step_count += 1 - - tool_name = action.tool if hasattr(action, 'tool') else str(action) - tool_input = action.tool_input if hasattr(action, 'tool_input') else {} - - # Truncate observation for logging - obs_preview = str(observation)[:200] + "..." if len(str(observation)) > 200 else str(observation) - - logger.info(f"[Step {step_count}] Tool: {tool_name}, Input: {tool_input}") - logger.debug(f"[Step {step_count}] Observation: {obs_preview}") - - self._emit_event(event_callback, { - "type": "mcp_step", - "step": step_count, - "max_steps": max_steps, - "tool": tool_name, - "tool_input": tool_input, - "observation_preview": obs_preview, - "message": f"Executed tool: {tool_name}" - }) - - elif isinstance(item, CodeReviewOutput): - # Final structured output - final_result = item - logger.info(f"Received structured output with {len(item.issues)} issues") - - elif isinstance(item, str): - # Intermediate text output - final_result = item - logger.debug(f"Intermediate output: {item[:100]}...") - - self._emit_event(event_callback, { - "type": "progress", - "step": step_count, - "max_steps": max_steps, - "message": "Processing..." - }) - - # Emit completion progress - self._emit_event(event_callback, { - "type": "progress", - "step": max_steps, - "max_steps": max_steps, - "message": f"Agent execution completed ({step_count} tool calls)" - }) - - # Process the result - if isinstance(final_result, CodeReviewOutput): - # Convert Pydantic model to dict - result_dict = { - "comment": final_result.comment, - "issues": [issue.model_dump() for issue in final_result.issues] - } - logger.info(f"Structured output: {len(final_result.issues)} issues found") - - # Log using PromptLogger - PromptLogger.log_llm_response( - str(result_dict), - metadata={"result_type": "structured", "issue_count": len(final_result.issues)}, - is_raw=False - ) - return result_dict - - elif isinstance(final_result, str) and final_result: - # Fallback: parse string result - logger.info(f"Agent returned string result, attempting to parse") - result_preview = final_result[:500] if len(final_result) > 500 else final_result - logger.info(f"Agent raw result preview: {result_preview}") - - PromptLogger.log_llm_response( - final_result, - metadata={"result_length": len(final_result)}, - is_raw=True - ) - - ResponseParser.reset_retry_state() - parsed_result = ResponseParser.extract_json_from_response(final_result) - - # Check if parsing failed and try LLM fix - if parsed_result.get("_needs_retry") and final_result: - logger.info("Initial parsing failed, attempting LLM-based fix...") - self._emit_event(event_callback, { - "type": "progress", - "step": max_steps, - "max_steps": max_steps, - "message": "Attempting to fix response format..." - }) - - fixed_result = await self._try_fix_response_with_llm( - final_result, llm, event_callback - ) - if fixed_result: - fixed_result.pop("_needs_retry", None) - fixed_result.pop("_raw_response", None) - return fixed_result - - parsed_result.pop("_needs_retry", None) - parsed_result.pop("_raw_response", None) - return parsed_result - else: - logger.warning("Agent returned empty or None result") - return ResponseParser.create_error_response( - "Empty response", "Agent returned no result" - ) - - except Exception as e: - # Log full error for debugging, sanitize for user display - logger.error(f"Agent execution error: {str(e)}", exc_info=True) - sanitized_message = create_user_friendly_error(e) - - self._emit_event(event_callback, { - "type": "error", - "message": sanitized_message - }) - raise - - async def _try_fix_response_with_llm( - self, - raw_response: str, - llm, - event_callback: Optional[Callable[[Dict], None]] - ) -> Optional[Dict[str, Any]]: - """ - Try to fix a malformed response using a direct LLM call. - - Args: - raw_response: The raw response that failed to parse - llm: The LLM instance to use for fixing - event_callback: Optional callback for progress events - - Returns: - Fixed and parsed response, or None if all retries failed - """ - fix_prompt = ResponseParser.get_fix_prompt(raw_response) - - for attempt in range(self.MAX_FIX_RETRIES): - try: - logger.info(f"LLM fix attempt {attempt + 1}/{self.MAX_FIX_RETRIES}") - self._emit_event(event_callback, { - "type": "progress", - "step": 0, - "max_steps": self.MAX_FIX_RETRIES, - "message": f"Fix attempt {attempt + 1}/{self.MAX_FIX_RETRIES}..." - }) - - # Make a simple direct LLM call (no MCP tools) - response = await llm.ainvoke(fix_prompt) - - # Extract the content from the response - if hasattr(response, 'content'): - fixed_text = response.content - else: - fixed_text = str(response) - - logger.info(f"LLM fix response preview: {fixed_text[:300] if len(fixed_text) > 300 else fixed_text}") - - # Try to parse the fixed response - # Don't track retry state for the fix attempts - ResponseParser._last_parse_needs_retry = False - fixed_result = ResponseParser.extract_json_from_response(fixed_text) - - # Check if parsing succeeded (no error markers) - if not fixed_result.get("_needs_retry"): - if "comment" in fixed_result and "issues" in fixed_result: - # Validate it's not an error response - comment = fixed_result.get("comment", "") - if not comment.startswith("Failed to parse"): - logger.info(f"LLM fix succeeded on attempt {attempt + 1}") - self._emit_event(event_callback, { - "type": "status", - "state": "fix_succeeded", - "message": f"Response fixed successfully on attempt {attempt + 1}" - }) - return fixed_result - - logger.warning(f"LLM fix attempt {attempt + 1} produced invalid result") - - except Exception as e: - logger.error(f"LLM fix attempt {attempt + 1} failed with error: {e}") - self._emit_event(event_callback, { - "type": "warning", - "message": f"Fix attempt {attempt + 1} failed: {str(e)}" - }) - - logger.error(f"All {self.MAX_FIX_RETRIES} LLM fix attempts failed") - self._emit_event(event_callback, { - "type": "warning", - "message": f"Failed to fix response after {self.MAX_FIX_RETRIES} attempts" - }) - return None - - async def _execute_direct_review( - self, - llm, - prompt: str, - event_callback: Optional[Callable[[Dict], None]] - ) -> Dict[str, Any]: - """ - Execute code review using direct LLM call (no MCP agent). - - This is more efficient when diff is already available. - """ - max_steps = 10 # Simplified progress for direct mode - - try: - self._emit_event(event_callback, { - "type": "progress", - "step": 1, - "max_steps": max_steps, - "message": "Sending request to LLM" - }) - - # Direct LLM call - response = await llm.ainvoke(prompt) - - # Extract content - if hasattr(response, 'content'): - raw_result = response.content - else: - raw_result = str(response) - - self._emit_event(event_callback, { - "type": "progress", - "step": 5, - "max_steps": max_steps, - "message": "Processing LLM response" - }) - - # Log response - if raw_result: - result_preview = raw_result[:500] if len(raw_result) > 500 else raw_result - logger.info(f"Direct review result preview: {result_preview}") - - PromptLogger.log_llm_response( - raw_result, - metadata={"mode": "direct", "result_length": len(raw_result)}, - is_raw=True - ) - else: - logger.warning("LLM returned empty response") - - # Parse result - ResponseParser.reset_retry_state() - parsed_result = ResponseParser.extract_json_from_response(raw_result) - - # Try to fix if needed - if parsed_result.get("_needs_retry") and raw_result: - logger.info("Direct mode: Initial parsing failed, attempting fix...") - fixed_result = await self._try_fix_response_with_llm( - raw_result, llm, event_callback - ) - if fixed_result: - fixed_result.pop("_needs_retry", None) - fixed_result.pop("_raw_response", None) - return fixed_result - - parsed_result.pop("_needs_retry", None) - parsed_result.pop("_raw_response", None) - - self._emit_event(event_callback, { - "type": "progress", - "step": max_steps, - "max_steps": max_steps, - "message": "Analysis completed" - }) - - return parsed_result - - except Exception as e: - logger.exception("Direct review execution failed") - self._emit_event(event_callback, { - "type": "error", - "message": f"Direct review failed: {str(e)}" - }) - raise - - def _build_prompt_with_diff( - self, - request: ReviewRequestDto, - pr_metadata: Dict[str, Any], - processed_diff: ProcessedDiff, - rag_context: Optional[Dict[str, Any]] = None - ) -> str: - """ - Build prompt for MCP agent with embedded diff. - - This prompt includes the actual diff content so agent doesn't need - to call getPullRequestDiff, but still has access to all other MCP tools. - - Supports three modes: - - FULL analysis (first review): Standard first review prompt - - Previous analysis (subsequent full review): Review with previous issues - - INCREMENTAL analysis: Focus on delta diff since last analyzed commit - """ - analysis_type = request.analysisType - analysis_mode_str = request.analysisMode or "FULL" - # Convert string to AnalysisMode enum for comparison - try: - analysis_mode = AnalysisMode(analysis_mode_str) - except ValueError: - logger.warning(f"Unknown analysis mode '{analysis_mode_str}', defaulting to FULL") - analysis_mode = AnalysisMode.FULL - has_previous_analysis = bool(request.previousCodeAnalysisIssues) - has_delta_diff = bool(request.deltaDiff) - - if analysis_type is not None and analysis_type == "BRANCH_ANALYSIS": - return PromptBuilder.build_branch_review_prompt_with_branch_issues_data(pr_metadata) - - # Build structured context for Lost-in-the-Middle protection - structured_context = None - if request.changedFiles: - try: - changed_files = request.changedFiles or [] - classified_files = FileClassifier.classify_files(changed_files) - - if rag_context and rag_context.get("relevant_code"): - rag_context["relevant_code"] = RagReranker.rerank_by_file_priority( - rag_context["relevant_code"], - classified_files - ) - rag_context["relevant_code"] = RagReranker.deduplicate_by_content( - rag_context["relevant_code"] - ) - rag_context["relevant_code"] = RagReranker.filter_by_relevance_threshold( - rag_context["relevant_code"], - min_score=RAG_MIN_RELEVANCE_SCORE, - min_results=3 - ) - - budget = ContextBudget.for_model(request.aiModel) - rag_token_budget = budget.rag_tokens - avg_tokens_per_chunk = 600 - max_rag_chunks = max(5, min(15, rag_token_budget // avg_tokens_per_chunk)) - - structured_context = PromptBuilder.build_structured_rag_section( - rag_context, - max_chunks=max_rag_chunks, - token_budget=rag_token_budget - ) - - stats = FileClassifier.get_priority_stats(classified_files) - logger.info(f"File classification for direct mode: {stats}") - - except Exception as e: - logger.warning(f"Failed to build structured context: {e}") - structured_context = None - - # Format full diff for prompt - formatted_diff = format_diff_for_prompt( - processed_diff, - include_stats=True, - max_chars=None # Let token budget handle truncation - ) - - # Check if we should use INCREMENTAL mode with delta diff - is_incremental = ( - analysis_mode == AnalysisMode.INCREMENTAL - and has_delta_diff - and has_previous_analysis - ) - - if is_incremental: - # INCREMENTAL mode: focus on delta diff - logger.info(f"Using INCREMENTAL analysis mode with delta diff") - - # Add commit hashes to pr_metadata for the prompt - pr_metadata["previousCommitHash"] = request.previousCommitHash - pr_metadata["currentCommitHash"] = request.currentCommitHash - - prompt = PromptBuilder.build_incremental_review_prompt( - pr_metadata=pr_metadata, - delta_diff_content=request.deltaDiff, - full_diff_content=formatted_diff, - rag_context=rag_context, - structured_context=structured_context - ) - elif has_previous_analysis: - # FULL mode with previous analysis - logger.info(f"Using FULL analysis mode with previous analysis data") - prompt = PromptBuilder.build_direct_review_prompt_with_previous_analysis( - pr_metadata=pr_metadata, - diff_content=formatted_diff, - rag_context=rag_context, - structured_context=structured_context - ) - else: - # FULL mode - first review - logger.info(f"Using FULL analysis mode (first review)") - prompt = PromptBuilder.build_direct_first_review_prompt( - pr_metadata=pr_metadata, - diff_content=formatted_diff, - rag_context=rag_context, - structured_context=structured_context - ) - - # Log prompt - prompt_metadata = { - "workspace": request.projectVcsWorkspace, - "repo": request.projectVcsRepoSlug, - "pr_id": request.pullRequestId, - "model": request.aiModel, - "provider": request.aiProvider, - "mode": "incremental" if is_incremental else "direct", - "analysis_mode": str(analysis_mode) if analysis_mode else "FULL", - "has_previous_analysis": has_previous_analysis, - "has_delta_diff": has_delta_diff, - "previous_commit_hash": request.previousCommitHash, - "current_commit_hash": request.currentCommitHash, - "changed_files_count": processed_diff.total_files, - "diff_size_bytes": processed_diff.processed_size_bytes, - "delta_diff_size_bytes": len(request.deltaDiff) if request.deltaDiff else 0, - "rag_chunks_count": len(rag_context.get("relevant_code", [])) if rag_context else 0, - } - - if rag_context: - PromptLogger.log_rag_context(rag_context, prompt_metadata, stage="rag_direct_mode") - - if structured_context: - PromptLogger.log_structured_context(structured_context, prompt_metadata) - - PromptLogger.log_prompt(prompt, prompt_metadata, stage="direct_full_prompt") - - return prompt - def _build_jvm_props( self, request: ReviewRequestDto, @@ -876,102 +383,6 @@ async def _fetch_rag_context( }) return None - def _build_prompt( - self, - request: ReviewRequestDto, - pr_metadata: Dict[str, Any], - rag_context: Optional[Dict[str, Any]] = None - ) -> str: - """Build the appropriate prompt based on previous analysis and RAG context.""" - analysis_type = request.analysisType - has_previous_analysis = bool(request.previousCodeAnalysisIssues) - - if analysis_type is not None and analysis_type == "BRANCH_ANALYSIS": - return PromptBuilder.build_branch_review_prompt_with_branch_issues_data(pr_metadata) - - # Build structured context for Lost-in-the-Middle protection - structured_context = None - if request.changedFiles: - try: - # Prepare file classification and reranking - changed_files = request.changedFiles or [] - classified_files = FileClassifier.classify_files(changed_files) - - # Rerank RAG results based on file priorities - if rag_context and rag_context.get("relevant_code"): - rag_context["relevant_code"] = RagReranker.rerank_by_file_priority( - rag_context["relevant_code"], - classified_files - ) - rag_context["relevant_code"] = RagReranker.deduplicate_by_content( - rag_context["relevant_code"] - ) - rag_context["relevant_code"] = RagReranker.filter_by_relevance_threshold( - rag_context["relevant_code"], - min_score=RAG_MIN_RELEVANCE_SCORE, - min_results=3 - ) - - # Get dynamic token budget based on model - budget = ContextBudget.for_model(request.aiModel) - rag_token_budget = budget.rag_tokens - - # Calculate max chunks based on token budget (roughly 500-800 tokens per chunk) - # Increase from fixed 5 to dynamic based on budget - avg_tokens_per_chunk = 600 - max_rag_chunks = max(5, min(15, rag_token_budget // avg_tokens_per_chunk)) - - # Build structured RAG section with dynamic budget - structured_context = PromptBuilder.build_structured_rag_section( - rag_context, - max_chunks=max_rag_chunks, - token_budget=rag_token_budget - ) - - # Log classification stats - stats = FileClassifier.get_priority_stats(classified_files) - logger.info(f"File classification for Lost-in-Middle protection: {stats}") - logger.info(f"Using token budget for model '{request.aiModel}': {budget.total_tokens} total, {rag_token_budget} for RAG, max_chunks={max_rag_chunks}") - - except Exception as e: - logger.warning(f"Failed to build structured context: {e}, falling back to legacy format") - structured_context = None - - if has_previous_analysis: - prompt = PromptBuilder.build_review_prompt_with_previous_analysis_data( - pr_metadata, rag_context, structured_context - ) - else: - prompt = PromptBuilder.build_first_review_prompt( - pr_metadata, rag_context, structured_context - ) - - # Log full prompt for debugging - prompt_metadata = { - "workspace": request.projectVcsWorkspace, - "repo": request.projectVcsRepoSlug, - "pr_id": request.pullRequestId, - "model": request.aiModel, - "provider": request.aiProvider, - "has_previous_analysis": has_previous_analysis, - "changed_files_count": len(request.changedFiles or []), - "rag_chunks_count": len(rag_context.get("relevant_code", [])) if rag_context else 0, - "has_structured_context": structured_context is not None, - } - - # Log RAG context separately (before full prompt) - if rag_context: - PromptLogger.log_rag_context(rag_context, prompt_metadata, stage="rag_after_reranking") - - # Log structured context - if structured_context: - PromptLogger.log_structured_context(structured_context, prompt_metadata) - - # Log full prompt - PromptLogger.log_prompt(prompt, prompt_metadata, stage="full_prompt") - - return prompt - def _create_mcp_client(self, config: Dict[str, Any]) -> MCPClient: """Create MCP client from configuration.""" try: @@ -1017,4 +428,4 @@ def _emit_event(callback: Optional[Callable[[Dict], None]], event: Dict[str, Any callback(event) except Exception as e: # Don't let callback errors break the processing - print(f"Warning: Event callback failed: {e}") \ No newline at end of file + logger.warning(f"Event callback failed: {e}") \ No newline at end of file diff --git a/python-ecosystem/mcp-client/utils/context_builder.py b/python-ecosystem/mcp-client/utils/context_builder.py index b2f996b9..e414d35e 100644 --- a/python-ecosystem/mcp-client/utils/context_builder.py +++ b/python-ecosystem/mcp-client/utils/context_builder.py @@ -22,6 +22,16 @@ CONTEXT_BUDGET_LOW_PRIORITY_PCT = float(os.environ.get("CONTEXT_BUDGET_LOW_PRIORITY_PCT", "0.20")) CONTEXT_BUDGET_RAG_PCT = float(os.environ.get("CONTEXT_BUDGET_RAG_PCT", "0.10")) +# Validate budget percentages sum to 1.0 +_budget_sum = CONTEXT_BUDGET_HIGH_PRIORITY_PCT + CONTEXT_BUDGET_MEDIUM_PRIORITY_PCT + CONTEXT_BUDGET_LOW_PRIORITY_PCT + CONTEXT_BUDGET_RAG_PCT +if abs(_budget_sum - 1.0) > 0.01: + logger.warning(f"Context budget percentages sum to {_budget_sum}, expected 1.0. Normalizing...") + _factor = 1.0 / _budget_sum + CONTEXT_BUDGET_HIGH_PRIORITY_PCT *= _factor + CONTEXT_BUDGET_MEDIUM_PRIORITY_PCT *= _factor + CONTEXT_BUDGET_LOW_PRIORITY_PCT *= _factor + CONTEXT_BUDGET_RAG_PCT *= _factor + # RAG Cache settings RAG_CACHE_TTL_SECONDS = int(os.environ.get("RAG_CACHE_TTL_SECONDS", "300")) RAG_CACHE_MAX_SIZE = int(os.environ.get("RAG_CACHE_MAX_SIZE", "100")) diff --git a/python-ecosystem/mcp-client/utils/mcp_config.py b/python-ecosystem/mcp-client/utils/mcp_config.py index c173e8c6..854689b7 100644 --- a/python-ecosystem/mcp-client/utils/mcp_config.py +++ b/python-ecosystem/mcp-client/utils/mcp_config.py @@ -28,8 +28,11 @@ def build_config(jar_path: str, jvm_props: Optional[Dict[str, str]] = None, sanitized_value = str(value).replace("\n", " ") jvm_args.append(f"-D{key}={sanitized_value}") - # For debugging, uncomment the next line: - #args = ["-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=*:5007"] + jvm_args + ["-jar", jar_path] + # Enable JVM debugging if MCP_DEBUG_PORT is set + debug_port = os.environ.get("MCP_DEBUG_PORT") + if debug_port: + jvm_args = [f"-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=*:{debug_port}"] + jvm_args + args = jvm_args + ["-jar", jar_path] mcp_servers = { diff --git a/python-ecosystem/mcp-client/utils/prompt_builder.py b/python-ecosystem/mcp-client/utils/prompt_builder.py deleted file mode 100644 index b4045d4c..00000000 --- a/python-ecosystem/mcp-client/utils/prompt_builder.py +++ /dev/null @@ -1,885 +0,0 @@ -from typing import Any, Dict, List, Optional -import json -from model.models import IssueDTO - -# Define valid issue categories -ISSUE_CATEGORIES = """ -Available issue categories (use EXACTLY one of these values): -- SECURITY: Security vulnerabilities, injection risks, authentication issues -- PERFORMANCE: Performance bottlenecks, inefficient algorithms, resource leaks -- CODE_QUALITY: Code smells, maintainability issues, complexity problems -- BUG_RISK: Potential bugs, edge cases, null pointer risks -- STYLE: Code style, formatting, naming conventions -- DOCUMENTATION: Missing or inadequate documentation -- BEST_PRACTICES: Violations of language/framework best practices -- ERROR_HANDLING: Improper exception handling, missing error checks -- TESTING: Test coverage issues, untestable code -- ARCHITECTURE: Design issues, coupling problems, SOLID violations -""" - -# Enhanced line number calculation instructions -LINE_NUMBER_INSTRUCTIONS = """ -⚠️ CRITICAL LINE NUMBER CALCULATION: -The "line" field MUST contain the EXACT line number where the issue occurs in the NEW version of the file. - -HOW TO CALCULATE LINE NUMBERS FROM UNIFIED DIFF: -1. Look at the hunk header: @@ -OLD_START,OLD_COUNT +NEW_START,NEW_COUNT @@ -2. Start counting from NEW_START (the number after the +) -3. For EACH line in the hunk: - - Lines starting with '+' (additions): Count them, they ARE in the new file - - Lines starting with ' ' (context): Count them, they ARE in the new file - - Lines starting with '-' (deletions): DO NOT count them, they are NOT in the new file -4. The issue line = NEW_START + (position in hunk, counting only '+' and ' ' lines) - -EXAMPLE: -@@ -10,5 +10,6 @@ - context line <- Line 10 in new file - context line <- Line 11 in new file --deleted line <- NOT in new file (don't count) -+added line <- Line 12 in new file (issue might be here!) - context line <- Line 13 in new file - -If the issue is on the "added line", report line: "12" (not 14!) - -VALIDATION: Before reporting a line number, verify: -- Is this line actually in the NEW file version? -- Does the line content match what you're describing in the issue? -""" - -# Issue deduplication instructions -ISSUE_DEDUPLICATION_INSTRUCTIONS = """ -⚠️ CRITICAL: AVOID DUPLICATE ISSUES - -Before reporting an issue, check if you've already reported the SAME root cause: - -MERGE THESE INTO ONE ISSUE: -- Multiple instances of the same hardcoded value (e.g., store ID '6' in 3 places) -- Same security vulnerability pattern repeated in different methods -- Same missing validation across multiple endpoints -- Same deprecated API usage in multiple files - -HOW TO REPORT GROUPED ISSUES: -1. Report ONE issue for the root cause -2. In the "reason" field, mention: "Found in X locations: [list files/lines]" -3. Use the FIRST occurrence's line number -4. In suggestedFixDiff, show the fix for ONE location as example - -EXAMPLE - WRONG (duplicate issues): -Issue 1: "Hardcoded store ID '6' in getRewriteUrl()" -Issue 2: "Hardcoded store ID '6' in processUrl()" -Issue 3: "Store ID 6 is hardcoded" - -EXAMPLE - CORRECT (merged into one): -Issue 1: "Hardcoded store ID '6' prevents multi-store compatibility. Found in 3 locations: - - Model/UrlProcessor.php:45 (getRewriteUrl) - - Model/UrlProcessor.php:89 (processUrl) - - Helper/Data.php:23 - Recommended: Use configuration or store manager to get store ID dynamically." -""" - -# Instructions for suggestedFixDiff format -SUGGESTED_FIX_DIFF_FORMAT = """ -📝 SUGGESTED FIX DIFF FORMAT: -When providing suggestedFixDiff, use standard unified diff format: - -``` ---- a/path/to/file.ext -+++ b/path/to/file.ext -@@ -START_LINE,COUNT +START_LINE,COUNT @@ - context line (unchanged) --removed line (starts with minus) -+added line (starts with plus) - context line (unchanged) -``` - -RULES: -1. Include file path headers: `--- a/file` and `+++ b/file` -2. Include hunk header: `@@ -old_start,old_count +new_start,new_count @@` -3. Prefix removed lines with `-` (minus) -4. Prefix added lines with `+` (plus) -5. Prefix context lines with ` ` (single space) -6. Include 1-3 context lines before/after changes -7. Use actual file path from the issue -8. The line numbers in @@ must match the ACTUAL lines in the file - -EXAMPLE: -"suggestedFixDiff": "--- a/src/UserService.java\\n+++ b/src/UserService.java\\n@@ -45,3 +45,4 @@\\n public User findById(Long id) {\\n- return repo.findById(id);\\n+ return repo.findById(id)\\n+ .orElseThrow(() -> new NotFoundException());\\n }" - -DO NOT use markdown code blocks inside the JSON value. -""" - -# Lost-in-the-Middle protection instructions -LOST_IN_MIDDLE_INSTRUCTIONS = """ -⚠️ CRITICAL: LOST-IN-THE-MIDDLE PROTECTION ACTIVE - -The context below is STRUCTURED BY PRIORITY. Follow this analysis order STRICTLY: - -📋 ANALYSIS PRIORITY ORDER (MANDATORY): -1️⃣ HIGH PRIORITY (50% attention): Core business logic, security, auth - analyze FIRST -2️⃣ MEDIUM PRIORITY (25% attention): Dependencies, shared utils, models -3️⃣ LOW PRIORITY (10% attention): Tests, configs - quick scan only -4️⃣ RAG CONTEXT (15% attention): Additional context from codebase - -🎯 FOCUS HIERARCHY: -- Security issues > Architecture problems > Performance > Code quality > Style -- Business impact > Technical details -- Root cause > Symptoms - -🛡️ BLOCK PR IMMEDIATELY IF FOUND: -- SQL Injection / XSS / Command Injection -- Hardcoded secrets/API keys -- Authentication bypass -- Remote Code Execution possibilities -""" - -class PromptBuilder: - @staticmethod - def build_first_review_prompt( - pr_metadata: Dict[str, Any], - rag_context: Dict[str, Any] = None, - structured_context: Optional[str] = None - ) -> str: - print("Building first review prompt") - workspace = pr_metadata.get("workspace", "") - repo = pr_metadata.get("repoSlug", "") - pr_id = pr_metadata.get("pullRequestId", pr_metadata.get("prId", "")) - - # Build RAG context section (legacy format for backward compatibility) - rag_section = "" - if not structured_context and rag_context and rag_context.get("relevant_code"): - rag_section = PromptBuilder._build_legacy_rag_section(rag_context) - - # Use structured context if provided (new Lost-in-Middle protected format) - context_section = "" - if structured_context: - context_section = f""" -{LOST_IN_MIDDLE_INSTRUCTIONS} - -{structured_context} -""" - elif rag_section: - context_section = rag_section - - prompt = f"""You are an expert code reviewer with 15+ years of experience in security, architecture, and code quality. -Workspace: {workspace} -Repository slug: {repo} -Pull Request: {pr_id} - -## MCP Tool Parameters -When calling MCP tools (getPullRequestDiff, getPullRequest, etc.), use these EXACT values: -- workspace: "{workspace}" (owner/organization name only - NOT the full repo path) -- repoSlug: "{repo}" -- pullRequestId: "{pr_id}" - -{context_section}Perform a PRIORITIZED code review: - -🎯 ANALYSIS FOCUS (in order of importance): -1. SECURITY: SQL injection, XSS, auth bypass, hardcoded secrets -2. ARCHITECTURE: Design issues, breaking changes, SOLID violations -3. PERFORMANCE: N+1 queries, memory leaks, inefficient algorithms -4. BUG_RISK: Edge cases, null checks, type mismatches -5. CODE_QUALITY: Maintainability, complexity, code smells - -{ISSUE_CATEGORIES} - -{ISSUE_DEDUPLICATION_INSTRUCTIONS} - -EFFICIENCY INSTRUCTIONS (YOU HAVE LIMITED STEPS - MAX 120): -1. First, retrieve the PR diff using getPullRequestDiff tool -2. Analyze the diff content directly - do NOT fetch each file individually unless absolutely necessary -3. After analysis, produce your JSON response IMMEDIATELY -4. Do NOT make redundant tool calls - each tool call uses one of your limited steps - -You MUST: -1. Retrieve diff using getPullRequestDiff MCP tool (this gives you all changes) -2. Analyze the diff to identify issues -3. STOP making tool calls and produce your final JSON response -4. Assign a category from the list above to EVERY issue - -DO NOT: -1. Fetch files one by one when the diff already shows the changes -2. Make more than 10-15 tool calls total -3. Continue making tool calls indefinitely -4. Report the SAME root cause as multiple separate issues - -{LINE_NUMBER_INSTRUCTIONS} - -{SUGGESTED_FIX_DIFF_FORMAT} - -CRITICAL: Your final response must be ONLY a valid JSON object in this exact format: -{{ - "comment": "Brief summary of the overall code review findings", - "issues": [ - {{ - "severity": "HIGH|MEDIUM|LOW|INFO", - "category": "SECURITY|PERFORMANCE|CODE_QUALITY|BUG_RISK|STYLE|DOCUMENTATION|BEST_PRACTICES|ERROR_HANDLING|TESTING|ARCHITECTURE", - "file": "file-path", - "line": "line-number-in-new-file", - "reason": "Detailed explanation of the issue", - "suggestedFixDescription": "Clear description of how to fix the issue", - "suggestedFixDiff": "Unified diff showing exact code changes (MUST follow SUGGESTED_FIX_DIFF_FORMAT above)", - "isResolved": false - }} - ] -}} - -IMPORTANT SCHEMA RULES: -- The "issues" field MUST be a JSON array [], NOT an object with numeric keys -- Do NOT include any "id" field in issues - it will be assigned by the system -- Each issue MUST have: severity, category, file, line, reason, isResolved -- REQUIRED FOR ALL ISSUES: Include "suggestedFixDescription" AND "suggestedFixDiff" with actual code fix in unified diff format -- The suggestedFixDiff must show the exact code change to fix the issue - -If no issues are found, return: -{{ - "comment": "Code review completed successfully with no issues found", - "issues": [] -}} - -Use the reportGenerator MCP tool if available to help structure this response. Do NOT include any markdown formatting, explanatory text, or other content - only the JSON object. -""" - return prompt - - @staticmethod - def build_review_prompt_with_previous_analysis_data( - pr_metadata: Dict[str, Any], - rag_context: Dict[str, Any] = None, - structured_context: Optional[str] = None - ) -> str: - print("Building review prompt with previous analysis data") - workspace = pr_metadata.get("workspace", "") - repo = pr_metadata.get("repoSlug", "") - pr_id = pr_metadata.get("pullRequestId", pr_metadata.get("prId", "")) - # 🆕 Get and format previous issues data - previous_issues: List[Dict[str, Any]] = pr_metadata.get("previousCodeAnalysisIssues", []) - - # We need a clean JSON string of the previous issues to inject into the prompt - previous_issues_json = json.dumps(previous_issues, indent=2, default=str) - - # Build RAG context section (legacy format for backward compatibility) - rag_section = "" - if not structured_context and rag_context and rag_context.get("relevant_code"): - rag_section = PromptBuilder._build_legacy_rag_section(rag_context) - - # Use structured context if provided (new Lost-in-Middle protected format) - context_section = "" - if structured_context: - context_section = f""" -{LOST_IN_MIDDLE_INSTRUCTIONS} - -{structured_context} -""" - elif rag_section: - context_section = rag_section - - prompt = f"""You are an expert code reviewer with 15+ years of experience performing a review on a subsequent version of a pull request. -Workspace: {workspace} -Repository slug: {repo} -Pull Request: {pr_id} - -## MCP Tool Parameters -When calling MCP tools (getPullRequestDiff, getPullRequest, etc.), use these EXACT values: -- workspace: "{workspace}" (owner/organization name only - NOT the full repo path) -- repoSlug: "{repo}" -- pullRequestId: "{pr_id}" - -{context_section}CRITICAL INSTRUCTIONS FOR RECURRING REVIEW: -1. The **Previous Analysis Issues** are provided below. Use this information to determine if any of these issues have been **resolved in the current diff**. -2. If a previously reported issue is **fixed** in the new code, Report it again with the status “resolved”. -3. If a previously reported issue **persists** (i.e., the relevant code wasn't changed or the fix was incomplete), you **MUST** report it again in the current review's 'issues' list. -4. Always review the **entire current diff** for **new** issues as well. - ---- PREVIOUS ANALYSIS ISSUES --- -{previous_issues_json} ---- END OF PREVIOUS ISSUES --- - -Perform a code review considering: -1. Code quality and best practices -2. Potential bugs and edge cases -3. Performance and maintainability -4. Security issues -5. Suggest concrete fixes in the form of DIFF Patch if applicable, and put it in suggested fix - -{ISSUE_CATEGORIES} - -{ISSUE_DEDUPLICATION_INSTRUCTIONS} - -EFFICIENCY INSTRUCTIONS (YOU HAVE LIMITED STEPS - MAX 120): -1. First, retrieve the PR diff using getPullRequestDiff tool -2. Analyze the diff content directly - do NOT fetch each file individually unless absolutely necessary -3. After analysis, produce your JSON response IMMEDIATELY -4. Do NOT make redundant tool calls - each tool call uses one of your limited steps - -You MUST: -1. Retrieve diff using getPullRequestDiff MCP tool (this gives you all changes) -2. Analyze the diff to identify issues and check previous issues -3. STOP making tool calls and produce your final JSON response -4. Assign a category from the list above to EVERY issue - -DO NOT: -1. Fetch files one by one when the diff already shows the changes -2. Make more than 10-15 tool calls total -3. Continue making tool calls indefinitely -4. Report the SAME root cause as multiple separate issues - -CRITICAL INSTRUCTION FOR LARGE PRs: -Report ALL UNIQUE issues found. Merge similar issues (same root cause) into one. - -{LINE_NUMBER_INSTRUCTIONS} - -{SUGGESTED_FIX_DIFF_FORMAT} - -CRITICAL: Your final response must be ONLY a valid JSON object in this exact format: -{{ - "comment": "Brief summary of the overall code review findings", - "issues": [ - {{ - "severity": "HIGH|MEDIUM|LOW|INFO", - "category": "SECURITY|PERFORMANCE|CODE_QUALITY|BUG_RISK|STYLE|DOCUMENTATION|BEST_PRACTICES|ERROR_HANDLING|TESTING|ARCHITECTURE", - "file": "file-path", - "line": "line-number-in-new-file", - "reason": "Detailed explanation of the issue", - "suggestedFixDescription": "Clear description of how to fix the issue", - "suggestedFixDiff": "Unified diff showing exact code changes (MUST follow SUGGESTED_FIX_DIFF_FORMAT above)", - "isResolved": false|true - }} - ] -}} - -IMPORTANT SCHEMA RULES: -- The "issues" field MUST be a JSON array [], NOT an object with numeric keys -- Do NOT include any "id" field in issues - it will be assigned by the system -- Each issue MUST have: severity, category, file, line, reason, isResolved -- REQUIRED FOR ALL ISSUES: Include "suggestedFixDescription" AND "suggestedFixDiff" with actual code fix in unified diff format -- The suggestedFixDiff must show the exact code change to fix the issue - -If no issues are found, return: -{{ - "comment": "Code review completed successfully with no issues found", - "issues": [] -}} - -If token limit exceeded, STOP IMMEDIATELY AND return: -{{ - "comment": "The code review process was not completed successfully due to exceeding the allowable number of tokens (fileDiff).", - "issues": [ - {{ - "severity": "LOW", - "category": "CODE_QUALITY", - "file": "", - "line": "0", - "reason": "The code review process was not completed successfully due to exceeding the allowable number of tokens (fileDiff).", - "suggestedFixDescription": "Increase the allowed number of tokens or choose a model with a larger context." - }} - ] -}} - -Use the reportGenerator MCP tool if available to help structure this response. Do NOT include any markdown formatting, explanatory text, or other content - only the JSON object. -""" - return prompt - - @staticmethod - def build_branch_review_prompt_with_branch_issues_data(pr_metadata: Dict[str, Any]) -> str: - print("Building branch review prompt with branch issues data") - workspace = pr_metadata.get("workspace", "") - repo = pr_metadata.get("repoSlug", "") - commit_hash = pr_metadata.get("commitHash", "") - branch = pr_metadata.get("branch", "") - # Get and format previous issues data - previous_issues: List[Dict[str, Any]] = pr_metadata.get("previousCodeAnalysisIssues", []) - - # We need a clean JSON string of the previous issues to inject into the prompt - previous_issues_json = json.dumps(previous_issues, indent=2, default=str) - prompt = f"""You are an expert code reviewer performing a branch reconciliation review after a PR merge. -Workspace: {workspace} -Repository slug: {repo} -Commit Hash: {commit_hash} -Branch: {branch} - -## MCP Tool Parameters -When calling MCP tools (getBranchFileContent, etc.), use these EXACT values: -- workspace: "{workspace}" (owner/organization name only - NOT the full repo path) -- repoSlug: "{repo}" - -CRITICAL INSTRUCTIONS FOR BRANCH RECONCILIATION: -1. The **Previous Analysis Issues** are provided below - these are issues that existed on the branch BEFORE this PR. -2. Your task is to determine if any of these pre-existing issues have been **resolved based on the current content of the file(s) on the branch**. -3. For EACH issue in the previous analysis, you MUST include it in your response with: - - "issueId": "" (copy the 'id' field from the previous issue) - - "isResolved": true (if the issue is fixed by this PR's changes) - - "isResolved": false (if the issue still persists) - - "reason": "Explanation of why it's resolved or still present" -4. DO NOT report new issues - this is ONLY for checking resolution status of existing issues. -5. You MUST retrieve the current file content using MCP tools to compare against the previous issues (e.g. via getBranchFileContent tool). -6. If you see similar errors, you MUST group them together. Set the duplicate to isResolved: true, and leave one of the errors in its original status. - -⚠️ CRITICAL FOR RESOLVED ISSUES: -When an issue is RESOLVED (isResolved: true), you MUST: -1. Provide a clear "reason" field explaining HOW the issue was fixed (e.g., "The null check was added on line 45", "The SQL injection vulnerability was fixed by using parameterized queries") -2. This "reason" will be stored as the resolution description for historical tracking -3. Be specific about what code change fixed the issue - -⚠️ CRITICAL FOR PERSISTING (UNRESOLVED) ISSUES: -When an issue PERSISTS (isResolved: false), you MUST: -1. COPY the "suggestedFixDiff" field EXACTLY from the original previous issue - DO NOT omit it -2. COPY the "suggestedFixDescription" field EXACTLY from the original previous issue -3. Keep the same severity and category -4. Only update the "reason" field to explain why it still persists -5. Update the "line" field if the line number changed due to other code changes - -Example for PERSISTING issue: -Previous issue had: {{"id": "123", "suggestedFixDiff": "--- a/file.py\\n+++ b/file.py\\n..."}} -Your response MUST include: {{"issueId": "123", "isResolved": false, "suggestedFixDiff": "--- a/file.py\\n+++ b/file.py\\n...", ...}} - - ---- PREVIOUS ANALYSIS ISSUES --- -{previous_issues_json} ---- END OF PREVIOUS ISSUES --- - -EFFICIENCY INSTRUCTIONS (YOU HAVE LIMITED STEPS - MAX 120): -1. For each file with issues, retrieve content using getBranchFileContent -2. Analyze content to determine if issues are resolved -3. After checking all relevant files, produce your JSON response IMMEDIATELY -4. Do NOT make redundant tool calls - each tool call uses one of your limited steps - -You MUST: -1. Retrieve file content for files with issues using getBranchFileContent MCP tool -2. For each previous issue, check if the current file content shows it resolved -3. STOP making tool calls and produce your final JSON response once you have analyzed all relevant files - -DO NOT: -1. Report new issues - focus ONLY on the provided previous issues -2. Make more than necessary tool calls - be efficient -3. Continue making tool calls indefinitely - -IMPORTANT LINE NUMBER INSTRUCTIONS: -The "line" field MUST contain the line number in the current version of the file on the branch. -If you retrieve the full source file content via getBranchFileContent, use the line number as it appears in that file. - -CRITICAL: Your final response must be ONLY a valid JSON object in this exact format: -{{ - "comment": "Summary of branch reconciliation - how many issues were resolved vs persisting", - "issues": [ - {{ - "issueId": "", - "severity": "HIGH|MEDIUM|LOW|INFO", - "category": "SECURITY|PERFORMANCE|CODE_QUALITY|BUG_RISK|STYLE|DOCUMENTATION|BEST_PRACTICES|ERROR_HANDLING|TESTING|ARCHITECTURE", - "file": "file-path", - "line": "line-number-in-current-file", - "reason": "For RESOLVED: Explain HOW the issue was fixed (e.g., 'Added null check on line 45'). For UNRESOLVED: Explain why it still persists.", - "suggestedFixDescription": "Clear description of how to fix the issue (copy from original for unresolved issues)", - "suggestedFixDiff": "Unified diff showing exact code changes (copy from original for unresolved issues)", - "isResolved": true - }} - ] -}} - -IMPORTANT: -- The "issues" field MUST be a JSON array [], NOT an object with numeric keys. -- You MUST include ALL previous issues in your response -- Each issue MUST have the "issueId" field matching the original issue ID -- Each issue MUST have "isResolved" as either true or false -- Each issue MUST have a "category" field from the allowed list -- FOR UNRESOLVED ISSUES: COPY "suggestedFixDescription" AND "suggestedFixDiff" from the original issue - DO NOT OMIT THEM -- The suggestedFixDiff is MANDATORY for unresolved issues - copy it verbatim from the previous issue data - -Use the reportGenerator MCP tool if available to help structure this response. Do NOT include any markdown formatting, explanatory text, or other content - only the JSON object. -""" - return prompt - - - @staticmethod - def get_additional_instructions() -> str: - """ - Get additional instructions for the MCP agent focusing on structured JSON output. - Note: Curly braces must be doubled to escape them for LangChain's ChatPromptTemplate. - - Returns: - String with additional instructions for the agent - """ - return ( - "CRITICAL INSTRUCTIONS:\n" - "1. You have a LIMITED number of steps (max 120). Plan efficiently - do NOT make unnecessary tool calls.\n" - "2. After retrieving the diff, analyze it and produce your final JSON response IMMEDIATELY.\n" - "3. Do NOT retrieve every file individually - use the diff output to identify issues.\n" - "4. Your FINAL response must be ONLY a valid JSON object with 'comment' and 'issues' fields.\n" - "5. The 'issues' field MUST be a JSON array [], NOT an object with numeric string keys.\n" - "6. If you cannot complete the review within your step limit, output your partial findings in JSON format.\n" - "7. Do NOT include any markdown formatting, explanations, or other text - only the JSON structure.\n" - "8. STOP making tool calls and produce output once you have enough information to analyze.\n" - "9. If you encounter errors with MCP tools, proceed with available information and note limitations in the comment field.\n" - "10. FOLLOW PRIORITY ORDER: Analyze HIGH priority sections FIRST, then MEDIUM, then LOW.\n" - "11. For LARGE PRs: Focus 60% attention on HIGH priority, 25% on MEDIUM, 15% on LOW/RAG." - ) - - @staticmethod - def _build_legacy_rag_section(rag_context: Dict[str, Any]) -> str: - """Build legacy RAG section for backward compatibility.""" - rag_section = "\n--- RELEVANT CODE CONTEXT FROM CODEBASE ---\n" - rag_section += "The following code snippets from the repository are semantically relevant to this PR:\n\n" - for idx, chunk in enumerate(rag_context.get("relevant_code", [])[:5], 1): - rag_section += f"Context {idx} (from {chunk.get('metadata', {}).get('path', 'unknown')}):\n" - rag_section += f"{chunk.get('text', '')}\n\n" - rag_section += "--- END OF RELEVANT CONTEXT ---\n\n" - return rag_section - - @staticmethod - def build_structured_rag_section( - rag_context: Dict[str, Any], - max_chunks: int = 5, - token_budget: int = 4000 - ) -> str: - """ - Build a structured RAG section with priority markers. - - Args: - rag_context: RAG query results - max_chunks: Maximum number of chunks to include - token_budget: Approximate token budget for RAG section - - Returns: - Formatted RAG section string - """ - if not rag_context or not rag_context.get("relevant_code"): - return "" - - relevant_code = rag_context.get("relevant_code", []) - related_files = rag_context.get("related_files", []) - - section_parts = [] - section_parts.append("=== RAG CONTEXT: Additional Relevant Code (5% attention) ===") - section_parts.append(f"Related files discovered: {len(related_files)}") - section_parts.append("") - - current_tokens = 0 - tokens_per_char = 0.25 - - for idx, chunk in enumerate(relevant_code[:max_chunks], 1): - chunk_text = chunk.get("text", "") - chunk_tokens = int(len(chunk_text) * tokens_per_char) - - if current_tokens + chunk_tokens > token_budget: - section_parts.append(f"[Remaining {len(relevant_code) - idx + 1} chunks omitted for token budget]") - break - - chunk_path = chunk.get("metadata", {}).get("path", "unknown") - chunk_score = chunk.get("score", 0) - - section_parts.append(f"### RAG Chunk {idx}: {chunk_path}") - section_parts.append(f"Relevance: {chunk_score:.3f}") - section_parts.append("```") - section_parts.append(chunk_text) - section_parts.append("```") - section_parts.append("") - - current_tokens += chunk_tokens - - section_parts.append("=== END RAG CONTEXT ===") - return "\n".join(section_parts) - - @staticmethod - def build_direct_first_review_prompt( - pr_metadata: Dict[str, Any], - diff_content: str, - rag_context: Dict[str, Any] = None, - structured_context: Optional[str] = None - ) -> str: - """ - Build prompt for review with embedded diff - first review. - - The diff is already embedded in the prompt. - Agent still has access to other MCP tools (getFile, getComments, etc.) - but should NOT call getPullRequestDiff. - """ - workspace = pr_metadata.get("workspace", "") - repo = pr_metadata.get("repoSlug", "") - pr_id = pr_metadata.get("pullRequestId", pr_metadata.get("prId", "")) - - # Build context section - context_section = "" - if structured_context: - context_section = f""" -{LOST_IN_MIDDLE_INSTRUCTIONS} - -{structured_context} -""" - elif rag_context and rag_context.get("relevant_code"): - context_section = PromptBuilder._build_legacy_rag_section(rag_context) - - prompt = f"""You are an expert code reviewer with 15+ years of experience in security, architecture, and code quality. -Workspace: {workspace} -Repository slug: {repo} -Pull Request: {pr_id} - -{context_section} - -=== PR DIFF (ALREADY PROVIDED - DO NOT CALL getPullRequestDiff) === -IMPORTANT: The diff is embedded below. Do NOT call getPullRequestDiff tool. -You may use other MCP tools (getBranchFileContent, getPullRequestComments, etc.) if needed. - -{diff_content} - -=== END OF DIFF === - -Perform a PRIORITIZED code review of the diff above: - -🎯 ANALYSIS FOCUS (in order of importance): -1. SECURITY: SQL injection, XSS, auth bypass, hardcoded secrets -2. ARCHITECTURE: Design issues, breaking changes, SOLID violations -3. PERFORMANCE: N+1 queries, memory leaks, inefficient algorithms -4. BUG_RISK: Edge cases, null checks, type mismatches -5. CODE_QUALITY: Maintainability, complexity, code smells - -{ISSUE_CATEGORIES} - -{ISSUE_DEDUPLICATION_INSTRUCTIONS} - -{LINE_NUMBER_INSTRUCTIONS} - -{SUGGESTED_FIX_DIFF_FORMAT} - -CRITICAL: Report ALL UNIQUE issues found. Merge similar issues (same root cause) into one. - -Your response must be ONLY a valid JSON object in this exact format: -{{ - "comment": "Brief summary of the overall code review findings", - "issues": [ - {{ - "severity": "HIGH|MEDIUM|LOW|INFO", - "category": "SECURITY|PERFORMANCE|CODE_QUALITY|BUG_RISK|STYLE|DOCUMENTATION|BEST_PRACTICES|ERROR_HANDLING|TESTING|ARCHITECTURE", - "file": "file-path", - "line": "line-number-in-new-file", - "reason": "Detailed explanation of the issue", - "suggestedFixDescription": "Clear description of how to fix the issue", - "suggestedFixDiff": "Unified diff showing exact code changes (MUST follow SUGGESTED_FIX_DIFF_FORMAT above)", - "isResolved": false - }} - ] -}} - -IMPORTANT: REQUIRED FOR ALL ISSUES - Include "suggestedFixDescription" AND "suggestedFixDiff" with actual code fix in unified diff format. - -If no issues are found, return: -{{ - "comment": "Code review completed successfully with no issues found", - "issues": [] -}} - -Do NOT include any markdown formatting, explanatory text, or other content - only the JSON object. -""" - return prompt - - @staticmethod - def build_direct_review_prompt_with_previous_analysis( - pr_metadata: Dict[str, Any], - diff_content: str, - rag_context: Dict[str, Any] = None, - structured_context: Optional[str] = None - ) -> str: - """ - Build prompt for direct review mode with previous analysis data. - """ - workspace = pr_metadata.get("workspace", "") - repo = pr_metadata.get("repoSlug", "") - pr_id = pr_metadata.get("pullRequestId", pr_metadata.get("prId", "")) - previous_issues: List[Dict[str, Any]] = pr_metadata.get("previousCodeAnalysisIssues", []) - previous_issues_json = json.dumps(previous_issues, indent=2, default=str) - - # Build context section - context_section = "" - if structured_context: - context_section = f""" -{LOST_IN_MIDDLE_INSTRUCTIONS} - -{structured_context} -""" - elif rag_context and rag_context.get("relevant_code"): - context_section = PromptBuilder._build_legacy_rag_section(rag_context) - - prompt = f"""You are an expert code reviewer with 15+ years of experience in security, architecture, and code quality. -Workspace: {workspace} -Repository slug: {repo} -Pull Request: {pr_id} - -{context_section} - -=== PR DIFF (ALREADY PROVIDED - DO NOT CALL getPullRequestDiff) === -IMPORTANT: The diff is embedded below. Do NOT call getPullRequestDiff tool. -You may use other MCP tools (getBranchFileContent, getPullRequestComments, etc.) if needed. - -{diff_content} - -=== END OF DIFF === - -=== PREVIOUS ANALYSIS ISSUES === -The following issues were found in a previous review. Check if they are still present or have been resolved: -{previous_issues_json} -=== END OF PREVIOUS ISSUES === - -Perform a PRIORITIZED code review of the diff above: - -🎯 TASKS: -1. Check if each previous issue is still present in the code -2. Mark resolved issues with "isResolved": true -3. Find NEW issues introduced in this PR version -4. Prioritize by security > architecture > performance > quality - -{ISSUE_CATEGORIES} - -IMPORTANT LINE NUMBER INSTRUCTIONS: -For existing issues, update line numbers if code moved. -For new issues, use line numbers from the NEW version of files. - -{SUGGESTED_FIX_DIFF_FORMAT} - -Your response must be ONLY a valid JSON object: -{{ - "comment": "Summary of changes since last review", - "issues": [ - {{ - "severity": "HIGH|MEDIUM|LOW|INFO", - "category": "SECURITY|PERFORMANCE|...", - "file": "file-path", - "line": "line-number-in-new-file", - "reason": "Explanation", - "suggestedFixDescription": "Clear description of how to fix the issue", - "suggestedFixDiff": "Unified diff showing exact code changes (MUST follow SUGGESTED_FIX_DIFF_FORMAT above)", - "isResolved": false|true - }} - ] -}} - -IMPORTANT: REQUIRED FOR ALL ISSUES - Include "suggestedFixDescription" AND "suggestedFixDiff" with actual code fix in unified diff format. - -Do NOT include any markdown formatting - only the JSON object. -""" - return prompt - - @staticmethod - def build_incremental_review_prompt( - pr_metadata: Dict[str, Any], - delta_diff_content: str, - full_diff_content: str, - rag_context: Dict[str, Any] = None, - structured_context: Optional[str] = None - ) -> str: - """ - Build prompt for INCREMENTAL analysis mode. - - This is used when re-reviewing a PR after new commits have been pushed. - The delta_diff contains only changes since the last analyzed commit, - while full_diff provides the complete PR diff for reference. - - Focus is on: - 1. Reviewing new/changed code in delta_diff - 2. Checking if previous issues are resolved - 3. Finding new issues introduced since last review - """ - print("Building INCREMENTAL review prompt with delta diff") - workspace = pr_metadata.get("workspace", "") - repo = pr_metadata.get("repoSlug", "") - pr_id = pr_metadata.get("pullRequestId", pr_metadata.get("prId", "")) - previous_commit = pr_metadata.get("previousCommitHash", "") - current_commit = pr_metadata.get("currentCommitHash", "") - previous_issues: List[Dict[str, Any]] = pr_metadata.get("previousCodeAnalysisIssues", []) - previous_issues_json = json.dumps(previous_issues, indent=2, default=str) - - # Build context section - context_section = "" - if structured_context: - context_section = f""" -{LOST_IN_MIDDLE_INSTRUCTIONS} - -{structured_context} -""" - elif rag_context and rag_context.get("relevant_code"): - context_section = PromptBuilder._build_legacy_rag_section(rag_context) - - prompt = f"""You are an expert code reviewer performing an INCREMENTAL review of changes since the last analysis. -Workspace: {workspace} -Repository slug: {repo} -Pull Request: {pr_id} - -## INCREMENTAL REVIEW MODE -This is a RE-REVIEW after new commits were pushed to the PR. -- Previous analyzed commit: {previous_commit} -- Current commit: {current_commit} - -{context_section} - -=== DELTA DIFF (CHANGES SINCE LAST REVIEW - PRIMARY FOCUS) === -IMPORTANT: This diff shows ONLY the changes made since the last analyzed commit. -Focus your review primarily on this delta diff as it contains the new code to review. - -{delta_diff_content} - -=== END OF DELTA DIFF === - -=== PREVIOUS ANALYSIS ISSUES === -These issues were found in the previous review iteration. -Check if each one has been RESOLVED in the new commits (delta diff): -{previous_issues_json} -=== END OF PREVIOUS ISSUES === - -## INCREMENTAL REVIEW TASKS (in order of priority): - -1. **DELTA DIFF ANALYSIS (80% attention)**: - - Focus on reviewing the DELTA DIFF (changes since last commit) - - Find NEW issues introduced in these changes - - These are the most important findings as they represent untested code - -2. **PREVIOUS ISSUES RESOLUTION CHECK (15% attention)**: - - Check each previous issue against the delta diff - - If the problematic code was modified/fixed in delta → mark "isResolved": true - - If the code is unchanged in delta → issue persists, report it again with "isResolved": false - - UPDATE line numbers if code has moved - -3. **CONTEXT VERIFICATION (5% attention)**: - - Use full PR diff only when needed to understand delta changes ( retrieve it via MCP tools ONLY if necessary ) - - Do NOT re-review code that hasn't changed - -{ISSUE_CATEGORIES} - -IMPORTANT LINE NUMBER INSTRUCTIONS: -The "line" field MUST contain the line number in the NEW version of the file (after changes). -For issues found in delta diff, calculate line numbers from the delta hunk headers. -For persisting issues, update line numbers if the code has moved. - -{SUGGESTED_FIX_DIFF_FORMAT} - -CRITICAL: Report ALL issues found in delta diff. Do not group them or omit them for brevity. - -Your response must be ONLY a valid JSON object in this exact format: -{{ - "comment": "Summary of incremental review: X new issues found in delta, Y previous issues resolved, Z issues persist", - "issues": [ - {{ - "issueId": "", - "severity": "HIGH|MEDIUM|LOW|INFO", - "category": "SECURITY|PERFORMANCE|CODE_QUALITY|BUG_RISK|STYLE|DOCUMENTATION|BEST_PRACTICES|ERROR_HANDLING|TESTING|ARCHITECTURE", - "file": "file-path", - "line": "line-number-in-new-file", - "reason": "Detailed explanation of the issue", - "suggestedFixDescription": "Clear description of how to fix the issue", - "suggestedFixDiff": "Unified diff showing exact code changes (MUST follow SUGGESTED_FIX_DIFF_FORMAT above)", - "isResolved": false|true - }} - ] -}} - -IMPORTANT SCHEMA RULES: -- The "issues" field MUST be a JSON array [], NOT an object with numeric keys -- Each issue MUST have: severity, category, file, line, reason, isResolved -- For resolved previous issues, still include them with "isResolved": true -- For new issues from delta diff, set "isResolved": false -- REQUIRED FOR ALL UNRESOLVED ISSUES: Include "suggestedFixDescription" AND "suggestedFixDiff" - -If no issues are found, return: -{{ - "comment": "Incremental review completed: All previous issues resolved, no new issues found", - "issues": [] -}} - -Do NOT include any markdown formatting, explanatory text, or other content - only the JSON object. -""" - return prompt \ No newline at end of file diff --git a/python-ecosystem/mcp-client/utils/prompts/prompt_builder.py b/python-ecosystem/mcp-client/utils/prompts/prompt_builder.py new file mode 100644 index 00000000..b3cad5b1 --- /dev/null +++ b/python-ecosystem/mcp-client/utils/prompts/prompt_builder.py @@ -0,0 +1,171 @@ +from typing import Any, Dict, List, Optional +import json +from model.models import IssueDTO +from utils.prompts.prompt_constants import ( + ADDITIONAL_INSTRUCTIONS, + BRANCH_REVIEW_PROMPT_TEMPLATE, + STAGE_0_PLANNING_PROMPT_TEMPLATE, + STAGE_1_BATCH_PROMPT_TEMPLATE, + STAGE_2_CROSS_FILE_PROMPT_TEMPLATE, + STAGE_3_AGGREGATION_PROMPT_TEMPLATE +) + +class PromptBuilder: + @staticmethod + def build_branch_review_prompt_with_branch_issues_data(pr_metadata: Dict[str, Any]) -> str: + print("Building branch review prompt with branch issues data") + workspace = pr_metadata.get("workspace", "") + repo = pr_metadata.get("repoSlug", "") + commit_hash = pr_metadata.get("commitHash", "") + branch = pr_metadata.get("branch", "") + # Get and format previous issues data + previous_issues: List[Dict[str, Any]] = pr_metadata.get("previousCodeAnalysisIssues", []) + + # We need a clean JSON string of the previous issues to inject into the prompt + previous_issues_json = json.dumps(previous_issues, indent=2, default=str) + + return BRANCH_REVIEW_PROMPT_TEMPLATE.format( + workspace=workspace, + repo=repo, + commit_hash=commit_hash, + branch=branch, + previous_issues_json=previous_issues_json + ) + + @staticmethod + def get_additional_instructions() -> str: + """ + Get additional instructions for the MCP agent focusing on structured JSON output. + Returns: + String with additional instructions for the agent + """ + return ADDITIONAL_INSTRUCTIONS + + @staticmethod + def build_stage_0_planning_prompt( + repo_slug: str, + pr_id: str, + pr_title: str, + author: str, + branch_name: str, + target_branch: str, + commit_hash: str, + changed_files_json: str + ) -> str: + """ + Build prompt for Stage 0: Planning & Prioritization. + """ + return STAGE_0_PLANNING_PROMPT_TEMPLATE.format( + repo_slug=repo_slug, + pr_id=pr_id, + pr_title=pr_title, + author=author, + branch_name=branch_name, + target_branch=target_branch, + commit_hash=commit_hash, + changed_files_json=changed_files_json + ) + + @staticmethod + def build_stage_1_batch_prompt( + files: List[Dict[str, str]], # List of {path, diff, type, old_code, focus_areas} + priority: str, + project_rules: str = "", + rag_context: str = "", + is_incremental: bool = False, + previous_issues: str = "" + ) -> str: + """ + Build prompt for Stage 1: Batch File Review. + In incremental mode, includes previous issues context and focuses on delta changes. + """ + files_context = "" + for i, f in enumerate(files): + diff_label = "Delta Diff (NEW CHANGES ONLY)" if is_incremental else "Diff" + files_context += f""" +--- +FILE #{i+1}: {f['path']} +Type: {f.get('type', 'MODIFIED')} +Focus Areas: {', '.join(f.get('focus_areas', []))} +Context: +{f.get('old_code', '')} + +{diff_label}: +{f.get('diff', '')} +--- +""" + + # Add incremental mode instructions if applicable + incremental_instructions = "" + if is_incremental: + incremental_instructions = """ +## INCREMENTAL REVIEW MODE +This is a follow-up review after the PR was updated with new commits. +The diff above shows ONLY the changes since the last review - focus on these NEW changes. +For any previous issues listed below, check if they are RESOLVED in the new changes. +""" + + return STAGE_1_BATCH_PROMPT_TEMPLATE.format( + project_rules=project_rules, + priority=priority, + files_context=files_context, + rag_context=rag_context or "(No additional codebase context available)", + incremental_instructions=incremental_instructions, + previous_issues=previous_issues + ) + + @staticmethod + def build_stage_2_cross_file_prompt( + repo_slug: str, + pr_title: str, + commit_hash: str, + stage_1_findings_json: str, + architecture_context: str, + migrations: str, + cross_file_concerns: List[str] + ) -> str: + """ + Build prompt for Stage 2: Cross-File & Architectural Review. + """ + concerns_text = "\n".join([f"- {c}" for c in cross_file_concerns]) + + return STAGE_2_CROSS_FILE_PROMPT_TEMPLATE.format( + repo_slug=repo_slug, + pr_title=pr_title, + commit_hash=commit_hash, + concerns_text=concerns_text, + stage_1_findings_json=stage_1_findings_json, + architecture_context=architecture_context, + migrations=migrations + ) + + @staticmethod + def build_stage_3_aggregation_prompt( + repo_slug: str, + pr_id: str, + author: str, + pr_title: str, + total_files: int, + additions: int, + deletions: int, + stage_0_plan: str, + stage_1_issues_json: str, + stage_2_findings_json: str, + recommendation: str + ) -> str: + """ + Build prompt for Stage 3: Aggregation & Final Report. + """ + return STAGE_3_AGGREGATION_PROMPT_TEMPLATE.format( + repo_slug=repo_slug, + pr_id=pr_id, + author=author, + pr_title=pr_title, + total_files=total_files, + additions=additions, + deletions=deletions, + stage_0_plan=stage_0_plan, + stage_1_issues_json=stage_1_issues_json, + stage_2_findings_json=stage_2_findings_json, + recommendation=recommendation + ) \ No newline at end of file diff --git a/python-ecosystem/mcp-client/utils/prompts/prompt_constants.py b/python-ecosystem/mcp-client/utils/prompts/prompt_constants.py new file mode 100644 index 00000000..ec9f98ec --- /dev/null +++ b/python-ecosystem/mcp-client/utils/prompts/prompt_constants.py @@ -0,0 +1,546 @@ + +# Valid issue categories +ISSUE_CATEGORIES = """ +Available issue categories (use EXACTLY one of these values): +- SECURITY: Security vulnerabilities, injection risks, authentication issues +- PERFORMANCE: Performance bottlenecks, inefficient algorithms, resource leaks +- CODE_QUALITY: Code smells, maintainability issues, complexity problems +- BUG_RISK: Potential bugs, edge cases, null pointer risks +- STYLE: Code style, formatting, naming conventions +- DOCUMENTATION: Missing or inadequate documentation +- BEST_PRACTICES: Violations of language/framework best practices +- ERROR_HANDLING: Improper exception handling, missing error checks +- TESTING: Test coverage issues, untestable code +- ARCHITECTURE: Design issues, coupling problems, SOLID violations +""" + +# Enhanced line number calculation instructions +LINE_NUMBER_INSTRUCTIONS = """ +⚠️ CRITICAL LINE NUMBER CALCULATION: +The "line" field MUST contain the EXACT line number where the issue occurs in the NEW version of the file. + +HOW TO CALCULATE LINE NUMBERS FROM UNIFIED DIFF: +1. Look at the hunk header: @@ -OLD_START,OLD_COUNT +NEW_START,NEW_COUNT @@ +2. Start counting from NEW_START (the number after the +) +3. For EACH line in the hunk: + - Lines starting with '+' (additions): Count them, they ARE in the new file + - Lines starting with ' ' (context): Count them, they ARE in the new file + - Lines starting with '-' (deletions): DO NOT count them, they are NOT in the new file +4. The issue line = NEW_START + (position in hunk, counting only '+' and ' ' lines) + +EXAMPLE: +@@ -10,5 +10,6 @@ + context line <- Line 10 in new file + context line <- Line 11 in new file +-deleted line <- NOT in new file (don't count) ++added line <- Line 12 in new file (issue might be here!) + context line <- Line 13 in new file + +If the issue is on the "added line", report line: "12" (not 14!) + +VALIDATION: Before reporting a line number, verify: +- Is this line actually in the NEW file version? +- Does the line content match what you're describing in the issue? +""" + +# Issue deduplication instructions +ISSUE_DEDUPLICATION_INSTRUCTIONS = """ +⚠️ CRITICAL: AVOID DUPLICATE ISSUES + +Before reporting an issue, check if you've already reported the SAME root cause: + +MERGE THESE INTO ONE ISSUE: +- Multiple instances of the same hardcoded value (e.g., store ID '6' in 3 places) +- Same security vulnerability pattern repeated in different methods +- Same missing validation across multiple endpoints +- Same deprecated API usage in multiple files + +HOW TO REPORT GROUPED ISSUES: +1. Report ONE issue for the root cause +2. In the "reason" field, mention: "Found in X locations: [list files/lines]" +3. Use the FIRST occurrence's line number +4. In suggestedFixDiff, show the fix for ONE location as example + +EXAMPLE - WRONG (duplicate issues): +Issue 1: "Hardcoded store ID '6' in getRewriteUrl()" +Issue 2: "Hardcoded store ID '6' in processUrl()" +Issue 3: "Store ID 6 is hardcoded" + +EXAMPLE - CORRECT (merged into one): +Issue 1: "Hardcoded store ID '6' prevents multi-store compatibility. Found in 3 locations: + - Model/UrlProcessor.php:45 (getRewriteUrl) + - Model/UrlProcessor.php:89 (processUrl) + - Helper/Data.php:23 + Recommended: Use configuration or store manager to get store ID dynamically." +""" + +# Instructions for suggestedFixDiff format +SUGGESTED_FIX_DIFF_FORMAT = """ +📝 SUGGESTED FIX DIFF FORMAT: +When providing suggestedFixDiff, use standard unified diff format: + +``` +--- a/path/to/file.ext ++++ b/path/to/file.ext +@@ -START_LINE,COUNT +START_LINE,COUNT @@ + context line (unchanged) +-removed line (starts with minus) ++added line (starts with plus) + context line (unchanged) +``` + +RULES: +1. Include file path headers: `--- a/file` and `+++ b/file` +2. Include hunk header: `@@ -old_start,old_count +new_start,new_count @@` +3. Prefix removed lines with `-` (minus) +4. Prefix added lines with `+` (plus) +5. Prefix context lines with ` ` (single space) +6. Include 1-3 context lines before/after changes +7. Use actual file path from the issue +8. The line numbers in @@ must match the ACTUAL lines in the file + +EXAMPLE: +"suggestedFixDiff": "--- a/src/UserService.java\\n+++ b/src/UserService.java\\n@@ -45,3 +45,4 @@\\n public User findById(Long id) {\\n- return repo.findById(id);\\n+ return repo.findById(id)\\n+ .orElseThrow(() -> new NotFoundException());\\n }" + +DO NOT use markdown code blocks inside the JSON value. +""" + +# Lost-in-the-Middle protection instructions +LOST_IN_MIDDLE_INSTRUCTIONS = """ +⚠️ CRITICAL: LOST-IN-THE-MIDDLE PROTECTION ACTIVE + +The context below is STRUCTURED BY PRIORITY. Follow this analysis order STRICTLY: + +📋 ANALYSIS PRIORITY ORDER (MANDATORY): +1️⃣ HIGH PRIORITY (50% attention): Core business logic, security, auth - analyze FIRST +2️⃣ MEDIUM PRIORITY (25% attention): Dependencies, shared utils, models +3️⃣ LOW PRIORITY (10% attention): Tests, configs - quick scan only +4️⃣ RAG CONTEXT (15% attention): Additional context from codebase + +🎯 FOCUS HIERARCHY: +- Security issues > Architecture problems > Performance > Code quality > Style +- Business impact > Technical details +- Root cause > Symptoms + +🛡️ BLOCK PR IMMEDIATELY IF FOUND: +- SQL Injection / XSS / Command Injection +- Hardcoded secrets/API keys +- Authentication bypass +- Remote Code Execution possibilities +""" + +ADDITIONAL_INSTRUCTIONS = ( + "CRITICAL INSTRUCTIONS:\n" + "1. You have a LIMITED number of steps (max 120). Plan efficiently - do NOT make unnecessary tool calls.\n" + "2. After retrieving the diff, analyze it and produce your final JSON response IMMEDIATELY.\n" + "3. Do NOT retrieve every file individually - use the diff output to identify issues.\n" + "4. Your FINAL response must be ONLY a valid JSON object with 'comment' and 'issues' fields.\n" + "5. The 'issues' field MUST be a JSON array [], NOT an object with numeric string keys.\n" + "6. If you cannot complete the review within your step limit, output your partial findings in JSON format.\n" + "7. Do NOT include any markdown formatting, explanations, or other text - only the JSON structure.\n" + "8. STOP making tool calls and produce output once you have enough information to analyze.\n" + "9. If you encounter errors with MCP tools, proceed with available information and note limitations in the comment field.\n" + "10. FOLLOW PRIORITY ORDER: Analyze HIGH priority sections FIRST, then MEDIUM, then LOW.\n" + "11. For LARGE PRs: Focus 60% attention on HIGH priority, 25% on MEDIUM, 15% on LOW/RAG." +) + +BRANCH_REVIEW_PROMPT_TEMPLATE = """You are an expert code reviewer performing a branch reconciliation review after a PR merge. +Workspace: {workspace} +Repository slug: {repo} +Commit Hash: {commit_hash} +Branch: {branch} + +## MCP Tool Parameters +When calling MCP tools (getBranchFileContent, etc.), use these EXACT values: +- workspace: "{workspace}" (owner/organization name only - NOT the full repo path) +- repoSlug: "{repo}" + +CRITICAL INSTRUCTIONS FOR BRANCH RECONCILIATION: +1. The **Previous Analysis Issues** are provided below - these are issues that existed on the branch BEFORE this PR. +2. Your task is to determine if any of these pre-existing issues have been **resolved based on the current content of the file(s) on the branch**. +3. For EACH issue in the previous analysis, you MUST include it in your response with: + - "issueId": "" (copy the 'id' field from the previous issue) + - "isResolved": true (if the issue is fixed by this PR's changes) + - "isResolved": false (if the issue still persists) + - "reason": "Explanation of why it's resolved or still present" +4. DO NOT report new issues - this is ONLY for checking resolution status of existing issues. +5. You MUST retrieve the current file content using MCP tools to compare against the previous issues (e.g. via getBranchFileContent tool). +6. If you see similar errors, you MUST group them together. Set the duplicate to isResolved: true, and leave one of the errors in its original status. + +⚠️ CRITICAL FOR RESOLVED ISSUES: +When an issue is RESOLVED (isResolved: true), you MUST: +1. Provide a clear "reason" field explaining HOW the issue was fixed (e.g., "The null check was added on line 45", "The SQL injection vulnerability was fixed by using parameterized queries") +2. This "reason" will be stored as the resolution description for historical tracking +3. Be specific about what code change fixed the issue + +⚠️ CRITICAL FOR PERSISTING (UNRESOLVED) ISSUES: +When an issue PERSISTS (isResolved: false), you MUST: +1. COPY the "suggestedFixDiff" field EXACTLY from the original previous issue - DO NOT omit it +2. COPY the "suggestedFixDescription" field EXACTLY from the original previous issue +3. Keep the same severity and category +4. Only update the "reason" field to explain why it still persists +5. Update the "line" field if the line number changed due to other code changes + +Example for PERSISTING issue: +Previous issue had: {{"id": "123", "suggestedFixDiff": "--- a/file.py\\n+++ b/file.py\\n..."}} +Your response MUST include: {{"issueId": "123", "isResolved": false, "suggestedFixDiff": "--- a/file.py\\n+++ b/file.py\\n...", ...}} + + +--- PREVIOUS ANALYSIS ISSUES --- +{previous_issues_json} +--- END OF PREVIOUS ISSUES --- + +EFFICIENCY INSTRUCTIONS (YOU HAVE LIMITED STEPS - MAX 120): +1. For each file with issues, retrieve content using getBranchFileContent +2. Analyze content to determine if issues are resolved +3. After checking all relevant files, produce your JSON response IMMEDIATELY +4. Do NOT make redundant tool calls - each tool call uses one of your limited steps + +You MUST: +1. Retrieve file content for files with issues using getBranchFileContent MCP tool +2. For each previous issue, check if the current file content shows it resolved +3. STOP making tool calls and produce your final JSON response once you have analyzed all relevant files + +DO NOT: +1. Report new issues - focus ONLY on the provided previous issues +2. Make more than necessary tool calls - be efficient +3. Continue making tool calls indefinitely + +IMPORTANT LINE NUMBER INSTRUCTIONS: +The "line" field MUST contain the line number in the current version of the file on the branch. +If you retrieve the full source file content via getBranchFileContent, use the line number as it appears in that file. + +CRITICAL: Your final response must be ONLY a valid JSON object in this exact format: +{{ + "comment": "Summary of branch reconciliation - how many issues were resolved vs persisting", + "issues": [ + {{ + "issueId": "", + "severity": "HIGH|MEDIUM|LOW|INFO", + "category": "SECURITY|PERFORMANCE|CODE_QUALITY|BUG_RISK|STYLE|DOCUMENTATION|BEST_PRACTICES|ERROR_HANDLING|TESTING|ARCHITECTURE", + "file": "file-path", + "line": "line-number-in-current-file", + "reason": "For RESOLVED: Explain HOW the issue was fixed (e.g., 'Added null check on line 45'). For UNRESOLVED: Explain why it still persists.", + "suggestedFixDescription": "Clear description of how to fix the issue (copy from original for unresolved issues)", + "suggestedFixDiff": "Unified diff showing exact code changes (copy from original for unresolved issues)", + "isResolved": true + }} + ] +}} + +IMPORTANT: +- The "issues" field MUST be a JSON array [], NOT an object with numeric keys. +- You MUST include ALL previous issues in your response +- Each issue MUST have the "issueId" field matching the original issue ID +- Each issue MUST have "isResolved" as either true or false +- Each issue MUST have a "category" field from the allowed list +- FOR UNRESOLVED ISSUES: COPY "suggestedFixDescription" AND "suggestedFixDiff" from the original issue - DO NOT OMIT THEM +- The suggestedFixDiff is MANDATORY for unresolved issues - copy it verbatim from the previous issue data +""" + +STAGE_0_PLANNING_PROMPT_TEMPLATE = """SYSTEM ROLE: +You are an expert PR scope analyzer for a code review system. +Your job is to prioritize files for review - ALL files must be included. +Output structured JSON—no filler, no explanations. + +--- + +USER PROMPT: + +Task: Analyze this PR and create a review plan for ALL changed files. + +## PR Metadata +- Repository: {repo_slug} +- PR ID: {pr_id} +- Title: {pr_title} +- Author: {author} +- Branch: {branch_name} +- Target: {target_branch} +- Commit: {commit_hash} + +## Changed Files Summary +```json +{changed_files_json} +``` + +Business Context +This PR introduces changes that need careful analysis. + +CRITICAL INSTRUCTION: +You MUST include EVERY file from the "Changed Files Summary" above. +- Files that need review go into "file_groups" +- Files that can be skipped go into "files_to_skip" with a reason +- The total count of files in file_groups + files_to_skip MUST equal the input file count + +Create a prioritized review plan in this JSON format: + +{{ + "analysis_summary": "2-sentence overview of PR scope and risk level", + "file_groups": [ + {{ + "group_id": "GROUP_A_SECURITY", + "priority": "CRITICAL", + "rationale": "reason this group is critical", + "files": [ + {{ + "path": "exact/path/from/input", + "focus_areas": ["SECURITY", "AUTHORIZATION"], + "risk_level": "HIGH", + "estimated_issues": 2 + }} + ] + }}, + {{ + "group_id": "GROUP_B_ARCHITECTURE", + "priority": "HIGH", + "rationale": "...", + "files": [...] + }}, + {{ + "group_id": "GROUP_C_MEDIUM", + "priority": "MEDIUM", + "rationale": "...", + "files": [...] + }}, + {{ + "group_id": "GROUP_D_LOW", + "priority": "LOW", + "rationale": "tests, config, docs", + "files": [...] + }} + ], + "files_to_skip": [ + {{ + "path": "exact/path/from/input", + "reason": "Auto-generated file / lock file / no logic" + }} + ], + "cross_file_concerns": [ + "Hypothesis 1: check if X affects Y", + "Hypothesis 2: check security of Z" + ] +}} + +Priority Guidelines: +- CRITICAL: security, auth, data access, payment, core business logic +- HIGH: architecture changes, API contracts, database schemas +- MEDIUM: refactoring, new utilities, business logic extensions +- LOW: tests, documentation, config files, styling +- files_to_skip: lock files, auto-generated code, binary assets, .md files (unless README changes are significant) + +REMEMBER: Every input file must appear exactly once - either in a file_group or in files_to_skip. +""" + +STAGE_1_BATCH_PROMPT_TEMPLATE = """SYSTEM ROLE: +You are a senior code reviewer analyzing a BATCH of files. +Your goal: Identify bugs, security risks, and quality issues in the provided files. +You are conservative: if a file looks safe, return an empty issues list for it. + +⚠️ CRITICAL: DIFF-ONLY CONTEXT RULE +You are reviewing ONLY the diff (changed lines), NOT the full file. +DO NOT report issues about code you CANNOT see in the diff: +- Missing imports/use statements - you can only see changes, not the file header +- Missing variable declarations - they may exist outside the diff context +- Missing function definitions - the function may be defined elsewhere in the file +- Missing class properties - they may be declared outside the visible changes + +ONLY report issues that you can VERIFY from the visible diff content. +If you suspect an issue but cannot confirm it from the diff, DO NOT report it. +When in doubt, assume the code is correct - the developer can see the full file, you cannot. + +{incremental_instructions} +PROJECT RULES: +{project_rules} + +CODEBASE CONTEXT (from RAG): +{rag_context} + +{previous_issues} + +SUGGESTED_FIX_DIFF_FORMAT: +Use standard unified diff format (git style). +- Header: `--- a/path/to/file` and `+++ b/path/to/file` +- Context: Provide 2 lines of context around changes. +- Additions: start with `+` +- Deletions: start with `-` +Must be valid printable text. + +BATCH INSTRUCTIONS: +Review each file below independently. +For each file, produce a review result. +Use the CODEBASE CONTEXT above to understand how the changed code integrates with existing patterns, dependencies, and architectural decisions. +If previous issue fixed in a current version, mark it as resolved. + +INPUT FILES: +Priority: {priority} + +{files_context} + +OUTPUT FORMAT: +Return ONLY valid JSON with this structure: +{{ + "reviews": [ + {{ + "file": "path/to/file1", + "analysis_summary": "Summary of findings for file 1", + "issues": [ + {{ + "severity": "HIGH|MEDIUM|LOW|INFO", + "category": "SECURITY|PERFORMANCE|CODE_QUALITY|BUG_RISK|STYLE|DOCUMENTATION|BEST_PRACTICES|ERROR_HANDLING|TESTING|ARCHITECTURE", + "file": "path/to/file1", + "line": "42", + "reason": "Detailed explanation of the issue", + "suggestedFixDescription": "Clear description of how to fix the issue", + "suggestedFixDiff": "Unified diff showing exact code changes (MUST follow SUGGESTED_FIX_DIFF_FORMAT)", + "isResolved": false|true + }} + ], + "confidence": "HIGH|MEDIUM|LOW|INFO", + "note": "Optional note" + }}, + {{ + "file": "path/to/file2", + "...": "..." + }} + ] +}} + +Constraints: +- Return exactly one review object per input file. +- Match file paths exactly. +- Skip style nits. +- suggestedFixDiff MUST be a valid unified diff string if a fix is proposed. +""" + +STAGE_2_CROSS_FILE_PROMPT_TEMPLATE = """SYSTEM ROLE: +You are a staff architect reviewing this PR for systemic risks. +Focus on: data flow, authorization patterns, consistency, database integrity, service boundaries. +At temperature 0.1, you will be conservative—that is correct. Flag even low-confidence concerns. +Return structured JSON. + +USER PROMPT: + +Task: Cross-file architectural and security review. + +PR Overview +Repository: {repo_slug} +Title: {pr_title} +Commit: {commit_hash} + +Hypotheses to Verify (from Planning Stage): +{concerns_text} + +All Findings from Stage 1 (Per-File Reviews) +{stage_1_findings_json} + +Architecture Reference +{architecture_context} + +Database Migrations in This PR +{migrations} + +Output Format +Return ONLY valid JSON: + +{{ + "pr_risk_level": "CRITICAL|HIGH|MEDIUM|LOW", + "cross_file_issues": [ + {{ + "id": "CROSS_001", + "severity": "HIGH", + "category": "SECURITY|ARCHITECTURE|DATA_INTEGRITY|BUSINESS_LOGIC", + "title": "Issue affecting multiple files", + "affected_files": ["path1", "path2"], + "description": "Pattern or risk spanning multiple files", + "evidence": "Which files exhibit this pattern and how they interact", + "business_impact": "What breaks if this is not fixed", + "suggestion": "How to fix across these files" + }} + ], + "data_flow_concerns": [ + {{ + "flow": "Data flow description...", + "gap": "Potential gap", + "files_involved": ["file1", "file2"], + "severity": "HIGH" + }} + ], + "immutability_enforcement": {{ + "rule": "Analysis results immutable after status=FINAL", + "check_pass": true, + "evidence": "..." + }}, + "database_integrity": {{ + "concerns": ["FK constraints", "cascade deletes"], + "findings": [] + }}, + "pr_recommendation": "PASS|PASS_WITH_WARNINGS|FAIL", + "confidence": "HIGH|MEDIUM|LOW|INFO" +}} + +Constraints: +- Do NOT re-report individual file issues; instead, focus on patterns +- Only flag cross-file concerns if at least 2 files are involved +- If Stage 1 found no HIGH/CRITICAL issues in security files, mark this as "PASS" with confidence "HIGH" +- If any CRITICAL issues exist from Stage 1, set pr_recommendation to "FAIL" +""" + +STAGE_3_AGGREGATION_PROMPT_TEMPLATE = """SYSTEM ROLE: +You are a senior review coordinator. Produce a concise executive summary for PR review. +Tone: professional, non-alarmist, but direct about blockers. +Format: clean markdown suitable for GitHub/GitLab/Bitbucket PR comments. + +USER PROMPT: + +Task: Produce final PR executive summary (issues will be posted separately). + +Input Data +PR Metadata +Repository: {repo_slug} +PR: #{pr_id} +Author: {author} +Title: {pr_title} +Files changed: {total_files} +Total changes: +{additions} -{deletions} + +All Findings +Stage 0 Plan: +{stage_0_plan} + +Stage 1 Issues: +{stage_1_issues_json} + +Stage 2 Cross-File Findings: +{stage_2_findings_json} + +Stage 2 Recommendation: {recommendation} + +Report Template +Produce markdown report with this exact structure (NO issues list - they are posted separately): + +# Pull Request Review: {pr_title} +**Status**: {{PASS | PASS WITH WARNINGS | FAIL}} +**Risk Level**: {{CRITICAL | HIGH | MEDIUM | LOW}} +**Review Coverage**: {{X}} files analyzed in depth +**Confidence**: HIGH / MEDIUM / LOW + +--- + +## Executive Summary +{{2-4 sentence summary of the PR scope, primary changes, and overall assessment. Mention key risk areas if any.}} + +## Recommendation +**Decision**: {{PASS | PASS WITH WARNINGS | FAIL}} + +{{1-2 sentences explaining the decision and any conditions or next steps.}} + +--- + +Constraints: +- This is human-facing; be clear and professional +- Keep it concise - detailed issues are posted in a separate comment +- Do NOT list individual issues in this summary +- Do NOT include token counts or internal reasoning +- If any CRITICAL issue exists, set Status to FAIL +- If HIGH issues exist but no CRITICAL, set Status to PASS WITH WARNINGS +"""