rework undo

- use dataclasses for the review/checkpoint undo cases, instead of the
nasty ad-hoc list structure
- expose backend review undo to Python, and hook it into GUI
- redo is not currently exposed on the GUI, and the backend can only
cope with reviews done by the new scheduler at the moment
- the initial undo prototype code was bumping mtime/usn on undo, but
that was not ideal, as it was breaking the queue handling which expected
the mtime to match. The original rationale for bumping mtime/usn was
to avoid problems with syncing, but various operations like removing
a revlog can't be synced anyway - so we just need to ensure we clear the
undo queue prior to syncing

pylib/anki/collection.py

@@ -11,6 +11,7 @@ import sys
 import time
 import traceback
 import weakref
+from dataclasses import dataclass, field
 from typing import Any, List, Literal, Optional, Sequence, Tuple, Union
 
 import anki._backend.backend_pb2 as _pb
@@ -34,6 +35,7 @@ from anki.scheduler import Scheduler as V2TestScheduler
 from anki.schedv2 import Scheduler as V2Scheduler
 from anki.sync import SyncAuth, SyncOutput, SyncStatus
 from anki.tags import TagManager
+from anki.types import assert_exhaustive
 from anki.utils import (
     devMode,
     from_json_bytes,
@@ -52,11 +54,27 @@ EmptyCardsReport = _pb.EmptyCardsReport
 GraphPreferences = _pb.GraphPreferences
 BuiltinSort = _pb.SortOrder.Builtin
 Preferences = _pb.Preferences
+UndoStatus = _pb.UndoStatus
+
+
+@dataclass
+class ReviewUndo:
+    card: Card
+    was_leech: bool
+
+
+@dataclass
+class Checkpoint:
+    name: str
+
+
+@dataclass
+class BackendUndo:
+    name: str
 
 
 class Collection:
     sched: Union[V1Scheduler, V2Scheduler]
-    _undo: List[Any]
 
     def __init__(
         self,
@@ -74,7 +92,7 @@ class Collection:
 
         self.log(self.path, anki.version)
         self._lastSave = time.time()
-        self.clearUndo()
+        self._undo: _UndoInfo = None
         self.media = MediaManager(self, server)
         self.models = ModelManager(self)
         self.decks = DeckManager(self)
@@ -146,7 +164,7 @@ class Collection:
 
     def upgrade_to_v2_scheduler(self) -> None:
         self._backend.upgrade_scheduler()
-        self.clearUndo()
+        self.clear_python_undo()
         self._loadScheduler()
 
     def is_2021_test_scheduler_enabled(self) -> bool:
@@ -228,7 +246,7 @@ class Collection:
             # outside of a transaction, we need to roll back
             self.db.rollback()
 
-        self._markOp(name)
+        self._save_checkpoint(name)
         self._lastSave = time.time()
 
     def autosave(self) -> Optional[bool]:
@@ -730,91 +748,136 @@ table.review-log {{ {revlog_style} }}
 
     # Undo
     ##########################################################################
-    # this data structure is a mess, and will be updated soon
-    # in the review case, [1, "Review", [firstReviewedCard, secondReviewedCard, ...], wasLeech]
-    # in the checkpoint case, [2, "action name"]
-    # wasLeech should have been recorded for each card, not globally
 
-    def clearUndo(self) -> None:
+    def undo_status(self) -> UndoStatus:
+        "Return the undo status. At the moment, redo is not supported."
+        # check backend first
+        status = self._backend.get_undo_status()
+        if status.undo or status.redo:
+            return status
+
+        if not self._undo:
+            return status
+
+        if isinstance(self._undo, _ReviewsUndo):
+            status.undo = self.tr(TR.SCHEDULING_REVIEW)
+        elif isinstance(self._undo, Checkpoint):
+            status.undo = self._undo.name
+        else:
+            assert_exhaustive(self._undo)
+            assert False
+
+        return status
+
+    def clear_python_undo(self) -> None:
+        """Clear the Python undo state.
+        The backend will automatically clear backend undo state when
+        any SQL DML is executed, or an operation that doesn't support undo
+        is run."""
         self._undo = None
 
-    def undoName(self) -> Any:
+    def undo(self) -> Union[None, BackendUndo, Checkpoint, ReviewUndo]:
-        "Undo menu item name, or None if undo unavailable."
+        """Returns ReviewUndo if undoing a v1/v2 scheduler review.
-        if not self._undo:
+        Returns None if the undo queue was empty."""
+        # backend?
+        status = self._backend.get_undo_status()
+        if status.undo:
+            self._backend.undo()
+            self.clear_python_undo()
+            return BackendUndo(name=status.undo)
+
+        if isinstance(self._undo, _ReviewsUndo):
+            return self._undo_review()
+        elif isinstance(self._undo, Checkpoint):
+            return self._undo_checkpoint()
+        elif self._undo is None:
             return None
-        return self._undo[1]
-
-    def undo(self) -> Any:
-        if self._undo[0] == 1:
-            return self._undoReview()
         else:
-            self._undoOp()
+            assert_exhaustive(self._undo)
+            assert False
 
-    def markReview(self, card: Card) -> None:
+    def save_card_review_undo_info(self, card: Card) -> None:
-        old: List[Any] = []
+        "Used by V1 and V2 schedulers to record state prior to review."
-        if self._undo:
+        if not isinstance(self._undo, _ReviewsUndo):
-            if self._undo[0] == 1:
+            self._undo = _ReviewsUndo()
-                old = self._undo[2]
+
-            self.clearUndo()
+        was_leech = card.note().hasTag("leech")
-        wasLeech = card.note().hasTag("leech") or False
+        entry = ReviewUndo(card=copy.copy(card), was_leech=was_leech)
-        self._undo = [
+        self._undo.entries.append(entry)
-            1,
+
-            self.tr(TR.SCHEDULING_REVIEW),
+    def _undo_checkpoint(self) -> Checkpoint:
-            old + [copy.copy(card)],
+        assert isinstance(self._undo, Checkpoint)
-            wasLeech,
+        self.rollback()
-        ]
+        undo = self._undo
+        self.clear_python_undo()
+        return undo
+
+    def _save_checkpoint(self, name: Optional[str]) -> None:
+        "Call via .save(). If name not provided, clear any existing checkpoint."
+        if name:
+            self._undo = Checkpoint(name=name)
+        else:
+            # saving disables old checkpoint, but not review undo
+            if not isinstance(self._undo, _ReviewsUndo):
+                self.clear_python_undo()
+
+    def _undo_review(self) -> ReviewUndo:
+        "Undo a v1/v2 review."
+        assert isinstance(self._undo, _ReviewsUndo)
+        entry = self._undo.entries.pop()
+        if not self._undo.entries:
+            self.clear_python_undo()
+
+        card = entry.card
 
-    def _undoReview(self) -> Any:
-        data = self._undo[2]
-        wasLeech = self._undo[3]
-        c = data.pop()  # pytype: disable=attribute-error
-        if not data:
-            self.clearUndo()
         # remove leech tag if it didn't have it before
-        if not wasLeech and c.note().hasTag("leech"):
+        if not entry.was_leech and card.note().hasTag("leech"):
-            c.note().delTag("leech")
+            card.note().delTag("leech")
-            c.note().flush()
+            card.note().flush()
+
         # write old data
-        c.flush()
+        card.flush()
+
         # and delete revlog entry if not previewing
-        conf = self.sched._cardConf(c)
+        conf = self.sched._cardConf(card)
         previewing = conf["dyn"] and not conf["resched"]
         if not previewing:
             last = self.db.scalar(
-                "select id from revlog where cid = ? " "order by id desc limit 1", c.id
+                "select id from revlog where cid = ? " "order by id desc limit 1",
+                card.id,
             )
             self.db.execute("delete from revlog where id = ?", last)
+
         # restore any siblings
         self.db.execute(
             "update cards set queue=type,mod=?,usn=? where queue=-2 and nid=?",
             intTime(),
             self.usn(),
-            c.nid,
+            card.nid,
         )
-        # and finally, update daily counts
+
-        if self.sched.is_2021:
+        # update daily counts
-            self._backend.requeue_undone_card(c.id)
+        n = card.queue
-        else:
+        if card.queue in (QUEUE_TYPE_DAY_LEARN_RELEARN, QUEUE_TYPE_PREVIEW):
-            n = c.queue
+            n = QUEUE_TYPE_LRN
-            if c.queue in (QUEUE_TYPE_DAY_LEARN_RELEARN, QUEUE_TYPE_PREVIEW):
+        type = ("new", "lrn", "rev")[n]
-                n = QUEUE_TYPE_LRN
+        self.sched._updateStats(card, type, -1)
-            type = ("new", "lrn", "rev")[n]
-            self.sched._updateStats(c, type, -1)
         self.sched.reps -= 1
-        return c.id
 
-    def _markOp(self, name: Optional[str]) -> None:
+        # and refresh the queues
-        "Call via .save()"
+        self.sched.reset()
-        if name:
-            self._undo = [2, name]
-        else:
-            # saving disables old checkpoint, but not review undo
-            if self._undo and self._undo[0] == 2:
-                self.clearUndo()
 
-    def _undoOp(self) -> None:
+        return entry
-        self.rollback()
+
-        self.clearUndo()
+    # legacy
+
+    clearUndo = clear_python_undo
+    markReview = save_card_review_undo_info
+
+    def undoName(self) -> Optional[str]:
+        "Undo menu item name, or None if undo unavailable."
+        status = self.undo_status()
+        return status.undo or None
 
     # DB maintenance
     ##########################################################################
@@ -946,3 +1009,11 @@ table.review-log {{ {revlog_style} }}
 
 # legacy name
 _Collection = Collection
+
+
+@dataclass
+class _ReviewsUndo:
+    entries: List[ReviewUndo] = field(default_factory=list)
+
+
+_UndoInfo = Union[_ReviewsUndo, Checkpoint, None]

pylib/anki/notes.py

@@ -154,7 +154,7 @@ class Note:
     # Tags
     ##################################################
 
-    def hasTag(self, tag: str) -> Any:
+    def hasTag(self, tag: str) -> bool:
         return self.col.tags.inList(tag, self.tags)
 
     def stringTags(self) -> Any:

pylib/anki/sched.py

@@ -45,7 +45,7 @@ class Scheduler(V2):
     def answerCard(self, card: Card, ease: int) -> None:
         self.col.log()
         assert 1 <= ease <= 4
-        self.col.markReview(card)
+        self.col.save_card_review_undo_info(card)
         if self._burySiblingsOnAnswer:
             self._burySiblings(card)
         card.reps += 1

pylib/anki/scheduler.py

@@ -121,8 +121,6 @@ class Scheduler:
         assert 1 <= ease <= 4
         assert 0 <= card.queue <= 4
 
-        self.col.markReview(card)
-
         new_state = self._answerCard(card, ease)
 
         self._handle_leech(card, new_state)

pylib/anki/schedv2.py

@@ -451,7 +451,7 @@ limit ?"""
         self.col.log()
         assert 1 <= ease <= 4
         assert 0 <= card.queue <= 4
-        self.col.markReview(card)
+        self.col.save_card_review_undo_info(card)
         if self._burySiblingsOnAnswer:
             self._burySiblings(card)
 

qt/aqt/addcards.py

@@ -178,7 +178,7 @@ class AddCards(QDialog):
                 if not askUser(tr(TR.ADDING_YOU_HAVE_A_CLOZE_DELETION_NOTE)):
                     return None
         self.mw.col.add_note(note, self.deckChooser.selectedId())
-        self.mw.col.clearUndo()
+        self.mw.col.clear_python_undo()
         self.addHistory(note)
         self.previousNote = note
         self.mw.requireReset(reason=ResetReason.AddCardsAddNote, context=self)

qt/aqt/main.py

@@ -27,10 +27,11 @@ import aqt.toolbar
 import aqt.webview
 from anki import hooks
 from anki._backend import RustBackend as _RustBackend
-from anki.collection import Collection
+from anki.collection import BackendUndo, Checkpoint, Collection, ReviewUndo
 from anki.decks import Deck
 from anki.hooks import runHook
 from anki.sound import AVTag, SoundOrVideoTag
+from anki.types import assert_exhaustive
 from anki.utils import devMode, ids2str, intTime, isMac, isWin, splitFields
 from aqt import gui_hooks
 from aqt.addons import DownloadLogEntry, check_and_prompt_for_updates, show_log_to_user
@@ -911,7 +912,7 @@ title="%s" %s>%s</button>""" % (
             self.media_syncer.start()
 
         def on_collection_sync_finished() -> None:
-            self.col.clearUndo()
+            self.col.clear_python_undo()
             self.col.models._clear_cache()
             gui_hooks.sync_did_finish()
             self.reset()
@@ -1024,29 +1025,63 @@ title="%s" %s>%s</button>""" % (
     ##########################################################################
 
     def onUndo(self) -> None:
-        n = self.col.undoName()
+        reviewing = self.state == "review"
-        if not n:
+        result = self.col.undo()
+
+        if result is None:
+            # should not happen
+            showInfo("nothing to undo")
+            self.maybeEnableUndo()
             return
-        cid = self.col.undo()
+
-        if cid and self.state == "review":
+        elif isinstance(result, ReviewUndo):
-            card = self.col.getCard(cid)
+            name = tr(TR.SCHEDULING_REVIEW)
-            self.col.sched.reset()
+
-            self.reviewer.cardQueue.append(card)
+            # restore the undone card if reviewing
-            self.reviewer.nextCard()
+            if reviewing:
-            gui_hooks.review_did_undo(cid)
+                cid = result.card.id
+                card = self.col.getCard(cid)
+                self.reviewer.cardQueue.append(card)
+                self.reviewer.nextCard()
+                gui_hooks.review_did_undo(cid)
+                self.maybeEnableUndo()
+                return
+
+        elif isinstance(result, BackendUndo):
+            name = result.name
+
+            # new scheduler takes care of rebuilding queue
+            if reviewing and self.col.sched.is_2021:
+                self.reviewer.nextCard()
+                self.maybeEnableUndo()
+                return
+
+        elif isinstance(result, Checkpoint):
+            name = result.name
+
         else:
-            self.reset()
+            assert_exhaustive(result)
-            tooltip(tr(TR.QT_MISC_REVERTED_TO_STATE_PRIOR_TO, val=n.lower()))
+            assert False
-            gui_hooks.state_did_revert(n)
+
+        self.reset()
+        tooltip(tr(TR.QT_MISC_REVERTED_TO_STATE_PRIOR_TO, val=name))
+        gui_hooks.state_did_revert(name)
         self.maybeEnableUndo()
 
     def maybeEnableUndo(self) -> None:
-        if self.col and self.col.undoName():
+        if self.col:
-            self.form.actionUndo.setText(tr(TR.QT_MISC_UNDO2, val=self.col.undoName()))
+            status = self.col.undo_status()
+            undo_action = status.undo or None
+        else:
+            undo_action = None
+
+        if undo_action:
+            undo_action = tr(TR.UNDO_UNDO_ACTION, val=undo_action)
+            self.form.actionUndo.setText(undo_action)
             self.form.actionUndo.setEnabled(True)
             gui_hooks.undo_state_did_change(True)
         else:
-            self.form.actionUndo.setText(tr(TR.QT_MISC_UNDO))
+            self.form.actionUndo.setText(tr(TR.UNDO_UNDO))
             self.form.actionUndo.setEnabled(False)
             gui_hooks.undo_state_did_change(False)
 

rslib/backend.proto

@@ -122,7 +122,6 @@ service BackendService {
   rpc UpgradeScheduler(Empty) returns (Empty);
   rpc GetQueuedCards(GetQueuedCardsIn) returns (GetQueuedCardsOut);
   rpc ClearCardQueues(Empty) returns (Empty);
-  rpc RequeueUndoneCard(CardID) returns (Empty);
 
   // stats
 
@@ -199,6 +198,9 @@ service BackendService {
   rpc OpenCollection(OpenCollectionIn) returns (Empty);
   rpc CloseCollection(CloseCollectionIn) returns (Empty);
   rpc CheckDatabase(Empty) returns (CheckDatabaseOut);
+  rpc GetUndoStatus(Empty) returns (UndoStatus);
+  rpc Undo(Empty) returns (UndoStatus);
+  rpc Redo(Empty) returns (UndoStatus);
 
   // sync
 
@@ -1391,3 +1393,8 @@ message GetQueuedCardsOut {
     CongratsInfoOut congrats_info = 2;
   }
 }
+
+message UndoStatus {
+  string undo = 1;
+  string redo = 2;
+}

rslib/src/backend/dbproxy.rs

@@ -75,6 +75,7 @@ pub(super) fn db_command_bytes(col: &mut Collection, input: &[u8]) -> Result<Vec
             args,
             first_row_only,
         } => {
+            maybe_clear_undo(col, &sql);
             if first_row_only {
                 db_query_row(&col.storage, &sql, &args)?
             } else {
@@ -94,11 +95,32 @@ pub(super) fn db_command_bytes(col: &mut Collection, input: &[u8]) -> Result<Vec
             col.storage.rollback_trx()?;
             DBResult::None
         }
-        DBRequest::ExecuteMany { sql, args } => db_execute_many(&col.storage, &sql, &args)?,
+        DBRequest::ExecuteMany { sql, args } => {
+            maybe_clear_undo(col, &sql);
+            db_execute_many(&col.storage, &sql, &args)?
+        }
     };
     Ok(serde_json::to_vec(&resp)?)
 }
 
+fn maybe_clear_undo(col: &mut Collection, sql: &str) {
+    if !is_dql(sql) {
+        println!("clearing undo due to {}", sql);
+        col.state.undo.clear();
+    }
+}
+
+/// Anything other than a select statement is false.
+fn is_dql(sql: &str) -> bool {
+    let head: String = sql
+        .trim_start()
+        .chars()
+        .take(10)
+        .map(|c| c.to_ascii_lowercase())
+        .collect();
+    head.starts_with("select ")
+}
+
 pub(super) fn db_query_row(ctx: &SqliteStorage, sql: &str, args: &[SqlValue]) -> Result<DBResult> {
     let mut stmt = ctx.db.prepare_cached(sql)?;
     let columns = stmt.column_count();

rslib/src/backend/mod.rs

@@ -712,11 +712,6 @@ impl BackendService for Backend {
         })
     }
 
-    fn requeue_undone_card(&self, input: pb::CardId) -> BackendResult<pb::Empty> {
-        self.with_col(|col| col.requeue_undone_card(input.into()))
-            .map(Into::into)
-    }
-
     // statistics
     //-----------------------------------------------
 
@@ -1320,6 +1315,24 @@ impl BackendService for Backend {
         })
     }
 
+    fn get_undo_status(&self, _input: pb::Empty) -> Result<pb::UndoStatus> {
+        self.with_col(|col| Ok(col.undo_status()))
+    }
+
+    fn undo(&self, _input: pb::Empty) -> Result<pb::UndoStatus> {
+        self.with_col(|col| {
+            col.undo()?;
+            Ok(col.undo_status())
+        })
+    }
+
+    fn redo(&self, _input: pb::Empty) -> Result<pb::UndoStatus> {
+        self.with_col(|col| {
+            col.redo()?;
+            Ok(col.undo_status())
+        })
+    }
+
     // sync
     //-------------------------------------------------------------------
 

rslib/src/card.rs

@@ -127,12 +127,12 @@ impl Card {
 pub(crate) struct CardUpdated(Card);
 
 impl Undo for CardUpdated {
-    fn undo(self: Box<Self>, col: &mut crate::collection::Collection, usn: Usn) -> Result<()> {
+    fn undo(self: Box<Self>, col: &mut crate::collection::Collection) -> Result<()> {
         let current = col
             .storage
             .get_card(self.0.id)?
             .ok_or_else(|| AnkiError::invalid_input("card disappeared"))?;
-        col.update_card(&mut self.0.clone(), &current, usn)
+        col.update_card_inner(&mut self.0.clone(), &current)
     }
 }
 
@@ -164,12 +164,17 @@ impl Collection {
         Ok(card)
     }
 
+    /// Marks the card as modified, then saves it.
     pub(crate) fn update_card(&mut self, card: &mut Card, original: &Card, usn: Usn) -> Result<()> {
+        card.set_modified(usn);
+        self.update_card_inner(card, original)
+    }
+
+    pub(crate) fn update_card_inner(&mut self, card: &mut Card, original: &Card) -> Result<()> {
         if card.id.0 == 0 {
             return Err(AnkiError::invalid_input("card id not set"));
         }
         self.save_undo(Box::new(CardUpdated(original.clone())));
-        card.set_modified(usn);
         self.storage.update_card(card)
     }
 

rslib/src/decks/mod.rs

@@ -271,11 +271,11 @@ impl Collection {
             let usn = col.usn()?;
 
             col.prepare_deck_for_update(deck, usn)?;
+            deck.set_modified(usn);
 
             if deck.id.0 == 0 {
                 // TODO: undo support
                 col.match_or_create_parents(deck, usn)?;
-                deck.set_modified(usn);
                 col.storage.add_deck(deck)
             } else if let Some(existing_deck) = col.storage.get_deck(deck.id)? {
                 let name_changed = existing_deck.name != deck.name;
@@ -285,7 +285,7 @@ impl Collection {
                     // rename children
                     col.rename_child_decks(&existing_deck, &deck.name, usn)?;
                 }
-                col.update_single_deck_no_check(deck, &existing_deck, usn)?;
+                col.update_single_deck_inner_undo_only(deck, &existing_deck)?;
                 if name_changed {
                     // after updating, we need to ensure all grandparents exist, which may not be the case
                     // in the parent->child case
@@ -313,15 +313,13 @@ impl Collection {
     }
 
     /// Update an individual, existing deck. Caller is responsible for ensuring deck
-    /// is normalized, matches parents, and is not a duplicate name. Bumps mtime.
+    /// is normalized, matches parents, is not a duplicate name, and bumping mtime.
-    pub(crate) fn update_single_deck_no_check(
+    pub(crate) fn update_single_deck_inner_undo_only(
         &mut self,
         deck: &mut Deck,
         original: &Deck,
-        usn: Usn,
     ) -> Result<()> {
         self.state.deck_cache.clear();
-        deck.set_modified(usn);
         self.save_undo(Box::new(DeckUpdated(original.clone())));
         self.storage.update_deck(deck)
     }
@@ -372,7 +370,8 @@ impl Collection {
             let child_only = &child_components[old_component_count..];
             let new_name = format!("{}\x1f{}", new_name, child_only.join("\x1f"));
             child.name = new_name;
-            self.update_single_deck_no_check(&mut child, &original, usn)?;
+            child.set_modified(usn);
+            self.update_single_deck_inner_undo_only(&mut child, &original)?;
         }
 
         Ok(())
@@ -601,7 +600,8 @@ impl Collection {
         let original = deck.clone();
         deck.reset_stats_if_day_changed(today);
         mutator(&mut deck.common);
-        self.update_single_deck_no_check(deck, &original, usn)
+        deck.set_modified(usn);
+        self.update_single_deck_inner_undo_only(deck, &original)
     }
 
     pub fn drag_drop_decks(
@@ -651,12 +651,12 @@ impl Collection {
 pub(crate) struct DeckUpdated(Deck);
 
 impl Undo for DeckUpdated {
-    fn undo(mut self: Box<Self>, col: &mut crate::collection::Collection, usn: Usn) -> Result<()> {
+    fn undo(mut self: Box<Self>, col: &mut crate::collection::Collection) -> Result<()> {
         let current = col
             .storage
             .get_deck(self.0.id)?
             .ok_or_else(|| AnkiError::invalid_input("deck disappeared"))?;
-        col.update_single_deck_no_check(&mut self.0, &current, usn)
+        col.update_single_deck_inner_undo_only(&mut self.0, &current)
     }
 }
 

rslib/src/notes.rs

@@ -369,24 +369,19 @@ impl Collection {
     ) -> Result<()> {
         self.canonify_note_tags(note, usn)?;
         note.prepare_for_update(nt, normalize_text)?;
-        self.update_note_inner_undo_and_mtime_only(
+        if mark_note_modified {
-            note,
+            note.set_modified(usn);
-            original,
+        }
-            if mark_note_modified { Some(usn) } else { None },
+        self.update_note_inner_undo_only(note, original)
-        )
     }
 
-    /// Bumps modification time if usn provided, saves in the undo queue, and commits to DB.
+    /// Saves in the undo queue, and commits to DB.
     /// No validation, card generation or normalization is done.
-    pub(crate) fn update_note_inner_undo_and_mtime_only(
+    pub(crate) fn update_note_inner_undo_only(
         &mut self,
         note: &mut Note,
         original: &Note,
-        update_usn: Option<Usn>,
     ) -> Result<()> {
-        if let Some(usn) = update_usn {
-            note.set_modified(usn);
-        }
         self.save_undo(Box::new(NoteUpdated(original.clone())));
         self.storage.update_note(note)?;
 
@@ -557,12 +552,12 @@ impl Collection {
 pub(crate) struct NoteUpdated(Note);
 
 impl Undo for NoteUpdated {
-    fn undo(self: Box<Self>, col: &mut crate::collection::Collection, usn: Usn) -> Result<()> {
+    fn undo(self: Box<Self>, col: &mut crate::collection::Collection) -> Result<()> {
         let current = col
             .storage
             .get_note(self.0.id)?
             .ok_or_else(|| AnkiError::invalid_input("note disappeared"))?;
-        col.update_note_inner_undo_and_mtime_only(&mut self.0.clone(), &current, Some(usn))
+        col.update_note_inner_undo_only(&mut self.0.clone(), &current)
     }
 }
 

rslib/src/revlog.rs

@@ -118,7 +118,7 @@ pub(crate) struct RevlogAdded(RevlogEntry);
 pub(crate) struct RevlogRemoved(RevlogEntry);
 
 impl Undo for RevlogAdded {
-    fn undo(self: Box<Self>, col: &mut crate::collection::Collection, _usn: Usn) -> Result<()> {
+    fn undo(self: Box<Self>, col: &mut crate::collection::Collection) -> Result<()> {
         col.storage.remove_revlog_entry(self.0.id)?;
         col.save_undo(Box::new(RevlogRemoved(self.0)));
         Ok(())
@@ -126,7 +126,7 @@ impl Undo for RevlogAdded {
 }
 
 impl Undo for RevlogRemoved {
-    fn undo(self: Box<Self>, col: &mut crate::collection::Collection, _usn: Usn) -> Result<()> {
+    fn undo(self: Box<Self>, col: &mut crate::collection::Collection) -> Result<()> {
         col.storage.add_revlog_entry(&self.0, false)?;
         col.save_undo(Box::new(RevlogAdded(self.0)));
         Ok(())

rslib/src/scheduler/answering/undo.rs

@@ -95,6 +95,8 @@ mod test {
             let deck = col.get_deck(DeckID(1))?.unwrap();
             assert_eq!(deck.common.review_studied, 1);
 
+            assert_eq!(col.next_card()?.is_some(), false);
+
             Ok(())
         };
 
@@ -119,6 +121,8 @@ mod test {
             let deck = col.get_deck(DeckID(1))?.unwrap();
             assert_eq!(deck.common.review_studied, 0);
 
+            assert_eq!(col.next_card()?.is_some(), true);
+
             Ok(())
         };
 
@@ -135,8 +139,6 @@ mod test {
         col.undo()?;
         assert_initial_state(&mut col)?;
 
-        // fixme: make sure queue state updated, esp. on redo
-
         Ok(())
     }
 }

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

@@ -13,7 +13,7 @@ use std::{
 
 use super::{
     limits::{remaining_limits_capped_to_parents, RemainingLimits},
-    CardQueues, LearningQueueEntry, QueueEntry, QueueEntryKind,
+    CardQueues, Counts, LearningQueueEntry, MainQueueEntry, MainQueueEntryKind,
 };
 use crate::deckconf::{NewCardOrder, ReviewCardOrder, ReviewMix};
 use crate::prelude::*;
@@ -41,22 +41,22 @@ pub(crate) struct NewCard {
     pub extra: u64,
 }
 
-impl From<DueCard> for QueueEntry {
+impl From<DueCard> for MainQueueEntry {
     fn from(c: DueCard) -> Self {
-        QueueEntry {
+        MainQueueEntry {
             id: c.id,
             mtime: c.mtime,
-            kind: QueueEntryKind::Review,
+            kind: MainQueueEntryKind::Review,
         }
     }
 }
 
-impl From<NewCard> for QueueEntry {
+impl From<NewCard> for MainQueueEntry {
     fn from(c: NewCard) -> Self {
-        QueueEntry {
+        MainQueueEntry {
             id: c.id,
             mtime: c.mtime,
-            kind: QueueEntryKind::New,
+            kind: MainQueueEntryKind::New,
         }
     }
 }
@@ -113,9 +113,12 @@ impl QueueBuilder {
         let main_iter = merge_new(main_iter, self.new, self.new_review_mix);
 
         CardQueues {
-            new_count,
+            counts: Counts {
-            review_count,
+                new: new_count,
-            learn_count,
+                review: review_count,
+                learning: learn_count,
+            },
+            undo: Vec::new(),
             main: main_iter.collect(),
             due_learning,
             later_learning,
@@ -130,7 +133,7 @@ fn merge_day_learning(
     reviews: Vec<DueCard>,
     day_learning: Vec<DueCard>,
     mode: ReviewMix,
-) -> Box<dyn ExactSizeIterator<Item = QueueEntry>> {
+) -> Box<dyn ExactSizeIterator<Item = MainQueueEntry>> {
     let day_learning_iter = day_learning.into_iter().map(Into::into);
     let reviews_iter = reviews.into_iter().map(Into::into);
 
@@ -142,10 +145,10 @@ fn merge_day_learning(
 }
 
 fn merge_new(
-    review_iter: impl ExactSizeIterator<Item = QueueEntry> + 'static,
+    review_iter: impl ExactSizeIterator<Item = MainQueueEntry> + 'static,
     new: Vec<NewCard>,
     mode: ReviewMix,
-) -> Box<dyn ExactSizeIterator<Item = QueueEntry>> {
+) -> Box<dyn ExactSizeIterator<Item = MainQueueEntry>> {
     let new_iter = new.into_iter().map(Into::into);
 
     match mode {
@@ -191,7 +194,7 @@ impl Collection {
         let timing = self.timing_for_timestamp(now)?;
         let (decks, parent_count) = self.storage.deck_with_parents_and_children(deck_id)?;
         let config = self.storage.get_deck_config_map()?;
-        let limits = remaining_limits_capped_to_parents(&decks, &config);
+        let limits = remaining_limits_capped_to_parents(&decks, &config, timing.days_elapsed);
         let selected_deck_limits = limits[parent_count];
 
         let mut queues = QueueBuilder::default();

rslib/src/scheduler/queue/entry.rs

@@ -0,0 +1,81 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+use super::{LearningQueueEntry, MainQueueEntry, MainQueueEntryKind};
+use crate::card::CardQueue;
+use crate::prelude::*;
+
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub(crate) enum QueueEntry {
+    IntradayLearning(LearningQueueEntry),
+    Main(MainQueueEntry),
+}
+
+impl QueueEntry {
+    pub fn card_id(&self) -> CardID {
+        match self {
+            QueueEntry::IntradayLearning(e) => e.id,
+            QueueEntry::Main(e) => e.id,
+        }
+    }
+
+    pub fn mtime(&self) -> TimestampSecs {
+        match self {
+            QueueEntry::IntradayLearning(e) => e.mtime,
+            QueueEntry::Main(e) => e.mtime,
+        }
+    }
+
+    pub fn kind(&self) -> QueueEntryKind {
+        match self {
+            QueueEntry::IntradayLearning(_e) => QueueEntryKind::Learning,
+            QueueEntry::Main(e) => match e.kind {
+                MainQueueEntryKind::New => QueueEntryKind::New,
+                MainQueueEntryKind::Review => QueueEntryKind::Review,
+            },
+        }
+    }
+}
+
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub(crate) enum QueueEntryKind {
+    New,
+    Review,
+    Learning,
+}
+
+impl From<&Card> for QueueEntry {
+    fn from(card: &Card) -> Self {
+        let kind = match card.queue {
+            CardQueue::Learn | CardQueue::PreviewRepeat => {
+                return QueueEntry::IntradayLearning(LearningQueueEntry {
+                    due: TimestampSecs(card.due as i64),
+                    id: card.id,
+                    mtime: card.mtime,
+                });
+            }
+            CardQueue::New => MainQueueEntryKind::New,
+            CardQueue::Review | CardQueue::DayLearn => MainQueueEntryKind::Review,
+            CardQueue::Suspended | CardQueue::SchedBuried | CardQueue::UserBuried => {
+                unreachable!()
+            }
+        };
+        QueueEntry::Main(MainQueueEntry {
+            id: card.id,
+            mtime: card.mtime,
+            kind,
+        })
+    }
+}
+
+impl From<LearningQueueEntry> for QueueEntry {
+    fn from(e: LearningQueueEntry) -> Self {
+        Self::IntradayLearning(e)
+    }
+}
+
+impl From<MainQueueEntry> for QueueEntry {
+    fn from(e: MainQueueEntry) -> Self {
+        Self::Main(e)
+    }
+}

rslib/src/scheduler/queue/learning.rs

@@ -6,9 +6,17 @@ use std::{
     collections::VecDeque,
 };
 
-use super::{CardQueues, LearningQueueEntry};
+use super::CardQueues;
 use crate::{prelude::*, scheduler::timing::SchedTimingToday};
 
+#[derive(Debug, Clone, Copy, PartialEq, PartialOrd, Eq, Ord)]
+pub(crate) struct LearningQueueEntry {
+    // due comes first, so the derived ordering sorts by due
+    pub due: TimestampSecs,
+    pub id: CardID,
+    pub mtime: TimestampSecs,
+}
+
 impl CardQueues {
     /// Check for any newly due cards, and then return the first, if any,
     /// that is due before now.
@@ -32,7 +40,7 @@ impl CardQueues {
     pub(super) fn pop_learning_entry(&mut self, id: CardID) -> Option<LearningQueueEntry> {
         if let Some(top) = self.due_learning.front() {
             if top.id == id {
-                self.learn_count -= 1;
+                self.counts.learning -= 1;
                 return self.due_learning.pop_front();
             }
         }
@@ -42,7 +50,7 @@ impl CardQueues {
         // so for now we also check the head of the later_due queue
         if let Some(top) = self.later_learning.peek() {
             if top.0.id == id {
-                // self.learn_count -= 1;
+                // self.counts.learning -= 1;
                 return self.later_learning.pop().map(|c| c.0);
             }
         }
@@ -52,20 +60,34 @@ impl CardQueues {
 
     /// Given the just-answered `card`, place it back in the learning queues if it's still
     /// due today. Avoid placing it in a position where it would be shown again immediately.
-    pub(super) fn maybe_requeue_learning_card(&mut self, card: &Card, timing: SchedTimingToday) {
+    pub(super) fn maybe_requeue_learning_card(
-        if !card.is_intraday_learning() {
+        &mut self,
-            return;
+        card: &Card,
+        timing: SchedTimingToday,
+    ) -> Option<LearningQueueEntry> {
+        // not due today?
+        if !card.is_intraday_learning() || card.due >= timing.next_day_at as i32 {
+            return None;
         }
 
+        let entry = LearningQueueEntry {
+            due: TimestampSecs(card.due as i64),
+            id: card.id,
+            mtime: card.mtime,
+        };
+
+        Some(self.requeue_learning_entry(entry, timing))
+    }
+
+    /// Caller must have validated learning entry is due today.
+    pub(super) fn requeue_learning_entry(
+        &mut self,
+        mut entry: LearningQueueEntry,
+        timing: SchedTimingToday,
+    ) -> LearningQueueEntry {
         let learn_ahead_limit = timing.now.adding_secs(self.learn_ahead_secs);
 
-        if card.due < learn_ahead_limit.0 as i32 {
+        if entry.due < learn_ahead_limit {
-            let mut entry = LearningQueueEntry {
-                due: TimestampSecs(card.due as i64),
-                id: card.id,
-                mtime: card.mtime,
-            };
-
             if self.learning_collapsed() {
                 if let Some(next) = self.due_learning.front() {
                     if next.due >= entry.due {
@@ -83,13 +105,12 @@ impl CardQueues {
                 // not collapsed; can add normally
                 self.push_due_learning_card(entry);
             }
-        } else if card.due < timing.next_day_at as i32 {
+        } else {
-            self.later_learning.push(Reverse(LearningQueueEntry {
+            // due outside current learn ahead limit, but later today
-                due: TimestampSecs(card.due as i64),
+            self.later_learning.push(Reverse(entry));
-                id: card.id,
+        }
-                mtime: card.mtime,
+
-            }));
+        entry
-        };
     }
 
     fn learning_collapsed(&self) -> bool {
@@ -98,7 +119,7 @@ impl CardQueues {
 
     /// Adds card, maintaining correct sort order, and increments learning count.
     pub(super) fn push_due_learning_card(&mut self, entry: LearningQueueEntry) {
-        self.learn_count += 1;
+        self.counts.learning += 1;
         let target_idx =
             binary_search_by(&self.due_learning, |e| e.due.cmp(&entry.due)).unwrap_or_else(|e| e);
         self.due_learning.insert(target_idx, entry);
@@ -113,6 +134,26 @@ impl CardQueues {
             self.push_due_learning_card(entry);
         }
     }
+
+    pub(super) fn remove_requeued_learning_card_after_undo(&mut self, id: CardID) {
+        let due_idx = self
+            .due_learning
+            .iter()
+            .enumerate()
+            .find_map(|(idx, entry)| if entry.id == id { Some(idx) } else { None });
+        if let Some(idx) = due_idx {
+            self.counts.learning -= 1;
+            self.due_learning.remove(idx);
+        } else {
+            // card may be in the later_learning binary heap - we can't remove
+            // it in place, so we have to rebuild it
+            self.later_learning = self
+                .later_learning
+                .drain()
+                .filter(|e| e.0.id != id)
+                .collect();
+        }
+    }
 }
 
 /// Adapted from the Rust stdlib VecDeque implementation; we can drop this when the following

rslib/src/scheduler/queue/limits.rs

@@ -12,12 +12,12 @@ pub(crate) struct RemainingLimits {
 }
 
 impl RemainingLimits {
-    pub(crate) fn new(deck: &Deck, config: Option<&DeckConf>) -> Self {
+    pub(crate) fn new(deck: &Deck, config: Option<&DeckConf>, today: u32) -> Self {
         if let Some(config) = config {
+            let (new_today, rev_today) = deck.new_rev_counts(today);
             RemainingLimits {
-                review: ((config.inner.reviews_per_day as i32) - deck.common.review_studied).max(0)
+                review: ((config.inner.reviews_per_day as i32) - rev_today).max(0) as u32,
-                    as u32,
+                new: ((config.inner.new_per_day as i32) - new_today).max(0) as u32,
-                new: ((config.inner.new_per_day as i32) - deck.common.new_studied).max(0) as u32,
             }
         } else {
             RemainingLimits {
@@ -36,8 +36,9 @@ impl RemainingLimits {
 pub(super) fn remaining_limits_capped_to_parents(
     decks: &[Deck],
     config: &HashMap<DeckConfID, DeckConf>,
+    today: u32,
 ) -> Vec<RemainingLimits> {
-    let mut limits = get_remaining_limits(decks, config);
+    let mut limits = get_remaining_limits(decks, config, today);
     cap_limits_to_parents(decks.iter().map(|d| d.name.as_str()), &mut limits);
     limits
 }
@@ -47,6 +48,7 @@ pub(super) fn remaining_limits_capped_to_parents(
 fn get_remaining_limits(
     decks: &[Deck],
     config: &HashMap<DeckConfID, DeckConf>,
+    today: u32,
 ) -> Vec<RemainingLimits> {
     decks
         .iter()
@@ -57,7 +59,7 @@ fn get_remaining_limits(
             } else {
                 None
             };
-            RemainingLimits::new(deck, config)
+            RemainingLimits::new(deck, config, today)
         })
         .collect()
 }

rslib/src/scheduler/queue/main.rs

@@ -1,21 +1,33 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
-use super::{CardQueues, QueueEntry, QueueEntryKind};
+use super::CardQueues;
 use crate::prelude::*;
 
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub(crate) struct MainQueueEntry {
+    pub id: CardID,
+    pub mtime: TimestampSecs,
+    pub kind: MainQueueEntryKind,
+}
+
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub(crate) enum MainQueueEntryKind {
+    New,
+    Review,
+}
+
 impl CardQueues {
-    pub(super) fn next_main_entry(&self) -> Option<QueueEntry> {
+    pub(super) fn next_main_entry(&self) -> Option<MainQueueEntry> {
         self.main.front().copied()
     }
 
-    pub(super) fn pop_main_entry(&mut self, id: CardID) -> Option<QueueEntry> {
+    pub(super) fn pop_main_entry(&mut self, id: CardID) -> Option<MainQueueEntry> {
         if let Some(last) = self.main.front() {
             if last.id == id {
                 match last.kind {
-                    QueueEntryKind::New => self.new_count -= 1,
+                    MainQueueEntryKind::New => self.counts.new -= 1,
-                    QueueEntryKind::Review => self.review_count -= 1,
+                    MainQueueEntryKind::Review => self.counts.review -= 1,
-                    QueueEntryKind::Learning => unreachable!(),
                 }
                 return self.main.pop_front();
             }
@@ -23,15 +35,4 @@ impl CardQueues {
 
         None
     }
-
-    /// Add an undone card back to the 'front' of the list, and update
-    /// the counts.
-    pub(super) fn push_main_entry(&mut self, entry: QueueEntry) {
-        match entry.kind {
-            QueueEntryKind::New => self.new_count += 1,
-            QueueEntryKind::Review => self.review_count += 1,
-            QueueEntryKind::Learning => unreachable!(),
-        }
-        self.main.push_front(entry);
-    }
 }

rslib/src/scheduler/queue/mod.rs

@@ -2,28 +2,40 @@
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
 mod builder;
+mod entry;
 mod learning;
 mod limits;
 mod main;
+mod undo;
 
 use std::{
     cmp::Reverse,
     collections::{BinaryHeap, VecDeque},
 };
 
-use crate::{backend_proto as pb, card::CardQueue, prelude::*, timestamp::TimestampSecs};
+use crate::{backend_proto as pb, prelude::*, timestamp::TimestampSecs};
 pub(crate) use builder::{DueCard, NewCard};
+pub(crate) use {
+    entry::{QueueEntry, QueueEntryKind},
+    learning::LearningQueueEntry,
+    main::{MainQueueEntry, MainQueueEntryKind},
+};
+
+use self::undo::QueueUpdateAfterAnsweringCard;
 
 use super::{states::NextCardStates, timing::SchedTimingToday};
 
 #[derive(Debug)]
 pub(crate) struct CardQueues {
-    new_count: usize,
+    counts: Counts,
-    review_count: usize,
+
-    learn_count: usize,
+    /// Any undone items take precedence.
+    undo: Vec<QueueEntry>,
+
+    main: VecDeque<MainQueueEntry>,
 
-    main: VecDeque<QueueEntry>,
     due_learning: VecDeque<LearningQueueEntry>,
+
     later_learning: BinaryHeap<Reverse<LearningQueueEntry>>,
 
     selected_deck: DeckID,
@@ -31,118 +43,71 @@ pub(crate) struct CardQueues {
     learn_ahead_secs: i64,
 }
 
-#[derive(Debug)]
+#[derive(Debug, Copy, Clone)]
 pub(crate) struct Counts {
-    new: usize,
+    pub new: usize,
-    learning: usize,
+    pub learning: usize,
-    review: usize,
+    pub review: usize,
+}
+
+#[derive(Debug)]
+pub(crate) struct QueuedCard {
+    pub card: Card,
+    pub kind: QueueEntryKind,
+    pub next_states: NextCardStates,
+}
+
+pub(crate) struct QueuedCards {
+    pub cards: Vec<QueuedCard>,
+    pub new_count: usize,
+    pub learning_count: usize,
+    pub review_count: usize,
 }
 
 impl CardQueues {
     /// Get the next due card, if there is one.
     fn next_entry(&mut self, now: TimestampSecs) -> Option<QueueEntry> {
-        self.next_learning_entry_due_before_now(now)
+        self.next_undo_entry()
             .map(Into::into)
-            .or_else(|| self.next_main_entry())
+            .or_else(|| self.next_learning_entry_due_before_now(now).map(Into::into))
+            .or_else(|| self.next_main_entry().map(Into::into))
             .or_else(|| self.next_learning_entry_learning_ahead().map(Into::into))
     }
 
-    /// Remove the provided card from the top of the learning or main queues.
+    /// Remove the provided card from the top of the queues.
     /// If it was not at the top, return an error.
-    fn pop_answered(&mut self, id: CardID) -> Result<()> {
+    fn pop_answered(&mut self, id: CardID) -> Result<QueueEntry> {
-        if self.pop_main_entry(id).is_none() && self.pop_learning_entry(id).is_none() {
+        if let Some(entry) = self.pop_undo_entry(id) {
-            Err(AnkiError::invalid_input("not at top of queue"))
+            Ok(entry)
+        } else if let Some(entry) = self.pop_main_entry(id) {
+            Ok(entry.into())
+        } else if let Some(entry) = self.pop_learning_entry(id) {
+            Ok(entry.into())
         } else {
-            Ok(())
+            Err(AnkiError::invalid_input("not at top of queue"))
         }
     }
 
-    fn counts(&self) -> Counts {
+    pub(crate) fn counts(&self) -> Counts {
-        Counts {
+        self.counts
-            new: self.new_count,
-            learning: self.learn_count,
-            review: self.review_count,
-        }
     }
 
     fn is_stale(&self, deck: DeckID, current_day: u32) -> bool {
         self.selected_deck != deck || self.current_day != current_day
     }
 
-    fn update_after_answering_card(&mut self, card: &Card, timing: SchedTimingToday) -> Result<()> {
+    fn update_after_answering_card(
-        self.pop_answered(card.id)?;
+        &mut self,
-        self.maybe_requeue_learning_card(card, timing);
+        card: &Card,
-        Ok(())
+        timing: SchedTimingToday,
-    }
+    ) -> Result<QueueUpdateAfterAnsweringCard> {
+        let entry = self.pop_answered(card.id)?;
+        let requeued_learning = self.maybe_requeue_learning_card(card, timing);
 
-    /// Add a just-undone card back to the appropriate queue, updating counts.
+        Ok(QueueUpdateAfterAnsweringCard {
-    pub(crate) fn push_undone_card(&mut self, card: &Card) {
+            entry,
-        if card.is_intraday_learning() {
+            learning_requeue: requeued_learning,
-            self.push_due_learning_card(LearningQueueEntry {
+        })
-                due: TimestampSecs(card.due as i64),
-                id: card.id,
-                mtime: card.mtime,
-            })
-        } else {
-            self.push_main_entry(card.into())
-        }
-    }
-}
-
-#[derive(Clone, Copy, Debug, PartialEq)]
-pub(crate) struct QueueEntry {
-    id: CardID,
-    mtime: TimestampSecs,
-    kind: QueueEntryKind,
-}
-
-#[derive(Clone, Copy, Debug, PartialEq)]
-pub(crate) enum QueueEntryKind {
-    New,
-    /// Includes day-learning cards
-    Review,
-    Learning,
-}
-
-impl PartialOrd for QueueEntry {
-    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
-        self.id.partial_cmp(&other.id)
-    }
-}
-
-impl From<&Card> for QueueEntry {
-    fn from(card: &Card) -> Self {
-        let kind = match card.queue {
-            CardQueue::Learn | CardQueue::PreviewRepeat => QueueEntryKind::Learning,
-            CardQueue::New => QueueEntryKind::New,
-            CardQueue::Review | CardQueue::DayLearn => QueueEntryKind::Review,
-            CardQueue::Suspended | CardQueue::SchedBuried | CardQueue::UserBuried => {
-                unreachable!()
-            }
-        };
-        QueueEntry {
-            id: card.id,
-            mtime: card.mtime,
-            kind,
-        }
-    }
-}
-
-#[derive(Debug, Clone, Copy, PartialEq, PartialOrd, Eq, Ord)]
-struct LearningQueueEntry {
-    // due comes first, so the derived ordering sorts by due
-    due: TimestampSecs,
-    id: CardID,
-    mtime: TimestampSecs,
-}
-
-impl From<LearningQueueEntry> for QueueEntry {
-    fn from(e: LearningQueueEntry) -> Self {
-        Self {
-            id: e.id,
-            mtime: e.mtime,
-            kind: QueueEntryKind::Learning,
-        }
     }
 }
 
@@ -168,30 +133,28 @@ impl Collection {
     }
 
     pub(crate) fn clear_queues(&mut self) {
+        // clearing the queue will remove any undone reviews from the undo queue,
+        // causing problems if we then try to redo them
+        self.state.undo.clear_redo();
         self.state.card_queues = None;
     }
 
-    /// FIXME: remove this once undoing is moved into backend
-    pub(crate) fn requeue_undone_card(&mut self, card_id: CardID) -> Result<()> {
-        let card = self.storage.get_card(card_id)?.ok_or(AnkiError::NotFound)?;
-        self.get_queues()?.push_undone_card(&card);
-        Ok(())
-    }
-
     pub(crate) fn update_queues_after_answering_card(
         &mut self,
         card: &Card,
         timing: SchedTimingToday,
     ) -> Result<()> {
         if let Some(queues) = &mut self.state.card_queues {
-            queues.update_after_answering_card(card, timing)
+            let mutation = queues.update_after_answering_card(card, timing)?;
+            self.save_undo(Box::new(mutation));
+            Ok(())
         } else {
             // we currenly allow the queues to be empty for unit tests
             Ok(())
         }
     }
 
-    fn get_queues(&mut self) -> Result<&mut CardQueues> {
+    pub(crate) fn get_queues(&mut self) -> Result<&mut CardQueues> {
         let timing = self.timing_today()?;
         let deck = self.get_current_deck_id();
         let need_rebuild = self
@@ -217,9 +180,9 @@ impl Collection {
         if let Some(entry) = queues.next_entry(TimestampSecs::now()) {
             let card = self
                 .storage
-                .get_card(entry.id)?
+                .get_card(entry.card_id())?
                 .ok_or(AnkiError::NotFound)?;
-            if card.mtime != entry.mtime {
+            if card.mtime != entry.mtime() {
                 return Err(AnkiError::invalid_input(
                     "bug: card modified without updating queue",
                 ));
@@ -231,7 +194,7 @@ impl Collection {
             cards.push(QueuedCard {
                 card,
                 next_states,
-                kind: entry.kind,
+                kind: entry.kind(),
             });
         }
 
@@ -255,17 +218,3 @@ impl Collection {
             .map(|mut resp| resp.cards.pop().unwrap()))
     }
 }
-
-#[derive(Debug)]
-pub(crate) struct QueuedCard {
-    pub card: Card,
-    pub kind: QueueEntryKind,
-    pub next_states: NextCardStates,
-}
-
-pub(crate) struct QueuedCards {
-    pub cards: Vec<QueuedCard>,
-    pub new_count: usize,
-    pub learning_count: usize,
-    pub review_count: usize,
-}

rslib/src/scheduler/queue/undo.rs

@@ -0,0 +1,83 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+use super::{CardQueues, LearningQueueEntry, QueueEntry, QueueEntryKind};
+use crate::{prelude::*, undo::Undo};
+
+#[derive(Debug)]
+pub(super) struct QueueUpdateAfterAnsweringCard {
+    pub entry: QueueEntry,
+    pub learning_requeue: Option<LearningQueueEntry>,
+}
+
+impl Undo for QueueUpdateAfterAnsweringCard {
+    fn undo(self: Box<Self>, col: &mut Collection) -> Result<()> {
+        let queues = col.get_queues()?;
+        if let Some(learning) = self.learning_requeue {
+            queues.remove_requeued_learning_card_after_undo(learning.id);
+        }
+        queues.push_undo_entry(self.entry);
+        col.save_undo(Box::new(QueueUpdateAfterUndoingAnswer {
+            entry: self.entry,
+            learning_requeue: self.learning_requeue,
+        }));
+
+        Ok(())
+    }
+}
+
+#[derive(Debug)]
+pub(super) struct QueueUpdateAfterUndoingAnswer {
+    pub entry: QueueEntry,
+    pub learning_requeue: Option<LearningQueueEntry>,
+}
+
+impl Undo for QueueUpdateAfterUndoingAnswer {
+    fn undo(self: Box<Self>, col: &mut Collection) -> Result<()> {
+        let timing = col.timing_today()?;
+        let queues = col.get_queues()?;
+        let mut modified_learning = None;
+        if let Some(learning) = self.learning_requeue {
+            modified_learning = Some(queues.requeue_learning_entry(learning, timing));
+        }
+        queues.pop_undo_entry(self.entry.card_id());
+        col.save_undo(Box::new(QueueUpdateAfterAnsweringCard {
+            entry: self.entry,
+            learning_requeue: modified_learning,
+        }));
+
+        Ok(())
+    }
+}
+
+impl CardQueues {
+    pub(super) fn next_undo_entry(&self) -> Option<QueueEntry> {
+        self.undo.last().copied()
+    }
+
+    pub(super) fn pop_undo_entry(&mut self, id: CardID) -> Option<QueueEntry> {
+        if let Some(last) = self.undo.last() {
+            if last.card_id() == id {
+                match last.kind() {
+                    QueueEntryKind::New => self.counts.new -= 1,
+                    QueueEntryKind::Review => self.counts.review -= 1,
+                    QueueEntryKind::Learning => self.counts.learning -= 1,
+                }
+                return self.undo.pop();
+            }
+        }
+
+        None
+    }
+
+    /// Add an undone card back to the 'front' of the list, and update
+    /// the counts.
+    pub(super) fn push_undo_entry(&mut self, entry: QueueEntry) {
+        match entry.kind() {
+            QueueEntryKind::New => self.counts.new += 1,
+            QueueEntryKind::Review => self.counts.review += 1,
+            QueueEntryKind::Learning => self.counts.learning += 1,
+        }
+        self.undo.push(entry);
+    }
+}

rslib/src/sync/mod.rs

@@ -326,6 +326,7 @@ where
             SyncActionRequired::NoChanges => Ok(state.into()),
             SyncActionRequired::FullSyncRequired { .. } => Ok(state.into()),
             SyncActionRequired::NormalSyncRequired => {
+                self.col.state.undo.clear();
                 self.col.storage.begin_trx()?;
                 self.col
                     .unbury_if_day_rolled_over(self.col.timing_today()?)?;

rslib/src/sync/server.rs

@@ -102,6 +102,7 @@ impl SyncServer for LocalServer {
         self.client_usn = client_usn;
         self.client_is_newer = client_is_newer;
 
+        self.col.state.undo.clear();
         self.col.storage.begin_rust_trx()?;
 
         // make sure any pending cards have been unburied first if necessary

rslib/src/tags.rs

@@ -255,7 +255,7 @@ impl Collection {
         self.storage.register_tag(&tag)
     }
 
-    pub(crate) fn remove_single_tag(&mut self, tag: &Tag, _usn: Usn) -> Result<()> {
+    pub(crate) fn remove_single_tag(&mut self, tag: &Tag) -> Result<()> {
         self.save_undo(Box::new(RemovedTag(tag.clone())));
         self.storage.remove_single_tag(&tag.name)
     }
@@ -488,14 +488,13 @@ struct AddedTag(Tag);
 struct RemovedTag(Tag);
 
 impl Undo for AddedTag {
-    fn undo(self: Box<Self>, col: &mut Collection, usn: Usn) -> Result<()> {
+    fn undo(self: Box<Self>, col: &mut Collection) -> Result<()> {
-        col.remove_single_tag(&self.0, usn)
+        col.remove_single_tag(&self.0)
     }
 }
 
 impl Undo for RemovedTag {
-    fn undo(mut self: Box<Self>, col: &mut Collection, usn: Usn) -> Result<()> {
+    fn undo(self: Box<Self>, col: &mut Collection) -> Result<()> {
-        self.0.usn = usn;
         col.register_tag_inner(&self.0)
     }
 }

rslib/src/undo.rs

@@ -1,10 +1,11 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
+use crate::backend_proto as pb;
+use crate::i18n::TR;
 use crate::{
     collection::{Collection, CollectionOp},
     err::Result,
-    types::Usn,
 };
 use std::{collections::VecDeque, fmt};
 
@@ -12,7 +13,7 @@ const UNDO_LIMIT: usize = 30;
 
 pub(crate) trait Undo: fmt::Debug + Send {
     /// Undo the recorded action.
-    fn undo(self: Box<Self>, col: &mut Collection, usn: Usn) -> Result<()>;
+    fn undo(self: Box<Self>, col: &mut Collection) -> Result<()>;
 }
 
 #[derive(Debug)]
@@ -54,9 +55,7 @@ impl UndoManager {
 
     pub(crate) fn begin_step(&mut self, op: Option<CollectionOp>) {
         if op.is_none() {
-            // action doesn't support undoing; clear the queue
+            self.clear();
-            self.undo_steps.clear();
-            self.redo_steps.clear();
         } else if self.mode == UndoMode::NormalOp {
             // a normal op clears the redo queue
             self.redo_steps.clear();
@@ -67,6 +66,15 @@ impl UndoManager {
         });
     }
 
+    pub(crate) fn clear(&mut self) {
+        self.undo_steps.clear();
+        self.redo_steps.clear();
+    }
+
+    pub(crate) fn clear_redo(&mut self) {
+        self.redo_steps.clear();
+    }
+
     pub(crate) fn end_step(&mut self) {
         if let Some(step) = self.current_step.take() {
             if self.mode == UndoMode::Undoing {
@@ -105,9 +113,8 @@ impl Collection {
             let changes = step.changes;
             self.state.undo.mode = UndoMode::Undoing;
             let res = self.transact(Some(step.kind), |col| {
-                let usn = col.usn()?;
                 for change in changes.into_iter().rev() {
-                    change.undo(col, usn)?;
+                    change.undo(col)?;
                 }
                 Ok(())
             });
@@ -122,9 +129,8 @@ impl Collection {
             let changes = step.changes;
             self.state.undo.mode = UndoMode::Redoing;
             let res = self.transact(Some(step.kind), |col| {
-                let usn = col.usn()?;
                 for change in changes.into_iter().rev() {
-                    change.undo(col, usn)?;
+                    change.undo(col)?;
                 }
                 Ok(())
             });
@@ -138,6 +144,27 @@ impl Collection {
     pub(crate) fn save_undo(&mut self, item: Box<dyn Undo>) {
         self.state.undo.save(item)
     }
+
+    pub fn describe_collection_op(&self, op: CollectionOp) -> String {
+        match op {
+            CollectionOp::UpdateCard => todo!(),
+            CollectionOp::AnswerCard => self.i18n.tr(TR::UndoAnswerCard),
+        }
+        .to_string()
+    }
+
+    pub fn undo_status(&self) -> pb::UndoStatus {
+        pb::UndoStatus {
+            undo: self
+                .can_undo()
+                .map(|op| self.describe_collection_op(op))
+                .unwrap_or_default(),
+            redo: self
+                .can_redo()
+                .map(|op| self.describe_collection_op(op))
+                .unwrap_or_default(),
+        }
+    }
 }
 
 #[cfg(test)]