TDD: fix 4 more bugs — UTF-8 truncation, date comparison, weights mismatch, FPR formula

- offline.rs: truncate_to_tokens panicked on multi-byte UTF-8 chars (€ at byte boundary)
- git_history.rs: last_modified used string comparison ("Wed" > "Mon") instead of parsing dates
- eval_benchmarks.rs: AggregateMetrics silently dropped results when weights length != results length
- eval_benchmarks.rs: FPR used FP/(FP+TP) instead of FP/(FP+TN), added total_tn tracking

330 tests, 0 warnings, 0 clippy issues.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: haasonsaas

src/core/eval_benchmarks.rs

@@ -200,6 +200,7 @@ pub struct AggregateMetrics {
     pub total_tp: usize,
     pub total_fp: usize,
     pub total_fn: usize,
+    pub total_tn: usize,
     pub micro_precision: f32,
     pub micro_recall: f32,
     pub micro_f1: f32,
@@ -218,6 +219,7 @@ impl AggregateMetrics {
         let total_tp: usize = results.iter().map(|r| r.true_positives).sum();
         let total_fp: usize = results.iter().map(|r| r.false_positives).sum();
         let total_fn: usize = results.iter().map(|r| r.false_negatives).sum();
+        let total_tn: usize = results.iter().map(|r| r.true_negatives).sum();
 
         let micro_precision = if total_tp + total_fp > 0 {
             total_tp as f32 / (total_tp + total_fp) as f32
@@ -241,16 +243,22 @@ impl AggregateMetrics {
         let macro_f1: f32 = results.iter().map(|r| r.f1).sum::<f32>() / n;
 
         let weighted_score = if let Some(ws) = weights {
-            let total_weight: f32 = ws.iter().sum();
-            if total_weight > 0.0 {
-                results
-                    .iter()
-                    .zip(ws.iter())
-                    .map(|(r, w)| r.f1 * w)
-                    .sum::<f32>()
-                    / total_weight
-            } else {
+            if ws.len() != results.len() {
+                // Length mismatch: fall back to macro_f1 rather than silently
+                // dropping results via zip truncation
                 macro_f1
+            } else {
+                let total_weight: f32 = ws.iter().sum();
+                if total_weight > 0.0 {
+                    results
+                        .iter()
+                        .zip(ws.iter())
+                        .map(|(r, w)| r.f1 * w)
+                        .sum::<f32>()
+                        / total_weight
+                } else {
+                    macro_f1
+                }
             }
         } else {
             macro_f1
@@ -261,6 +269,7 @@ impl AggregateMetrics {
             total_tp,
             total_fp,
             total_fn,
+            total_tn,
             micro_precision,
             micro_recall,
             micro_f1,
@@ -304,9 +313,9 @@ pub fn evaluate_against_thresholds(
         ));
     }
 
-    let fpr = if result.aggregate.total_fp + result.aggregate.total_tp > 0 {
+    let fpr = if result.aggregate.total_fp + result.aggregate.total_tn > 0 {
         result.aggregate.total_fp as f32
-            / (result.aggregate.total_fp + result.aggregate.total_tp) as f32
+            / (result.aggregate.total_fp + result.aggregate.total_tn) as f32
     } else {
         0.0
     };
@@ -893,6 +902,70 @@ mod tests {
         assert!(by_cat.is_empty());
     }
 
+    // BUG: AggregateMetrics::compute silently drops results when weights.len() < results.len()
+    #[test]
+    fn test_aggregate_weights_length_mismatch() {
+        let r1 = FixtureResult::compute("a", 2, 0, 2, 0, 0); // f1 = 1.0
+        let r2 = FixtureResult::compute("b", 2, 0, 0, 0, 2); // f1 = 0.0
+        let r3 = FixtureResult::compute("c", 2, 0, 2, 0, 0); // f1 = 1.0
+
+        // Only 2 weights for 3 results — r3 (f1=1.0) is silently dropped
+        let weights = vec![1.0, 1.0];
+        let agg = AggregateMetrics::compute(&[&r1, &r2, &r3], Some(&weights));
+
+        // macro_f1 = (1.0 + 0.0 + 1.0) / 3 = 0.667
+        // With mismatched weights, zip truncates: (1.0*1 + 0.0*1) / 2 = 0.5
+        // The third fixture is silently ignored, deflating the score!
+        // On mismatch, should fall back to macro_f1 rather than give wrong answer.
+        assert!(
+            (agg.weighted_score - agg.macro_f1).abs() < 0.05,
+            "Mismatched weights should fall back to macro_f1, got weighted={:.3} vs macro={:.3}",
+            agg.weighted_score, agg.macro_f1
+        );
+    }
+
+    // BUG: FPR formula uses FP/(FP+TP) which equals 1-precision, making it redundant.
+    // Should track total_tn and use FP/(FP+TN) for real false positive rate.
+    #[test]
+    fn test_fpr_uses_true_negatives() {
+        let r1 = FixtureResult::compute("a", 10, 10, 10, 0, 1);
+        // r1: tp=10, fp=1, fn=0, tn=10
+        let r2 = FixtureResult::compute("b", 0, 10, 0, 0, 0);
+        // r2: tp=0, fp=0, fn=0, tn=10
+
+        let agg = AggregateMetrics::compute(&[&r1, &r2], None);
+        // total_tp=10, total_fp=1, total_fn=0
+        // If total_tn were tracked: total_tn=20
+
+        let result = BenchmarkResult {
+            suite_name: "test".to_string(),
+            fixture_results: vec![r1, r2],
+            aggregate: agg,
+            by_category: HashMap::new(),
+            by_difficulty: HashMap::new(),
+            threshold_pass: true,
+            threshold_failures: vec![],
+            timestamp: "2024-01-01".to_string(),
+        };
+
+        // With real FPR = FP/(FP+TN) = 1/(1+20) = 0.048 → passes 0.06 threshold
+        // With buggy FPR = FP/(FP+TP) = 1/(1+10) = 0.091 → fails 0.06 threshold
+        let thresholds = BenchmarkThresholds {
+            max_false_positive_rate: 0.06,
+            min_precision: 0.0,
+            min_recall: 0.0,
+            min_f1: 0.0,
+            min_weighted_score: 0.0,
+        };
+
+        let (pass, failures) = evaluate_against_thresholds(&result, &thresholds);
+        assert!(
+            pass,
+            "FPR should be ~0.048 (FP/(FP+TN)), not 0.091 (FP/(FP+TP)): {:?}",
+            failures
+        );
+    }
+
     // BUG: compare_results never populates improvements vector
     #[test]
     fn test_compare_results_detects_improvements() {

src/core/git_history.rs

@@ -119,11 +119,10 @@ impl GitHistoryAnalyzer {
                     info.bug_fix_count += 1;
                 }
 
-                if info
-                    .last_modified
-                    .as_ref()
-                    .is_none_or(|d| entry.date > *d)
-                {
+                if info.last_modified.as_ref().is_none_or(|d| {
+                    parse_date_for_comparison(&entry.date)
+                        > parse_date_for_comparison(d)
+                }) {
                     info.last_modified = Some(entry.date.clone());
                 }
             }
@@ -288,6 +287,55 @@ impl GitHistoryAnalyzer {
     }
 }
 
+/// Parse a date string into a comparable tuple (year, month, day, time).
+/// Handles both git default format ("Wed Jan 1 12:00:00 2025 +0000")
+/// and ISO format ("2025-01-01 12:00:00 +0000").
+fn parse_date_for_comparison(date: &str) -> (i32, u32, u32, String) {
+    let parts: Vec<&str> = date.split_whitespace().collect();
+
+    // ISO format: "2025-01-01 ..."
+    if parts
+        .first()
+        .is_some_and(|p| p.contains('-') && p.len() == 10)
+    {
+        // ISO dates sort correctly as strings, but parse for consistency
+        let date_parts: Vec<&str> = parts[0].split('-').collect();
+        if date_parts.len() == 3 {
+            let year = date_parts[0].parse::<i32>().unwrap_or(0);
+            let month = date_parts[1].parse::<u32>().unwrap_or(0);
+            let day = date_parts[2].parse::<u32>().unwrap_or(0);
+            let time = parts.get(1).unwrap_or(&"").to_string();
+            return (year, month, day, time);
+        }
+    }
+
+    // Git default format: "Wed Jan 1 12:00:00 2025 +0000"
+    if parts.len() >= 5 {
+        let month = match parts[1] {
+            "Jan" => 1,
+            "Feb" => 2,
+            "Mar" => 3,
+            "Apr" => 4,
+            "May" => 5,
+            "Jun" => 6,
+            "Jul" => 7,
+            "Aug" => 8,
+            "Sep" => 9,
+            "Oct" => 10,
+            "Nov" => 11,
+            "Dec" => 12,
+            _ => 0,
+        };
+        let day = parts[2].parse::<u32>().unwrap_or(0);
+        let year = parts[4].parse::<i32>().unwrap_or(0);
+        let time = parts[3].to_string();
+        return (year, month, day, time);
+    }
+
+    // Fallback: use the raw string (best effort)
+    (0, 0, 0, date.to_string())
+}
+
 /// Detect if a commit message indicates a bug fix.
 fn is_bug_fix_commit(message: &str) -> bool {
     let lower = message.to_lowercase();
@@ -640,6 +688,46 @@ Date:   Tue Jan 2 12:00:00 2024 +0000
         );
     }
 
+    // BUG: last_modified uses string comparison which is lexicographic, not chronological.
+    // "Wed Jan 1" > "Mon Feb 1" because 'W' > 'M', but Feb 1 is newer than Jan 1.
+    #[test]
+    fn test_last_modified_chronological_order() {
+        let mut analyzer = GitHistoryAnalyzer::new();
+        analyzer.ingest_log(vec![
+            GitLogEntry {
+                hash: "newer".to_string(),
+                author: "alice".to_string(),
+                date: "Mon Feb 3 12:00:00 2025 +0000".to_string(), // NEWER
+                message: "newer commit".to_string(),
+                files_changed: vec![FileChange {
+                    file_path: PathBuf::from("f.rs"),
+                    lines_added: 1,
+                    lines_removed: 0,
+                }],
+            },
+            GitLogEntry {
+                hash: "older".to_string(),
+                author: "bob".to_string(),
+                date: "Wed Jan 1 12:00:00 2025 +0000".to_string(), // OLDER
+                message: "older commit".to_string(),
+                files_changed: vec![FileChange {
+                    file_path: PathBuf::from("f.rs"),
+                    lines_added: 1,
+                    lines_removed: 0,
+                }],
+            },
+        ]);
+
+        let info = analyzer.file_info(Path::new("f.rs")).unwrap();
+        // Feb 3 is newer than Jan 1, so last_modified should contain "Feb"
+        // Bug: string comparison "Wed Jan..." > "Mon Feb..." because 'W' > 'M'
+        assert!(
+            info.last_modified.as_ref().unwrap().contains("Feb"),
+            "last_modified should be Feb 3 (newer), got: {}",
+            info.last_modified.as_ref().unwrap()
+        );
+    }
+
     // BUG: commit_count accumulates but distinct_authors only counts latest batch
     #[test]
     fn test_cumulative_commit_count() {

src/core/offline.rs

@@ -305,7 +305,12 @@ fn truncate_to_tokens(text: &str, max_tokens: usize) -> String {
         return text.to_string();
     }
 
-    let mut truncated = text[..max_chars].to_string();
+    // Find a valid UTF-8 char boundary at or before max_chars
+    let mut end = max_chars;
+    while end > 0 && !text.is_char_boundary(end) {
+        end -= 1;
+    }
+    let mut truncated = text[..end].to_string();
     truncated.push_str("\n[Truncated for local model context window]");
     truncated
 }
@@ -648,6 +653,21 @@ mod tests {
         );
     }
 
+    // BUG: truncate_to_tokens slices at byte offset without char boundary check
+    #[test]
+    fn test_truncate_to_tokens_utf8_safety() {
+        // '€' is 3 bytes (U+20AC). With context_window=504, budget=4,
+        // system_budget=1, max_chars=4. "€€" is 6 bytes.
+        // text[..4] lands inside the second '€' (bytes 3-5), panicking.
+        let system = "€€"; // 6 bytes
+        let user = "short";
+        // context_window=504 → budget=4, system_budget=1, user_budget=3
+        // system max_chars=4, which falls mid-char in "€€"
+        let (sys, _usr) = optimize_prompt_for_local(system, user, 504);
+        // Should not panic; result must be valid UTF-8
+        assert!(!sys.is_empty() || sys.contains("Truncated") || sys.is_empty());
+    }
+
     // BUG: validate() doesn't check for obviously wrong base_url formats
     #[test]
     fn test_validate_rejects_invalid_url() {