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 622988037..e6e09922e 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, 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) + from zeeguu.core.model import Exercise exercise = Exercise( @@ -387,26 +396,66 @@ 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() + # 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 815b8384e..b963e4a40 100644 --- a/zeeguu/core/test/test_scheduling.py +++ b/zeeguu/core/test/test_scheduling.py @@ -342,3 +342,256 @@ 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): + """ + 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.exercise import Exercise + from zeeguu.core.model.user_word import UserWord + 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 + + new_bookmark = BookmarkRule(self.four_levels_user).bookmark + new_user_word = new_bookmark.user_word + db_session.commit() + new_user_word_id = new_user_word.id + old_user_word_id = old_user_word.id + + 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(FourLevelsPerWord, "update", side_effect=fake_update): + # 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 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 + + # 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) + + assert result is not None + assert result.id == winner_id + 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/basicSR.py b/zeeguu/core/word_scheduling/basicSR/basicSR.py index d125a5439..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) @@ -95,6 +102,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 +130,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 +143,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..907aa6b09 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 @@ -106,30 +108,42 @@ 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]) - 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