Fixed statistics filter to be per book per day instead of just per day

Author: paviro

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "koshelf"
-version = "1.2.0"
+version = "1.2.1"
 description = "Transform your KOReader library into a beautiful reading dashboard with statistics."
 repository = "https://github.com/paviro/KOShelf"
 license = "EUPL-1.2 license"

Cargo.toml

@@ -208,9 +208,9 @@ KoShelf can operate in several modes:
 - `--heatmap-scale-max`: Maximum value for heatmap color intensity scaling (e.g., "auto", "1h", "1h30m", "45min"). Values above this will still be shown but use the highest color intensity. Default is "auto" for automatic scaling
 - `--timezone`: Timezone to interpret timestamps (IANA name, e.g., `Australia/Sydney`); defaults to system local
 - `--day-start-time`: Logical day start time as `HH:MM` (default: `00:00`)
-- `--min-pages-per-day`: Minimum pages read per day to be counted in statistics (optional)
-- `--min-time-per-day`: Minimum reading time per day to be counted in statistics (e.g., "15m", "1h") (optional)
-  > **Note:** If both `--min-pages-per-day` and `--min-time-per-day` are provided, a day is counted if **either** condition is met.
+- `--min-pages-per-day`: Minimum pages read per book per day to be counted in statistics (optional)
+- `--min-time-per-day`: Minimum reading time per book per day to be counted in statistics (e.g., "15m", "1h") (optional)
+  > **Note:** If both `--min-pages-per-day` and `--min-time-per-day` are provided, a book's data for a day is counted if **either** condition is met for that book on that day. These filters apply **per book per day**, meaning each book must individually meet the threshold for each day to be included in statistics.
 - `--github`: Print GitHub repository URL
 
 ### Example

README.md

@@ -332,39 +332,40 @@ impl StatisticsCalculator {
 
         (total_completions, books_completed, most_completions)
     }
-    /// Filter statistics to exclude days that don't meet the minimum requirements
+    /// Filter statistics to exclude days that don't meet the minimum requirements per book
     pub fn filter_stats(
         stats_data: &mut StatisticsData, 
         time_config: &TimeConfig, 
         min_pages: Option<u32>, 
         min_time: Option<u32>
     ) {
-        // 1. Aggregate daily totals
-        let mut daily_pages: HashMap<String, i64> = HashMap::new();
-        let mut daily_time: HashMap<String, i64> = HashMap::new();
+        // 1. Aggregate daily totals per book per day
+        let mut daily_book_pages: HashMap<(i64, String), i64> = HashMap::new();
+        let mut daily_book_time: HashMap<(i64, String), i64> = HashMap::new();
 
         for stat in &stats_data.page_stats {
             if stat.duration <= 0 { continue; }
 
             let date = time_config.date_for_timestamp(stat.start_time);
             let date_str = date.format("%Y-%m-%d").to_string();
+            let key = (stat.id_book, date_str);
 
-            *daily_pages.entry(date_str.clone()).or_insert(0) += 1;
-            *daily_time.entry(date_str).or_insert(0) += stat.duration;
+            *daily_book_pages.entry(key.clone()).or_insert(0) += 1;
+            *daily_book_time.entry(key).or_insert(0) += stat.duration;
         }
 
-        // 2. Identify valid dates
-        let mut valid_dates: HashMap<String, bool> = HashMap::new();
+        // 2. Identify valid (book, date) combinations
+        let mut valid_book_dates: HashMap<(i64, String), bool> = HashMap::new();
 
-        // Collect all unique dates
-        let mut all_dates: Vec<String> = daily_pages.keys().cloned().collect();
-        all_dates.extend(daily_time.keys().cloned());
-        all_dates.sort();
-        all_dates.dedup();
+        // Collect all unique (book, date) combinations
+        let mut all_book_dates: Vec<(i64, String)> = daily_book_pages.keys().cloned().collect();
+        all_book_dates.extend(daily_book_time.keys().cloned());
+        all_book_dates.sort();
+        all_book_dates.dedup();
 
-        for date in all_dates {
-            let pages = *daily_pages.get(&date).unwrap_or(&0);
-            let time = *daily_time.get(&date).unwrap_or(&0);
+        for book_date in all_book_dates {
+            let pages = *daily_book_pages.get(&book_date).unwrap_or(&0);
+            let time = *daily_book_time.get(&book_date).unwrap_or(&0);
 
             let pages_ok = min_pages.map_or(false, |min| pages >= min as i64);
             let time_ok = min_time.map_or(false, |min| time >= min as i64);
@@ -378,15 +379,16 @@ impl StatisticsCalculator {
                 time_ok
             };
 
-            valid_dates.insert(date, is_valid);
+            valid_book_dates.insert(book_date, is_valid);
         }
 
-        // 3. Filter page_stats
+        // 3. Filter page_stats based on per-book-per-day validity
         stats_data.page_stats.retain(|stat| {
             if stat.duration <= 0 { return false; }
             let date = time_config.date_for_timestamp(stat.start_time);
             let date_str = date.format("%Y-%m-%d").to_string();
-            *valid_dates.get(&date_str).unwrap_or(&false)
+            let key = (stat.id_book, date_str);
+            *valid_book_dates.get(&key).unwrap_or(&false)
         });
     }
 }

src/statistics.rs

@@ -3,133 +3,148 @@ use crate::statistics::StatisticsCalculator;
 use crate::time_config::TimeConfig;
 use std::collections::HashMap;
 
-fn create_mock_stats() -> StatisticsData {
-    // Create 3 days of data:
-    // Day 1 (2023-01-01): 5 pages, 10 mins (600s)
-    // Day 2 (2023-01-02): 20 pages, 5 mins (300s)
-    // Day 3 (2023-01-03): 2 pages, 60 mins (3600s)
-
+#[test]
+fn test_filter_stats_per_book_per_day() {
+    // Comprehensive test for per-book-per-day filtering
+    // Tests multiple scenarios:
+    // - Day 1: Book 1 (10 pages, 600s) passes pages threshold, Book 2 (5 pages, 300s) fails both
+    // - Day 2: Book 1 (2 pages, 3600s) passes time threshold, Book 2 (15 pages, 200s) passes pages threshold
+    // - Day 3: Book 1 (3 pages, 100s) fails both, Book 2 (4 pages, 500s) fails both
+    
     let day1_ts = 1672531200; // 2023-01-01 00:00:00 UTC
     let day2_ts = 1672617600; // 2023-01-02 00:00:00 UTC
     let day3_ts = 1672704000; // 2023-01-03 00:00:00 UTC
-
     let mut page_stats = Vec::new();
-
-    // Day 1: 5 pages, 10 mins
-    for i in 0..5 {
+    
+    // Day 1: Book 1 - 10 pages, 600s (passes pages threshold)
+    for i in 0..10 {
         page_stats.push(PageStat {
             id_book: 1,
             page: i,
-            start_time: day1_ts + i * 60, // 1 min per page roughly
-            duration: 120,                // 2 mins per page -> total 10 mins
+            start_time: day1_ts + i * 60,
+            duration: 60,
         });
     }
-
-    // Day 2: 20 pages, 5 mins
-    for i in 0..20 {
+    
+    // Day 1: Book 2 - 5 pages, 300s (fails both thresholds)
+    for i in 0..5 {
         page_stats.push(PageStat {
-            id_book: 1,
+            id_book: 2,
             page: i,
-            start_time: day2_ts + i * 10,
-            duration: 15, // 15s per page -> total 300s (5 mins)
+            start_time: day1_ts + i * 60,
+            duration: 60,
         });
     }
-
-    // Day 3: 2 pages, 60 mins
+    
+    // Day 2: Book 1 - 2 pages, 3600s (passes time threshold)
     for i in 0..2 {
         page_stats.push(PageStat {
             id_book: 1,
-            page: i,
-            start_time: day3_ts + i * 1800,
-            duration: 1800, // 30 mins per page -> total 60 mins
+            page: i + 10,
+            start_time: day2_ts + i * 1800,
+            duration: 1800,
         });
     }
-
-    StatisticsData {
-        books: vec![StatBook {
-            id: 1,
-            title: "Test Book".to_string(),
-            authors: "Author".to_string(),
-            notes: None,
-            last_open: None,
-            highlights: None,
-            pages: None,
-            md5: "abc".to_string(),
-            total_read_time: None,
-            total_read_pages: None,
-            completions: None,
-        }],
-        page_stats,
-        stats_by_md5: HashMap::new(),
+    
+    // Day 2: Book 2 - 15 pages, 200s (passes pages threshold)
+    for i in 0..15 {
+        page_stats.push(PageStat {
+            id_book: 2,
+            page: i + 5,
+            start_time: day2_ts + i * 10,
+            duration: 13,
+        });
     }
-}
-
-#[test]
-fn test_filter_stats_min_pages() {
-    let mut data = create_mock_stats();
-    let time_config = TimeConfig::new(None, 0); // UTC by default
-
-    // Filter: min 10 pages
-    // Should keep Day 2 (20 pages), remove Day 1 (5) and Day 3 (2)
-    StatisticsCalculator::filter_stats(&mut data, &time_config, Some(10), None);
-
-    let remaining_days: Vec<i64> = data.page_stats.iter().map(|s| s.start_time).collect();
-    assert!(!remaining_days.is_empty());
-
-    for ts in remaining_days {
-        // All timestamps should be from Day 2
-        assert!(ts >= 1672617600 && ts < 1672704000);
+    
+    // Day 3: Book 1 - 3 pages, 100s (fails both)
+    for i in 0..3 {
+        page_stats.push(PageStat {
+            id_book: 1,
+            page: i + 12,
+            start_time: day3_ts + i * 30,
+            duration: 33,
+        });
     }
-}
-
-#[test]
-fn test_filter_stats_min_time() {
-    let mut data = create_mock_stats();
-    let time_config = TimeConfig::new(None, 0);
-
-    // Filter: min 30 mins (1800s)
-    // Should keep Day 3 (60 mins), remove Day 1 (10 mins) and Day 2 (5 mins)
-    StatisticsCalculator::filter_stats(&mut data, &time_config, None, Some(1800));
-
-    let remaining_days: Vec<i64> = data.page_stats.iter().map(|s| s.start_time).collect();
-    assert!(!remaining_days.is_empty());
-
-    for ts in remaining_days {
-        // All timestamps should be from Day 3
-        assert!(ts >= 1672704000);
+    
+    // Day 3: Book 2 - 4 pages, 500s (fails both)
+    for i in 0..4 {
+        page_stats.push(PageStat {
+            id_book: 2,
+            page: i + 20,
+            start_time: day3_ts + i * 100,
+            duration: 125,
+        });
     }
-}
-
-#[test]
-fn test_filter_stats_or_logic() {
-    let mut data = create_mock_stats();
+    
+    let mut data = StatisticsData {
+        books: vec![
+            StatBook {
+                id: 1,
+                title: "Book 1".to_string(),
+                authors: "Author 1".to_string(),
+                notes: None,
+                last_open: None,
+                highlights: None,
+                pages: None,
+                md5: "abc".to_string(),
+                total_read_time: None,
+                total_read_pages: None,
+                completions: None,
+            },
+            StatBook {
+                id: 2,
+                title: "Book 2".to_string(),
+                authors: "Author 2".to_string(),
+                notes: None,
+                last_open: None,
+                highlights: None,
+                pages: None,
+                md5: "def".to_string(),
+                total_read_time: None,
+                total_read_pages: None,
+                completions: None,
+            },
+        ],
+        page_stats,
+        stats_by_md5: HashMap::new(),
+    };
+    
     let time_config = TimeConfig::new(None, 0);
-
-    // Filter: min 10 pages OR min 30 mins
-    // Should keep Day 2 (20 pages) AND Day 3 (60 mins)
-    // Should remove Day 1 (5 pages, 10 mins)
-    StatisticsCalculator::filter_stats(&mut data, &time_config, Some(10), Some(1800));
-
-    let remaining_days: Vec<i64> = data.page_stats.iter().map(|s| s.start_time).collect();
-    assert!(!remaining_days.is_empty());
-
-    let mut has_day2 = false;
-    let mut has_day3 = false;
-    let mut has_day1 = false;
-
-    for ts in remaining_days {
-        if ts >= 1672531200 && ts < 1672617600 {
-            has_day1 = true;
-        }
-        if ts >= 1672617600 && ts < 1672704000 {
-            has_day2 = true;
-        }
-        if ts >= 1672704000 {
-            has_day3 = true;
+    // Filter: min 8 pages OR min 1800s (30 mins)
+    StatisticsCalculator::filter_stats(&mut data, &time_config, Some(8), Some(1800));
+    
+    // Expected results:
+    // Day 1: Book 1 kept (10 pages), Book 2 filtered (5 pages, 300s)
+    // Day 2: Book 1 kept (3600s), Book 2 kept (15 pages)
+    // Day 3: Both filtered
+    // Total: 10 + 2 + 15 = 27 pages
+    
+    assert_eq!(data.page_stats.len(), 27, "Should have 27 pages remaining");
+    
+    let mut book1_day1 = 0;
+    let mut book1_day2 = 0;
+    let mut book1_day3 = 0;
+    let mut book2_day1 = 0;
+    let mut book2_day2 = 0;
+    let mut book2_day3 = 0;
+    
+    for stat in &data.page_stats {
+        match (stat.id_book, stat.start_time) {
+            (1, ts) if ts >= day1_ts && ts < day2_ts => book1_day1 += 1,
+            (1, ts) if ts >= day2_ts && ts < day3_ts => book1_day2 += 1,
+            (1, ts) if ts >= day3_ts => book1_day3 += 1,
+            (2, ts) if ts >= day1_ts && ts < day2_ts => book2_day1 += 1,
+            (2, ts) if ts >= day2_ts && ts < day3_ts => book2_day2 += 1,
+            (2, ts) if ts >= day3_ts => book2_day3 += 1,
+            _ => {}
         }
     }
-
-    assert!(!has_day1, "Day 1 should be removed");
-    assert!(has_day2, "Day 2 should be kept (pages)");
-    assert!(has_day3, "Day 3 should be kept (time)");
+    
+    assert_eq!(book1_day1, 10, "Book 1 Day 1: 10 pages (passes pages threshold)");
+    assert_eq!(book1_day2, 2, "Book 1 Day 2: 2 pages (passes time threshold)");
+    assert_eq!(book1_day3, 0, "Book 1 Day 3: filtered (fails both)");
+    assert_eq!(book2_day1, 0, "Book 2 Day 1: filtered (fails both)");
+    assert_eq!(book2_day2, 15, "Book 2 Day 2: 15 pages (passes pages threshold)");
+    assert_eq!(book2_day3, 0, "Book 2 Day 3: filtered (fails both)");
 }
+