From 011487da3defd3a8936b1aa84b90638be7618622 Mon Sep 17 00:00:00 2001 From: Vlad Poenaru Date: Wed, 11 Nov 2026 08:07:02 -0800 Subject: [PATCH] lore: sort emails on a total order `semcode lore -b nvme_tcp_queue --limit 0` aborts with "user-provided comparison function does not correctly implement a total order". Any result set that mixes Date headers that parse with ones that do not can trigger this panic. `sort_emails_by_date()` compares parsed dates when both Date headers parse and raw strings otherwise, which is not transitive. With A = "Tue, 01 Jan 2030 00:00:00 +0000", B = "U-not-a-date" and C = "Wed, 01 Jan 2020 00:00:00 +0000", it says A < B and B < C by string but C < A by date. Rust's sort panics when it detects such a comparator. Sort on a key instead: emails whose date parses come first in time order, the rest follow by raw header, and ties break on the raw header and then on `message_id`. Same-second emails now come out in the same order on every run, whatever order the query returned them in. The function's signature and its five callers are unchanged. The key is computed once per email, so each Date header is parsed once rather than on every comparison. The newest-first sort in `dig` was already a total order; its same-instant ties now break on `message_id` too. Both new tests fail on the old comparator. One sorts the A/B/C triple from all six input orders. The other sorts 2,000 emails, a third of them with unparseable dates and most sharing a timestamp, from two fixed permutations, and checks the order and that both permutations give the same result. --- src/lore_writers.rs | 145 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 133 insertions(+), 12 deletions(-) diff --git a/src/lore_writers.rs b/src/lore_writers.rs index c7e7678..dd81616 100644 --- a/src/lore_writers.rs +++ b/src/lore_writers.rs @@ -389,18 +389,23 @@ fn convert_to_asctime(rfc2822_date: &str) -> String { } } -/// Sort emails by date (oldest first) using RFC 2822 date parsing +/// Sort emails by date, oldest first. +/// +/// Emails whose Date header parses as RFC 2822 come first, in time order; +/// the rest follow, ordered by the raw header. Ties break on the raw header, +/// then on message_id, so the order is a total one and is the same on every +/// run whatever order the emails arrived in. pub fn sort_emails_by_date(emails: &mut [crate::types::LoreEmailInfo]) { - emails.sort_by(|a, b| { - use chrono::DateTime; - let a_parsed = DateTime::parse_from_rfc2822(&a.date); - let b_parsed = DateTime::parse_from_rfc2822(&b.date); - - match (a_parsed, b_parsed) { - (Ok(a_time), Ok(b_time)) => a_time.cmp(&b_time), - // If parsing fails, fall back to string comparison - _ => a.date.cmp(&b.date), - } + emails.sort_by_cached_key(|e| { + let ts = chrono::DateTime::parse_from_rfc2822(&e.date) + .ok() + .map(|d| d.timestamp()); + ( + ts.is_none(), + ts.unwrap_or(0), + e.date.clone(), + e.message_id.clone(), + ) }); } @@ -1098,7 +1103,10 @@ pub async fn dig_lore_by_commit_to_writer( let b_date = chrono::DateTime::parse_from_rfc2822(&b.date).ok(); match (b_date, a_date) { - (Some(b_dt), Some(a_dt)) => b_dt.cmp(&a_dt), // Descending order (newest first) + // Descending order (newest first); same-instant emails by message_id + (Some(b_dt), Some(a_dt)) => b_dt + .cmp(&a_dt) + .then_with(|| a.message_id.cmp(&b.message_id)), (Some(_), None) => std::cmp::Ordering::Less, // b is valid, a is not - b comes first (None, Some(_)) => std::cmp::Ordering::Greater, // a is valid, b is not - a comes first (None, None) => b.date.cmp(&a.date), // Both invalid - fall back to string comparison @@ -1272,3 +1280,116 @@ pub async fn dig_lore_by_commit_to_writer( Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::HashMap; + + fn email(date: &str, message_id: &str) -> LoreEmailInfo { + LoreEmailInfo { + git_commit_sha: String::new(), + from: String::new(), + date: date.to_string(), + date_timestamp: 0, + message_id: message_id.to_string(), + in_reply_to: None, + subject: String::new(), + references: None, + recipients: String::new(), + body: String::new(), + symbols: Vec::new(), + } + } + + fn ids(emails: &[LoreEmailInfo]) -> Vec<&str> { + emails.iter().map(|e| e.message_id.as_str()).collect() + } + + /// C is older than A, and B's date does not parse, so the order is C, A, B + /// from every starting order, although the raw strings sort A, B, C. + #[test] + fn sort_emails_by_date_handles_the_intransitive_triple() { + let a = email("Tue, 01 Jan 2030 00:00:00 +0000", "a"); + let b = email("U-not-a-date", "b"); + let c = email("Wed, 01 Jan 2020 00:00:00 +0000", "c"); + let orders = [ + [&a, &b, &c], + [&a, &c, &b], + [&b, &a, &c], + [&b, &c, &a], + [&c, &a, &b], + [&c, &b, &a], + ]; + for order in orders { + let mut emails: Vec = order.iter().map(|e| (*e).clone()).collect(); + sort_emails_by_date(&mut emails); + let input: Vec<&str> = order.iter().map(|e| e.message_id.as_str()).collect(); + assert_eq!(ids(&emails), ["c", "a", "b"], "input order {input:?}"); + } + } + + /// The emails in a fixed pseudo-random order: `i -> (mul * i + add) % n` + /// is a permutation of `0..n` whenever `mul` and `n` are coprime. + fn permuted(emails: &[LoreEmailInfo], mul: usize, add: usize) -> Vec { + let n = emails.len(); + (0..n) + .map(|i| emails[(mul * i + add) % n].clone()) + .collect() + } + + #[test] + fn sort_emails_by_date_mixed_large_input() { + const N: usize = 2000; + const BASE: i64 = 1_700_000_000; + // Every third email has an unparseable date. The rest share 50 + // timestamps and the unparseable ones 17 raw strings, so most keys + // tie on the date and only message_id separates them. + let mut timestamp_of: HashMap> = HashMap::new(); + let mut emails = Vec::with_capacity(N); + for k in 0..N { + let message_id = format!("<{k:05}@example.org>"); + let (date, ts) = if k % 3 == 0 { + (format!("not-a-date-{:02}", k % 17), None) + } else { + let ts = BASE + (k as i64 % 50) * 3600; + let date = chrono::DateTime::from_timestamp(ts, 0) + .unwrap() + .to_rfc2822(); + (date, Some(ts)) + }; + timestamp_of.insert(message_id.clone(), ts); + emails.push(email(&date, &message_id)); + } + + let mut sorted = permuted(&emails, 1237, 411); + sort_emails_by_date(&mut sorted); + + assert_eq!(sorted.len(), N); + let unique: HashSet<&str> = ids(&sorted).into_iter().collect(); + assert_eq!(unique.len(), N, "sorting lost or duplicated emails"); + + let parseable = N - N.div_ceil(3); + for (i, pair) in sorted.windows(2).enumerate() { + let (x, y) = (&pair[0], &pair[1]); + let (tx, ty) = (timestamp_of[&x.message_id], timestamp_of[&y.message_id]); + assert_eq!(tx.is_some(), i < parseable, "position {i}"); + let in_order = match (tx, ty) { + (Some(tx), Some(ty)) => (tx, &x.message_id) < (ty, &y.message_id), + (Some(_), None) => true, + (None, Some(_)) => false, + (None, None) => (&x.date, &x.message_id) < (&y.date, &y.message_id), + }; + assert!( + in_order, + "position {i}: {} ({}) before {} ({})", + x.message_id, x.date, y.message_id, y.date + ); + } + + // A different input order gives the same output. + let mut again = permuted(&emails, 7, 1999); + sort_emails_by_date(&mut again); + assert_eq!(ids(&again), ids(&sorted)); + } +}