From b9402b5c4782cd401eb0141092a0256cb234dfff Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sat, 21 Aug 2021 14:13:34 +1000 Subject: [PATCH] support limiting interday learning cards by review limit again Context: https://forums.ankiweb.net/t/more-cards-today-question-about-v3/12400/10 Previously, interday learning cards and reviews were gathered at the same time in v3, with the review limit being applied to both of them. The order cards were gathered in would change the ratio of gathered learning cards and reviews, but as they were displayed together in a single count, a changing ratio was not apparent, and no special handling was required by the deck tree code. Showing interday learning cards in the learning count, while still applying a review limit to them, makes things more complicated, as a changing ratio will result in different counts. The deck tree code is not able to know which order cards will appear in, so without changes, we would have had a situation where the deck list may show different counts to those seen when clicking on a deck. One way to solve this would have been to introduce a separate limit for interday learning cards. But this would have meant users needed to juggle two different limits, instead of having a single one that controls total number of (non-intraday) cards shown. Instead, the scheduler now fetches interday cards prior to reviews - the rationale for that order is that learning cards tend to be more fragile/urgent than reviews. The option to show learning cards before/after/mixed with reviews still exists, but it applies only after cards have been capped to the daily limit. To ensure the deck tree code matches the counts the scheduler gives, it too applies limits to interday learning cards first, and reviews afterwards. --- proto/anki/decks.proto | 4 + rslib/src/decks/counts.rs | 4 + rslib/src/decks/tree.rs | 95 +++++++++++++++--------- rslib/src/scheduler/answering/mod.rs | 2 +- rslib/src/scheduler/queue/builder/mod.rs | 61 +++++++-------- rslib/src/storage/deck/mod.rs | 9 ++- 6 files changed, 104 insertions(+), 71 deletions(-) diff --git a/proto/anki/decks.proto b/proto/anki/decks.proto index 5e24ea1cc..250e15e4c 100644 --- a/proto/anki/decks.proto +++ b/proto/anki/decks.proto @@ -134,10 +134,14 @@ message DeckTreeNode { uint32 level = 4; bool collapsed = 5; + // counts after limits applied uint32 review_count = 6; uint32 learn_count = 7; uint32 new_count = 8; + // due counts without limits applied + uint32 intraday_learning_total = 9; + uint32 interday_learning_total = 10; bool filtered = 16; // low index so key can be packed into a byte, but at bottom diff --git a/rslib/src/decks/counts.rs b/rslib/src/decks/counts.rs index 9dd905222..db6ad7002 100644 --- a/rslib/src/decks/counts.rs +++ b/rslib/src/decks/counts.rs @@ -8,7 +8,11 @@ use crate::{backend_proto as pb, prelude::*}; pub(crate) struct DueCounts { pub new: u32, pub review: u32, + /// interday+intraday pub learning: u32, + + pub intraday_learning: u32, + pub interday_learning: u32, } impl Deck { diff --git a/rslib/src/decks/tree.rs b/rslib/src/decks/tree.rs index 433a6d988..c41043cf9 100644 --- a/rslib/src/decks/tree.rs +++ b/rslib/src/decks/tree.rs @@ -4,6 +4,7 @@ use std::{ collections::{HashMap, HashSet}, iter::Peekable, + ops::AddAssign, }; use serde_tuple::Serialize_tuple; @@ -85,6 +86,8 @@ fn add_counts(node: &mut DeckTreeNode, counts: &HashMap) { node.new_count = counts.new; node.review_count = counts.review; node.learn_count = counts.learning; + node.intraday_learning_total = counts.intraday_learning; + node.interday_learning_total = counts.interday_learning; } for child in &mut node.children { add_counts(child, counts); @@ -151,51 +154,75 @@ fn apply_limits_v2( original_rev_count + child_rev_total } -/// Add child counts, then limit to remaining limit. The v3 scheduler does not -/// propagate limits down the tree. Limits for a deck affect only the amount -/// that deck itself will gather. -/// The v3 scheduler also caps the new limit to the remaining review limit, -/// so no new cards will be introduced when there is a backlog that exceeds -/// the review limits. +/// A temporary container used during count summation and limit application. +#[derive(Default, Clone)] +struct NodeCountsV3 { + new: u32, + review: u32, + intraday_learning: u32, + interday_learning: u32, +} + +impl NodeCountsV3 { + fn capped(&self, remaining: &RemainingLimits) -> Self { + let mut capped = self.clone(); + // apply review limit to interday learning + capped.interday_learning = capped.interday_learning.min(remaining.review); + let mut remaining_reviews = remaining.review.saturating_sub(capped.interday_learning); + // any remaining review limit is applied to reviews + capped.review = capped.review.min(remaining_reviews); + remaining_reviews = remaining_reviews.saturating_sub(capped.review); + // new cards last, capped to new and remaining review limits + capped.new = capped.new.min(remaining_reviews).min(remaining.new); + capped + } +} +impl AddAssign for NodeCountsV3 { + fn add_assign(&mut self, rhs: Self) { + self.new += rhs.new; + self.review += rhs.review; + self.intraday_learning += rhs.intraday_learning; + self.interday_learning += rhs.interday_learning; + } +} + +/// Adjust new, review and learning counts based on the daily limits. +/// As part of this process, the separate interday and intraday learning +/// counts are combined after the limits have been applied. fn apply_limits_v3( node: &mut DeckTreeNode, limits: &HashMap, -) -> (u32, u32) { - let mut remaining = limits +) -> NodeCountsV3 { + let remaining = limits .get(&DeckId(node.deck_id)) .copied() .unwrap_or_default(); - // recurse into children, tallying their counts - let mut child_new_total = 0; - let mut child_rev_total = 0; + // cap current node's own cards + let this_node_uncapped = NodeCountsV3 { + new: node.new_count, + review: node.review_count, + intraday_learning: node.intraday_learning_total, + interday_learning: node.interday_learning_total, + }; + let mut individually_capped_total = this_node_uncapped.capped(&remaining); + // and add the capped values from child decks for child in &mut node.children { - let child_counts = apply_limits_v3(child, limits); - child_new_total += child_counts.0; - child_rev_total += child_counts.1; - // no limit on learning cards - node.learn_count += child.learn_count; + individually_capped_total += apply_limits_v3(child, limits); } - // new limits capped to review limits - remaining.new = remaining.new.min( - remaining - .review - .saturating_sub(node.review_count) - .saturating_sub(child_rev_total), - ); + // We already have a tally of the current deck's capped cards+its child decks' + // capped cards, which we'll return to the parent. But because clicking on a + // given deck imposes that deck's limits on the total number of cards shown, + // the tally we'll display needs to be capped again by the limits of the current + // deck. + let total_constrained_by_current_deck = individually_capped_total.capped(&remaining); + node.new_count = total_constrained_by_current_deck.new; + node.review_count = total_constrained_by_current_deck.review; + node.learn_count = total_constrained_by_current_deck.intraday_learning + + total_constrained_by_current_deck.interday_learning; - // parents want the child total without caps - let out = ( - node.new_count.min(remaining.new) + child_new_total, - node.review_count.min(remaining.review) + child_rev_total, - ); - - // but the current node needs to cap after adding children - node.new_count = (node.new_count + child_new_total).min(remaining.new); - node.review_count = (node.review_count + child_rev_total).min(remaining.review); - - out + individually_capped_total } fn hide_default_deck(node: &mut DeckTreeNode) { diff --git a/rslib/src/scheduler/answering/mod.rs b/rslib/src/scheduler/answering/mod.rs index a1d5016cb..3b255af15 100644 --- a/rslib/src/scheduler/answering/mod.rs +++ b/rslib/src/scheduler/answering/mod.rs @@ -313,7 +313,7 @@ impl Collection { let mut review_delta = 0; match from_queue { CardQueue::New => new_delta += 1, - CardQueue::Review => review_delta += 1, + CardQueue::Review | CardQueue::DayLearn => review_delta += 1, _ => {} } self.update_deck_stats( diff --git a/rslib/src/scheduler/queue/builder/mod.rs b/rslib/src/scheduler/queue/builder/mod.rs index 0b1192ab0..e97d44912 100644 --- a/rslib/src/scheduler/queue/builder/mod.rs +++ b/rslib/src/scheduler/queue/builder/mod.rs @@ -263,45 +263,40 @@ impl Collection { queues.add_intraday_learning_card(card, bury) })?; - // 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 - }, - )?; + // interday learning, then reviews + let mut add_due_cards = |kind: DueCardKind| -> Result<()> { + if selected_deck_limits.review != 0 { + self.storage.for_each_due_card_in_active_decks( + timing.days_elapsed, + sort_options.review_order, + kind, + |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(card, bury) { + selected_deck_limits.review -= 1; + limits.review -= 1; + } - // reviews - if selected_deck_limits.review != 0 { - self.storage.for_each_due_card_in_active_decks( - timing.days_elapsed, - sort_options.review_order, - 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(card, bury) { - selected_deck_limits.review -= 1; - limits.review -= 1; - } + true + }, + )?; + } + Ok(()) + }; + add_due_cards(DueCardKind::Learning)?; + add_due_cards(DueCardKind::Review)?; - true - }, - )?; - } - - // New cards last + // cap new cards to the remaining review limit for limit in remaining.values_mut() { limit.new = limit.new.min(limit.review).min(selected_deck_limits.review); } selected_deck_limits.new = selected_deck_limits.new.min(selected_deck_limits.review); + + // new cards last let can_exit_early = sort_options.new_gather_priority == NewCardGatherPriority::Deck; let reverse = sort_options.new_gather_priority == NewCardGatherPriority::HighestPosition; for deck in &decks { diff --git a/rslib/src/storage/deck/mod.rs b/rslib/src/storage/deck/mod.rs index 5ca7aa1a3..0818f4f8e 100644 --- a/rslib/src/storage/deck/mod.rs +++ b/rslib/src/storage/deck/mod.rs @@ -42,15 +42,18 @@ fn row_to_due_counts(row: &Row) -> Result<(DeckId, DueCounts)> { let deck_id = row.get(0)?; let new = row.get(1)?; let review = row.get(2)?; - let interday: u32 = row.get(3)?; - let intraday: u32 = row.get(4)?; - let learning = intraday + interday; + let interday_learning: u32 = row.get(3)?; + let intraday_learning: u32 = row.get(4)?; + // used as-is in v1/v2; recalculated in v3 after limits are applied + let learning = intraday_learning + interday_learning; Ok(( deck_id, DueCounts { new, review, learning, + intraday_learning, + interday_learning, }, )) }