From 2399bf492add0d3c742a595a4e32781ca80209d9 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Mon, 20 Nov 2023 13:59:36 +1000 Subject: [PATCH] Fix FSRS retrievability sorting issues - We must use interval, not stability to infer days_elapsed - We must use original due date in a filtered deck - Use retrievability in filtered deck sorting, not just regular sorting --- rslib/src/scheduler/filtered/mod.rs | 6 ++++-- rslib/src/search/mod.rs | 2 +- rslib/src/search/sqlwriter.rs | 2 +- rslib/src/storage/card/filtered.rs | 15 ++++++++++----- rslib/src/storage/card/mod.rs | 3 ++- rslib/src/storage/sqlite.rs | 13 ++++++++----- 6 files changed, 26 insertions(+), 15 deletions(-) diff --git a/rslib/src/scheduler/filtered/mod.rs b/rslib/src/scheduler/filtered/mod.rs index 31e5cb518..56c2f45f5 100644 --- a/rslib/src/scheduler/filtered/mod.rs +++ b/rslib/src/scheduler/filtered/mod.rs @@ -98,8 +98,9 @@ impl Collection { fn build_filtered_deck(&mut self, ctx: DeckFilterContext) -> Result { let start = -100_000; let mut position = start; + let fsrs = self.get_config_bool(BoolKey::Fsrs); for term in ctx.config.search_terms.iter().take(2) { - position = self.move_cards_matching_term(&ctx, term, position)?; + position = self.move_cards_matching_term(&ctx, term, position, fsrs)?; } Ok((position - start) as usize) @@ -112,6 +113,7 @@ impl Collection { ctx: &DeckFilterContext, term: &FilteredSearchTerm, mut position: i32, + fsrs: bool, ) -> Result { let search = format!( "{} -is:suspended -is:buried -deck:filtered", @@ -121,7 +123,7 @@ impl Collection { format!("({})", term.search) } ); - let order = order_and_limit_for_search(term, ctx.today, TimestampSecs::now().0); + let order = order_and_limit_for_search(term, ctx.today, TimestampSecs::now().0, fsrs); for mut card in self.all_cards_for_search_in_order(&search, SortMode::Custom(order))? { let original = card.clone(); diff --git a/rslib/src/search/mod.rs b/rslib/src/search/mod.rs index 3e15366e3..e3cc15f16 100644 --- a/rslib/src/search/mod.rs +++ b/rslib/src/search/mod.rs @@ -377,7 +377,7 @@ fn card_order_from_sort_column(column: Column, timing: SchedTimingToday) -> Cow< Column::Stability => "extract_fsrs_variable(c.data, 's') asc".into(), Column::Difficulty => "extract_fsrs_variable(c.data, 'd') asc".into(), Column::Retrievability => format!( - "extract_fsrs_retrievability(c.data, c.due, c.ivl, {}) asc", + "extract_fsrs_retrievability(c.data, case when c.odue !=0 then c.odue else c.due end, c.ivl, {}) asc", timing.days_elapsed ) .into(), diff --git a/rslib/src/search/sqlwriter.rs b/rslib/src/search/sqlwriter.rs index 94880e1ae..95f1ff3b7 100644 --- a/rslib/src/search/sqlwriter.rs +++ b/rslib/src/search/sqlwriter.rs @@ -383,7 +383,7 @@ impl SqlWriter<'_> { let elap = self.col.timing_today()?.days_elapsed; write!( self.sql, - "extract_fsrs_retrievability(c.data, c.due, c.ivl, {elap}) {op} {r}" + "extract_fsrs_retrievability(c.data, case when c.odue !=0 then c.odue else c.due end, c.ivl, {elap}) {op} {r}" ) .unwrap() } diff --git a/rslib/src/storage/card/filtered.rs b/rslib/src/storage/card/filtered.rs index a84a4fed0..babfb1521 100644 --- a/rslib/src/storage/card/filtered.rs +++ b/rslib/src/storage/card/filtered.rs @@ -9,6 +9,7 @@ pub(crate) fn order_and_limit_for_search( term: &FilteredSearchTerm, today: u32, current_timestamp: i64, + fsrs: bool, ) -> String { let temp_string; let order = match term.order() { @@ -25,13 +26,17 @@ pub(crate) fn order_and_limit_for_search( &temp_string } FilteredSearchOrder::DuePriority => { - temp_string = format!( - " + temp_string = if fsrs { + format!("extract_fsrs_relative_overdueness(data, due, {today}, ivl) desc") + } else { + format!( + " (case when queue={rev_queue} and due <= {today} then (ivl / cast({today}-due+0.001 as real)) else 100000+due end)", - rev_queue = CardQueue::Review as i8, - today = today - ); + rev_queue = CardQueue::Review as i8, + today = today + ) + }; &temp_string } }; diff --git a/rslib/src/storage/card/mod.rs b/rslib/src/storage/card/mod.rs index f40005f0f..f69024c35 100644 --- a/rslib/src/storage/card/mod.rs +++ b/rslib/src/storage/card/mod.rs @@ -730,7 +730,8 @@ impl fmt::Display for ReviewOrderSubclause { &temp_string } ReviewOrderSubclause::RelativeOverduenessFsrs { today } => { - temp_string = format!("extract_fsrs_relative_overdueness(data, due, {today}) desc"); + temp_string = + format!("extract_fsrs_relative_overdueness(data, due, {today}, ivl) desc"); &temp_string } }; diff --git a/rslib/src/storage/sqlite.rs b/rslib/src/storage/sqlite.rs index 3b5456f55..60190ab85 100644 --- a/rslib/src/storage/sqlite.rs +++ b/rslib/src/storage/sqlite.rs @@ -307,15 +307,15 @@ fn add_extract_fsrs_retrievability(db: &Connection) -> rusqlite::Result<()> { ) } -/// eg. extract_fsrs_retrievability(card.data, card.due, timing.days_elapsed) -> -/// float | null. The higher the number, the more overdue. +/// eg. extract_fsrs_retrievability(card.data, card.due, timing.days_elapsed, +/// card.ivl) -> float | null. The higher the number, the more overdue. fn add_extract_fsrs_relative_overdueness(db: &Connection) -> rusqlite::Result<()> { db.create_scalar_function( "extract_fsrs_relative_overdueness", - 3, + 4, FunctionFlags::SQLITE_DETERMINISTIC, move |ctx| { - assert_eq!(ctx.len(), 3, "called with unexpected number of arguments"); + assert_eq!(ctx.len(), 4, "called with unexpected number of arguments"); let Ok(card_data) = ctx.get_raw(0).as_str() else { return Ok(None); @@ -334,6 +334,9 @@ fn add_extract_fsrs_relative_overdueness(db: &Connection) -> rusqlite::Result<() let Ok(days_elapsed) = ctx.get_raw(2).as_i64() else { return Ok(None); }; + let Ok(interval) = ctx.get_raw(3).as_i64() else { + return Ok(None); + }; let Some(state) = card_data.memory_state() else { return Ok(None); }; @@ -343,7 +346,7 @@ fn add_extract_fsrs_relative_overdueness(db: &Connection) -> rusqlite::Result<() // avoid div by zero desired_retrievability = desired_retrievability.max(0.0001); - let review_day = due.saturating_sub(state.stability as i64); + let review_day = due.saturating_sub(interval); let days_elapsed = days_elapsed.saturating_sub(review_day) as u32; let current_retrievability = FSRS::new(None) .unwrap()