fix: resolve silent data corruption bugs (P1)

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 {

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 {

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::<f64>() / 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::<f64>() / 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<String, usize> = 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<String> = 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<String> = 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;
 

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,

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<RuleFinding>
         }
     }
 
-    // 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))
     });

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<String> {
     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);
         }
     }