Skip to content
This repository has been archived by the owner on Feb 7, 2023. It is now read-only.

fix incorrect nested review counts in v2 scheduler #1

Merged
merged 1 commit into from
Jun 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions pylib/tests/test_schedv2.py
Original file line number Diff line number Diff line change
Expand Up @@ -446,21 +446,21 @@ def test_review_limits():
tree = col.sched.deck_due_tree().children
# (('parent', 1514457677462, 5, 0, 0, (('child', 1514457677463, 5, 0, 0, ()),)))
assert tree[0].review_count == 5 # parent
assert tree[0].children[0].review_count == 5 # child
assert tree[0].children[0].review_count == 10 # child

# .counts() should match
col.decks.select(child["id"])
col.sched.reset()
assert col.sched.counts() == (0, 0, 5)
assert col.sched.counts() == (0, 0, 10)

# answering a card in the child should decrement parent count
c = col.sched.getCard()
col.sched.answerCard(c, 3)
assert col.sched.counts() == (0, 0, 4)
assert col.sched.counts() == (0, 0, 9)

tree = col.sched.deck_due_tree().children
assert tree[0].review_count == 4 # parent
assert tree[0].children[0].review_count == 4 # child
assert tree[0].children[0].review_count == 9 # child


def test_button_spacing():
Expand Down
63 changes: 56 additions & 7 deletions rslib/src/decks/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use super::{Deck, DeckKind, DueCounts};
use crate::{
backend_proto::DeckTreeNode,
collection::Collection,
config::SchedulerVersion,
deckconf::{DeckConf, DeckConfID},
decks::DeckID,
err::Result,
Expand Down Expand Up @@ -122,6 +123,44 @@ fn apply_limits(
node.review_count = (node.review_count + child_rev_total).min(remaining_rev);
}

/// Apply parent new limits to children, and add child counts to parents.
/// Unlike v1, reviews are not capped by their parents, and we return the
/// uncapped review amount to add to the parent. This is a bit of a hack, and
/// just tides us over until the v2 queue building code can be reworked.
/// Counts are (new, review).
fn apply_limits_v2(
node: &mut DeckTreeNode,
today: u32,
decks: &HashMap<DeckID, Deck>,
dconf: &HashMap<DeckConfID, DeckConf>,
parent_limits: (u32, u32),
) -> u32 {
let original_rev_count = node.review_count;

let (mut remaining_new, remaining_rev) =
remaining_counts_for_deck(DeckID(node.deck_id), today, decks, dconf);

// cap remaining to parent limits
remaining_new = remaining_new.min(parent_limits.0);

// apply our limit to children and tally their counts
let mut child_new_total = 0;
let mut child_rev_total = 0;
for child in &mut node.children {
child_rev_total +=
apply_limits_v2(child, today, decks, dconf, (remaining_new, remaining_rev));
child_new_total += child.new_count;
// no limit on learning cards
node.learn_count += child.learn_count;
}

// add child counts to our count, capped to remaining limit
node.new_count = (node.new_count + child_new_total).min(remaining_new);
node.review_count = (node.review_count + child_rev_total).min(remaining_rev);

original_rev_count + child_rev_total
}

fn remaining_counts_for_deck(
did: DeckID,
today: u32,
Expand Down Expand Up @@ -251,13 +290,23 @@ impl Collection {
.map(|d| (d.id, d))
.collect();
add_counts(&mut tree, &counts);
apply_limits(
&mut tree,
days_elapsed,
&decks_map,
&dconf,
(std::u32::MAX, std::u32::MAX),
);
if self.sched_ver() == SchedulerVersion::V2 {
apply_limits_v2(
&mut tree,
days_elapsed,
&decks_map,
&dconf,
(std::u32::MAX, std::u32::MAX),
);
} else {
apply_limits(
&mut tree,
days_elapsed,
&decks_map,
&dconf,
(std::u32::MAX, std::u32::MAX),
);
}
}

Ok(tree)
Expand Down