fix: deck picker subtitle ignores 9999 cap#21035
Conversation
The subtitle reads the deck tree protobuf root's new, review and learn count fields, each independently clamped at 9999 by the backend. As a result the collection wide total stops at 9999 (or 3 × 9999 in mixed queue collections) even when the user has many more cards due. The per deck 9999 clamp is intentional in the upstream backend, but the overall total at the top of the deck picker is not. Route that total through Scheduler.counts (which sources from getQueuedCards and is both limit aware and unclamped, same as StudyOptionsFragment) summed across top level decks. Side benefit: Custom Study limit extensions are now reflected in the subtitle. Fixes ankidroid#17605
There was a problem hiding this comment.
Hey, thanks for the PR!! This seems good quality, but it breaches our AI policy for new contributors:
https://github.com/ankidroid/Anki-Android/blob/main/AI_POLICY.md
emdash:
> * `getQueuedCards` and is both limit-aware and unclamped — the same sourceIt needs a test, and then a rewrite [assuming my comments are accurate and the uncapped values are correct], I'd happily merge after this is performed, great work!
| // (backend behavior, shared with Anki Desktop), so summing the root's three | ||
| // fields underreports for large collections — see issue #17605. Route through | ||
| // `allDecksCountsUncapped` (uncapped, limit-aware via getQueuedCards) instead. | ||
| val flowOfCardsDue = |
| combine(flowOfDeckDueTree, flowOfDeckListInInitialState) { tree, inInitialState -> | ||
| if (tree == null || inInitialState != false) return@combine null | ||
| tree.newCount + tree.revCount + tree.lrnCount | ||
| withCol { sched.allDecksCountsUncapped().count() } |
There was a problem hiding this comment.
com.ichi2.anki.libanki.sched.DeckNode's node property is a anki.decks.DeckTreeNode with uncapped values:
I would define accessor properties in com.ichi2.anki.libanki.sched.DeckNode to expose the uncapped counts, and use them to build the total count.
Most of the work here will be defining an intuitive naming pattern to distinguish uncapped counts from regular counts, as the uncapped counts do not include children, whereas the capped counts do
There was a problem hiding this comment.
I tried this. But the new_uncapped field counts all new cards in the deck, not just today's due, so the subtitle went from 39k to 254k on my collection. Should we be summing today's due (as limit aware) or the backlog?
I saw that in rslib's due_counts.sql the new line has no due filter (the review and learning lines do), so new_uncapped counts the full new pool while the other three fields are today's due.
There was a problem hiding this comment.
Behaviour is currently undefined. But I was incorrect in the general case
What do you think? I suspect we want to ignore the limits if the count is 9999, and apply them otherwise.
Regardless, some tests (and a regression test for using uncapped properties incorrectly) would move this forwards
david-allison
added
the
Needs Author Reply
Follow up to review feedback on ankidroid#21035. The proto DeckTreeNode already exposes per-deck uncapped fields (new_uncapped, review_uncapped, intraday_learning, interday_learning_uncapped). Surfacing them as accessor properties on DeckNode lets the total be computed as a pure tree walk over the already fetched data, with no backend calls and no mutating state. Changes: * DeckNode gains four accessor properties for the uncapped per-deck counts and a totalDueUncapped() method that sums them across the receiver and all descendants. * DeckPickerViewModel.flowOfCardsDue now calls tree.totalDueUncapped() directly, replacing the previous withCol path. * The Scheduler.allDecksCountsUncapped extension is removed as it is no longer needed. * Added three unit tests in DeckNodeTest covering a single leaf, a small subtree without double counting, and the case where the aggregated counts would saturate at 9999.
onurds
force-pushed
the
fix-17605-uncap-deck-picker-subtitle
branch
from
5d9f2df to
2768981
Compare
2 participants
Purpose / Description
The "X cards due" subtitle on the deck picker home screen is capped at 9999
even when the user has many more cards due. Per discussion on r/Anki,
the 9999 cap on individual decks seems to be an intentional architectural decision in
the backend (rslib). Large numbers are not useful for individual
decks, and the clamp simplifies storage and display. That part is fine.
However, the overall total at the top of the screen also gets clamped,
because it is computed by reading the same count fields off the root of the
deck tree protobuf. That looks accidental to me, i think it would be a better idea to show the total amount of due cards for better understanding. But I would understand if it is also an architectural decision.
Fixes
Approach
Scheduler.counts() reads from getQueuedCards, which returns limit aware
and unclamped counts. This is the same source
StudyOptionsFragmentuses toshow the correct count for individual decks after Custom Study. Added a small
Scheduler.allDecksCountsUncapped()extension that iterates top level decksand sums counts(), and switched
DeckPickerViewModel.flowOfCardsDueto useit instead of the clamped protobuf root. The 9999 cap on individual decks is preserved.
Side benefit: Custom Study limit extensions are now reflected in the subtitle.
How Has This Been Tested?
Screenshots
Checklist