From 752558629f911040fd6d6e1ff0cf54bda459bbcf Mon Sep 17 00:00:00 2001 From: Brummel Date: Mon, 18 May 2026 19:40:08 +0200 Subject: [PATCH] fix: deterministic fusion tie-order + seeded random eval sampling Co-Authored-By: Claude Sonnet 4.6 --- src/bin/alpha_id.rs | 12 ++++++++++-- src/fusion.rs | 4 ++-- tests/eval_compare_tests.rs | 17 ++++++++++++++++ tests/fusion_tests.rs | 39 +++++++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 4 deletions(-) diff --git a/src/bin/alpha_id.rs b/src/bin/alpha_id.rs index 26e3130..a781f8c 100644 --- a/src/bin/alpha_id.rs +++ b/src/bin/alpha_id.rs @@ -37,12 +37,20 @@ enum Cmd { fn run_eval(p: &Pipeline, m: Mode, cases: usize, seed: u64) -> (f64, f64) { use alpha_id::eval::{inject_errors, recall_at_k}; - let billable: Vec<_> = p.entries.iter().enumerate() + use rand::SeedableRng; + use rand::seq::SliceRandom; + let all_billable: Vec<_> = p.entries.iter().enumerate() .filter(|(_, e)| e.valid) .filter(|(_, e)| alpha_id::corpus::primary_code(e) .and_then(|c| p.meta.get(c)) .map(|mt| mt.para295 == "P").unwrap_or(false)) - .take(cases).collect(); + .collect(); + let mut rng = rand::rngs::StdRng::seed_from_u64(seed); + let n = cases.min(all_billable.len()); + let billable: Vec<_> = all_billable + .choose_multiple(&mut rng, n) + .cloned() + .collect(); let mut sum = 0.0; for (k, (_, e)) in billable.iter().enumerate() { let code = alpha_id::corpus::primary_code(e).unwrap().to_string(); diff --git a/src/fusion.rs b/src/fusion.rs index 9cb18ab..67967fb 100644 --- a/src/fusion.rs +++ b/src/fusion.rs @@ -12,7 +12,7 @@ pub fn rrf_merge(a: &[usize], b: &[usize], k: usize) -> Vec { *score.entry(id).or_default() += 1.0 / (k as f64 + rank as f64 + 1.0); } let mut v: Vec<(usize, f64)> = score.into_iter().collect(); - v.sort_by(|x, y| y.1.partial_cmp(&x.1).unwrap()); + v.sort_by(|x, y| f64::total_cmp(&y.1, &x.1).then(x.0.cmp(&y.0))); v.into_iter().map(|(id, _)| id).collect() } @@ -41,7 +41,7 @@ pub fn cross_segment_dedupe(cands: Vec) -> Vec<(DedupCandidate, f32)> .into_values() .map(|mut d| { d.source_segments.sort(); let s = d.best_score; (d, s) }) .collect(); - out.sort_by(|a, b| b.1.partial_cmp(&a.1).unwrap()); + out.sort_by(|a, b| f32::total_cmp(&b.1, &a.1).then(a.0.icd_code.cmp(&b.0.icd_code))); out } diff --git a/tests/eval_compare_tests.rs b/tests/eval_compare_tests.rs index bbf212d..02977f1 100644 --- a/tests/eval_compare_tests.rs +++ b/tests/eval_compare_tests.rs @@ -10,3 +10,20 @@ fn eval_mode_c_runs_and_reports_billable_recall() { let s = String::from_utf8_lossy(&out.stdout); assert!(s.contains("BillableRecall@10=")); } + +#[test] +fn eval_is_deterministic_same_seed() { + let run = || { + let out = Command::new(env!("CARGO_BIN_EXE_alpha-id")) + .args(["eval", "--mode", "c", "--cases", "40", "--seed", "42"]) + .arg("--config").arg("config/default.toml") + .output().unwrap(); + assert!(out.status.success(), "stderr: {}", String::from_utf8_lossy(&out.stderr)); + String::from_utf8(out.stdout).unwrap() + }; + let first = run(); + let second = run(); + assert!(!first.is_empty(), "eval produced no output"); + assert_eq!(first, second, + "eval output differed between runs with identical args (seed=42):\n run1: {first:?}\n run2: {second:?}"); +} diff --git a/tests/fusion_tests.rs b/tests/fusion_tests.rs index 5a70d5c..479a3c4 100644 --- a/tests/fusion_tests.rs +++ b/tests/fusion_tests.rs @@ -29,3 +29,42 @@ fn cross_segment_keeps_max_score_and_all_sources() { assert_eq!(merged[0].1, 0.9); // max assert_eq!(merged[0].0.source_segments, vec![0, 2]); // all sources } + +#[test] +fn fusion_is_deterministic_across_runs() { + // Build inputs with deliberate score ties so HashMap iteration order would + // matter without the deterministic tiebreaker. + // rrf_merge: entries 5 and 10 appear at symmetric positions in both lists, + // so their RRF scores are identical. The tiebreaker must put 5 before 10. + let lex = vec![5usize, 10, 20]; + let sem = vec![10usize, 5, 30]; + let first = rrf_merge(&lex, &sem, 60); + for _ in 0..4 { + let run = rrf_merge(&lex, &sem, 60); + assert_eq!(first, run, "rrf_merge output changed between runs"); + } + // Verify the tiebreaker: 5 and 10 have equal scores (symmetric positions); + // ascending id means 5 should come before 10. + let pos5 = first.iter().position(|&x| x == 5).expect("5 missing"); + let pos10 = first.iter().position(|&x| x == 10).expect("10 missing"); + assert!(pos5 < pos10, "tiebreaker should put smaller id first: got pos5={pos5} pos10={pos10}"); + + // cross_segment_dedupe: two codes with equal score — lexicographic tiebreaker. + let cands = vec![ + cand("Z99.89", 0, Some(0.7)), + cand("A00.0", 1, Some(0.7)), + cand("M50.00", 2, Some(0.5)), + ]; + let first_dedup = cross_segment_dedupe(cands.clone()); + for _ in 0..4 { + let run = cross_segment_dedupe(cands.clone()); + let ids: Vec<&str> = run.iter().map(|(d, _)| d.icd_code.as_str()).collect(); + let first_ids: Vec<&str> = first_dedup.iter().map(|(d, _)| d.icd_code.as_str()).collect(); + assert_eq!(first_ids, ids, "cross_segment_dedupe output changed between runs"); + } + // Verify tiebreaker: A00.0 < Z99.89 lexicographically, so A00.0 first among ties. + assert_eq!(first_dedup[0].0.icd_code, "A00.0", + "tiebreaker should put A00.0 before Z99.89 (both score 0.7)"); + assert_eq!(first_dedup[1].0.icd_code, "Z99.89"); + assert_eq!(first_dedup[2].0.icd_code, "M50.00"); +}