fix(sports_rankings): skip a malformed poll entry; rank each side by its own id (#791)

* fix(sports_rankings): skip a malformed poll entry; rank each side by its own id

The two review points deferred on #789 until sports_rankings was the only
copy (the nine scoreboards adopted it in ledmatrix-plugins #648):

- A poll entry whose "current" is None or a string raised TypeError at
  `current_rank <= 0`; the broad except returned {} without setting the
  cache timestamp, so every team lost its rank and the standings were
  re-fetched on every update. The entry is now skipped.
- _ranked_positions() fell back to abbreviations for BOTH sides when only
  one side's id was unparseable, so a colliding abbreviation could rank the
  wrong school. It now decides id-or-abbreviation per side.

Well-formed ESPN data (ids on both sides, int ranks) ranks as before.
SPORTS_UNIFICATION.md: family 8 marked done and adopted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs(changelog): a numeric-string rank is still accepted; say which values are skipped

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Chuck
2026-10-12 14:07:58 -04:00
committed by GitHub
co-authored by Claude Opus 5.5
parent ef97bd158d
commit 886e199cae
4 changed files with 52 additions and 18 deletions
+11
View File
@@ -19,6 +19,17 @@ accepts both, but the store flags the old spelling as deprecated
## Unreleased ## Unreleased
### Fixes
- `SportsRankingsMixin` (#789's deferred review points, now that it is the
only copy): a poll entry whose `current` cannot be converted to an
integer (`None`, a non-numeric string) is skipped instead of throwing
away the whole poll -- which left
every team unranked and re-fetched the standings on every update -- and
`_ranked_positions()` decides id-or-abbreviation per side, so one side
without an id no longer drops the other side's id and lets a shared
abbreviation rank the wrong school. Well-formed ESPN data ranks as before.
## 3.8.5 ## 3.8.5
Fewer ESPN requests on the hourly schedule refresh (#754), and sports Fewer ESPN requests on the hourly schedule refresh (#754), and sports
+2 -2
View File
@@ -383,7 +383,7 @@ ufc's MMA managers override `update()` and never build the pools, so the
rotation is dormant there. `_best_rank`, `_is_ranked_game` and rotation is dormant there. `_best_rank`, `_is_ranked_game` and
`_passes_other_filters` were family 8. `_passes_other_filters` were family 8.
### Family 8: rankings (reconciled; promoted, core 3.8.5) ### Family 8: rankings (done: core 3.8.5, adopted)
ledmatrix-plugins `scripts/test_rankings.py` (#644) pinned 79 rows across the ledmatrix-plugins `scripts/test_rankings.py` (#644) pinned 79 rows across the
nine plugins first, every ESPN request answered offline from recorded-shape nine plugins first, every ESPN request answered offline from recorded-shape
@@ -502,7 +502,7 @@ release.
| 5 | Game-over check | `SportsLive._is_game_really_over` (5) | Pure logic, no pixels; one seam, `FINAL_PERIOD`. The pilot for the procedure. Reconciled to one body and promoted as `sports_game_over`; shipped in 3.8.1 and adopted. See [Family 5](#family-5-the-game-over-check-done-core-381-adopted) | | 5 | Game-over check | `SportsLive._is_game_really_over` (5) | Pure logic, no pixels; one seam, `FINAL_PERIOD`. The pilot for the procedure. Reconciled to one body and promoted as `sports_game_over`; shipped in 3.8.1 and adopted. See [Family 5](#family-5-the-game-over-check-done-core-381-adopted) |
| 6 | Favourite matching | `_is_favorite_game` (7 across three classes), `_select_games_for_display` (2: nrl), `_select_recent_games_for_display` (3) | Everything that asks "is this a favourite" goes through the 3.5.0 `_favorite_key` seam. Reconciled to one body each and promoted as `sports_favorites`; shipped in 3.8.2 and adopted. See [Family 6](#family-6-favourite-matching-done-core-382-adopted) | | 6 | Favourite matching | `_is_favorite_game` (7 across three classes), `_select_games_for_display` (2: nrl), `_select_recent_games_for_display` (3) | Everything that asks "is this a favourite" goes through the 3.5.0 `_favorite_key` seam. Reconciled to one body each and promoted as `sports_favorites`; shipped in 3.8.2 and adopted. See [Family 6](#family-6-favourite-matching-done-core-382-adopted) |
| 7 | Other-games rotation | `_by_importance`, `_other_games_window`, `_advance_other_games_if_due` (2 each: football), `_rotate_other_games_on_display` (2: ufc), with `_attach_odds_to_rotated_games` (3; ufc had none) | One outlier each; football carried two fixes the other eight lacked. Reconciled to one body each, on a `_rankings_loaded` seam, and promoted as `sports_rotation`; done: core 3.8.4, adopted (ledmatrix-plugins #643). See [Family 7](#family-7-the-other-games-rotation-done-core-384-adopted) | | 7 | Other-games rotation | `_by_importance`, `_other_games_window`, `_advance_other_games_if_due` (2 each: football), `_rotate_other_games_on_display` (2: ufc), with `_attach_odds_to_rotated_games` (3; ufc had none) | One outlier each; football carried two fixes the other eight lacked. Reconciled to one body each, on a `_rankings_loaded` seam, and promoted as `sports_rotation`; done: core 3.8.4, adopted (ledmatrix-plugins #643). See [Family 7](#family-7-the-other-games-rotation-done-core-384-adopted) |
| 8 | Rankings | `_fetch_team_rankings` (3), `_choose_poll` (3), `_load_division_team_ids`, `_passes_other_filters`, `_best_rank`, `_is_ranked_game` (2 each: football) | Needs 7; the rank badge and the "ranked only" filter read it. Reconciled to one body each and promoted as `sports_rankings` (core 3.8.5). See [Family 8](#family-8-rankings-reconciled-promoted-core-385) | | 8 | Rankings | `_fetch_team_rankings` (3), `_choose_poll` (3), `_load_division_team_ids`, `_passes_other_filters`, `_best_rank`, `_is_ranked_game` (2 each: football) | Needs 7; the rank badge and the "ranked only" filter read it. Reconciled to one body each and promoted as `sports_rankings`; done: core 3.8.5, adopted (ledmatrix-plugins #648). See [Family 8](#family-8-rankings-done-core-385-adopted) |
| 9 | Live fetch and odds | `_fetch_todays_games` (5), `_fetch_odds` (3), `_attach_odds_to_rotated_games` (3) | The prerequisite for one shared ESPN poller across plugins | | 9 | Live fetch and odds | `_fetch_todays_games` (5), `_fetch_odds` (3), `_attach_odds_to_rotated_games` (3) | The prerequisite for one shared ESPN poller across plugins |
| 10 | View model | `_extract_game_details_common` (9 of 9) | Every renderer reads it; its keys are additive-only, so reconcile to the superset and leave sport extras in `_extract_game_details` | | 10 | View model | `_extract_game_details_common` (9 of 9) | Every renderer reads it; its keys are additive-only, so reconcile to the superset and leave sport extras in `_extract_game_details` |
| 11 | Switch scorebug | `_load_fonts` (4), `_load_custom_font_from_element_config` (6), `_get_layout_offset` (5), `_fit_score_font` (2: football), `_load_and_resize_logo` (9), `_draw_dynamic_odds` (9), then `_draw_scorebug_layout` (20: Upcoming 9, Recent 8, Live 2, ufc's Core 1) | Needs 10 and the twin decisions below. Several releases: fonts and offsets, logos, odds, then one mode's layout per release | | 11 | Switch scorebug | `_load_fonts` (4), `_load_custom_font_from_element_config` (6), `_get_layout_offset` (5), `_fit_score_font` (2: football), `_load_and_resize_logo` (9), `_draw_dynamic_odds` (9), then `_draw_scorebug_layout` (20: Upcoming 9, Recent 8, Live 2, ufc's Core 1) | Needs 10 and the twin decisions below. Several releases: fonts and offsets, logos, odds, then one mode's layout per release |
+23 -16
View File
@@ -216,7 +216,12 @@ class SportsRankingsMixin:
for team_data in poll.get("ranks", []): for team_data in poll.get("ranks", []):
team_info = team_data.get("team", {}) team_info = team_data.get("team", {})
team_abbr = team_info.get("abbreviation", "") team_abbr = team_info.get("abbreviation", "")
current_rank = team_data.get("current", 0) # One malformed entry (a None or non-numeric "current") is
# skipped, not allowed to throw away the whole poll.
try:
current_rank = int(team_data.get("current"))
except (TypeError, ValueError):
continue
if current_rank <= 0: if current_rank <= 0:
continue continue
if team_abbr: if team_abbr:
@@ -337,28 +342,30 @@ class SportsRankingsMixin:
return self._division_team_ids return self._division_team_ids
def _ranked_positions(self, game: Dict) -> List[int]: def _ranked_positions(self, game: Dict) -> List[int]:
"""Poll positions held by either side: ids first, abbreviations second. """Poll positions held by either side: by id, else by abbreviation.
The id table is authoritative whenever the game carries ids and a poll The id table is authoritative whenever a side carries an id and a poll
has been parsed, because it is built from a top-division poll alone -- has been parsed, because it is built from a top-division poll alone --
so an FCS side cannot appear in it however it is abbreviated, and an so an FCS side cannot appear in it however it is abbreviated, and an
abbreviation shared across divisions cannot promote the wrong school. abbreviation shared across divisions cannot promote the wrong school.
The abbreviation table stays as the fallback for a game dict with no Decided per side: a side without a usable id falls back to its
ids on it. abbreviation, without costing the other side its id.
""" """
by_id = getattr(self, "_ranked_team_ids", None) or {} by_id = getattr(self, "_ranked_team_ids", None) or {}
ids = []
for side in ("home_id", "away_id"):
try:
ids.append(int(game.get(side))) # type: ignore[arg-type] # None: caught below
except (TypeError, ValueError):
ids = []
break
if by_id and ids:
return [rank for rank in (by_id.get(i, 0) for i in ids) if rank]
by_abbr = getattr(self, "_team_rankings_cache", None) or {} by_abbr = getattr(self, "_team_rankings_cache", None) or {}
return [rank for rank in (by_abbr.get(game.get("home_abbr"), 0), found = []
by_abbr.get(game.get("away_abbr"), 0)) if rank] for side in ("home", "away"):
rank = None
if by_id:
try:
rank = by_id.get(int(game.get(f"{side}_id")), 0) # type: ignore[arg-type] # None: caught below
except (TypeError, ValueError):
pass
if rank is None:
rank = by_abbr.get(game.get(f"{side}_abbr"), 0)
if rank:
found.append(rank)
return found
def _is_ranked_game(self, game: Dict) -> bool: def _is_ranked_game(self, game: Dict) -> bool:
return bool(self._ranked_positions(game)) return bool(self._ranked_positions(game))
+16
View File
@@ -173,6 +173,11 @@ def test_only_a_league_with_a_poll_is_asked(clock, league, asked):
{"current": 3, "team": {"id": "abc", "abbreviation": "OSU"}}, {"current": 3, "team": {"id": "abc", "abbreviation": "OSU"}},
{"current": 4, "team": {"abbreviation": "TEX"}}]}]}, {"current": 4, "team": {"abbreviation": "TEX"}}]}]},
{"OSU": 3, "TEX": 4}, {87: 2}), {"OSU": 3, "TEX": 4}, {87: 2}),
({"rankings": [{"name": "AP Top 25", "type": "ap", "ranks": [ # #789: one bad entry
{"current": None, "team": {"id": "61", "abbreviation": "UGA"}},
{"current": "n/a", "team": {"id": "87", "abbreviation": "ND"}},
{"current": 3, "team": {"id": "194", "abbreviation": "OSU"}}]}]},
{"OSU": 3}, {194: 3}),
]) ])
def test_payload_shapes(clock, payload, abbr, ids): def test_payload_shapes(clock, payload, abbr, ids):
host = Host(payload=payload) host = Host(payload=payload)
@@ -320,6 +325,17 @@ def test_ranked_and_best_rank(clock, label, best):
assert host._is_ranked_game(game(label)) is (best != 99) assert host._is_ranked_game(game(label)) is (best != 99)
@pytest.mark.parametrize("home, away, best", [
(NEWARK, (None, "VAN"), 99), # Newark's id is unranked; "OSU" must not rank it
(("61", None), (None, "VAN"), 1), # UGA's id still counts with no id on the other side
])
def test_each_side_is_ranked_by_its_own_id(clock, home, away, best):
"""#789: one side's missing id no longer sends both sides to abbreviations."""
g = {f"{side}_{key}": value for side, team in (("home", home), ("away", away))
for key, value in zip(("id", "abbr"), team) if value is not None}
assert loaded()._best_rank(g) == best
#: filter -> (quality, divisions, poll loaded?, kept: one Y/. per GAMES entry) #: filter -> (quality, divisions, poll loaded?, kept: one Y/. per GAMES entry)
FILTERS = { FILTERS = {
"any": ("any", (), True, "YYYYYYYY"), "any": ("any", (), True, "YYYYYYYY"),