fix: correct two regressions from code-review fixes#25
Merged
Conversation
- removeMatch: revert targeted-write optimisation — recalculateStatsFromHistory replays all matches, so all players' ELOs can change via chain reaction; writing only the 2-4 participants left every other player stale in Supabase - saveMatch: add DB revert in the catch block — if updatePlayer succeeds for some players but createMatch then throws, the already-written Supabase rows are now corrected back to the post-rollback ELOs so DB stays consistent https://claude.ai/code/session_01TgxiTLy7fCh5xZPJT8eTVt
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
2 participants
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
Two regressions introduced in PR #23 that were caught by a follow-up code review.
removeMatch— stale ELOs for uninvolved playersThe previous fix changed
removeMatchto only write back the 2–4 players directly in the deleted match. ButrecalculateStatsFromHistory()replays all matches from scratch — any player who faced A or B in a later match also gets a new ELO via chain reaction. Those players were never written back to Supabase, leaving their DB records stale. Reverted to writing all players.saveMatch— partial DB write not reverted on failureupdatePlayercalls run sequentially beforecreateMatch. IfcreateMatch(or a laterupdatePlayer) throws, the catch block rolls back local state correctly — but any Supabase player rows already written retain the inflated ELOs. Added a compensatingPromise.allin the catch block that re-writes those players back to their post-rollback (correct) ELOs.Test plan
npm test)https://claude.ai/code/session_01TgxiTLy7fCh5xZPJT8eTVt
Generated by Claude Code