revert some interday learning changes in v3

Interday learning cards are now counted in the learning count again,
and are no longer subject to the daily review limit.

The thinking behind the original change was that interday learning cards
are scheduled more like reviews, and counting them in the review count
would allow the learning count to focus on intraday learning - the red
number reflecting the fact that they are the most fragile memories. And
counting them together made it practical to apply the review limit
to both at once.

Since the release, there have been a number of users expecting to see
interday learning cards included in the learning count (the latest being
https://forums.ankiweb.net/t/feedback-and-a-feature-adjustment-request-for-2-1-45/12308),
and a good argument can be made for that too - they are, after all, listed
in the learning steps, and do tend to be harder than reviews. Short of
introducing another count to keep track of interday and intraday learning
separately, moving back to the old behaviour seems like the best move.

This also means it is not really practical to apply the review limit to
interday learning cards anymore, as the limit would be split between two
different numbers, and how much each number is capped would depend on
the order cards are introduced. The scheduler could figure this out, but
the deck list code does not know card order, and would need significant
changes to be able to produce numbers that matched the scheduler. And
even if we ignore implementation complexities, I think it would be more
difficult for users to reason about - the influence of the review limit
on new cards is confusing enough as it is.

pylib/tests/test_schedv2.py

@@ -317,11 +317,7 @@ def test_learn_day():
     c.due -= 1
     c.flush()
     col.reset()
-    if is_2021():
-        # it appears in the review queue
-        assert col.sched.counts() == (0, 0, 1)
-    else:
-        assert col.sched.counts() == (0, 1, 0)
+    assert col.sched.counts() == (0, 1, 0)
     c = col.sched.getCard()
     # nextIvl should work
     assert ni(c, 3) == 86400 * 2

rslib/src/decks/counts.rs

@@ -30,14 +30,12 @@ impl Collection {
         days_elapsed: u32,
         learn_cutoff: u32,
         limit_to: Option<&str>,
-        v3: bool,
     ) -> Result<HashMap<DeckId, DueCounts>> {
         self.storage.due_counts(
             self.scheduler_version(),
             days_elapsed,
             learn_cutoff,
             limit_to,
-            v3,
         )
     }
 

rslib/src/decks/tree.rs

@@ -277,7 +277,7 @@ impl Collection {
             let learn_cutoff = (now.0 as u32) + self.learn_ahead_secs();
             let sched_ver = self.scheduler_version();
             let v3 = self.get_config_bool(BoolKey::Sched2021);
-            let counts = self.due_counts(days_elapsed, learn_cutoff, limit, v3)?;
+            let counts = self.due_counts(days_elapsed, learn_cutoff, limit)?;
             let dconf = self.storage.get_deck_config_map()?;
             add_counts(&mut tree, &counts);
             let limits = remaining_limits_map(decks_map.values(), &dconf, days_elapsed);

rslib/src/scheduler/answering/mod.rs

@@ -315,7 +315,7 @@ impl Collection {
         let mut review_delta = 0;
         match from_queue {
             CardQueue::New => new_delta += 1,
-            CardQueue::Review | CardQueue::DayLearn => review_delta += 1,
+            CardQueue::Review => review_delta += 1,
             _ => {}
         }
         self.update_deck_stats(

rslib/src/scheduler/queue/builder/gathering.rs

@@ -2,7 +2,7 @@
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
 use super::{BuryMode, DueCard, NewCard, QueueBuilder};
-use crate::{card::CardQueue, prelude::*};
+use crate::{prelude::*, scheduler::queue::DueCardKind};
 
 impl QueueBuilder {
     pub(in super::super) fn add_intraday_learning_card(
@@ -15,12 +15,7 @@ impl QueueBuilder {
     }
 
     /// True if limit should be decremented.
-    pub(in super::super) fn add_due_card(
-        &mut self,
-        queue: CardQueue,
-        card: DueCard,
-        bury_mode: BuryMode,
-    ) -> bool {
+    pub(in super::super) fn add_due_card(&mut self, card: DueCard, bury_mode: BuryMode) -> bool {
         let bury_this_card = self
             .get_and_update_bury_mode_for_note(card.note_id, bury_mode)
             .map(|mode| mode.bury_reviews)
@@ -28,14 +23,9 @@ impl QueueBuilder {
         if bury_this_card {
             false
         } else {
-            match queue {
-                CardQueue::DayLearn => {
-                    self.day_learning.push(card);
-                }
-                CardQueue::Review => {
-                    self.review.push(card);
-                }
-                _ => unreachable!(),
+            match card.kind {
+                DueCardKind::Review => self.review.push(card),
+                DueCardKind::Learning => self.day_learning.push(card),
             }
 
             true

rslib/src/scheduler/queue/builder/mod.rs

@@ -19,7 +19,7 @@ use crate::{
 };
 
 /// Temporary holder for review cards that will be built into a queue.
-#[derive(Debug, Default, Clone)]
+#[derive(Debug, Clone)]
 pub(crate) struct DueCard {
     pub id: CardId,
     pub note_id: NoteId,
@@ -29,6 +29,13 @@ pub(crate) struct DueCard {
     pub hash: u64,
     pub current_deck_id: DeckId,
     pub original_deck_id: DeckId,
+    pub kind: DueCardKind,
+}
+
+#[derive(Debug, Clone, Copy)]
+pub(crate) enum DueCardKind {
+    Review,
+    Learning,
 }
 
 /// Temporary holder for new cards that will be built into a queue.
@@ -48,7 +55,10 @@ impl From<DueCard> for MainQueueEntry {
         MainQueueEntry {
             id: c.id,
             mtime: c.mtime,
-            kind: MainQueueEntryKind::Review,
+            kind: match c.kind {
+                DueCardKind::Review => MainQueueEntryKind::Review,
+                DueCardKind::Learning => MainQueueEntryKind::InterdayLearning,
+            },
         }
     }
 }
@@ -117,25 +127,33 @@ impl QueueBuilder {
     ) -> CardQueues {
         self.sort_new();
 
-        // intraday learning
-        let learning = sort_learning(self.learning);
+        // intraday learning and total learn count
+        let intraday_learning = sort_learning(self.learning);
         let now = TimestampSecs::now();
         let cutoff = now.adding_secs(learn_ahead_secs);
-        let learn_count = learning.iter().take_while(|e| e.due <= cutoff).count();
+        let learn_count = intraday_learning
+            .iter()
+            .take_while(|e| e.due <= cutoff)
+            .count()
+            + self.day_learning.len();
 
-        // merge interday learning into main, and cap to parent review count
-        let main_iter = merge_day_learning(
+        // cap and note down review + new counts
+        self.review.truncate(top_deck_limits.review as usize);
+        let review_count = self.review.len();
+        self.new.truncate(top_deck_limits.new as usize);
+        let new_count = self.new.len();
+
+        // merge interday and new cards into main
+        let with_interday_learn = merge_day_learning(
             self.review,
             self.day_learning,
             self.sort_options.day_learn_mix,
         );
-        let main_iter = main_iter.take(top_deck_limits.review as usize);
-        let review_count = main_iter.len();
-
-        // cap to parent new count, note down the new count, then merge new in
-        self.new.truncate(top_deck_limits.new as usize);
-        let new_count = self.new.len();
-        let main_iter = merge_new(main_iter, self.new, self.sort_options.new_review_mix);
+        let main_iter = merge_new(
+            with_interday_learn,
+            self.new,
+            self.sort_options.new_review_mix,
+        );
 
         CardQueues {
             counts: Counts {
@@ -144,7 +162,7 @@ impl QueueBuilder {
                 learning: learn_count,
             },
             main: main_iter.collect(),
-            intraday_learning: learning,
+            intraday_learning,
             learn_ahead_secs,
             selected_deck,
             current_day,
@@ -192,7 +210,7 @@ impl Collection {
         let now = TimestampSecs::now();
         let timing = self.timing_for_timestamp(now)?;
         let decks = self.storage.deck_with_children(deck_id)?;
-        // need full map, since filtered decks may contain cards from decks/
+        // need full map, since filtered decks may contain cards from decks
         // outside tree
         let deck_map = self.storage.get_decks_map()?;
         let config = self.storage.get_deck_config_map()?;
@@ -245,18 +263,31 @@ impl Collection {
                 queues.add_intraday_learning_card(card, bury)
             })?;
 
-        // reviews and interday learning next
+        // interday learning
+        self.storage.for_each_due_card_in_active_decks(
+            timing.days_elapsed,
+            sort_options.review_order,
+            DueCardKind::Learning,
+            |card| {
+                let bury = get_bury_mode(card.original_deck_id.or(card.current_deck_id));
+                queues.add_due_card(card, bury);
+                true
+            },
+        )?;
+
+        // reviews
         if selected_deck_limits.review != 0 {
-            self.storage.for_each_review_card_in_active_decks(
+            self.storage.for_each_due_card_in_active_decks(
                 timing.days_elapsed,
                 sort_options.review_order,
-                |queue, card| {
+                DueCardKind::Review,
+                |card| {
                     if selected_deck_limits.review == 0 {
                         return false;
                     }
                     let bury = get_bury_mode(card.original_deck_id.or(card.current_deck_id));
                     let limits = remaining.get_mut(&card.current_deck_id).unwrap();
-                    if limits.review != 0 && queues.add_due_card(queue, card, bury) {
+                    if limits.review != 0 && queues.add_due_card(card, bury) {
                         selected_deck_limits.review -= 1;
                         limits.review -= 1;
                     }

rslib/src/scheduler/queue/entry.rs

@@ -31,6 +31,7 @@ impl QueueEntry {
             QueueEntry::Main(e) => match e.kind {
                 MainQueueEntryKind::New => QueueEntryKind::New,
                 MainQueueEntryKind::Review => QueueEntryKind::Review,
+                MainQueueEntryKind::InterdayLearning => QueueEntryKind::Learning,
             },
         }
     }

rslib/src/scheduler/queue/learning.rs

@@ -181,8 +181,8 @@ impl CardQueues {
     }
 }
 
-/// Adapted from the Rust stdlib VecDeque implementation; we can drop this when the following
-/// lands: https://github.com/rust-lang/rust/issues/78021
+/// Adapted from the Rust stdlib VecDeque implementation; we can drop this after updating
+/// to Rust 1.54.0
 fn binary_search_by<'a, F, T>(deque: &'a VecDeque<T>, mut f: F) -> Result<usize, usize>
 where
     F: FnMut(&'a T) -> Ordering,

rslib/src/scheduler/queue/main.rs

@@ -15,6 +15,7 @@ pub(crate) struct MainQueueEntry {
 pub(crate) enum MainQueueEntryKind {
     New,
     Review,
+    InterdayLearning,
 }
 
 impl CardQueues {
@@ -24,6 +25,7 @@ impl CardQueues {
             match head.kind {
                 MainQueueEntryKind::New => self.counts.new -= 1,
                 MainQueueEntryKind::Review => self.counts.review -= 1,
+                MainQueueEntryKind::InterdayLearning => self.counts.learning -= 1,
             };
             head
         })
@@ -34,6 +36,7 @@ impl CardQueues {
         match entry.kind {
             MainQueueEntryKind::New => self.counts.new += 1,
             MainQueueEntryKind::Review => self.counts.review += 1,
+            MainQueueEntryKind::InterdayLearning => self.counts.learning += 1,
         };
         self.main.push_front(entry);
     }

rslib/src/scheduler/queue/mod.rs

@@ -9,7 +9,7 @@ pub(crate) mod undo;
 
 use std::collections::VecDeque;
 
-pub(crate) use builder::{DueCard, NewCard};
+pub(crate) use builder::{DueCard, DueCardKind, NewCard};
 pub(crate) use entry::{QueueEntry, QueueEntryKind};
 pub(crate) use learning::LearningQueueEntry;
 pub(crate) use main::{MainQueueEntry, MainQueueEntryKind};

rslib/src/storage/card/due_cards.sql

@@ -1,5 +1,4 @@
-SELECT queue,
-  id,
+SELECT id,
   nid,
   due,
   cast(ivl AS integer),
@@ -12,6 +11,6 @@ WHERE did IN (
     FROM active_decks
   )
   AND (
-    queue IN (2, 3)
+    queue = ?
     AND due <= ?
   )

rslib/src/storage/card/mod.rs

@@ -20,7 +20,7 @@ use crate::{
     notes::NoteId,
     scheduler::{
         congrats::CongratsInfo,
-        queue::{DueCard, NewCard},
+        queue::{DueCard, DueCardKind, NewCard},
     },
     timestamp::{TimestampMillis, TimestampSecs},
     types::Usn,
@@ -184,20 +184,24 @@ impl super::SqliteStorage {
                 original_deck_id: row.get(5)?,
                 interval: 0,
                 hash: 0,
+                kind: DueCardKind::Learning,
             })
         }
 
         Ok(())
     }
 
-    pub(crate) fn for_each_review_card_in_active_decks<F>(
+    /// Call func() for each review card or interday learning card, stopping
+    /// when it returns false or no more cards found.
+    pub(crate) fn for_each_due_card_in_active_decks<F>(
         &self,
         day_cutoff: u32,
         order: ReviewCardOrder,
+        kind: DueCardKind,
         mut func: F,
     ) -> Result<()>
     where
-        F: FnMut(CardQueue, DueCard) -> bool,
+        F: FnMut(DueCard) -> bool,
     {
         let order_clause = review_order_sql(order);
         let mut stmt = self.db.prepare_cached(&format!(
@@ -205,22 +209,23 @@ impl super::SqliteStorage {
             include_str!("due_cards.sql"),
             order_clause
         ))?;
-        let mut rows = stmt.query(params![day_cutoff])?;
+        let queue = match kind {
+            DueCardKind::Review => CardQueue::Review,
+            DueCardKind::Learning => CardQueue::DayLearn,
+        };
+        let mut rows = stmt.query(params![queue as i8, day_cutoff])?;
         while let Some(row) = rows.next()? {
-            let queue: CardQueue = row.get(0)?;
-            if !func(
-                queue,
-                DueCard {
-                    id: row.get(1)?,
-                    note_id: row.get(2)?,
-                    due: row.get(3).ok().unwrap_or_default(),
-                    interval: row.get(4)?,
-                    mtime: row.get(5)?,
-                    current_deck_id: row.get(6)?,
-                    original_deck_id: row.get(7)?,
-                    hash: 0,
-                },
-            ) {
+            if !func(DueCard {
+                id: row.get(0)?,
+                note_id: row.get(1)?,
+                due: row.get(2).ok().unwrap_or_default(),
+                interval: row.get(3)?,
+                mtime: row.get(4)?,
+                current_deck_id: row.get(5)?,
+                original_deck_id: row.get(6)?,
+                hash: 0,
+                kind,
+            }) {
                 break;
             }
         }

rslib/src/storage/deck/mod.rs

@@ -38,18 +38,13 @@ fn row_to_deck(row: &Row) -> Result<Deck> {
     })
 }
 
-fn row_to_due_counts(row: &Row, v3: bool) -> Result<(DeckId, DueCounts)> {
+fn row_to_due_counts(row: &Row) -> Result<(DeckId, DueCounts)> {
     let deck_id = row.get(0)?;
     let new = row.get(1)?;
-    let mut review = row.get(2)?;
+    let review = row.get(2)?;
     let interday: u32 = row.get(3)?;
     let intraday: u32 = row.get(4)?;
-    let learning = if v3 {
-        review += interday;
-        intraday
-    } else {
-        intraday + interday
-    };
+    let learning = intraday + interday;
     Ok((
         deck_id,
         DueCounts {
@@ -265,7 +260,6 @@ impl SqliteStorage {
         day_cutoff: u32,
         learn_cutoff: u32,
         top_deck: Option<&str>,
-        v3: bool,
     ) -> Result<HashMap<DeckId, DueCounts>> {
         let sched_ver = sched as u8;
         let mut params = named_params! {
@@ -306,7 +300,7 @@ impl SqliteStorage {
 
         self.db
             .prepare_cached(sql)?
-            .query_and_then(&*params, |row| row_to_due_counts(row, v3))?
+            .query_and_then(&*params, |row| row_to_due_counts(row))?
             .collect()
     }