Disable burying of previously gathered cards (#2361)

* Enforce hierarchical bury modes

Interday learning burying is only allowed if review burying is enabled
and review burying is only allowed if new burying is enabled.
Closes #2352.

* Switch front end to new bury modes

* Wording tweaks (dae)

* Hide interday option if using v2 scheduler (dae)

ftl/core/deck-config.ftl

@@ -109,6 +109,23 @@ deck-config-bury-interday-learning-tooltip =
     Whether other `learning` cards of the same note with intervals > 1 day
     will be delayed until the next day.
 
+deck-config-bury-siblings = Bury siblings
+deck-config-do-not-bury = Do not bury siblings
+deck-config-bury-if-new = Bury if new
+deck-config-bury-if-new-or-review = Bury if new or review
+deck-config-bury-if-new-review-or-interday = Bury if new, review, or interday learning
+deck-config-bury-tooltip =
+    Siblings are other cards from the same note (eg forward/reverse cards, or
+    other cloze deletions from the same text).
+    
+    When this option is off, multiple cards from the same note may be seen on the same
+    day. When enabled, Anki will automatically *bury* siblings, hiding them until the next
+    day. This option allows you to choose which kinds of cards may be buried when you answer
+    one of their siblings.
+
+    When using the V3 scheduler, interday learning cards can also be buried. Interday
+    learning cards are cards with a current learning step of one or more days.
+
 ## Ordering section
 
 deck-config-ordering-title = Display Order

proto/anki/deckconfig.proto

@@ -124,8 +124,13 @@ message DeckConfig {
     bool show_timer = 25;
     bool skip_question_when_replaying_answer = 26;
 
+    // the new scheduler doesn't allow toggling these booleans freely anymore,
+    // but they are continued to be used for reasons of backwards compatibility
+
     bool bury_new = 27;
+    // only respected if bury_new
     bool bury_reviews = 28;
+    // only respected if bury_new and bury_reviews
     bool bury_interday_learning = 29;
 
     bytes other = 255;

rslib/src/scheduler/answering/mod.rs

@@ -12,6 +12,7 @@ use rand::prelude::*;
 use rand::rngs::StdRng;
 use revlog::RevlogEntryPartial;
 
+use super::queue::BuryMode;
 use super::states::steps::LearningSteps;
 use super::states::CardState;
 use super::states::FilteredState;
@@ -285,16 +286,10 @@ impl Collection {
     }
 
     fn maybe_bury_siblings(&mut self, card: &Card, config: &DeckConfig) -> Result<()> {
-        if config.inner.bury_new || config.inner.bury_reviews {
+        let bury_mode = BuryMode::from_deck_config(config);
-            self.bury_siblings(
+        if bury_mode.any_burying() {
-                card.id,
+            self.bury_siblings(card.id, card.note_id, bury_mode)?;
-                card.note_id,
-                config.inner.bury_new,
-                config.inner.bury_reviews,
-                config.inner.bury_interday_learning,
-            )?;
         }
-
         Ok(())
     }
 

rslib/src/scheduler/bury_and_suspend.rs

@@ -1,6 +1,7 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
+use super::queue::BuryMode;
 use super::timing::SchedTimingToday;
 use crate::card::CardQueue;
 use crate::config::SchedulerVersion;
@@ -140,17 +141,9 @@ impl Collection {
         &mut self,
         cid: CardId,
         nid: NoteId,
-        include_new: bool,
+        bury_mode: BuryMode,
-        include_reviews: bool,
-        include_day_learn: bool,
     ) -> Result<usize> {
-        let cards = self.storage.all_siblings_for_bury(
+        let cards = self.storage.all_siblings_for_bury(cid, nid, bury_mode)?;
-            cid,
-            nid,
-            include_new,
-            include_reviews,
-            include_day_learn,
-        )?;
         self.bury_or_suspend_cards_inner(cards, BuryOrSuspendMode::BurySched)
     }
 }

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

@@ -1,11 +1,12 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
-use super::BuryMode;
+use super::super::BuryMode;
 use super::Context;
 use super::DueCard;
 use super::NewCard;
 use super::QueueBuilder;
+use crate::deckconfig::DeckConfig;
 use crate::prelude::*;
 
 pub(super) enum DueOrNewCard {
@@ -47,15 +48,31 @@ impl Context {
             .get(&deck_id)
             .and_then(|deck| deck.config_id())
             .and_then(|config_id| self.config_map.get(&config_id))
-            .map(|config| BuryMode {
+            .map(BuryMode::from_deck_config)
-                bury_new: config.inner.bury_new,
-                bury_reviews: config.inner.bury_reviews,
-                bury_interday_learning: config.inner.bury_interday_learning,
-            })
             .unwrap_or_default()
     }
 }
 
+impl BuryMode {
+    pub(crate) fn from_deck_config(config: &DeckConfig) -> BuryMode {
+        let cfg = &config.inner;
+        // Since cards are gathered in a certain order (learning > review > new) and
+        // a card can only bury siblings that are gathered later, only the four bury
+        // modes following this order are allowed.
+        // Booleans are continued to be used for reasons of backwards compatibility.
+        // https://github.com/ankitects/anki/issues/2352
+        BuryMode {
+            bury_new: cfg.bury_new,
+            bury_reviews: cfg.bury_new && cfg.bury_reviews,
+            bury_interday_learning: cfg.bury_new && cfg.bury_reviews && cfg.bury_interday_learning,
+        }
+    }
+
+    pub(crate) fn any_burying(self) -> bool {
+        self.bury_interday_learning || self.bury_reviews || self.bury_new
+    }
+}
+
 impl QueueBuilder {
     /// If burying is enabled in `new_settings`, existing entry will be updated.
     /// Returns a copy made before changing the entry, so that a card with

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

@@ -13,6 +13,7 @@ use std::collections::VecDeque;
 use intersperser::Intersperser;
 use sized_chain::SizedChain;
 
+use super::BuryMode;
 use super::CardQueues;
 use super::Counts;
 use super::LearningQueueEntry;
@@ -89,15 +90,6 @@ impl From<DueCard> for LearningQueueEntry {
     }
 }
 
-/// When we encounter a card with new or review burying enabled, all future
-/// siblings need to be buried, regardless of their own settings.
-#[derive(Default, Debug, Clone, Copy)]
-pub(super) struct BuryMode {
-    bury_new: bool,
-    bury_reviews: bool,
-    bury_interday_learning: bool,
-}
-
 #[derive(Default, Clone, Debug)]
 pub(super) struct QueueSortOptions {
     pub(super) new_order: NewCardSortOrder,
@@ -459,4 +451,35 @@ mod test {
 
         Ok(())
     }
+
+    impl Collection {
+        fn card_queue_len(&mut self) -> usize {
+            self.get_queued_cards(5, false).unwrap().cards.len()
+        }
+    }
+
+    #[test]
+    fn new_card_potentially_burying_review_card() {
+        let mut col = open_test_collection();
+        // add one new and one review card
+        col.add_new_note_with_fields("basic (and reversed card)", &["foo", "bar"]);
+        let card = col.get_first_card();
+        col.set_due_date(&[card.id], "0", None).unwrap();
+        // Potentially problematic config: New cards are shown first and would bury
+        // review siblings. This poses a problem because we gather review cards first.
+        col.update_default_deck_config(|config| {
+            config.new_mix = ReviewMix::BeforeReviews as i32;
+            config.bury_new = false;
+            config.bury_reviews = true;
+        });
+
+        let old_queue_len = col.card_queue_len();
+        col.answer_easy();
+        col.clear_study_queues();
+
+        // The number of cards in the queue must decrease by exactly 1, either because
+        // no burying was performed, or the first built queue anticipated it and didn't
+        // include the buried card.
+        assert_eq!(col.card_queue_len(), old_queue_len - 1);
+    }
 }

rslib/src/scheduler/queue/mod.rs

@@ -66,6 +66,15 @@ pub struct QueuedCards {
     pub review_count: usize,
 }
 
+/// When we encounter a card with new or review burying enabled, all future
+/// siblings need to be buried, regardless of their own settings.
+#[derive(Default, Debug, Clone, Copy)]
+pub(crate) struct BuryMode {
+    pub(crate) bury_new: bool,
+    pub(crate) bury_reviews: bool,
+    pub(crate) bury_interday_learning: bool,
+}
+
 impl Collection {
     pub fn get_next_card(&mut self) -> Result<Option<QueuedCard>> {
         self.get_queued_cards(1, false)

rslib/src/storage/card/mod.rs

@@ -31,6 +31,7 @@ use crate::decks::DeckKind;
 use crate::error::Result;
 use crate::notes::NoteId;
 use crate::scheduler::congrats::CongratsInfo;
+use crate::scheduler::queue::BuryMode;
 use crate::scheduler::queue::DueCard;
 use crate::scheduler::queue::DueCardKind;
 use crate::scheduler::queue::NewCard;
@@ -471,16 +472,14 @@ impl super::SqliteStorage {
         &self,
         cid: CardId,
         nid: NoteId,
-        include_new: bool,
+        bury_mode: BuryMode,
-        include_reviews: bool,
-        include_day_learn: bool,
     ) -> Result<Vec<Card>> {
         let params = named_params! {
             ":card_id": cid,
             ":note_id": nid,
-            ":include_new": include_new,
+            ":include_new": bury_mode.bury_new,
-            ":include_reviews": include_reviews,
+            ":include_reviews": bury_mode.bury_reviews,
-            ":include_day_learn": include_day_learn,
+            ":include_day_learn": bury_mode.bury_interday_learning      ,
             ":new_queue": CardQueue::New as i8,
             ":review_queue": CardQueue::Review as i8,
             ":daylearn_queue": CardQueue::DayLearn as i8,

rslib/src/tests.rs

@@ -8,6 +8,7 @@ use tempfile::TempDir;
 
 use crate::collection::open_test_collection;
 use crate::collection::CollectionBuilder;
+use crate::deckconfig::DeckConfigInner;
 use crate::deckconfig::UpdateDeckConfigsRequest;
 use crate::io::create_dir;
 use crate::media::MediaManager;
@@ -88,23 +89,23 @@ impl Collection {
         self.storage.get_all_cards().pop().unwrap()
     }
 
-    pub(crate) fn get_first_deck_config(&mut self) -> DeckConfig {
-        self.storage.all_deck_config().unwrap().pop().unwrap()
-    }
-
     pub(crate) fn set_default_learn_steps(&mut self, steps: Vec<f32>) {
-        let mut config = self.get_first_deck_config();
+        self.update_default_deck_config(|config| config.learn_steps = steps);
-        config.inner.learn_steps = steps;
-        self.update_default_deck_config(config);
     }
 
     pub(crate) fn set_default_relearn_steps(&mut self, steps: Vec<f32>) {
-        let mut config = self.get_first_deck_config();
+        self.update_default_deck_config(|config| config.relearn_steps = steps);
-        config.inner.relearn_steps = steps;
-        self.update_default_deck_config(config);
     }
 
-    pub(crate) fn update_default_deck_config(&mut self, config: DeckConfig) {
+    pub(crate) fn update_default_deck_config(
+        &mut self,
+        modifier: impl FnOnce(&mut DeckConfigInner),
+    ) {
+        let mut config = self
+            .get_deck_config(DeckConfigId(1), false)
+            .unwrap()
+            .unwrap();
+        modifier(&mut config.inner);
         self.update_deck_configs(UpdateDeckConfigsRequest {
             target_deck_id: DeckId(1),
             configs: vec![config],

ts/deck-options/BuryOptions.svelte

@@ -3,6 +3,7 @@ Copyright: Ankitects Pty Ltd and contributors
 License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 -->
 <script lang="ts">
+    import type { anki } from "@tslib/backend_proto";
     import * as tr from "@tslib/ftl";
     import type Carousel from "bootstrap/js/dist/carousel";
     import type Modal from "bootstrap/js/dist/modal";
@@ -10,10 +11,10 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     import DynamicallySlottable from "../components/DynamicallySlottable.svelte";
     import Item from "../components/Item.svelte";
     import TitledContainer from "../components/TitledContainer.svelte";
+    import EnumSelectorRow from "./EnumSelectorRow.svelte";
     import HelpModal from "./HelpModal.svelte";
     import type { DeckOptionsState } from "./lib";
     import SettingTitle from "./SettingTitle.svelte";
-    import SwitchRow from "./SwitchRow.svelte";
     import type { DeckOption } from "./types";
 
     export let state: DeckOptionsState;
@@ -22,29 +23,54 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     const config = state.currentConfig;
     const defaults = state.defaults;
 
-    const settings = {
+    const burySiblings: DeckOption = {
-        buryNewSiblings: {
+        title: tr.deckConfigBurySiblings(),
-            title: tr.deckConfigBuryNewSiblings(),
+        help: tr.deckConfigBuryTooltip(),
-            help: tr.deckConfigBuryNewTooltip(),
-        },
-        buryReviewSiblings: {
-            title: tr.deckConfigBuryReviewSiblings(),
-            help: tr.deckConfigBuryReviewTooltip(),
-        },
-        buryInterdayLearningSiblings: {
-            title: tr.deckConfigBuryInterdayLearningSiblings(),
-            help: tr.deckConfigBuryInterdayLearningTooltip(),
-        },
     };
-    const helpSections = Object.values(settings) as DeckOption[];
 
     let modal: Modal;
     let carousel: Carousel;
 
-    function openHelpModal(index: number): void {
+    function openHelpModal(): void {
         modal.show();
-        carousel.to(index);
+        carousel.to(0);
     }
+
+    const enum BuryMode {
+        NONE,
+        NEW,
+        NEW_REVIEW,
+        NEW_REVIEW_LEARNING,
+    }
+
+    const buryModeChoices = [
+        tr.deckConfigDoNotBury(),
+        tr.deckConfigBuryIfNew(),
+        tr.deckConfigBuryIfNewOrReview(),
+    ];
+    if (state.v3Scheduler) {
+        buryModeChoices.push(tr.deckConfigBuryIfNewReviewOrInterday());
+    }
+
+    function buryModeFromConfig(config: anki.deckconfig.DeckConfig.Config): BuryMode {
+        // burying review cards is only allowed if also burying new cards
+        const buryReviews = config.buryNew && config.buryReviews;
+        // burying learning cards is only allowed if also burying review and new cards
+        const buryInterdayLearning =
+            buryReviews && config.buryInterdayLearning && state.v3Scheduler;
+        return (
+            Number(config.buryNew) + Number(buryReviews) + Number(buryInterdayLearning)
+        );
+    }
+
+    function buryModeToConfig(mode: BuryMode) {
+        $config.buryNew = mode >= 1;
+        $config.buryReviews = mode >= 2;
+        $config.buryInterdayLearning = mode >= 3;
+    }
+
+    let mode = buryModeFromConfig($config);
+    $: buryModeToConfig(mode);
 </script>
 
 <TitledContainer title={tr.deckConfigBuryTitle()}>
@@ -52,7 +78,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
         title={tr.deckConfigBuryTitle()}
         url="https://docs.ankiweb.net/studying.html#siblings-and-burying"
         slot="tooltip"
-        {helpSections}
+        helpSections={[burySiblings]}
         on:mount={(e) => {
             modal = e.detail.modal;
             carousel = e.detail.carousel;
@@ -60,46 +86,15 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     />
     <DynamicallySlottable slotHost={Item} {api}>
         <Item>
-            <SwitchRow bind:value={$config.buryNew} defaultValue={defaults.buryNew}>
+            <EnumSelectorRow
-                <SettingTitle
+                bind:value={mode}
-                    on:click={() =>
+                defaultValue={buryModeFromConfig(defaults)}
-                        openHelpModal(Object.keys(settings).indexOf("buryNewSiblings"))}
+                choices={buryModeChoices}
-                    >{settings.buryNewSiblings.title}</SettingTitle
-                >
-            </SwitchRow>
-        </Item>
-
-        <Item>
-            <SwitchRow
-                bind:value={$config.buryReviews}
-                defaultValue={defaults.buryReviews}
             >
-                <SettingTitle
+                <SettingTitle on:click={() => openHelpModal()}
-                    on:click={() =>
+                    >{tr.deckConfigBurySiblings()}</SettingTitle
-                        openHelpModal(
-                            Object.keys(settings).indexOf("buryReviewSiblings"),
-                        )}>{settings.buryReviewSiblings.title}</SettingTitle
                 >
-            </SwitchRow>
+            </EnumSelectorRow>
         </Item>
-
-        {#if state.v3Scheduler}
-            <Item>
-                <SwitchRow
-                    bind:value={$config.buryInterdayLearning}
-                    defaultValue={defaults.buryInterdayLearning}
-                >
-                    <SettingTitle
-                        on:click={() =>
-                            openHelpModal(
-                                Object.keys(settings).indexOf(
-                                    "buryInterdayLearningSiblings",
-                                ),
-                            )}
-                        >{settings.buryInterdayLearningSiblings.title}</SettingTitle
-                    >
-                </SwitchRow>
-            </Item>
-        {/if}
     </DynamicallySlottable>
 </TitledContainer>