Rank spirit scores on the average, not the rounded value - #568
Merged
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, matches the described defect, and is backed by targeted new tests plus updated API expectations.
Pull request overview
This PR fixes spirit ranking ties caused by ranking on a rounded average (1 decimal) instead of the full-precision average, ensuring teams with distinct underlying averages no longer get forced into the same rank when their displayed values match.
Changes:
- Store full-precision per-team spirit averages in
update_tournament_spirit_rankings, and defer rounding until after ranking is computed. - Update
rank_spirit_scoresto rank by full-precision average (rounded to 6 decimals for float-noise resistance) and break ties usingtournament.current_seeding. - Add focused unit tests for the regression case (BT/KL/KA) and adjust API test expectations for sequential (tie-broken) ranks.
File summaries
| File | Description |
|---|---|
| server/tournament/utils.py | Ranks spirit scores using full-precision averages and uses current_seeding as a deterministic tie-breaker before rounding for display/storage. |
| server/tests/test_spirit_ranking.py | Adds regression and behavior tests covering full-precision ranking, tie-breaking by placement, float-noise handling, and backward-compat behavior. |
| server/tests/test_api.py | Updates expected spirit ranking output to reflect sequential ranks after tie-breaking. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Joe2k
force-pushed
the
fix-spirit-score-tie-break
branch
from
September 2, 2026 16:54
7f7ceac to
1735751
Compare
Kerala's spirit captain reported a three-way tie at 10.4 between Bengal,
Kerala and Karnataka that should not have been one:
Karnataka 83/8 = 10.375
Kerala 73/7 = 10.4286
The average was rounded to one decimal before being stored, and
rank_spirit_scores built its order from the set of distinct rounded values, so
teams sharing a displayed figure necessarily shared a rank. These two are more
than half a rounding step apart.
Ranks now come from the unrounded average. Teams genuinely level are separated
by their final placement, read from current_seeding, so ranks are sequential
where equal scores used to share one.
Scores are kept and shown to two decimals, which also tells these three apart
at a glance: 10.43, 10.40, 10.38.
Ranking compares averages at six decimals. Ten matches of 10.4 accumulate to
an average of 10.400000000000002, and comparing raw floats would rank that
above an exact 10.4 on representation alone.
Standings will not change until the rankings are recomputed, which happens
whenever a spirit score in the tournament is next saved.
Joe2k
force-pushed
the
fix-spirit-score-tie-break
branch
from
September 3, 2026 00:31
1735751 to
bf603ba
Compare
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.
Kerala's spirit captain reported a three-way tie at 10.4 between Bengal, Kerala and Karnataka that shouldn't have been one:
Both displayed as 10.4 — and both were being ranked as 10.4.
Cause
update_tournament_spirit_rankingsrounded the average to one decimal before storing it, andrank_spirit_scoresbuilt its order from the set of distinct rounded values:So any teams sharing a displayed figure necessarily shared a rank. Kerala and Karnataka are 0.054 apart — more than half a rounding step.
The rounding was correct — 10.375 → 10.4 is proper. Ranking on the rounded value was the defect.
Change
current_seeding({position: team_id}). Ranks are consequently sequential where equal scores used to share one — visible in the updatedtest_api.pyexpectation, where six teams on 0.0 move from all-rank-3 to ranks 3–8 in finishing order.toFixed(2)), so these three now read 10.43 / 10.40 / 10.38 instead of three identical 10.4s.10.400000000000002, and raw float comparison would rank that above an exact 10.4 on representation alone, silently pre-empting the placement tie-break.final_standingsis optional because migration0039still calls this with one argument, on rankings recorded beforeself_pointsexisted — covered by a test.Testing
./scripts/lintpasses. Five tests inserver/tests/test_spirit_ranking.pyuse the real BT/KL/KA figures. Full suite: 489 tests, 3 errors, all pre-existing and unrelated (TOPSCORE_CLIENT_IDenv var, two SeleniumBase UI tests).Note
Standings won't change on merge.
spirit_rankingis stored JSON, recomputed only when a spirit score in that tournament is next saved (api.py:2618,2652). The affected tournament needs a recompute — happy to add a management command if that's easier than re-saving a score.🤖 Generated with Claude Code