use std::collections::HashSet;
use std::path::{Path, PathBuf};
use crate::AnalysisError;
use crate::diff_parse::parse_unified_diff;
use crate::impact::{
analyze_diff_impact_with_graph, compute_risk_batch, map_hunks_to_functions, CallerDetail,
DiffTestInfo, RiskLevel, RiskScore,
};
use crate::note::path_matches_mention;
use crate::Store;
#[derive(Debug, Clone, serde::Serialize)]
pub struct ReviewResult {
pub changed_functions: Vec<ReviewedFunction>,
pub affected_callers: Vec<CallerDetail>,
pub affected_tests: Vec<DiffTestInfo>,
pub relevant_notes: Vec<ReviewNoteEntry>,
pub risk_summary: RiskSummary,
pub stale_warning: Option<Vec<String>>,
#[serde(skip_serializing_if = "Vec::is_empty")]
pub warnings: Vec<String>,
}
#[derive(Debug, Clone, serde::Serialize)]
pub struct ReviewedFunction {
pub name: String,
#[serde(serialize_with = "crate::serialize_path_normalized")]
pub file: PathBuf,
pub line_start: u32,
pub risk: RiskScore,
}
#[derive(Debug, Clone, serde::Serialize)]
pub struct ReviewNoteEntry {
pub text: String,
pub sentiment: f32,
pub matching_files: Vec<String>,
}
#[derive(Debug, Clone, serde::Serialize)]
pub struct RiskSummary {
pub high: usize,
pub medium: usize,
pub low: usize,
pub overall: RiskLevel,
}
pub fn review_diff(
store: &Store,
diff_text: &str,
root: &Path,
) -> Result<Option<ReviewResult>, AnalysisError> {
let _span = tracing::info_span!("review_diff").entered();
let mut warnings: Vec<String> = Vec::new();
let hunks = parse_unified_diff(diff_text);
if hunks.is_empty() {
return Ok(None);
}
let changed = map_hunks_to_functions(store, &hunks);
if changed.is_empty() {
return Ok(None);
}
let graph = store.get_call_graph()?;
let test_chunks = store.find_test_chunks()?;
let impact = analyze_diff_impact_with_graph(store, changed, &graph, &test_chunks, root)?;
let changed_names: Vec<&str> = impact
.changed_functions
.iter()
.map(|f| f.name.as_str())
.collect();
let risk_scores = compute_risk_batch(&changed_names, &graph, &test_chunks);
let reviewed_functions: Vec<ReviewedFunction> = impact
.changed_functions
.iter()
.zip(risk_scores)
.map(|(cf, risk)| ReviewedFunction {
name: cf.name.clone(),
file: cf.file.clone(),
line_start: cf.line_start,
risk,
})
.collect();
let changed_file_strings: Vec<String> = impact
.changed_functions
.iter()
.map(|f| f.file.to_string_lossy().into_owned())
.collect();
let changed_files: HashSet<&str> = changed_file_strings.iter().map(|s| s.as_str()).collect();
let relevant_notes = match match_notes(store, &changed_files) {
Ok(notes) => notes,
Err(e) => {
tracing::warn!(error = %e, "Failed to load notes for review");
warnings.push(format!("Failed to load notes for review: {e}"));
Vec::new()
}
};
let origins: Vec<&str> = changed_files.iter().copied().collect();
let stale_warning = match store.check_origins_stale(&origins, root) {
Ok(stale) if stale.is_empty() => None,
Ok(stale) => Some(stale.into_iter().collect()),
Err(e) => {
tracing::warn!(error = %e, "Failed to check staleness");
warnings.push(format!("Failed to check staleness: {e}"));
None
}
};
let risk_summary = build_risk_summary(&reviewed_functions);
let affected_callers: Vec<CallerDetail> = impact
.all_callers
.into_iter()
.map(|mut c| {
c.file = PathBuf::from(crate::rel_display(&c.file, root));
c
})
.collect();
let affected_tests: Vec<DiffTestInfo> = impact
.all_tests
.into_iter()
.map(|mut t| {
t.file = PathBuf::from(crate::rel_display(&t.file, root));
t
})
.collect();
Ok(Some(ReviewResult {
changed_functions: reviewed_functions,
affected_callers,
affected_tests,
relevant_notes,
risk_summary,
stale_warning,
warnings,
}))
}
fn match_notes(
store: &Store,
changed_files: &HashSet<&str>,
) -> Result<Vec<ReviewNoteEntry>, AnalysisError> {
let _span = tracing::info_span!("match_notes").entered();
let notes = store.list_notes_summaries()?;
Ok(notes
.into_iter()
.filter_map(|note| {
let matching: Vec<String> = changed_files
.iter()
.filter(|file| {
note.mentions
.iter()
.any(|mention| path_matches_mention(file, mention))
})
.map(|f| f.to_string())
.collect();
if matching.is_empty() {
None
} else {
Some(ReviewNoteEntry {
text: note.text,
sentiment: note.sentiment,
matching_files: matching,
})
}
})
.collect())
}
fn build_risk_summary(functions: &[ReviewedFunction]) -> RiskSummary {
let high = functions
.iter()
.filter(|f| f.risk.risk_level == RiskLevel::High)
.count();
let medium = functions
.iter()
.filter(|f| f.risk.risk_level == RiskLevel::Medium)
.count();
let low = functions
.iter()
.filter(|f| f.risk.risk_level == RiskLevel::Low)
.count();
let overall = if high > 0 {
RiskLevel::High
} else if medium > 0 {
RiskLevel::Medium
} else {
RiskLevel::Low
};
RiskSummary {
high,
medium,
low,
overall,
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::impact::RiskScore;
use crate::note::path_matches_mention;
fn mock_reviewed(name: &str, level: RiskLevel) -> ReviewedFunction {
ReviewedFunction {
name: name.to_string(),
file: PathBuf::from("src/lib.rs"),
line_start: 1,
risk: RiskScore {
risk_level: level,
blast_radius: level,
caller_count: 0,
test_count: 0,
test_ratio: 0.0,
score: 0.0,
},
}
}
#[test]
fn test_risk_summary_empty() {
let summary = build_risk_summary(&[]);
assert_eq!(summary.high, 0);
assert_eq!(summary.medium, 0);
assert_eq!(summary.low, 0);
assert!(matches!(summary.overall, RiskLevel::Low));
}
#[test]
fn test_risk_summary_all_high() {
let funcs = vec![
mock_reviewed("a", RiskLevel::High),
mock_reviewed("b", RiskLevel::High),
];
let summary = build_risk_summary(&funcs);
assert_eq!(summary.high, 2);
assert!(matches!(summary.overall, RiskLevel::High));
}
#[test]
fn test_risk_summary_mixed() {
let funcs = vec![
mock_reviewed("a", RiskLevel::Low),
mock_reviewed("b", RiskLevel::Medium),
mock_reviewed("c", RiskLevel::Low),
];
let summary = build_risk_summary(&funcs);
assert_eq!(summary.high, 0);
assert_eq!(summary.medium, 1);
assert_eq!(summary.low, 2);
assert!(matches!(summary.overall, RiskLevel::Medium));
}
#[test]
fn test_path_matches_exact() {
assert!(path_matches_mention("src/gather.rs", "src/gather.rs"));
}
#[test]
fn test_path_matches_suffix_component_boundary() {
assert!(path_matches_mention("src/gather.rs", "gather.rs"));
}
#[test]
fn test_path_does_not_match_mid_component_suffix() {
assert!(!path_matches_mention("src/gatherer.rs", "gather.rs"));
}
#[test]
fn test_path_matches_prefix_component_boundary() {
assert!(path_matches_mention("src/store/chunks.rs", "src/store"));
}
#[test]
fn test_path_does_not_match_non_prefix_path() {
assert!(!path_matches_mention("my_src/store/chunks.rs", "src/store"));
}
#[test]
fn test_path_does_not_match_longer_mention() {
assert!(!path_matches_mention("store.rs", "src/store.rs"));
}
#[test]
fn test_match_notes_returns_matching_notes() {
use crate::store::ModelInfo;
use tempfile::TempDir;
let dir = TempDir::new().unwrap();
let db_path = dir.path().join("index.db");
let store = crate::Store::open(&db_path).unwrap();
store.init(&ModelInfo::default()).unwrap();
store
.replace_notes_for_file(
&[crate::note::Note {
id: "note:gather".to_string(),
text: "gather needs review".to_string(),
sentiment: -0.5,
mentions: vec!["gather.rs".to_string()],
}],
&dir.path().join("notes.toml"),
0,
)
.unwrap();
store
.replace_notes_for_file(
&[crate::note::Note {
id: "note:other".to_string(),
text: "unrelated note".to_string(),
sentiment: 0.0,
mentions: vec!["src/other.rs".to_string()],
}],
&dir.path().join("notes2.toml"),
0,
)
.unwrap();
let changed_files: HashSet<&str> =
["src/gather.rs", "src/index.rs"].iter().copied().collect();
let notes = match_notes(&store, &changed_files).unwrap();
assert_eq!(
notes.len(),
1,
"Expected exactly 1 matching note for changed files {:?}, got: {:?}",
changed_files,
notes.iter().map(|n| &n.text).collect::<Vec<_>>()
);
assert_eq!(notes[0].text, "gather needs review");
assert!(
notes[0]
.matching_files
.contains(&"src/gather.rs".to_string()),
"matching_files should include src/gather.rs, got {:?}",
notes[0].matching_files
);
}
#[test]
fn test_match_notes_directory_prefix_match() {
use crate::store::ModelInfo;
use tempfile::TempDir;
let dir = TempDir::new().unwrap();
let db_path = dir.path().join("index.db");
let store = crate::Store::open(&db_path).unwrap();
store.init(&ModelInfo::default()).unwrap();
store
.replace_notes_for_file(
&[crate::note::Note {
id: "note:store-dir".to_string(),
text: "store module has schema issues".to_string(),
sentiment: -0.5,
mentions: vec!["src/store".to_string()],
}],
&dir.path().join("notes.toml"),
0,
)
.unwrap();
let changed_files: HashSet<&str> = ["src/store/chunks.rs"].iter().copied().collect();
let notes = match_notes(&store, &changed_files).unwrap();
assert_eq!(
notes.len(),
1,
"Directory-prefix mention 'src/store' should match 'src/store/chunks.rs'"
);
assert!(notes[0]
.matching_files
.contains(&"src/store/chunks.rs".to_string()));
}
#[test]
fn test_match_notes_no_match() {
use crate::store::ModelInfo;
use tempfile::TempDir;
let dir = TempDir::new().unwrap();
let db_path = dir.path().join("index.db");
let store = crate::Store::open(&db_path).unwrap();
store.init(&ModelInfo::default()).unwrap();
store
.replace_notes_for_file(
&[crate::note::Note {
id: "note:unrelated".to_string(),
text: "unrelated note".to_string(),
sentiment: 0.0,
mentions: vec!["src/unrelated.rs".to_string()],
}],
&dir.path().join("notes.toml"),
0,
)
.unwrap();
let changed_files: HashSet<&str> = ["src/other.rs"].iter().copied().collect();
let notes = match_notes(&store, &changed_files).unwrap();
assert!(
notes.is_empty(),
"Expected no matches for unrelated changed file, got: {:?}",
notes.iter().map(|n| &n.text).collect::<Vec<_>>()
);
}
}