From 229412e40f063c7e9aec7d388dfbd0b1bf2537ec Mon Sep 17 00:00:00 2001 From: "Anaz S. Aji" Date: Thu, 11 Jun 2026 11:31:54 +0700 Subject: [PATCH] fix: complete Uteke memory Phase 2+3 (#259) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 — Inject memory context into LLM prompt: - memory_context passed through execute_review → engine::review_diff - Appended to final_context before LLM call - Shows 'ℹ Review enriched with Uteke memory context.' when used Phase 3 — Extract real findings from ReviewResult: - ReviewResult extended with total_issues, severity_summary, categories - Helper methods: empty() for no-issue results, with_issues() for real data - save_findings() now receives actual counts/severities/categories - Shows '💾 Saved N findings to Uteke memory.' after --learn 506 tests pass, clippy clean. --- src/commands/review.rs | 128 ++++++++++++++++++++++++++++++----------- src/engine/review.rs | 21 ++++++- src/main.rs | 25 ++++++-- 3 files changed, 133 insertions(+), 41 deletions(-) diff --git a/src/commands/review.rs b/src/commands/review.rs index a8eaadc..53a9fc9 100644 --- a/src/commands/review.rs +++ b/src/commands/review.rs @@ -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, +} + +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 = + std::collections::HashMap::new(); + let mut cat_set: std::collections::HashSet = 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 = 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 { // 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 @@ -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. \ @@ -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.", @@ -145,6 +204,7 @@ pub async fn execute_review( opts.stream, !opts.no_cache, opts.quiet, + memory_context, ) .await { @@ -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()); } @@ -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, Option) { @@ -340,7 +399,7 @@ fn get_diff(opts: &ReviewOptions, _config: &Config) -> Result { /// /// 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, @@ -349,15 +408,13 @@ async fn execute_chunked_review( progress: &ProgressReporter, diff: &str, max_size: usize, + memory_context: Option<&str>, ) -> Result { 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(); @@ -414,6 +471,7 @@ async fn execute_chunked_review( opts.stream, !opts.no_cache, opts.quiet, + memory_context, ) .await { @@ -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 @@ -604,5 +662,9 @@ async fn execute_chunked_review( ); } - Ok(ReviewResult { exit_code, output }) + Ok(ReviewResult::with_issues( + exit_code, + output, + &filtered_response, + )) } diff --git a/src/engine/review.rs b/src/engine/review.rs index 34ded77..81b8c7c 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -60,10 +60,21 @@ pub async fn review_diff_with_cache( stream: bool, use_cache: bool, quiet: bool, + memory_context: Option<&str>, ) -> std::result::Result { - 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, @@ -71,6 +82,7 @@ async fn review_diff_inner( stream: bool, use_cache: bool, quiet: bool, + memory_context: Option<&str>, ) -> std::result::Result { debug!( diff_len = diff.len(), @@ -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 = if stream { llm::review_diff_stream( llm_config, diff --git a/src/main.rs b/src/main.rs index e9ab4b3..d9b6b21 100644 --- a/src/main.rs +++ b/src/main.rs @@ -694,9 +694,6 @@ async fn cmd_review(globals: &GlobalOptions, opts: ReviewOpts) -> Result { 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, @@ -704,9 +701,19 @@ async fn cmd_review(globals: &GlobalOptions, opts: ReviewOpts) -> Result { &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) @@ -729,10 +736,16 @@ async fn cmd_review(globals: &GlobalOptions, opts: ReviewOpts) -> Result { 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)