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

Commit

Permalink
fix incorrect nested review counts in v2 scheduler
Browse files Browse the repository at this point in the history
  • Loading branch information
dae authored and mikehardy committed Jun 11, 2022
1 parent 2296461 commit cae833d
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 11 deletions.
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

0 comments on commit cae833d

Please sign in to comment.