Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 95 additions & 33 deletions src/commands/review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,39 +48,94 @@ pub struct ReviewOptions {

/// Result of a review command execution.
pub struct ReviewResult {
/// Exit code (0 = ok, 2 = blocked).
/// Exit code (0 = ok, 1 = issues, 2 = blocked).
pub exit_code: i32,
/// The formatted output string.
pub output: String,
/// Total issues found (for memory integration).
pub total_issues: usize,
/// Severity summary (e.g. "1 critical, 2 major, 0 minor").
pub severity_summary: String,
/// Issue categories (e.g. ["security", "performance"]).
pub categories: Vec<String>,
}

impl ReviewResult {
/// Create a result from formatted output, extracting issue stats from the response.
fn with_issues(
exit_code: i32,
output: String,
response: &crate::engine::types::ReviewResponse,
) -> Self {
let total_issues = response.issues.len();
let mut sev_counts: std::collections::HashMap<String, usize> =
std::collections::HashMap::new();
let mut cat_set: std::collections::HashSet<String> = std::collections::HashSet::new();

for issue in &response.issues {
let sev = format!("{:?}", issue.severity).to_lowercase();
*sev_counts.entry(sev).or_insert(0) += 1;
if let Some(ref t) = issue.issue_type {
if !t.is_empty() {
cat_set.insert(t.clone());
}
}
}

let severity_summary = if sev_counts.is_empty() {
String::new()
} else {
let mut parts: Vec<String> = sev_counts
.into_iter()
.map(|(k, v)| format!("{v} {k}"))
.collect();
parts.sort();
parts.join(", ")
};

Self {
exit_code,
output,
total_issues,
severity_summary,
categories: cat_set.into_iter().collect(),
}
}

/// Create a result with no issues (empty diff, errors, etc).
fn empty(exit_code: i32, output: String) -> Self {
Self {
exit_code,
output,
total_issues: 0,
severity_summary: String::new(),
categories: Vec::new(),
}
}
}

/// Execute the review command.
///
/// Gets the diff, validates its size, calls the LLM engine, formats output,
/// and returns the appropriate exit code along with the formatted output.
#[allow(clippy::cast_possible_truncation)]
#[allow(clippy::cast_possible_truncation, clippy::too_many_arguments)]
pub async fn execute_review(
config: &Config,
llm_config: &crate::engine::LLMConfig,
opts: &ReviewOptions,
format: OutputFormat,
progress: &ProgressReporter,
memory_context: Option<&str>,
) -> Result<ReviewResult> {
// 1. Get the diff
let diff = get_diff(opts, config)?;

if diff.trim().is_empty() {
if opts.quiet {
return Ok(ReviewResult {
exit_code: EXIT_OK,
output: String::new(),
});
return Ok(ReviewResult::empty(EXIT_OK, String::new()));
}
let output = format!("{}\n", "No changes to review.".yellow());
return Ok(ReviewResult {
exit_code: EXIT_OK,
output,
});
return Ok(ReviewResult::empty(EXIT_OK, output));
}

// 2. Emit parsing_diff event if progress enabled
Expand All @@ -95,15 +150,20 @@ pub async fn execute_review(
if opts.auto_chunk {
// Auto-chunk mode: split diff and review each chunk
return execute_chunked_review(
config, llm_config, opts, format, progress, &diff, max_size,
config,
llm_config,
opts,
format,
progress,
&diff,
max_size,
memory_context,
)
.await;
}
if config.hook.on_violation == "disallow" {
// Return blocked result instead of error
return Ok(ReviewResult {
exit_code: EXIT_BLOCKED,
output: format!(
return Ok(ReviewResult::empty(EXIT_BLOCKED, format!(
"{}\n",
format!(
"❌ Diff too large ({} chars, max {}). Commit blocked. \
Expand All @@ -114,8 +174,7 @@ pub async fn execute_review(
)
.red()
.bold(),
),
});
)));
}
anyhow::bail!(
"Diff too large ({} chars, max {}). Use --auto-chunk to review in pieces, --base to review a specific branch, or increase hook.max_diff_size.",
Expand Down Expand Up @@ -145,6 +204,7 @@ pub async fn execute_review(
opts.stream,
!opts.no_cache,
opts.quiet,
memory_context,
)
.await
{
Expand Down Expand Up @@ -187,10 +247,7 @@ pub async fn execute_review(
}]
}]
});
return Ok(ReviewResult {
exit_code: EXIT_OK,
output: format!("{warning_output}\n"),
});
return Ok(ReviewResult::empty(EXIT_OK, format!("{warning_output}\n")));
}
return Err(e.into());
}
Expand Down Expand Up @@ -271,10 +328,12 @@ pub async fn execute_review(
);
}

Ok(ReviewResult { exit_code, output })
Ok(ReviewResult::with_issues(
exit_code,
output,
&filtered_response,
))
}

/// Helper to get git commit and branch context for debt tracking.
/// Returns (Some(commit), Some(branch)) if in a git repo, (None, None) otherwise.
/// Results are cached per process to avoid spawning git on every review.
fn get_git_context() -> (Option<String>, Option<String>) {
Expand Down Expand Up @@ -340,7 +399,7 @@ fn get_diff(opts: &ReviewOptions, _config: &Config) -> Result<String> {
///
/// Splits the diff into chunks, reviews each independently, then
/// merges and deduplicates findings.
#[allow(clippy::cast_possible_truncation)]
#[allow(clippy::cast_possible_truncation, clippy::too_many_arguments)]
async fn execute_chunked_review(
config: &Config,
llm_config: &crate::engine::LLMConfig,
Expand All @@ -349,15 +408,13 @@ async fn execute_chunked_review(
progress: &ProgressReporter,
diff: &str,
max_size: usize,
memory_context: Option<&str>,
) -> Result<ReviewResult> {
let chunks = chunker::chunk_diff(diff, max_size);

if chunks.is_empty() {
let output = format!("{}\n", "No changes to review.".yellow());
return Ok(ReviewResult {
exit_code: EXIT_OK,
output,
});
return Ok(ReviewResult::empty(EXIT_OK, output));
}

let total_chunks = chunks.len();
Expand Down Expand Up @@ -414,6 +471,7 @@ async fn execute_chunked_review(
opts.stream,
!opts.no_cache,
opts.quiet,
memory_context,
)
.await
{
Expand Down Expand Up @@ -494,15 +552,15 @@ async fn execute_chunked_review(
if progress.is_enabled() {
progress.error("All chunks failed during chunked review", "chunked_review");
}
return Ok(ReviewResult {
exit_code: EXIT_BLOCKED,
output: format!(
return Ok(ReviewResult::empty(
EXIT_BLOCKED,
format!(
"{}\n",
"❌ All review chunks failed. Check your API key and try again."
.red()
.bold()
),
});
));
}

// Build merged response
Expand Down Expand Up @@ -604,5 +662,9 @@ async fn execute_chunked_review(
);
}

Ok(ReviewResult { exit_code, output })
Ok(ReviewResult::with_issues(
exit_code,
output,
&filtered_response,
))
}
21 changes: 19 additions & 2 deletions src/engine/review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,29 @@ pub async fn review_diff_with_cache(
stream: bool,
use_cache: bool,
quiet: bool,
memory_context: Option<&str>,
) -> std::result::Result<ReviewResponse, CoraError> {
review_diff_inner(config, llm_config, diff, stream, use_cache, quiet).await
review_diff_inner(
config,
llm_config,
diff,
stream,
use_cache,
quiet,
memory_context,
)
.await
}

#[allow(clippy::too_many_arguments)]
async fn review_diff_inner(
config: &Config,
llm_config: &LLMConfig,
diff: &str,
stream: bool,
use_cache: bool,
quiet: bool,
memory_context: Option<&str>,
) -> std::result::Result<ReviewResponse, CoraError> {
debug!(
diff_len = diff.len(),
Expand Down Expand Up @@ -217,7 +229,12 @@ async fn review_diff_inner(
(None, ctx) => ctx,
};

// Call LLM — but preserve deterministic rule findings even on LLM failure
// Inject memory context from Uteke (if --memory flag was used)
let final_context = match (memory_context, final_context) {
(Some(mem), Some(ctx)) => Some(format!("{mem}\n\n{ctx}")),
(Some(mem), None) => Some(mem.to_string()),
(None, ctx) => ctx,
}; // but preserve deterministic rule findings even on LLM failure
let llm_result: Result<ReviewResponse, CoraError> = if stream {
llm::review_diff_stream(
llm_config,
Expand Down
25 changes: 19 additions & 6 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -694,19 +694,26 @@ async fn cmd_review(globals: &GlobalOptions, opts: ReviewOpts) -> Result<i32> {
String::new()
};

// TODO: inject memory_context into review prompt (Phase 2 integration)
let _ = &memory_context;

// Execute the review (returns formatted output)
let result = review::execute_review(
&config,
&llm_config,
&review_opts,
effective_format,
&progress_reporter,
if !memory_context.is_empty() {
Some(memory_context.as_str())
} else {
None
},
)
.await?;

// Show memory status if context was used
if !memory_context.is_empty() && !opts.quiet {
eprintln!("{}", "ℹ Review enriched with Uteke memory context.".cyan());
}

// Print the formatted output (to file if --output-file, else stdout)
if let Some(ref path) = opts.output_file {
std::fs::write(path, &result.output)
Expand All @@ -729,10 +736,16 @@ async fn cmd_review(globals: &GlobalOptions, opts: ReviewOpts) -> Result<i32> {
let project = repo_name_from_git().unwrap_or_else(|| "unknown".to_string());
memory_backend.save_findings(
&project,
0, // TODO: extract from result
"",
&[],
result.total_issues,
&result.severity_summary,
&result.categories,
);
if !opts.quiet {
eprintln!(
"{}",
format!("💾 Saved {} findings to Uteke memory.", result.total_issues).dimmed()
);
}
}

Ok(result.exit_code)
Expand Down
Loading