From 7fe9fe33fa2c7e9fcabd05385c30ff090c12dc97 Mon Sep 17 00:00:00 2001 From: rostislav Date: Tue, 13 Jan 2026 01:07:38 +0200 Subject: [PATCH 1/7] Add prompt constants for code review instructions and guidelines - Introduced ISSUE_CATEGORIES for categorizing issues during code reviews. - Added LINE_NUMBER_INSTRUCTIONS to guide accurate line number reporting from diffs. - Included ISSUE_DEDUPLICATION_INSTRUCTIONS to prevent duplicate issue reporting. - Defined SUGGESTED_FIX_DIFF_FORMAT for consistent format in suggested fixes. - Implemented LOST_IN_MIDDLE_INSTRUCTIONS to prioritize critical analysis areas. - Added ADDITIONAL_INSTRUCTIONS for efficient review process management. - Created FIRST_REVIEW_PROMPT_TEMPLATE and REVIEW_WITH_PREVIOUS_ANALYSIS_DATA_TEMPLATE for structured review prompts. - Established templates for various review scenarios including DIRECT_FIRST_REVIEW and INCREMENTAL_REVIEW. --- ...1.1.0__add_info_severity_to_constraint.sql | 9 + .../mcp-client/inspect_mcp_agent.py | 18 + python-ecosystem/mcp-client/model/models.py | 104 +- .../mcp-client/server/web_server.py | 2 +- .../mcp-client/service/command_service.py | 3 +- .../mcp-client/service/llm_reranker.py | 20 +- .../service/multi_stage_orchestrator.py | 1011 +++++++++++++++++ .../service/pooled_review_service.py | 2 +- .../mcp-client/service/review_service.py | 557 +-------- .../utils/prompts/prompt_builder.py | 319 ++++++ .../prompt_constants.py} | 628 +++++----- 11 files changed, 1843 insertions(+), 830 deletions(-) create mode 100644 java-ecosystem/libs/core/src/main/resources/db/migration/1.1.0/V1.1.0__add_info_severity_to_constraint.sql create mode 100644 python-ecosystem/mcp-client/inspect_mcp_agent.py create mode 100644 python-ecosystem/mcp-client/service/multi_stage_orchestrator.py create mode 100644 python-ecosystem/mcp-client/utils/prompts/prompt_builder.py rename python-ecosystem/mcp-client/utils/{prompt_builder.py => prompts/prompt_constants.py} (66%) 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/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/model/models.py b/python-ecosystem/mcp-client/model/models.py index 0b46faba..9011e8c3 100644 --- a/python-ecosystem/mcp-client/model/models.py +++ b/python-ecosystem/mcp-client/model/models.py @@ -28,7 +28,7 @@ class IssueDTO(BaseModel): 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 + severity: Optional[str] = None # HIGH|MEDIUM|LOW title: Optional[str] = None description: Optional[str] = None # Typically holds the suggestedFix file: Optional[str] = None @@ -82,7 +82,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 +103,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 +110,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 +132,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 +145,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')") @@ -175,4 +170,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..56c45e6b 100644 --- a/python-ecosystem/mcp-client/service/command_service.py +++ b/python-ecosystem/mcp-client/service/command_service.py @@ -319,10 +319,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..56198bd4 --- /dev/null +++ b/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py @@ -0,0 +1,1011 @@ +import logging +import asyncio +import json +from typing import Dict, Any, List, Optional, Callable +from datetime import datetime + +from model.models import ( + ReviewRequestDto, + ReviewPlan, + CodeReviewOutput, + CodeReviewIssue, + ReviewFile, + FileGroup, + CrossFileAnalysisResult, + FileReviewOutput, + FileReviewBatchOutput +) +from utils.prompts.prompt_builder import PromptBuilder +from utils.response_parser import ResponseParser +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.summary 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.summary 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) + + 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 + + # Create a CodeReviewIssue for the persisting previous issue + persisting_issue = CodeReviewIssue( + id=str(issue_id) if issue_id else None, + severity=prev_data.get('severity', 'MEDIUM'), + category=prev_data.get('category', prev_data.get('issueCategory', 'CODE_QUALITY')), + file=file_path, + line=str(prev_data.get('line', prev_data.get('lineNumber', '1'))), + reason=prev_data.get('reason', prev_data.get('description', 'Issue from previous analysis')), + suggestedFixDescription=prev_data.get('suggestedFixDescription', ''), + suggestedFixDiff=prev_data.get('suggestedFixDiff', ''), + isResolved=is_resolved + ) + 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 + ) + rag_context_text = self._format_rag_context( + rag_response.get("context", {}), + set(batch_file_paths) + ) + 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 + ) -> str: + """ + Format RAG context into a readable string for the prompt. + Optionally filter to chunks relevant to specific files. + """ + 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 "" + + 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)) + + # 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..b550a5d3 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 diff --git a/python-ecosystem/mcp-client/service/review_service.py b/python-ecosystem/mcp-client/service/review_service.py index baeef56a..6bc5108c 100644 --- a/python-ecosystem/mcp-client/service/review_service.py +++ b/python-ecosystem/mcp-client/service/review_service.py @@ -10,7 +10,7 @@ from model.models import ReviewRequestDto, CodeReviewOutput, CodeReviewIssue, AnalysisMode 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.llm_reranker import LLMReranker @@ -23,6 +23,7 @@ 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.multi_stage_orchestrator import MultiStageReviewOrchestrator logger = logging.getLogger(__name__) @@ -69,32 +70,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 +130,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 +147,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 +154,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 = self._build_prompt(request, pr_metadata, rag_context) + + 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 +241,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, 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..92cf5478 --- /dev/null +++ b/python-ecosystem/mcp-client/utils/prompts/prompt_builder.py @@ -0,0 +1,319 @@ +from typing import Any, Dict, List, Optional +import json +from model.models import IssueDTO +from utils.prompts.prompt_constants import ( + ISSUE_CATEGORIES, + LINE_NUMBER_INSTRUCTIONS, + ISSUE_DEDUPLICATION_INSTRUCTIONS, + SUGGESTED_FIX_DIFF_FORMAT, + LOST_IN_MIDDLE_INSTRUCTIONS, + ADDITIONAL_INSTRUCTIONS, + FIRST_REVIEW_PROMPT_TEMPLATE, + REVIEW_WITH_PREVIOUS_ANALYSIS_DATA_TEMPLATE, + BRANCH_REVIEW_PROMPT_TEMPLATE, + DIRECT_FIRST_REVIEW_PROMPT_TEMPLATE, + DIRECT_REVIEW_WITH_PREVIOUS_ANALYSIS_PROMPT_TEMPLATE, + INCREMENTAL_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_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 + + return FIRST_REVIEW_PROMPT_TEMPLATE.format( + workspace=workspace, + repo=repo, + pr_id=pr_id, + context_section=context_section, + issue_categories=ISSUE_CATEGORIES, + issue_deduplication_instructions=ISSUE_DEDUPLICATION_INSTRUCTIONS, + line_number_instructions=LINE_NUMBER_INSTRUCTIONS, + suggested_fix_diff_format=SUGGESTED_FIX_DIFF_FORMAT + ) + + @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 + + return REVIEW_WITH_PREVIOUS_ANALYSIS_DATA_TEMPLATE.format( + workspace=workspace, + repo=repo, + pr_id=pr_id, + context_section=context_section, + previous_issues_json=previous_issues_json, + issue_categories=ISSUE_CATEGORIES, + issue_deduplication_instructions=ISSUE_DEDUPLICATION_INSTRUCTIONS, + line_number_instructions=LINE_NUMBER_INSTRUCTIONS, + suggested_fix_diff_format=SUGGESTED_FIX_DIFF_FORMAT + ) + + @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 _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_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/prompt_builder.py b/python-ecosystem/mcp-client/utils/prompts/prompt_constants.py similarity index 66% rename from python-ecosystem/mcp-client/utils/prompt_builder.py rename to python-ecosystem/mcp-client/utils/prompts/prompt_constants.py index b4045d4c..b997e0bc 100644 --- a/python-ecosystem/mcp-client/utils/prompt_builder.py +++ b/python-ecosystem/mcp-client/utils/prompts/prompt_constants.py @@ -1,8 +1,5 @@ -from typing import Any, Dict, List, Optional -import json -from model.models import IssueDTO -# Define valid issue categories +# Valid issue categories ISSUE_CATEGORIES = """ Available issue categories (use EXACTLY one of these values): - SECURITY: Security vulnerabilities, injection risks, authentication issues @@ -33,11 +30,11 @@ EXAMPLE: @@ -10,5 +10,6 @@ - context line <- Line 10 in new file - context line <- Line 11 in new file + 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 + context line <- Line 13 in new file If the issue is on the "added line", report line: "12" (not 14!) @@ -86,10 +83,10 @@ --- a/path/to/file.ext +++ b/path/to/file.ext @@ -START_LINE,COUNT +START_LINE,COUNT @@ - context line (unchanged) + context line (unchanged) -removed line (starts with minus) +added line (starts with plus) - context line (unchanged) + context line (unchanged) ``` RULES: @@ -132,35 +129,22 @@ - 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. +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." +) + +FIRST_REVIEW_PROMPT_TEMPLATE = """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} @@ -180,9 +164,9 @@ def build_first_review_prompt( 4. BUG_RISK: Edge cases, null checks, type mismatches 5. CODE_QUALITY: Maintainability, complexity, code smells -{ISSUE_CATEGORIES} +{issue_categories} -{ISSUE_DEDUPLICATION_INSTRUCTIONS} +{issue_deduplication_instructions} EFFICIENCY INSTRUCTIONS (YOU HAVE LIMITED STEPS - MAX 120): 1. First, retrieve the PR diff using getPullRequestDiff tool @@ -202,9 +186,9 @@ def build_first_review_prompt( 3. Continue making tool calls indefinitely 4. Report the SAME root cause as multiple separate issues -{LINE_NUMBER_INSTRUCTIONS} +{line_number_instructions} -{SUGGESTED_FIX_DIFF_FORMAT} +{suggested_fix_diff_format} CRITICAL: Your final response must be ONLY a valid JSON object in this exact format: {{ @@ -238,41 +222,8 @@ def build_first_review_prompt( 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. +REVIEW_WITH_PREVIOUS_ANALYSIS_DATA_TEMPLATE = """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} @@ -300,9 +251,9 @@ def build_review_prompt_with_previous_analysis_data( 4. Security issues 5. Suggest concrete fixes in the form of DIFF Patch if applicable, and put it in suggested fix -{ISSUE_CATEGORIES} +{issue_categories} -{ISSUE_DEDUPLICATION_INSTRUCTIONS} +{issue_deduplication_instructions} EFFICIENCY INSTRUCTIONS (YOU HAVE LIMITED STEPS - MAX 120): 1. First, retrieve the PR diff using getPullRequestDiff tool @@ -325,9 +276,9 @@ def build_review_prompt_with_previous_analysis_data( CRITICAL INSTRUCTION FOR LARGE PRs: Report ALL UNIQUE issues found. Merge similar issues (same root cause) into one. -{LINE_NUMBER_INSTRUCTIONS} +{line_number_instructions} -{SUGGESTED_FIX_DIFF_FORMAT} +{suggested_fix_diff_format} CRITICAL: Your final response must be ONLY a valid JSON object in this exact format: {{ @@ -376,21 +327,8 @@ def build_review_prompt_with_previous_analysis_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 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. + +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} @@ -485,128 +423,8 @@ def build_branch_review_prompt_with_branch_issues_data(pr_metadata: Dict[str, An 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. +DIRECT_FIRST_REVIEW_PROMPT_TEMPLATE = """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} @@ -630,13 +448,13 @@ def build_direct_first_review_prompt( 4. BUG_RISK: Edge cases, null checks, type mismatches 5. CODE_QUALITY: Maintainability, complexity, code smells -{ISSUE_CATEGORIES} +{issue_categories} -{ISSUE_DEDUPLICATION_INSTRUCTIONS} +{issue_deduplication_instructions} -{LINE_NUMBER_INSTRUCTIONS} +{line_number_instructions} -{SUGGESTED_FIX_DIFF_FORMAT} +{suggested_fix_diff_format} CRITICAL: Report ALL UNIQUE issues found. Merge similar issues (same root cause) into one. @@ -667,36 +485,8 @@ def build_direct_first_review_prompt( 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. +DIRECT_REVIEW_WITH_PREVIOUS_ANALYSIS_PROMPT_TEMPLATE = """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} @@ -724,13 +514,13 @@ def build_direct_review_prompt_with_previous_analysis( 3. Find NEW issues introduced in this PR version 4. Prioritize by security > architecture > performance > quality -{ISSUE_CATEGORIES} +{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} +{suggested_fix_diff_format} Your response must be ONLY a valid JSON object: {{ @@ -753,49 +543,8 @@ def build_direct_review_prompt_with_previous_analysis( 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. +INCREMENTAL_REVIEW_PROMPT_TEMPLATE = """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} @@ -838,14 +587,14 @@ def build_incremental_review_prompt( - 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} +{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} +{suggested_fix_diff_format} CRITICAL: Report ALL issues found in delta diff. Do not group them or omit them for brevity. @@ -882,4 +631,291 @@ def build_incremental_review_prompt( Do NOT include any markdown formatting, explanatory text, or other content - only the JSON object. """ - return prompt \ No newline at end of file + +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. +{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. Aggregate all findings into a clear, actionable report. +Tone: professional, non-alarmist, but direct about blockers. +Format: clean markdown suitable for GitHub/GitLab PR comments. + +USER PROMPT: + +Task: Produce final PR assessment report. + +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: + +# 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-3 sentence summary of the PR scope, primary changes, and overall risk level}} + +## Recommendation +Decision: {{PASS | PASS WITH WARNINGS | FAIL}} +--- + +Constraints: +- This is human-facing; be clear and professional +- Bold action items +- Do NOT include token counts or internal reasoning +- If any CRITICAL issue exists, set Status to FAIL +""" From c8fa9ab35ea87afad776788ced50a1c031d77425 Mon Sep 17 00:00:00 2001 From: rostislav Date: Tue, 13 Jan 2026 02:22:53 +0200 Subject: [PATCH 2/7] feat: Enhance issue handling and reporting with additional fields and detailed markdown generation --- .../request/ai/AiRequestPreviousIssueDTO.java | 8 +- .../CommentOnBitbucketCloudAction.java | 18 ++++ .../formatters/MarkdownAnalysisFormatter.java | 83 ++++++++++++-- .../bitbucket/service/ReportGenerator.java | 16 +++ .../actions/CommentOnPullRequestAction.java | 62 +++++++++++ .../bitbucket/BitbucketReportingService.java | 41 ++++++- .../github/GitHubReportingService.java | 101 +++++++++++------- python-ecosystem/mcp-client/model/models.py | 3 + .../service/multi_stage_orchestrator.py | 29 +++-- .../utils/prompts/prompt_constants.py | 20 ++-- 10 files changed, 311 insertions(+), 70 deletions(-) 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/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/model/models.py b/python-ecosystem/mcp-client/model/models.py index 9011e8c3..5c7171f8 100644 --- a/python-ecosystem/mcp-client/model/models.py +++ b/python-ecosystem/mcp-client/model/models.py @@ -153,6 +153,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): diff --git a/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py b/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py index 56198bd4..22e41ead 100644 --- a/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py +++ b/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py @@ -295,17 +295,27 @@ async def _reconcile_previous_issues( # For now, we'll re-report it as persisting unless LLM marked it resolved pass - # Create a CodeReviewIssue for the persisting previous issue + # 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', 'MEDIUM'), - category=prev_data.get('category', prev_data.get('issueCategory', 'CODE_QUALITY')), - file=file_path, - line=str(prev_data.get('line', prev_data.get('lineNumber', '1'))), - reason=prev_data.get('reason', prev_data.get('description', 'Issue from previous analysis')), - suggestedFixDescription=prev_data.get('suggestedFixDescription', ''), - suggestedFixDiff=prev_data.get('suggestedFixDiff', ''), - isResolved=is_resolved + 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) @@ -714,7 +724,6 @@ async def _execute_stage_3_aggregation( 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 ) diff --git a/python-ecosystem/mcp-client/utils/prompts/prompt_constants.py b/python-ecosystem/mcp-client/utils/prompts/prompt_constants.py index b997e0bc..24814e2b 100644 --- a/python-ecosystem/mcp-client/utils/prompts/prompt_constants.py +++ b/python-ecosystem/mcp-client/utils/prompts/prompt_constants.py @@ -867,13 +867,13 @@ """ STAGE_3_AGGREGATION_PROMPT_TEMPLATE = """SYSTEM ROLE: -You are a senior review coordinator. Aggregate all findings into a clear, actionable report. +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 PR comments. +Format: clean markdown suitable for GitHub/GitLab/Bitbucket PR comments. USER PROMPT: -Task: Produce final PR assessment report. +Task: Produce final PR executive summary (issues will be posted separately). Input Data PR Metadata @@ -897,25 +897,31 @@ Stage 2 Recommendation: {recommendation} Report Template -Produce markdown report with this exact structure: +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-3 sentence summary of the PR scope, primary changes, and overall risk level}} +{{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}} +**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 -- Bold action items +- 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 """ From ba7bfb46bc05337c61f5e508a7602e2d79113fd8 Mon Sep 17 00:00:00 2001 From: rostislav Date: Tue, 13 Jan 2026 02:27:51 +0200 Subject: [PATCH 3/7] feat: Enhance RAG context filtering to exclude stale chunks from modified PR files --- .../service/multi_stage_orchestrator.py | 38 ++++++++++++++++++- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py b/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py index 22e41ead..ef92032b 100644 --- a/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py +++ b/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py @@ -586,9 +586,12 @@ async def _review_file_batch( 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) + set(batch_file_paths), + pr_changed_files=request.changedFiles ) logger.debug(f"Batch RAG context retrieved for {batch_file_paths}") except Exception as e: @@ -874,11 +877,18 @@ def _clean_json_text(self, text: str) -> str: def _format_rag_context( self, rag_context: Optional[Dict[str, Any]], - relevant_files: Optional[set] = None + 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 "" @@ -888,6 +898,15 @@ def _format_rag_context( 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: @@ -900,6 +919,21 @@ def _format_rag_context( 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 From 7970ac2d5693610a7ff9f4fa845284bd065d3474 Mon Sep 17 00:00:00 2001 From: rostislav Date: Tue, 13 Jan 2026 02:47:30 +0200 Subject: [PATCH 4/7] feat: Remove unused prompt templates and add method for additional instructions --- .../mcp-client/utils/prompts/prompt_builder.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/python-ecosystem/mcp-client/utils/prompts/prompt_builder.py b/python-ecosystem/mcp-client/utils/prompts/prompt_builder.py index 92cf5478..6d677777 100644 --- a/python-ecosystem/mcp-client/utils/prompts/prompt_builder.py +++ b/python-ecosystem/mcp-client/utils/prompts/prompt_builder.py @@ -11,9 +11,6 @@ FIRST_REVIEW_PROMPT_TEMPLATE, REVIEW_WITH_PREVIOUS_ANALYSIS_DATA_TEMPLATE, BRANCH_REVIEW_PROMPT_TEMPLATE, - DIRECT_FIRST_REVIEW_PROMPT_TEMPLATE, - DIRECT_REVIEW_WITH_PREVIOUS_ANALYSIS_PROMPT_TEMPLATE, - INCREMENTAL_REVIEW_PROMPT_TEMPLATE, STAGE_0_PLANNING_PROMPT_TEMPLATE, STAGE_1_BATCH_PROMPT_TEMPLATE, STAGE_2_CROSS_FILE_PROMPT_TEMPLATE, @@ -124,6 +121,15 @@ def build_branch_review_prompt_with_branch_issues_data(pr_metadata: Dict[str, An 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_legacy_rag_section(rag_context: Dict[str, Any]) -> str: """Build legacy RAG section for backward compatibility.""" From e9204db3ed5e2850379c354af210ecfb6bb16873 Mon Sep 17 00:00:00 2001 From: rostislav Date: Thu, 15 Jan 2026 00:21:41 +0200 Subject: [PATCH 5/7] feat: Update IssueDTO to include new fields for improved issue tracking and backward compatibility --- python-ecosystem/mcp-client/model/models.py | 18 +++++++++++++----- .../service/multi_stage_orchestrator.py | 5 +++++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/python-ecosystem/mcp-client/model/models.py b/python-ecosystem/mcp-client/model/models.py index 5c7171f8..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 # HIGH|MEDIUM|LOW - 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 diff --git a/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py b/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py index ef92032b..718becfe 100644 --- a/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py +++ b/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py @@ -265,6 +265,11 @@ async def _reconcile_previous_issues( 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') From 750d203fed148143f689fcc5a988690b49938c41 Mon Sep 17 00:00:00 2001 From: rostislav Date: Thu, 15 Jan 2026 10:44:56 +0200 Subject: [PATCH 6/7] refactor: Simplify prompt building by removing legacy methods and unused constants --- frontend | 2 +- .../mcp-client/service/review_service.py | 98 +---- .../utils/prompts/prompt_builder.py | 154 ------- .../utils/prompts/prompt_constants.py | 407 +----------------- 4 files changed, 15 insertions(+), 646 deletions(-) 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/python-ecosystem/mcp-client/service/review_service.py b/python-ecosystem/mcp-client/service/review_service.py index 6bc5108c..6eac8133 100644 --- a/python-ecosystem/mcp-client/service/review_service.py +++ b/python-ecosystem/mcp-client/service/review_service.py @@ -175,7 +175,7 @@ async def _process_review( logger.info("Executing Branch Analysis & Reconciliation mode") # Build specific prompt for branch analysis pr_metadata = self._build_pr_metadata(request) - prompt = self._build_prompt(request, pr_metadata, rag_context) + prompt = PromptBuilder.build_branch_review_prompt_with_branch_issues_data(pr_metadata) result = await orchestrator.execute_branch_analysis(prompt) else: @@ -389,102 +389,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: diff --git a/python-ecosystem/mcp-client/utils/prompts/prompt_builder.py b/python-ecosystem/mcp-client/utils/prompts/prompt_builder.py index 6d677777..b3cad5b1 100644 --- a/python-ecosystem/mcp-client/utils/prompts/prompt_builder.py +++ b/python-ecosystem/mcp-client/utils/prompts/prompt_builder.py @@ -2,14 +2,7 @@ import json from model.models import IssueDTO from utils.prompts.prompt_constants import ( - ISSUE_CATEGORIES, - LINE_NUMBER_INSTRUCTIONS, - ISSUE_DEDUPLICATION_INSTRUCTIONS, - SUGGESTED_FIX_DIFF_FORMAT, - LOST_IN_MIDDLE_INSTRUCTIONS, ADDITIONAL_INSTRUCTIONS, - FIRST_REVIEW_PROMPT_TEMPLATE, - REVIEW_WITH_PREVIOUS_ANALYSIS_DATA_TEMPLATE, BRANCH_REVIEW_PROMPT_TEMPLATE, STAGE_0_PLANNING_PROMPT_TEMPLATE, STAGE_1_BATCH_PROMPT_TEMPLATE, @@ -18,88 +11,6 @@ ) 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 - - return FIRST_REVIEW_PROMPT_TEMPLATE.format( - workspace=workspace, - repo=repo, - pr_id=pr_id, - context_section=context_section, - issue_categories=ISSUE_CATEGORIES, - issue_deduplication_instructions=ISSUE_DEDUPLICATION_INSTRUCTIONS, - line_number_instructions=LINE_NUMBER_INSTRUCTIONS, - suggested_fix_diff_format=SUGGESTED_FIX_DIFF_FORMAT - ) - - @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 - - return REVIEW_WITH_PREVIOUS_ANALYSIS_DATA_TEMPLATE.format( - workspace=workspace, - repo=repo, - pr_id=pr_id, - context_section=context_section, - previous_issues_json=previous_issues_json, - issue_categories=ISSUE_CATEGORIES, - issue_deduplication_instructions=ISSUE_DEDUPLICATION_INSTRUCTIONS, - line_number_instructions=LINE_NUMBER_INSTRUCTIONS, - suggested_fix_diff_format=SUGGESTED_FIX_DIFF_FORMAT - ) - @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") @@ -130,71 +41,6 @@ def get_additional_instructions() -> str: """ return ADDITIONAL_INSTRUCTIONS - @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_stage_0_planning_prompt( repo_slug: str, diff --git a/python-ecosystem/mcp-client/utils/prompts/prompt_constants.py b/python-ecosystem/mcp-client/utils/prompts/prompt_constants.py index 24814e2b..ec9f98ec 100644 --- a/python-ecosystem/mcp-client/utils/prompts/prompt_constants.py +++ b/python-ecosystem/mcp-client/utils/prompts/prompt_constants.py @@ -144,190 +144,6 @@ "11. For LARGE PRs: Focus 60% attention on HIGH priority, 25% on MEDIUM, 15% on LOW/RAG." ) -FIRST_REVIEW_PROMPT_TEMPLATE = """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. -""" - -REVIEW_WITH_PREVIOUS_ANALYSIS_DATA_TEMPLATE = """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. -""" - BRANCH_REVIEW_PROMPT_TEMPLATE = """You are an expert code reviewer performing a branch reconciliation review after a PR merge. Workspace: {workspace} Repository slug: {repo} @@ -420,216 +236,6 @@ - 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. -""" - -DIRECT_FIRST_REVIEW_PROMPT_TEMPLATE = """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. -""" - -DIRECT_REVIEW_WITH_PREVIOUS_ANALYSIS_PROMPT_TEMPLATE = """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. -""" - -INCREMENTAL_REVIEW_PROMPT_TEMPLATE = """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. """ STAGE_0_PLANNING_PROMPT_TEMPLATE = """SYSTEM ROLE: @@ -729,6 +335,19 @@ 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} From c60e8e5bd4897e64686daf7079af14645b0af00c Mon Sep 17 00:00:00 2001 From: rostislav Date: Thu, 15 Jan 2026 14:02:59 +0200 Subject: [PATCH 7/7] feat: Implement connection pooling in RagClient and add budget validation in context builder --- .../analysis/BranchAnalysisProcessor.java | 4 +- python-ecosystem/mcp-client/main.py | 29 ++++---- .../mcp-client/service/command_service.py | 7 +- .../service/multi_stage_orchestrator.py | 9 +-- .../service/pooled_review_service.py | 2 +- .../mcp-client/service/rag_client.py | 66 ++++++++++++------- .../mcp-client/service/review_service.py | 22 +++---- .../mcp-client/utils/context_builder.py | 10 +++ .../mcp-client/utils/mcp_config.py | 7 +- 9 files changed, 88 insertions(+), 68 deletions(-) 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/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/service/command_service.py b/python-ecosystem/mcp-client/service/command_service.py index 56c45e6b..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, { diff --git a/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py b/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py index 718becfe..12876ce7 100644 --- a/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py +++ b/python-ecosystem/mcp-client/service/multi_stage_orchestrator.py @@ -2,7 +2,6 @@ import asyncio import json from typing import Dict, Any, List, Optional, Callable -from datetime import datetime from model.models import ( ReviewRequestDto, @@ -12,11 +11,9 @@ ReviewFile, FileGroup, CrossFileAnalysisResult, - FileReviewOutput, FileReviewBatchOutput ) from utils.prompts.prompt_builder import PromptBuilder -from utils.response_parser import ResponseParser from utils.diff_processor import ProcessedDiff, DiffProcessor from mcp_use import MCPAgent @@ -138,7 +135,7 @@ async def execute_branch_analysis(self, prompt: str) -> Dict[str, Any]: issues = [i.model_dump() for i in item.issues] if item.issues else [] return { "issues": issues, - "comment": item.summary or "Branch analysis completed." + "comment": item.comment or "Branch analysis completed." } if isinstance(item, str): @@ -150,7 +147,7 @@ async def execute_branch_analysis(self, prompt: str) -> Dict[str, Any]: issues = [i.model_dump() for i in data.issues] if data.issues else [] return { "issues": issues, - "comment": data.summary or "Branch analysis completed." + "comment": data.comment or "Branch analysis completed." } return {"issues": [], "comment": "No issues found."} @@ -647,7 +644,7 @@ async def _review_file_batch( except Exception as parse_err: logger.error(f"Batch review failed: {parse_err}") return [] - + return [] async def _execute_stage_2_cross_file( diff --git a/python-ecosystem/mcp-client/service/pooled_review_service.py b/python-ecosystem/mcp-client/service/pooled_review_service.py index b550a5d3..dce735bb 100644 --- a/python-ecosystem/mcp-client/service/pooled_review_service.py +++ b/python-ecosystem/mcp-client/service/pooled_review_service.py @@ -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 6eac8133..ef9bb770 100644 --- a/python-ecosystem/mcp-client/service/review_service.py +++ b/python-ecosystem/mcp-client/service/review_service.py @@ -4,25 +4,19 @@ 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.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__) @@ -434,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 = {