diff --git a/src/config/loader.rs b/src/config/loader.rs index 49bf9a7..d402128 100644 --- a/src/config/loader.rs +++ b/src/config/loader.rs @@ -339,19 +339,39 @@ pub fn build_llm_config( let cora_base_url = std::env::var("CORA_BASE_URL").ok(); let provider = cora_provider - .or_else(|| auto_preset.map(|p| p.name.to_string())) + .or_else(|| { + if config.provider.provider + != crate::config::schema::Config::default().provider.provider + { + Some(config.provider.provider.clone()) + } else { + auto_preset.map(|p| p.name.to_string()) + } + }) .unwrap_or_else(|| config.provider.provider.clone()); let model = cora_model - .or_else(|| auto_preset.map(|p| p.default_model.to_string())) + .or_else(|| { + if config.provider.model != crate::config::schema::Config::default().provider.model { + Some(config.provider.model.clone()) + } else { + auto_preset.map(|p| p.default_model.to_string()) + } + }) .unwrap_or_else(|| config.provider.model.clone()); let base_url = cora_base_url .or_else(|| { - // Check if the auto-detected preset has a custom URL override - auto_preset.and_then(|p| std::env::var(p.env_url).ok()) + if config.provider.base_url + != crate::config::schema::Config::default().provider.base_url + { + Some(config.provider.base_url.clone()) + } else { + auto_preset + .and_then(|p| std::env::var(p.env_url).ok()) + .or_else(|| auto_preset.map(|p| p.default_base_url.to_string())) + } }) - .or_else(|| auto_preset.map(|p| p.default_base_url.to_string())) .unwrap_or_else(|| config.provider.base_url.clone()); Ok(LLMConfig { diff --git a/src/config/schema.rs b/src/config/schema.rs index 705a0f2..5fdb9db 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -424,6 +424,9 @@ impl CoraFile { if let Some(sa) = &r.static_analysis { config.static_analysis.clone_from(sa); } + if let Some(cc) = &r.context_chain { + config.context_chain.clone_from(cc); + } } if let Some(s) = &self.scan { if let Some(v) = &s.system_prompt { diff --git a/src/engine/debt_tracker.rs b/src/engine/debt_tracker.rs index b8a56dd..3cf25e9 100644 --- a/src/engine/debt_tracker.rs +++ b/src/engine/debt_tracker.rs @@ -211,13 +211,19 @@ fn snapshot_filename(snapshot: &DebtSnapshot) -> String { use std::hash::{Hash, Hasher}; let mut hasher = std::collections::hash_map::DefaultHasher::new(); snapshot.timestamp.hash(&mut hasher); - for (k, v) in &snapshot.findings { - k.hash(&mut hasher); - v.hash(&mut hasher); - } - for (k, v) in &snapshot.categories { - k.hash(&mut hasher); - v.hash(&mut hasher); + { + let mut findings_sorted: Vec<_> = snapshot.findings.iter().collect(); + findings_sorted.sort_by_key(|(k, _)| *k); + for (k, v) in findings_sorted { + k.hash(&mut hasher); + v.hash(&mut hasher); + } + let mut categories_sorted: Vec<_> = snapshot.categories.iter().collect(); + categories_sorted.sort_by_key(|(k, _)| *k); + for (k, v) in categories_sorted { + k.hash(&mut hasher); + v.hash(&mut hasher); + } } let content_hash = format!("{:08x}", hasher.finish()); format!("{date}_{short_hash}_{content_hash}.json") @@ -418,7 +424,12 @@ pub fn aggregate(snapshots: &[DebtSnapshot]) -> DebtReport { } else { previous.iter().map(|s| s.quality_score).sum::() / previous.len() as f64 }; - let quality_score_change = quality_score_avg - previous_avg_quality; + let recent_avg_quality = if recent.is_empty() { + quality_score_avg + } else { + recent.iter().map(|s| s.quality_score).sum::() / recent.len() as f64 + }; + let quality_score_change = recent_avg_quality - previous_avg_quality; // Per-category with trend let mut category_reports = Vec::new(); @@ -429,12 +440,25 @@ pub fn aggregate(snapshots: &[DebtSnapshot]) -> DebtReport { } } + // Build recent category counts separately for accurate delta + let mut recent_categories: HashMap = HashMap::new(); + for snap in recent { + for (cat, count) in &snap.categories { + *recent_categories.entry(cat.clone()).or_insert(0) += count; + } + } + // Collect all unique category names - let mut all_cat_names: Vec = all_categories.keys().cloned().collect(); + let all_cat_names: std::collections::HashSet<&String> = all_categories + .keys() + .chain(recent_categories.keys()) + .chain(previous_categories.keys()) + .collect(); + let mut all_cat_names: Vec = all_cat_names.into_iter().cloned().collect(); all_cat_names.sort(); for cat_name in &all_cat_names { - let count = all_categories.get(cat_name).copied().unwrap_or(0); + let count = recent_categories.get(cat_name).copied().unwrap_or(0); let prev_count = previous_categories.get(cat_name).copied().unwrap_or(0); let change = count as i64 - prev_count as i64; diff --git a/src/engine/review.rs b/src/engine/review.rs index 185c3d3..623bd6d 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -264,13 +264,18 @@ async fn review_diff_inner( Ok(resp) => resp, Err(e) => { // LLM failed — return deterministic findings only (don't silently swallow them) - if !rule_findings.is_empty() || !secrets_findings.is_empty() { + if !rule_findings.is_empty() + || !secrets_findings.is_empty() + || !security_findings.is_empty() + { let n_rules = rule_findings.len(); let n_secrets = secrets_findings.len(); + let n_security = security_findings.len(); debug!( error = %e, rule_findings = n_rules, secrets_findings = n_secrets, + security_findings = n_security, "LLM call failed, returning deterministic findings only" ); let mut all_deterministic = @@ -282,7 +287,7 @@ async fn review_diff_inner( let mut fallback = ReviewResponse { issues: all_deterministic, summary: format!( - "LLM review failed: {e}. Showing {n_rules} rule + {n_secrets} secrets findings." + "LLM review failed: {e}. Showing {n_rules} rule + {n_secrets} secrets + {n_security} security findings." ), tokens_used: None, should_block: false, diff --git a/src/engine/rules/mod.rs b/src/engine/rules/mod.rs index 7b4859e..c8028f8 100644 --- a/src/engine/rules/mod.rs +++ b/src/engine/rules/mod.rs @@ -8,7 +8,17 @@ use tracing::debug; use crate::engine::diff_parser::{DiffLineType, FileChunk, parse_diff}; use crate::engine::rules::types::{RuleFinding, RulesConfig}; -use crate::engine::types::ReviewIssue; +use crate::engine::types::{ReviewIssue, Severity}; + +/// Map severity to a numeric rank for sorting (Critical=4, Major=3, Minor=2, Info=1). +fn severity_rank(sev: Severity) -> u8 { + match sev { + Severity::Critical => 4, + Severity::Major => 3, + Severity::Minor => 2, + Severity::Info => 1, + } +} /// Run all rules (built-in + custom) against parsed diff chunks. /// @@ -79,10 +89,12 @@ pub fn run_rules(chunks: &[FileChunk], config: &RulesConfig) -> Vec } } - // Sort by severity (Critical first) then by file + line + // Sort by severity descending (Critical first) then by file + line findings.sort_by(|a, b| { - a.severity - .cmp(&b.severity) + let rank_a = severity_rank(a.severity); + let rank_b = severity_rank(b.severity); + rank_b + .cmp(&rank_a) .then_with(|| a.file.cmp(&b.file)) .then_with(|| a.line.cmp(&b.line)) }); diff --git a/src/hook/install.rs b/src/hook/install.rs index a62e3f9..5a571ec 100644 --- a/src/hook/install.rs +++ b/src/hook/install.rs @@ -3,24 +3,43 @@ use tracing::debug; use crate::hook::template::HOOK_TEMPLATE; +/// Sentinel marker identifying a cora-managed hook. +const CORA_HOOK_SENTINEL: &str = "# cora-managed-hook"; + /// Install the pre-commit hook to `.git/hooks/pre-commit`. pub fn install_hook() -> Result { let hooks_dir = find_git_hooks_dir()?; let hook_path = hooks_dir.join("pre-commit"); - // Check if a hook already exists + // Check if a hook already exists and handle accordingly if hook_path.is_file() { let existing = std::fs::read_to_string(&hook_path)?; - if existing.contains("cora") { - // Already has cora hook — back up and overwrite - let backup = hooks_dir.join("pre-commit.cora.bak"); - std::fs::copy(&hook_path, &backup)?; - debug!(path = %backup.display(), "backed up existing hook"); + if existing.contains(CORA_HOOK_SENTINEL) { + // Already a cora-managed hook — just overwrite + debug!("existing hook is cora-managed, overwriting"); } else { - // Different hook — back up and append cora - let backup = hooks_dir.join("pre-commit.pre-cora.bak"); + // Non-cora hook — back it up and compose a wrapper + let backup = hooks_dir.join("pre-commit.bak"); std::fs::copy(&hook_path, &backup)?; - debug!(path = %backup.display(), "backed up existing hook"); + debug!(path = %backup.display(), "backed up existing non-cora hook"); + + // Build a wrapper that runs the original hook first, then cora + let wrapper = format!( + "{existing}\n\n# cora-managed-hook — the section below was added by `cora hook install`\n{HOOK_TEMPLATE}" + ); + std::fs::write(&hook_path, &wrapper) + .with_context(|| format!("failed to write {}", hook_path.display()))?; + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let perms = std::fs::Permissions::from_mode(0o755); + std::fs::set_permissions(&hook_path, perms)?; + } + + let path_str = hook_path.display().to_string(); + debug!(path = %path_str, "installed pre-commit hook (wrapped existing)"); + return Ok(path_str); } }