don't fetch reviews in deck order

- fetch reviews from all child decks at once, sorted by due order
- shuffle the gathered cards as we did previously
- review limits on child decks are ignored - only the current deck and
its parents control what the limit is
- to make the deck list consistent with actual counts, we can't sum the
child counts, as the sum in the parent limit>child limit case may not
reflect the actual number of cards that would be presented

anki/sched.py

@@ -238,9 +238,11 @@ order by due""" % self._deckLimit(),
             # learning
             lrn = self._lrnForDeck(deck['id'])
             # reviews
-            rlim = self._deckRevLimitSingle(deck)
             if p:
-                rlim = min(rlim, lims[p][1])
+                plim = lims[p][1]
+            else:
+                plim = None
+            rlim = self._deckRevLimitSingle(deck, parentLimit=plim)
             rev = self._revForDeck(deck['id'], rlim)
             # save to list
             data.append([deck['name'], deck['id'], rev, lrn, new])
@@ -286,14 +288,12 @@ order by due""" % self._deckLimit(),
             children = self._groupChildrenMain(children)
             # tally up children counts
             for ch in children:
-                rev += ch[2]
                 lrn += ch[3]
                 new += ch[4]
             # limit the counts to the deck's limits
             conf = self.col.decks.confForDid(did)
             deck = self.col.decks.get(did)
             if not conf['dyn']:
-                rev = max(0, min(rev, conf['rev']['perDay']-deck['revToday'][1]))
                 new = max(0, min(new, conf['new']['perDay']-deck['newToday'][1]))
             tree.append((head, did, rev, lrn, new, children))
         return tuple(tree)
@@ -712,68 +712,78 @@ and due <= ? limit ?)""",
     # Reviews
     ##########################################################################
 
-    def _deckRevLimit(self, did):
+    def _currentRevLimit(self):
-        return self._deckNewLimit(did, self._deckRevLimitSingle)
+        d = self.col.decks.get(self.col.decks.selected(), default=False)
+        return self._deckRevLimitSingle(d)
+
+    def _deckRevLimitSingle(self, d, parentLimit=None):
+        # invalid deck selected?
+        if not d:
+            return 0
 
-    def _deckRevLimitSingle(self, d):
         if d['dyn']:
             return self.reportLimit
+
         c = self.col.decks.confForDid(d['id'])
-        return max(0, c['rev']['perDay'] - d['revToday'][1])
+        lim = max(0, c['rev']['perDay'] - d['revToday'][1])
+
+        if parentLimit is not None:
+            return min(parentLimit, lim)
+        elif '::' not in d['name']:
+            return lim
+        else:
+            for parent in self.col.decks.parents(d['id']):
+                # pass in dummy parentLimit so we don't do parent lookup again
+                lim = min(lim, self._deckRevLimitSingle(parent, parentLimit=lim))
+            return lim
 
     def _revForDeck(self, did, lim):
+        dids = [did] + [x[1] for x in self.col.decks.children(did)]
         lim = min(lim, self.reportLimit)
         return self.col.db.scalar(
             """
 select count() from
-(select 1 from cards where did = ? and queue = 2
+(select 1 from cards where did in %s and queue = 2
-and due <= ? limit ?)""",
+and due <= ? limit ?)""" % ids2str(dids),
-            did, self.today, lim)
+            self.today, lim)
 
     def _resetRevCount(self):
-        def cntFn(did, lim):
+        lim = self._currentRevLimit()
-            return self.col.db.scalar("""
+        self.revCount = self.col.db.scalar("""
 select count() from (select id from cards where
-did = ? and queue = 2 and due <= ? limit %d)""" % lim,
+did in %s and queue = 2 and due <= ? limit %d)""" % (
-                                      did, self.today)
+            ids2str(self.col.decks.active()), lim), self.today)
-        self.revCount = self._walkingCount(
-            self._deckRevLimitSingle, cntFn)
 
     def _resetRev(self):
         self._resetRevCount()
         self._revQueue = []
-        self._revDids = self.col.decks.active()[:]
 
     def _fillRev(self):
         if self._revQueue:
             return True
         if not self.revCount:
             return False
-        while self._revDids:
+
-            did = self._revDids[0]
+        lim = min(self.queueLimit, self._currentRevLimit())
-            lim = min(self.queueLimit, self._deckRevLimit(did))
         if lim:
-                # fill the queue with the current did
             self._revQueue = self.col.db.list("""
 select id from cards where
-did = ? and queue = 2 and due <= ? limit ?""",
+did in %s and queue = 2 and due <= ?
-                                                  did, self.today, lim)
+order by due
+limit ?""" % (ids2str(self.col.decks.active())),
+                    self.today, lim)
+
             if self._revQueue:
-                    # ordering
+                if self.col.decks.get(self.col.decks.selected(), default=False)['dyn']:
-                    if self.col.decks.get(did)['dyn']:
                     # dynamic decks need due order preserved
                     self._revQueue.reverse()
                 else:
-                        # random order for regular reviews
+                    # fixme: as soon as a card is answered, this is no longer consistent
                     r = random.Random()
                     r.seed(self.today)
                     r.shuffle(self._revQueue)
-                    # is the current did empty?
-                    if len(self._revQueue) < lim:
-                        self._revDids.pop(0)
                 return True
-            # nothing left in the deck; move to next
+
-            self._revDids.pop(0)
         if self.revCount:
             # if we didn't get a card but the count is non-zero,
             # we need to check again for any cards that were

tests/test_sched.py

@@ -345,6 +345,68 @@ def test_reviews():
     c.load()
     assert c.queue == -1
 
+def test_review_limits():
+    d = getEmptyCol()
+
+    parent = d.decks.get(d.decks.id("parent"))
+    child = d.decks.get(d.decks.id("parent::child"))
+
+    pconf = d.decks.getConf(d.decks.confId("parentConf"))
+    cconf = d.decks.getConf(d.decks.confId("childConf"))
+
+    pconf['rev']['perDay'] = 5
+    d.decks.updateConf(pconf)
+    d.decks.setConf(parent, pconf['id'])
+    cconf['rev']['perDay'] = 10
+    d.decks.updateConf(cconf)
+    d.decks.setConf(child, cconf['id'])
+
+    m = d.models.current()
+    m['did'] = child['id']
+    d.models.save(m)
+
+    # add some cards
+    for i in range(20):
+        f = d.newNote()
+        f['Front'] = "one"; f['Back'] = "two"
+        d.addNote(f)
+
+        # make them reviews
+        c = f.cards()[0]
+        c.queue = c.type = 2
+        c.due = 0
+        c.flush()
+
+    tree = d.sched.deckDueTree()
+    # (('Default', 1, 0, 0, 0, ()), ('parent', 1514457677462, 5, 0, 0, (('child', 1514457677463, 5, 0, 0, ()),)))
+    assert tree[1][2] == 5 # parent
+    assert tree[1][5][0][2] == 5 # child
+
+    # .counts() should match
+    d.decks.select(child['id'])
+    d.sched.reset()
+    assert d.sched.counts() == (0, 0, 5)
+
+    # answering a card in the child should decrement parent count
+    c = d.sched.getCard()
+    d.sched.answerCard(c, 3)
+    assert d.sched.counts() == (0, 0, 4)
+
+    tree = d.sched.deckDueTree()
+    assert tree[1][2] == 4 # parent
+    assert tree[1][5][0][2] == 4 # child
+
+    # switch limits
+    d.decks.setConf(parent, cconf['id'])
+    d.decks.setConf(child, pconf['id'])
+    d.decks.select(parent['id'])
+    d.sched.reset()
+
+    # child limits do not affect the parent
+    tree = d.sched.deckDueTree()
+    assert tree[1][2] == 9 # parent
+    assert tree[1][5][0][2] == 4 # child
+
 def test_button_spacing():
     d = getEmptyCol()
     f = d.newNote()
@@ -854,7 +916,7 @@ def test_deckDue():
     d.reset()
     assert len(d.decks.decks) == 5
     cnts = d.sched.deckDueList()
-    assert cnts[0] == ["Default", 1, 0, 0, 1]
+    assert cnts[0] == ["Default", 1, 1, 0, 1]
     assert cnts[1] == ["Default::1", default1, 1, 0, 0]
     assert cnts[2] == ["foo", d.decks.id("foo"), 0, 0, 0]
     assert cnts[3] == ["foo::bar", foobar, 0, 0, 1]