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
This commit is contained in:
Damien Elmes 2023-11-20 13:59:36 +10:00
parent 1e477406e6
commit 2399bf492a
6 changed files with 26 additions and 15 deletions

View File

@ -98,8 +98,9 @@ impl Collection {
fn build_filtered_deck(&mut self, ctx: DeckFilterContext) -> Result<usize> { fn build_filtered_deck(&mut self, ctx: DeckFilterContext) -> Result<usize> {
let start = -100_000; let start = -100_000;
let mut position = start; let mut position = start;
let fsrs = self.get_config_bool(BoolKey::Fsrs);
for term in ctx.config.search_terms.iter().take(2) { 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) Ok((position - start) as usize)
@ -112,6 +113,7 @@ impl Collection {
ctx: &DeckFilterContext, ctx: &DeckFilterContext,
term: &FilteredSearchTerm, term: &FilteredSearchTerm,
mut position: i32, mut position: i32,
fsrs: bool,
) -> Result<i32> { ) -> Result<i32> {
let search = format!( let search = format!(
"{} -is:suspended -is:buried -deck:filtered", "{} -is:suspended -is:buried -deck:filtered",
@ -121,7 +123,7 @@ impl Collection {
format!("({})", term.search) 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))? { for mut card in self.all_cards_for_search_in_order(&search, SortMode::Custom(order))? {
let original = card.clone(); let original = card.clone();

View File

@ -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::Stability => "extract_fsrs_variable(c.data, 's') asc".into(),
Column::Difficulty => "extract_fsrs_variable(c.data, 'd') asc".into(), Column::Difficulty => "extract_fsrs_variable(c.data, 'd') asc".into(),
Column::Retrievability => format!( 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 timing.days_elapsed
) )
.into(), .into(),

View File

@ -383,7 +383,7 @@ impl SqlWriter<'_> {
let elap = self.col.timing_today()?.days_elapsed; let elap = self.col.timing_today()?.days_elapsed;
write!( write!(
self.sql, 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() .unwrap()
} }

View File

@ -9,6 +9,7 @@ pub(crate) fn order_and_limit_for_search(
term: &FilteredSearchTerm, term: &FilteredSearchTerm,
today: u32, today: u32,
current_timestamp: i64, current_timestamp: i64,
fsrs: bool,
) -> String { ) -> String {
let temp_string; let temp_string;
let order = match term.order() { let order = match term.order() {
@ -25,13 +26,17 @@ pub(crate) fn order_and_limit_for_search(
&temp_string &temp_string
} }
FilteredSearchOrder::DuePriority => { 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} (case when queue={rev_queue} and due <= {today}
then (ivl / cast({today}-due+0.001 as real)) else 100000+due end)", then (ivl / cast({today}-due+0.001 as real)) else 100000+due end)",
rev_queue = CardQueue::Review as i8, rev_queue = CardQueue::Review as i8,
today = today today = today
); )
};
&temp_string &temp_string
} }
}; };

View File

@ -730,7 +730,8 @@ impl fmt::Display for ReviewOrderSubclause {
&temp_string &temp_string
} }
ReviewOrderSubclause::RelativeOverduenessFsrs { today } => { 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 &temp_string
} }
}; };

View File

@ -307,15 +307,15 @@ fn add_extract_fsrs_retrievability(db: &Connection) -> rusqlite::Result<()> {
) )
} }
/// eg. extract_fsrs_retrievability(card.data, card.due, timing.days_elapsed) -> /// eg. extract_fsrs_retrievability(card.data, card.due, timing.days_elapsed,
/// float | null. The higher the number, the more overdue. /// card.ivl) -> float | null. The higher the number, the more overdue.
fn add_extract_fsrs_relative_overdueness(db: &Connection) -> rusqlite::Result<()> { fn add_extract_fsrs_relative_overdueness(db: &Connection) -> rusqlite::Result<()> {
db.create_scalar_function( db.create_scalar_function(
"extract_fsrs_relative_overdueness", "extract_fsrs_relative_overdueness",
3, 4,
FunctionFlags::SQLITE_DETERMINISTIC, FunctionFlags::SQLITE_DETERMINISTIC,
move |ctx| { 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 { let Ok(card_data) = ctx.get_raw(0).as_str() else {
return Ok(None); 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 { let Ok(days_elapsed) = ctx.get_raw(2).as_i64() else {
return Ok(None); return Ok(None);
}; };
let Ok(interval) = ctx.get_raw(3).as_i64() else {
return Ok(None);
};
let Some(state) = card_data.memory_state() else { let Some(state) = card_data.memory_state() else {
return Ok(None); return Ok(None);
}; };
@ -343,7 +346,7 @@ fn add_extract_fsrs_relative_overdueness(db: &Connection) -> rusqlite::Result<()
// avoid div by zero // avoid div by zero
desired_retrievability = desired_retrievability.max(0.0001); 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 days_elapsed = days_elapsed.saturating_sub(review_day) as u32;
let current_retrievability = FSRS::new(None) let current_retrievability = FSRS::new(None)
.unwrap() .unwrap()