Fix/remove the lower limit of interval when set due date

[L-M-Sherlock]

previous pr: #4035

The interval should be zero if user set a card's due date as today which they has reviewed today.

[dyzur]

I was doing reviews of some cards. I chose one of the cards at random and I went to the browser and and it says 2025-06-08, since I rated the card as 3(good), it put the interval to 2 days. I did the review today. I checked the retrievability in the column it shows 100% retrievability. If I change the due-date to 0, today, it somehow becomes 89%. And when I check the card info regardless of due date being changed or not, it even dropped to 29%.

[user1823]

There's a problem with this solution. Historically, ivl of a review card can't be 0.

Also, we have some Rust code that treats a card as a new card if fsrs items are absent and ivl = 0. Some add-ons (e.g. Learn Now feature in AJT card management) take advantage of this.

anki/rslib/src/scheduler/fsrs/memory_state.rs

Line 228 in c1fc459

} else if self.ctype == CardType::New || self.interval == 0 {

I don't see this change as breaking anything yet, but we are diverging from a convention and this might introduce problems.

[dae]

Yep, that is a good point. At the very least I think we'll need to audit our codebase for any checks against 0, and I can't be sure if it will affect add-ons moving forward.

I can't remember if we have an open issue for it, but I recall previous discussion about potentially storing the last review/reschedule date in the card to simplify elapsed time calculations. Is that a potential alternative way to solve this?

[user1823]

previous discussion about potentially storing the last review/reschedule date in the card

That's a potential alternative but, as I mentioned in that issue, it will also increase the size of the database, which is already a concern for some people.

As the current issue is very rare (affects only those cards that were studied today and are being scheduled today), I don't think that we should adopt a solution that increases the size of database for all users.

Unfortunately, I don't currently have a good idea to solve this.

---

Another issue with this solution:
SM-2 works by multiplying the current interval by a factor. If ivl = 0, the intervals of the card will never increase. This isn't an issue with FSRS, but the code changed by this PR isn't FSRS-specific and we need to account for the fact that few users might turn off FSRS after playing with it.

[dae]

Appreciate you thinking of all the potential downfalls! To clarify, I wasn't advocating for the card data change only to solve this issue, but if we're making it for the previously-discussed reasons, it could potentially solve this too. If due dates are stored as days-past-collection-creation like Anki does for review, we're talking an extra ~7 bytes per card, which is not huge in the grand scheme of things.

[dae]

While I'm a bit nervous about unintended consequences here, card.interval could have been set to 0 by third party tools, so we need to handle it robustly anyway.

Do you still object to this @user1823? If so, anyone else want to weigh in / potentially offer an alternative path to solve the issue Jarrett's trying to solve?

[user1823]

Though I am a bit nervous about the potential negative consequences too, I think that this is the easiest solution possible.

To reduce the risk of downfalls, I propose the following:

diff --git a/rslib/src/scheduler/states/review.rs b/rslib/src/scheduler/states/review.rs
index e7591bb25..c09bf3eca 100644
--- a/rslib/src/scheduler/states/review.rs
+++ b/rslib/src/scheduler/states/review.rs
@@ -83,7 +83,7 @@ impl ReviewState {
         } else {
             let (minimum, maximum) = ctx.min_and_max_review_intervals(ctx.minimum_lapse_interval);
             let interval = ctx.with_review_fuzz(
-                (self.scheduled_days as f32) * ctx.lapse_multiplier,
+                (self.scheduled_days.max(1.0) as f32) * ctx.lapse_multiplier,
                 minimum,
                 maximum,
             );
@@ -211,7 +211,7 @@ impl ReviewState {
     }

     fn passing_nonearly_review_intervals(self, ctx: &StateContext) -> (u32, u32, u32) {
-        let current_interval = self.scheduled_days as f32;
+        let current_interval = self.scheduled_days.max(1.0) as f32;
         let days_late = self.days_late().max(0) as f32;

         // hard
@@ -251,8 +251,8 @@ impl ReviewState {
     /// FIXME: this needs reworking in the future; it overly penalizes reviews
     /// done shortly before the due date.
     fn passing_early_review_intervals(self, ctx: &StateContext) -> (u32, u32, u32) {
-        let scheduled = self.scheduled_days as f32;
-        let elapsed = (self.scheduled_days as f32) + (self.days_late() as f32);
+        let scheduled = self.scheduled_days.max(1.0) as f32;
+        let elapsed = self.elapsed_days as f32;

         let hard_interval = {
             let factor = ctx.hard_multiplier;

[user1823]

I updated my comment to include one more minor change:

diff --git a/rslib/src/scheduler/states/review.rs b/rslib/src/scheduler/states/review.rs
index c0b8f0a43..c09bf3eca 100644
--- a/rslib/src/scheduler/states/review.rs
+++ b/rslib/src/scheduler/states/review.rs
@@ -252,7 +252,7 @@ impl ReviewState {
     /// done shortly before the due date.
     fn passing_early_review_intervals(self, ctx: &StateContext) -> (u32, u32, u32) {
         let scheduled = self.scheduled_days.max(1.0) as f32;
-        let elapsed = (self.scheduled_days as f32) + (self.days_late() as f32);
+        let elapsed = self.elapsed_days as f32;

         let hard_interval = {
             let factor = ctx.hard_multiplier;

You can include this too if you like it.

[dae]

Reusing the max'ed scheduled figure makes sense, but removing days_late()?

[user1823]

days_late is defined as:

anki/rslib/src/scheduler/states/review.rs

Lines 45 to 47 in e5d34fb

	pub(crate) fn days_late(&self) -> i32 {
	self.elapsed_days as i32 - self.scheduled_days as i32
	}

So, self.scheduled_days + self.days_late() is simply equal to self.elapsed_days (unless I am misunderstanding something). My suggestion removes this redundant calculation.

This would be a non-functional change and my original motivation for this suggestion was to avoid the confusion regarding the places where we needed to use .max(1.0) with scheduled_days.

[user1823]

The following patch should fix #2 in https://forums.ankiweb.net/t/anki-25-06-beta/62271/14

diff --git a/rslib/src/scheduler/reviews.rs b/rslib/src/scheduler/reviews.rs
index 53802fb9a..b8d4aeb6a 100644
--- a/rslib/src/scheduler/reviews.rs
+++ b/rslib/src/scheduler/reviews.rs
@@ -34,7 +34,7 @@ impl Card {
         let new_due = (today + days_from_today) as i32;
         let fsrs_enabled = self.memory_state.is_some();
         let new_interval = if fsrs_enabled {
-            self.interval.saturating_add_signed(new_due - self.due)
+            self.interval.saturating_add_signed(new_due - self.original_or_current_due() as i32)
         } else if force_reset || !matches!(self.ctype, CardType::Review | CardType::Relearn) {
             days_from_today
         } else {

[dae]

Thanks, you're right. The other fix is appreciated too!

[dae]

Thanks to @L-M-Sherlock too!