Skip to content

refactor+perf(study-screen): custom scheduler #19950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mikehardy merged 3 commits into from
Jan 6, 2026
Merged

refactor+perf(study-screen): custom scheduler #19950

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f457ba5
refactor: extract util
BrayanDSO Dec 4, 2025
f2f5962
refactor: load custom scheduler with Col config
BrayanDSO Dec 4, 2025
c6bab3b
perf(study-screen): use await instead of while loop
BrayanDSO Dec 27, 2025
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: 1 addition & 7 deletions AnkiDroid/src/androidTest/java/com/ichi2/anki/ReviewerFragmentTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import androidx.test.espresso.matcher.ViewMatchers.withId
import androidx.test.ext.junit.rules.ActivityScenarioRule
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.google.android.material.textfield.TextInputEditText
import com.ichi2.anki.libanki.Collection
import com.ichi2.anki.preferences.sharedPrefs
import com.ichi2.anki.tests.InstrumentedTest
import com.ichi2.anki.tests.checkWithTimeout
Expand All @@ -34,6 +33,7 @@ import com.ichi2.anki.testutil.closeGetStartedScreenIfExists
import com.ichi2.anki.testutil.grantPermissions
import com.ichi2.anki.testutil.notificationPermission
import com.ichi2.anki.testutil.reviewDeckWithName
import com.ichi2.anki.utils.ext.cardStateCustomizer
import com.ichi2.testutils.common.Flaky
import com.ichi2.testutils.common.OS
import org.hamcrest.MatcherAssert.assertThat
Expand Down Expand Up @@ -209,9 +209,3 @@ class ReviewerFragmentTest : InstrumentedTest() {
}
}
}

private var Collection.cardStateCustomizer: String?
get() = config.get("cardStateCustomizer")
set(value) {
config.set("cardStateCustomizer", value)
}
8 changes: 1 addition & 7 deletions AnkiDroid/src/androidTest/java/com/ichi2/anki/ReviewerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import androidx.test.espresso.matcher.ViewMatchers.withResourceName
import androidx.test.espresso.matcher.ViewMatchers.withText
import androidx.test.ext.junit.rules.ActivityScenarioRule
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.ichi2.anki.libanki.Collection
import com.ichi2.anki.preferences.sharedPrefs
import com.ichi2.anki.tests.InstrumentedTest
import com.ichi2.anki.tests.checkWithTimeout
Expand All @@ -40,6 +39,7 @@ import com.ichi2.anki.testutil.closeBackupCollectionDialogIfExists
import com.ichi2.anki.testutil.closeGetStartedScreenIfExists
import com.ichi2.anki.testutil.grantPermissions
import com.ichi2.anki.testutil.notificationPermission
import com.ichi2.anki.utils.ext.cardStateCustomizer
import com.ichi2.testutils.common.Flaky
import com.ichi2.testutils.common.OS
import org.hamcrest.MatcherAssert.assertThat
Expand Down Expand Up @@ -228,9 +228,3 @@ class ReviewerTest : InstrumentedTest() {
}
}
}

private var Collection.cardStateCustomizer: String?
get() = config.get("cardStateCustomizer")
set(value) {
config.set("cardStateCustomizer", value)
}
30 changes: 14 additions & 16 deletions AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import com.ichi2.anki.ui.windows.reviewer.autoadvance.AutoAdvance
import com.ichi2.anki.utils.CollectionPreferences
import com.ichi2.anki.utils.Destination
import com.ichi2.anki.utils.ext.answerCard
import com.ichi2.anki.utils.ext.cardStateCustomizer
import com.ichi2.anki.utils.ext.cardStatsNoCardClean
import com.ichi2.anki.utils.ext.currentCardStudy
import com.ichi2.anki.utils.ext.flag
Expand All @@ -72,7 +73,6 @@ import com.ichi2.anki.utils.ext.previousCardStudy
import com.ichi2.anki.utils.ext.setUserFlagForCards
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.Deferred
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.withTimeoutOrNull
Expand Down Expand Up @@ -120,6 +120,7 @@ class ReviewerViewModel(

override val server: AnkiServer = AnkiServer(this, repository.getServerPort()).also { it.start() }
private val stateMutationKey = TimeManager.time.intTimeMS().toString()
private val stateMutationJs: Deferred<String> = asyncIO { withCol { cardStateCustomizer } }
private var typedAnswer = ""

private val autoAdvance = AutoAdvance(this)
Expand All @@ -130,16 +131,14 @@ class ReviewerViewModel(
* A flag that determines if the SchedulingStates in CurrentQueueState are
* safe to persist in the database when answering a card. This is used to
* ensure that the custom JS scheduler has persisted its SchedulingStates
* back to the Reviewer before we save it to the database. If the custom
* scheduler has not been configured, then it is safe to immediately set
* this to true.
* back to the Reviewer before we save it to the database.
*
* This flag should be set to false when we show the front of the card
* and only set to true once we know the custom scheduler has finished its
* execution, or set to true immediately if the custom scheduler has not
* This flag should be reset when we show the front of the card
* and only complete once we know the custom scheduler has finished its
* execution, or complete immediately if the custom scheduler has not
* been configured.
*/
private var statesMutated = true
private var mutationSignal = CompletableDeferred(Unit)

val answerButtonsNextTimeFlow: MutableStateFlow<AnswerButtonsNextTime?> = MutableStateFlow(null)
private val shouldShowNextTimes: Deferred<Boolean> =
Expand Down Expand Up @@ -193,9 +192,7 @@ class ReviewerViewModel(
fun onShowAnswer() {
Timber.v("ReviewerViewModel::onShowAnswer")
launchCatchingIO {
while (!statesMutated) {
delay(50)
}
mutationSignal.await()
Copy link
Member

@david-allison david-allison Dec 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely off-topic & could be moved to another issue.

I'm concerned there's no cancellation support.

What happens when a card is flipped, but this hasn't run yet? What if there's an error, or the code is slow?

Copy link
Member Author

@BrayanDSO BrayanDSO Dec 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionally, the only difference in comparison with the previous approach is how it waits for the signal, i.e. while...delay(50) vs CompletableDeferred.await().

Should it have some kind of timeout instead of waiting indefinitely? Perhaps.

Is the custom scheduler logic convoluted? Yes. And that is a consequence of how Anki desktop is architectured. I'm mostly following how the desktop version make the reviewer work, even in a barely used setting like this one.

ReviewerFragmentTest tests if the custom scheduler is working, so that is covered.

Making the logic safer to future refactors need some integration tests, and I'm still far from them.

So, create another issue if you want. Or simply wait until this turns into a problem (or not).


val typedAnswerResult = CompletableDeferred<String>()
if (typeAnswerFlow.value != null) {
Expand Down Expand Up @@ -250,7 +247,7 @@ class ReviewerViewModel(
}

fun onStateMutationCallback() {
statesMutated = true
mutationSignal.complete(Unit)
}

private suspend fun emitEditNoteDestination() {
Expand Down Expand Up @@ -411,13 +408,14 @@ class ReviewerViewModel(
}

private suspend fun runStateMutationHook() {
val state = queueState.await() ?: return
val js = state.customSchedulingJs
val js = stateMutationJs.await()
Copy link
Member

@mikehardy mikehardy Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to the question of hanging completions - looks like the js hook itself is potentially just hanging

it's fine to me as a roughly direct / same impact swith of style (without altering risk, seemingly?) from previous style

if (js.isEmpty()) {
statesMutated = true
if (!mutationSignal.isCompleted) {
mutationSignal.complete(Unit)
}
return
}
statesMutated = false
mutationSignal = CompletableDeferred()
statesMutationEvalFlow.emit(
"anki.mutateNextCardStates('$stateMutationKey', async (states, customData, ctx) => { $js });",
)
Expand Down
10 changes: 10 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/anki/utils/ext/Collection.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package com.ichi2.anki.utils.ext

import androidx.annotation.CheckResult
import anki.collection.OpChangesWithCount
import anki.config.ConfigKey
import com.ichi2.anki.Flag
import com.ichi2.anki.libanki.Collection

Expand All @@ -26,3 +27,12 @@ fun Collection.setUserFlagForCards(
cids: Iterable<Long>,
flag: Flag,
): OpChangesWithCount = setUserFlagForCards(cids, flag.code)

/**
* The `Custom scheduling` global setting in deck options.
*/
var Collection.cardStateCustomizer: String
get() = config.getString(ConfigKey.String.CARD_STATE_CUSTOMIZER)
set(value) {
config.setString(ConfigKey.String.CARD_STATE_CUSTOMIZER, value)
}