From 6b7450669515cf2db1a8cb0d40c10d41d5e852b2 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Fri, 21 Aug 2026 16:24:15 -0400 Subject: [PATCH] fix(sports): fetch odds for the games shown, not the whole schedule window (#494) * fix(sports): fetch odds for the games shown, not the whole window SportsUpcoming.update() walked every upcoming game in the schedule window and called _fetch_odds() on each one inside that collection loop, narrowing to upcoming_games_to_show only afterwards. Each call is a separate sequential ESPN request. The comment sitting above it said odds were fetched "only for games that will be displayed". The only narrowing it actually applied was show_favorite_teams_only, which is not the default, so in the usual configuration nothing narrowed it at all. Measured on devpi, where the football plugin has the same shape: 467 odds requests in one 35s burst, 467 distinct events 315 NFL + 152 college-football -- roughly a whole season plugin football-scoreboard operation timed out after 30.0s The burst repeats each time the 1h odds TTL expires: 67 -> 327 -> 957 -> 1261 requests/hour across four consecutive hours. Between expiries the cache works and the rate is zero, so this is a thundering herd on expiry, not a caching failure. The fetch now runs after selection, over team_games -- the list already cut to upcoming_games_to_show. This mirrors the fix the football plugin already carries; the shared base class never got it. SportsLive is deliberately left as it is: it walks the raw event list because it has to find which games are live, but only fetches odds for a game that has already passed the is_live/is_halftime test, so its fan-out is bounded by how many games are actually in progress. The test pins that distinction rather than assuming it. The test reads the AST rather than the source text, and asserts the full set of call sites, so a new one has to be classified deliberately instead of inheriting whichever behaviour it happens to land in. Writing it that way is what turned up the SportsLive site, which I had missed. Verified: reverting the fix fails the test with the offending iterable named ("iterates over 'events'"). 525 passed, 9 skipped across the sports and odds suites. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * test(sports): check the odds guard structurally, not by its text Review caught that _guards_above() collected an `if` test even when the call sat in that if's `else`, so moving _fetch_odds() into the else of the is_live/is_halftime test would still pass -- while fetching odds for exactly the non-live games the guard exists to exclude. Verifying that turned up a wider hole in the same assertion. It matched substrings of the *unparsed source*, so a negated condition satisfied it too: if not (details["is_live"] or details["is_halftime"]): self._fetch_odds(details) # every non-live game Both names still appear in that text, so `"is_live" in guards` held and the test passed on code doing the opposite of what it claims to check. The guard test is now structural. It walks the AST for an enclosing `if` whose *body* (never its `else`) contains the call, and whose test references both names without either sitting under a `not`. Verified by mutation: fetching odds for non-live games now fails with "does not sit in the true branch of a test requiring the game to be in progress". Moving the call into the else of the *favourites* test still passes, which is correct -- the game there is still live, so the in-progress contract holds and the fan-out stays bounded by how many games are actually in play. 525 passed, 9 skipped across the sports and odds suites. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW --------- Co-authored-by: Claude Opus 5 (1M context) --- src/base_classes/sports/modes.py | 17 +++- test/test_sports_odds_fanout.py | 136 +++++++++++++++++++++++++++++++ 2 files changed, 151 insertions(+), 2 deletions(-) create mode 100644 test/test_sports_odds_fanout.py diff --git a/src/base_classes/sports/modes.py b/src/base_classes/sports/modes.py index 06e73467..8f6e3cce 100644 --- a/src/base_classes/sports/modes.py +++ b/src/base_classes/sports/modes.py @@ -145,8 +145,7 @@ class SportsUpcoming(SportsCore): if (game['home_abbr'] in self.favorite_teams or game['away_abbr'] in self.favorite_teams): favorite_games_found += 1 - if self.show_odds: - self._fetch_odds(game) + # Odds are NOT fetched here -- see after selection below. # Enhanced logging for debugging self.logger.info(f"Found {all_upcoming_games} total upcoming games in data") @@ -190,6 +189,20 @@ class SportsUpcoming(SportsCore): # Limit to the specified number of upcoming games team_games = team_games[:self.upcoming_games_to_show] + # Odds are fetched here, for the games that survived selection, + # rather than in the loop that collects them. That loop walks every + # upcoming game in the schedule window, and for a college league + # the window is enormous -- a live rig logged 946 upcoming games in + # one cycle and displayed 1 of them. The comment up there claimed + # odds were fetched "only for games that will be displayed", but + # the only narrowing it applied was show_favorite_teams_only, which + # is not the default; in the usual case nothing narrowed it at all + # and every game cost a separate ESPN request on a Pi that is also + # driving the panel. + if self.show_odds: + for game in team_games: + self._fetch_odds(game) + # Log changes or periodically should_log = ( current_time - self.last_log_time >= self.log_interval or diff --git a/test/test_sports_odds_fanout.py b/test/test_sports_odds_fanout.py new file mode 100644 index 00000000..c33930e9 --- /dev/null +++ b/test/test_sports_odds_fanout.py @@ -0,0 +1,136 @@ +"""Odds must be fetched for the games shown, not every game in the window. + +SportsUpcoming.update() collected every upcoming game in the schedule window +and called _fetch_odds() on each one *inside* that collection loop, narrowing +to upcoming_games_to_show only afterwards. The comment there said odds were +fetched "only for games that will be displayed", but the sole narrowing it +applied was show_favorite_teams_only, which is not the default -- so in the +usual configuration nothing narrowed it at all. + +Measured on a live rig: a college league produced 946 upcoming games in one +cycle and displayed 1 of them. The same shape on the football plugin produced +a burst of 467 sequential ESPN requests that ran for 35s and blew that +plugin's 30s update budget, and it repeats every time the 1h odds TTL expires. + +SportsLive is deliberately different: it walks the raw event list because it +has to find which games are live, but only fetches odds for a game that has +already passed the is_live/is_halftime test, so the fan-out is bounded by how +many games are actually in progress. +""" +import ast +from pathlib import Path + +import pytest + +MODES = (Path(__file__).resolve().parent.parent + / "src" / "base_classes" / "sports" / "modes.py") +TREE = ast.parse(MODES.read_text(encoding="utf-8")) + + +def _fetch_sites(): + """(class name, method name, lineno) for every self._fetch_odds(...) call.""" + calls = [n.lineno for n in ast.walk(TREE) + if isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute) + and n.func.attr == "_fetch_odds"] + sites = [] + for cls in [n for n in ast.walk(TREE) if isinstance(n, ast.ClassDef)]: + for fn in [n for n in cls.body if isinstance(n, ast.FunctionDef)]: + for lineno in calls: + if fn.lineno <= lineno <= (fn.end_lineno or fn.lineno): + sites.append((cls.name, fn.name, lineno)) + assert len(sites) == len(calls), "a _fetch_odds call sits outside any method" + return sites + + +def _innermost_loop_iterable(lineno): + best = None + for node in ast.walk(TREE): + if isinstance(node, ast.For) and \ + node.lineno <= lineno <= (node.end_lineno or node.lineno): + if best is None or node.lineno > best.lineno: + best = node + return None if best is None else ast.unparse(best.iter) + + +def _spans(body, lineno): + """True when `lineno` falls inside this list of statements.""" + return any(n.lineno <= lineno <= (n.end_lineno or n.lineno) for n in body) + + +def _parents(tree): + table = {} + for node in ast.walk(tree): + for child in ast.iter_child_nodes(node): + table[child] = node + return table + + +PARENTS = _parents(TREE) + + +def _mentions_positively(test, names): + """True when `test` references every name, none of them under a `not`. + + Structural, not textual. Matching the unparsed source would accept + `not (details["is_live"] or details["is_halftime"])` -- which selects + exactly the non-live games this guard exists to exclude -- because the + names still appear in the text. + """ + found = set() + for node in ast.walk(test): + if not (isinstance(node, ast.Constant) and node.value in names): + continue + negated = False + cursor = node + while cursor is not test and cursor in PARENTS: + cursor = PARENTS[cursor] + if isinstance(cursor, ast.UnaryOp) and isinstance(cursor.op, ast.Not): + negated = True + break + if not negated: + found.add(node.value) + return found >= set(names) + + +def _guarded_by_positive(lineno, names): + """True when some enclosing `if` runs this line only if `names` hold. + + Only the TRUE branch counts: an `if` whose `else` contains the call would + otherwise look like a guard while doing the opposite. + """ + for node in ast.walk(TREE): + if isinstance(node, ast.If) and _spans(node.body, lineno) \ + and _mentions_positively(node.test, names): + return True + return False + + +def test_every_fetch_site_is_accounted_for(): + """A new call site must be classified deliberately, not inherited silently.""" + found = {(cls, fn) for cls, fn, _ in _fetch_sites()} + assert found == {("SportsUpcoming", "update"), ("SportsLive", "update")}, ( + f"unexpected _fetch_odds call sites: {sorted(found)}. Each one is a " + "sequential ESPN request per game -- classify it here on purpose.") + + +def test_upcoming_fetches_only_the_selected_games(): + for cls, _fn, lineno in _fetch_sites(): + if cls != "SportsUpcoming": + continue + iterable = _innermost_loop_iterable(lineno) + assert iterable == "team_games", ( + f"SportsUpcoming._fetch_odds at line {lineno} iterates over " + f"{iterable!r}. It must run over team_games -- already narrowed to " + "upcoming_games_to_show -- not over every event in the schedule " + "window. Each item costs one sequential ESPN request.") + + +def test_live_only_fetches_for_games_actually_in_progress(): + for cls, _fn, lineno in _fetch_sites(): + if cls != "SportsLive": + continue + assert _guarded_by_positive(lineno, {"is_live", "is_halftime"}), ( + f"SportsLive._fetch_odds at line {lineno} does not sit in the true " + "branch of a test requiring the game to be in progress. Without " + "that, it fans out across the whole event list -- one sequential " + "ESPN request per game.")