feat(sports): prioritise favourites even when they are not exclusive - #332
Merged
Merged
Conversation
"I want to see my favorites and other teams, not ONLY my favorites, which is
why it was off. Shouldn't we still prioritize favorites in upcoming and recent
even if 'show favorites only' is off?"
They should have, and they did not. show_favorite_teams_only was the whole
story: on, and you saw nothing but your teams; off, and your teams were ignored
ENTIRELY -- selection just took the next N games league-wide. Measured against
a real board's cached schedule: 946 upcoming college games in the window, 119
of them involving a configured favourite, and the three cards actually shown
contained none of them. There was no way to ask for "my teams, plus some
others", which is what most people want.
_favorites_first is that middle setting: favourites first, then up to
other_{upcoming,recent}_games_to_show other games, re-sorted so the cards still
read as a schedule rather than favourites-then-the-rest.
Both limits are TOTALS here, not per-team budgets. In favourites-only mode
upcoming_games_to_show is per team, which is reasonable when the list is your
own handful of teams; applied to a dynamic group it is not -- AP_TOP_10
resolves to a dozen teams, and three games each is 28 cards before a single
other game is added.
Defaults are deliberately the league-wide counts already configured, so a board
that upgrades keeps every game it was showing and simply gains its favourites.
The change is additive; 0 opts into favourites-only.
Ported to all nine sports plugins in this PR per the shared-code rule. Three of
them (afl, nrl, soccer) carry an older recent-games branch with a different log
line and needed their own rewire.
Ablation: replacing _favorites_first with the old chronological take fails 8 of
the 12 checks. The existing wording test was updated rather than left passing --
the branch it described no longer exists in that form.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 504 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
afl, nrl and soccer keep their game limits at the root of the schema rather than in a per-league game_limits block, and the root has an explicit x-propertyOrder. Anything missing from it is dropped from the form entirely, so the two new settings existed in the schema and were invisible in the UI -- caught by scripts/test_property_order_coverage.py, which is exactly the failure it is there to catch. Each key is placed immediately after the setting it modifies, so the form reads "N games, then N others" rather than stranding it at the end. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
…over Enlarging the selection is the obvious way to see more matchups and it does the opposite. The panel already cycles the selected list -- index advances every upcoming_game_duration and persists across visits -- so the limit is a POOL SIZE, not a card count. With a 20s slot and 15s cards that is roughly one card per visit, so a 119-game pool needs ~119 visits per lap and makes any given game rarer, not commoner. It also costs one ESPN odds request per pooled game, each blocking up to 1.5s in _fetch_odds. So the pool stays short and the non-favourite slice moves instead. The window advances by its own width, so consecutive windows are disjoint and the board walks the schedule rather than resampling the front of it, and it wraps so a short list still cycles. Measured on a real board's cached schedule, favourites UGA+AUB with 3 others rotating every 30 minutes: 18 distinct other matchups in three hours, pool still 6 cards, lap still ~90s of airtime, favourites present in every window. Advancing is time-based rather than per-update. update() runs every 30s; a window that moved with it would change the games list identity on every pass, reset the display index to 0, and no card past the first would ever be reached. It advances by however many intervals actually elapsed, so a mode that was not displayed for an hour catches up instead of crawling one step. Favourites are deliberately NOT rotated: for upcoming games the soonest ones are the point, and rotating them would show a week-8 fixture instead of Saturday's. 0 pins the window, restoring the fixed "next N others". Also supplies the new attributes to test_favorites_log_says_which_case's probe, which builds its object with __new__ -- a missing attribute raises inside update()'s own try/except, so the log line silently never appears and the failure reads as a wording change rather than an incomplete object. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
This was referenced Aug 27, 2026
feat(sports): fill the other slots with games worth watching, and make the settings take effect
#333
Closed
ChuckBuilds
added a commit
that referenced
this pull request
Aug 30, 2026
…works (#335) * feat(football): favourites first, ranked others, and a rotation that works Football-scoreboard only. The same work is prepared for the other eight sports plugins and is deliberately held back so each can be judged on its own board rather than nine changing at once. Selection - Five settings added in #332 never reached the display code. Managers read a translated config and that translation is an explicit whitelist, so every one of them was declared in the schema, rendered in the UI, read by sports.py and dropped in between: set it, save it, nothing happens. - Other games are filtered by quality -- ranked by default -- instead of taken in kickoff order. On a real board's college slate, 923 non-favourite upcoming games, 235 of them involving a ranked team. - Within that pool the better matchup leads, and each team appears once. The upcoming list is a whole SEASON, not a week -- ledpi logs 947 games -- so ordering by rank alone put the #1 team's twelve games above the #2 team's first one, and the board walked one team's schedule. - Favourite slots are shared between your teams: each gets its next game before any gets a second. Walked across a real 901-game season with two favourites at a limit of 2, nine days showed one team twice and the other not at all. - The FBS/FCS filter needs one side in a checked division, not both. Requiring both silently removed five of twenty ranked matchups -- a ranked side hosting an FCS school is still a game about a team you asked for. - ncaa_fb defaults to five games with no favourite team, rather than one. Data - College rankings came from the wrong endpoint. /standings was tried first and /rankings only on a 404, but college football answers /standings with 200 and no rankings key, so the fallback never fired: the rank badge never appeared and the ranked filter passed everything, an empty table failing open. - Each poll is fetched once, not once per configured group. - Leagues with no poll no longer request one -- roughly 2,900 dead requests a day, per league, at a 30s interval. - The FBS/FCS group lookup runs for college football alone, the one league ESPN publishes those rosters for. Everywhere else it 500s or returns an empty list. - The division cache expires instead of being held for the life of the process. Pacing - The rotation now re-cuts the slice on the display path. It only ever ran from update(), which returns early until upcoming_update_interval -- an hour, and not settable -- so a four-minute rotation produced one jump of fifteen windows once an hour. ledpi showed the same two matchups 89 times in six hours; it now cycles the ranked slate in about twenty minutes. - recent_update_interval, upcoming_update_interval and stale_game_timeout are declared and translated. They were read by the code and reachable by nobody. Safety - The filters fail open as a set, not only per check: if they leave nothing at all, the unfiltered list is used rather than blanking the mode. Asking for 0 other games is still an explicit "favourites only". - "broadcast" reads coverage off the slate. The payload always carries the key and it is simply empty in leagues ESPN publishes no listings for, so the usual missing-means-allowed reading never fired and the setting removed every non-favourite game there. - A loaded poll that matches no game on the schedule now warns, throttled, and names the abbreviations it holds. - Config values are clamped to their declared ranges. A string where an integer belongs raised inside update()'s own try/except, which surfaces as a mode that renders nothing. Repo guards - scripts/check_selection_settings.py finds every settings block structurally and requires each setting the plugin's own code reads. Deriving the requirement from the code is what lets these nine plugins be ported one at a time without a red build. - run_plugin_tests names the checks that failed instead of the last line logged, which for a script that warns on stderr was the warning, pass or fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * test(football): build the resolver's kwargs once, not a keyword it may not take Codacy flagged `DynamicTeamResolver(cache_manager=cache)` as an unexpected keyword, and it is right about the code as written. Five plugins ship a class of that name and three of them -- basketball, hockey, lacrosse -- take no cache_manager at all, so `from dynamic_team_resolver import ...` gives an analyser a call it can prove wrong for whichever class the bare module name resolved to. The runtime guard made it safe; it did not make it checkable. The signature is now inspected once and the argument passed as **kwargs, which also stops the same condition being written twice -- once for the first resolver and once for the second, where the two spellings could drift apart. Verified against both shapes: football's own resolver, and hockey's swapped in, which takes no cache_manager. The second constructs cleanly and then fails the three behavioural checks, because hockey has not been ported yet and still keys its cache per pattern rather than per sport -- which is the bug this PR fixes, and exactly what that test exists to catch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * fix(football): act on the review -- one selection path, and two silent failures Five of the eight findings held up. The two most serious were mine and recent. 1. The adapter coerced the divisions list before the coercion helper could see it: `list(game_limits.get("other_games_divisions", ["fbs"]))`. A hand-edited "fbs" became ['f','b','s'] -- already a list, so _normalise_divisions' string branch never fired -- and the filter then intersected against three letters that name no division and rejected every non-favourite game. A null was worse: list(None) raises inside _adapt_config_for_manager, which _initialize_managers catches and logs once, leaving all six managers None and the plugin enabled but rendering nothing. Passed through raw now. 2. The no-favourites branch had its own selection path -- filter, sort, truncate -- and so never built the selection pools. Nothing rotated, nothing was ordered by rank. That is the DEFAULT configuration for college football, and this PR had just made it the default for five games, so the board most in need of the pacing work was the one board not getting it. Both branches now call _favorites_first, the no-favourites case with a favourite limit of 0, which retires _filtered_or_all: one path, one fallback. 3. fetch_standings suppressed every non-HTTP failure. `status not in (404, None)` reads None for ConnectionError, Timeout and a body that will not parse, so a board that could not reach ESPN said one debug line and ran the ranked filter against an empty table. Only 404 is routine now. 4. The division roster asked for datetime.now().year. College football's 2026 season runs into January 2027, when that returns groups which do not exist, the roster comes back empty and division filtering fails open -- through the bowls and the playoff. Derived from the season instead. 5. The window advance is a read-modify-write with two writers -- update() and the display path -- and neither held a lock at that point. Interleaved, both see the interval elapsed and each add a width, skipping a window nobody sees. Now under _games_lock, which is an RLock and which the display path takes again immediately after. 6. The rotation gated on `others`, but the whole-set fallback slices `unfiltered`. In exactly the case that triggers the fallback the pool read as empty and the fallback was pinned until the next fetch. Two findings I did not take: - Odds are fetched for the selection, so a card rotated in between fetches renders without a spread. I widened the fetch to cover the rotation pool and scripts/test_odds_fetch_scope.py failed -- correctly. That guard exists because a rig once made 946 odds requests to put one game on the panel, and widening the scope inside update() is exactly what it forbids. Reverted; the gap is real but belongs in a change that revisits that decision deliberately, not in this one. - The README's default for upcoming_games_to_show was stale against the new ncaa_fb schema default. Corrected -- that one I did take. The odds guard needed a change of its own: it located the trim by searching for the literal slice, which merging the two selection branches moved inside the helper. It now accepts either shape, and still fails when the fetch is pointed back at the collected list, which is the bug it was written for. Two probes built with __new__ needed the lock supplied, which is the failure mode their own comments warn about: a missing attribute raises inside update()'s try/except, and the test reads as a wording change rather than an incomplete object. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * fix(football): stop the due-check recomposing every frame, and tell the truth about game counts The rotation due-check guessed which pool _compose_selection would slice as `others or unfiltered`, but the composer only falls back to the unfiltered list when favourites contribute nothing. With a favourite playing and the filters rejecting every other game, the guess said 'rotate', the recompose produced an identical favourites-only list, the rotation clock never advanced, and selection re-ran on every display() call forever. The check now mirrors the composer's rule; the new test fails on the old expression and no other check. The games_to_show schema descriptions still said 'N games per favorite team', which is only true with show_favorite_teams_only on -- in the default favourites-first mode they are totals. Reworded, and the 'additive, never a removal' comment scoped to the counts it is true of. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YF7Q48EYCCkU1Vs932uDY1 * fix: address the CodeRabbit review All six findings verified against the branch before fixing: - broadcast is coerced to a string at extraction; ESPN's undocumented payloads have served it as an object, and a non-string reached _note_broadcast_coverage's .strip() inside update()'s try/except. - _setting_int also catches OverflowError: json parses a bare Infinity and int(inf) raises from __init__, outside any fallback. - check_selection_settings rejects unknown plugin ids up front instead of printing OK and then crashing on the block count. - run_plugin_tests keeps a labelled tail from each output stream; concatenating them reported only stderr when both had evidence. - README: the AP_TOP_10 example now says 'up to 12 favorites' with the overlap caveat, and the rotation example fence names a language. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YF7Q48EYCCkU1Vs932uDY1 --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Chuck <chuck@example.com>
ChuckBuilds
pushed a commit
that referenced
this pull request
Aug 31, 2026
… this Main moved a long way under this branch. Resolving hunk by hunk showed the work splits cleanly in two, and the halves want opposite resolutions. **football-scoreboard is fully superseded.** What this branch does for football shipped as #335 and was then improved by #341, #343 and #344. Main is strictly ahead everywhere they overlap: it season-corrects the division roster lookup, str()-guards ESPN's broadcast field, catches OverflowError on a bare Infinity, routes the no-favourites branch through _favorites_first so it builds selection pools, and adds _attach_odds_to_rotated_games, which this branch does not have at all. Several of main's comments describe this branch's own approach as the old way. Every football hunk therefore takes main, and football's sports.py, manager.py, data_sources.py, tests and README now match origin/main byte for byte -- this branch no longer changes football in any way. **The other eight lineages still need all of it.** They have _favorites_first and nothing else: no _compose_selection, no rotation, no _passes_other_filters. Their settings were in the schema with nothing reading them. Those hunks take this branch. Three resolutions worth naming: - baseball, basketball, hockey and lacrosse: main re-adds _is_favorite_game beside the rotation methods this branch adds. Taking both would have defined it twice in one class, and Python takes the last -- main's copy would have silently shadowed this branch's. Kept one, plus main's _DRAWS_SCORE ClassVar. - nrl: main defines _is_favorite_game TWICE in SportsCore, at 320 and 2177, and they do not agree -- the first matches on ESPN team id, the second on abbreviation. #189 moved nrl to ids deliberately; #332 added the abbr copy, which shadowed and silently reverted that fix. This branch removes the duplicate, so the fix is restored. That is a live bug on main today. - Manifests and plugins.json take main, then the eight changed plugins are re-bumped on top of the versions main has since published. Football is not bumped, because nothing about it changed. READMEs: football takes main; afl, baseball and basketball keep both sections, since this branch documents selection and main documents the matchup separator. Verified: run_plugin_tests.py --all gives 222 passed, 2 skipped, 1 failed, and that one failure -- football's test_favorite_live_boost.py, "excluded team hidden from recent/final scores in default (no-favorites) path" -- reproduces identically on a clean origin/main worktree. It is main's, not this merge's: _favorites_first(games, 0, N) does not apply the exclude filter the old filter/sort/truncate path did. The eight apply excludes elsewhere and pass. check_selection_settings, check_manifest_version_fields, check_scroll_adoption and check_module_collisions all pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9
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.
They should have, and they did not.
show_favorite_teams_onlywas the whole story: on, and you saw nothing but your teams; off, and your teams were ignored entirely — selection just took the next N games league-wide. There was no way to ask for "my teams, plus some others", which is what most people actually want.Measured, not asserted
Against a real board's cached NCAA schedule (946 upcoming games, 119 involving a configured favourite, favourites =
UGA, AUB, AP_TOP_10,upcoming_games_to_show: 3):119 favourite games available; zero of them reaching the panel.
The design
_favorites_firstputs favourites first, tops up withother_upcoming_games_to_show/other_recent_games_to_showothers, then re-sorts by time so the cards still read as a schedule rather than favourites-then-the-rest.Both limits are totals, not per-team budgets. In favourites-only mode
upcoming_games_to_showis per team — reasonable when the list is your own handful of teams. Applied to a dynamic group it is not:AP_TOP_10resolves to a dozen teams, and three games each is 28 cards before a single other game is added.Defaults are the league-wide counts already configured, so a board that upgrades keeps every game it was showing and simply gains its favourites. The change is additive — never a removal.
0opts into favourites-only.Scope
All nine sports plugins, per the shared-code rule. Three of them (afl, nrl, soccer) carry an older recent-games branch with a different log line and needed their own rewire — worth knowing for the next person who greps for one anchor and assumes nine hits.
Verification
run_plugin_tests.py --alltest_favorites_log_says_which_case.pywas updated rather than left passing — the branch whose wording it guarded no longer exists in that form, and it failed on exactly the two checks describing the old behaviour.🤖 Generated with Claude Code
https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW