From f16557699218a71d455b73b1886e5f372adcb7fc Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Tue, 23 Feb 2021 12:44:26 +1000 Subject: [PATCH] implement leech handling Also change the default for new users to "tag only" --- pylib/anki/scheduler.py | 53 ++++++++++-------------- pylib/rsbridge/lib.rs | 1 + pylib/tests/test_schedv2.py | 1 + rslib/backend.proto | 2 + rslib/src/backend/generic.rs | 6 +++ rslib/src/backend/mod.rs | 7 +++- rslib/src/backend/sched/states/review.rs | 2 + rslib/src/deckconf/schema11.rs | 2 +- rslib/src/notes.rs | 28 ++++++++++++- rslib/src/sched/answering.rs | 21 +++++++++- rslib/src/sched/states/filtered.rs | 11 ++++- rslib/src/sched/states/mod.rs | 13 ++++++ rslib/src/sched/states/normal.rs | 9 ++++ rslib/src/sched/states/review.rs | 53 +++++++++++++++++++++++- 14 files changed, 171 insertions(+), 38 deletions(-) diff --git a/pylib/anki/scheduler.py b/pylib/anki/scheduler.py index 324dc2ebd..bf989c005 100644 --- a/pylib/anki/scheduler.py +++ b/pylib/anki/scheduler.py @@ -465,22 +465,20 @@ limit ?""" ########################################################################## def answerCard(self, card: Card, ease: int) -> None: - self.col.log() assert 1 <= ease <= 4 assert 0 <= card.queue <= 4 + self.col.markReview(card) + if self._burySiblingsOnAnswer: self._burySiblings(card) - self._answerCard(card, ease) + new_state = self._answerCard(card, ease) - self._maybe_requeue_card(card) + if not self._handle_leech(card, new_state): + self._maybe_requeue_card(card) - card.mod = intTime() - card.usn = self.col.usn() - card.flush() - - def _answerCard(self, card: Card, ease: int) -> None: + def _answerCard(self, card: Card, ease: int) -> _pb.SchedulingState: states = self.col._backend.get_next_card_states(card.id) if ease == BUTTON_ONE: new_state = states.again @@ -509,6 +507,22 @@ limit ?""" # fixme: tests assume card will be mutated, so we need to reload it card.load() + return new_state + + def _handle_leech(self, card: Card, new_state: _pb.SchedulingState) -> bool: + "True if was leech." + if self.col._backend.state_is_leech(new_state): + if hooks.card_did_leech.count() > 0: + hooks.card_did_leech(card) + # leech hooks assumed that card mutations would be saved for them + card.mod = intTime() + card.usn = self.col.usn() + card.flush() + + return True + else: + return False + def _maybe_requeue_card(self, card: Card) -> None: # preview cards if card.queue == QUEUE_TYPE_PREVIEW: @@ -604,29 +618,6 @@ limit ?""" return self._interval_for_state(new_state) - # Leeches - ########################################################################## - - def _checkLeech(self, card: Card, conf: QueueConfig) -> bool: - "Leech handler. True if card was a leech." - lf = conf["leechFails"] - if not lf: - return False - # if over threshold or every half threshold reps after that - if card.lapses >= lf and (card.lapses - lf) % (max(lf // 2, 1)) == 0: - # add a leech tag - f = card.note() - f.addTag("leech") - f.flush() - # handle - a = conf["leechAction"] - if a == LEECH_SUSPEND: - card.queue = QUEUE_TYPE_SUSPENDED - # notify UI - hooks.card_did_leech(card) - return True - return False - # Sibling spacing ########################################################################## diff --git a/pylib/rsbridge/lib.rs b/pylib/rsbridge/lib.rs index a7c2ffc5b..6c780f751 100644 --- a/pylib/rsbridge/lib.rs +++ b/pylib/rsbridge/lib.rs @@ -54,6 +54,7 @@ fn want_release_gil(method: u32) -> bool { | BackendMethod::JoinSearchNodes | BackendMethod::ReplaceSearchNode | BackendMethod::BuildSearchString + | BackendMethod::StateIsLeech ) } else { false diff --git a/pylib/tests/test_schedv2.py b/pylib/tests/test_schedv2.py index 4a3b6c1c2..04c46ff33 100644 --- a/pylib/tests/test_schedv2.py +++ b/pylib/tests/test_schedv2.py @@ -405,6 +405,7 @@ def test_reviews(): assert c.queue == QUEUE_TYPE_SUSPENDED c.load() assert c.queue == QUEUE_TYPE_SUSPENDED + assert "leech" in c.note().tags def test_review_limits(): diff --git a/rslib/backend.proto b/rslib/backend.proto index 207f4b703..4fe09b5fd 100644 --- a/rslib/backend.proto +++ b/rslib/backend.proto @@ -117,6 +117,7 @@ service BackendService { rpc SortDeck(SortDeckIn) returns (Empty); rpc GetNextCardStates(CardID) returns (NextCardStates); rpc DescribeNextStates(NextCardStates) returns (StringList); + rpc StateIsLeech(SchedulingState) returns (Bool); rpc AnswerCard(AnswerCardIn) returns (Empty); rpc UpgradeScheduler(Empty) returns (Empty); @@ -1283,6 +1284,7 @@ message SchedulingState { uint32 elapsed_days = 2; float ease_factor = 3; uint32 lapses = 4; + bool leeched = 5; } message Relearning { Review review = 1; diff --git a/rslib/src/backend/generic.rs b/rslib/src/backend/generic.rs index 96e4b23d1..0b3665f62 100644 --- a/rslib/src/backend/generic.rs +++ b/rslib/src/backend/generic.rs @@ -15,6 +15,12 @@ impl From for pb::String { } } +impl From for pb::Bool { + fn from(val: bool) -> Self { + pb::Bool { val } + } +} + impl From for pb::Int64 { fn from(val: i64) -> Self { pb::Int64 { val } diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index 23a57433d..8e89f5c19 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -33,7 +33,7 @@ use crate::{ sched::{ new::NewCardSortOrder, parse_due_date_str, - states::NextCardStates, + states::{CardState, NextCardStates}, timespan::{answer_button_time, time_span}, }, search::{ @@ -683,6 +683,11 @@ impl BackendService for Backend { .map(Into::into) } + fn state_is_leech(&self, input: pb::SchedulingState) -> BackendResult { + let state: CardState = input.into(); + Ok(state.leeched().into()) + } + fn answer_card(&self, input: pb::AnswerCardIn) -> BackendResult { self.with_col(|col| col.answer_card(&input.into())) .map(Into::into) diff --git a/rslib/src/backend/sched/states/review.rs b/rslib/src/backend/sched/states/review.rs index eff0f0ce9..a75bf113f 100644 --- a/rslib/src/backend/sched/states/review.rs +++ b/rslib/src/backend/sched/states/review.rs @@ -10,6 +10,7 @@ impl From for ReviewState { elapsed_days: state.elapsed_days, ease_factor: state.ease_factor, lapses: state.lapses, + leeched: state.leeched, } } } @@ -21,6 +22,7 @@ impl From for pb::scheduling_state::Review { elapsed_days: state.elapsed_days, ease_factor: state.ease_factor, lapses: state.lapses, + leeched: state.leeched, } } } diff --git a/rslib/src/deckconf/schema11.rs b/rslib/src/deckconf/schema11.rs index a6326ee9d..057dba532 100644 --- a/rslib/src/deckconf/schema11.rs +++ b/rslib/src/deckconf/schema11.rs @@ -130,7 +130,7 @@ pub struct LapseConfSchema11 { impl Default for LeechAction { fn default() -> Self { - LeechAction::Suspend + LeechAction::TagOnly } } diff --git a/rslib/src/notes.rs b/rslib/src/notes.rs index a1606ac2e..6be535d98 100644 --- a/rslib/src/notes.rs +++ b/rslib/src/notes.rs @@ -32,7 +32,7 @@ pub(crate) struct TransformNoteOutput { pub mark_modified: bool, } -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Clone)] pub struct Note { pub id: NoteID, pub guid: String, @@ -467,6 +467,32 @@ impl Collection { Ok(DuplicateState::Empty) } } + + /// Update the tags of the provided note, canonifying before save. Requires a transaction. + /// Fixme: this currently pulls in the note type, and does more work than necessary. We + /// could add a separate method to the storage layer to just update the tags in the future, + /// though it does slightly complicate the undo story. + pub(crate) fn update_note_tags(&mut self, nid: NoteID, mutator: F) -> Result<()> + where + F: Fn(&mut Vec), + { + self.transform_notes(&[nid], |note, _nt| { + let mut tags = note.tags.clone(); + mutator(&mut tags); + let changed = if tags != note.tags { + note.tags = tags; + true + } else { + false + }; + Ok(TransformNoteOutput { + changed, + generate_cards: false, + mark_modified: true, + }) + }) + .map(|_| ()) + } } #[cfg(test)] diff --git a/rslib/src/sched/answering.rs b/rslib/src/sched/answering.rs index f3b9d5a0d..87c68554f 100644 --- a/rslib/src/sched/answering.rs +++ b/rslib/src/sched/answering.rs @@ -4,7 +4,7 @@ use crate::{ backend_proto, card::{CardQueue, CardType}, - deckconf::DeckConf, + deckconf::{DeckConf, LeechAction}, decks::{Deck, DeckKind}, prelude::*, revlog::{RevlogEntry, RevlogReviewKind}, @@ -63,6 +63,7 @@ impl AnswerContext { easy_multiplier: self.config.inner.easy_multiplier, interval_multiplier: self.config.inner.interval_multiplier, maximum_review_interval: self.config.inner.maximum_review_interval, + leech_threshold: self.config.inner.leech_threshold, relearn_steps: self.relearn_steps(), lapse_multiplier: self.config.inner.lapse_multiplier, minimum_lapse_interval: self.config.inner.minimum_lapse_interval, @@ -105,6 +106,7 @@ impl AnswerContext { as u32, ease_factor, lapses, + leeched: false, } .into(), CardType::Relearn => RelearnState { @@ -117,6 +119,7 @@ impl AnswerContext { elapsed_days: interval, ease_factor, lapses, + leeched: false, }, } .into(), @@ -197,7 +200,7 @@ impl Card { self.original_due = 0; } - match next { + let revlog = match next { CardState::Normal(normal) => match normal { NormalState::New(next) => self.apply_new_state(current, next, ctx), NormalState::Learning(next) => self.apply_learning_state(current, next, ctx), @@ -210,7 +213,13 @@ impl Card { self.apply_rescheduling_state(current, next, ctx) } }, + }?; + + if next.leeched() && ctx.config.inner.leech_action() == LeechAction::Suspend { + self.queue = CardQueue::Suspended; } + + Ok(revlog) } fn apply_new_state( @@ -386,6 +395,7 @@ pub struct RevlogEntryPartial { } impl RevlogEntryPartial { + /// Returns None in the Preview case, since preview cards do not currently log. fn maybe_new( current: CardState, next: CardState, @@ -508,6 +518,9 @@ impl Collection { self.storage.add_revlog_entry(&revlog)?; } self.update_card(&mut card, &original, usn)?; + if answer.new_state.leeched() { + self.add_leech_tag(card.note_id)?; + } // fixme: we're reusing code used by python, which means re-feteching the target deck // - might want to avoid that in the future @@ -556,6 +569,10 @@ impl Collection { let state_ctx = ctx.state_context(); Ok(current.next_states(&state_ctx)) } + + fn add_leech_tag(&mut self, nid: NoteID) -> Result<()> { + self.update_note_tags(nid, |tags| tags.push("leech".into())) + } } /// Return a consistent seed for a given card at a given number of reps. diff --git a/rslib/src/sched/states/filtered.rs b/rslib/src/sched/states/filtered.rs index 448b08528..a74966df0 100644 --- a/rslib/src/sched/states/filtered.rs +++ b/rslib/src/sched/states/filtered.rs @@ -3,7 +3,9 @@ use crate::revlog::RevlogReviewKind; -use super::{IntervalKind, NextCardStates, PreviewState, ReschedulingFilterState, StateContext}; +use super::{ + IntervalKind, NextCardStates, PreviewState, ReschedulingFilterState, ReviewState, StateContext, +}; #[derive(Debug, Clone, Copy, PartialEq)] pub enum FilteredState { @@ -32,4 +34,11 @@ impl FilteredState { FilteredState::Rescheduling(state) => state.next_states(ctx), } } + + pub(crate) fn review_state(self) -> Option { + match self { + FilteredState::Preview(state) => state.original_state.review_state(), + FilteredState::Rescheduling(state) => state.original_state.review_state(), + } + } } diff --git a/rslib/src/sched/states/mod.rs b/rslib/src/sched/states/mod.rs index fcd64fdd1..9012c65f5 100644 --- a/rslib/src/sched/states/mod.rs +++ b/rslib/src/sched/states/mod.rs @@ -54,6 +54,18 @@ impl CardState { CardState::Filtered(state) => state.next_states(&ctx), } } + + /// Returns underlying review state, if it exists. + pub(crate) fn review_state(self) -> Option { + match self { + CardState::Normal(state) => state.review_state(), + CardState::Filtered(state) => state.review_state(), + } + } + + pub(crate) fn leeched(self) -> bool { + self.review_state().map(|r| r.leeched).unwrap_or_default() + } } /// Info required during state transitions. @@ -70,6 +82,7 @@ pub(crate) struct StateContext<'a> { pub easy_multiplier: f32, pub interval_multiplier: f32, pub maximum_review_interval: u32, + pub leech_threshold: u32, // relearning pub relearn_steps: LearningSteps<'a>, diff --git a/rslib/src/sched/states/normal.rs b/rslib/src/sched/states/normal.rs index 0b7704547..9b59c1b05 100644 --- a/rslib/src/sched/states/normal.rs +++ b/rslib/src/sched/states/normal.rs @@ -55,6 +55,15 @@ impl NormalState { NormalState::Relearning(state) => state.next_states(ctx), } } + + pub(crate) fn review_state(self) -> Option { + match self { + NormalState::New(_) => None, + NormalState::Learning(_) => None, + NormalState::Review(state) => Some(state), + NormalState::Relearning(RelearnState { review, .. }) => Some(review), + } + } } impl From for NormalState { diff --git a/rslib/src/sched/states/review.rs b/rslib/src/sched/states/review.rs index f145931d7..526ca6a3b 100644 --- a/rslib/src/sched/states/review.rs +++ b/rslib/src/sched/states/review.rs @@ -19,6 +19,7 @@ pub struct ReviewState { pub elapsed_days: u32, pub ease_factor: f32, pub lapses: u32, + pub leeched: bool, } impl Default for ReviewState { @@ -28,6 +29,7 @@ impl Default for ReviewState { elapsed_days: 0, ease_factor: INITIAL_EASE_FACTOR, lapses: 0, + leeched: false, } } } @@ -71,11 +73,14 @@ impl ReviewState { } fn answer_again(self, ctx: &StateContext) -> CardState { + let lapses = self.lapses + 1; + let leeched = leech_threshold_met(lapses, ctx.leech_threshold); let again_review = ReviewState { scheduled_days: self.failing_review_interval(ctx), elapsed_days: 0, ease_factor: (self.ease_factor + EASE_FACTOR_AGAIN_DELTA).max(MINIMUM_EASE_FACTOR), - lapses: self.lapses + 1, + lapses, + leeched, }; if let Some(again_delay) = ctx.relearn_steps.again_delay_secs_relearn() { @@ -196,6 +201,18 @@ impl ReviewState { } } +/// True when lapses is at threshold, or every half threshold after that. +/// Non-even thresholds round up the half threshold. +fn leech_threshold_met(lapses: u32, threshold: u32) -> bool { + if threshold > 0 { + let half_threshold = (threshold as f32 / 2.0).ceil().max(1.0) as u32; + // at threshold, and every half threshold after that, rounding up + lapses >= threshold && (lapses - threshold) % half_threshold == 0 + } else { + false + } +} + /// Transform the provided hard/good/easy interval. /// - Apply configured interval multiplier. /// - Apply fuzz. @@ -214,3 +231,37 @@ fn constrain_passing_interval(ctx: &StateContext, interval: f32, minimum: u32, f .min(ctx.maximum_review_interval) .max(1) } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn leech_threshold() { + assert_eq!(leech_threshold_met(0, 3), false); + assert_eq!(leech_threshold_met(1, 3), false); + assert_eq!(leech_threshold_met(2, 3), false); + assert_eq!(leech_threshold_met(3, 3), true); + assert_eq!(leech_threshold_met(4, 3), false); + assert_eq!(leech_threshold_met(5, 3), true); + assert_eq!(leech_threshold_met(6, 3), false); + assert_eq!(leech_threshold_met(7, 3), true); + + assert_eq!(leech_threshold_met(7, 8), false); + assert_eq!(leech_threshold_met(8, 8), true); + assert_eq!(leech_threshold_met(9, 8), false); + assert_eq!(leech_threshold_met(10, 8), false); + assert_eq!(leech_threshold_met(11, 8), false); + assert_eq!(leech_threshold_met(12, 8), true); + assert_eq!(leech_threshold_met(13, 8), false); + + // 0 means off + assert_eq!(leech_threshold_met(0, 0), false); + + // no div by zero; half of 1 is 1 + assert_eq!(leech_threshold_met(0, 1), false); + assert_eq!(leech_threshold_met(1, 1), true); + assert_eq!(leech_threshold_met(2, 1), true); + assert_eq!(leech_threshold_met(3, 1), true); + } +}