From 98245780da5464971e6fb76dfa7e8633acc8f566 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 2 Jan 2026 16:07:20 +0000 Subject: [PATCH 1/2] Fix mixed Int/Float comparisons in eldritch-core Previously, mixed type comparisons (e.g., `Int` vs `Float`) fell back to ordering by type discriminant because they are different types. This caused `max(1, 0.8)` to return `0.8` because `Int` (2) < `Float` (3). This change adds a special case in `Value::cmp_helper` to handle `Int` vs `Float` and `Float` vs `Int` by converting the integer to a float and using `total_cmp`. This ensures numerical correctness for `max`, `min`, and general comparisons. Added regression test `tests/max_min_mixed.rs`. --- .../lib/eldritchv2/eldritch-core/src/ast.rs | 21 ++++++ .../eldritch-core/tests/max_min_mixed.rs | 64 +++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 implants/lib/eldritchv2/eldritch-core/tests/max_min_mixed.rs diff --git a/implants/lib/eldritchv2/eldritch-core/src/ast.rs b/implants/lib/eldritchv2/eldritch-core/src/ast.rs index 3a9d6a42e..b18c60905 100644 --- a/implants/lib/eldritchv2/eldritch-core/src/ast.rs +++ b/implants/lib/eldritchv2/eldritch-core/src/ast.rs @@ -243,6 +243,27 @@ impl Value { visited.insert(pair); } + // Special case for Int vs Float comparison to make them behave numerically + // This must be done before discriminant check because they have different discriminants + // but we want them to compare by value. + match (self, other) { + (Value::Int(i), Value::Float(f)) => { + if p1 != 0 && p2 != 0 { + let pair = (p1, p2); + visited.remove(&pair); + } + return (*i as f64).total_cmp(f); + } + (Value::Float(f), Value::Int(i)) => { + if p1 != 0 && p2 != 0 { + let pair = (p1, p2); + visited.remove(&pair); + } + return f.total_cmp(&(*i as f64)); + } + _ => {} + } + // Define an ordering between types: // None < Bool < Int < Float < String < Bytes < List < Tuple < Dict < Set < Function < Native < Bound < Foreign let self_discriminant = self.discriminant_value(); diff --git a/implants/lib/eldritchv2/eldritch-core/tests/max_min_mixed.rs b/implants/lib/eldritchv2/eldritch-core/tests/max_min_mixed.rs new file mode 100644 index 000000000..4e5731fb6 --- /dev/null +++ b/implants/lib/eldritchv2/eldritch-core/tests/max_min_mixed.rs @@ -0,0 +1,64 @@ +#[cfg(test)] +mod tests { + use eldritch_core::{Interpreter, Value}; + + #[test] + fn test_max_min_mixed_types() { + let code = r#" +tests = [ + [1, 0.8, 1], + [1, 1.8, 1.8], + [10, 9.999, 10], + [10, 19.999, 19.999], + [1, 1, 1], + [1, 10, 10], + [11, 0, 11], + [.1, 0, .1], + [.2, .1, .2], + [.2, 2, 2], +] +errors = [] +for a, b, c in tests: + r = max(a, b) + if r != c: + errors.append(f"FAIL max({a}, {b}) != '{c}' got '{r}'") + +tests = [ + [1, 0.8, 0.8], + [1, 1.8, 1], + [10, 9.999, 9.999], + [10, 19.999, 10], + [1, 1, 1], + [1, 10, 1], + [11, 0, 0], + [.1, 0, 0], + [.2, .1, .1], + [.2, 2, .2], +] +for a, b, c in tests: + r = min(a, b) + if r != c: + errors.append(f"FAIL min({a}, {b}) != '{c}' got '{r}'") + +errors +"#; + let mut interp = Interpreter::new(); + match interp.interpret(code) { + Ok(val) => { + match val { + Value::List(l) => { + let errors = l.read(); + if !errors.is_empty() { + for err in errors.iter() { + println!("{}", err); + } + panic!("Test failed with {} errors", errors.len()); + } + }, + _ => panic!("Expected list of errors"), + } + }, + Err(e) => panic!("Interpreter error: {}", e), + } + } +} From 19163f61603d4447b5b86dccb1aadf3432273076 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 2 Jan 2026 16:16:57 +0000 Subject: [PATCH 2/2] Fix mixed Int/Float comparisons in eldritch-core Previously, mixed type comparisons (e.g., `Int` vs `Float`) fell back to ordering by type discriminant because they are different types. This caused `max(1, 0.8)` to return `0.8` because `Int` (2) < `Float` (3). This change adds a special case in `Value::cmp_helper` to handle `Int` vs `Float` and `Float` vs `Int` by converting the integer to a float and using `total_cmp`. This ensures numerical correctness for `max`, `min`, and general comparisons. Added regression test `tests/max_min_mixed.rs`. Ran `cargo fmt` to ensure code style compliance. --- .../eldritch-core/tests/max_min_mixed.rs | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/implants/lib/eldritchv2/eldritch-core/tests/max_min_mixed.rs b/implants/lib/eldritchv2/eldritch-core/tests/max_min_mixed.rs index 4e5731fb6..56c2cd897 100644 --- a/implants/lib/eldritchv2/eldritch-core/tests/max_min_mixed.rs +++ b/implants/lib/eldritchv2/eldritch-core/tests/max_min_mixed.rs @@ -44,19 +44,17 @@ errors "#; let mut interp = Interpreter::new(); match interp.interpret(code) { - Ok(val) => { - match val { - Value::List(l) => { - let errors = l.read(); - if !errors.is_empty() { - for err in errors.iter() { - println!("{}", err); - } - panic!("Test failed with {} errors", errors.len()); - } - }, - _ => panic!("Expected list of errors"), - } + Ok(val) => match val { + Value::List(l) => { + let errors = l.read(); + if !errors.is_empty() { + for err in errors.iter() { + println!("{}", err); + } + panic!("Test failed with {} errors", errors.len()); + } + } + _ => panic!("Expected list of errors"), }, Err(e) => panic!("Interpreter error: {}", e), }