Fix SR exercise-outcome crashes + move translation-validation off the hot path#679
Open
mircealungu wants to merge 3 commits into
Open
Fix SR exercise-outcome crashes + move translation-validation off the hot path#679mircealungu wants to merge 3 commits into
mircealungu wants to merge 3 commits into
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
|
ArchLens detected architectural changes in the following views: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Investigates and fixes the 6 new error signatures in the word-scheduling (SR) path from the 2026-07-15 afternoon log digest (zapi / api.zeeguu.org), then removes the root cause architecturally.
The crashes (all in
/report_exercise_outcome)InvalidRequestError: Instance '<UserWord>' has been deleted(~4×) — the lazy LLM translation-validation inside the scheduler re-homed the practiced word to a corrected meaning and deleted the original UserWord mid-request; the Exercise was then bound to the deleted object.IntegrityError: Duplicate entry for key 'unique_user_word_schedule'(~3×) — two concurrent reports both passed the "no schedule yet" check and inserted; the loser's commit hit the unique constraint.AttributeError: 'NoneType' object has no attribute 'update_schedule'(1×) —find_or_createreturnedNone(word judged unfit/duplicate) andupdate()dereferenced it.Fixes
Immediate guards (commit 1):
report_exercise_outcomelogs the Exercise against the UserWordscheduler.update()returns (the survivor), never a deletedself.find_or_createcatches the duplicate-insertIntegrityError, rolls back, and returns the winning row.update()no longer dereferences aNoneschedule.Model constraint (commit 2): declared
unique_user_word_schedulein__table_args__so the invariant lives next to the column and the test DB enforces it — the duplicate-race test now exercises the real constraint.Root-cause removal — validation off the hot path (commit 3): the deeper smell was a synchronous LLM call sitting in the exercise-report path that could delete the practiced word.
find_or_createis now a fast, non-destructive "ensure a schedule row exists" primitive — no LLM, no re-home.run_in_background), but only when the word actually got scheduled (learning pipeline not full); a full pipeline leaves it for the night.tools/validate_scheduled_meanings.pyis the backstop (added to the ops crontab — separate ops change).validate_and_fix, and unschedules words that turn out unfit/duplicate.Tests
zeeguu/core/test/test_scheduling.py— 12 passing, covering all three crash regressions plus the new async behavior (no inline validation; worker no-op when valid / unschedules unfit; trigger fires only for a scheduled, not-yet-valid word).Follow-up (not in this PR)
validate_scheduled_meaningscrontab line (needs commit + crontab deploy).zeeguu-docs/future-work/translation-validation-off-exercise-hot-path.md.🤖 Generated with Claude Code