diff --git a/ftl/core/database-check.ftl b/ftl/core/database-check.ftl index 8a9e4e178..ce2f827cd 100644 --- a/ftl/core/database-check.ftl +++ b/ftl/core/database-check.ftl @@ -5,6 +5,11 @@ database-check-card-properties = [one] Fixed { $count } invalid card property. *[other] Fixed { $count } invalid card properties. } +database-check-card-last-review-time-empty = + { $count -> + [one] Added last review time to { $count } card. + *[other] Added last review time to { $count } cards. + } database-check-missing-templates = { $count -> [one] Deleted { $count } card with missing template. diff --git a/rslib/src/dbcheck.rs b/rslib/src/dbcheck.rs index f58a2184a..42b9977be 100644 --- a/rslib/src/dbcheck.rs +++ b/rslib/src/dbcheck.rs @@ -24,6 +24,7 @@ use crate::notetype::NotetypeId; use crate::notetype::NotetypeKind; use crate::prelude::*; use crate::progress::ThrottlingProgressHandler; +use crate::storage::card::CardFixStats; use crate::timestamp::TimestampMillis; use crate::timestamp::TimestampSecs; @@ -40,6 +41,7 @@ pub struct CheckDatabaseOutput { notetypes_recovered: usize, invalid_utf8: usize, invalid_ids: usize, + card_last_review_time_empty: usize, } #[derive(Debug, Clone, Copy, Default)] @@ -69,6 +71,11 @@ impl CheckDatabaseOutput { if self.card_properties_invalid > 0 { probs.push(tr.database_check_card_properties(self.card_properties_invalid)); } + if self.card_last_review_time_empty > 0 { + probs.push( + tr.database_check_card_last_review_time_empty(self.card_last_review_time_empty), + ); + } if self.cards_missing_note > 0 { probs.push(tr.database_check_card_missing_note(self.cards_missing_note)); } @@ -158,14 +165,25 @@ impl Collection { fn check_card_properties(&mut self, out: &mut CheckDatabaseOutput) -> Result<()> { let timing = self.timing_today()?; - let (new_cnt, other_cnt) = self.storage.fix_card_properties( + let CardFixStats { + new_cards_fixed, + other_cards_fixed, + last_review_time_fixed, + } = self.storage.fix_card_properties( timing.days_elapsed, TimestampSecs::now(), self.usn()?, self.scheduler_version() == SchedulerVersion::V1, )?; - out.card_position_too_high = new_cnt; - out.card_properties_invalid += other_cnt; + out.card_position_too_high = new_cards_fixed; + out.card_properties_invalid += other_cards_fixed; + out.card_last_review_time_empty = last_review_time_fixed; + + // Trigger one-way sync if last_review_time was updated to avoid conflicts + if last_review_time_fixed > 0 { + self.set_schema_modified()?; + } + Ok(()) } diff --git a/rslib/src/revlog/mod.rs b/rslib/src/revlog/mod.rs index ad7f30261..f52698388 100644 --- a/rslib/src/revlog/mod.rs +++ b/rslib/src/revlog/mod.rs @@ -84,6 +84,42 @@ impl RevlogEntry { }) .unwrap() } + + /// Returns true if this entry represents a reset operation. + /// These entries are created when a card is reset using + /// [`Collection::reschedule_cards_as_new`]. + /// The 0 value of `ease_factor` differentiates it + /// from entry created by [`Collection::set_due_date`] that has + /// `RevlogReviewKind::Manual` but non-zero `ease_factor`. + pub(crate) fn is_reset(&self) -> bool { + self.review_kind == RevlogReviewKind::Manual && self.ease_factor == 0 + } + + /// Returns true if this entry represents a cramming operation. + /// These entries are created when a card is reviewed in a + /// filtered deck with "Reschedule cards based on my answers + /// in this deck" disabled. + /// [`crate::scheduler::answering::CardStateUpdater::apply_preview_state`]. + /// The 0 value of `ease_factor` distinguishes it from the entry + /// created when a card is reviewed before its due date in a + /// filtered deck with reschedule enabled or using Grade Now. + pub(crate) fn is_cramming(&self) -> bool { + self.review_kind == RevlogReviewKind::Filtered && self.ease_factor == 0 + } + + pub(crate) fn has_rating(&self) -> bool { + self.button_chosen > 0 + } + + /// Returns true if the review entry is not manually rescheduled and not + /// cramming. Used to filter out entries that shouldn't be considered + /// for statistics and scheduling. + pub(crate) fn has_rating_and_affects_scheduling(&self) -> bool { + // not rescheduled/set due date/reset + self.has_rating() + // not cramming + && !self.is_cramming() + } } impl Collection { diff --git a/rslib/src/scheduler/fsrs/memory_state.rs b/rslib/src/scheduler/fsrs/memory_state.rs index 199b19329..062f5bcca 100644 --- a/rslib/src/scheduler/fsrs/memory_state.rs +++ b/rslib/src/scheduler/fsrs/memory_state.rs @@ -306,15 +306,15 @@ pub(crate) fn fsrs_items_for_memory_states( .collect() } -struct LastRevlogInfo { +pub(crate) struct LastRevlogInfo { /// Used to determine the actual elapsed time between the last time the user /// reviewed the card and now, so that we can determine an accurate period /// when the card has subsequently been rescheduled to a different day. - last_reviewed_at: Option, + pub(crate) last_reviewed_at: Option, } -/// Return a map of cards to info about last review/reschedule. -fn get_last_revlog_info(revlogs: &[RevlogEntry]) -> HashMap { +/// Return a map of cards to info about last review. +pub(crate) fn get_last_revlog_info(revlogs: &[RevlogEntry]) -> HashMap { let mut out = HashMap::new(); revlogs .iter() @@ -323,8 +323,10 @@ fn get_last_revlog_info(revlogs: &[RevlogEntry]) -> HashMap= 1 { + if e.has_rating_and_affects_scheduling() { last_reviewed_at = Some(e.id.as_secs()); + } else if e.is_reset() { + last_reviewed_at = None; } } out.insert(card_id, LastRevlogInfo { last_reviewed_at }); diff --git a/rslib/src/scheduler/fsrs/params.rs b/rslib/src/scheduler/fsrs/params.rs index 63bdebe79..726870fe1 100644 --- a/rslib/src/scheduler/fsrs/params.rs +++ b/rslib/src/scheduler/fsrs/params.rs @@ -394,13 +394,13 @@ pub(crate) fn reviews_for_fsrs( let mut revlogs_complete = false; // Working backwards from the latest review... for (index, entry) in entries.iter().enumerate().rev() { - if entry.review_kind == RevlogReviewKind::Filtered && entry.ease_factor == 0 { + if entry.is_cramming() { continue; } // For incomplete review histories, initial memory state is based on the first // user-graded review after the cutoff date with interval >= 1d. let within_cutoff = entry.id.0 > ignore_revlogs_before.0; - let user_graded = matches!(entry.button_chosen, 1..=4); + let user_graded = entry.has_rating(); let interday = entry.interval >= 1 || entry.interval <= -86400; if user_graded && within_cutoff && interday { first_user_grade_idx = Some(index); @@ -409,10 +409,7 @@ pub(crate) fn reviews_for_fsrs( if user_graded && entry.review_kind == RevlogReviewKind::Learning { first_of_last_learn_entries = Some(index); revlogs_complete = true; - } else if matches!( - (entry.review_kind, entry.ease_factor), - (RevlogReviewKind::Manual, 0) - ) { + } else if entry.is_reset() { // Ignore entries prior to a `Reset` if a learning step has come after, // but consider revlogs complete. if first_of_last_learn_entries.is_some() { @@ -472,16 +469,7 @@ pub(crate) fn reviews_for_fsrs( } // Filter out unwanted entries - entries.retain(|entry| { - !( - // set due date, reset or rescheduled - (entry.review_kind == RevlogReviewKind::Manual || entry.button_chosen == 0) - || // cram - (entry.review_kind == RevlogReviewKind::Filtered && entry.ease_factor == 0) - || // rescheduled - (entry.review_kind == RevlogReviewKind::Rescheduled) - ) - }); + entries.retain(|entry| entry.has_rating_and_affects_scheduling()); // Compute delta_t for each entry let delta_ts = iter::once(0) @@ -560,10 +548,14 @@ pub(crate) mod tests { } pub(crate) fn revlog(review_kind: RevlogReviewKind, days_ago: i64) -> RevlogEntry { + let button_chosen = match review_kind { + RevlogReviewKind::Manual | RevlogReviewKind::Rescheduled => 0, + _ => 3, + }; RevlogEntry { review_kind, id: days_ago_ms(days_ago).into(), - button_chosen: 3, + button_chosen, interval: 1, ..Default::default() } diff --git a/rslib/src/stats/card.rs b/rslib/src/stats/card.rs index ee8592d91..008977fe9 100644 --- a/rslib/src/stats/card.rs +++ b/rslib/src/stats/card.rs @@ -197,7 +197,7 @@ impl Collection { } fn average_and_total_secs_strings(revlog: &[RevlogEntry]) -> (f32, f32) { - let normal_answer_count = revlog.iter().filter(|r| r.button_chosen > 0).count(); + let normal_answer_count = revlog.iter().filter(|r| r.has_rating()).count(); let total_secs: f32 = revlog .iter() .map(|entry| (entry.taken_millis as f32) / 1000.0) diff --git a/rslib/src/stats/graphs/retention.rs b/rslib/src/stats/graphs/retention.rs index c21f43301..231a892f0 100644 --- a/rslib/src/stats/graphs/retention.rs +++ b/rslib/src/stats/graphs/retention.rs @@ -53,10 +53,7 @@ impl GraphsContext { self.revlog .iter() .filter(|review| { - // not rescheduled/set due date/reset - review.button_chosen > 0 - // not cramming - && (review.review_kind != RevlogReviewKind::Filtered || review.ease_factor != 0) + review.has_rating_and_affects_scheduling() // cards with an interval ≥ 1 day && (review.review_kind == RevlogReviewKind::Review || review.last_interval <= -86400 diff --git a/rslib/src/storage/card/mod.rs b/rslib/src/storage/card/mod.rs index a1db247c3..1d0d62fd7 100644 --- a/rslib/src/storage/card/mod.rs +++ b/rslib/src/storage/card/mod.rs @@ -33,6 +33,7 @@ use crate::decks::DeckKind; use crate::error::Result; use crate::notes::NoteId; use crate::scheduler::congrats::CongratsInfo; +use crate::scheduler::fsrs::memory_state::get_last_revlog_info; use crate::scheduler::queue::BuryMode; use crate::scheduler::queue::DueCard; use crate::scheduler::queue::DueCardKind; @@ -42,6 +43,13 @@ use crate::timestamp::TimestampMillis; use crate::timestamp::TimestampSecs; use crate::types::Usn; +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct CardFixStats { + pub new_cards_fixed: usize, + pub other_cards_fixed: usize, + pub last_review_time_fixed: usize, +} + impl FromSql for CardType { fn column_result(value: ValueRef<'_>) -> result::Result { if let ValueRef::Integer(i) = value { @@ -365,7 +373,7 @@ impl super::SqliteStorage { mtime: TimestampSecs, usn: Usn, v1_sched: bool, - ) -> Result<(usize, usize)> { + ) -> Result { let new_cnt = self .db .prepare(include_str!("fix_due_new.sql"))? @@ -390,7 +398,24 @@ impl super::SqliteStorage { .db .prepare(include_str!("fix_ordinal.sql"))? .execute(params![mtime, usn])?; - Ok((new_cnt, other_cnt)) + let mut last_review_time_cnt = 0; + let revlog = self.get_all_revlog_entries_in_card_order()?; + let last_revlog_info = get_last_revlog_info(&revlog); + for (card_id, last_revlog_info) in last_revlog_info { + let card = self.get_card(card_id)?; + if let Some(mut card) = card { + if card.ctype != CardType::New && card.last_review_time.is_none() { + card.last_review_time = last_revlog_info.last_reviewed_at; + self.update_card(&card)?; + last_review_time_cnt += 1; + } + } + } + Ok(CardFixStats { + new_cards_fixed: new_cnt, + other_cards_fixed: other_cnt, + last_review_time_fixed: last_review_time_cnt, + }) } pub(crate) fn delete_orphaned_cards(&self) -> Result {