Coverage for src / pipeline / review_runner.py: 62%
63 statements
« prev ^ index » next coverage.py v7.13.0, created at 2026-01-04 04:43 +0000
1"""ReviewRunner: Code review pipeline stage.
3Extracted from MalaOrchestrator to separate review orchestration from main
4orchestration logic. This module handles:
5- Running code reviews via injected CodeReviewer protocol
6- Retry decisions and no-progress detection
7- Session log path tracking
9The ReviewRunner receives explicit inputs and returns explicit outputs,
10making it testable without SDK or subprocess dependencies.
12Design principles:
13- Protocol-based dependencies for testability
14- Explicit input/output types for clarity
15- Pure functions where possible
16"""
18from __future__ import annotations
20import logging
21from dataclasses import dataclass, field
22from pathlib import Path
23from typing import TYPE_CHECKING, cast
25logger = logging.getLogger(__name__)
27if TYPE_CHECKING:
28 from src.infra.clients.review_output_parser import ReviewResult
29 from src.core.protocols import (
30 CodeReviewer,
31 GateChecker,
32 ReviewResultProtocol,
33 ValidationSpecProtocol,
34 )
35 from src.domain.validation.spec import ValidationSpec
38@dataclass
39class ReviewRunnerConfig:
40 """Configuration for ReviewRunner behavior.
42 Attributes:
43 max_review_retries: Maximum number of review retry attempts.
44 review_timeout: Timeout in seconds for review operations.
45 thinking_mode: Deprecated, kept for backward compatibility.
46 capture_session_log: Deprecated, kept for backward compatibility.
47 """
49 max_review_retries: int = 3
50 review_timeout: int = 1200
51 # Deprecated fields (kept for backward compatibility with orchestrator)
52 thinking_mode: str | None = None
53 capture_session_log: bool = False
56@dataclass
57class ReviewInput:
58 """Input for running a code review.
60 Bundles all data needed to run a single review check.
62 Attributes:
63 issue_id: The issue being reviewed.
64 repo_path: Path to the repository.
65 commit_sha: Current commit SHA to review.
66 issue_description: Issue description for scope verification.
67 baseline_commit: Optional baseline commit for cumulative diff.
68 commit_shas: Optional list of commit SHAs to review directly.
69 claude_session_id: Optional Claude session ID for external review context.
70 """
72 issue_id: str
73 repo_path: Path
74 commit_sha: str
75 issue_description: str | None = None
76 baseline_commit: str | None = None
77 commit_shas: list[str] | None = None
78 claude_session_id: str | None = None
81@dataclass
82class ReviewOutput:
83 """Output from a code review check.
85 Attributes:
86 result: The ReviewResult from the review.
87 session_log_path: Path to the review session log (if captured).
88 """
90 result: ReviewResult | ReviewResultProtocol
91 session_log_path: str | None = None
94@dataclass
95class NoProgressInput:
96 """Input for no-progress check before review retry.
98 Attributes:
99 log_path: Path to the session log file.
100 log_offset: Byte offset marking the end of the previous attempt.
101 previous_commit_hash: Commit hash from the previous attempt.
102 current_commit_hash: Commit hash from this attempt.
103 spec: Optional ValidationSpec for evidence detection.
104 """
106 log_path: Path
107 log_offset: int
108 previous_commit_hash: str | None
109 current_commit_hash: str | None
110 spec: ValidationSpec | None = None
113@dataclass
114class ReviewRunner:
115 """Code review runner for post-gate validation.
117 This class encapsulates the review orchestration logic that was previously
118 inline in MalaOrchestrator. It receives a CodeReviewer (protocol) for
119 actual review execution.
121 The ReviewRunner is responsible for:
122 - Running code reviews via the injected CodeReviewer
123 - Checking no-progress conditions for retry termination
124 - Tracking session log paths
126 Usage:
127 runner = ReviewRunner(
128 code_reviewer=reviewer,
129 config=ReviewRunnerConfig(max_review_retries=3),
130 )
131 output = await runner.run_review(input)
133 Attributes:
134 code_reviewer: CodeReviewer implementation for running reviews.
135 config: Configuration for review behavior.
136 gate_checker: Optional GateChecker for no-progress detection.
137 """
139 code_reviewer: CodeReviewer
140 config: ReviewRunnerConfig = field(default_factory=ReviewRunnerConfig)
141 gate_checker: GateChecker | None = None
143 async def run_review(self, input: ReviewInput) -> ReviewOutput:
144 """Run code review on the given commit.
146 This method invokes the injected CodeReviewer with the appropriate
147 parameters derived from the input.
149 Args:
150 input: ReviewInput with commit_sha, issue_description, etc.
152 Returns:
153 ReviewOutput with result and optional session log path.
154 """
155 import tempfile
157 # Build diff range
158 # Use commit's own parent as default baseline, not HEAD~1
159 # This ensures historical reviews compare against the correct base
160 baseline = input.baseline_commit or f"{input.commit_sha}~1"
161 diff_range = f"{baseline}..{input.commit_sha}"
162 logger.info(
163 "Review started: issue_id=%s diff_range=%s", input.issue_id, diff_range
164 )
166 # Create context file if issue_description provided
167 # Use NamedTemporaryFile to avoid permission issues on shared systems
168 context_file: Path | None = None
169 temp_file = None
170 if input.issue_description:
171 temp_file = tempfile.NamedTemporaryFile(
172 mode="w",
173 prefix=f"review-context-{input.issue_id}-",
174 suffix=".txt",
175 delete=False,
176 )
177 # Set context_file immediately so cleanup happens even if write/close fails
178 context_file = Path(temp_file.name)
180 try:
181 # Write and close inside try block to ensure cleanup on failure
182 if temp_file is not None and input.issue_description is not None:
183 temp_file.write(input.issue_description)
184 temp_file.close()
185 result = await self.code_reviewer(
186 diff_range=diff_range,
187 context_file=context_file,
188 timeout=self.config.review_timeout,
189 claude_session_id=input.claude_session_id,
190 commit_shas=input.commit_shas,
191 )
193 session_log_path = None
194 if result.review_log_path:
195 session_log_path = str(result.review_log_path)
197 logger.info(
198 "Review result: issue_id=%s passed=%s issues=%d",
199 input.issue_id,
200 result.passed,
201 len(result.issues),
202 )
204 return ReviewOutput(
205 result=result,
206 session_log_path=session_log_path,
207 )
208 finally:
209 # Clean up context file after review completes (success or failure)
210 if context_file is not None and context_file.exists():
211 context_file.unlink()
213 def check_no_progress(self, input: NoProgressInput) -> bool:
214 """Check if no progress was made since the last review attempt.
216 No progress is detected when the commit hash hasn't changed and
217 there are no uncommitted changes in the working tree.
219 This should be called before running a review retry to avoid
220 wasting resources on a review that will likely fail again.
222 Args:
223 input: NoProgressInput with log_path, offsets, and commit hashes.
225 Returns:
226 True if no progress was made, False if progress was detected.
228 Raises:
229 ValueError: If gate_checker is not set (required for no-progress).
230 """
231 if self.gate_checker is None:
232 raise ValueError("gate_checker must be set for no-progress detection")
234 return self.gate_checker.check_no_progress(
235 log_path=input.log_path,
236 log_offset=input.log_offset,
237 previous_commit_hash=input.previous_commit_hash,
238 current_commit_hash=input.current_commit_hash,
239 spec=cast("ValidationSpecProtocol | None", input.spec),
240 check_validation_evidence=False, # Only commit/working-tree for reviews
241 )