Minor refactoring

- fsrs_items_for_memory_state - fsrs_items_for_memory_states
- single_card_revlog_to_item -> fsrs_item_for_memory_state
(to match fsrs_items_for_training)
- single_card_revlog_to_items -> reviews_for_fsrs
- Use struct instead of tuple for reviews_for_fsrs output
- Don't return count, since we're already returning the filtered list

rslib/src/scheduler/answering/mod.rs

@@ -32,7 +32,7 @@ use crate::deckconfig::DeckConfig;
 use crate::deckconfig::LeechAction;
 use crate::decks::Deck;
 use crate::prelude::*;
-use crate::scheduler::fsrs::memory_state::single_card_revlog_to_item;
+use crate::scheduler::fsrs::memory_state::fsrs_item_for_memory_state;
 use crate::scheduler::states::PreviewState;
 use crate::search::SearchNode;
 
@@ -437,7 +437,7 @@ impl Collection {
                 // and will need its initial memory state to be calculated based on review
                 // history.
                 let revlog = self.revlog_for_srs(SearchNode::CardIds(card.id.to_string()))?;
-                let item = single_card_revlog_to_item(
+                let item = fsrs_item_for_memory_state(
                     &fsrs,
                     revlog,
                     timing.next_day_at,

rslib/src/scheduler/fsrs/memory_state.rs

@@ -14,7 +14,7 @@ use crate::card::CardType;
 use crate::prelude::*;
 use crate::revlog::RevlogEntry;
 use crate::revlog::RevlogReviewKind;
-use crate::scheduler::fsrs::params::single_card_revlog_to_items;
+use crate::scheduler::fsrs::params::reviews_for_fsrs;
 use crate::scheduler::fsrs::params::Params;
 use crate::scheduler::states::fuzz::with_review_fuzz;
 use crate::search::Negated;
@@ -71,7 +71,7 @@ impl Collection {
             };
             let fsrs = FSRS::new(req.as_ref().map(|w| &w.params[..]).or(Some([].as_slice())))?;
             let historical_retention = req.as_ref().map(|w| w.historical_retention);
-            let items = fsrs_items_for_memory_state(
+            let items = fsrs_items_for_memory_states(
                 &fsrs,
                 revlog,
                 timing.next_day_at,
@@ -156,7 +156,7 @@ impl Collection {
         let historical_retention = config.inner.historical_retention;
         let fsrs = FSRS::new(Some(config.fsrs_params()))?;
         let revlog = self.revlog_for_srs(SearchNode::CardIds(card.id.to_string()))?;
-        let item = single_card_revlog_to_item(
+        let item = fsrs_item_for_memory_state(
             &fsrs,
             revlog,
             self.timing_today()?.next_day_at,
@@ -175,7 +175,7 @@ impl Card {
     pub(crate) fn set_memory_state(
         &mut self,
         fsrs: &FSRS,
-        item: Option<FsrsItemWithStartingState>,
+        item: Option<FsrsItemForMemoryState>,
         historical_retention: f32,
     ) -> Result<()> {
         let memory_state = if let Some(i) = item {
@@ -196,22 +196,21 @@ impl Card {
 }
 
 #[derive(Debug)]
-pub(crate) struct FsrsItemWithStartingState {
+pub(crate) struct FsrsItemForMemoryState {
     pub item: FSRSItem,
     /// When revlogs have been truncated, this stores the initial state at first
     /// review
     pub starting_state: Option<MemoryState>,
 }
 
-/// When updating memory state, FSRS only requires the last FSRSItem that
+/// Like [fsrs_item_for_memory_state], but for updating multiple cards at once.
-/// contains the full history.
+pub(crate) fn fsrs_items_for_memory_states(
-pub(crate) fn fsrs_items_for_memory_state(
     fsrs: &FSRS,
     revlogs: Vec<RevlogEntry>,
     next_day_at: TimestampSecs,
     historical_retention: f32,
     ignore_revlogs_before: TimestampMillis,
-) -> Result<Vec<(CardId, Option<FsrsItemWithStartingState>)>> {
+) -> Result<Vec<(CardId, Option<FsrsItemForMemoryState>)>> {
     revlogs
         .into_iter()
         .chunk_by(|r| r.cid)
@@ -219,7 +218,7 @@ pub(crate) fn fsrs_items_for_memory_state(
         .map(|(card_id, group)| {
             Ok((
                 card_id,
-                single_card_revlog_to_item(
+                fsrs_item_for_memory_state(
                     fsrs,
                     group.collect(),
                     next_day_at,
@@ -273,27 +272,25 @@ fn get_last_revlog_info(revlogs: &[RevlogEntry]) -> HashMap<CardId, LastRevlogIn
 /// a truncated revlog), we return the starting state inferred from the first
 /// revlog entry, so that the first review is not treated as if started from
 /// scratch.
-pub(crate) fn single_card_revlog_to_item(
+pub(crate) fn fsrs_item_for_memory_state(
     fsrs: &FSRS,
     entries: Vec<RevlogEntry>,
     next_day_at: TimestampSecs,
     historical_retention: f32,
     ignore_revlogs_before: TimestampMillis,
-) -> Result<Option<FsrsItemWithStartingState>> {
+) -> Result<Option<FsrsItemForMemoryState>> {
     struct FirstReview {
         interval: f32,
         ease_factor: f32,
     }
-    if let Some((mut items, revlogs_complete, _, filtered_entries)) =
+    if let Some(mut output) = reviews_for_fsrs(entries, next_day_at, false, ignore_revlogs_before) {
-        single_card_revlog_to_items(entries, next_day_at, false, ignore_revlogs_before)
+        let mut item = output.fsrs_items.pop().unwrap();
-    {
+        if output.revlogs_complete {
-        let mut item = items.pop().unwrap();
+            Ok(Some(FsrsItemForMemoryState {
-        if revlogs_complete {
-            Ok(Some(FsrsItemWithStartingState {
                 item,
                 starting_state: None,
             }))
-        } else if let Some(first_non_manual_entry) = filtered_entries.first() {
+        } else if let Some(first_non_manual_entry) = output.filtered_revlogs.first() {
             // the revlog has been truncated, but not fully
             let first_review = FirstReview {
                 interval: first_non_manual_entry.interval.max(1) as f32,
@@ -315,7 +312,7 @@ pub(crate) fn single_card_revlog_to_item(
             }
             // remove the first review because it has been converted to the starting state
             item.reviews.remove(0);
-            Ok(Some(FsrsItemWithStartingState {
+            Ok(Some(FsrsItemForMemoryState {
                 item,
                 starting_state: Some(starting_state),
             }))
@@ -353,7 +350,7 @@ mod tests {
         // cards without any learning steps due to truncated history still have memory
         // state calculated
         let fsrs = FSRS::new(Some(&[])).unwrap();
-        let item = single_card_revlog_to_item(
+        let item = fsrs_item_for_memory_state(
             &fsrs,
             vec![
                 RevlogEntry {
@@ -389,7 +386,7 @@ mod tests {
         );
         // but if there's only a single revlog entry, we'll fall back on the first
         // non-manual entry
-        let item = single_card_revlog_to_item(
+        let item = fsrs_item_for_memory_state(
             &fsrs,
             vec![RevlogEntry {
                 ease_factor: 2500,

rslib/src/scheduler/fsrs/params.rs

@@ -229,36 +229,41 @@ fn fsrs_items_for_training(
         .chunk_by(|r| r.cid)
         .into_iter()
         .filter_map(|(_cid, entries)| {
-            single_card_revlog_to_items(entries.collect(), next_day_at, true, review_revlogs_before)
+            reviews_for_fsrs(entries.collect(), next_day_at, true, review_revlogs_before)
         })
         .flat_map(|i| {
-            review_count += i.2;
+            review_count += i.filtered_revlogs.len();
 
-            i.0
+            i.fsrs_items
         })
         .collect_vec();
     revlogs.sort_by_cached_key(|r| r.reviews.len());
     (revlogs, review_count)
 }
 
-/// Transform the revlog history for a card into a list of FSRSItems. FSRS
+pub(crate) struct ReviewsForFsrs {
-/// expects multiple items for a given card when training - for revlog
+    /// The revlog entries that remain after filtering (e.g. excluding
-/// `[1,2,3]`, we create FSRSItems corresponding to `[1,2]` and `[1,2,3]`
+    /// review entries prior to a card being reset).
-/// in training, and `[1]`, [1,2]` and `[1,2,3]` when calculating memory
+    pub filtered_revlogs: Vec<RevlogEntry>,
-/// state.
+    /// FSRS items derived from the filtered revlogs.
+    pub fsrs_items: Vec<FSRSItem>,
+    /// True if there is enough history to derive memory state from history
+    /// alone. If false, memory state will be derived from SM2.
+    pub revlogs_complete: bool,
+}
+
+/// Filter out unwanted revlog entries, then create a series of FSRS items for
+/// training/memory state calculation.
 ///
-/// Returns (items, revlog_complete, review_count).
+/// Filtering consists of removing revlog entries before the supplied timestamp,
-/// revlog_complete is assumed when the revlogs have a learning step, or start
+/// and removing items such as reviews that happened prior to a card being reset
-/// with manual scheduling. When revlogs are incomplete, the starting difficulty
+/// to new.
-/// is later inferred from the SM2 data, instead of using the standard FSRS
+pub(crate) fn reviews_for_fsrs(
-/// initial difficulty. review_count is the number of reviews used after
-/// filtering out unwanted ones.
-pub(crate) fn single_card_revlog_to_items(
     mut entries: Vec<RevlogEntry>,
     next_day_at: TimestampSecs,
     training: bool,
     ignore_revlogs_before: TimestampMillis,
-) -> Option<(Vec<FSRSItem>, bool, usize, Vec<RevlogEntry>)> {
+) -> Option<ReviewsForFsrs> {
     let mut first_of_last_learn_entries = None;
     let mut non_manual_entries = None;
     let mut revlogs_complete = false;
@@ -375,7 +380,11 @@ pub(crate) fn single_card_revlog_to_items(
     if items.is_empty() {
         None
     } else {
-        Some((items, revlogs_complete, entries.len(), entries))
+        Some(ReviewsForFsrs {
+            fsrs_items: items,
+            revlogs_complete,
+            filtered_revlogs: entries,
+        })
     }
 }
 
@@ -434,8 +443,8 @@ pub(crate) mod tests {
         training: bool,
         ignore_before: TimestampMillis,
     ) -> Option<Vec<FSRSItem>> {
-        single_card_revlog_to_items(revlog.to_vec(), NEXT_DAY_AT, training, ignore_before)
+        reviews_for_fsrs(revlog.to_vec(), NEXT_DAY_AT, training, ignore_before)
-            .map(|i| i.0)
+            .map(|i| i.fsrs_items)
     }
 
     pub(crate) fn convert(revlog: &[RevlogEntry], training: bool) -> Option<Vec<FSRSItem>> {

rslib/src/stats/card.rs

@@ -6,7 +6,7 @@ use fsrs::FSRS;
 use crate::card::CardType;
 use crate::prelude::*;
 use crate::revlog::RevlogEntry;
-use crate::scheduler::fsrs::memory_state::single_card_revlog_to_item;
+use crate::scheduler::fsrs::memory_state::fsrs_item_for_memory_state;
 use crate::scheduler::fsrs::params::ignore_revlogs_before_ms_from_config;
 use crate::scheduler::timing::is_unix_epoch_timestamp;
 
@@ -144,7 +144,7 @@ impl Collection {
 
         for entry in revlog {
             accumulated_revlog.push(entry.clone());
-            let item = single_card_revlog_to_item(
+            let item = fsrs_item_for_memory_state(
                 &fsrs,
                 accumulated_revlog.clone(),
                 next_day_at,