Make Hard use current step's interval if it's not the first one (#1561)

* Make hard repeat the current step's interval in v3

Unless for the first step to avoid identical interval with Again.

* Make Hard repeat the current step's interval in v2

* Adjust test to new Hard behaviour

pylib/anki/scheduler/v2.py

@@ -645,14 +645,18 @@ limit ?"""
         return int(delay * 60)
 
     def _delayForRepeatingGrade(self, conf: QueueConfig, left: int) -> Any:
-        # halfway between last and next
         delay1 = self._delayForGrade(conf, left)
-        if len(conf["delays"]) > 1:
-            delay2 = self._delayForGrade(conf, left - 1)
-        else:
-            delay2 = delay1 * 2
-        avg = (delay1 + max(delay1, delay2)) // 2
-        return avg
+        # first step?
+        if len(conf["delays"]) == left % 1000:
+            # halfway between last and next to avoid same interval with Again
+            if len(conf["delays"]) > 1:
+                delay2 = self._delayForGrade(conf, left - 1)
+            else:
+                # no next step, use dummy
+                delay2 = delay1 * 2
+            avg = (delay1 + max(delay1, delay2)) // 2
+            return avg
+        return delay1
 
     def _lrnConf(self, card: Card) -> Any:
         if card.type in (CARD_TYPE_REV, CARD_TYPE_RELEARNING):

pylib/tests/test_schedv2.py

@@ -585,7 +585,7 @@ def test_nextIvl():
     assert ni(c, 4) == 4 * 86400
     col.sched.answerCard(c, 3)
     assert ni(c, 1) == 30
-    assert ni(c, 2) == (180 + 600) // 2
+    assert ni(c, 2) == 180
     assert ni(c, 3) == 600
     assert ni(c, 4) == 4 * 86400
     col.sched.answerCard(c, 3)

rslib/src/scheduler/states/steps.rs

@@ -40,27 +40,23 @@ impl<'a> LearningSteps<'a> {
         self.secs_at_index(0)
     }
 
-    // fixme: the logic here is not ideal, but tries to match
-    // the current python code
-
     pub(crate) fn hard_delay_secs(self, remaining: u32) -> Option<u32> {
         let idx = self.get_index(remaining);
-        if let Some(current) = self
-            .secs_at_index(idx)
+        self.secs_at_index(idx)
             // if current is invalid, try first step
             .or_else(|| self.steps.first().copied().map(to_secs))
-        {
-            let next = if self.steps.len() > 1 {
-                self.secs_at_index(idx + 1).unwrap_or(60)
-            } else {
-                current.saturating_mul(2)
-            }
-            .max(current);
-
-            Some(current.saturating_add(next) / 2)
-        } else {
-            None
-        }
+            .map(|current| {
+                // special case to avoid Hard and Again showing same interval
+                if idx == 0 {
+                    // if there is no next step, simulate one with twice the interval of `current`
+                    let next = self
+                        .secs_at_index(idx + 1)
+                        .unwrap_or_else(|| current.saturating_mul(2));
+                    current.saturating_add(next) / 2
+                } else {
+                    current
+                }
+            })
     }
 
     pub(crate) fn good_delay_secs(self, remaining: u32) -> Option<u32> {
@@ -82,3 +78,29 @@ impl<'a> LearningSteps<'a> {
         self.steps.len() as u32
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    macro_rules! assert_delay_secs {
+        ($steps:expr, $remaining:expr, $again_delay:expr, $hard_delay:expr, $good_delay:expr) => {
+            let steps = LearningSteps::new(&$steps);
+            assert_eq!(steps.again_delay_secs_learn(), $again_delay);
+            assert_eq!(steps.hard_delay_secs($remaining), $hard_delay);
+            assert_eq!(steps.good_delay_secs($remaining), $good_delay);
+        };
+    }
+
+    #[test]
+    fn delay_secs() {
+        assert_delay_secs!([10.0], 1, 600, Some(900), None);
+
+        assert_delay_secs!([1.0, 10.0], 2, 60, Some(330), Some(600));
+        assert_delay_secs!([1.0, 10.0], 1, 60, Some(600), None);
+
+        assert_delay_secs!([1.0, 10.0, 100.0], 3, 60, Some(330), Some(600));
+        assert_delay_secs!([1.0, 10.0, 100.0], 2, 60, Some(600), Some(6000));
+        assert_delay_secs!([1.0, 10.0, 100.0], 1, 60, Some(6000), None);
+    }
+}