Fix/ignore revlogs before Forget entry

[L-M-Sherlock]

close #3001

[user1823]

if we find the Forget entry before the Learn entry, we should ignore all the entries

Why?

Also, this doesn't solve the main issue which is that "new" cards are being used for optimization.

[L-M-Sherlock]

Why?

The code checks the entry from the latest to the oldest. If the code finds the Forget entry before it finds the Learn entry, it means the card hasn't been learned after the user Forget the card.

[user1823]

The code checks the entry from the latest to the oldest.

Ok. I get it now.

Then, there are two effects of this change:

• It filters out "new" cards (i.e. cards that were Reset)
• During training, it filters out the cards that were first reset and then introduced using Set Due Date rather than being introduced normally. During scheduling, the memory states of these cards would be calculated using memory_state_from_sm2 at the first rating (which happened before the Forget rating in time).

The first effect is desirable but the second effect is not (especially the effect on calculation of memory states).

One alternative is to simply add -is:new to the search query. This produces the first effect above. The differences:
For the cards mentioned in 2nd point above

• the card is used for training (which is not good, especially because the older revlogs are also used)
• the normal formulas are used during calculation of memory states from the complete revlog (this effect would be almost the same as your approach)

So, both of them are not very good but your approach is the better one of them.

My idea of filtering the revlogs before the Forget rating doesn't have any of the above problems. But if it is too complicated to implement, then your approach would be a good alternative.

[dae]

Elsewhere, we check 'filtered || ease factor == 0'. Did you mean 'manual || button == 0' instead of 'manual && ease factor == 0'? Does the following code better express your intended change?

diff --git a/ftl/core-repo b/ftl/core-repo
--- a/ftl/core-repo
+++ b/ftl/core-repo
@@ -1 +1 @@
-Subproject commit 5499a04af2574f6126043045dd2f37e5b46fd90d
+Subproject commit 5499a04af2574f6126043045dd2f37e5b46fd90d-dirty
diff --git a/rslib/src/scheduler/fsrs/weights.rs b/rslib/src/scheduler/fsrs/weights.rs
index c9a2e5280..ec0abbc8e 100644
--- a/rslib/src/scheduler/fsrs/weights.rs
+++ b/rslib/src/scheduler/fsrs/weights.rs
@@ -186,22 +186,19 @@ pub(crate) fn single_card_revlog_to_items(
     let mut last_learn_entry = None;
     let mut revlogs_complete = false;
     for (index, entry) in entries.iter().enumerate().rev() {
-        if matches!(
+        if entry.review_kind == RevlogReviewKind::Manual || entry.button_chosen == 0 {
+            // if we find the `Forget` entry before the `Learn` entry, we should
+            // ignore all the entries
+            break;
+        } else if matches!(
             (entry.review_kind, entry.button_chosen),
             (RevlogReviewKind::Learning, 1..=4)
         ) {
+            // we may set this multiple times as we step back through learning entries
             last_learn_entry = Some(index);
             revlogs_complete = true;
         } else if last_learn_entry.is_some() {
-            break;
-        // if we find the `Forget` entry before the `Learn` entry, we should
-        // ignore all the entries
-        } else if matches!(
-            (entry.review_kind, entry.ease_factor),
-            (RevlogReviewKind::Manual, 0)
-        ) && last_learn_entry.is_none()
-        {
-            revlogs_complete = false;
+            // we encountered a non-learn entry after learning steps found; stop
             break;
         }
     }
@@ -410,9 +407,13 @@ pub(crate) mod tests {
                     },
                     revlog(RevlogReviewKind::Review, 4),
                 ],
-                true,
+                false,
             ),
-            fsrs_items!([review(0), review(1)], [review(0), review(1), review(5)])
+            fsrs_items!(
+                [review(0)],
+                [review(0), review(1)],
+                [review(0), review(1), review(5)]
+            )
         );
     }
 

[user1823]

Did you mean 'manual || button == 0'

AFAIK, this doesn't differentiate between Forget and Set Due Date. The difference between these entries is the ease factor.

[dae]

Doesn't it make sense to ignore the learning entries if a 'set due date' comes after them as well?

[user1823]

Doesn't it make sense to ignore the learning entries if a 'set due date' comes after them as well?

No, set due date is fundamentally different from forget.

• Forget means that the user wants to make a fresh start with the card.
• Set due date means that the user doesn't like the interval given by Anki and wants to see it after a shorter/longer interval.

Also, IIRC, the Reschedule entry added by Anki when "Reschedule cards on change" is turned on can't be differentiated from Set Due Date.

[dae]

I see - thanks for clarifying.

[dae]

And thanks @L-M-Sherlock