From 987c1825a60f1ffd06879a1e350d09ae249f94b5 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Fri, 3 Nov 2023 10:07:47 +1000 Subject: [PATCH] Fix panic when enabling FSRS with add-on-rescheduled cards https://forums.ankiweb.net/t/error-upon-fsrs-activation-on-anki-23-10/36488 --- rslib/src/scheduler/fsrs/memory_state.rs | 58 +++++++++++++++--------- rslib/src/scheduler/fsrs/weights.rs | 18 +++++--- 2 files changed, 48 insertions(+), 28 deletions(-) diff --git a/rslib/src/scheduler/fsrs/memory_state.rs b/rslib/src/scheduler/fsrs/memory_state.rs index e23982014..9720f1c84 100644 --- a/rslib/src/scheduler/fsrs/memory_state.rs +++ b/rslib/src/scheduler/fsrs/memory_state.rs @@ -12,7 +12,6 @@ use itertools::Itertools; use crate::card::CardType; use crate::prelude::*; use crate::revlog::RevlogEntry; -use crate::revlog::RevlogReviewKind; use crate::scheduler::fsrs::weights::single_card_revlog_to_items; use crate::scheduler::fsrs::weights::Weights; use crate::scheduler::states::fuzz::with_review_fuzz; @@ -235,27 +234,39 @@ pub(crate) fn single_card_revlog_to_item( next_day_at: TimestampSecs, sm2_retention: f32, ) -> Option { - let have_learning = entries + struct FirstReview { + interval: f32, + ease_factor: f32, + } + let first_review = entries .iter() - .any(|e| e.review_kind == RevlogReviewKind::Learning); - if have_learning { - let items = single_card_revlog_to_items(entries, next_day_at, false); - Some(FsrsItemWithStartingState { - item: items.unwrap().pop().unwrap(), - starting_state: None, - }) - } else if let Some(first_review) = entries.iter().find(|e| e.button_chosen > 0) { - let ease_factor = if first_review.ease_factor == 0 { - 2500 - } else { - first_review.ease_factor - }; - let interval = first_review.interval.max(1); - let starting_state = - fsrs.memory_state_from_sm2(ease_factor as f32 / 1000.0, interval as f32, sm2_retention); - let items = single_card_revlog_to_items(entries, next_day_at, false); - items.and_then(|mut items| { - let mut item = items.pop().unwrap(); + .find(|e| e.button_chosen > 0) + .map(|e| FirstReview { + interval: e.interval.max(1) as f32, + ease_factor: if e.ease_factor == 0 { + 2500 + } else { + e.ease_factor + } as f32 + / 1000.0, + }); + if let Some((mut items, found_learning)) = + single_card_revlog_to_items(entries, next_day_at, false) + { + let mut item = items.pop().unwrap(); + if found_learning { + // we assume the revlog is complete + Some(FsrsItemWithStartingState { + item, + starting_state: None, + }) + } else if let Some(first_review) = first_review { + // the revlog has been truncated, but not fully + let starting_state = fsrs.memory_state_from_sm2( + first_review.ease_factor, + first_review.interval, + sm2_retention, + ); item.reviews.remove(0); if item.reviews.is_empty() { None @@ -265,7 +276,10 @@ pub(crate) fn single_card_revlog_to_item( starting_state: Some(starting_state), }) } - }) + } else { + // only manual rescheduling; treat like empty + None + } } else { None } diff --git a/rslib/src/scheduler/fsrs/weights.rs b/rslib/src/scheduler/fsrs/weights.rs index c9e771c52..ff066e4b3 100644 --- a/rslib/src/scheduler/fsrs/weights.rs +++ b/rslib/src/scheduler/fsrs/weights.rs @@ -101,7 +101,7 @@ fn fsrs_items_for_training(revlogs: Vec, next_day_at: TimestampSecs .filter_map(|(_cid, entries)| { single_card_revlog_to_items(entries.collect(), next_day_at, true) }) - .flatten() + .flat_map(|i| i.0) .collect_vec(); revlogs.sort_by_cached_key(|r| r.reviews.len()); revlogs @@ -111,16 +111,22 @@ fn fsrs_items_for_training(revlogs: Vec, next_day_at: TimestampSecs /// expects multiple items for a given card when training - for revlog /// `[1,2,3]`, we create FSRSItems corresponding to `[1,2]` and `[1,2,3]` /// in training, and `[1]`, [1,2]` and `[1,2,3]` when calculating memory -/// state. +/// state. Returns (items, found_learn_entry), the latter of which is used +/// to determine whether the revlogs have been truncated when not training. pub(crate) fn single_card_revlog_to_items( mut entries: Vec, next_day_at: TimestampSecs, training: bool, -) -> Option> { +) -> Option<(Vec, bool)> { let mut last_learn_entry = None; + let mut found_learn_entry = false; for (index, entry) in entries.iter().enumerate().rev() { - if entry.review_kind == RevlogReviewKind::Learning { + if matches!( + (entry.review_kind, entry.button_chosen), + (RevlogReviewKind::Learning, 1..=4) + ) { last_learn_entry = Some(index); + found_learn_entry = true; } else if last_learn_entry.is_some() { break; } @@ -199,7 +205,7 @@ pub(crate) fn single_card_revlog_to_items( if items.is_empty() { None } else { - Some(items) + Some((items, found_learn_entry)) } } @@ -229,7 +235,7 @@ pub(crate) mod tests { } pub(crate) fn convert(revlog: &[RevlogEntry], training: bool) -> Option> { - single_card_revlog_to_items(revlog.to_vec(), NEXT_DAY_AT, training) + single_card_revlog_to_items(revlog.to_vec(), NEXT_DAY_AT, training).map(|i| i.0) } #[macro_export]