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.
This commit is contained in:
Damien Elmes 2021-08-21 14:13:34 +10:00
parent 6977b6ab6f
commit b9402b5c47
6 changed files with 104 additions and 71 deletions

View file

@ -134,10 +134,14 @@ message DeckTreeNode {
uint32 level = 4; uint32 level = 4;
bool collapsed = 5; bool collapsed = 5;
// counts after limits applied
uint32 review_count = 6; uint32 review_count = 6;
uint32 learn_count = 7; uint32 learn_count = 7;
uint32 new_count = 8; uint32 new_count = 8;
// due counts without limits applied
uint32 intraday_learning_total = 9;
uint32 interday_learning_total = 10;
bool filtered = 16; bool filtered = 16;
// low index so key can be packed into a byte, but at bottom // low index so key can be packed into a byte, but at bottom

View file

@ -8,7 +8,11 @@ use crate::{backend_proto as pb, prelude::*};
pub(crate) struct DueCounts { pub(crate) struct DueCounts {
pub new: u32, pub new: u32,
pub review: u32, pub review: u32,
/// interday+intraday
pub learning: u32, pub learning: u32,
pub intraday_learning: u32,
pub interday_learning: u32,
} }
impl Deck { impl Deck {

View file

@ -4,6 +4,7 @@
use std::{ use std::{
collections::{HashMap, HashSet}, collections::{HashMap, HashSet},
iter::Peekable, iter::Peekable,
ops::AddAssign,
}; };
use serde_tuple::Serialize_tuple; use serde_tuple::Serialize_tuple;
@ -85,6 +86,8 @@ fn add_counts(node: &mut DeckTreeNode, counts: &HashMap<DeckId, DueCounts>) {
node.new_count = counts.new; node.new_count = counts.new;
node.review_count = counts.review; node.review_count = counts.review;
node.learn_count = counts.learning; 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 { for child in &mut node.children {
add_counts(child, counts); add_counts(child, counts);
@ -151,51 +154,75 @@ fn apply_limits_v2(
original_rev_count + child_rev_total original_rev_count + child_rev_total
} }
/// Add child counts, then limit to remaining limit. The v3 scheduler does not /// A temporary container used during count summation and limit application.
/// propagate limits down the tree. Limits for a deck affect only the amount #[derive(Default, Clone)]
/// that deck itself will gather. struct NodeCountsV3 {
/// The v3 scheduler also caps the new limit to the remaining review limit, new: u32,
/// so no new cards will be introduced when there is a backlog that exceeds review: u32,
/// the review limits. 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( fn apply_limits_v3(
node: &mut DeckTreeNode, node: &mut DeckTreeNode,
limits: &HashMap<DeckId, RemainingLimits>, limits: &HashMap<DeckId, RemainingLimits>,
) -> (u32, u32) { ) -> NodeCountsV3 {
let mut remaining = limits let remaining = limits
.get(&DeckId(node.deck_id)) .get(&DeckId(node.deck_id))
.copied() .copied()
.unwrap_or_default(); .unwrap_or_default();
// recurse into children, tallying their counts // cap current node's own cards
let mut child_new_total = 0; let this_node_uncapped = NodeCountsV3 {
let mut child_rev_total = 0; 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 { for child in &mut node.children {
let child_counts = apply_limits_v3(child, limits); individually_capped_total += 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;
} }
// new limits capped to review limits // We already have a tally of the current deck's capped cards+its child decks'
remaining.new = remaining.new.min( // capped cards, which we'll return to the parent. But because clicking on a
remaining // given deck imposes that deck's limits on the total number of cards shown,
.review // the tally we'll display needs to be capped again by the limits of the current
.saturating_sub(node.review_count) // deck.
.saturating_sub(child_rev_total), 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 individually_capped_total
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
} }
fn hide_default_deck(node: &mut DeckTreeNode) { fn hide_default_deck(node: &mut DeckTreeNode) {

View file

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

View file

@ -263,24 +263,13 @@ impl Collection {
queues.add_intraday_learning_card(card, bury) queues.add_intraday_learning_card(card, bury)
})?; })?;
// interday learning // interday learning, then reviews
self.storage.for_each_due_card_in_active_decks( let mut add_due_cards = |kind: DueCardKind| -> Result<()> {
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 { if selected_deck_limits.review != 0 {
self.storage.for_each_due_card_in_active_decks( self.storage.for_each_due_card_in_active_decks(
timing.days_elapsed, timing.days_elapsed,
sort_options.review_order, sort_options.review_order,
DueCardKind::Review, kind,
|card| { |card| {
if selected_deck_limits.review == 0 { if selected_deck_limits.review == 0 {
return false; return false;
@ -296,12 +285,18 @@ impl Collection {
}, },
)?; )?;
} }
Ok(())
};
add_due_cards(DueCardKind::Learning)?;
add_due_cards(DueCardKind::Review)?;
// New cards last // cap new cards to the remaining review limit
for limit in remaining.values_mut() { for limit in remaining.values_mut() {
limit.new = limit.new.min(limit.review).min(selected_deck_limits.review); 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); 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 can_exit_early = sort_options.new_gather_priority == NewCardGatherPriority::Deck;
let reverse = sort_options.new_gather_priority == NewCardGatherPriority::HighestPosition; let reverse = sort_options.new_gather_priority == NewCardGatherPriority::HighestPosition;
for deck in &decks { for deck in &decks {

View file

@ -42,15 +42,18 @@ fn row_to_due_counts(row: &Row) -> Result<(DeckId, DueCounts)> {
let deck_id = row.get(0)?; let deck_id = row.get(0)?;
let new = row.get(1)?; let new = row.get(1)?;
let review = row.get(2)?; let review = row.get(2)?;
let interday: u32 = row.get(3)?; let interday_learning: u32 = row.get(3)?;
let intraday: u32 = row.get(4)?; let intraday_learning: u32 = row.get(4)?;
let learning = intraday + interday; // used as-is in v1/v2; recalculated in v3 after limits are applied
let learning = intraday_learning + interday_learning;
Ok(( Ok((
deck_id, deck_id,
DueCounts { DueCounts {
new, new,
review, review,
learning, learning,
intraday_learning,
interday_learning,
}, },
)) ))
} }