From dd08a09860ea1b789a215d6c9e0b054153e72cf4 Mon Sep 17 00:00:00 2001 From: Trey T Date: Sun, 17 May 2026 15:59:33 -0500 Subject: [PATCH] =?UTF-8?q?Vocab=20Practice=20=E2=80=94=20apply=20code-rev?= =?UTF-8?q?iew=20fixes=20for=20the=20SRS=20session=20work?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the lead-review findings on the new vocab SRS/persistence code: - Resume a persisted study group only when every stored verb resolves, else rebuild fresh — a partial resume desynced learnedCount from a shrunken queue and then persisted the loss. - studyAgain() clears the finished group before building a new one. - VocabStudyGroupStore.persist() collapses duplicate group records (cross-device sync races) down to the newest. - Learn mode now derives its verb list from the live quiz queue, so it browses exactly what's left to learn instead of a stale frozen pool. - VerbExampleGenerator retries when the first batch throws, not only when it returns bad data. - Per-verb example generation tracks in-flight verbs in a Set, so the loading spinner clears reliably and duplicate generations are skipped. Co-Authored-By: Claude Opus 4.7 (1M context) --- Conjuga/Conjuga/Models/VocabStudyGroup.swift | 17 +++++-- .../Services/VerbExampleGenerator.swift | 7 ++- .../Conjuga/Services/VocabSessionQueue.swift | 4 ++ .../Vocab/VocabFlashcardPracticeView.swift | 51 ++++++++++--------- .../VocabMultipleChoicePracticeView.swift | 18 +++---- 5 files changed, 57 insertions(+), 40 deletions(-) diff --git a/Conjuga/Conjuga/Models/VocabStudyGroup.swift b/Conjuga/Conjuga/Models/VocabStudyGroup.swift index dd6bc7e..90301f4 100644 --- a/Conjuga/Conjuga/Models/VocabStudyGroup.swift +++ b/Conjuga/Conjuga/Models/VocabStudyGroup.swift @@ -54,12 +54,21 @@ struct VocabStudyGroupStore { return (try? context.fetch(descriptor))?.first } - /// Write the in-progress group, creating it if needed. + /// Write the in-progress group, creating it if needed. If duplicate records + /// exist (two devices created a group before sync settled), the newest is + /// updated and the rest deleted so a single record survives. func persist(entries: [StoredVocabEntry], learnedCount: Int) { let data = (try? JSONEncoder().encode(entries)) ?? Data() - if let group = activeGroup() { - group.entriesJSON = data - group.learnedCount = learnedCount + let id = VocabStudyGroup.activeID + let descriptor = FetchDescriptor( + predicate: #Predicate { $0.id == id }, + sortBy: [SortDescriptor(\VocabStudyGroup.createdAt, order: .reverse)] + ) + let existing = (try? context.fetch(descriptor)) ?? [] + if let newest = existing.first { + newest.entriesJSON = data + newest.learnedCount = learnedCount + for duplicate in existing.dropFirst() { context.delete(duplicate) } } else { context.insert(VocabStudyGroup(entriesJSON: data, learnedCount: learnedCount)) } diff --git a/Conjuga/Conjuga/Services/VerbExampleGenerator.swift b/Conjuga/Conjuga/Services/VerbExampleGenerator.swift index 1e62064..3bd05ce 100644 --- a/Conjuga/Conjuga/Services/VerbExampleGenerator.swift +++ b/Conjuga/Conjuga/Services/VerbExampleGenerator.swift @@ -54,12 +54,15 @@ struct VerbExampleGenerator { tenseIds: [String], formsByTense: [String: [String]] ) async throws -> [VerbExample] { - let firstPass = try await generateBatch( + // A thrown error here (model busy, context overflow) shouldn't abort the + // whole generation — treat it as a fully-failed batch so the retry below + // still runs, same as the retry path's own `try?`. + let firstPass = (try? await generateBatch( verbInfinitive: verbInfinitive, verbEnglish: verbEnglish, tenseIds: tenseIds, formsByTense: formsByTense - ) + )) ?? [:] var valid: [String: VerbExample] = [:] var failedTenses: [String] = [] diff --git a/Conjuga/Conjuga/Services/VocabSessionQueue.swift b/Conjuga/Conjuga/Services/VocabSessionQueue.swift index 82654df..776ec50 100644 --- a/Conjuga/Conjuga/Services/VocabSessionQueue.swift +++ b/Conjuga/Conjuga/Services/VocabSessionQueue.swift @@ -82,6 +82,10 @@ struct VocabSessionQueue { var entry = queue.removeFirst() switch rating { + // Again/Hard always (re)set the card to `.learning`. A card already in + // `.review` therefore drops back a step — an intentional lapse, matching + // Anki: missing a card you'd previously passed sends it back through the + // learning steps rather than letting it graduate. case .again: entry.state = .learning insert(entry, offset: Int.random(in: 5...8)) diff --git a/Conjuga/Conjuga/Views/Practice/Vocab/VocabFlashcardPracticeView.swift b/Conjuga/Conjuga/Views/Practice/Vocab/VocabFlashcardPracticeView.swift index 7b0ee20..29f6e9b 100644 --- a/Conjuga/Conjuga/Views/Practice/Vocab/VocabFlashcardPracticeView.swift +++ b/Conjuga/Conjuga/Views/Practice/Vocab/VocabFlashcardPracticeView.swift @@ -33,18 +33,22 @@ struct VocabFlashcardPracticeView: View { @AppStorage("vocabFlashcardMode") private var modeRaw: String = Mode.quiz.rawValue - /// The fetched session pool (due-first + new, capped). Quiz mode feeds it - /// into `VocabSessionQueue`; Learn mode walks it directly on a loop. - @State private var sessionVerbs: [Verb] = [] @State private var session: VocabSessionQueue? @State private var learnIndex: Int = 0 @State private var revealed: Bool = false @State private var exampleByVerbId: [Int: VerbExample] = [:] - @State private var generatingExampleForVerbId: Int? = nil + @State private var generatingVerbIds: Set = [] @State private var speech = SpeechService() private var cloudContext: ModelContext { cloudModelContextProvider() } + /// The session's un-graduated verbs, derived live from the quiz queue so + /// Learn mode walks exactly what's left to learn — it stays in sync as + /// quiz mode graduates cards rather than browsing a stale frozen pool. + private var sessionVerbs: [Verb] { + session?.queue.map(\.verb) ?? [] + } + private var mode: Mode { get { Mode(rawValue: modeRaw) ?? .quiz } nonmutating set { modeRaw = newValue.rawValue } @@ -370,7 +374,7 @@ struct VocabFlashcardPracticeView: View { .frame(maxWidth: .infinity, alignment: .leading) .padding() .background(.fill.quinary, in: RoundedRectangle(cornerRadius: 12)) - } else if generatingExampleForVerbId == verb.id { + } else if generatingVerbIds.contains(verb.id) { HStack(spacing: 8) { ProgressView() Text("Generating example…") @@ -386,13 +390,13 @@ struct VocabFlashcardPracticeView: View { // MARK: - Logic private func loadIfNeeded() { - guard sessionVerbs.isEmpty else { return } + guard session == nil else { return } switch kind { case .standard: loadStandardSession() case .reviewLearned: - sessionVerbs = VocabVerbPool.reviewLearnedVerbs(localContext: localContext, cloudContext: cloudContext) - session = VocabSessionQueue(verbs: sessionVerbs) + let verbs = VocabVerbPool.reviewLearnedVerbs(localContext: localContext, cloudContext: cloudContext) + session = VocabSessionQueue(verbs: verbs) } primeExampleForCurrent() } @@ -409,17 +413,18 @@ struct VocabFlashcardPracticeView: View { guard let verb = byId[e.verbId] else { return nil } return (verb, VocabSessionQueue.CardState(rawValue: e.state) ?? .new) } - if !entries.isEmpty { - sessionVerbs = entries.map(\.verb) + // Resume only if EVERY stored verb resolved. A partial resume + // would desync learnedCount from a shrunken queue and then + // persist that loss — fall through to a fresh rebuild instead. + if entries.count == stored.count { session = VocabSessionQueue(entries: entries, learnedCount: group.learnedCount) return } } } - // No active group — start fresh and persist immediately so closing - // the app right away still resumes this set. + // No active group (or it couldn't be fully resolved) — start fresh and + // persist immediately so closing the app right away still resumes this set. let verbs = VocabVerbPool.sessionVerbs(localContext: localContext, cloudContext: cloudContext) - sessionVerbs = verbs session = VocabSessionQueue(verbs: verbs) persistGroup() } @@ -449,10 +454,10 @@ struct VocabFlashcardPracticeView: View { private func studyAgain() { switch kind { case .standard: - // The finished group was already cleared by persistGroup(); - // build and persist a fresh set. + // Clear the finished group explicitly before building a fresh one, + // so a fresh set is never appended onto a stale group record. + VocabStudyGroupStore(context: cloudContext).clear() let verbs = VocabVerbPool.sessionVerbs(localContext: localContext, cloudContext: cloudContext) - sessionVerbs = verbs session = VocabSessionQueue(verbs: verbs) persistGroup() case .reviewLearned: @@ -478,15 +483,15 @@ struct VocabFlashcardPracticeView: View { private func primeExampleForCurrent() { guard let verb = currentVerb else { return } - if exampleByVerbId[verb.id] != nil { return } + let verbId = verb.id + if exampleByVerbId[verbId] != nil || generatingVerbIds.contains(verbId) { return } - if let cached = exampleCache.examples(for: verb.id)?.first { - exampleByVerbId[verb.id] = cached + if let cached = exampleCache.examples(for: verbId)?.first { + exampleByVerbId[verbId] = cached return } guard VerbExampleGenerator.isAvailable else { return } - generatingExampleForVerbId = verb.id - let verbId = verb.id + generatingVerbIds.insert(verbId) let infinitive = verb.infinitive let english = verb.english let formsByTense = ReferenceStore(context: localContext) @@ -507,9 +512,7 @@ struct VocabFlashcardPracticeView: View { } catch { // Silent — the example block just stays hidden. } - if generatingExampleForVerbId == verbId { - generatingExampleForVerbId = nil - } + generatingVerbIds.remove(verbId) } } } diff --git a/Conjuga/Conjuga/Views/Practice/Vocab/VocabMultipleChoicePracticeView.swift b/Conjuga/Conjuga/Views/Practice/Vocab/VocabMultipleChoicePracticeView.swift index 9011074..cf5d054 100644 --- a/Conjuga/Conjuga/Views/Practice/Vocab/VocabMultipleChoicePracticeView.swift +++ b/Conjuga/Conjuga/Views/Practice/Vocab/VocabMultipleChoicePracticeView.swift @@ -17,7 +17,7 @@ struct VocabMultipleChoicePracticeView: View { @State private var options: [Verb] = [] @State private var selectedOption: Verb? = nil @State private var exampleByVerbId: [Int: VerbExample] = [:] - @State private var generatingExampleForVerbId: Int? = nil + @State private var generatingVerbIds: Set = [] @State private var speech = SpeechService() private var cloudContext: ModelContext { cloudModelContextProvider() } @@ -140,7 +140,7 @@ struct VocabMultipleChoicePracticeView: View { .frame(maxWidth: .infinity, alignment: .leading) .padding() .background(.fill.quinary, in: RoundedRectangle(cornerRadius: 12)) - } else if generatingExampleForVerbId == verb.id { + } else if generatingVerbIds.contains(verb.id) { HStack(spacing: 8) { ProgressView() Text("Generating example…") @@ -264,14 +264,14 @@ struct VocabMultipleChoicePracticeView: View { private func primeExampleForCurrent() { guard let verb = currentVerb else { return } - if exampleByVerbId[verb.id] != nil { return } - if let cached = exampleCache.examples(for: verb.id)?.first { - exampleByVerbId[verb.id] = cached + let verbId = verb.id + if exampleByVerbId[verbId] != nil || generatingVerbIds.contains(verbId) { return } + if let cached = exampleCache.examples(for: verbId)?.first { + exampleByVerbId[verbId] = cached return } guard VerbExampleGenerator.isAvailable else { return } - generatingExampleForVerbId = verb.id - let verbId = verb.id + generatingVerbIds.insert(verbId) let infinitive = verb.infinitive let english = verb.english let formsByTense = ReferenceStore(context: localContext) @@ -290,9 +290,7 @@ struct VocabMultipleChoicePracticeView: View { exampleByVerbId[verbId] = pick } } catch {} - if generatingExampleForVerbId == verbId { - generatingExampleForVerbId = nil - } + generatingVerbIds.remove(verbId) } } }