fix: enforce prediction write guards atomically at the database layer#130
Merged
adrunkhuman merged 1 commit intomasterfrom Mar 21, 2026
Merged
fix: enforce prediction write guards atomically at the database layer#130adrunkhuman merged 1 commit intomasterfrom
adrunkhuman merged 1 commit intomasterfrom
Conversation
Fixes two race conditions in the prediction write path: - #120: ThreadPredictionHandler's first-write-wins rule was enforced at the app layer (get_prediction check before save), leaving a window where a concurrent DM save could land and be overwritten by the subsequent thread write. - #122: Neither handler revalidated fixture state at write time, so a results calculation closing the fixture between the handler's read and the actual INSERT could silently succeed. Both are fixed by moving the invariants into the DB layer inside a single BEGIN IMMEDIATE transaction: - try_save_prediction(): thread path — fixture-open check + INSERT OR IGNORE (first-write-wins). Returns SaveResult.FIXTURE_CLOSED or SaveResult.DUPLICATE instead of writing. - save_prediction_guarded(): DM path — fixture-open check + upsert (re-submissions intentionally allowed). Returns FIXTURE_CLOSED only. Also fixes user_name staleness on upsert in both save_prediction and save_prediction_guarded (DO UPDATE SET was missing user_name). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
🚅 Deployed to the matchday-typer-pr-130 environment in patient-quietude
|
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
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.
Summary
SaveResultenum and two new DB methods (try_save_prediction,save_prediction_guarded) that perform fixture-open validation and duplicate checks inside a singleBEGIN IMMEDIATEtransaction — making both guards atomictry_save_prediction(first-write-wins); removes the app-levelget_prediction+ conditional that left a race windowsave_prediction_guarded(upsert, re-submissions still work) and handlesFIXTURE_CLOSEDgracefullyuser_namestaleness on upsert:DO UPDATE SETwas missinguser_name = excluded.user_namein bothsave_predictionandsave_prediction_guardedCloses #120
Closes #122
Test plan
uv run pytest tests/test_database.py -k "TrySave or SaveGuarded" -v— new DB unit tests (fixture-open guard, first-write-wins, duplicate, both-conditions ordering)uv run pytest tests/test_thread_prediction_handler.py tests/test_dm_prediction_handler.py -v— handler tests including new fixture-closed paths and updated monkeypatch targetsuv run pytest— full suite (284 pass, 1 skip)uv run ruff check typer_bot/ tests/— clean🤖 Generated with Claude Code