From d0f28efff8c1fe2d3b02da4d82b96020510511fa Mon Sep 17 00:00:00 2001 From: Mircea Lungu Date: Wed, 15 Jul 2026 21:59:36 +0300 Subject: [PATCH 1/3] Fix three SR exercise-outcome crashes from the 2026-07-15 log digest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reporting an exercise outcome could crash in three ways, all in the word-scheduling path behind /report_exercise_outcome (the broad except in the endpoint turned each into a "FAIL" + logged traceback, silently dropping the user's exercise result): 1. "Instance UserWord has been deleted" — report_exercise_outcome built the Exercise bound to `self`, then ran the scheduler, whose lazy translation-validation (find_or_create -> validate_and_fix -> _fix_bookmark -> cleanup_old_user_word) can move the word to a corrected meaning and delete the original UserWord. The Exercise (and caller) were left pointing at a deleted instance. Now the scheduler runs first and returns the surviving UserWord; the Exercise is logged against that. 2. Duplicate 'unique_user_word_schedule' IntegrityError — two concurrent reports for the same new word both passed the "no schedule yet" check and inserted. FourLevelsPerWord.find_or_create now catches the IntegrityError, rolls back, and returns the row the winning request created. 3. "'NoneType' object has no attribute 'update_schedule'" — find_or_create returns None when the word is judged unfit (invalid / duplicate / unvalidatable translation); BasicSRSchedule.update dereferenced it. Now guarded. BasicSRSchedule.update returns the scheduled UserWord so callers never hold a stale/deleted reference. Adds three regression tests that each fail on the pre-fix code. Co-Authored-By: Claude Opus 4.8 --- zeeguu/core/model/user_word.py | 19 ++- zeeguu/core/test/test_scheduling.py | 134 ++++++++++++++++++ .../core/word_scheduling/basicSR/basicSR.py | 31 +++- .../basicSR/four_levels_per_word.py | 13 +- 4 files changed, 187 insertions(+), 10 deletions(-) diff --git a/zeeguu/core/model/user_word.py b/zeeguu/core/model/user_word.py index 622988037..00379a069 100644 --- a/zeeguu/core/model/user_word.py +++ b/zeeguu/core/model/user_word.py @@ -379,6 +379,15 @@ def report_exercise_outcome( if not time: time = datetime.now() + + # Update the schedule FIRST. The lazy translation-validation inside the + # scheduler may replace this UserWord with a corrected one and delete + # `self`; scheduler.update() returns the UserWord that survived. We must + # log the Exercise against that survivor — binding it to a deleted `self` + # raised "Instance UserWord has been deleted" (SR log digest 2026-07-15). + scheduler = self.get_scheduler() + practiced_user_word = scheduler.update(db_session, self, exercise_outcome, time) + from zeeguu.core.model import Exercise exercise = Exercise( @@ -387,18 +396,16 @@ def report_exercise_outcome( solving_speed, time, session_id, - self, + practiced_user_word, other_feedback, ) db_session.add(exercise) if source.source != "DAILY_AUDIO_LESSON" and exercise.is_correct(): from zeeguu.core import events - events.exercise_correct.send(None, user_id=self.user.id, db_session=db_session) - - - scheduler = self.get_scheduler() - scheduler.update(db_session, self, exercise_outcome, time) + events.exercise_correct.send( + None, user_id=practiced_user_word.user.id, db_session=db_session + ) db_session.commit() diff --git a/zeeguu/core/test/test_scheduling.py b/zeeguu/core/test/test_scheduling.py index 815b8384e..77444069a 100644 --- a/zeeguu/core/test/test_scheduling.py +++ b/zeeguu/core/test/test_scheduling.py @@ -342,3 +342,137 @@ def _new_schedule_after_exercise(self, bookmark, outcome, date: datetime = None) bookmark.user_word.get_scheduler(), bookmark.user_word, db_session ).schedule return schedule + + # ================================================================================================================ + # Regression tests for the SR exercise-outcome failures in the 2026-07-15 log digest + # ================================================================================================================ + + def test_update_returns_user_word_when_scheduler_declines(self): + """ + find_or_create() returns None when a word is judged unfit for study + (invalid / duplicate / unvalidatable translation). update() must not + dereference that None ('NoneType' has no attribute 'update_schedule') + and must return the passed-in user_word. + """ + from unittest.mock import patch + from zeeguu.core.word_scheduling.basicSR.basicSR import BasicSRSchedule + + bookmark = BookmarkRule(self.four_levels_user).bookmark + user_word = bookmark.user_word + scheduler = user_word.get_scheduler() + + with patch.object(scheduler, "find_or_create", return_value=None): + result = scheduler.update( + db_session, user_word, OutcomeRule().correct.outcome, datetime.now() + ) + + assert result is user_word + # nothing should have been scheduled + assert BasicSRSchedule.find_by_user_word(user_word) is None + + def test_report_outcome_logs_exercise_against_surviving_word(self): + """ + The lazy translation-validation inside the scheduler can replace the + practiced UserWord with a corrected one and delete the original. The + Exercise must be logged against the survivor, not the deleted original + (which raised 'Instance UserWord has been deleted'). + """ + from unittest.mock import patch + from zeeguu.core.model.meaning import Meaning + from zeeguu.core.model.exercise import Exercise + from zeeguu.core.model.user_word import UserWord + from zeeguu.core.llm_services.validation_service import ( + UserWordValidationService, + ) + + old_bookmark = BookmarkRule(self.four_levels_user).bookmark + old_user_word = old_bookmark.user_word + # force the validation path inside find_or_create + old_user_word.meaning.validated = Meaning.NOT_VALIDATED + db_session.add(old_user_word.meaning) + + new_bookmark = BookmarkRule(self.four_levels_user).bookmark + new_user_word = new_bookmark.user_word + new_user_word.meaning.validated = Meaning.VALID + db_session.add(new_user_word.meaning) + db_session.commit() + new_user_word_id = new_user_word.id + old_user_word_id = old_user_word.id + + def fake_validate_and_fix(db_sess, user_word): + # Simulate a validation-fix that moves to a different meaning: + # delete the original UserWord and hand back the corrected one. + db_sess.delete(user_word) + db_sess.commit() + return UserWord.query.get(new_user_word_id) + + with patch.object( + UserWordValidationService, + "validate_and_fix", + side_effect=fake_validate_and_fix, + ): + # Must not raise "Instance UserWord has been deleted" + old_user_word.report_exercise_outcome( + db_session, + "Recognize", + OutcomeRule().correct.outcome, + 1000, + None, + "", + ) + + # the original was deleted, the exercise is logged against the survivor + assert UserWord.query.get(old_user_word_id) is None + logged = Exercise.query.filter_by(user_word_id=new_user_word_id).all() + assert len(logged) == 1 + assert ( + Exercise.query.filter_by(user_word_id=old_user_word_id).count() == 0 + ) + + def test_find_or_create_recovers_from_duplicate_schedule_race(self): + """ + Two concurrent requests can both pass the "no schedule yet" check and + try to insert. The unique_user_word_schedule constraint makes the loser's + commit raise IntegrityError; find_or_create must roll back and return the + row the winner created instead of surfacing a 500. + """ + from unittest.mock import patch + from sqlalchemy.exc import IntegrityError + from zeeguu.core.model.meaning import Meaning + from zeeguu.core.word_scheduling.basicSR.basicSR import BasicSRSchedule + from zeeguu.core.word_scheduling.basicSR.four_levels_per_word import ( + FourLevelsPerWord, + ) + + bookmark = BookmarkRule(self.four_levels_user).bookmark + user_word = bookmark.user_word + user_word.meaning.validated = Meaning.VALID # skip validation branch + db_session.add(user_word.meaning) + db_session.commit() + + # The "winner" row that a concurrent request already committed. + winner = FourLevelsPerWord(user_word=user_word) + db_session.add(winner) + db_session.commit() + winner_id = winner.id + + original_commit = db_session.commit + + # find() returns None first (our SELECT missed the winner), then the + # winner on the post-rollback re-fetch. commit() raises once, as if the + # unique constraint fired on our duplicate insert. + with patch.object( + BasicSRSchedule, "find", side_effect=[None, winner] + ), patch.object( + db_session, + "commit", + side_effect=IntegrityError("insert", {}, Exception("duplicate")), + ): + result = FourLevelsPerWord.find_or_create(db_session, user_word) + + # restore and make sure nothing extra was written + assert result is not None + assert result.id == winner_id + assert ( + BasicSRSchedule.query.filter_by(user_word_id=user_word.id).count() == 1 + ) diff --git a/zeeguu/core/word_scheduling/basicSR/basicSR.py b/zeeguu/core/word_scheduling/basicSR/basicSR.py index d125a5439..3839b8bed 100644 --- a/zeeguu/core/word_scheduling/basicSR/basicSR.py +++ b/zeeguu/core/word_scheduling/basicSR/basicSR.py @@ -95,6 +95,17 @@ def find(cls, user_word): @classmethod def update(cls, db_session, user_word, outcome, time: datetime = None): + """ + Record the effect of an exercise outcome on the schedule. + + Returns the UserWord that was actually scheduled. This is normally the + `user_word` passed in, but the lazy translation-validation inside + find_or_create() may replace it with a corrected UserWord (deleting the + original). Callers that keep a reference to `user_word` — e.g. to log an + Exercise against it — MUST use the returned value instead; binding a row + to the now-deleted original raised "Instance UserWord has been deleted" + (SR log digest 2026-07-15). + """ if not time: time = datetime.now() @@ -112,7 +123,7 @@ def update(cls, db_session, user_word, outcome, time: datetime = None): user_word.user_preference = UserWordExPreference.DONT_USE_IN_EXERCISES db_session.add(user_word) - return + return user_word correctness = ExerciseOutcome.is_correct(outcome) @@ -125,18 +136,32 @@ def update(cls, db_session, user_word, outcome, time: datetime = None): if schedule and schedule.there_was_no_need_for_practice_on_date(time): # nothing to update in this case - return + return user_word if not schedule and more_scheduled_words_than_user_prefers: # we are not adding this word to scheduled words - return + return user_word # pipeline is not full, and the word was not scheduled before if not schedule and not more_scheduled_words_than_user_prefers: schedule = cls.find_or_create(db_session, user_word) + # find_or_create returns None when the word was judged unfit for study + # (invalid / duplicate / unvalidatable translation) — there is nothing + # to schedule, so don't dereference it (guards the 'NoneType' has no + # attribute 'update_schedule' crash from the same digest). + if schedule is None: + return user_word + + # Capture the scheduled UserWord *before* update_schedule: find_or_create + # may have swapped in a corrected UserWord, and update_schedule may delete + # the schedule row (set_meaning_as_learned), detaching it afterwards. + scheduled_user_word = schedule.user_word + schedule.update_schedule(db_session, correctness, time) + return scheduled_user_word + @classmethod def user_words_not_scheduled(cls, user, limit): # Import here to avoid circular imports diff --git a/zeeguu/core/word_scheduling/basicSR/four_levels_per_word.py b/zeeguu/core/word_scheduling/basicSR/four_levels_per_word.py index 2019a837b..35a5aa3bd 100644 --- a/zeeguu/core/word_scheduling/basicSR/four_levels_per_word.py +++ b/zeeguu/core/word_scheduling/basicSR/four_levels_per_word.py @@ -1,6 +1,8 @@ from .basicSR import ONE_DAY, BasicSRSchedule from datetime import datetime, timedelta +from sqlalchemy.exc import IntegrityError + from ...model import UserWord MAX_LEVEL = 4 @@ -130,6 +132,15 @@ def find_or_create(cls, db_session, user_word): schedule = cls(user_word) user_word.level = 1 db_session.add_all([schedule, user_word]) - db_session.commit() + try: + db_session.commit() + except IntegrityError: + # A concurrent request already created the schedule for this + # user_word (the unique_user_word_schedule constraint fired). + # Roll back our duplicate insert and use the existing row. + # (SR log digest 2026-07-15: duplicate entry for key + # 'unique_user_word_schedule'.) + db_session.rollback() + schedule = super(FourLevelsPerWord, cls).find(user_word) return schedule From 3b3feb0b744adc01305ba29abb5b3e9f6f6632d5 Mon Sep 17 00:00:00 2001 From: Mircea Lungu Date: Wed, 15 Jul 2026 22:31:46 +0300 Subject: [PATCH 2/3] Declare unique_user_word_schedule on the model; test the real constraint The uniqueness of basic_sr_schedule.user_word_id lived only in the migration SQL, so the SQLite test DB never enforced it. Declaring it in __table_args__ documents the invariant next to the column and lets the test DB enforce it. With the constraint now live in tests, the duplicate-schedule-race regression test exercises the *real* IntegrityError instead of a mocked one. Co-Authored-By: Claude Opus 4.8 --- zeeguu/core/test/test_scheduling.py | 20 ++++++------------- .../core/word_scheduling/basicSR/basicSR.py | 9 ++++++++- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/zeeguu/core/test/test_scheduling.py b/zeeguu/core/test/test_scheduling.py index 77444069a..4ff9f61db 100644 --- a/zeeguu/core/test/test_scheduling.py +++ b/zeeguu/core/test/test_scheduling.py @@ -437,7 +437,6 @@ def test_find_or_create_recovers_from_duplicate_schedule_race(self): row the winner created instead of surfacing a 500. """ from unittest.mock import patch - from sqlalchemy.exc import IntegrityError from zeeguu.core.model.meaning import Meaning from zeeguu.core.word_scheduling.basicSR.basicSR import BasicSRSchedule from zeeguu.core.word_scheduling.basicSR.four_levels_per_word import ( @@ -456,21 +455,14 @@ def test_find_or_create_recovers_from_duplicate_schedule_race(self): db_session.commit() winner_id = winner.id - original_commit = db_session.commit - - # find() returns None first (our SELECT missed the winner), then the - # winner on the post-rollback re-fetch. commit() raises once, as if the - # unique constraint fired on our duplicate insert. - with patch.object( - BasicSRSchedule, "find", side_effect=[None, winner] - ), patch.object( - db_session, - "commit", - side_effect=IntegrityError("insert", {}, Exception("duplicate")), - ): + # Force the create path — as if our SELECT ran before the winner + # committed — so find() returns None first and then the winner on the + # post-rollback re-fetch. The real unique_user_word_schedule constraint + # then fires on our duplicate insert; find_or_create must roll back and + # return the winner's row instead of surfacing a 500. + with patch.object(BasicSRSchedule, "find", side_effect=[None, winner]): result = FourLevelsPerWord.find_or_create(db_session, user_word) - # restore and make sure nothing extra was written assert result is not None assert result.id == winner_id assert ( diff --git a/zeeguu/core/word_scheduling/basicSR/basicSR.py b/zeeguu/core/word_scheduling/basicSR/basicSR.py index 3839b8bed..3abe33319 100644 --- a/zeeguu/core/word_scheduling/basicSR/basicSR.py +++ b/zeeguu/core/word_scheduling/basicSR/basicSR.py @@ -15,7 +15,14 @@ class BasicSRSchedule(db.Model): - __table_args__ = {"mysql_collate": "utf8_bin"} + # A user_word has at most one schedule row. This mirrors the DB constraint + # added in tools/migrations/25-05-24--adding_the_user_word_table.sql; keeping + # it on the model documents the invariant and lets the test DB enforce it. + # (Single-table inheritance: FourLevelsPerWord shares this table.) + __table_args__ = ( + db.UniqueConstraint("user_word_id", name="unique_user_word_schedule"), + {"mysql_collate": "utf8_bin"}, + ) __tablename__ = "basic_sr_schedule" id = db.Column(db.Integer, primary_key=True) From 7bd82c4f4dc18de1927b4e6a828215cf2077abc4 Mon Sep 17 00:00:00 2001 From: Mircea Lungu Date: Thu, 16 Jul 2026 13:07:45 +0300 Subject: [PATCH 3/3] Move LLM translation-validation off the exercise-report hot path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scheduling a word used to run a synchronous LLM translation-validation inside FourLevelsPerWord.find_or_create, on the first exercise of an unvalidated word. That put an LLM round-trip on /report_exercise_outcome and — when the validator re-homed the word to a corrected meaning — deleted the practiced UserWord mid-request (the "Instance UserWord has been deleted" crash). Validation now happens off the hot path: - find_or_create is a fast, non-destructive primitive: it ensures a schedule row exists and never calls the LLM. meaning.validated != VALID is the "still needs validation" marker. - UserWord.report_exercise_outcome fires UserWordValidationService.validate_scheduled_user_word via run_in_background, but only when the word actually got scheduled (pipeline not full) and isn't VALID yet. A full pipeline leaves the word unscheduled and unvalidated — the nightly batch handles it if it's ever scheduled. Guarded off under TESTING / no app context so it never spawns threads in the test DB. - The worker is idempotent, re-queries by id (thread-safe), re-homes via the existing validate_and_fix, and clears the schedule if the word turns out unfit/duplicate so a known-bad word leaves the rotation. Backstop: tools/validate_scheduled_meanings.py is added to the ops crontab nightly (separate ops change) to catch anything the async path missed. Adds tests: find_or_create no longer validates inline; the worker is a no-op when already valid and unschedules unfit words; the trigger fires only for a scheduled, not-yet-valid word. Co-Authored-By: Claude Opus 4.8 --- .../core/llm_services/validation_service.py | 43 +++++ zeeguu/core/model/user_word.py | 52 +++++- zeeguu/core/test/test_scheduling.py | 167 +++++++++++++++--- .../basicSR/four_levels_per_word.py | 33 ++-- 4 files changed, 255 insertions(+), 40 deletions(-) diff --git a/zeeguu/core/llm_services/validation_service.py b/zeeguu/core/llm_services/validation_service.py index e6496af34..a119a8906 100644 --- a/zeeguu/core/llm_services/validation_service.py +++ b/zeeguu/core/llm_services/validation_service.py @@ -30,6 +30,49 @@ class UserWordValidationService: """Validates and fixes user_word translations before exercises.""" + @classmethod + def validate_scheduled_user_word(cls, user_word_id): + """ + Validate a just-scheduled word off the exercise-report hot path. + + Designed to run from `run_in_background` (which gives it its own Flask + app context and thread-local db.session) or from the nightly + tools/validate_scheduled_meanings.py batch. It re-queries by id, so it is + safe to hand only an id across the thread boundary. + + Idempotent: a word whose meaning is already VALID is a no-op. Otherwise it + validates + fixes the translation (which may re-home the word to a + corrected meaning) and, if the word turns out unfit or a duplicate, + removes it from the schedule so it leaves the exercise rotation. + """ + from zeeguu.core.model.db import db + from zeeguu.core.model.user_word import UserWord + from zeeguu.core.model.meaning import Meaning + from zeeguu.core.word_scheduling.basicSR.basicSR import BasicSRSchedule + + db_session = db.session + + user_word = UserWord.query.get(user_word_id) + if user_word is None: + return + if user_word.meaning.validated == Meaning.VALID: + return # already validated — nothing to do + + fixed_user_word = cls.validate_and_fix(db_session, user_word) + if fixed_user_word is None: + # Invalid with no usable correction: drop it from the rotation so a + # known-bad translation doesn't keep coming up in exercises. + BasicSRSchedule.clear_user_word_schedule(db_session, user_word) + return + + # validate_and_fix may have re-homed to a corrected UserWord; run the + # duplicate check against whichever word we ended up with. + if cls.check_for_duplicate_meaning(db_session, fixed_user_word): + BasicSRSchedule.clear_user_word_schedule(db_session, fixed_user_word) + return + + db_session.commit() + @classmethod def check_for_duplicate_meaning(cls, db_session, user_word) -> bool: """ diff --git a/zeeguu/core/model/user_word.py b/zeeguu/core/model/user_word.py index 00379a069..e6e09922e 100644 --- a/zeeguu/core/model/user_word.py +++ b/zeeguu/core/model/user_word.py @@ -380,11 +380,11 @@ def report_exercise_outcome( if not time: time = datetime.now() - # Update the schedule FIRST. The lazy translation-validation inside the - # scheduler may replace this UserWord with a corrected one and delete - # `self`; scheduler.update() returns the UserWord that survived. We must - # log the Exercise against that survivor — binding it to a deleted `self` - # raised "Instance UserWord has been deleted" (SR log digest 2026-07-15). + # Update the schedule FIRST, and log the Exercise against the UserWord + # scheduler.update() returns. That is normally `self`, but a scheduler + # can replace the practiced word with a corrected one and delete `self`; + # binding the Exercise to a deleted `self` raised "Instance UserWord has + # been deleted" (SR log digest 2026-07-15). scheduler = self.get_scheduler() practiced_user_word = scheduler.update(db_session, self, exercise_outcome, time) @@ -409,11 +409,53 @@ def report_exercise_outcome( db_session.commit() + # Validate the practiced word's translation off the hot path (async). + self._maybe_validate_off_hot_path(practiced_user_word) + # This needs to be re-thought, currently the updates are done in # the BasicSRSchedule.update call. # self.update_fit_for_study(db_session) # self.update_learned_status(db_session) + @staticmethod + def _maybe_validate_off_hot_path(user_word): + """ + Kick off translation-validation for `user_word` in a background thread, + but only when it's worth doing and safe to do: + + - skip if the meaning is already VALID (nothing to validate); + - skip if the word isn't actually scheduled — a full learning pipeline + leaves the word unscheduled, and the nightly + tools/validate_scheduled_meanings.py batch will pick it up if it ever + gets scheduled (this is the "delay till the night" half of the trade); + - skip under tests / outside an app context, where spawning a thread + would hit a fresh (empty) in-memory DB. + + The background worker re-queries by id, so we only pass the id across the + thread boundary. + """ + from flask import current_app, has_app_context + from zeeguu.core.model.meaning import Meaning + from zeeguu.core.word_scheduling.basicSR.basicSR import BasicSRSchedule + + if user_word is None: + return + if user_word.meaning.validated == Meaning.VALID: + return + if BasicSRSchedule.find_by_user_word(user_word) is None: + return + if not has_app_context() or current_app.config.get("TESTING"): + return + + from zeeguu.api.utils.background import run_in_background + from zeeguu.core.llm_services.validation_service import ( + UserWordValidationService, + ) + + run_in_background( + UserWordValidationService.validate_scheduled_user_word, user_word.id + ) + @classmethod def find_or_create(cls, session, user, meaning, is_user_added=False): """ diff --git a/zeeguu/core/test/test_scheduling.py b/zeeguu/core/test/test_scheduling.py index 4ff9f61db..b963e4a40 100644 --- a/zeeguu/core/test/test_scheduling.py +++ b/zeeguu/core/test/test_scheduling.py @@ -372,45 +372,35 @@ def test_update_returns_user_word_when_scheduler_declines(self): def test_report_outcome_logs_exercise_against_surviving_word(self): """ - The lazy translation-validation inside the scheduler can replace the - practiced UserWord with a corrected one and delete the original. The - Exercise must be logged against the survivor, not the deleted original - (which raised 'Instance UserWord has been deleted'). + A scheduler can replace the practiced UserWord with a corrected one and + delete the original (as the validation re-home does). report_exercise_outcome + must log the Exercise against the survivor scheduler.update() returns, not + the deleted original (which raised 'Instance UserWord has been deleted'). """ from unittest.mock import patch - from zeeguu.core.model.meaning import Meaning from zeeguu.core.model.exercise import Exercise from zeeguu.core.model.user_word import UserWord - from zeeguu.core.llm_services.validation_service import ( - UserWordValidationService, + from zeeguu.core.word_scheduling.basicSR.four_levels_per_word import ( + FourLevelsPerWord, ) old_bookmark = BookmarkRule(self.four_levels_user).bookmark old_user_word = old_bookmark.user_word - # force the validation path inside find_or_create - old_user_word.meaning.validated = Meaning.NOT_VALIDATED - db_session.add(old_user_word.meaning) new_bookmark = BookmarkRule(self.four_levels_user).bookmark new_user_word = new_bookmark.user_word - new_user_word.meaning.validated = Meaning.VALID - db_session.add(new_user_word.meaning) db_session.commit() new_user_word_id = new_user_word.id old_user_word_id = old_user_word.id - def fake_validate_and_fix(db_sess, user_word): - # Simulate a validation-fix that moves to a different meaning: - # delete the original UserWord and hand back the corrected one. + def fake_update(db_sess, user_word, outcome, time=None): + # Simulate a scheduler that re-homes the word: delete the original, + # return the survivor (mirrors validate_and_fix's re-home). db_sess.delete(user_word) db_sess.commit() return UserWord.query.get(new_user_word_id) - with patch.object( - UserWordValidationService, - "validate_and_fix", - side_effect=fake_validate_and_fix, - ): + with patch.object(FourLevelsPerWord, "update", side_effect=fake_update): # Must not raise "Instance UserWord has been deleted" old_user_word.report_exercise_outcome( db_session, @@ -468,3 +458,140 @@ def test_find_or_create_recovers_from_duplicate_schedule_race(self): assert ( BasicSRSchedule.query.filter_by(user_word_id=user_word.id).count() == 1 ) + + # ================================================================================================================ + # Off-hot-path (async + nightly) translation validation + # ================================================================================================================ + + def test_find_or_create_does_not_validate_inline(self): + """ + Scheduling must no longer call the LLM validator on the hot path — an + unvalidated word is scheduled as-is, validation happens off-path. + """ + from unittest.mock import patch + from zeeguu.core.model.meaning import Meaning + from zeeguu.core.word_scheduling.basicSR.basicSR import BasicSRSchedule + from zeeguu.core.word_scheduling.basicSR.four_levels_per_word import ( + FourLevelsPerWord, + ) + from zeeguu.core.llm_services.validation_service import ( + UserWordValidationService, + ) + + bookmark = BookmarkRule(self.four_levels_user).bookmark + user_word = bookmark.user_word + user_word.meaning.validated = Meaning.NOT_VALIDATED + db_session.add(user_word.meaning) + db_session.commit() + + with patch.object( + UserWordValidationService, "validate_and_fix" + ) as vf, patch.object( + UserWordValidationService, "check_for_duplicate_meaning" + ) as dup: + schedule = FourLevelsPerWord.find_or_create(db_session, user_word) + + vf.assert_not_called() + dup.assert_not_called() + assert schedule is not None + assert BasicSRSchedule.find_by_user_word(user_word) is not None + + def test_validate_scheduled_user_word_is_noop_when_already_valid(self): + """The worker is idempotent: a VALID meaning does no validation work.""" + from unittest.mock import patch + from zeeguu.core.model.meaning import Meaning + from zeeguu.core.llm_services.validation_service import ( + UserWordValidationService, + ) + + bookmark = BookmarkRule(self.four_levels_user).bookmark + user_word = bookmark.user_word + user_word.meaning.validated = Meaning.VALID + db_session.add(user_word.meaning) + db_session.commit() + + with patch.object(UserWordValidationService, "validate_and_fix") as vf: + UserWordValidationService.validate_scheduled_user_word(user_word.id) + + vf.assert_not_called() + + def test_validate_scheduled_user_word_unfit_leaves_rotation(self): + """ + When validation deems the word unfit (validate_and_fix -> None), the + worker drops it from the schedule so a known-bad word stops appearing. + """ + from unittest.mock import patch + from zeeguu.core.model.meaning import Meaning + from zeeguu.core.word_scheduling.basicSR.basicSR import BasicSRSchedule + from zeeguu.core.word_scheduling.basicSR.four_levels_per_word import ( + FourLevelsPerWord, + ) + from zeeguu.core.llm_services.validation_service import ( + UserWordValidationService, + ) + + bookmark = BookmarkRule(self.four_levels_user).bookmark + user_word = bookmark.user_word + user_word.meaning.validated = Meaning.NOT_VALIDATED + schedule = FourLevelsPerWord(user_word=user_word) + db_session.add_all([user_word.meaning, schedule]) + db_session.commit() + uw_id = user_word.id + assert BasicSRSchedule.find_by_user_word(user_word) is not None + + with patch.object( + UserWordValidationService, "validate_and_fix", return_value=None + ): + UserWordValidationService.validate_scheduled_user_word(uw_id) + + assert BasicSRSchedule.query.filter_by(user_word_id=uw_id).count() == 0 + + def test_off_hot_path_validation_fires_only_for_scheduled_unvalidated_word(self): + """ + The trigger implements the pipeline compromise: fire background validation + for a scheduled, not-yet-valid word; skip it when the word isn't scheduled + (full pipeline → left for the nightly batch). + """ + from unittest.mock import patch + from zeeguu.core.model.meaning import Meaning + from zeeguu.core.model.user_word import UserWord + from zeeguu.core.word_scheduling.basicSR.four_levels_per_word import ( + FourLevelsPerWord, + ) + from zeeguu.core.llm_services.validation_service import ( + UserWordValidationService, + ) + + # Scheduled + unvalidated → fires + scheduled_bm = BookmarkRule(self.four_levels_user).bookmark + scheduled_uw = scheduled_bm.user_word + scheduled_uw.meaning.validated = Meaning.NOT_VALIDATED + db_session.add_all( + [scheduled_uw.meaning, FourLevelsPerWord(user_word=scheduled_uw)] + ) + + # Unscheduled + unvalidated → does NOT fire (nightly handles it) + unscheduled_bm = BookmarkRule(self.four_levels_user).bookmark + unscheduled_uw = unscheduled_bm.user_word + unscheduled_uw.meaning.validated = Meaning.NOT_VALIDATED + db_session.add(unscheduled_uw.meaning) + db_session.commit() + + self.app.config["TESTING"] = False + try: + with patch( + "zeeguu.api.utils.background.run_in_background" + ) as run_bg: + UserWord._maybe_validate_off_hot_path(scheduled_uw) + assert run_bg.call_count == 1 + assert ( + run_bg.call_args.args[0] + == UserWordValidationService.validate_scheduled_user_word + ) + assert run_bg.call_args.args[1] == scheduled_uw.id + + run_bg.reset_mock() + UserWord._maybe_validate_off_hot_path(unscheduled_uw) + run_bg.assert_not_called() + finally: + self.app.config["TESTING"] = True diff --git a/zeeguu/core/word_scheduling/basicSR/four_levels_per_word.py b/zeeguu/core/word_scheduling/basicSR/four_levels_per_word.py index 35a5aa3bd..907aa6b09 100644 --- a/zeeguu/core/word_scheduling/basicSR/four_levels_per_word.py +++ b/zeeguu/core/word_scheduling/basicSR/four_levels_per_word.py @@ -108,27 +108,30 @@ def get_cooling_interval_dictionary(cls): @classmethod def find_or_create(cls, db_session, user_word): - + """ + Ensure a schedule row exists for `user_word` and return it (None only if + the word is not fit for study). + + This is a fast, non-destructive primitive: it does NOT call the LLM + translation-validator. Validation used to run here synchronously, which + put an LLM round-trip on the exercise-report hot path and — when it + re-homed the word to a corrected meaning — could delete the practiced + UserWord mid-request ("Instance UserWord has been deleted", + SR log digest 2026-07-15). + + Validation now happens off the hot path: asynchronously right after the + word is scheduled (UserWordValidationService.validate_scheduled_user_word, + triggered from UserWord.report_exercise_outcome) and, as a backstop, in + the nightly tools/validate_scheduled_meanings.py batch. Until then the + word is scheduled as-is, with meaning.validated != VALID acting as the + "still needs validation" marker. + """ schedule = super(FourLevelsPerWord, cls).find(user_word) if not schedule: - # Validate translation before first schedule (if not already validated as correct) - from zeeguu.core.model.meaning import Meaning - if user_word.meaning.validated != Meaning.VALID: - from zeeguu.core.llm_services.validation_service import UserWordValidationService - user_word = UserWordValidationService.validate_and_fix(db_session, user_word) - if user_word is None: - return None # Validation failed, word is not fit for study - - # After validation, check if still fit for study if not user_word.fit_for_study: return None # Don't create schedule for unfit words - # Check for duplicate meanings (same word with equivalent translation already being learned) - from zeeguu.core.llm_services.validation_service import UserWordValidationService - if UserWordValidationService.check_for_duplicate_meaning(db_session, user_word): - return None # Duplicate meaning, don't schedule - schedule = cls(user_word) user_word.level = 1 db_session.add_all([schedule, user_word])