diff --git a/CLAUDE.md b/CLAUDE.md index 700704e..6b1a7c6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -31,6 +31,16 @@ designed past. - Do not invent a requirement to justify a design. If the task is lookup, a page-marking feature nobody asked for is not evidence that an index is needed. +## The SRS schedulers are measured, not argued + +`stores/simple-srs.ts` is a Midori clone and every scheduling rule in it has been changed +on simulated evidence; some tempting changes were measured and rejected. Read +[docs/simple-srs-decisions.md](docs/simple-srs-decisions.md) before editing it or the +simple-SRS branches of `app/(tabs)/lists/study.tsx`. Simulate a deck through +`nextSimpleReview` itself rather than reimplementing the rules — the lapse bug survived +every test for months because the tests called `simpleGraduate` directly and the defect +was in the argument the screen passed it. + ## Reader lookup and furigana scoring is settled — measure before you touch it Tap ranking, furigana resolution, counter readings and the deinflection rule diff --git a/app/(tabs)/lists/study.tsx b/app/(tabs)/lists/study.tsx index 008e325..5c4d87e 100644 --- a/app/(tabs)/lists/study.tsx +++ b/app/(tabs)/lists/study.tsx @@ -20,6 +20,9 @@ import { MnemonicClozeInput } from "@/components/MnemonicClozeInput"; import { shouldAskAsCloze } from "@/lib/rtk-cloze"; import { rtkReviewScope, singleList, type ListScope } from "@/lib/list-scope"; import { isRtkReviewList } from "@/lib/rtk-review"; +import { SRS_CARD_SELECT } from "@/db/srs-card-columns"; +import { resolveNewPerSession, UNLIMITED } from "@/stores/simple-srs"; +import { loadSimpleQueuePools } from "@/db/simple-queue"; import { getFaceText, getKanjiFaceText, @@ -74,12 +77,9 @@ import { reviewCard, Rating, getFsrsInstance, previewIntervals } from "@/stores/ import { formatInterval as formatDueInterval } from "@/lib/format-interval"; import { japaneseFontStyle } from "@/lib/japanese-font"; import { - simpleGraduate, - simpleReviewFail, - simpleInitCard, + nextSimpleReview, endOfLogicalDayEpochDays, endOfLogicalDayISO, - SIMPLE_SRS_REQUIRED_CORRECT, } from "@/stores/simple-srs"; import { useAtomValue } from "jotai"; import { useListsStore, parseListRow } from "@/stores/lists"; @@ -465,7 +465,6 @@ interface StudyCardViewProps { frontFaces: CardFace[]; backFaces: CardFace[]; flashcardMode: FlashcardMode; - simpleCorrectCount: number; // Voice/typing (only active for cursor card) voiceStatus: "idle" | "correct" | "wrong"; voiceHeard: string | null; @@ -496,7 +495,6 @@ const StudyCardView = React.memo( frontFaces, backFaces, flashcardMode, - simpleCorrectCount, voiceStatus, voiceHeard, isListening, @@ -595,10 +593,7 @@ const StudyCardView = React.memo( // Stats & checks const stats = getCardStats(item.srsCard, flashcardMode); - const checkCount = - flashcardMode === "simple_srs" && item.srsCard - ? (simpleCorrectCount ?? getCheckCount(item.srsCard, "simple_srs")) - : getCheckCount(item.srsCard, flashcardMode); + const checkCount = getCheckCount(item.srsCard, flashcardMode); // Render kana face with pitch accent visualization when available function renderFaceContent( @@ -909,6 +904,8 @@ interface SrsSnapshot { simpleStage: number | null; simpleN: number | null; simpleInterval: number | null; + simpleRelearning: number | null; + simpleRun: number | null; lastConfusionCheck: string | null; learningSteps: number; } @@ -923,7 +920,6 @@ interface StudyCard { snapshot?: SrsSnapshot | null; reviewLogId?: string | null; preStudyPosition?: number | null; - wasNewSimpleSrs?: boolean; reQueueOf?: number; } @@ -941,6 +937,8 @@ function captureSnapshot(card: SrsCardRow): SrsSnapshot { simpleStage: card.simpleStage, simpleN: card.simpleN, simpleInterval: card.simpleInterval, + simpleRelearning: card.simpleRelearning, + simpleRun: card.simpleRun, lastConfusionCheck: card.lastConfusionCheck, learningSteps: card.learningSteps, }; @@ -952,8 +950,9 @@ function getCheckCount(srsCard: SrsCardRow | undefined, mode: FlashcardMode): nu if (mode === "simple_srs") { if (srsCard.simpleStage == null) return 0; if (srsCard.simpleStage === 1) return 3; // graduated - // stage 0 (learning): 1 check after init, 2 if interval has grown - return srsCard.simpleInterval != null && srsCard.simpleInterval > 0.5 ? 2 : 1; + // The real count, now that it is persisted — this used to be guessed from + // the interval, because the count died with the screen. + return srsCard.simpleRun ?? 0; } if (mode === "srs") { @@ -1065,6 +1064,12 @@ function StudyScreen() { type SessionPhase = "studying" | "done" | "waiting"; const [sessionPhase, setSessionPhase] = useState("studying"); const reviewedSrsIdsRef = useRef>(new Set()); + /** + * Unseen cards this session has answered, which spend its allowance. Ids, not + * a count: re-rating a card re-applies the same answer, and the deque keeps + * the pre-answer row, so a counter spent a slot every walk-back. + */ + const newAnsweredRef = useRef>(new Set()); const [reviewedCount, setReviewedCount] = useState(0); const [settingsVisible, setSettingsVisible] = useState(false); const [statsVisible, setStatsVisible] = useState(false); @@ -1084,7 +1089,6 @@ function StudyScreen() { const sessionCorrectRef = useRef(0); const cursorCardRef = useRef(null); // Simple SRS: track correct-in-a-row per card (need 3 to graduate) - const simpleCorrectCountRef = useRef(new Map()); // Simple SRS: track cards that completed this session (graduated or passed review) const completedSrsIdsRef = useRef(new Set()); @@ -1333,19 +1337,15 @@ function StudyScreen() { useEffect(() => { if (!dictDb || !userDb || !list) return; + // A different list or scope is a new session; "Continue Studying" is not, + // and must keep both so it neither re-serves answered cards nor hands out + // another full allowance of new ones. + newAnsweredRef.current.clear(); + reviewedSrsIdsRef.current.clear(); loadQueue(); }, [dictDb, userDb, list?.id, scope]); - const srsSelectClause = `SELECT id, entry_id as entryId, kanji_literal as kanjiLiteral, list_id as listId, due, - stability, difficulty, elapsed_days as elapsedDays, - scheduled_days as scheduledDays, reps, lapses, state, - last_review as lastReview, front_mode as frontMode, - back_mode as backMode, created_at as createdAt, - updated_at as updatedAt, - simple_stage as simpleStage, simple_n as simpleN, - simple_interval as simpleInterval, - last_confusion_check as lastConfusionCheck, - learning_steps as learningSteps`; + const srsSelectClause = SRS_CARD_SELECT; /** Resolve SRS card rows into QueueItems with full dict entries */ async function resolveCards(srsRows: SrsCardRow[]): Promise { @@ -1631,18 +1631,39 @@ function StudyScreen() { { // Dynamic deque: load all due + all new into pools, // then seed the active deque with the first batch. - const dueRows = await userDb.getAllAsync( - `${srsSelectClause} FROM srs_cards - WHERE ${scope.clause} AND simple_stage IS NOT NULL AND simple_n <= ? AND deleted_at IS NULL - ORDER BY simple_n ASC`, - [...scope.args, dueCutoffDays], - ); - const newRows = await userDb.getAllAsync( - `${srsSelectClause} FROM srs_cards - WHERE ${scope.clause} AND simple_stage IS NULL AND deleted_at IS NULL - ORDER BY created_at ASC`, - [...scope.args], - ); + // A flagged or smart review is assembled on purpose and is all-new, + // so a cap would show ten of it and call that a session (-1 is + // SQLite's own "no limit"). Everywhere else the cap is the point: + // the whole list at once is why nothing could ever reach three + // correct answers. + // + // Read from the row, not from `list`: the settings modal reloads the + // queue from its own close handler, which closes over the pre-save + // value, and depending on it here instead would race that reload. + let newCap: number = UNLIMITED; + if (!isEphemeralListId(listId)) { + const capRow = await userDb.getFirstAsync<{ new_per_session: number | null }>( + "SELECT new_per_session FROM lists WHERE id = ?", + [listId], + ); + newCap = resolveNewPerSession(capRow?.new_per_session); + } + // "Continue Studying" calls this again, so the allowance is what is + // LEFT of the cap rather than a fresh one — otherwise ten taps + // introduce a hundred new cards. It counts cards the session has + // answered, not cards it has loaded: a reload re-queries the unseen + // cards still sitting unanswered in the pool, and counting those + // again left a session unable to offer new cards it still owed. + const newAllowance = + newCap === UNLIMITED ? UNLIMITED : Math.max(0, newCap - newAnsweredRef.current.size); + const { dueRows, newRows } = await loadSimpleQueuePools({ + query: (sql, args) => userDb.getAllAsync(sql, args), + scope, + dueCutoffDays, + newCap, + newAllowance, + excludeIds: reviewedSrsIdsRef.current, + }); if (dueRows.length > 0 || newRows.length > 0) { const dueItems = await resolveCards(dueRows); @@ -1819,7 +1840,6 @@ function StudyScreen() { snapshot, reviewLogId, preStudyPosition: null, - wasNewSimpleSrs: card ? card.simpleStage == null : false, }; // Push failed card to end for re-review (non-FSRS only; FSRS uses timer) if (sessionMode !== "srs") { @@ -1886,7 +1906,6 @@ function StudyScreen() { const snapshot = card ? captureSnapshot(card) : null; let reviewLogId: string | null = null; let preStudyPosition: number | null = null; - const wasNewSimpleSrs = card ? card.simpleStage == null : false; let shouldReQueue = false; let reQueueItem = item; @@ -1943,7 +1962,6 @@ function StudyScreen() { snapshot, reviewLogId, preStudyPosition, - wasNewSimpleSrs, }; if (shouldReQueue) { updatedCards.push({ @@ -2028,7 +2046,6 @@ function StudyScreen() { snapshot, reviewLogId, preStudyPosition: null, - wasNewSimpleSrs: false, }; cardsRef.current = updatedCards; setCards(updatedCards); @@ -2095,7 +2112,6 @@ function StudyScreen() { snapshot, reviewLogId, preStudyPosition: null, - wasNewSimpleSrs: false, }; cardsRef.current = updatedCards; setCards(updatedCards); @@ -2216,69 +2232,34 @@ function StudyScreen() { ): Promise<{ graduated: boolean; card: SrsCardRow }> { if (!userDb) return { graduated: true, card }; const now = new Date().toISOString(); - const isNew = card.simpleStage == null; - const isEasy = action === "easy"; const pass = action !== "fail"; - let updates: { simpleStage: number; simpleN: number; simpleInterval: number }; - let graduated = false; + // The decision lives in stores/simple-srs.ts so it can be tested: the lapse + // bug was in which flag this call site handed over, not in the arithmetic. + // The correct-in-a-row count rides in `updates` and is persisted — it used + // to live in a ref that died with the screen, so a card answered twice and + // left came back to a count of zero and stayed due forever. + // An unseen card's first answer is what spends the session's allowance. + if (card.simpleStage == null) newAnsweredRef.current.add(card.id); - if (!pass) { - // FAIL: preserve interval, reset to learning, increment lapses - if (isNew) { - updates = simpleInitCard(); - } else { - updates = simpleReviewFail(card); - } - // Reset correct count for this card - simpleCorrectCountRef.current.set(card.id, 0); - } else if (isEasy) { - // EASY: skip correctCount gate, immediately graduate - if (isNew) { - const init = simpleInitCard(); - updates = simpleGraduate({ ...card, ...init }, true, card.lapses > 0); - } else { - updates = simpleGraduate(card, true, card.lapses > 0); - } - graduated = true; - simpleCorrectCountRef.current.delete(card.id); - } else { - // CORRECT: behavior depends on card state - if (isNew) { - // First time seeing this card -- initialize and start counting - updates = simpleInitCard(); - simpleCorrectCountRef.current.set(card.id, 1); - // Not graduated yet (need 3 correct) - } else if (card.simpleStage === 1) { - // Already graduated, due for review -- pass once → done - updates = simpleGraduate(card, false, card.lapses > 0); - graduated = true; - simpleCorrectCountRef.current.delete(card.id); - } else { - // Learning/lapsed card (stage 0) -- need 3 correct to graduate - const count = (simpleCorrectCountRef.current.get(card.id) ?? 0) + 1; - simpleCorrectCountRef.current.set(card.id, count); - - if (count >= SIMPLE_SRS_REQUIRED_CORRECT) { - // Graduated! Update interval and schedule next review - updates = simpleGraduate(card, false, card.lapses > 0); - graduated = true; - simpleCorrectCountRef.current.delete(card.id); - } else { - // Still learning -- keep current state, card will be re-queued - updates = { - simpleStage: 0, - simpleN: 0, // immediately due - simpleInterval: card.simpleInterval ?? 0, - }; - } - } - } + const decision = nextSimpleReview(card, action); + const updates = decision.updates; + const graduated = decision.graduated; await userDb.runAsync( `UPDATE srs_cards SET simple_stage = ?, simple_n = ?, simple_interval = ?, - reps = reps + 1, lapses = lapses + ?, updated_at = ? WHERE id = ?`, - [updates.simpleStage, updates.simpleN, updates.simpleInterval, pass ? 0 : 1, now, card.id], + simple_relearning = ?, simple_run = ?, reps = reps + 1, lapses = lapses + ?, + updated_at = ? WHERE id = ?`, + [ + updates.simpleStage, + updates.simpleN, + updates.simpleInterval, + updates.simpleRelearning, + updates.simpleRun, + pass ? 0 : 1, + now, + card.id, + ], ); return { @@ -2372,15 +2353,7 @@ function StudyScreen() { // Find the srs_card for this confused entry const cardRow = await userDb.getFirstAsync( - `SELECT id, entry_id as entryId, kanji_literal as kanjiLiteral, list_id as listId, due, - stability, difficulty, elapsed_days as elapsedDays, - scheduled_days as scheduledDays, reps, lapses, state, - last_review as lastReview, front_mode as frontMode, - back_mode as backMode, created_at as createdAt, - updated_at as updatedAt, - simple_stage as simpleStage, simple_n as simpleN, - simple_interval as simpleInterval, - last_confusion_check as lastConfusionCheck + `${SRS_CARD_SELECT} FROM srs_cards WHERE ${scope.clause} AND entry_id = ? AND kanji_literal IS NULL AND deleted_at IS NULL ORDER BY due ASC`, [...scope.args, result.entry.id], @@ -2395,9 +2368,24 @@ function StudyScreen() { await rateSrsCard(cardRow, Rating.Again); } + // Drop every stale copy of it first. The pools hold the row as it was at + // session start, and the deque may already have pulled one — either + // carries a correct-run from before this fail reset it, so answering it + // would graduate the card on one pass and write it twice in one session. + // Only copies not yet answered are dropped; an answered one is history. + const queuedId = queuedCard.id; + duePoolRef.current = duePoolRef.current.filter((item) => item.srsCard?.id !== queuedId); + newPoolRef.current = newPoolRef.current.filter((item) => item.srsCard?.id !== queuedId); + + // Back in learning, so it is no longer one of the cards finished. + completedSrsIdsRef.current.delete(queuedId); + // Push to end of current session setCards((prev) => [ - ...prev, + ...prev.filter( + (card, index) => + index <= cursor || card.status !== "pending" || card.item.srsCard?.id !== queuedId, + ), { item: { kind: "entry" as const, entry: result.entry as DictEntry, srsCard: queuedCard }, status: "pending" as CardStatus, @@ -2453,15 +2441,19 @@ function StudyScreen() { } // Reset completed status on undo completedSrsIdsRef.current.delete(card.id); + reviewedSrsIdsRef.current.delete(card.id); } else if (sessionMode === "simple_srs" && card && snap) { // Restore simple SRS fields await userDb.runAsync( `UPDATE srs_cards SET simple_stage = ?, simple_n = ?, simple_interval = ?, - reps = ?, lapses = ?, last_confusion_check = ?, updated_at = ? WHERE id = ?`, + simple_relearning = ?, simple_run = ?, reps = ?, lapses = ?, + last_confusion_check = ?, updated_at = ? WHERE id = ?`, [ snap.simpleStage, snap.simpleN, snap.simpleInterval, + snap.simpleRelearning, + snap.simpleRun, snap.reps, snap.lapses, snap.lastConfusionCheck, @@ -2469,9 +2461,13 @@ function StudyScreen() { card.id, ], ); - // Reset correct count and completed status for this card on undo - simpleCorrectCountRef.current.delete(card.id); + // The restored snapshot carries the correct-in-a-row count with it now. completedSrsIdsRef.current.delete(card.id); + // An undone answer is not an answer: refund the allowance it spent and + // let the card be loaded again, or walking back five cards left the + // session with no new cards while the database still called them unseen. + if (snap.simpleStage == null) newAnsweredRef.current.delete(card.id); + reviewedSrsIdsRef.current.delete(card.id); } else if (sessionMode === "add_order" && studyCard.preStudyPosition != null) { // Restore study_position await userDb.runAsync("UPDATE lists SET study_position = ?, updated_at = ? WHERE id = ?", [ @@ -2510,8 +2506,21 @@ function StudyScreen() { } next.push({ item: cards[cursor].item, status: "revealed", flipped: true }); for (let i = cursor + 1; i < cards.length; i++) { - if (cards[i].reQueueOf == null) { + const reQueuedBy = cards[i].reQueueOf; + // A re-queue copy is void only if the answer that created it is being + // undone. One owned by a card BEFORE the cursor keeps its place: its + // owner has already left the pool, so dropping the copy stranded the + // card for the rest of the session and the progress bar could never + // reach 100%. + if (reQueuedBy == null) { next.push({ item: cards[i].item, status: "pending", flipped: false }); + } else if (reQueuedBy < cursor) { + next.push({ + item: cards[i].item, + status: "pending", + flipped: false, + reQueueOf: reQueuedBy, + }); } } cardsRef.current = next; @@ -2846,7 +2855,6 @@ function StudyScreen() { onPress={() => { setSessionPhase("studying"); setReviewedCount(0); - reviewedSrsIdsRef.current.clear(); loadQueue(); }} /> @@ -2959,11 +2967,6 @@ function StudyScreen() { frontFaces={frontFaces} backFaces={backFaces} flashcardMode={sessionMode} - simpleCorrectCount={ - studyCard.item.srsCard - ? (simpleCorrectCountRef.current.get(studyCard.item.srsCard.id) ?? 0) - : 0 - } voiceStatus={isCursor ? voiceStatus : "idle"} voiceHeard={isCursor ? voiceHeard : null} isListening={isCursor && isListening} diff --git a/components/DueCardsSection.tsx b/components/DueCardsSection.tsx index 47a1ce4..b67bf9f 100644 --- a/components/DueCardsSection.tsx +++ b/components/DueCardsSection.tsx @@ -6,7 +6,12 @@ import { useUserDb } from "@/db/user-provider"; import { useSync } from "@/db/sync-provider"; import { useListsStore } from "@/stores/lists"; import { getCardStateDistribution, type CardDistribution } from "@/lib/practice-stats"; -import { srsEpochDaysToDate, endOfLogicalDayEpochDays } from "@/stores/simple-srs"; +import { + srsEpochDaysToDate, + endOfLogicalDayEpochDays, + resolveNewPerSession, + learningCapFor, +} from "@/stores/simple-srs"; import { useAtomValue } from "jotai"; import { dayResetHourAtom } from "@/stores/settings"; import type { FlashcardMode } from "@/db/types"; @@ -122,7 +127,7 @@ export function DueCardsSection({ if (flashcardMode === "simple_srs") { const rows = await userDb.getAllAsync<{ dueDays: number }>( `SELECT simple_n as dueDays - FROM srs_cards WHERE list_id = ? AND simple_stage IS NOT NULL AND deleted_at IS NULL`, + FROM srs_cards WHERE list_id = ? AND simple_stage = 1 AND deleted_at IS NULL`, [listId], ); for (const row of rows) { @@ -130,6 +135,26 @@ export function DueCardsSection({ const idx = dateToBucketIndex(dueDate, dayResetHour); counts[idx]++; } + // What the session will actually serve, not what the deck holds: cards in + // learning are all due now and the session shows three times the + // new-card cap of them, plus up to a cap of unseen cards. Counting the + // deck promised 8,000 due where the session showed thirty and then said + // it was done; ignoring the new cards under-reported a fresh list as 0. + const capRow = await userDb.getFirstAsync<{ new_per_session: number | null }>( + "SELECT new_per_session FROM lists WHERE id = ?", + [listId], + ); + const newCap = resolveNewPerSession(capRow?.new_per_session); + const pending = await userDb.getFirstAsync<{ learning: number; unseen: number }>( + // SUM(CASE …) rather than COUNT(*) FILTER, which needs SQLite 3.30+. + `SELECT + SUM(CASE WHEN simple_stage = 0 AND simple_n <= ? THEN 1 ELSE 0 END) as learning, + SUM(CASE WHEN simple_stage IS NULL THEN 1 ELSE 0 END) as unseen + FROM srs_cards WHERE list_id = ? AND deleted_at IS NULL`, + [endOfLogicalDayEpochDays(dayResetHour), listId], + ); + counts[0] += Math.min(pending?.learning ?? 0, learningCapFor(newCap)); + counts[0] += Math.min(pending?.unseen ?? 0, newCap); } else { const newRow = await userDb.getFirstAsync<{ c: number }>( "SELECT COUNT(*) as c FROM srs_cards WHERE list_id = ? AND state = 0 AND deleted_at IS NULL", diff --git a/components/FlashcardSettingsModal.tsx b/components/FlashcardSettingsModal.tsx index 8f7d458..a0bdcfa 100644 --- a/components/FlashcardSettingsModal.tsx +++ b/components/FlashcardSettingsModal.tsx @@ -14,6 +14,7 @@ import { alert } from "@/lib/confirm"; import { requestVoicePermissions } from "@/lib/voice-recognition"; import { useSync } from "@/db/sync-provider"; import type { CardFace, FlashcardMode } from "@/db/types"; +import { DEFAULT_NEW_PER_SESSION } from "@/stores/simple-srs"; interface FlashcardSettingsModalProps { visible: boolean; @@ -55,6 +56,7 @@ export function FlashcardSettingsModal({ const [buttonAnimation, setButtonAnimation] = useAtom(flashcardButtonAnimationAtom); const [learningStepsText, setLearningStepsText] = useState(""); const [relearningStepsText, setRelearningStepsText] = useState(""); + const [newPerSessionText, setNewPerSessionText] = useState(""); const [hasKanjiEntries, setHasKanjiEntries] = useState(false); const inputMode = voiceMode @@ -85,6 +87,7 @@ export function FlashcardSettingsModal({ setMnemonicCloze(list.mnemonicCloze ?? false); setLearningStepsText(list.learningSteps ? list.learningSteps.join(", ") : ""); setRelearningStepsText(list.relearningSteps ? list.relearningSteps.join(", ") : ""); + setNewPerSessionText(list.newPerSession != null ? String(list.newPerSession) : ""); } }, [visible, list]); @@ -121,6 +124,23 @@ export function FlashcardSettingsModal({ async function handleSave() { if (!userDb) return; + // Empty means "use the default", not zero: a cap of 0 would send a list + // with nothing due straight to the done screen. The input only shows for + // simple SRS, so any other mode keeps whatever the list already stored + // rather than writing NULL over it. + let newPerSession: number | null = list?.newPerSession ?? null; + if (mode === "simple_srs") { + newPerSession = null; + if (newPerSessionText.trim()) { + const parsed = Number(newPerSessionText.trim()); + if (!Number.isInteger(parsed) || parsed < 1 || parsed > 500) { + alert("Invalid number", "New cards per session must be a whole number from 1 to 500."); + return; + } + newPerSession = parsed; + } + } + // Validate learning steps if provided let learningSteps: string[] | null = null; let relearningSteps: string[] | null = null; @@ -159,7 +179,8 @@ export function FlashcardSettingsModal({ typingMode === (list.typingMode ?? false) && mnemonicCloze === (list.mnemonicCloze ?? false) && arrEq(learningSteps, list.learningSteps) && - arrEq(relearningSteps, list.relearningSteps); + arrEq(relearningSteps, list.relearningSteps) && + newPerSession === (list.newPerSession ?? null); if (unchanged) { onClose(false); return; @@ -168,7 +189,7 @@ export function FlashcardSettingsModal({ const now = new Date().toISOString(); await userDb.runAsync( - "UPDATE lists SET flashcard_mode = ?, front_faces = ?, back_faces = ?, auto_play_audio = ?, confusion_detection = ?, voice_mode = ?, typing_mode = ?, mnemonic_cloze = ?, learning_steps = ?, relearning_steps = ?, configured = 1, updated_at = ? WHERE id = ?", + "UPDATE lists SET flashcard_mode = ?, front_faces = ?, back_faces = ?, auto_play_audio = ?, confusion_detection = ?, voice_mode = ?, typing_mode = ?, mnemonic_cloze = ?, learning_steps = ?, relearning_steps = ?, new_per_session = ?, configured = 1, updated_at = ? WHERE id = ?", [ mode, JSON.stringify(frontFaces), @@ -180,6 +201,7 @@ export function FlashcardSettingsModal({ mnemonicCloze ? 1 : 0, learningSteps ? JSON.stringify(learningSteps) : null, relearningSteps ? JSON.stringify(relearningSteps) : null, + newPerSession, now, listId, ], @@ -196,13 +218,17 @@ export function FlashcardSettingsModal({ mnemonicCloze, learningSteps, relearningSteps, + newPerSession, updatedAt: now, }); markDirty(); if (onStartStudy) { onStartStudy(); } else { - onClose(mode !== list?.flashcardMode); + // The queue is rebuilt for a changed mode and for a changed new-card cap: + // the cap decides how many cards the session loads, so leaving it until + // the next session means the setting looks ignored. + onClose(mode !== list?.flashcardMode || newPerSession !== (list?.newPerSession ?? null)); } } @@ -297,6 +323,28 @@ export function FlashcardSettingsModal({ )} + {/* New cards per session (simple SRS only) */} + {mode === "simple_srs" && ( + + + New cards per session + + + + Each new card costs about ten reviews a day once it settles in. + + + )} + {/* Front faces */} Front diff --git a/components/StudyStatisticsModal.tsx b/components/StudyStatisticsModal.tsx index 46dbea4..a9e041a 100644 --- a/components/StudyStatisticsModal.tsx +++ b/components/StudyStatisticsModal.tsx @@ -39,7 +39,8 @@ export function StudyStatisticsModal({ if (flashcardMode === "simple_srs") { const now = new Date().toISOString(); await userDb.runAsync( - "UPDATE srs_cards SET simple_stage = NULL, simple_n = NULL, simple_interval = NULL, updated_at = ? WHERE list_id = ?", + `UPDATE srs_cards SET simple_stage = NULL, simple_n = NULL, simple_interval = NULL, + simple_relearning = 0, simple_run = 0, updated_at = ? WHERE list_id = ?`, [now, listId], ); } else if (flashcardMode === "srs") { diff --git a/db/schema.ts b/db/schema.ts index daee374..ab6bb79 100644 --- a/db/schema.ts +++ b/db/schema.ts @@ -118,6 +118,7 @@ export const lists = sqliteTable("lists", { voiceMode: integer("voice_mode").notNull().default(0), typingMode: integer("typing_mode").notNull().default(0), mnemonicCloze: integer("mnemonic_cloze").notNull().default(0), + newPerSession: integer("new_per_session").default(10), disableFlipAnimation: integer("disable_flip_animation").notNull().default(0), disableSwipeAnimation: integer("disable_swipe_animation").notNull().default(0), isDefault: integer("is_default").notNull().default(0), @@ -161,6 +162,8 @@ export const srsCards = sqliteTable("srs_cards", { simpleStage: integer("simple_stage"), simpleN: real("simple_n"), simpleInterval: real("simple_interval"), + simpleRelearning: integer("simple_relearning").default(0), + simpleRun: integer("simple_run").default(0), lastConfusionCheck: text("last_confusion_check"), deletedAt: text("deleted_at"), learningSteps: integer("learning_steps").notNull().default(0), diff --git a/db/simple-queue.test.ts b/db/simple-queue.test.ts new file mode 100644 index 0000000..5b6fccd --- /dev/null +++ b/db/simple-queue.test.ts @@ -0,0 +1,240 @@ +/** + * The queries that fill a simple-SRS session, run through the shipped + * `loadSimpleQueuePools` rather than a retyped copy of its SQL. + * + * This file exists because review proved the previous version did not gate + * anything: `lib/new-per-session.test.ts` typed the queries out again inside + * the test, so deleting the cap from the screen, deleting the learning LIMIT, + * or flipping its ORDER BY all left the whole suite green. + */ +import Database from "better-sqlite3"; +import { beforeEach, describe, expect, it } from "vitest"; + +import { loadSimpleQueuePools, type SimpleQueueQuerier } from "./simple-queue"; +import { USER_DB_MIGRATIONS } from "./user-migrations"; +import type { SrsCardRow } from "./types"; +import { DEFAULT_NEW_PER_SESSION, UNLIMITED } from "@/stores/simple-srs"; + +/** Days since 2001-01-01, the epoch `simple_n` is stored in. */ +const CUTOFF = (Date.now() - Date.parse("2001-01-01T00:00:00Z")) / 86_400_000 + 1; + +type Card = { + id: string; + stage: number | null; + n: number | null; + /** ISO timestamp; `updated_at` is the learning slice's ordering key. */ + updatedAt?: string; + /** Deliberately set against rowid order, so the ORDER BY is what sorts. */ + createdAt?: string; + deleted?: boolean; +}; + +let db: Database.Database; + +const query: SimpleQueueQuerier = async (sql, args) => + db.prepare(sql).all(...(args as unknown[])) as SrsCardRow[]; + +const scope = { clause: "list_id = ?", args: ["l"] }; + +function insert(c: Card) { + db.prepare( + `INSERT INTO srs_cards (id, entry_id, list_id, due, stability, difficulty, elapsed_days, + scheduled_days, reps, lapses, state, front_mode, back_mode, simple_stage, simple_n, + simple_interval, created_at, updated_at, deleted_at) + VALUES (?, 1, 'l', 'x', 0, 0, 0, 0, 3, 1, 0, 'kanji', 'english', ?, ?, 0.3333, ?, ?, ?)`, + ).run( + c.id, + c.stage, + c.n, + c.createdAt ?? `created-${c.id}`, + c.updatedAt ?? "2026-01-01T00:00:00.000Z", + c.deleted ? "gone" : null, + ); +} + +beforeEach(() => { + db = new Database(":memory:"); + db.pragma("foreign_keys = OFF"); + for (const sql of USER_DB_MIGRATIONS) { + try { + db.exec(sql); + } catch (err) { + if (!/duplicate column/i.test(String(err))) throw err; + } + } +}); + +/** `newCap` is the list's setting; `allowance` is what the session has left. */ +const pools = (newCap: number, excludeIds?: Set, newAllowance?: number) => + loadSimpleQueuePools({ query, scope, dueCutoffDays: CUTOFF, newCap, newAllowance, excludeIds }); + +const ids = (rows: SrsCardRow[]) => rows.map((r) => r.id); + +describe("the new-card cap", () => { + it("introduces the allowance, not the list", async () => { + for (let i = 0; i < 8000; i++) insert({ id: `n${i}`, stage: null, n: null }); + expect((await pools(DEFAULT_NEW_PER_SESSION)).newRows).toHaveLength(10); + }); + + it("introduces the oldest first, whatever order the rows were written in", async () => { + // created_at runs against rowid here on purpose: with the two agreeing, + // deleting the ORDER BY entirely still passed. + for (let i = 0; i < 20; i++) { + const day = String(20 - i).padStart(2, "0"); + insert({ id: `n${i}`, stage: null, n: null, createdAt: `2026-01-${day}` }); + } + expect(ids((await pools(3)).newRows)).toEqual(["n19", "n18", "n17"]); + }); + + it("gives an ephemeral session every unseen card", async () => { + for (let i = 0; i < 45; i++) insert({ id: `n${i}`, stage: null, n: null }); + expect((await pools(UNLIMITED)).newRows).toHaveLength(45); + }); + + it("introduces nothing once the session has spent its allowance", async () => { + for (let i = 0; i < 50; i++) insert({ id: `n${i}`, stage: null, n: null }); + expect((await pools(10, undefined, 0)).newRows).toHaveLength(0); + }); + + it("keeps serving the cards in learning after the allowance is spent", async () => { + // The learning slice is sized from the list's cap, never from what is left + // of it: keying it off the allowance emptied the whole queue once the + // session had introduced its new cards, and a mid-session settings save — + // which now rebuilds the queue — sent the session to the done screen with + // cards still owed. + for (let i = 0; i < 20; i++) insert({ id: `l${i}`, stage: 0, n: 0 }); + for (let i = 0; i < 20; i++) insert({ id: `n${i}`, stage: null, n: null }); + const { dueRows, newRows } = await pools(10, undefined, 0); + expect(newRows).toHaveLength(0); + expect(dueRows).toHaveLength(20); + }); +}); + +describe("the learning slice", () => { + it("is bounded at three times the new cap", async () => { + // A deck the uncapped session left behind: every card in learning, every + // one due, which is the queue that came back unchanged every day. + for (let i = 0; i < 8000; i++) insert({ id: `l${i}`, stage: 0, n: 0 }); + expect((await pools(DEFAULT_NEW_PER_SESSION)).dueRows).toHaveLength(30); + }); + + // A guard, not a gate: this passes without the `rowid ASC` tiebreak too, + // because SQLite happens to return rowid order for equal keys. The tiebreak + // makes that a promise rather than a coincidence — the backfill stamps every + // card the same `updated_at` and there is no index on it. + it("breaks a shared timestamp on rowid, so the slice is the same every build", async () => { + for (let i = 0; i < 40; i++) insert({ id: `l${i}`, stage: 0, n: 0, updatedAt: "same" }); + // Comparing two calls proves nothing — same function, same rows. What can + // be said is which cards the slice takes. + expect(ids((await pools(DEFAULT_NEW_PER_SESSION)).dueRows).slice(0, 3)).toEqual([ + "l0", + "l1", + "l2", + ]); + }); + + it("takes the most recently answered first, so cards in hand get finished", async () => { + insert({ id: "old", stage: 0, n: 0, updatedAt: "2026-01-01T00:00:00.000Z" }); + insert({ id: "newest", stage: 0, n: 0, updatedAt: "2026-06-01T00:00:00.000Z" }); + insert({ id: "middle", stage: 0, n: 0, updatedAt: "2026-03-01T00:00:00.000Z" }); + const { dueRows } = await pools(1); + expect(ids(dueRows)).toEqual(["newest", "middle", "old"]); + }); + + it("leads the queue, ahead of the graduated cards due today", async () => { + insert({ id: "graduated", stage: 1, n: 0 }); + insert({ id: "learning", stage: 0, n: 0 }); + expect(ids((await pools(1)).dueRows)).toEqual(["learning", "graduated"]); + }); + + it("is unbounded for an ephemeral session", async () => { + for (let i = 0; i < 60; i++) insert({ id: `l${i}`, stage: 0, n: 0 }); + expect((await pools(UNLIMITED)).dueRows).toHaveLength(60); + }); +}); + +describe("the graduated slice", () => { + it("is never capped — it is the review load the learner owes", async () => { + for (let i = 0; i < 500; i++) insert({ id: `g${i}`, stage: 1, n: 0 }); + expect((await pools(DEFAULT_NEW_PER_SESSION)).dueRows).toHaveLength(500); + }); + + it("includes a card due exactly on the cutoff, and excludes one past it", async () => { + insert({ id: "today", stage: 1, n: CUTOFF - 1 }); + insert({ id: "on-the-cutoff", stage: 1, n: CUTOFF }); + insert({ id: "tomorrow", stage: 1, n: CUTOFF + 1 }); + expect(ids((await pools(1)).dueRows)).toEqual(["today", "on-the-cutoff"]); + }); + + it("serves only the stages the engine writes", async () => { + // The engine writes 0 and 1 and nothing else — pinned by + // `stores/simple-srs.test.ts` — and the statistics, the badge and the lists + // index all key on `= 1`, so this query does too. A stage nothing can + // create is in no slice, and in no bucket either; consistent beats clever. + insert({ id: "odd", stage: 2, n: 0 }); + insert({ id: "graduated", stage: 1, n: 0 }); + expect(ids((await pools(1)).dueRows)).toEqual(["graduated"]); + }); + + it("comes back in due order, oldest first", async () => { + insert({ id: "later", stage: 1, n: CUTOFF - 1 }); + insert({ id: "earlier", stage: 1, n: CUTOFF - 5 }); + expect(ids((await pools(1)).dueRows)).toEqual(["earlier", "later"]); + }); +}); + +describe("cards already answered this session", () => { + it("are held back, so Continue Studying does not re-serve them", async () => { + // A graduated card's 0.633-day interval falls inside today's cutoff, so + // without this it comes straight back on the next queue build. + insert({ id: "just-passed", stage: 1, n: CUTOFF - 0.2 }); + insert({ id: "still-due", stage: 1, n: CUTOFF - 1 }); + const { dueRows } = await pools(1, new Set(["just-passed"])); + expect(ids(dueRows)).toEqual(["still-due"]); + }); + + it("do not eat the cap they are excluded from", async () => { + // The LIMIT would otherwise spend its slots on the excluded rows and hand + // back a short batch. + for (let i = 0; i < 40; i++) insert({ id: `n${i}`, stage: null, n: null }); + const spent = new Set(Array.from({ length: 10 }, (_, i) => `n${i}`)); + const { newRows } = await pools(DEFAULT_NEW_PER_SESSION, spent); + expect(newRows).toHaveLength(10); + expect(ids(newRows)[0]).toBe("n10"); + }); + + it("do not eat the learning slice either", async () => { + for (let i = 0; i < 60; i++) { + insert({ + id: `l${i}`, + stage: 0, + n: 0, + updatedAt: `2026-01-${String(i + 1).padStart(2, "0")}`, + }); + } + const spent = new Set(["l59", "l58", "l57"]); + const { dueRows } = await pools(DEFAULT_NEW_PER_SESSION, spent); + expect(dueRows).toHaveLength(30); + expect(ids(dueRows)).not.toContain("l59"); + }); +}); + +describe("rows no session may see", () => { + it("skips a soft-deleted card in every slice", async () => { + insert({ id: "g", stage: 1, n: 0, deleted: true }); + insert({ id: "l", stage: 0, n: 0, deleted: true }); + insert({ id: "n", stage: null, n: null, deleted: true }); + const { dueRows, newRows } = await pools(DEFAULT_NEW_PER_SESSION); + expect(dueRows).toHaveLength(0); + expect(newRows).toHaveLength(0); + }); + + it("keeps the three slices disjoint", async () => { + insert({ id: "g", stage: 1, n: 0 }); + insert({ id: "l", stage: 0, n: 0 }); + insert({ id: "n", stage: null, n: null }); + const { dueRows, newRows } = await pools(DEFAULT_NEW_PER_SESSION); + expect(ids(dueRows).sort()).toEqual(["g", "l"]); + expect(ids(newRows)).toEqual(["n"]); + }); +}); diff --git a/db/simple-queue.ts b/db/simple-queue.ts new file mode 100644 index 0000000..a774cba --- /dev/null +++ b/db/simple-queue.ts @@ -0,0 +1,142 @@ +/** + * The three queries that fill a simple-SRS session, in one place. + * + * They live here rather than inline in the study screen because the screen is + * covered by no test: review proved that `newCap = UNLIMITED`, deleting the + * learning `LIMIT`, and deleting `simple_run` from the persistence UPDATE each + * left the whole suite green — the 8,000-card queue and the graduation fix both + * fully revertable without a single failure. `db/simple-queue.test.ts` runs + * THESE functions, so the SQL a session uses is the SQL a test gates. + * + * That gates the SQL and not the screen calling it, which an earlier commit + * message claimed and review disproved: the same one-line reverts still passed + * once the queries had moved. `app/study-wiring.test.ts` reads the screen's + * source for exactly those edits — a guard, not a behavioural test, and the + * honest limit of testing a React screen nothing can render here. + */ + +import { SRS_CARD_SELECT } from "./srs-card-columns"; +import type { SrsCardRow } from "./types"; +import { learningCapFor, UNLIMITED } from "@/stores/simple-srs"; + +/** Runs a parameterised query, the shape both `WrappedUserDb` and a test give. */ +export type SimpleQueueQuerier = (sql: string, args: unknown[]) => Promise; + +/** The part of a `ListScope` these queries need. */ +export interface SimpleQueueScope { + clause: string; + args: readonly unknown[]; +} + +export interface SimpleQueueRequest { + query: SimpleQueueQuerier; + scope: SimpleQueueScope; + /** End of the logical day, in the epoch `simple_n` is stored in. */ + dueCutoffDays: number; + /** + * The list's `new_per_session`, or `UNLIMITED`. The learning slice is sized + * from THIS, never from the allowance below: a session that has already + * introduced its new cards has no allowance left, and keying the learning + * slice off it emptied the whole queue and sent the session to the done + * screen with cards still owed. + */ + newCap: number; + /** + * Unseen cards this session may still introduce — the cap less the ones it + * has already answered. Defaults to `newCap`. + */ + newAllowance?: number; + /** Cards already answered in this session, which must not come back. */ + excludeIds?: ReadonlySet; +} + +export interface SimpleQueuePools { + /** Cards in learning first, then graduated cards due today. */ + dueRows: SrsCardRow[]; + newRows: SrsCardRow[]; +} + +/** + * Graduated and due today. Never capped — this is the real review load. + * + * `= 1` because the engine writes 0 and 1 and nothing else, which + * `stores/simple-srs.test.ts` pins. Every other consumer agrees — + * `DueCardsSection`, the lists index, `stores/lists.ts`, `lib/practice-stats.ts` + * — and `!= 0` here would have served a stage this split cannot otherwise reach + * while those four counted it in no bucket at all. + */ +export const graduatedDueSql = (scopeClause: string) => + `${SRS_CARD_SELECT} FROM srs_cards + WHERE ${scopeClause} AND simple_stage = 1 AND simple_n <= ? AND deleted_at IS NULL + ORDER BY simple_n ASC`; + +/** + * Still in learning, capped. A deck left by the uncapped session has thousands + * of these, every one due, which is the queue that came back unchanged every + * day. Most recently answered first, so the cards already in hand get finished + * before the backlog is reached. + */ +export const learningDueSql = (scopeClause: string) => + `${SRS_CARD_SELECT} FROM srs_cards + WHERE ${scopeClause} AND simple_stage = 0 AND simple_n <= ? AND deleted_at IS NULL + ORDER BY updated_at DESC, rowid ASC + LIMIT ?`; + +/** Never seen, capped at the list's `new_per_session`. */ +export const unseenSql = (scopeClause: string) => + `${SRS_CARD_SELECT} FROM srs_cards + WHERE ${scopeClause} AND simple_stage IS NULL AND deleted_at IS NULL + ORDER BY created_at ASC, rowid ASC + LIMIT ?`; + +/** + * A capped query cannot simply drop the excluded rows afterwards — the LIMIT + * would have already spent its slots on them — so it asks for enough extra to + * cover every exclusion and trims after filtering. + */ +function limitWith(cap: number, excluded: number): number { + return cap === UNLIMITED ? UNLIMITED : cap + excluded; +} + +function withoutExcluded( + rows: SrsCardRow[], + excludeIds: ReadonlySet | undefined, + cap: number, +): SrsCardRow[] { + const kept = excludeIds?.size ? rows.filter((row) => !excludeIds.has(row.id)) : rows; + return cap === UNLIMITED ? kept : kept.slice(0, cap); +} + +/** Load the pools a simple-SRS session studies from. */ +export async function loadSimpleQueuePools({ + query, + scope, + dueCutoffDays, + newCap, + newAllowance = newCap, + excludeIds, +}: SimpleQueueRequest): Promise { + const excluded = excludeIds?.size ?? 0; + const learningCap = learningCapFor(newCap); + + const graduated = await query(graduatedDueSql(scope.clause), [...scope.args, dueCutoffDays]); + const learning = await query(learningDueSql(scope.clause), [ + ...scope.args, + dueCutoffDays, + limitWith(learningCap, excluded), + ]); + const unseen = await query(unseenSql(scope.clause), [ + ...scope.args, + limitWith(newAllowance, excluded), + ]); + + return { + // Learning leads: a graduated load bigger than the learner's stamina would + // otherwise mean nothing in learning or new ever advances. + dueRows: [ + ...withoutExcluded(learning, excludeIds, learningCap), + ...withoutExcluded(graduated, excludeIds, UNLIMITED), + ], + newRows: withoutExcluded(unseen, excludeIds, newAllowance), + }; +} diff --git a/db/simple-relearning-migration.test.ts b/db/simple-relearning-migration.test.ts new file mode 100644 index 0000000..1df8ee1 --- /dev/null +++ b/db/simple-relearning-migration.test.ts @@ -0,0 +1,94 @@ +/** + * The backfill that comes with `simple_relearning`. + * + * Cards sitting in a lapse when the column lands are relearning by definition, + * and the column's `DEFAULT 0` would quietly forgive their halving. Cards that + * merely failed at some point in the past must NOT be marked — that lifetime + * reading is the bug this column exists to end. + */ +import Database from "better-sqlite3"; +import { describe, expect, it } from "vitest"; + +import { USER_DB_MIGRATIONS } from "./user-migrations"; + +/** + * Split at the statements this change added, found by what they say rather than + * by counting from the end of the array — later phases append more migrations, + * and a `slice(-2)` silently starts testing the wrong pair. + */ +const ADD_COLUMN = USER_DB_MIGRATIONS.findIndex((sql) => + /ALTER TABLE srs_cards ADD COLUMN simple_relearning/.test(sql), +); +const BACKFILL = USER_DB_MIGRATIONS.findIndex((sql) => + /UPDATE srs_cards SET simple_relearning = 1/.test(sql), +); +if (ADD_COLUMN < 0 || BACKFILL !== ADD_COLUMN + 1) { + throw new Error("the simple_relearning migrations are not where this test expects them"); +} +const BEFORE = USER_DB_MIGRATIONS.slice(0, ADD_COLUMN); +const ADDED = USER_DB_MIGRATIONS.slice(ADD_COLUMN, BACKFILL + 1); + +function apply(db: Database.Database, statements: readonly string[]) { + for (const sql of statements) { + try { + db.exec(sql); + } catch (err) { + if (!/duplicate column/i.test(String(err))) throw err; + } + } +} + +function card(id: string, stage: number | null, lapses: number) { + return `INSERT INTO srs_cards (id, entry_id, list_id, due, stability, difficulty, elapsed_days, + scheduled_days, reps, lapses, state, front_mode, back_mode, simple_stage, simple_n, + simple_interval, created_at, updated_at) + VALUES ('${id}', 1, 'l', 'x', 0, 0, 0, 0, 9, ${lapses}, 0, 'kanji', 'english', + ${stage === null ? "NULL" : stage}, 0, 56.61, 'created', 'OLD')`; +} + +describe("the simple_relearning backfill", () => { + const migrated = () => { + const db = new Database(":memory:"); + db.pragma("foreign_keys = OFF"); + apply(db, BEFORE); + db.exec(card("mid-lapse", 0, 4)); + db.exec(card("graduated", 1, 4)); + db.exec(card("never-seen", null, 0)); + db.exec(card("deleted-mid-lapse", 0, 4)); + db.exec("UPDATE srs_cards SET deleted_at = 'gone' WHERE id = 'deleted-mid-lapse'"); + apply(db, ADDED); + return db; + }; + + it("marks a card that is in a lapse right now", () => { + const row = migrated().prepare("SELECT * FROM srs_cards WHERE id = 'mid-lapse'").get() as any; + expect(row.simple_relearning).toBe(1); + }); + + it("leaves a graduated card alone however often it has failed", () => { + const row = migrated().prepare("SELECT * FROM srs_cards WHERE id = 'graduated'").get() as any; + expect(row.simple_relearning).toBe(0); + expect(row.updated_at).toBe("OLD"); + }); + + it("leaves an unseen card at zero", () => { + const row = migrated().prepare("SELECT * FROM srs_cards WHERE id = 'never-seen'").get() as any; + expect(row.simple_relearning).toBe(0); + }); + + it("leaves updated_at alone, so a stale device cannot win last-write-wins", () => { + // Every device runs this statement over its own rows, so the backfill does + // not need to travel. Bumping the timestamp would make a device that has + // not been opened in months the newest writer of every lapsed card: sync + // skips the remote row when `localTs >= remoteTs` and pushes the stale one. + const row = migrated().prepare("SELECT * FROM srs_cards WHERE id = 'mid-lapse'").get() as any; + expect(row.updated_at).toBe("OLD"); + }); + + it("leaves a soft-deleted card alone, so it cannot be resurrected by a push", () => { + const db = migrated(); + const row = db.prepare("SELECT * FROM srs_cards WHERE id = 'deleted-mid-lapse'").get() as any; + expect(row.simple_relearning).toBe(0); + expect(row.updated_at).toBe("OLD"); + }); +}); diff --git a/db/simple-repair.test.ts b/db/simple-repair.test.ts new file mode 100644 index 0000000..faef57c --- /dev/null +++ b/db/simple-repair.test.ts @@ -0,0 +1,343 @@ +/** + * The repair of decks the lapse bug flattened. + * + * Fixing the rule does not fix the stored intervals: a card the bug ratcheted + * is sitting at the 8-hour floor and comes back every day forever. Measured at + * 200 answers a day, 3,579 of 8,000 such cards are still queued a year later + * without a repair and 47 with one — `yarn simulate:srs drain`, and the + * decisions are in docs/simple-srs-decisions.md. + * + * What must NOT move: a card that has barely been seen, a card on a healthy + * interval, an FSRS-only card, and any learning card — a card at stage 0 sits + * at 1/3 because that is where new cards start, and with the whole list loading + * as new those are most of the stage-0 population. + * + * It runs once per device, so the statement itself is still written to be + * idempotent: the flag is the mechanism, the spread starting at 2 is the belt. + */ +import Database from "better-sqlite3"; +import { describe, expect, it } from "vitest"; + +import { USER_DB_MIGRATIONS } from "./user-migrations"; +import { REPAIR_RATCHETED_INTERVALS, repairRatchetedIntervals } from "./simple-repair"; +import { createTestDb } from "@/test/test-db"; + +const BEFORE = USER_DB_MIGRATIONS; +const REPAIR = [REPAIR_RATCHETED_INTERVALS]; + +function apply(db: Database.Database, statements: readonly string[]) { + for (const sql of statements) { + try { + db.exec(sql); + } catch (err) { + if (!/duplicate column/i.test(String(err))) throw err; + } + } +} + +type Card = { + id: string; + stage: number | null; + n: number | null; + interval: number | null; + reps: number; + lapses: number; + relearning?: number; + run?: number; + deleted?: boolean; +}; + +function insert(db: Database.Database, c: Card) { + db.exec(`INSERT INTO srs_cards (id, entry_id, list_id, due, stability, difficulty, elapsed_days, + scheduled_days, reps, lapses, state, front_mode, back_mode, simple_stage, simple_n, + simple_interval, simple_relearning, simple_run, created_at, updated_at) + VALUES ('${c.id}', 1, 'l', 'x', 0, 0, 0, 0, ${c.reps}, ${c.lapses}, 0, 'kanji', 'english', + ${c.stage ?? "NULL"}, ${c.n ?? "NULL"}, ${c.interval ?? "NULL"}, + ${c.relearning ?? 0}, ${c.run ?? 0}, 'created', 'OLD')`); + if (c.deleted) db.exec(`UPDATE srs_cards SET deleted_at = 'gone' WHERE id = '${c.id}'`); +} + +const FLOOR = 1 / 3; + +/** Days since 2001-01-01, the epoch `simple_n` is in. */ +const today = () => (Date.now() - Date.parse("2001-01-01T00:00:00Z")) / 86_400_000; + +function repaired() { + const db = new Database(":memory:"); + db.pragma("foreign_keys = OFF"); + apply(db, BEFORE); + insert(db, { id: "ratcheted", stage: 1, n: 0, interval: FLOOR, reps: 40, lapses: 5 }); + insert(db, { + id: "mid-lapse", + stage: 0, + n: 0, + interval: FLOOR, + reps: 40, + lapses: 6, + relearning: 1, + run: 2, + }); + insert(db, { id: "barely-seen", stage: 1, n: 9000, interval: FLOOR, reps: 4, lapses: 1 }); + // The reps threshold is the measured discriminator, so both sides of it are + // pinned: 11 must not move, 12 must. + insert(db, { id: "reps-11", stage: 1, n: 0, interval: FLOOR, reps: 11, lapses: 2 }); + insert(db, { id: "reps-12", stage: 1, n: 0, interval: FLOOR, reps: 12, lapses: 2 }); + // Nothing in the engine writes a stage above 1, and the clause must not + // reach one if anything ever does. + insert(db, { id: "stage-2", stage: 2, n: 0, interval: FLOOR, reps: 40, lapses: 5 }); + insert(db, { id: "never-failed", stage: 1, n: 9000, interval: FLOOR, reps: 40, lapses: 0 }); + insert(db, { id: "healthy", stage: 1, n: 9100, interval: 56.61, reps: 40, lapses: 2 }); + insert(db, { id: "part-way-down", stage: 1, n: 0, interval: 0.88, reps: 40, lapses: 5 }); + // Just above the clause: 0.4 is a halving step, and still left to climb. + insert(db, { id: "just-above", stage: 1, n: 0, interval: 0.4, reps: 40, lapses: 5 }); + // A list moved from FSRS to simple SRS carries its FSRS reps and lapses. + insert(db, { id: "was-fsrs", stage: 1, n: 0, interval: 1 / 3, reps: 40, lapses: 5 }); + insert(db, { + id: "deleted", + stage: 1, + n: 0, + interval: FLOOR, + reps: 40, + lapses: 5, + deleted: true, + }); + insert(db, { id: "unseen", stage: null, n: null, interval: null, reps: 0, lapses: 0 }); + insert(db, { id: "fsrs-only", stage: null, n: null, interval: null, reps: 5, lapses: 1 }); + apply(db, REPAIR); + return db; +} + +const row = (db: Database.Database, id: string) => + db.prepare("SELECT * FROM srs_cards WHERE id = ?").get(id) as Record; + +describe("the interval repair", () => { + it("lifts a ratcheted card off the floor and gives it a date inside the next month", () => { + const r = row(repaired(), "ratcheted"); + expect(r.simple_interval).toBeGreaterThanOrEqual(2); + expect(r.simple_interval).toBeLessThanOrEqual(30); + expect(Number(r.simple_n) - today()).toBeGreaterThanOrEqual(2); + expect(Number(r.simple_n) - today()).toBeLessThanOrEqual(30); + }); + + it("leaves updated_at alone, so a stale device cannot win last-write-wins", () => { + // Each device repairs its own rows; a bumped timestamp would let a device + // opened for the first time in months push its values over another's + // reviews, because sync skips the remote row when `localTs >= remoteTs`. + expect(row(repaired(), "ratcheted").updated_at).toBe("OLD"); + }); + + it("leaves a card part-way down the ratchet to climb on its own", () => { + // 0.88 days is not the floor: with the rule fixed it multiplies to 1.67, + // 3.2, 6.1 … by itself. The clause is for cards PINNED at the floor, where + // a card honestly at 0.88 after one lapse is indistinguishable from a + // victim — at 75% accuracy, including them doubles the cards wrongly lifted. + expect(row(repaired(), "part-way-down").simple_interval).toBe(0.88); + }); + + it("leaves a soft-deleted card alone rather than rewriting and pushing it", () => { + expect(row(repaired(), "deleted").simple_interval).toBeCloseTo(FLOOR); + }); + + it("leaves a card with too few answers behind it alone", () => { + // Honestly at the floor: a young card that failed once and re-graduated + // lands there. Measured, those have a median of 4 answers; a card the + // ratchet dragged down from 56 days has 32. + expect(row(repaired(), "barely-seen").simple_interval).toBeCloseTo(FLOOR); + }); + + it("leaves every learning card alone, mid-lapse or never graduated", () => { + const r = row(repaired(), "mid-lapse"); + // A stage-0 card is at 1/3 because that is where new cards start, and the + // 8,000-card queue meant most of the deck never left stage 0. Lifting them + // would hand unlearned words a fortnight; a genuinely ratcheted one climbs + // 0.33 → 0.63 → 1.2 on its own once the rule is fixed. + expect(r.simple_interval).toBeCloseTo(FLOOR); + expect(r.simple_n).toBe(0); + expect(r.updated_at).toBe("OLD"); + }); + + it("holds the measured reps threshold at exactly twelve", () => { + const db = repaired(); + expect(row(db, "reps-11").simple_interval).toBeCloseTo(FLOOR); + expect(Number(row(db, "reps-12").simple_interval)).toBeGreaterThanOrEqual(2); + }); + + it("does not reach a stage the engine never writes", () => { + expect(row(repaired(), "stage-2").simple_interval).toBeCloseTo(FLOOR); + }); + + it("leaves a card one halving above the clause alone", () => { + expect(row(repaired(), "just-above").simple_interval).toBe(0.4); + }); + + it("leaves a card that has never failed alone — the bug cannot have touched it", () => { + expect(row(repaired(), "never-failed").simple_interval).toBeCloseTo(FLOOR); + }); + + it("leaves a healthy interval and an FSRS-only card alone", () => { + const db = repaired(); + expect(row(db, "healthy").simple_interval).toBe(56.61); + expect(row(db, "fsrs-only").simple_interval).toBe(null); + }); + + it("rewrites the schedule and nothing else", () => { + const r = row(repaired(), "ratcheted"); + // History is not the schedule: a repair that reset these would erase the + // evidence the next repair, and the statistics screen, read. + expect(r.reps).toBe(40); + expect(r.lapses).toBe(5); + expect(r.simple_stage).toBe(1); + expect(r.simple_relearning).toBe(0); + expect(r.simple_run).toBe(0); + expect(r.created_at).toBe("created"); + expect(r.updated_at).toBe("OLD"); + }); + + // Recorded, not asserted as desirable: `reps` is not per-mode, so a list + // switched from FSRS to simple SRS brings its FSRS count with it and its cards + // qualify after one simple graduation. Nothing in the row can tell them apart. + it("lifts a card whose reps were earned under FSRS", () => { + expect(Number(row(repaired(), "was-fsrs").simple_interval)).toBeGreaterThanOrEqual(2); + }); + + it("spreads the deck instead of stacking it on one day", () => { + const db = new Database(":memory:"); + db.pragma("foreign_keys = OFF"); + apply(db, BEFORE); + for (let i = 0; i < 300; i++) { + insert(db, { id: `c${i}`, stage: 1, n: 0, interval: FLOOR, reps: 40, lapses: 5 }); + } + apply(db, REPAIR); + const spread = db + .prepare( + `SELECT count(DISTINCT simple_interval) d, min(simple_interval) lo, + max(simple_interval) hi FROM srs_cards`, + ) + .get() as { d: number; lo: number; hi: number }; + expect(spread.d).toBe(29); + expect(spread.lo).toBe(2); + expect(spread.hi).toBe(30); + }); + + it("is a no-op the second time, so it can run on every launch", () => { + const db = repaired(); + const before = row(db, "ratcheted"); + apply(db, REPAIR); + expect(row(db, "ratcheted")).toEqual(before); + }); +}); + +describe("repairRatchetedIntervals", () => { + it("reports how many cards it lifted, and never bumps updated_at", async () => { + const db = createTestDb(); + const now = "2026-04-05T00:00:00.000Z"; + for (const [id, interval, reps] of [ + ["ratcheted", 1 / 3, 40], + ["honest", 1 / 3, 4], + ["healthy", 56.61, 40], + ] as const) { + await db.runAsync( + `INSERT INTO srs_cards (id, entry_id, list_id, due, stability, difficulty, elapsed_days, + scheduled_days, reps, lapses, state, simple_stage, simple_n, simple_interval, + created_at, updated_at) + VALUES (?, 1, NULL, 'x', 0, 0, 0, 0, ?, 5, 0, 1, 0, ?, ?, 'OLD')`, + [id, reps, interval, now], + ); + } + + expect(await repairRatchetedIntervals(db)).toBe(1); + + const row = await db.getFirstAsync<{ simple_interval: number; updated_at: string }>( + "SELECT simple_interval, updated_at FROM srs_cards WHERE id = 'ratcheted'", + ); + expect(row!.simple_interval).toBeGreaterThanOrEqual(2); + // Never bumped: a stale device would otherwise win last-write-wins and push + // months-old values over reviews done on another device. + expect(row!.updated_at).toBe("OLD"); + }); +}); + +describe("the repair runs once per device", () => { + async function deck() { + const db = createTestDb(); + const now = "2026-04-05T00:00:00.000Z"; + await db.runAsync( + `INSERT INTO srs_cards (id, entry_id, list_id, due, stability, difficulty, elapsed_days, + scheduled_days, reps, lapses, state, simple_stage, simple_n, simple_interval, + created_at, updated_at) + VALUES ('r', 1, NULL, 'x', 0, 0, 0, 0, 40, 5, 0, 1, 0, 0.3333, ?, 'OLD')`, + [now], + ); + return db; + } + + it("lifts on the first launch and does nothing on the next", async () => { + const db = await deck(); + expect(await repairRatchetedIntervals(db)).toBe(1); + // -1, not 0: the flag said it had already run, so the clause was never + // even evaluated. + expect(await repairRatchetedIntervals(db)).toBe(-1); + }); + + it("does not re-lift a card the fixed engine honestly put back at the floor", async () => { + // Three honest lapses leave a card at exactly MIN_INTERVAL with reps 15 and + // lapses 3 — every condition of the clause, with no bug involved. Running + // the repair on every launch would bury the learner's hardest words again + // and again; this is why it is flag-keyed. + const db = await deck(); + await repairRatchetedIntervals(db); + await db.runAsync( + "UPDATE srs_cards SET simple_interval = 0.3333, simple_n = 0, reps = 15, lapses = 3 WHERE id = 'r'", + ); + expect(await repairRatchetedIntervals(db)).toBe(-1); + expect( + await db.getFirstAsync<{ simple_interval: number }>( + "SELECT simple_interval FROM srs_cards WHERE id = 'r'", + ), + ).toEqual({ simple_interval: 0.3333 }); + }); + + it("claims nothing on an empty database, so a reinstall still gets repaired", async () => { + // A fresh install has no cards: the deck arrives on the next sync pull, and + // the repair never pushes, so if the flag were spent here the deck would + // stay flattened forever on every device. + const db = createTestDb(); + expect(await repairRatchetedIntervals(db)).toBe(0); + expect(await repairRatchetedIntervals(db)).toBe(0); + + const now = "2026-04-05T00:00:00.000Z"; + await db.runAsync( + `INSERT INTO srs_cards (id, entry_id, list_id, due, stability, difficulty, elapsed_days, + scheduled_days, reps, lapses, state, simple_stage, simple_n, simple_interval, + created_at, updated_at) + VALUES ('pulled', 1, NULL, 'x', 0, 0, 0, 0, 40, 5, 0, 1, 0, 0.3333, ?, 'OLD')`, + [now], + ); + expect(await repairRatchetedIntervals(db)).toBe(1); + expect(await repairRatchetedIntervals(db)).toBe(-1); + }); + + it("settles on a healthy deck after one run", async () => { + const db = createTestDb(); + const now = "2026-04-05T00:00:00.000Z"; + await db.runAsync( + `INSERT INTO srs_cards (id, entry_id, list_id, due, stability, difficulty, elapsed_days, + scheduled_days, reps, lapses, state, simple_stage, simple_n, simple_interval, + created_at, updated_at) + VALUES ('healthy', 1, NULL, 'x', 0, 0, 0, 0, 40, 0, 0, 1, 9000, 56.61, ?, 'OLD')`, + [now], + ); + expect(await repairRatchetedIntervals(db)).toBe(0); + expect(await repairRatchetedIntervals(db)).toBe(-1); + }); + + it("leaves the flag unwritten when it fails, so the next launch retries", async () => { + const broken = { + getFirstAsync: async () => null, + runAsync: async () => { + throw new Error("no such table: srs_cards"); + }, + } as unknown as Parameters[0]; + await expect(repairRatchetedIntervals(broken)).resolves.toBe(0); + }); +}); diff --git a/db/simple-repair.ts b/db/simple-repair.ts new file mode 100644 index 0000000..0aa64e1 --- /dev/null +++ b/db/simple-repair.ts @@ -0,0 +1,89 @@ +/** + * Lift simple-SRS cards the lapse bug ratcheted to the 8-hour floor. + * + * ONCE per device, keyed to an `app_flags` row. Both halves of that matter, and + * both were got wrong first: + * + * - It cannot bump `updated_at`. A device opened for the first time in months + * would become the newest writer of every card it touched, and sync skips the + * remote row when `localTs >= remoteTs` — so it would push months-old values + * over reviews done elsewhere. + * - It cannot run on every launch. The clause matches rows the FIXED engine + * legitimately produces: a card that fails and re-graduates from a short + * interval lands at exactly `MIN_INTERVAL`, so three honest lapses leave it at + * 1/3 with `reps = 15` and `lapses = 3`, matching every condition. As a + * one-time repair that is a bounded cost, measured and accepted in + * docs/simple-srs-decisions.md. Run on every launch it becomes a standing + * policy that pushes a learner's hardest cards a month out, again and again. + * + * The cost of running once is that a device still on an old build can push a + * ratcheted interval back afterwards, and nothing lifts it again. That is the + * better trade: one stale card against repeatedly burying the words someone is + * working hardest on. + */ + +import type { WrappedUserDb } from "./user-db"; + +/** The `app_flags` key that records this device has run the repair. */ +export const REPAIR_FLAG = "simple_intervals_repaired"; + +/** + * `reps >= 12` is the discriminator, measured rather than guessed: a card the + * bug ratcheted down from 56 days has a median of 32 answers behind it, while + * one honestly at the floor has 4. `lapses >= 3` was the obvious guard and is + * wrong — under the bug a single lapse starts the ratchet, so it misses a third + * of them and lifts more honest cards. `yarn simulate:srs discriminate`. + * + * `simple_stage = 1` only: a learning card sits at 1/3 because that is where + * new cards START, and with the whole list loading as new those are most of the + * stage-0 population. They are bounded by the learning cap instead. + * + * The spread starts at 2 so a repaired row can never match again, which keeps + * the statement idempotent even though the flag already stops it re-running. + */ +export const REPAIR_RATCHETED_INTERVALS = `UPDATE srs_cards SET + simple_interval = 2 + (rowid % 29), + simple_n = (julianday('now') - julianday('2001-01-01')) + 2 + (rowid % 29) + WHERE simple_stage = 1 AND simple_interval <= 0.34 AND lapses > 0 AND reps >= 12 + AND deleted_at IS NULL`; + +/** + * Run the repair if this device has not. Returns how many cards were lifted, or + * -1 if it had already run. + * + * "Has not" means the flag is absent, and the flag is only written once the + * device has actually seen a simple-SRS deck — see below. + */ +export async function repairRatchetedIntervals(db: WrappedUserDb): Promise { + try { + const done = await db.getFirstAsync<{ value: string }>( + "SELECT value FROM app_flags WHERE key = ?", + [REPAIR_FLAG], + ); + if (done) return -1; + + const result = await db.runAsync(REPAIR_RATCHETED_INTERVALS, []); + + // Only claim the repair is done once there was a deck to repair. A fresh + // install has no cards yet: writing the flag then would spend the one run + // on an empty table, and the ratcheted deck arriving on the next sync pull + // would stay flattened forever, with no path back — the repair never + // pushes, so the remote copy is flattened too. A reinstall is the common + // way a user meets this, not an edge case. + const seen = await db.getFirstAsync<{ c: number }>( + "SELECT COUNT(*) as c FROM srs_cards WHERE simple_stage IS NOT NULL", + ); + if ((seen?.c ?? 0) > 0) { + await db.runAsync("INSERT OR REPLACE INTO app_flags (key, value) VALUES (?, ?)", [ + REPAIR_FLAG, + new Date().toISOString(), + ]); + } + return result?.changes ?? 0; + } catch (err) { + // A deck left at the floor is bad; a launch that fails because of it is + // worse. The flag is not written, so the next launch tries again. + console.warn("[SimpleSRS] Interval repair skipped:", err); + return 0; + } +} diff --git a/db/sql-arity.test.ts b/db/sql-arity.test.ts new file mode 100644 index 0000000..cff9198 --- /dev/null +++ b/db/sql-arity.test.ts @@ -0,0 +1,163 @@ +/** + * Every literal SQL call must bind as many values as it has placeholders. + * + * op-sqlite does not check the count: a short args array binds NULL for the + * missing tail, so `SET a = ?, b = ?` with one value writes the timestamp into + * `a` and NULL into the WHERE id — zero rows, no error, and the store still + * mutates so the UI looks saved. That shipped once in the flashcard settings + * modal, which is why this gate counts them instead of a reviewer. + * + * What it cannot see, so that nobody reads a pass as more than it is: args + * assembled in a variable rather than written inline, and arrays carrying a + * spread — which is every scoped query in the study screen, `[...scope.args, + * x]`, because the spread's length is not knowable here. Interpolated SQL is + * skipped too, since a fragment helper may bring placeholders of its own. + */ +import { describe, expect, it } from "vitest"; +import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import ts from "typescript"; + +const QUERY_METHODS = new Set(["runAsync", "getAllAsync", "getFirstAsync", "execAsync"]); + +// `fileURLToPath`, not `.pathname`: a checkout path containing a space +// arrives percent-encoded and `git -C` is handed a directory that does not +// exist, so the gate fails before it scans a single query. +const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), ".."); + +function sourceFiles(): string[] { + return execFileSync( + "git", + ["-C", REPO_ROOT, "ls-files", "app", "components", "db", "lib", "stores", "hooks"], + { encoding: "utf8" }, + ) + .split("\n") + .filter((f) => /\.(ts|tsx)$/.test(f) && !/\.test\.tsx?$/.test(f)); +} + +/** `?` placeholders in a SQL string, ignoring any inside a quoted literal. */ +/** Drop `--` and block comments, so a `don't` in one cannot open quote state. */ +function stripComments(sql: string): string { + return sql.replace(/--[^\n]*/g, "").replace(/\/\*[\s\S]*?\*\//g, ""); +} + +function countPlaceholders(raw: string): number { + const sql = stripComments(raw); + let count = 0; + let quote: string | null = null; + for (let i = 0; i < sql.length; i++) { + const ch = sql[i]; + if (quote) { + if (ch === quote) quote = null; + continue; + } + if (ch === "'" || ch === '"') quote = ch; + else if (ch === "?") count++; + } + return count; +} + +type Mismatch = { file: string; line: number; sql: number; args: number }; + +/** Call sites this gate can check, against every call site there is. */ +export function coverage(): { checked: number; total: number } { + let checked = 0; + let total = 0; + for (const file of sourceFiles()) { + const text = readFileSync(join(REPO_ROOT, file), "utf8"); + const source = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true); + const visit = (node: ts.Node) => { + if (ts.isCallExpression(node) && ts.isPropertyAccessExpression(node.expression)) { + if (QUERY_METHODS.has(node.expression.name.text)) { + total++; + const [sqlArg, argsArg] = node.arguments; + const literal = + sqlArg && + (ts.isStringLiteralLike(sqlArg) || ts.isNoSubstitutionTemplateLiteral(sqlArg)); + if ( + literal && + argsArg && + ts.isArrayLiteralExpression(argsArg) && + !argsArg.elements.some((e) => ts.isSpreadElement(e)) + ) { + checked++; + } + } + } + ts.forEachChild(node, visit); + }; + visit(source); + } + return { checked, total }; +} + +function mismatchesIn(file: string): Mismatch[] { + const text = readFileSync(join(REPO_ROOT, file), "utf8"); + const source = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true); + const found: Mismatch[] = []; + + const visit = (node: ts.Node) => { + if (ts.isCallExpression(node) && ts.isPropertyAccessExpression(node.expression)) { + const method = node.expression.name.text; + const [sqlArg, argsArg] = node.arguments; + if ( + QUERY_METHODS.has(method) && + sqlArg && + argsArg && + ts.isArrayLiteralExpression(argsArg) && + !argsArg.elements.some((e) => ts.isSpreadElement(e)) + ) { + // Only a literal SQL string can be counted; an interpolated one may + // carry placeholders from a fragment helper. + const sql = ts.isStringLiteralLike(sqlArg) + ? sqlArg.text + : ts.isNoSubstitutionTemplateLiteral(sqlArg) + ? sqlArg.text + : null; + if (sql != null) { + const placeholders = countPlaceholders(sql); + if (placeholders !== argsArg.elements.length) { + found.push({ + file, + line: source.getLineAndCharacterOfPosition(node.getStart(source)).line + 1, + sql: placeholders, + args: argsArg.elements.length, + }); + } + } + } + } + ts.forEachChild(node, visit); + }; + + visit(source); + return found; +} + +describe("SQL placeholder arity", () => { + it("binds one value per placeholder in every literal query", () => { + const files = sourceFiles(); + expect(files.length).toBeGreaterThan(100); + const mismatches = files.flatMap(mismatchesIn); + expect( + mismatches.map((m) => `${m.file}:${m.line} ${m.sql} placeholders, ${m.args} args`), + ).toEqual([]); + }); + + it("reports how much of the codebase it can actually see", () => { + // So a passing run is never read as "every query is checked". Quoting this + // share anywhere means running this test; it has been quoted wrong twice. + const { checked, total } = coverage(); + expect(total).toBeGreaterThan(checked); + console.log(`[sql-arity] checks ${checked} of ${total} query call sites`); + }); + + it("counts placeholders outside quoted literals and comments only", () => { + expect(countPlaceholders("SET a = ?, b = ? WHERE id = ?")).toBe(3); + expect(countPlaceholders("SET note = 'what? why?' WHERE id = ?")).toBe(1); + expect(countPlaceholders("-- don't ask why?\nSET a = ?")).toBe(1); + expect(countPlaceholders("/* why? */ SET a = ?")).toBe(1); + }); +}); diff --git a/db/srs-card-columns.test.ts b/db/srs-card-columns.test.ts new file mode 100644 index 0000000..1b8b130 --- /dev/null +++ b/db/srs-card-columns.test.ts @@ -0,0 +1,99 @@ +/** + * The hydration, which nothing else gates. + * + * The lapse fix turns on one alias surviving the SELECT: drop + * `simple_relearning as simpleRelearning` and `card.simpleRelearning` is + * `undefined`, so `=== 1` is false, the halving never fires, and every unit + * test in stores/simple-srs.test.ts still passes. This reads a row back the + * way the study screen does and hands it to the real decision. + */ +import Database from "better-sqlite3"; +import { describe, expect, it } from "vitest"; + +import { USER_DB_MIGRATIONS } from "./user-migrations"; +import { SRS_CARD_SELECT } from "./srs-card-columns"; +import { nextSimpleReview } from "@/stores/simple-srs"; +import type { SrsCardRow } from "./types"; + +function dbWithCard(relearning: number): Database.Database { + const db = new Database(":memory:"); + db.pragma("foreign_keys = OFF"); + for (const sql of USER_DB_MIGRATIONS) { + try { + db.exec(sql); + } catch (err) { + if (!/duplicate column/i.test(String(err))) throw err; + } + } + db.exec(`INSERT INTO srs_cards (id, entry_id, list_id, due, stability, difficulty, elapsed_days, + scheduled_days, reps, lapses, state, front_mode, back_mode, simple_stage, simple_n, + simple_interval, simple_relearning, created_at, updated_at) + VALUES ('c', 1, 'l', 'x', 0, 0, 0, 0, 9, 3, 0, 'kanji', 'english', 1, 0, 56.61, + ${relearning}, 'now', 'now')`); + return db; +} + +const read = (db: Database.Database) => + db.prepare(`${SRS_CARD_SELECT} FROM srs_cards WHERE id = 'c'`).get() as SrsCardRow; + +describe("a card read back the way the study screen reads it", () => { + it("carries simpleRelearning, so a card in a lapse halves once", () => { + const card = read(dbWithCard(1)); + expect(card.simpleRelearning).toBe(1); + expect(nextSimpleReview(card, "pass").updates.simpleInterval).toBeCloseTo(28.305, 3); + }); + + it("and a card that merely failed long ago keeps growing", () => { + const card = read(dbWithCard(0)); + expect(card.simpleRelearning).toBe(0); + expect(card.lapses).toBe(3); // the lifetime counter the bug used to read + expect(nextSimpleReview(card, "pass").updates.simpleInterval).toBeCloseTo(56.61 * 1.9, 2); + }); +}); + +/** + * The count that used to die with the screen. + * + * A learning card needs three correct answers in a row, and until it has them + * its `simple_n` is 0 — permanently due. The count lived in a React ref, so a + * card answered twice and left came back to zero: with a deck bigger than one + * sitting, no card ever reached three and the queue never shrank. + */ +describe("the correct-in-a-row count survives a reload", () => { + function learningCard(run: number): Database.Database { + const db = new Database(":memory:"); + db.pragma("foreign_keys = OFF"); + for (const sql of USER_DB_MIGRATIONS) { + try { + db.exec(sql); + } catch (err) { + if (!/duplicate column/i.test(String(err))) throw err; + } + } + db.exec(`INSERT INTO srs_cards (id, entry_id, list_id, due, stability, difficulty, + elapsed_days, scheduled_days, reps, lapses, state, front_mode, back_mode, simple_stage, + simple_n, simple_interval, simple_relearning, simple_run, created_at, updated_at) + VALUES ('c', 1, 'l', 'x', 0, 0, 0, 0, 2, 0, 0, 'kanji', 'english', 0, 0, 0.3333, 0, + ${run}, 'now', 'now')`); + return db; + } + + it("counts up instead of restarting", () => { + const card = read(learningCard(2)); + expect(card.simpleRun).toBe(2); + // Third correct answer in a row: this one graduates. + expect(nextSimpleReview(card, "pass").graduated).toBe(true); + }); + + it("a card answered once before is not treated as untouched", () => { + const card = read(learningCard(1)); + const step = nextSimpleReview(card, "pass"); + expect(step.graduated).toBe(false); + expect(step.updates.simpleRun).toBe(2); + }); + + it("a fail puts the count back to zero", () => { + const card = read(learningCard(2)); + expect(nextSimpleReview(card, "fail").updates.simpleRun).toBe(0); + }); +}); diff --git a/db/srs-card-columns.ts b/db/srs-card-columns.ts new file mode 100644 index 0000000..0a1da61 --- /dev/null +++ b/db/srs-card-columns.ts @@ -0,0 +1,20 @@ +/** + * The columns a queued SRS card is read with, aliased to `SrsCardRow`. + * + * Shared so a test can hydrate a row exactly as the study screen does. The + * lapse fix turns on `simple_relearning` surviving this list: drop the alias + * and `card.simpleRelearning` is `undefined`, the halving never fires, and + * every unit test still passes. + */ +export const SRS_CARD_SELECT = `SELECT id, entry_id as entryId, kanji_literal as kanjiLiteral, list_id as listId, due, + stability, difficulty, elapsed_days as elapsedDays, + scheduled_days as scheduledDays, reps, lapses, state, + last_review as lastReview, front_mode as frontMode, + back_mode as backMode, created_at as createdAt, + updated_at as updatedAt, + simple_stage as simpleStage, simple_n as simpleN, + simple_interval as simpleInterval, + simple_relearning as simpleRelearning, + simple_run as simpleRun, + last_confusion_check as lastConfusionCheck, + learning_steps as learningSteps`; diff --git a/db/types.ts b/db/types.ts index 1445309..d5f214a 100644 --- a/db/types.ts +++ b/db/types.ts @@ -157,6 +157,8 @@ export interface WordList { disableSwipeAnimation: boolean; learningSteps: string[] | null; relearningSteps: string[] | null; + /** Unseen cards a simple-SRS session may introduce. Absent reads as the default. */ + newPerSession?: number | null; isDefault: boolean; createdAt: string; updatedAt: string; @@ -218,6 +220,10 @@ export interface SrsCardRow { simpleStage: number | null; simpleN: number | null; simpleInterval: number | null; + /** 1 while the card is recovering from a lapse — see stores/simple-srs.ts. */ + simpleRelearning: number | null; + /** Correct answers in a row so far, toward the three a learning card needs. */ + simpleRun: number | null; // Confusion detection cooldown lastConfusionCheck: string | null; // FSRS learning step index diff --git a/db/user-migrations.ts b/db/user-migrations.ts index a0912d6..68b7c16 100644 --- a/db/user-migrations.ts +++ b/db/user-migrations.ts @@ -248,4 +248,39 @@ export const USER_DB_MIGRATIONS = [ // Ask a kanji card by blanking the keyword out of the learner's own story // (lib/rtk-cloze.ts) instead of showing the character. `ALTER TABLE lists ADD COLUMN mnemonic_cloze INTEGER NOT NULL DEFAULT 0`, + // Whether a simple-SRS card is recovering from a lapse RIGHT NOW. The halved + // re-graduation used `lapses > 0`, a lifetime counter, so every pass after a + // card's first failure halved its interval and the card ratcheted down to the + // 8-hour floor and never left the daily queue again. + `ALTER TABLE srs_cards ADD COLUMN simple_relearning INTEGER DEFAULT 0`, + // Cards sitting in a lapse when this lands are relearning by definition. + // + // `updated_at` is deliberately NOT touched. Every device runs this same + // statement over its own rows, so the backfill does not need to travel — and + // bumping the timestamp would make a device that has not been opened in + // months the newest writer of every lapsed card, so `sync` would skip the + // remote row (`localTs >= remoteTs`) and push stale values over reviews done + // elsewhere. The soft-delete guard is here for the same reason: a bumped row + // would win and push its `deleted_at = NULL` back, resurrecting the card. + `UPDATE srs_cards SET simple_relearning = 1 + WHERE simple_stage = 0 AND lapses > 0 AND deleted_at IS NULL`, + // How many correct answers in a row a learning card has. It lived in a React + // ref, so it died with the screen: a card answered twice and left was back to + // zero, and with `simple_n = 0` it stayed due forever. Nullable because a + // restore from a backup that predates the column binds an explicit NULL for + // it — `INSERT OR REPLACE` does NOT fall back to the column default for a + // named NULL, so NOT NULL here would fail the whole restore. + `ALTER TABLE srs_cards ADD COLUMN simple_run INTEGER DEFAULT 0`, + // How many unseen cards a simple-SRS session may introduce. The session used + // to load the whole list at once, so an 8,000-word list put 8,000 cards in + // front of the learner, none of them could reach the three correct answers a + // card needs, and the same queue came back every day. Midori, which this + // engine clones, mixes in about ten. + `ALTER TABLE lists ADD COLUMN new_per_session INTEGER DEFAULT 10`, ]; + +// The repair of already-ratcheted intervals is deliberately NOT here. It is +// keyed to an `app_flags` row rather than `PRAGMA user_version`, and the flag is +// only written once the device has seen a simple-SRS deck — a fresh install +// would otherwise spend its one run on an empty table and never repair the deck +// that arrives on the next sync. See `db/simple-repair.ts`. diff --git a/db/user-provider.native.tsx b/db/user-provider.native.tsx index c84ed30..2a14536 100644 --- a/db/user-provider.native.tsx +++ b/db/user-provider.native.tsx @@ -3,6 +3,7 @@ import { open } from "@op-engineering/op-sqlite"; import type { DB } from "@op-engineering/op-sqlite"; import { wrapUserDb, type WrappedUserDb } from "./user-db"; import { USER_DB_MIGRATIONS } from "./user-migrations"; +import { repairRatchetedIntervals } from "./simple-repair"; import { migrateLegacyMnemonics } from "./mnemonic-migration"; import { makeDefaultListId } from "@/lib/seed-default-lists"; import { cleanupOrphanedSmartLists } from "@/lib/smart-review"; @@ -64,8 +65,14 @@ export function UserDatabaseProvider({ console.log(`[UserDB Native] Migrated to version ${USER_DB_MIGRATIONS.length}`); } - // Programmatic migration: rename random default IDs → deterministic slugs const wrapped = wrapUserDb(db, { shouldNotifyError: () => live.current }); + + // Once per device, flag-keyed rather than `user_version`-keyed — see + // db/simple-repair.ts for why it is neither a migration nor per-launch. + const lifted = await repairRatchetedIntervals(wrapped); + if (lifted > 0) console.log(`[UserDB Native] Lifted ${lifted} ratcheted cards`); + + // Programmatic migration: rename random default IDs → deterministic slugs const migrated = await wrapped.getFirstAsync<{ value: string }>( "SELECT value FROM app_flags WHERE key = 'default_ids_migrated'", ); diff --git a/db/user-provider.web.tsx b/db/user-provider.web.tsx index c63b7aa..7bc795e 100644 --- a/db/user-provider.web.tsx +++ b/db/user-provider.web.tsx @@ -2,6 +2,7 @@ import React, { createContext, useContext, useEffect, useState, useCallback, use import * as SQLite from "expo-sqlite"; import type { WrappedUserDb } from "./user-db"; import { USER_DB_MIGRATIONS } from "./user-migrations"; +import { repairRatchetedIntervals } from "./simple-repair"; import { migrateLegacyMnemonics } from "./mnemonic-migration"; import { makeDefaultListId } from "@/lib/seed-default-lists"; import { cleanupOrphanedSmartLists } from "@/lib/smart-review"; @@ -109,6 +110,13 @@ async function openAndMigrateUserDb(): Promise<{ sync: () => {}, }; + // Once per device, flag-keyed rather than `user_version`-keyed — see + // db/simple-repair.ts for why it is neither a migration nor per-launch. The + // flag also stops it firing mid-session, since this runs again whenever the + // web lock is reacquired. + const lifted = await repairRatchetedIntervals(wrapped); + if (lifted > 0) console.log(`[UserDB Web] Lifted ${lifted} ratcheted cards`); + // Programmatic migration: rename random default IDs → deterministic slugs // Disable FKs during migration — UPDATE on PK internally does DELETE+INSERT // which triggers CASCADE and destroys the FK references we just updated. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index ebae9c0..3359f83 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1241,16 +1241,22 @@ Simple spaced repetition with a day-based epoch and fixed interval multipliers. **Card states:** -- `NULL` d → New (never seen) -- `s:0` → Learning/lapsed (in-session, needs 3 correct to graduate) +- `NULL` `s` → New (never seen) +- `s:0` → Learning/lapsed (needs 3 correct in a row to graduate, counted in + `simple_run` on the card) - `s:1` → Graduated (scheduled for future review) +- `simple_relearning` → 1 while a card is recovering from a lapse, cleared by the + re-graduation that pays for it **Interval parameters:** - Initial interval: 1/3 day (~8 hours) - Correct: interval × 1.9 (chain: 0.33 → 0.63 → 1.20 → 2.29 → 4.34 → 8.25 → 15.68 → 29.79 → 56.61 → 107.55 → 204.37 → 365) - Easy: interval × 2.6125 (1.9 × 1.375 bonus) -- Lapse: interval × 0.5 +- Lapse: the interval is preserved on the failure itself and halved **once**, on the + re-graduation that follows (`simple_relearning`) — never again. Scoping that to + `lapses > 0`, a lifetime counter, ratcheted every card that had ever failed down to + the floor; see [simple-srs-decisions.md](simple-srs-decisions.md) - Max interval: 365 days - Due dates stored as fractional days since 2001-01-01 (Mac/Core Foundation epoch) @@ -1258,17 +1264,39 @@ Simple spaced repetition with a day-based epoch and fixed interval multipliers. - **Dynamic deque**: maintains ~10 active cards at a time, pulling from pools as needed - Two pools loaded at session start: due cards (priority) and new cards + - Both pools are loaded by `loadSimpleQueuePools` in `db/simple-queue.ts`, which is where + the SQL lives so a test can run it rather than a copy of it + - The new pool is capped at `lists.new_per_session` (default 10), read from the row + inside `loadQueue`, less the unseen cards this session has already answered — so + "Continue Studying" tops the session up to the cap rather than granting another one. + Smart Review and the flagged-cards list are exempt (`isEphemeralListId`), since both are + assembled on purpose and are entirely new + - The due pool is loaded in two parts: graduated cards due today, uncapped — that is the + real review load — and cards still in learning, capped at `learningCapFor()` (three + times `new_per_session`), ordered most-recently-answered first and placed at the head + of the queue. A deck left by the uncapped session has thousands of stage-0 cards all + due at once; `DueCardsSection` applies the same cap to its "due today" count - When active deque drops below 10 pending cards, pulls more from due pool first, then new pool - This naturally introduces new cards as the deque thins out - **Re-queue**: failed/learning cards push to back of the active deque (~10 cards away, not 300+) - Graduated cards (stage 1) pass in 1 correct answer → recalculate interval, removed from session -- New/learning cards (stage 0) need 3 correct to graduate +- New/learning cards (stage 0) need 3 correct in a row to graduate, counted in + `simple_run` on the row — it used to live in a screen ref, so a card answered once a + session never graduated at all - **Continuous session**: no checkpoint screen — session flows until all pools and deque are exhausted - **Progress bar**: tracks only due cards (not new cards) — turns green when all due cards graduated - Counter: `completedDueCards / totalDueAtStart (totalInList)` - `simpleDueIdsRef` tracks which cards were due at start; `completedSrsIdsRef` tracks graduated due cards - Session length is dynamic — failing cards extends the session, new cards trickle in continuously +**Repair:** `db/simple-repair.ts` lifts cards the pre-fix lapse rule ratcheted to the floor. +It runs once per device, keyed to an `app_flags` row — not as a migration, and not on every +launch, because the clause also matches cards the fixed engine honestly leaves at the floor. + +Scheduling changes here are measured by simulating a deck through `nextSimpleReview` itself, +with `yarn simulate:srs`; [simple-srs-decisions.md](simple-srs-decisions.md) records what has +been decided, the numbers behind it, and what was measured and rejected. + ### FSRS (`ts-fsrs` library) Full FSRS 5.0 implementation via `ts-fsrs`. Uses the same logical day cutoff for due cards. diff --git a/docs/simple-srs-decisions.md b/docs/simple-srs-decisions.md new file mode 100644 index 0000000..32d334f --- /dev/null +++ b/docs/simple-srs-decisions.md @@ -0,0 +1,256 @@ +# Simple SRS — decisions + +`stores/simple-srs.ts` is a clone of Midori's scheduler, and `app/(tabs)/lists/study.tsx` +drives it. Every entry here is a question that was argued out with numbers. Re-opening one +is fine, but bring a measurement, not an opinion. + +## How to measure a change here + +The engine is a pure function of the card row: `nextSimpleReview(card, action)` decides one +answer, and nothing else in the session changes a card's schedule. So a scheduling change +is measured by simulating a deck through that exact function — not through a reimplementation +of the rules, which is how the first version of this measurement missed the bug entirely. + +**`yarn simulate:srs` regenerates every number below**, from +`scripts/simulate-simple-srs.ts`. It ships for a reason: review could not reproduce an +earlier version of these tables from their prose description, because the answer-count +spread, the daily budget and the intra-day re-queue rule all change the result and none of +them were written down. A number nobody else can produce is not a measurement. Quote the +script's output, and if a figure here disagrees with it, the script is right. + +Its assumptions, all of which matter: 8,000 cards (Sam's list), 90% accuracy on graduated +cards and 80% on cards in learning, a fixed daily answer budget, and a card that fails or +has not yet collected three correct answers pushed back into the same day's queue — because +the session re-queues it about ten cards away rather than ending. **The answers-per-day +figure decides more than any rule does**; see the repair decision. + +Reference figures for the fixed engine, fresh deck, ten new cards a day: the queue settles +at 79 cards shown on d30 and 117 on d365, with 139 and 227 answers given. It does not grow. + +## The lapse multiplier applies once (2026-10-13) + +`simpleGraduate`'s third argument was `card.lapses > 0` — a lifetime counter nothing resets +— so every graduation after a card's first ever failure multiplied its interval by 0.5 +instead of 1.9. A card climbed to 56 days, failed once, and then ratcheted 28 → 14 → 7 → +3.5 → … to the 8-hour floor, where it was due every day for the rest of its life. At 90% +accuracy, over 20,000 sampled cards, **92% sit at or below a one-day interval after 40 +answers** and 96% after 45; under the corrected rule it is 0.1%, and 0.01% at 45. (An earlier +version of this line said 96% at 40, from a 400-card sample — `yarn simulate:srs ratchet`.) + +The flag is now `simple_relearning`, a column, set on the failure and cleared by the +re-graduation that pays for it. Midori's own file is the check — 2,450 cards with a median +interval of 37 days, 187 at the 365-day cap and 107 below a day, which is the shape the +corrected rule produces and nothing like what the shipped one did. (Measured from Sam's own +Midori export, which is not in this repo, so that one figure is the only number here +`yarn simulate:srs` cannot reproduce.) + +## The correct-in-a-row count lives on the card (2026-10-13) + +A learning card needs three correct answers in a row. That count lived in a React ref, so +it died with the screen: at one pass per session a card never graduated, ever. It is +`simple_run` on the row now. + +This mattered because of the next entry — with the whole list in one queue, one pass per +card per session was the only rate available. + +## A session introduces ten unseen cards, not the whole list (2026-10-13) + +The new pool was every unseen card in the list. On 8,000 words that is 8,000 cards in one +queue, which is also why nothing collected three consecutive answers. The cap is +`lists.new_per_session`, default 10 (Midori's number), clamped to 1–500 by +`resolveNewPerSession` — zero would send a list with nothing due straight to the done +screen. + +Smart Review and the flagged-cards list are exempt: both are assembled on purpose and are +entirely new, so a cap would show ten of a deliberate selection and call that a session. +The RTK review list cannot reach the branch at all — `lib/rtk-review.ts` writes +`flashcard_mode 'srs'` and its multi-scope forces FSRS. That correction is worth recording +because the opposite was asserted before it was checked. + +## Already-ratcheted decks are repaired, not left to recover (2026-10-13) + +Fixing the rule does not fix the stored intervals. A deck the bug has flattened is 8,000 +cards at the 8-hour floor, all due today and all due again tomorrow. Queue size by day, +every run driven by the shipped `nextSimpleReview`: + +| answers/day | repair | d1 | d7 | d30 | d90 | d180 | d365 | +| ----------- | ------------ | ---- | ---- | ---- | ---- | ---- | ---- | +| 1000 | none | 8000 | 7338 | 5270 | 1328 | 81 | 18 | +| 1000 | spread 2–30d | 8000 | 3607 | 243 | 116 | 64 | 58 | +| 1000 | flat 7d | 8000 | 3461 | 174 | 553 | 579 | 13 | +| 200 | none | 8000 | 7863 | 7462 | 6676 | 5552 | 3579 | +| 200 | spread 2–30d | 8000 | 7106 | 5311 | 3431 | 1166 | 47 | +| 200 | flat 7d | 8000 | 7072 | 6094 | 4860 | 2272 | 252 | +| 150 | none | 8000 | 7900 | 7581 | 6980 | 6164 | 4664 | +| 150 | spread 2–30d | 8000 | 7343 | 5958 | 4530 | 2839 | 76 | + +A four-digit cell is one sample of a stochastic process at seed 7; `--seed=N` resamples +(the `ratchet` and `discriminate` scenarios are exhaustive sweeps and ignore it). Every row +of a comparison shares one seed, which was not true at first: sampling each row from its own +stream confounded the thing being compared with the sample, and produced a conclusion below +that reversed at every other seed. + +**At 1,000 answers a day the deck heals itself in about four months, and at 150–200 it does +not.** That single number is why the first conclusion here — natural recovery, no repair — +was wrong: it was measured at an effort nobody sustains. At 200 a day, 3,579 cards are still +queued a year later, because a card answered today returns within hours and takes tomorrow's +slot. + +So there is a one-time repair: a migration that spreads the cards pinned at the floor over +the next month. The rows in the table above are simulated with every card repaired, which is the upper +bound; the clause below is narrower, and the stage split under **A learning card is bounded, +not repaired** says by how much. What was rejected, +and why: + +- **No repair.** The numbers above, at the effort the deck is actually reviewed at. +- **A flat lift to 7 days.** Half the drain of the spread at every horizon — 2,272 still + queued at d180 against 1,166 — and it re-synchronises the deck: at 1,000 answers a day the + queue falls to 174 by d30 and then swings back to 553 and 579 on d90 and d180, the whole + deck returning together, where the spread settles at 116 and 64. +- **A narrower spread.** A shorter window drains more slowly for the same reason the flat + lift does: more cards land on the same days. The backlog is the complaint, and a card + given more days than it has earned costs one failed review when it comes back. + +### Which rows the repair touches + +`simple_stage = 1 AND simple_interval <= 0.34 AND lapses > 0 AND reps >= 12 AND deleted_at IS +NULL`, and the matched rows get both a new interval and a new due date inside the next +month. + +`reps >= 12` is the discriminator, and it was measured rather than reasoned. Replaying +3,600 cards under each rule, counting `reps` the way `rateSimpleSrsCard` does: + +The sample is 400 seeds × nine answer counts (4, 8, 12, 16, 24, 32, 48, 64, 96) = 3,600 +cards under each rule, because `reps` only varies if cards have lived different lengths — a +fixed-length replay gives every card the same count and the clause then catches everything. + +| clause | ratchet victims caught | honest floor cards lifted | +| ------------------------------------- | ---------------------- | ------------------------- | +| `interval <= 0.34, reps >= 3` @ 90% | 2359 / 2359 | 111 / 111 | +| `interval <= 0.34, reps >= 12` @ 90% | 2127 / 2359 | 13 / 111 | +| `interval <= 0.34, reps >= 20` @ 90% | 1735 / 2359 | 0 / 111 | +| `interval <= 0.34, lapses >= 3` @ 90% | 1568 / 2359 | 17 / 111 | +| **the shipped clause** @ 90% | 1589 / 2359 | — | +| `interval <= 0.34, reps >= 12` @ 75% | 2543 / 3031 | 167 / 469 | +| `interval <= 0.34, lapses >= 3` @ 75% | 2504 / 3031 | 259 / 469 | +| **the shipped clause** @ 75% | 1098 / 3031 | — | + +The shipped clause also requires `simple_stage = 1`, so it reaches 1,589 of the 2,359 — 93% +of the 1,714 that are graduated, and none of the rest. **The `reps >= 12` row is the +discriminator's power, not the clause's reach**; quoting 2,127 for the clause overstates it +by a third. + +A card the ratchet dragged down from 56 days has a median of 32 answers behind it; one +honestly at the floor has 4, and never more than 16 at 90% accuracy. **`lapses >= 3` is the +obvious guard and it is wrong**: under the bug a single lapse starts the ratchet, so it +catches only 1,568 of 2,359 victims — a third of the deck left flattened. That was proposed +in review, and the measurement is the only reason it was not shipped. + +`simple_interval <= 1` rather than `<= 0.34` catches cards part-way down the ratchet at the +cost of lifting far more honest ones. A card at 0.88 days climbs on its own once the rule is +fixed — 1.67, 3.18, 6.04 — so only the cards pinned at the floor need the repair. + +### A learning card is bounded, not repaired (2026-10-13) + +**An earlier version of the repair rewrote stage-0 intervals too, and that was wrong.** A +stage-0 card sits at 1/3 because that is where every new card starts, not because of the +ratchet: `simpleReviewFail` preserves the interval and the learning path never multiplies +it. With the whole list loading as new and the correct-in-a-row count dying with the +screen, cards that never graduated at all are most of the stage-0 population — and they +have the reps to pass any threshold, because the bug made them answer the same words every +day. Lifting those would hand thousands of unlearned words a fortnight each. + +Of 2,359 floored cards at 90% accuracy, 1,714 are graduated and 645 are in learning; at +75% the split inverts, 1,263 against 1,768. So the clause repairs 93% of the floored +graduated cards (87% at 75%) and none of the learning ones — 67% and 36% of the floored +population. + +The learning ones are the real complaint, and no rewrite of a stored interval fixes them: +they are words with one pass each that never graduated, and they are all due, every day. +**Capping them is what fixes it.** The learning slice of the due pool is limited to +`learningCapFor(newPerSession)` — three learning answers are one new card's worth of work — +ordered most-recently-answered first, so the cards already in hand get finished before the +backlog is reached, and placed at the head of the queue so a graduated load bigger than the +learner's stamina cannot stop anything in learning from advancing. The list screen's "due +today" badge applies the same cap, or it promises 8,000 cards for a session that shows +thirty and then says it is done. + +Cards shown per day on a legacy deck of 8,000 stage-0 cards, and two counts of what was +learned: cards that touched stage 1 at any point, and cards still graduated at the end on an +interval longer than a day — i.e. actually kept (`yarn simulate:srs learningCap`, seed 7): + +| learning cap | answers/day | d1 | d7 | d30 | d180 | d365 | touched stage 1 | kept | +| ------------ | ----------- | ---- | ---- | ---- | ---- | ---- | --------------- | ---- | +| none | 200 | 8000 | 8000 | 8000 | 8000 | 5480 | 8000 | 2642 | +| 30 (3 × 10) | 200 | 30 | 119 | 86 | 133 | 140 | 3706 | 3674 | +| 10 | 200 | 10 | 47 | 75 | 106 | 112 | 3240 | 3223 | +| none | 400 | 8000 | 8000 | 8000 | 5256 | 762 | 8000 | 7489 | +| 30 (3 × 10) | 400 | 30 | 137 | 221 | 246 | 230 | 7051 | 7006 | +| 10 | 400 | 10 | 47 | 75 | 118 | 126 | 3650 | 3631 | + +**Which column you read decides the answer, and an earlier version of this section read the +wrong one.** It said the cap cost thousands of words, counting cards that touched stage 1. +Uncapped, 8,000 do — and only 2,642 are still there at the end, because an uncapped deck +churns: a card graduates at 0.633 days, comes back the same day, fails, and sits at the floor +again. Capped, almost everything touched is kept (3,674 of 3,706). **On words actually +retained the cap wins at the effort this is about**, 3,674 against 2,642, while showing 140 +cards a day instead of 8,000. + +The ×3 is ahead of a flat cap at both efforts, and that needed four seeds to say, because a +single seed said the opposite (`yarn simulate:srs capSeeds`): + +| seed | answers/day | kept, cap 30 | kept, cap 10 | uncapped | +| ---- | ----------- | ------------ | ------------ | -------- | +| 7 | 200 | 3674 | 3223 | 2642 | +| 11 | 200 | 3673 | 3226 | 2614 | +| 42 | 200 | 3665 | 3215 | 2657 | +| 99 | 200 | 3669 | 3221 | 2639 | +| 7 | 400 | 7006 | 3631 | 7489 | +| 11 | 400 | 6991 | 3626 | 7451 | + +Uncapped is ahead only at 400 answers a day, and only by about 6% — for which it shows the +entire 8,000-card deck every day for six months. A cap of ten costs half the words at that +effort, which is what the multiplier is for: three answers graduate one card, so ten slots +cannot deliver them alongside reviews. + +A genuinely ratcheted card currently in learning is the price of not repairing stage 0: it +re-graduates to the floor once and then climbs 0.33 → 0.63 → 1.2 by itself. + +Accepted trades, written down because they are real: + +- A card honestly at the floor with twelve or more answers behind it is lifted with the + rest. Nothing in the row distinguishes "ratcheted down from 56 days" from "never + climbed" — the simple path writes no `review_logs` entry, so there is no history to + replay — and at 75% accuracy that is 167 cards in 2,700. The cost is one failed review + each. +- `reps` is not per-mode. A list switched from FSRS to simple SRS brings its FSRS count + with it, so its cards qualify after a single simple graduation lands them at the floor. + Bounded and self-correcting; there is nothing in the row to tell them apart, and adding a + per-mode counter for one migration is not worth it. +- The victims that stopped part-way down the ratchet, and every ratcheted card currently in + learning, are missed. They climb on their own. +- A list moved from simple SRS to FSRS has its dormant `simple_*` columns rewritten and its + `updated_at` bumped, so rows that no longer schedule anything get pushed. Harmless, and + cheaper than a `flashcard_mode` join that would also skip a list switched back later. +- **The repair runs once per device, keyed to an `app_flags` row, and cannot run on every + launch.** The clause matches rows the FIXED engine legitimately produces: a card that fails + and re-graduates from a short interval lands at exactly `MIN_INTERVAL`, so three honest + lapses leave it at 1/3 with `reps = 15` and `lapses = 3`, matching every condition. Once, + that is the bounded cost in the table above. Every launch, it is a standing policy that + buries the words someone is working hardest on, over and over. It was written that way + first and review caught it. +- It does not bump `updated_at`, so it never travels: a device opened for the first time in + months would otherwise become the newest writer of every card it touched and push stale + values over reviews done elsewhere. The price is that a device still on an old build can + push a ratcheted interval back, and nothing lifts it again. That is the better trade — one + stale card against repeatedly burying hard words — and it is why the repair is not a + migration keyed to `PRAGMA user_version` either. +- Cards arriving after the repair has run are never repaired: a backup restore, a list + import, **and a sync pull**, which is the common one. The flag is only written once the + device has seen a simple-SRS deck, so a fresh install does not spend its one run on an + empty table — review proved that version left a reinstalling user with a permanently + flattened deck, since the repair never pushes either. A deck that arrives _after_ a + repaired launch still stays as it is. +- A restore no longer resurrects a soft-deleted row. `INSERT OR REPLACE` cleared + `deleted_at` because it deleted and re-inserted; an upsert does not name the column unless + the backup carried it. Deliberate, and the better behaviour. diff --git a/lib/data-backup.test.ts b/lib/data-backup.test.ts index 62e1b0b..09ec57e 100644 --- a/lib/data-backup.test.ts +++ b/lib/data-backup.test.ts @@ -1,6 +1,14 @@ +import Database from "better-sqlite3"; import { describe, expect, it } from "vitest"; import { createTestDb } from "@/test/test-db"; -import { attemptBackup, importBackup } from "./data-backup"; +import { + attemptBackup, + importBackup, + BACKUP_TABLES, + TABLE_COLUMNS, + IMPORT_ORDER, + PRIMARY_KEYS, +} from "./data-backup"; describe("data backup", () => { it("round-trips local-only history tables through backup and import", async () => { @@ -103,7 +111,9 @@ describe("data backup", () => { const failingDb = { ...targetDb, runAsync: async (sql: string, params?: any[]) => { - if (sql.includes("INSERT OR REPLACE INTO lists")) { + // Matches whatever statement shape the restore uses; it is an upsert + // now, because `INSERT OR REPLACE` deletes the row and cascades. + if (/INTO lists\b/.test(sql)) { throw new Error("lists import failed"); } return targetDb.runAsync(sql, params); @@ -131,3 +141,360 @@ describe("data backup", () => { } }); }); + +/** + * A schema change must not cost the user their data in either direction. + * + * Review proved both halves broke: a backup from a build with + * `new_per_session` restored onto one without it failed per TABLE, so every + * list and every flashcard was skipped; and `attemptBackup`'s SELECT names the + * new columns while the recovery screen only runs when the database failed to + * migrate — precisely when they may not exist. + */ +describe("backup across a schema change", () => { + /** A `WrappedUserDb`-shaped wrapper over better-sqlite3. */ + function wrap(db: Database.Database) { + return { + getAllAsync: async (sql: string, args: unknown[] = []) => + db.prepare(sql).all(...(args as unknown[])) as T[], + getFirstAsync: async (sql: string, args: unknown[] = []) => + (db.prepare(sql).get(...(args as unknown[])) ?? null) as T | null, + runAsync: async (sql: string, args: unknown[] = []) => { + db.prepare(sql).run(...(args as unknown[])); + }, + execAsync: async (sql: string) => { + db.exec(sql); + }, + } as unknown as Parameters[0]; + } + + /** The `lists` table as it was before `new_per_session` existed. */ + function olderDb(): Database.Database { + const db = new Database(":memory:"); + db.exec(`CREATE TABLE lists ( + id TEXT PRIMARY KEY, name TEXT, description TEXT, + flashcard_mode TEXT DEFAULT 'add_order', front_faces TEXT, back_faces TEXT, + study_position INTEGER DEFAULT 0, configured INTEGER DEFAULT 0, + auto_play_audio INTEGER DEFAULT 0, confusion_detection INTEGER DEFAULT 1, + voice_mode INTEGER DEFAULT 0, typing_mode INTEGER DEFAULT 0, + disable_flip_animation INTEGER DEFAULT 0, disable_swipe_animation INTEGER DEFAULT 0, + is_default INTEGER DEFAULT 0, learning_steps TEXT, relearning_steps TEXT, + mnemonic_cloze INTEGER DEFAULT 0, created_at TEXT, updated_at TEXT)`); + return db; + } + + it("restores onto an older build instead of losing every list", async () => { + const db = olderDb(); + const backup = JSON.stringify({ + version: 1, + tables: { + lists: [ + { id: "l1", name: "Core 8k", new_per_session: 25, created_at: "x", updated_at: "y" }, + ], + }, + }); + const result = await importBackup(wrap(db), backup); + expect(result.failed).toEqual([]); + expect(result.succeeded).toContain("lists"); + expect(db.prepare("SELECT name FROM lists WHERE id = 'l1'").get()).toEqual({ name: "Core 8k" }); + }); + + it("takes the column default for a column an older backup never had", async () => { + const db = new Database(":memory:"); + db.exec(`CREATE TABLE lists (id TEXT PRIMARY KEY, name TEXT, + new_per_session INTEGER DEFAULT 10, created_at TEXT, updated_at TEXT)`); + // No `new_per_session` key at all, the way a pre-feature backup looks. + const backup = JSON.stringify({ + version: 1, + tables: { lists: [{ id: "l1", name: "Core 8k", created_at: "x", updated_at: "y" }] }, + }); + await importBackup(wrap(db), backup); + expect(db.prepare("SELECT new_per_session FROM lists WHERE id = 'l1'").get()).toEqual({ + new_per_session: 10, + }); + }); + + it("rescues the rows a failed migration leaves readable", async () => { + const db = olderDb(); + db.prepare("INSERT INTO lists (id, name) VALUES ('l1', 'Core 8k')").run(); + // BACKUP_TABLES names `new_per_session`, which this database lacks. + const result = await attemptBackup(wrap(db) as never); + expect(result.failed).not.toContain("lists"); + expect(result.tables.lists).toHaveLength(1); + expect((result.tables.lists[0] as { name: string }).name).toBe("Core 8k"); + }); +}); + +/** + * The export's SELECT and the restore's column list must agree. + * + * They had drifted: `books.read_complete` was exported and then dropped on + * restore, and `user_kanji_notes.updated_at` was in neither, so a restored + * mnemonic was invisible to the push, which selects `WHERE updated_at > ?`. + */ +describe("the backup's column lists", () => { + const selected = (query: string) => + query + .replace(/^SELECT\s+/i, "") + .replace(/\s+FROM\s+.*$/i, "") + .split(",") + .map((col) => col.trim()); + + it("restores every column it exports", () => { + const drift: string[] = []; + for (const table of BACKUP_TABLES) { + const restored = new Set(TABLE_COLUMNS[table.name] ?? []); + for (const col of selected(table.query)) { + if (!restored.has(col)) drift.push(`${table.name}.${col}`); + } + } + expect(drift).toEqual([]); + }); + + it("exports every column it would restore", () => { + const drift: string[] = []; + for (const table of BACKUP_TABLES) { + const exported = new Set(selected(table.query)); + for (const col of TABLE_COLUMNS[table.name] ?? []) { + if (!exported.has(col)) drift.push(`${table.name}.${col}`); + } + } + expect(drift).toEqual([]); + }); +}); + +describe("restoring one table does not take its neighbours with it", () => { + it("keeps a list's words when only `lists` is in the backup", async () => { + // `INSERT OR REPLACE` is a DELETE then an INSERT, so restoring the parent + // fired every cascade: the list came back with zero words and its cards + // orphaned. A partial backup is the normal case — `attemptBackup` reports + // per table and only the ones that succeeded are written. + const db = createTestDb(); + const now = "2026-04-05T00:00:00.000Z"; + await db.runAsync("PRAGMA foreign_keys = ON"); + await db.runAsync( + `INSERT INTO lists (id, name, is_default, study_position, created_at, updated_at) + VALUES ('L', 'List', 0, 0, ?, ?)`, + [now, now], + ); + await db.runAsync( + `INSERT INTO list_entries (id, list_id, entry_id, added_at, position, updated_at) + VALUES ('e1', 'L', 1001, ?, 0, ?)`, + [now, now], + ); + await db.runAsync( + `INSERT INTO srs_cards + (id, entry_id, list_id, due, stability, difficulty, elapsed_days, scheduled_days, + reps, lapses, state, created_at, updated_at) + VALUES ('c1', 1001, 'L', ?, 0, 0, 0, 0, 0, 0, 0, ?, ?)`, + [now, now, now], + ); + + const result = await importBackup( + db, + JSON.stringify({ + version: 1, + tables: { lists: [{ id: "L", name: "List renamed", created_at: now, updated_at: now }] }, + }), + ); + + expect(result.failed).toEqual([]); + expect(await db.getFirstAsync<{ c: number }>("SELECT COUNT(*) as c FROM list_entries")).toEqual( + { c: 1 }, + ); + expect( + await db.getFirstAsync<{ list_id: string | null }>("SELECT list_id FROM srs_cards"), + ).toEqual({ list_id: "L" }); + // and the row itself really was updated + expect(await db.getFirstAsync<{ name: string }>("SELECT name FROM lists")).toEqual({ + name: "List renamed", + }); + }); + + it("skips a row with no primary key rather than writing a NULL-keyed one", async () => { + // SQLite allows NULL in a TEXT PRIMARY KEY, and sync keys on it. Every + // other column is filled, or the row would fail for its own reasons and + // the guard would go untested. + const db = createTestDb(); + const now = "2026-04-05T00:00:00.000Z"; + const result = await importBackup( + db, + JSON.stringify({ + version: 1, + tables: { + furigana_pins: [ + { + book_id: "b", + surface: "\u6c34", + reading: "\u307f\u305a", + created_at: now, + updated_at: now, + deleted_at: null, + }, + ], + }, + }), + ); + expect(result.failed).toEqual([]); + expect( + await db.getFirstAsync<{ c: number }>("SELECT COUNT(*) as c FROM furigana_pins"), + ).toEqual({ c: 0 }); + }); + + it("reports a locked or corrupt table as failed, not as a clean rescue", async () => { + // The column fallback must not swallow a real error: re-selecting fewer + // columns would hand `DbRecoveryScreen` whatever came back and call it a + // success. + const db = createTestDb(); + let thrown = 0; + const busy = { + ...db, + getAllAsync: async (sql: string, params?: unknown[]) => { + // Transient, the way a lock is: a retry would succeed. The fallback + // must not retry, or it reports the table rescued. + if (/FROM books/.test(sql) && thrown++ === 0) { + throw new Error("SQLITE_BUSY: database is locked"); + } + return db.getAllAsync(sql, params as never); + }, + } as unknown as Parameters[0]; + const result = await attemptBackup(busy); + expect(result.failed).toContain("books"); + expect(result.succeeded).not.toContain("books"); + }); +}); + +describe("a row that will not go in does not take the table with it", () => { + it("survives a collision on a unique index other than the primary key", async () => { + // `confusion_pairs` carries a unique index on its five content columns, so + // a pair that already exists locally under a different id raises an error + // the primary key's ON CONFLICT cannot absorb. That used to abort the table + // and drop every row after it. + const db = createTestDb(); + const now = "2026-04-05T00:00:00.000Z"; + await db.runAsync( + `INSERT INTO confusion_pairs (id, entry_id_a, kanji_literal_a, entry_id_b, + kanji_literal_b, confusion_type, confusion_count, last_confused_at, created_at) + VALUES ('local', 1, '漢', 2, '字', 'meaning', 1, ?, ?)`, + [now, now], + ); + + const result = await importBackup( + db, + JSON.stringify({ + version: 1, + tables: { + confusion_pairs: [ + { + id: "incoming", + entry_id_a: 1, + kanji_literal_a: "漢", + entry_id_b: 2, + kanji_literal_b: "字", + confusion_type: "meaning", + confusion_count: 9, + last_confused_at: now, + created_at: now, + }, + { + id: "unrelated", + entry_id_a: 7, + kanji_literal_a: null, + entry_id_b: 8, + kanji_literal_b: null, + confusion_type: "meaning", + confusion_count: 1, + last_confused_at: now, + created_at: now, + }, + ], + }, + }), + ); + + expect(result.failed).toEqual([]); + // The colliding pair was replaced, and the row behind it still landed. + const rows = await db.getAllAsync<{ id: string }>("SELECT id FROM confusion_pairs ORDER BY id"); + expect(rows.map((r) => r.id)).toEqual(["incoming", "unrelated"]); + }); + + it("writes the rows it can when one row is unwritable", async () => { + const db = createTestDb(); + const now = "2026-04-05T00:00:00.000Z"; + const result = await importBackup( + db, + JSON.stringify({ + version: 1, + tables: { + lists: [ + // No `name`, which is NOT NULL: this row cannot go in. + { id: "bad", created_at: now, updated_at: now }, + { id: "good", name: "Core 8k", created_at: now, updated_at: now }, + ], + }, + }), + ); + expect(result.failed).toEqual([]); + expect(await db.getAllAsync<{ id: string }>("SELECT id FROM lists")).toEqual([{ id: "good" }]); + }); + + it("builds valid SQL for a row that carries only its primary key", async () => { + // `ON CONFLICT(id) DO UPDATE SET` with nothing to set is a syntax error, + // and cost the whole table. The row still cannot go in — `lists.name` is + // NOT NULL — but it must fail on the constraint, not on the statement. + const db = createTestDb(); + const result = await importBackup( + db, + JSON.stringify({ version: 1, tables: { lists: [{ id: "L" }] } }), + ); + expect(result.failed).toHaveLength(1); + expect(result.failed[0].error).toMatch(/NOT NULL/i); + expect(result.failed[0].error).not.toMatch(/incomplete input|syntax/i); + }); +}); + +describe("the restore's primary keys", () => { + it("names the real primary key of every table it restores", async () => { + // A wrong name makes `ON CONFLICT(pk)` throw and costs the whole table, and + // the map is easy to leave stale — it carried two tables that are not even + // restored. + const db = createTestDb(); + const wrong: string[] = []; + for (const table of IMPORT_ORDER) { + const info = await db.getAllAsync<{ name: string; pk: number }>( + `PRAGMA table_info(${table})`, + ); + const actual = info.filter((c) => c.pk > 0).map((c) => c.name); + const used = PRIMARY_KEYS[table] ?? "id"; + if (actual.length !== 1 || actual[0] !== used) { + wrong.push(`${table}: restore uses ${used}, schema says ${actual.join("+") || "none"}`); + } + } + expect(wrong).toEqual([]); + }); +}); + +describe("a backup that predates a required column", () => { + it("restores mnemonics from a pre-feature backup instead of losing all of them", async () => { + // `user_kanji_notes.updated_at` is NOT NULL with no default and was in + // neither the export nor the restore list, so every row of a backup taken + // before it failed on the constraint. Pre-existing on master; the "survives + // a schema change" work is what makes it this file's problem. + const db = createTestDb(); + const result = await importBackup( + db, + JSON.stringify({ + version: 1, + tables: { user_kanji_notes: [{ literal: "水", mnemonic: "water", keyword: "water" }] }, + }), + ); + expect(result.failed).toEqual([]); + const row = await db.getFirstAsync<{ mnemonic: string; updated_at: string }>( + "SELECT mnemonic, updated_at FROM user_kanji_notes WHERE literal = '水'", + ); + expect(row?.mnemonic).toBe("water"); + // Stamped at the epoch, not now: a restored note must not outrank a live + // one in `updated_at > ?` comparisons it never earned. + expect(row?.updated_at).toBe("1970-01-01T00:00:00.000Z"); + }); +}); diff --git a/lib/data-backup.ts b/lib/data-backup.ts index 70a6ebd..0b64087 100644 --- a/lib/data-backup.ts +++ b/lib/data-backup.ts @@ -21,7 +21,7 @@ export const BACKUP_TABLES: { { name: "lists", query: - "SELECT id, name, description, flashcard_mode, front_faces, back_faces, study_position, configured, auto_play_audio, confusion_detection, voice_mode, typing_mode, disable_flip_animation, disable_swipe_animation, is_default, learning_steps, relearning_steps, mnemonic_cloze, created_at, updated_at FROM lists", + "SELECT id, name, description, flashcard_mode, front_faces, back_faces, study_position, configured, auto_play_audio, confusion_detection, voice_mode, typing_mode, disable_flip_animation, disable_swipe_animation, is_default, learning_steps, relearning_steps, mnemonic_cloze, new_per_session, created_at, updated_at FROM lists", }, { name: "list_entries", @@ -31,7 +31,7 @@ export const BACKUP_TABLES: { { name: "srs_cards", query: - "SELECT id, entry_id, list_id, due, stability, difficulty, elapsed_days, scheduled_days, reps, lapses, state, last_review, front_mode, back_mode, simple_stage, simple_n, simple_interval, last_confusion_check, kanji_literal, learning_steps, created_at, updated_at FROM srs_cards", + "SELECT id, entry_id, list_id, due, stability, difficulty, elapsed_days, scheduled_days, reps, lapses, state, last_review, front_mode, back_mode, simple_stage, simple_n, simple_interval, simple_relearning, simple_run, last_confusion_check, kanji_literal, learning_steps, created_at, updated_at FROM srs_cards", }, { name: "review_logs", @@ -63,7 +63,12 @@ export const BACKUP_TABLES: { query: "SELECT id, book_id, surface, reading, created_at, updated_at, deleted_at FROM furigana_pins", }, - { name: "user_kanji_notes", query: "SELECT literal, mnemonic, keyword FROM user_kanji_notes" }, + { + name: "user_kanji_notes", + // `updated_at` or a restored mnemonic is invisible to the push, which + // selects `WHERE updated_at > ?`. + query: "SELECT literal, mnemonic, keyword, updated_at FROM user_kanji_notes", + }, { name: "practice_sessions", query: @@ -82,7 +87,28 @@ export const BACKUP_TABLES: { ]; // Column names for each table — used for INSERT during import -const TABLE_COLUMNS: Record = { +/** + * Where a table in `IMPORT_ORDER` has a primary key that is not `id`. A wrong + * name makes `ON CONFLICT(pk)` throw, so this list is checked against the + * schema by `lib/data-backup.test.ts`. + */ +/** + * Columns a restore must supply if the backup did not, because the schema makes + * them NOT NULL with no default. Checked against `PRAGMA table_info` by + * `lib/data-backup.test.ts`. + */ +const REQUIRED_TIMESTAMPS: Record = { + user_kanji_notes: ["updated_at"], +}; + +/** When a restore stands in for a timestamp a backup never had. */ +const RESTORE_TIMESTAMP = new Date(0).toISOString(); + +export const PRIMARY_KEYS: Record = { + user_kanji_notes: "literal", +}; + +export const TABLE_COLUMNS: Record = { lists: [ "id", "name", @@ -102,6 +128,7 @@ const TABLE_COLUMNS: Record = { "learning_steps", "relearning_steps", "mnemonic_cloze", + "new_per_session", "created_at", "updated_at", ], @@ -132,6 +159,8 @@ const TABLE_COLUMNS: Record = { "simple_stage", "simple_n", "simple_interval", + "simple_relearning", + "simple_run", "last_confusion_check", "kanji_literal", "learning_steps", @@ -199,11 +228,12 @@ const TABLE_COLUMNS: Record = { "last_read_at", "is_default", "saved", + "read_complete", "created_at", "updated_at", ], furigana_pins: ["id", "book_id", "surface", "reading", "created_at", "updated_at", "deleted_at"], - user_kanji_notes: ["literal", "mnemonic", "keyword"], + user_kanji_notes: ["literal", "mnemonic", "keyword", "updated_at"], practice_sessions: [ "id", "session_id", @@ -242,7 +272,7 @@ const TABLE_COLUMNS: Record = { }; // Import order matters for FK constraints -const IMPORT_ORDER = [ +export const IMPORT_ORDER = [ "lists", "list_entries", "srs_cards", @@ -258,6 +288,20 @@ const IMPORT_ORDER = [ "course_progress", ]; +/** The columns a table actually has, or null if it cannot be read at all. */ +export async function existingColumns( + db: WrappedUserDb, + tableName: string, +): Promise | null> { + try { + const rows = await db.getAllAsync<{ name: string }>(`PRAGMA table_info(${tableName})`); + if (!rows.length) return null; + return new Set(rows.map((row) => row.name)); + } catch { + return null; + } +} + export async function attemptBackup(db: WrappedUserDb): Promise { const result: BackupResult = { tables: {}, succeeded: [], failed: [] }; for (const table of BACKUP_TABLES) { @@ -266,6 +310,30 @@ export async function attemptBackup(db: WrappedUserDb): Promise { result.tables[table.name] = rows; result.succeeded.push(table.name); } catch (err) { + // This runs on a database that failed to open or migrate, which is + // exactly when a column the query names may not exist yet. Falling back + // to the columns the table really has rescues the rows instead of + // reporting the whole table as lost — a missing `new_per_session` used to + // cost the user every list, and a missing `simple_relearning` every card. + // + // Only for a missing column. A locked, corrupt or absent table must stay + // a failure: re-selecting fewer columns would report a clean rescue of + // whatever came back, and the recovery screen would believe it. + const cols = /no such column/i.test(String(err)) + ? await existingColumns(db, table.name) + : null; + const wanted = TABLE_COLUMNS[table.name]; + if (cols && wanted) { + const usable = wanted.filter((col) => cols.has(col)); + try { + const rows = await db.getAllAsync(`SELECT ${usable.join(", ")} FROM ${table.name}`); + result.tables[table.name] = rows; + result.succeeded.push(table.name); + continue; + } catch (fallbackErr) { + console.error(`[Recovery] Fallback backup of ${table.name} failed:`, fallbackErr); + } + } console.error(`[Recovery] Failed to backup ${table.name}:`, err); result.failed.push(table.name); } @@ -301,8 +369,19 @@ export async function importBackup( continue; } - const cols = TABLE_COLUMNS[tableName]; - if (!cols) { + const allowed = TABLE_COLUMNS[tableName]; + if (!allowed) { + result.skipped.push(tableName); + continue; + } + + // Only the columns this database actually has. A backup written by a newer + // build names columns an older one lacks, and SQLite rejects the whole + // statement for an unknown column — so one added column used to cost every + // list and every flashcard in the restore, not just that column. + const present = await existingColumns(db, tableName); + const cols = present ? allowed.filter((col) => present.has(col)) : allowed; + if (!cols.length) { result.skipped.push(tableName); continue; } @@ -312,16 +391,83 @@ export async function importBackup( `Importing ${tableName}...`, ); + // `INSERT OR REPLACE` is a DELETE followed by an INSERT, so restoring a + // parent row fires every foreign-key cascade hanging off it: restoring + // `lists` alone emptied `list_entries` and set `srs_cards.list_id` to NULL, + // leaving the list with no words and its cards orphaned. An upsert touches + // only the row named. + const pk = PRIMARY_KEYS[tableName] ?? "id"; + const upsert = (columns: string[]) => { + const assignments = columns + .filter((col) => col !== pk) + .map((col) => `${col} = excluded.${col}`) + .join(", "); + return `INSERT INTO ${tableName} (${columns.join(", ")}) + VALUES (${columns.map(() => "?").join(", ")}) + ON CONFLICT(${pk}) ${assignments ? `DO UPDATE SET ${assignments}` : "DO NOTHING"}`; + }; + try { - const placeholders = cols.map(() => "?").join(", "); - const sql = `INSERT OR REPLACE INTO ${tableName} (${cols.join(", ")}) VALUES (${placeholders})`; + const sql = upsert(cols); + const rowErrors: string[] = []; for (const row of rows) { - const values = cols.map((col) => row[col] ?? null); - await db.runAsync(sql, values); + // A row with no primary key would insert under a NULL one — SQLite + // allows that in a TEXT PRIMARY KEY — and then poison sync, which keys + // on it. Skip it rather than write it. + if (row[pk] == null) { + processedRows++; + continue; + } + // A named NULL is written as NULL; an upsert does NOT fall back to the + // column default. So a column the backup never had is left out of the + // statement entirely, and takes its default. + const named = cols.filter((col) => row[col] !== undefined); + const values: unknown[] = named.map((col) => row[col] ?? null); + + // A timestamp a backup never carried is metadata, not the user's data, + // and the schema requires it: `user_kanji_notes.updated_at` is NOT NULL + // with no default, so every mnemonic in a pre-feature backup failed on + // it. Supplying one restores the row; leaving it out loses the lot. + for (const col of REQUIRED_TIMESTAMPS[tableName] ?? []) { + if (cols.includes(col) && row[col] === undefined) { + named.push(col); + values.push(RESTORE_TIMESTAMP); + } + } + try { + await db.runAsync(named.length === cols.length ? sql : upsert(named), values); + } catch (rowErr) { + // A table can carry a unique index besides its primary key — + // `confusion_pairs` does — and a row that collides on it raises an + // error the pk's ON CONFLICT cannot absorb. Replacing that one row is + // safe (nothing references it); aborting the table was not, because a + // single colliding pair used to cost every row after it. + if (/UNIQUE constraint failed/i.test(String(rowErr))) { + await db.runAsync( + `INSERT OR REPLACE INTO ${tableName} (${named.join(", ")}) + VALUES (${named.map(() => "?").join(", ")})`, + values, + ); + } else { + // One bad row is not the table. Collect it and keep going, so a + // truncated or hand-edited file costs that row and not the rest. + rowErrors.push(String(rowErr)); + } + } processedRows++; } + // Every row failed, so the table did: report it as before, with the + // first row's error rather than a wrapper around it. + if (rowErrors.length === rows.length && rows.length > 0) { + result.failed.push({ table: tableName, error: rowErrors[0] }); + continue; + } + if (rowErrors.length > 0) { + console.warn(`[Import] ${rowErrors.length} row(s) of ${tableName} failed`, rowErrors[0]); + } + result.succeeded.push(tableName); result.totalRows += rows.length; } catch (err) { diff --git a/lib/list-transfer.test.ts b/lib/list-transfer.test.ts index ba252d1..0d9e4b6 100644 --- a/lib/list-transfer.test.ts +++ b/lib/list-transfer.test.ts @@ -1,5 +1,12 @@ import { describe, test, expect } from "vitest"; -import { parseListImport, type JitenExportFile } from "./list-transfer"; +import { createTestDb } from "@/test/test-db"; +import { DEFAULT_NEW_PER_SESSION, resolveNewPerSession } from "@/stores/simple-srs"; +import { + buildListExport, + importListToDb, + parseListImport, + type JitenExportFile, +} from "./list-transfer"; // ─── Helpers ─── @@ -214,3 +221,39 @@ describe("parseListImport", () => { }); }); }); + +describe("the new-card cap travels with a shared list", () => { + test("exports it and imports it, so a shared list keeps its pace", async () => { + const db = createTestDb(); + const now = "2026-05-01T00:00:00.000Z"; + await db.runAsync( + `INSERT INTO lists (id, name, is_default, study_position, flashcard_mode, + new_per_session, created_at, updated_at) + VALUES ('src', 'Core 8k', 0, 0, 'simple_srs', 40, ?, ?)`, + [now, now], + ); + const exported = await buildListExport(db, "src", false); + expect(exported.list.newPerSession).toBe(40); + + const listId = await importListToDb(db, exported, false); + const row = await db.getFirstAsync<{ new_per_session: number }>( + "SELECT new_per_session FROM lists WHERE id = ?", + [listId], + ); + expect(row?.new_per_session).toBe(40); + }); + + test("leaves a file that predates the column unset, rather than pinning it", async () => { + // No `newPerSession` key at all, the way a pre-feature file looks. NULL is + // the meaningful "unset" state: the settings input shows a placeholder, and + // `resolveNewPerSession` turns it into the default at read time. + const db = createTestDb(); + const listId = await importListToDb(db, validExport(), false, undefined); + const row = await db.getFirstAsync<{ new_per_session: number | null }>( + "SELECT new_per_session FROM lists WHERE id = ?", + [listId], + ); + expect(row?.new_per_session).toBeNull(); + expect(resolveNewPerSession(row?.new_per_session)).toBe(DEFAULT_NEW_PER_SESSION); + }); +}); diff --git a/lib/list-transfer.ts b/lib/list-transfer.ts index 09e86cf..7cdf89d 100644 --- a/lib/list-transfer.ts +++ b/lib/list-transfer.ts @@ -1,6 +1,7 @@ import type { WrappedUserDb } from "@/db/user-db"; import type { CardFace, FlashcardMode } from "@/db/types"; import { parseFaces } from "./card-faces"; +import { resolveNewPerSession } from "@/stores/simple-srs"; // ─── Export file schema ─── @@ -19,6 +20,8 @@ export interface JitenExportFile { frontFaces: CardFace[]; backFaces: CardFace[]; autoPlayAudio: boolean; + /** Unseen cards a simple-SRS session introduces; absent means the default. */ + newPerSession?: number | null; }; entries: JitenExportEntry[]; @@ -62,6 +65,10 @@ export interface JitenExportFile { stage: number; n: number; interval: number; + /** 1 while the card is recovering from a lapse. Absent in older payloads. */ + relearning?: number; + /** Correct answers in a row so far. Absent in older payloads. */ + run?: number; }[]; }; } @@ -93,6 +100,7 @@ export async function buildListExport( frontFaces, backFaces, autoPlayAudio: Boolean(listRow.auto_play_audio ?? listRow.autoPlayAudio ?? 0), + newPerSession: listRow.new_per_session ?? listRow.newPerSession ?? null, }, entries: [], }; @@ -118,7 +126,7 @@ export async function buildListExport( if (flashcardMode === "simple_srs") { // Export simple SRS data const srsRows = await userDb.getAllAsync( - "SELECT entry_id, kanji_literal, simple_stage, simple_n, simple_interval FROM srs_cards WHERE list_id = ? AND simple_stage IS NOT NULL", + "SELECT entry_id, kanji_literal, simple_stage, simple_n, simple_interval, simple_relearning, simple_run FROM srs_cards WHERE list_id = ? AND simple_stage IS NOT NULL", [listId], ); @@ -130,6 +138,9 @@ export async function buildListExport( stage: card.simple_stage, n: card.simple_n, interval: card.simple_interval, + // A card shared mid-lapse owes its halving on the other side too. + relearning: card.simple_relearning ?? 0, + run: card.simple_run ?? 0, }; if (card.kanji_literal != null) base.kanjiLiteral = card.kanji_literal; return base; @@ -245,8 +256,8 @@ export async function importListToDb( const configured = hasStudyData ? 1 : 0; await userDb.runAsync( - `INSERT OR IGNORE INTO lists (id, name, description, flashcard_mode, front_faces, back_faces, study_position, configured, auto_play_audio, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + `INSERT OR IGNORE INTO lists (id, name, description, flashcard_mode, front_faces, back_faces, study_position, configured, auto_play_audio, new_per_session, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, [ listId, name, @@ -258,6 +269,11 @@ export async function importListToDb( studyPosition, configured, data.list.autoPlayAudio ? 1 : 0, + // A shared list carries its pace. A file that predates the column has no + // value, and NULL is the meaningful state for that — the settings input + // shows a placeholder rather than a number, and the list follows the + // default if it ever changes — so it is not resolved to ten here. + data.list.newPerSession == null ? null : resolveNewPerSession(data.list.newPerSession), now, now, ], @@ -293,8 +309,8 @@ export async function importListToDb( const kanjiLiteral = "kanjiLiteral" in card ? ((card as any).kanjiLiteral ?? null) : null; const entryId = kanjiLiteral != null ? 0 : card.entryId; await userDb.runAsync( - `INSERT OR IGNORE INTO srs_cards (id, entry_id, kanji_literal, list_id, due, stability, difficulty, elapsed_days, scheduled_days, reps, lapses, state, front_mode, back_mode, simple_stage, simple_n, simple_interval, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + `INSERT OR IGNORE INTO srs_cards (id, entry_id, kanji_literal, list_id, due, stability, difficulty, elapsed_days, scheduled_days, reps, lapses, state, front_mode, back_mode, simple_stage, simple_n, simple_interval, simple_relearning, simple_run, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, [ cardId, entryId, @@ -313,6 +329,8 @@ export async function importListToDb( card.stage, card.n, card.interval, + card.relearning ?? 0, + card.run ?? 0, now, now, ], diff --git a/lib/new-per-session.test.ts b/lib/new-per-session.test.ts new file mode 100644 index 0000000..b6ce25d --- /dev/null +++ b/lib/new-per-session.test.ts @@ -0,0 +1,39 @@ +/** + * The cap on unseen cards a session introduces, and how many cards in learning + * it takes on alongside them. + * + * The simple-SRS session used to load the whole list: 8,000 words meant 8,000 + * cards at once, none of which could collect the three correct answers a card + * needs to graduate, so the same queue came back every day. Midori, which this + * engine clones, mixes in about ten. + * + * These are the arithmetic. The QUERIES that apply them are gated by + * `db/simple-queue.test.ts`, which runs the shipped `loadSimpleQueuePools`: + * this file used to retype that SQL, and so proved nothing about the session. + */ +import { describe, expect, it } from "vitest"; + +import { + DEFAULT_NEW_PER_SESSION, + learningCapFor, + resolveNewPerSession, + SIMPLE_SRS_REQUIRED_CORRECT, + UNLIMITED, +} from "@/stores/simple-srs"; + +describe("resolveNewPerSession", () => { + it("falls back to the default when the list says nothing", () => { + expect(resolveNewPerSession(null)).toBe(DEFAULT_NEW_PER_SESSION); + expect(resolveNewPerSession(undefined)).toBe(DEFAULT_NEW_PER_SESSION); + }); + + it("never returns zero, which would send a session straight to the done screen", () => { + expect(resolveNewPerSession(0)).toBe(1); + expect(resolveNewPerSession(-5)).toBe(1); + }); + + it("keeps a chosen value and caps the absurd", () => { + expect(resolveNewPerSession(25)).toBe(25); + expect(resolveNewPerSession(10_000)).toBe(500); + }); +}); diff --git a/lib/study-wiring.test.ts b/lib/study-wiring.test.ts new file mode 100644 index 0000000..7ca0ca6 --- /dev/null +++ b/lib/study-wiring.test.ts @@ -0,0 +1,105 @@ +/** + * The study screen's wiring, checked in its source. + * + * This is a guard, not a behavioural test, and it exists because review proved + * the same thing three rounds running: no test imports + * `app/(tabs)/lists/study.tsx`, so the whole fix is one-line revertable with the + * full suite green. Moving the SQL into `db/simple-queue.ts` gated the SQL; it + * did not gate the screen *calling* it, and the commit message that claimed + * otherwise was wrong. + * + * Each assertion below corresponds to a mutation that was demonstrated to + * survive 2,570 passing tests: + * + * - `newCap = UNLIMITED` — the 8,000-card queue, restored. + * - deleting `simple_run = ?` from the persistence UPDATE — the graduation fix, + * fully reverted. + * - `newAllowance` → `newCap` — "Continue Studying" grants a fresh allowance. + * - deleting `newAnsweredRef.current.add` — the allowance never spends. + * + * A source check cannot prove behaviour, and a determined edit can satisfy it + * while breaking the screen. It catches the accidental revert, which is the one + * that actually happens. + * + * It lives in `lib/` and not beside the screen because `app/` is the + * expo-router route directory: a test file there is bundled as a route, and its + * `vitest` import drags Vite into the web build, which fails on a dynamic + * `import(filepath)` Metro cannot resolve. + */ +import { describe, expect, it } from "vitest"; +import { readFileSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), ".."); +const SOURCE = readFileSync(join(REPO_ROOT, "app/(tabs)/lists/study.tsx"), "utf8"); + +/** Source with comments stripped, so a comment cannot satisfy an assertion. */ +const CODE = SOURCE.replace(/\/\*[\s\S]*?\*\//g, "").replace(/^\s*\/\/.*$/gm, ""); + +describe("the study screen loads its pools through the tested module", () => { + it("calls loadSimpleQueuePools rather than its own SQL", () => { + expect(CODE).toContain("loadSimpleQueuePools({"); + // No hand-rolled copy of the queries it replaced. + expect(CODE).not.toMatch(/simple_stage\s+IS\s+NULL\s+AND\s+deleted_at/); + expect(CODE).not.toMatch(/simple_stage\s*=\s*0\s+AND\s+simple_n/); + }); + + it("resolves the cap from the list row instead of hard-coding one", () => { + expect(CODE).toMatch(/newCap\s*=\s*resolveNewPerSession\(/); + expect(CODE).toContain("SELECT new_per_session FROM lists WHERE id = ?"); + // The ephemeral exemption is the only unlimited path. + expect(CODE).toMatch(/isEphemeralListId\(listId\)/); + }); + + it("passes both the cap and what is left of it", () => { + const call = CODE.slice(CODE.indexOf("loadSimpleQueuePools({")); + const args = call.slice(0, call.indexOf("});")); + expect(args).toMatch(/\bnewCap\b/); + expect(args).toMatch(/\bnewAllowance\b/); + expect(args).toMatch(/excludeIds/); + }); + + it("computes the allowance from the cards it has answered", () => { + expect(CODE).toMatch( + /newAllowance\s*=[\s\S]{0,120}Math\.max\(0,\s*newCap\s*-\s*newAnsweredRef\.current\.size\)/, + ); + expect(CODE).toMatch(/newAnsweredRef\.current\.add\(card\.id\)/); + // And an undone answer is refunded, or a walk-back strands the session. + expect(CODE).toMatch(/newAnsweredRef\.current\.delete\(card\.id\)/); + }); +}); + +describe("the study screen persists what the engine decided", () => { + it("writes every field a simple-SRS answer produces", () => { + const update = CODE.slice(CODE.indexOf("UPDATE srs_cards SET simple_stage = ?")); + const statement = update.slice(0, update.indexOf("`")); + for (const column of [ + "simple_stage", + "simple_n", + "simple_interval", + "simple_relearning", + "simple_run", + ]) { + expect(statement).toContain(`${column} = ?`); + } + const binds = update.slice(0, update.indexOf("card.id,")); + for (const field of [ + "updates.simpleStage", + "updates.simpleN", + "updates.simpleInterval", + "updates.simpleRelearning", + "updates.simpleRun", + ]) { + expect(binds).toContain(field); + } + }); + + it("takes the decision from nextSimpleReview rather than recomputing it", () => { + expect(CODE).toMatch(/nextSimpleReview\(card,\s*action\)/); + // The lapse bug was this call site handing `simpleGraduate` a lifetime + // counter, so the screen must not reach for the engine's internals. + expect(CODE).not.toContain("simpleGraduate("); + expect(CODE).not.toContain("card.lapses > 0"); + }); +}); diff --git a/lib/version.ts b/lib/version.ts index fabaac3..b9dd68e 100644 --- a/lib/version.ts +++ b/lib/version.ts @@ -4,8 +4,8 @@ export const APP_VERSION = { version: "1.0.0", // Marketing version for app stores buildNumber: 23, // Native build number (increments only for native builds) - otaVersion: 32, // OTA update version (increments for JS updates) - timestamp: "2026-10-05T00:59:34Z", // Last update timestamp + otaVersion: 34, // OTA update version (increments for JS updates) + timestamp: "2026-10-12T08:11:02Z", // Last update timestamp channel: "testflight", // Release channel }; diff --git a/package.json b/package.json index 02762c8..0d3d37a 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,8 @@ "probe:names": "tsx scripts/probe-name-furigana.ts", "check:reading-order": "tsx scripts/reading-order-check.ts", "why:highlight": "tsx scripts/why-highlight.ts", - "proxy:bookmarks": "tsx scripts/make-proxy-bookmarks.ts" + "proxy:bookmarks": "tsx scripts/make-proxy-bookmarks.ts", + "simulate:srs": "tsx scripts/simulate-simple-srs.ts" }, "dependencies": { "@tradersamwise/jiten-reader-react-native": "0.1.0", diff --git a/scripts/simulate-simple-srs.ts b/scripts/simulate-simple-srs.ts new file mode 100644 index 0000000..aca7024 --- /dev/null +++ b/scripts/simulate-simple-srs.ts @@ -0,0 +1,373 @@ +/** + * Every number in docs/simple-srs-decisions.md, regenerated. + * + * yarn simulate:srs # all scenarios + * yarn simulate:srs ratchet # one, by name + * + * This exists because review could not reproduce the tables from their prose + * description: the answer-count spread, the daily budget and the intra-day + * re-queue rule all change the result, and none of them were written down. A + * number nobody else can produce is not a measurement, so the simulation + * ships and the doc quotes its output. + * + * Everything runs through the shipped `nextSimpleReview`, never a restatement + * of the rules — the lapse bug lived in a call site and survived every test + * that reimplemented the arithmetic. + */ + +import { + dateToSrsEpochDays, + nextSimpleReview, + learningCapFor, + UNLIMITED, +} from "../stores/simple-srs"; +import type { SrsCardRow } from "../db/types"; + +const DAY = 86_400_000; +const DECK = 8000; +/** Accuracy on graduated cards, and on cards still in learning. */ +const PASS_GRADUATED = 0.9; +const PASS_LEARNING = 0.8; + +/** + * Pass `--seed=N` to resample. A four-digit cell is one sample of a stochastic + * process: the conclusions here hold across seeds, the exact numbers do not, so + * re-run before quoting a new one. + */ +const SEED = Number(process.argv.find((a) => a.startsWith("--seed="))?.slice(7) ?? 7); + +/** Deterministic LCG, so every run of this file prints the same table. */ +function rng(seed: number) { + let s = seed; + return () => (s = (s * 1103515245 + 12345) & 0x7fffffff) / 0x7fffffff; +} + +type Card = SrsCardRow & { id: string }; + +function card(id: string, fields: Partial = {}): Card { + return { + id, + simpleStage: null, + simpleN: null, + simpleInterval: null, + simpleRelearning: 0, + simpleRun: 0, + reps: 0, + lapses: 0, + ...fields, + } as Card; +} + +/** + * One day of study. + * + * The queue is what the session loads: cards in learning (optionally capped) + * ahead of graduated cards due by the cutoff, then unseen cards up to the new + * cap. The learner answers up to `budget` cards; a card that fails or has not + * yet collected three correct answers is pushed back into the same day's queue, + * because the session re-queues it about ten cards away rather than ending. + */ +function studyOneDay( + deck: Card[], + day: number, + t0: number, + budget: number, + newPerDay: number, + learningCap: number, + rand: () => number, +): { shown: number; answered: number } { + const cutoff = dateToSrsEpochDays(new Date(t0 + (day + 1) * DAY)); + const learning = deck.filter((c) => c.simpleStage === 0 && (c.simpleN ?? 0) <= cutoff); + const graduated = deck.filter((c) => c.simpleStage === 1 && (c.simpleN ?? 0) <= cutoff); + const unseen = deck.filter((c) => c.simpleStage == null); + + const queue = [ + ...(learningCap === UNLIMITED ? learning : learning.slice(0, learningCap)), + ...graduated, + ...unseen.slice(0, newPerDay), + ]; + const shown = queue.length; + + let answered = 0; + while (queue.length && answered < budget) { + const c = queue.shift()!; + answered++; + const threshold = c.simpleStage === 1 ? PASS_GRADUATED : PASS_LEARNING; + const decision = nextSimpleReview(c, rand() < threshold ? "pass" : "fail"); + Object.assign(c, decision.updates); + c.reps = (c.reps ?? 0) + 1; + if (decision.updates.simpleRun === 0 && decision.updates.simpleStage === 0) { + c.lapses = (c.lapses ?? 0) + 1; + } + if (decision.graduated) { + c.simpleN = dateToSrsEpochDays(new Date(t0 + day * DAY)) + (c.simpleInterval ?? 0); + } else { + queue.push(c); + } + } + return { shown, answered }; +} + +interface Run { + shown: number[]; + answered: number[]; + /** Cards that reached stage 1 at any point. */ + graduatedEver: number; + /** Cards still graduated at the end, above a one-day interval — i.e. learned. */ + learnedAtEnd: number; +} + +function simulate(opts: { + deck: Card[]; + days: number; + budget: number; + newPerDay: number; + learningCap: number; + seed?: number; +}): Run { + const t0 = Date.now(); + const rand = rng(opts.seed ?? SEED); + const shown: number[] = []; + const answered: number[] = []; + const graduated = new Set(); + for (let day = 0; day < opts.days; day++) { + const result = studyOneDay( + opts.deck, + day, + t0, + opts.budget, + opts.newPerDay, + opts.learningCap, + rand, + ); + for (const c of opts.deck) if (c.simpleStage === 1) graduated.add(c.id); + shown.push(result.shown); + answered.push(result.answered); + } + // "Touched stage 1 once" flatters a deck that is churning: a card can + // graduate, fail, and sit back at the floor. What the learner has actually + // kept is a card still graduated and no longer due daily. + const learnedAtEnd = opts.deck.filter( + (c) => c.simpleStage === 1 && (c.simpleInterval ?? 0) > 1, + ).length; + return { shown, answered, graduatedEver: graduated.size, learnedAtEnd }; +} + +const MARKS = [0, 6, 29, 89, 179, 364]; +const at = (xs: number[]) => MARKS.map((d) => (xs[d] ?? 0).toString().padStart(5)); +const header = "| " + MARKS.map((d) => `d${d + 1}`.padStart(5)).join(" | ") + " |"; + +/** Floored: every card graduated but pinned at the 8-hour floor. */ +const flooredDeck = (interval: (i: number) => number) => + Array.from({ length: DECK }, (_, i) => + card(`f${i}`, { + simpleStage: 1, + simpleN: 0, + simpleInterval: interval(i), + reps: 40, + lapses: 5, + }), + ); + +/** Left in learning by the uncapped session: answered once, never graduated. */ +const learningDeck = () => + Array.from({ length: DECK }, (_, i) => + card(`l${i}`, { simpleStage: 0, simpleN: 0, simpleInterval: 1 / 3, reps: 14, lapses: 2 }), + ); + +const SCENARIOS: Record void> = { + /** The interval a card ends on after N answers, under each lapse rule. */ + ratchet() { + console.log("\n## The ratchet (one card, 20,000 samples, 90% accuracy)\n"); + console.log("| answers | at or below 1 day, bugged | with the fix |"); + console.log("| --- | --- | --- |"); + for (const answers of [20, 40, 45, 50]) { + const counts = [0, 0]; + for (const [slot, bugged] of [ + [0, true], + [1, false], + ] as const) { + for (let seed = 1; seed <= 20_000; seed++) { + const rand = rng(seed * 7919 + answers); + const c = card("c"); + for (let i = 0; i < answers; i++) { + const pass = rand() < 0.9; + // The bug: anything that ever failed counts as relearning, forever. + const seen = bugged ? { ...c, simpleRelearning: (c.lapses ?? 0) > 0 ? 1 : 0 } : c; + const d = nextSimpleReview(seen as SrsCardRow, pass ? "pass" : "fail"); + Object.assign(c, d.updates); + c.reps = (c.reps ?? 0) + 1; + if (!pass) c.lapses = (c.lapses ?? 0) + 1; + } + if ((c.simpleInterval ?? 0) <= 1) counts[slot]++; + } + } + const pct = (n: number) => `${((100 * n) / 20_000).toFixed(1)}%`; + console.log(`| ${answers} | ${pct(counts[0])} | ${pct(counts[1])} |`); + } + }, + + /** Does a flattened deck drain on its own, and does a repair help? */ + drain() { + console.log("\n## Draining a deck of 8,000 cards pinned at the floor\n"); + console.log(`(seed ${SEED})\n`); + console.log("| answers/day | repair | " + header.slice(2)); + console.log("| --- | --- |" + MARKS.map(() => " --- |").join("")); + for (const budget of [1000, 200, 150]) { + for (const [label, interval] of [ + ["none", () => 1 / 3], + ["spread 2-30d", (i: number) => 2 + (i % 29)], + ["flat 7d", () => 7], + ] as const) { + const run = simulate({ + deck: flooredDeck(interval), + days: 365, + budget, + newPerDay: 0, + learningCap: UNLIMITED, + seed: SEED, + }); + console.log(`| ${budget} | ${label} | ` + at(run.shown).join(" | ") + " |"); + } + } + }, + + /** Which rows a repair can tell apart from honest ones. */ + discriminate() { + console.log("\n## Telling a ratchet victim from a card honestly at the floor\n"); + // Reps only varies if cards have lived different lengths, so the sample is + // 400 seeds x nine answer counts = 3,600 cards, under each rule. + const ANSWER_COUNTS = [4, 8, 12, 16, 24, 32, 48, 64, 96]; + const population = (accuracy: number, bugged: boolean) => { + const out: Card[] = []; + for (let seed = 1; seed <= 400; seed++) { + for (const answers of ANSWER_COUNTS) { + const rand = rng(seed * 7919 + answers); + const c = card(`${seed}-${answers}`); + for (let i = 0; i < answers; i++) { + const pass = rand() < accuracy; + const seen = bugged ? { ...c, simpleRelearning: (c.lapses ?? 0) > 0 ? 1 : 0 } : c; + const d = nextSimpleReview(seen as SrsCardRow, pass ? "pass" : "fail"); + Object.assign(c, d.updates); + c.reps = (c.reps ?? 0) + 1; + if (!pass) c.lapses = (c.lapses ?? 0) + 1; + } + out.push(c); + } + } + return out; + }; + const floored = (c: Card) => (c.simpleInterval ?? 0) <= 0.34; + console.log("| accuracy | clause | victims caught | honest lifted |"); + console.log("| --- | --- | --- | --- |"); + for (const accuracy of [0.9, 0.75]) { + const victims = population(accuracy, true).filter(floored); + const honest = population(accuracy, false).filter(floored); + for (const [label, test] of [ + ["reps >= 3", (c: Card) => (c.reps ?? 0) >= 3], + ["reps >= 12", (c: Card) => (c.reps ?? 0) >= 12], + ["reps >= 20", (c: Card) => (c.reps ?? 0) >= 20], + ["lapses >= 3", (c: Card) => (c.lapses ?? 0) >= 3], + ] as const) { + const hit = (c: Card) => floored(c) && (c.lapses ?? 0) > 0 && test(c); + console.log( + `| ${accuracy} | interval <= 0.34, ${label} | ` + + `${victims.filter(hit).length} / ${victims.length} | ` + + `${honest.filter(hit).length} / ${honest.length} |`, + ); + } + const graduated = victims.filter((c) => c.simpleStage === 1); + const shipped = graduated.filter((c) => (c.reps ?? 0) >= 12 && (c.lapses ?? 0) > 0); + console.log( + `| ${accuracy} | **the shipped clause** (also \`simple_stage = 1\`) | ` + + `${shipped.length} / ${victims.length} | — |`, + ); + console.log( + `| ${accuracy} | *of which graduated* | ${graduated.length} / ${victims.length} | — |`, + ); + } + }, + + /** What capping the learning slice costs and saves. */ + learningCap() { + console.log(`(seed ${SEED})`); + console.log("\n## Capping the learning slice of a legacy deck\n"); + console.log( + "| learning cap | answers/day | " + header.slice(2) + " touched stage 1 | learned |", + ); + console.log("| --- | --- |" + MARKS.map(() => " --- |").join("") + " --- | --- |"); + for (const budget of [200, 400]) { + for (const cap of [UNLIMITED, learningCapFor(10), 10]) { + const run = simulate({ + deck: learningDeck(), + days: 365, + budget, + newPerDay: 0, + learningCap: cap, + // Every row of a comparison shares the seed, or the thing being + // compared is confounded with the sample: sampling each cap from its + // own stream made a cap of ten look ahead of thirty at 200 answers a + // day, which reversed at every other seed. + seed: SEED, + }); + const label = cap === UNLIMITED ? "none" : String(cap); + console.log( + `| ${label} | ${budget} | ` + + at(run.shown).join(" | ") + + ` | ${run.graduatedEver} | ${run.learnedAtEnd} |`, + ); + } + } + }, + + /** Is the x3 multiplier ahead of a flat cap, across seeds? */ + capSeeds() { + console.log("\n## The learning cap's multiplier, across seeds\n"); + console.log("| seed | answers/day | kept, cap 30 | kept, cap 10 | uncapped |"); + console.log("| --- | --- | --- | --- | --- |"); + for (const seed of [7, 11, 42, 99]) { + for (const budget of [200, 400]) { + const kept = (cap: number) => + simulate({ + deck: learningDeck(), + days: 365, + budget, + newPerDay: 0, + learningCap: cap, + seed, + }).learnedAtEnd; + console.log( + `| ${seed} | ${budget} | ${kept(learningCapFor(10))} | ${kept(10)} | ${kept(UNLIMITED)} |`, + ); + } + } + }, + + /** The steady state the fixed engine settles into on a fresh deck. */ + steady() { + console.log("\n## A fresh 8,000-card deck, ten new a day, fixed engine\n"); + const deck = Array.from({ length: DECK }, (_, i) => card(`n${i}`)); + const run = simulate({ + deck, + days: 365, + budget: 1000, + newPerDay: 10, + learningCap: learningCapFor(10), + }); + console.log("| | " + header.slice(2)); + console.log("| --- |" + MARKS.map(() => " --- |").join("")); + console.log("| shown | " + at(run.shown).join(" | ") + " |"); + console.log("| answered | " + at(run.answered).join(" | ") + " |"); + }, +}; + +const requested = process.argv.slice(2).filter((a) => !a.startsWith("-")); +const names = requested.length ? requested : Object.keys(SCENARIOS); +for (const name of names) { + const scenario = SCENARIOS[name]; + if (!scenario) { + console.error(`unknown scenario: ${name} (have: ${Object.keys(SCENARIOS).join(", ")})`); + process.exit(1); + } + scenario(); +} diff --git a/stores/lists.ts b/stores/lists.ts index f92cf18..a147385 100644 --- a/stores/lists.ts +++ b/stores/lists.ts @@ -45,6 +45,7 @@ export function parseListRow(row: any): WordList { voiceMode: Boolean(row.voiceMode ?? row.voice_mode ?? 0), typingMode: Boolean(row.typingMode ?? row.typing_mode ?? 0), mnemonicCloze: Boolean(row.mnemonicCloze ?? row.mnemonic_cloze ?? 0), + newPerSession: row.newPerSession ?? row.new_per_session ?? null, disableFlipAnimation: Boolean(row.disableFlipAnimation ?? row.disable_flip_animation ?? 0), disableSwipeAnimation: Boolean(row.disableSwipeAnimation ?? row.disable_swipe_animation ?? 0), learningSteps: (() => { diff --git a/stores/simple-srs.test.ts b/stores/simple-srs.test.ts index c1d511d..28f611e 100644 --- a/stores/simple-srs.test.ts +++ b/stores/simple-srs.test.ts @@ -1,5 +1,6 @@ import { describe, test, expect } from "vitest"; import { + nextSimpleReview, dateToSrsEpochDays, srsEpochDaysToDate, getSimpleDueDate, @@ -357,3 +358,168 @@ describe("session flow", () => { expect(result.simpleStage).toBe(1); }); }); + +/** + * The lapse multiplier belongs to the lapse, not to the card's history. + * + * It was scoped to `lapses > 0` — a lifetime counter nothing resets — so every + * graduation after a card's first ever failure halved the interval instead of + * multiplying it. A card climbed to 56 days, failed once, and then ratcheted + * 28 → 14 → 7 → 3.5 → … → the 8-hour floor, where it stayed due daily forever. + * Side by side with Midori on the same list, Midori's queue decayed and this + * one did not. + */ +describe("the lapse multiplier is scoped to the lapse", () => { + // `lapses: 3` is the whole point: this card HAS failed before. Under the old + // scoping that alone forced the halving, which is what these pin. + const card = (over: Partial = {}): SrsCardRow => + ({ + simpleStage: 1, + simpleN: 0, + simpleInterval: 56.61, + simpleRelearning: 0, + lapses: 3, + ...over, + }) as SrsCardRow; + + test("a pass while relearning halves once", () => { + expect(simpleGraduate(card({ simpleRelearning: 1 }), false, true).simpleInterval).toBeCloseTo( + 28.305, + 3, + ); + }); + + test("and clears the flag, so the next pass multiplies again", () => { + const first = simpleGraduate(card({ simpleRelearning: 1 }), false, true); + expect(first.simpleRelearning).toBe(0); + const second = simpleGraduate(card({ simpleInterval: first.simpleInterval }), false, false); + expect(second.simpleInterval).toBeCloseTo(28.305 * 1.9, 3); + }); + + test("a card that has lapsed in the past but is not relearning grows", () => { + expect(simpleGraduate(card(), false, false).simpleInterval).toBeCloseTo(56.61 * 1.9, 2); + }); + + test("ten passes after one lapse end far above the floor, not on it", () => { + let c = card({ simpleInterval: 56.61, simpleRelearning: 1 }); + let relearning = true; + for (let i = 0; i < 10; i++) { + const u = simpleGraduate(c, false, relearning); + relearning = u.simpleRelearning === 1; + c = card({ simpleInterval: u.simpleInterval, simpleRelearning: u.simpleRelearning }); + } + expect(c.simpleInterval).toBeGreaterThan(100); + }); + + test("a fail marks the card as relearning", () => { + expect(simpleReviewFail(card()).simpleRelearning).toBe(1); + }); + + test("a new card is not relearning", () => { + expect(simpleInitCard().simpleRelearning).toBe(0); + }); +}); + +/** + * The wiring, not the arithmetic. + * + * The lapse bug lived in which flag the study screen handed to simpleGraduate, + * and every test above calls simpleGraduate directly — so all of them stayed + * green with the screen still passing `lapses > 0`. These drive the whole + * answer the way the screen does. + */ +describe("one answer, decided the way the screen decides it", () => { + const graduated = (over: Partial = {}): SrsCardRow => + ({ + simpleStage: 1, + simpleN: 0, + simpleInterval: 56.61, + simpleRelearning: 0, + lapses: 3, + ...over, + }) as SrsCardRow; + + test("fail → three passes → the card recovers instead of ratcheting", () => { + let card = graduated(); + + const fail = nextSimpleReview(card, "fail"); + expect(fail.graduated).toBe(false); + expect(fail.updates.simpleRelearning).toBe(1); + card = { ...card, ...fail.updates }; + + for (let i = 0; i < 2; i++) { + const step = nextSimpleReview(card, "pass"); + expect(step.graduated).toBe(false); + // The lapse is still being served, so the flag has to survive the re-queue. + expect(step.updates.simpleRelearning).toBe(1); + card = { ...card, ...step.updates }; + } + + const third = nextSimpleReview(card, "pass"); + expect(third.graduated).toBe(true); + expect(third.updates.simpleInterval).toBeCloseTo(28.305, 3); // halved ONCE + expect(third.updates.simpleRelearning).toBe(0); + card = { ...card, ...third.updates }; + + // And the next review grows again — this is the whole fix. + const after = nextSimpleReview(card, "pass"); + expect(after.updates.simpleInterval).toBeCloseTo(28.305 * 1.9, 3); + }); + + test("a card that failed long ago but is not relearning grows", () => { + const step = nextSimpleReview(graduated(), "pass"); + expect(step.updates.simpleInterval).toBeCloseTo(56.61 * 1.9, 2); + }); + + test("easy graduates at once and still halves only inside a lapse", () => { + expect( + nextSimpleReview(graduated({ simpleRelearning: 1 }), "easy").updates.simpleInterval, + ).toBeCloseTo(28.305, 3); + expect(nextSimpleReview(graduated(), "easy").updates.simpleInterval).toBeCloseTo( + 56.61 * 2.6125, + 2, + ); + }); + + test("a new card counts up to three, then graduates", () => { + const fresh = { + simpleStage: null, + simpleInterval: null, + simpleRelearning: 0, + lapses: 0, + } as SrsCardRow; + const first = nextSimpleReview(fresh, "pass"); + expect(first.graduated).toBe(false); + expect(first.updates.simpleRun).toBe(1); + const once = { ...fresh, ...first.updates }; + const second = nextSimpleReview(once, "pass"); + expect(second.updates.simpleRun).toBe(2); + expect(nextSimpleReview({ ...once, ...second.updates }, "pass").graduated).toBe(true); + }); + + test("a graduated card due for review settles in one pass", () => { + expect(nextSimpleReview(graduated(), "pass").graduated).toBe(true); + }); +}); + +describe("the stage domain", () => { + test("writes only 0 and 1, which every consumer keys on", () => { + // `db/simple-queue.ts`, `DueCardsSection`, the lists index, `stores/lists.ts` + // and `lib/practice-stats.ts` all treat stage 1 as graduated and 0 as + // learning. A third value would be served by some and counted by none, so + // the invariant is pinned where it is produced. + const stages = new Set(); + const seen: SrsCardRow[] = [ + { simpleStage: null, simpleN: null, simpleInterval: null, simpleRelearning: 0, simpleRun: 0 }, + { simpleStage: 0, simpleN: 0, simpleInterval: 1 / 3, simpleRelearning: 0, simpleRun: 0 }, + { simpleStage: 0, simpleN: 0, simpleInterval: 1 / 3, simpleRelearning: 1, simpleRun: 2 }, + { simpleStage: 1, simpleN: 9000, simpleInterval: 56.61, simpleRelearning: 0, simpleRun: 0 }, + ] as unknown as SrsCardRow[]; + for (const card of seen) { + for (const action of ["pass", "easy", "fail"] as const) { + stages.add(nextSimpleReview(card, action).updates.simpleStage); + } + } + expect([...stages].sort()).toEqual([0, 1]); + }); +}); diff --git a/stores/simple-srs.ts b/stores/simple-srs.ts index 313e0f1..4383bb7 100644 --- a/stores/simple-srs.ts +++ b/stores/simple-srs.ts @@ -9,7 +9,11 @@ * - Due date = n (directly stored, not computed) * - Correct multiplier: ×1.9, Easy multiplier: ×2.6125 (1.9 × 1.375) * - Fail: preserve interval, reset correctCount, set n=0 (immediately due) - * - Re-graduation (3 correct after lapse): interval ×0.5, clamp [1/3, 365] + * - Re-graduation (3 correct after lapse): interval ×0.5, clamp [1/3, 365]. + * ONCE — the halving belongs to the lapse being recovered from, not to the + * card's history. Scoped to `lapses > 0` instead, every pass after a card's + * first ever failure halved it, so the interval ratcheted down to the floor + * and the card was due every 8 hours for the rest of its life. * - Normal graduation (never lapsed): interval ×1.9 (or ×2.6125), clamp [1/3, 365] * - Graduated cards due for review: pass once → done (recalculate interval) * - New/learning cards: need 3 correct in a row to graduate @@ -19,6 +23,16 @@ import type { SrsCardRow } from "@/db/types"; +/** Every field a simple-SRS review writes back to `srs_cards`. */ +export interface SimpleSrsUpdate { + simpleStage: number; + simpleN: number; + simpleInterval: number; + simpleRelearning: number; + /** Correct answers in a row, toward the three a learning card needs. */ + simpleRun: number; +} + const MAC_EPOCH_MS = new Date("2001-01-01T00:00:00Z").getTime(); const CORRECT_MULTIPLIER = 1.9; const EASY_MULTIPLIER = 1.9 * 1.375; // 2.6125 @@ -91,21 +105,17 @@ export function getEndOfLogicalDay(resetHour: number, now?: Date): Date { * * @param card - The SRS card being reviewed * @param easy - Whether this was an "easy" action (long-press pass) - * @param hasLapsed - Whether the card has ever been failed (lapses > 0) + * @param isRelearning - Whether the card is recovering from a lapse right now */ export function simpleGraduate( card: SrsCardRow, easy: boolean, - hasLapsed: boolean, -): { - simpleStage: number; - simpleN: number; - simpleInterval: number; -} { + isRelearning: boolean, +): SimpleSrsUpdate { const now = dateToSrsEpochDays(); let newInterval: number; - if (hasLapsed) { + if (isRelearning) { // Re-graduating after a lapse: halve the interval, clamp [1/3, 365] const currentInterval = card.simpleInterval ?? INITIAL_INTERVAL; newInterval = Math.max(MIN_INTERVAL, Math.min(currentInterval * 0.5, MAX_INTERVAL)); @@ -120,35 +130,120 @@ export function simpleGraduate( simpleStage: 1, simpleN: now + newInterval, // n = due date simpleInterval: newInterval, + // The lapse this graduation recovered from is paid for. + simpleRelearning: 0, + simpleRun: 0, }; } /** Process a fail review — returns updated simple SRS fields */ -export function simpleReviewFail(card: SrsCardRow): { - simpleStage: number; - simpleN: number; - simpleInterval: number; -} { +export function simpleReviewFail(card: SrsCardRow): SimpleSrsUpdate { // Fail: preserve interval, set stage to 0 (learning), n=0 (immediately due) return { simpleStage: 0, simpleN: 0, simpleInterval: card.simpleInterval ?? INITIAL_INTERVAL, + simpleRelearning: 1, + simpleRun: 0, }; } /** Initialize a new card for simple SRS (first time seeing it) */ -export function simpleInitCard(): { - simpleStage: number; - simpleN: number; - simpleInterval: number; -} { +export function simpleInitCard(): SimpleSrsUpdate { return { simpleStage: 0, simpleN: 0, // immediately due (in learning) simpleInterval: INITIAL_INTERVAL, + simpleRelearning: 0, + simpleRun: 0, }; } /** Number of correct answers needed to graduate from learning */ export const SIMPLE_SRS_REQUIRED_CORRECT = REQUIRED_CORRECT; + +/** + * Unseen cards a session introduces when the list does not say otherwise. + * Midori mixes in about ten; the whole list at once is what kept 8,000 cards + * in the queue every day, since none of them could reach three correct answers. + */ +export const DEFAULT_NEW_PER_SESSION = 10; + +/** What a list's `newPerSession` means, with the floor and ceiling applied. */ +export function resolveNewPerSession(value: number | null | undefined): number { + if (value == null || !Number.isFinite(value)) return DEFAULT_NEW_PER_SESSION; + return Math.max(1, Math.min(Math.floor(value), 500)); +} + +/** SQLite's own "no limit" for a bound LIMIT, used by the ephemeral lists. */ +export const UNLIMITED = -1; + +/** + * How many cards in learning a session takes on, given its new-card cap. + * + * A learning card needs three correct answers, so three learning slots are one + * new card's worth of work. Without a bound, a deck left by the uncapped + * session puts every stage-0 card in the queue at once — all of them due, none + * of them finishable — which is the queue that came back unchanged every day. + */ +export function learningCapFor(newPerSession: number): number { + return newPerSession === UNLIMITED ? UNLIMITED : newPerSession * REQUIRED_CORRECT; +} + +/** What a single simple-SRS answer does to a card. */ +export interface SimpleSrsDecision { + updates: SimpleSrsUpdate; + /** Whether the card leaves the session, or is re-queued to be asked again. */ + graduated: boolean; +} + +/** + * Decide one simple-SRS answer. + * + * Extracted from the study screen so the WIRING is testable: the lapse bug + * lived here, in which flag the call site handed to `simpleGraduate`, and a + * test that only called `simpleGraduate` could not see it. + */ +export function nextSimpleReview( + card: SrsCardRow, + action: "pass" | "easy" | "fail", +): SimpleSrsDecision { + const isNew = card.simpleStage == null; + const relearning = card.simpleRelearning === 1; + + if (action === "fail") { + return { updates: isNew ? simpleInitCard() : simpleReviewFail(card), graduated: false }; + } + + if (action === "easy") { + // Easy skips the correct-in-a-row gate and graduates at once. + const base = isNew ? { ...card, ...simpleInitCard() } : card; + return { updates: simpleGraduate(base, true, relearning), graduated: true }; + } + + if (isNew) { + // First sight: start the card and start counting. + return { updates: { ...simpleInitCard(), simpleRun: 1 }, graduated: false }; + } + + if (card.simpleStage === 1) { + // Graduated and due: one correct answer settles it. + return { updates: simpleGraduate(card, false, relearning), graduated: true }; + } + + const run = (card.simpleRun ?? 0) + 1; + if (run >= REQUIRED_CORRECT) { + return { updates: simpleGraduate(card, false, relearning), graduated: true }; + } + return { + updates: { + simpleStage: 0, + simpleN: 0, // immediately due + simpleInterval: card.simpleInterval ?? 0, + // Still inside the lapse it is recovering from, if it was in one. + simpleRelearning: card.simpleRelearning ?? 0, + simpleRun: run, + }, + graduated: false, + }; +}