Suppress manual revlog entry if the previous entry was also manual

Means we lose some detail in the history, but should reduce the
collection bloat caused by a user experimenting with reschedule multiple
times in a short period, when they don't restore from backup afterwards.

https://forums.ankiweb.net/t/possible-syncing-limitation-by-fsrs-manual-scheduling-data-accumulation/37610
This commit is contained in:
Damien Elmes 2023-11-27 11:23:00 +10:00
parent 8b6abd3f8f
commit d767e9ad3c

View File

@ -12,6 +12,7 @@ use itertools::Itertools;
use crate::card::CardType; use crate::card::CardType;
use crate::prelude::*; use crate::prelude::*;
use crate::revlog::RevlogEntry; use crate::revlog::RevlogEntry;
use crate::revlog::RevlogReviewKind;
use crate::scheduler::fsrs::weights::single_card_revlog_to_items; use crate::scheduler::fsrs::weights::single_card_revlog_to_items;
use crate::scheduler::fsrs::weights::Weights; use crate::scheduler::fsrs::weights::Weights;
use crate::scheduler::states::fuzz::with_review_fuzz; use crate::scheduler::states::fuzz::with_review_fuzz;
@ -51,8 +52,8 @@ impl Collection {
SearchBuilder::all([search.into(), SearchNode::State(StateKind::New).negated()]); SearchBuilder::all([search.into(), SearchNode::State(StateKind::New).negated()]);
let revlog = self.revlog_for_srs(search)?; let revlog = self.revlog_for_srs(search)?;
let reschedule = req.as_ref().map(|e| e.reschedule).unwrap_or_default(); let reschedule = req.as_ref().map(|e| e.reschedule).unwrap_or_default();
let last_reviews = if reschedule { let last_revlog_info = if reschedule {
Some(get_last_reviews(&revlog)) Some(get_last_revlog_info(&revlog))
} else { } else {
None None
}; };
@ -75,40 +76,46 @@ impl Collection {
card.set_memory_state(&fsrs, item, sm2_retention.unwrap()); card.set_memory_state(&fsrs, item, sm2_retention.unwrap());
card.desired_retention = desired_retention; card.desired_retention = desired_retention;
// if rescheduling // if rescheduling
if let Some(reviews) = &last_reviews { if let Some(reviews) = &last_revlog_info {
// and we have a last review time for the card // and we have a last review time for the card
if let Some(last_review) = reviews.get(&card.id) { if let Some(last_info) = reviews.get(&card.id) {
let days_elapsed = if let Some(last_review) = &last_info.last_reviewed_at {
timing.next_day_at.elapsed_days_since(*last_review) as i32; let days_elapsed =
// and the card's not new timing.next_day_at.elapsed_days_since(*last_review) as i32;
if let Some(state) = &card.memory_state { // and the card's not new
// or in (re)learning if let Some(state) = &card.memory_state {
if card.ctype == CardType::Review { // or in (re)learning
// reschedule it if card.ctype == CardType::Review {
let original_interval = card.interval; // reschedule it
let interval = fsrs.next_interval( let original_interval = card.interval;
Some(state.stability), let interval = fsrs.next_interval(
card.desired_retention.unwrap(), Some(state.stability),
0, card.desired_retention.unwrap(),
) as f32; 0,
card.interval = with_review_fuzz( )
card.get_fuzz_factor(), as f32;
interval, card.interval = with_review_fuzz(
1, card.get_fuzz_factor(),
req.max_interval, interval,
); 1,
let due = if card.original_due != 0 { req.max_interval,
&mut card.original_due );
} else { let due = if card.original_due != 0 {
&mut card.due &mut card.original_due
}; } else {
*due = (timing.days_elapsed as i32) - days_elapsed &mut card.due
+ card.interval as i32; };
self.log_manually_scheduled_review( *due = (timing.days_elapsed as i32) - days_elapsed
&card, + card.interval as i32;
original_interval, // Add a manual revlog entry if the last entry wasn't manual
usn, if !last_info.last_revlog_is_manual {
)?; self.log_manually_scheduled_review(
&card,
original_interval,
usn,
)?;
}
}
} }
} }
} }
@ -204,21 +211,39 @@ pub(crate) fn fsrs_items_for_memory_state(
.collect() .collect()
} }
/// Return a map of cards to the last time they were reviewed. struct LastRevlogInfo {
fn get_last_reviews(revlogs: &[RevlogEntry]) -> HashMap<CardId, TimestampSecs> { /// 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<TimestampSecs>,
/// If true, the last action on this card was a reschedule, so we
/// can avoid writing an extra revlog entry on another reschedule.
last_revlog_is_manual: bool,
}
/// Return a map of cards to info about last review/reschedule.
fn get_last_revlog_info(revlogs: &[RevlogEntry]) -> HashMap<CardId, LastRevlogInfo> {
let mut out = HashMap::new(); let mut out = HashMap::new();
revlogs revlogs
.iter() .iter()
.group_by(|r| r.cid) .group_by(|r| r.cid)
.into_iter() .into_iter()
.for_each(|(card_id, group)| { .for_each(|(card_id, group)| {
let mut last_ts = TimestampSecs::zero(); let mut last_reviewed_at = None;
for entry in group.into_iter().filter(|r| r.button_chosen >= 1) { let mut last_revlog_is_manual = false;
last_ts = entry.id.as_secs(); for e in group.into_iter() {
} if e.button_chosen >= 1 {
if last_ts != TimestampSecs::zero() { last_reviewed_at = Some(e.id.as_secs());
out.insert(card_id, last_ts); }
last_revlog_is_manual = e.review_kind == RevlogReviewKind::Manual;
} }
out.insert(
card_id,
LastRevlogInfo {
last_reviewed_at,
last_revlog_is_manual,
},
);
}); });
out out
} }