Fix/Retrievability SQL (#4424)

* Fix/SQL Retrievability Underflow

* Added: Reminder

* ./check

* Apply code diff changes

Co-authored-by: user1823 <92206575+user1823@users.noreply.github.com>

* Slight cleanup

* fix bug again

* extra label comment

* Update rslib/src/storage/sqlite.rs

Co-authored-by: user1823 <92206575+user1823@users.noreply.github.com>

* Fix: Ignore new cards

* use const

---------

Co-authored-by: user1823 <92206575+user1823@users.noreply.github.com>

rslib/src/search/sqlwriter.rs

@@ -434,9 +434,10 @@ impl SqlWriter<'_> {
                     let timing = self.col.timing_today()?;
                     (timing.days_elapsed, timing.next_day_at, timing.now)
                 };
+                const NEW_TYPE: i8 = CardType::New as i8;
                 write!(
                     self.sql,
-                    "extract_fsrs_retrievability(c.data, case when c.odue !=0 then c.odue else c.due end, c.ivl, {elap}, {next_day_at}, {now}) {op} {r}"
+                    "case when c.type = {NEW_TYPE} then false else (extract_fsrs_retrievability(c.data, case when c.odue !=0 then c.odue else c.due end, c.ivl, {elap}, {next_day_at}, {now}) {op} {r}) end"
                 )
                 .unwrap()
             }

rslib/src/storage/card/filtered.rs

@@ -54,7 +54,7 @@ fn build_retrievability_query(
 ) -> String {
     if fsrs {
         format!(
-            "extract_fsrs_relative_retrievability(c.data, case when c.odue !=0 then c.odue else c.due end, {today}, ivl, {next_day_at}, {now}) {order}"
+            "extract_fsrs_relative_retrievability(c.data, case when c.odue !=0 then c.odue else c.due end, ivl, {today}, {next_day_at}, {now}) {order}"
         )
     } else {
         format!(

rslib/src/storage/card/mod.rs

@@ -837,7 +837,7 @@ impl fmt::Display for ReviewOrderSubclause {
                 let next_day_at = timing.next_day_at.0;
                 let now = timing.now.0;
                 temp_string =
-                    format!("extract_fsrs_relative_retrievability(data, case when odue !=0 then odue else due end, {today}, ivl, {next_day_at}, {now}) {order}");
+                    format!("extract_fsrs_relative_retrievability(data, case when odue !=0 then odue else due end, ivl, {today}, {next_day_at}, {now}) {order}");
                 &temp_string
             }
             ReviewOrderSubclause::Added => "nid asc, ord asc",

rslib/src/storage/sqlite.rs

@@ -332,23 +332,30 @@ fn add_extract_fsrs_retrievability(db: &Connection) -> rusqlite::Result<()> {
                 return Ok(None);
             };
             let seconds_elapsed = if let Some(last_review_time) = card_data.last_review_time {
-                now.saturating_sub(last_review_time.0) as u32
+                // This and any following
+                // (x as u32).saturating_sub(y as u32)
+                // must not be changed to
+                // x.saturating_sub(y) as u32
+                // as x and y are i64's and saturating_sub will therfore allow negative numbers
+                // before converting to u32 in the latter example.
+                (now as u32).saturating_sub(last_review_time.0 as u32)
             } else if due > 365_000 {
                 // (re)learning card in seconds
                 let Ok(ivl) = ctx.get_raw(2).as_i64() else {
                     return Ok(None);
                 };
-                let last_review_time = due.saturating_sub(ivl);
+                let last_review_time = (due as u32).saturating_sub(ivl as u32);
-                now.saturating_sub(last_review_time) as u32
+                (now as u32).saturating_sub(last_review_time)
             } else {
                 let Ok(ivl) = ctx.get_raw(2).as_i64() else {
                     return Ok(None);
                 };
-                let Ok(days_elapsed) = ctx.get_raw(3).as_i64() else {
+                // timing.days_elapsed
+                let Ok(today) = ctx.get_raw(3).as_i64() else {
                     return Ok(None);
                 };
-                let review_day = due.saturating_sub(ivl);
+                let review_day = (due as u32).saturating_sub(ivl as u32);
-                days_elapsed.saturating_sub(review_day) as u32 * 86_400
+                (today as u32).saturating_sub(review_day) * 86_400
             };
             let decay = card_data.decay.unwrap_or(FSRS5_DEFAULT_DECAY);
             let retrievability = card_data.memory_state().map(|state| {
@@ -364,7 +371,7 @@ fn add_extract_fsrs_retrievability(db: &Connection) -> rusqlite::Result<()> {
 }
 
 /// eg. extract_fsrs_relative_retrievability(card.data, card.due,
-/// timing.days_elapsed, card.ivl, timing.next_day_at, timing.now) -> float |
+/// card.ivl, timing.days_elapsed, timing.next_day_at, timing.now) -> float |
 /// null. The higher the number, the higher the card's retrievability relative
 /// to the configured desired retention.
 fn add_extract_fsrs_relative_retrievability(db: &Connection) -> rusqlite::Result<()> {
@@ -378,25 +385,32 @@ fn add_extract_fsrs_relative_retrievability(db: &Connection) -> rusqlite::Result
             let Ok(due) = ctx.get_raw(1).as_i64() else {
                 return Ok(None);
             };
-            let Ok(interval) = ctx.get_raw(3).as_i64() else {
+            let Ok(interval) = ctx.get_raw(2).as_i64() else {
                 return Ok(None);
             };
+            /*
+            // Unused
             let Ok(next_day_at) = ctx.get_raw(4).as_i64() else {
                 return Ok(None);
             };
+            */
             let Ok(now) = ctx.get_raw(5).as_i64() else {
                 return Ok(None);
             };
-            let days_elapsed = if due > 365_000 {
+            let secs_elapsed = if due > 365_000 {
-                // (re)learning
+                // (re)learning card with due in seconds
-                (next_day_at as u32).saturating_sub(due as u32) / 86_400
+
+                // Don't change this to now.subtracting_sub(due) as u32
+                // for the same reasons listed in the comment
+                // in add_extract_fsrs_retrievability
+                (now as u32).saturating_sub(due as u32)
             } else {
-                let Ok(days_elapsed) = ctx.get_raw(2).as_i64() else {
+                // timing.days_elapsed
+                let Ok(today) = ctx.get_raw(3).as_i64() else {
                     return Ok(None);
                 };
                 let review_day = due.saturating_sub(interval);
-
+                (today as u32).saturating_sub(review_day as u32) * 86_400
-                (days_elapsed as u32).saturating_sub(review_day as u32)
             };
             if let Ok(card_data) = ctx.get_raw(0).as_str() {
                 if !card_data.is_empty() {
@@ -410,23 +424,12 @@ fn add_extract_fsrs_relative_retrievability(db: &Connection) -> rusqlite::Result
 
                         let seconds_elapsed =
                             if let Some(last_review_time) = card_data.last_review_time {
-                                now.saturating_sub(last_review_time.0) as u32
+                                // Don't change this to now.subtracting_sub(due) as u32
-                            } else if due > 365_000 {
+                                // for the same reasons listed in the comment
-                                // (re)learning card in seconds
+                                // in add_extract_fsrs_retrievability
-                                let Ok(ivl) = ctx.get_raw(2).as_i64() else {
+                                (now as u32).saturating_sub(last_review_time.0 as u32)
-                                    return Ok(None);
-                                };
-                                let last_review_time = due.saturating_sub(ivl);
-                                now.saturating_sub(last_review_time) as u32
                             } else {
-                                let Ok(ivl) = ctx.get_raw(2).as_i64() else {
+                                secs_elapsed
-                                    return Ok(None);
-                                };
-                                let Ok(days_elapsed) = ctx.get_raw(3).as_i64() else {
-                                    return Ok(None);
-                                };
-                                let review_day = due.saturating_sub(ivl);
-                                days_elapsed.saturating_sub(review_day) as u32 * 86_400
                             };
 
                         let current_retrievability = FSRS::new(None)
@@ -441,7 +444,7 @@ fn add_extract_fsrs_relative_retrievability(db: &Connection) -> rusqlite::Result
                     }
                 }
             }
-
+            let days_elapsed = secs_elapsed / 86_400;
             // FSRS data missing; fall back to SM2 ordering
             Ok(Some(
                 -((days_elapsed as f32) + 0.001) / (interval as f32).max(1.0),