From 4feffb62bbf6434dffe14c3e8fa3c748e67cb7af Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Tue, 29 Sep 2026 23:00:15 -0400 Subject: [PATCH 1/2] fix(memories): sort Month span by time within a day The Month sort keyed on the display day only. The sort is stable, so a day's photos kept DB order, merged across libraries, and their times jumped back and forth. Month now sorts like Week: full date, then timestamp (undated items last). The sort moves into sort_memories so it can be tested directly; the new test covers a multi-library, out-of-order day. Co-Authored-By: Claude Opus 5.5 --- src/memories.rs | 92 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 67 insertions(+), 25 deletions(-) diff --git a/src/memories.rs b/src/memories.rs index becd537..9e437c5 100644 --- a/src/memories.rs +++ b/src/memories.rs @@ -660,35 +660,34 @@ pub fn gather_memory_items( // Sort once over the merged result set. The SQL filter handles the // matching; sort order is purely UI concern. - match span_mode { - // Month: chronological — gives an "overview" feel. - MemoriesSpan::Month => memories_with_dates.sort_by(|a, b| a.1.cmp(&b.1)), - // Week: full date then timestamp (oldest → newest). - MemoriesSpan::Week => { - memories_with_dates.sort_by(|a, b| { - a.1.cmp(&b.1) - .then_with(|| match (a.0.created, b.0.created) { - (Some(at), Some(bt)) => at.cmp(&bt), - (Some(_), None) => std::cmp::Ordering::Less, - (None, Some(_)) => std::cmp::Ordering::Greater, - (None, None) => std::cmp::Ordering::Equal, - }) - }); - } - // Day: same calendar day across years, sub-sorted by timestamp. - MemoriesSpan::Day => { - memories_with_dates.sort_by(|a, b| match (a.0.created, b.0.created) { - (Some(at), Some(bt)) => at.cmp(&bt), - (Some(_), None) => std::cmp::Ordering::Less, - (None, Some(_)) => std::cmp::Ordering::Greater, - (None, None) => std::cmp::Ordering::Equal, - }); - } - } + sort_memories(&mut memories_with_dates, span_mode); Ok(memories_with_dates.into_iter().map(|(m, _)| m).collect()) } +/// Order `(item, display_day)` pairs for the given span, oldest first. +/// +/// Rows arrive in DB order, merged across libraries, so every span needs a +/// timestamp key within a day — without one, a day's photos keep that +/// arbitrary order and their times jump back and forth. +fn sort_memories(memories: &mut [(MemoryItem, NaiveDate)], span: MemoriesSpan) { + // Timestamped items before ones without a date. + let by_created = |a: &MemoryItem, b: &MemoryItem| match (a.created, b.created) { + (Some(at), Some(bt)) => at.cmp(&bt), + (Some(_), None) => std::cmp::Ordering::Less, + (None, Some(_)) => std::cmp::Ordering::Greater, + (None, None) => std::cmp::Ordering::Equal, + }; + match span { + // Month / Week: full date then timestamp (oldest → newest). + MemoriesSpan::Month | MemoriesSpan::Week => { + memories.sort_by(|a, b| a.1.cmp(&b.1).then_with(|| by_created(&a.0, &b.0))); + } + // Day: same calendar day across years, sorted by timestamp. + MemoriesSpan::Day => memories.sort_by(|a, b| by_created(&a.0, &b.0)), + } +} + #[cfg(test)] mod tests { use super::*; @@ -1195,6 +1194,49 @@ mod tests { assert_eq!(memories_with_dates[2].0.created.unwrap(), jan_16_2021_8am); } + #[test] + fn month_span_sorts_by_time_within_a_day() { + // Same display day, rows in DB order (merged across libraries) — + // the Month sort used to keep this order, so times jumped around. + let day = NaiveDate::from_ymd_opt(2019, 9, 14).unwrap(); + let at = |h: u32| day.and_hms_opt(h, 0, 0).unwrap().and_utc().timestamp(); + let item = |path: &str, created: Option, library_id: i32| MemoryItem { + path: path.to_string(), + created, + modified: created, + library_id, + date_utc: created, + camera_offset_minutes: None, + date_taken_tz_source: None, + wall_clock: None, + }; + let earlier_day = NaiveDate::from_ymd_opt(2019, 9, 2).unwrap(); + let mut memories = vec![ + (item("lib1-15h.jpg", Some(at(15)), 1), day), + (item("undated.jpg", None, 1), day), + (item("lib2-09h.jpg", Some(at(9)), 2), day), + (item("lib1-12h.jpg", Some(at(12)), 1), day), + ( + item("sept-2nd.jpg", Some(at(20) - 12 * 86_400), 2), + earlier_day, + ), + ]; + + sort_memories(&mut memories, MemoriesSpan::Month); + + let paths: Vec<&str> = memories.iter().map(|(m, _)| m.path.as_str()).collect(); + assert_eq!( + paths, + vec![ + "sept-2nd.jpg", + "lib2-09h.jpg", + "lib1-12h.jpg", + "lib1-15h.jpg", + "undated.jpg" + ] + ); + } + // ---- Day-matching (ticket #119) ---- #[test] From 75581b3f6a39a0d7a9b6738f082fd91c730d9e0a Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Tue, 29 Sep 2026 23:03:51 -0400 Subject: [PATCH 2/2] test(memories): test the real Week sort; cover every span via sort_memories test_week_span_sorting_chronological_by_day sorted its own inline copy (day-of-month across years), so it kept passing after 7d2a314 made the real Week sort chronological (full date, then time), and it disagreed with production. Replace it with a test that calls sort_memories, and add one for Day, so all three spans are pinned against the real sort. Co-Authored-By: Claude Opus 5.5 --- src/memories.rs | 197 +++++++++++++++++++++--------------------------- 1 file changed, 88 insertions(+), 109 deletions(-) diff --git a/src/memories.rs b/src/memories.rs index 9e437c5..51a631a 100644 --- a/src/memories.rs +++ b/src/memories.rs @@ -1093,114 +1093,8 @@ mod tests { assert!(!excluder.is_excluded(&keep)); } - #[test] - fn test_week_span_sorting_chronological_by_day() { - // Test that Week span sorts by: - // 1. Day of month (ascending) - // 2. Full timestamp oldest to newest (year + time combined) - - // Create test data: - // - Jan 15, 2024 at 9:00 AM - // - Jan 15, 2020 at 10:00 AM - // - Jan 16, 2021 at 8:00 AM - - let jan_15_2024_9am = NaiveDate::from_ymd_opt(2024, 1, 15) - .unwrap() - .and_hms_opt(9, 0, 0) - .unwrap() - .and_utc() - .timestamp(); - - let jan_15_2020_10am = NaiveDate::from_ymd_opt(2020, 1, 15) - .unwrap() - .and_hms_opt(10, 0, 0) - .unwrap() - .and_utc() - .timestamp(); - - let jan_16_2021_8am = NaiveDate::from_ymd_opt(2021, 1, 16) - .unwrap() - .and_hms_opt(8, 0, 0) - .unwrap() - .and_utc() - .timestamp(); - - let mut memories_with_dates = [ - ( - MemoryItem { - path: "photo1.jpg".to_string(), - created: Some(jan_15_2024_9am), - modified: Some(jan_15_2024_9am), - library_id: 1, - date_utc: None, - camera_offset_minutes: None, - date_taken_tz_source: None, - wall_clock: None, - }, - NaiveDate::from_ymd_opt(2024, 1, 15).unwrap(), - ), - ( - MemoryItem { - path: "photo2.jpg".to_string(), - created: Some(jan_15_2020_10am), - modified: Some(jan_15_2020_10am), - library_id: 1, - date_utc: None, - camera_offset_minutes: None, - date_taken_tz_source: None, - wall_clock: None, - }, - NaiveDate::from_ymd_opt(2020, 1, 15).unwrap(), - ), - ( - MemoryItem { - path: "photo3.jpg".to_string(), - created: Some(jan_16_2021_8am), - modified: Some(jan_16_2021_8am), - library_id: 1, - date_utc: None, - camera_offset_minutes: None, - date_taken_tz_source: None, - wall_clock: None, - }, - NaiveDate::from_ymd_opt(2021, 1, 16).unwrap(), - ), - ]; - - // Sort using Week span logic - memories_with_dates.sort_by(|a, b| { - // First, sort by day of month - let day_cmp = a.1.day().cmp(&b.1.day()); - if day_cmp != std::cmp::Ordering::Equal { - return day_cmp; - } - - // Then sort by full created timestamp (oldest to newest) - match (a.0.created, b.0.created) { - (Some(a_time), Some(b_time)) => a_time.cmp(&b_time), - (Some(_), None) => std::cmp::Ordering::Less, - (None, Some(_)) => std::cmp::Ordering::Greater, - (None, None) => std::cmp::Ordering::Equal, - } - }); - - // Expected order: - // 1. Jan 15, 2020 at 10:00 AM (oldest Jan 15 photo) - // 2. Jan 15, 2024 at 9:00 AM (newer Jan 15 photo) - // 3. Jan 16, 2021 at 8:00 AM (all Jan 16 photos after Jan 15) - - assert_eq!(memories_with_dates[0].0.created.unwrap(), jan_15_2020_10am); - assert_eq!(memories_with_dates[1].0.created.unwrap(), jan_15_2024_9am); - assert_eq!(memories_with_dates[2].0.created.unwrap(), jan_16_2021_8am); - } - - #[test] - fn month_span_sorts_by_time_within_a_day() { - // Same display day, rows in DB order (merged across libraries) — - // the Month sort used to keep this order, so times jumped around. - let day = NaiveDate::from_ymd_opt(2019, 9, 14).unwrap(); - let at = |h: u32| day.and_hms_opt(h, 0, 0).unwrap().and_utc().timestamp(); - let item = |path: &str, created: Option, library_id: i32| MemoryItem { + fn memory_item(path: &str, created: Option, library_id: i32) -> MemoryItem { + MemoryItem { path: path.to_string(), created, modified: created, @@ -1209,7 +1103,92 @@ mod tests { camera_offset_minutes: None, date_taken_tz_source: None, wall_clock: None, - }; + } + } + + fn utc_ts(y: i32, m: u32, d: u32, h: u32) -> i64 { + NaiveDate::from_ymd_opt(y, m, d) + .unwrap() + .and_hms_opt(h, 0, 0) + .unwrap() + .and_utc() + .timestamp() + } + + #[test] + fn week_span_sorts_by_full_date_then_time() { + // Week is chronological (7d2a314): full date first, so years group + // together, then time within a day. + let jan = |y, d| NaiveDate::from_ymd_opt(y, 1, d).unwrap(); + let mut memories = vec![ + ( + memory_item("2024-01-15.jpg", Some(utc_ts(2024, 1, 15, 9)), 1), + jan(2024, 15), + ), + ( + memory_item("2020-01-15-10h.jpg", Some(utc_ts(2020, 1, 15, 10)), 1), + jan(2020, 15), + ), + ( + memory_item("2021-01-16.jpg", Some(utc_ts(2021, 1, 16, 8)), 2), + jan(2021, 16), + ), + ( + memory_item("2020-01-15-07h.jpg", Some(utc_ts(2020, 1, 15, 7)), 2), + jan(2020, 15), + ), + ]; + + sort_memories(&mut memories, MemoriesSpan::Week); + + let paths: Vec<&str> = memories.iter().map(|(m, _)| m.path.as_str()).collect(); + assert_eq!( + paths, + vec![ + "2020-01-15-07h.jpg", + "2020-01-15-10h.jpg", + "2021-01-16.jpg", + "2024-01-15.jpg" + ] + ); + } + + #[test] + fn day_span_sorts_by_timestamp_across_years() { + // Day: the same calendar day across years, oldest first; undated last. + let d = |y| NaiveDate::from_ymd_opt(y, 9, 29).unwrap(); + let mut memories = vec![ + ( + memory_item("2022.jpg", Some(utc_ts(2022, 9, 29, 8)), 1), + d(2022), + ), + (memory_item("undated.jpg", None, 1), d(2022)), + ( + memory_item("2018-18h.jpg", Some(utc_ts(2018, 9, 29, 18)), 2), + d(2018), + ), + ( + memory_item("2018-06h.jpg", Some(utc_ts(2018, 9, 29, 6)), 1), + d(2018), + ), + ]; + + sort_memories(&mut memories, MemoriesSpan::Day); + + let paths: Vec<&str> = memories.iter().map(|(m, _)| m.path.as_str()).collect(); + assert_eq!( + paths, + vec!["2018-06h.jpg", "2018-18h.jpg", "2022.jpg", "undated.jpg"] + ); + } + + #[test] + fn month_span_sorts_by_time_within_a_day() { + // Same display day, rows in DB order (merged across libraries) — + // the Month sort used to keep this order, so times jumped around. + let day = NaiveDate::from_ymd_opt(2019, 9, 14).unwrap(); + let at = |h| utc_ts(2019, 9, 14, h); + let item = memory_item; let earlier_day = NaiveDate::from_ymd_opt(2019, 9, 2).unwrap(); let mut memories = vec![ (item("lib1-15h.jpg", Some(at(15)), 1), day),