Catch team-picker drift against ESPN instead of waiting for a bug report - #236
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds ChangesESPN picker synchronization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Checker
participant ESPN
participant PluginSchema
CLI->>Checker: run picker validation
Checker->>PluginSchema: locate favorite_teams pickers
Checker->>ESPN: fetch league teams
ESPN-->>Checker: return abbreviations and display names
Checker->>PluginSchema: report drift or apply updates
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 | 76 |
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
plugins/lacrosse-scoreboard/manager.py (1)
952-953: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid blind exception handling in both favorite-team diagnostics.
Narrow the failure boundary or preserve traceback information so unexpected checker regressions remain diagnosable.
plugins/lacrosse-scoreboard/manager.py#L952-L953: replace the blind catch or log withexc_info=True.plugins/nrl-scoreboard/manager.py#L476-L477: replace the blind catch or log withexc_info=True.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/lacrosse-scoreboard/manager.py` around lines 952 - 953, Update the favorite-team diagnostic exception handlers in plugins/lacrosse-scoreboard/manager.py at lines 952-953 and plugins/nrl-scoreboard/manager.py at lines 476-477 to preserve traceback details by passing exc_info=True to the logger calls, or narrow the caught exception boundary; retain the existing skipped-check behavior in both locations.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins.json`:
- Line 79: Update the top-level last_updated metadata for the eight referenced
plugin manifests from 2026-07-17 to 2026-07-29, matching their new release
dates. Then run the repository’s pre-commit hook to regenerate plugins.json; do
not edit the generated registry manually.
In `@plugins/basketball-scoreboard/manager.py`:
- Around line 943-975: BasketballScoreboardPlugin and HockeyScoreboardPlugin
must reset their cached favorite-team checker after configuration reloads. In
plugins/basketball-scoreboard/manager.py lines 943-975 and
plugins/hockey-scoreboard/manager.py lines 985-1017, add
on_config_change(new_config) that calls super().on_config_change(new_config) and
resets self._favorite_check when it exists, following the established
afl-scoreboard, baseball-scoreboard, and football-scoreboard pattern.
In `@plugins/hockey-scoreboard/hockey_favorite_check.py`:
- Around line 201-208: Update _parse_date in
plugins/hockey-scoreboard/hockey_favorite_check.py lines 201-208 to assign
timezone.utc to parsed datetimes lacking tzinfo, and add a ScheduleNoteTests
case covering an offset-less date string. Apply the identical _parse_date change
in plugins/afl-scoreboard/afl_favorite_check.py lines 201-208,
plugins/baseball-scoreboard/baseball_favorite_check.py lines 201-208,
plugins/basketball-scoreboard/basketball_favorite_check.py lines 201-208,
plugins/football-scoreboard/football_favorite_check.py lines 201-208,
plugins/lacrosse-scoreboard/lacrosse_favorite_check.py lines 201-208, and
plugins/nrl-scoreboard/nrl_favorite_check.py lines 201-208, preserving
byte-for-byte parity across all copies.
- Around line 146-154: Update the team-roster request flow near the payload
parsing to retain the response from requests.get, call
response.raise_for_status() before parsing JSON, then parse response.json() and
preserve the existing teams mapping.
In `@plugins/nrl-scoreboard/manager.py`:
- Around line 455-475: Serialize the entire favorite-check flow in
_check_favorite_teams with _config_lock, including checker lookup/creation,
manager snapshot, and checker.schedule. Update on_config_change to perform
reset() under that same lock, preventing stale favorites from being scheduled
after a configuration reset and ensuring new favorites are validated.
In `@scripts/check_team_pickers.py`:
- Around line 143-202: Handle an empty successful result from fetch_teams in the
per-picker loop before the existing `if not live: continue` path. Add a warning
identifying `where` and the ESPN path, then skip comparison without allowing the
picker to be reported as OK or included in a successful checked tally; preserve
the existing connectivity-failure warning behavior and normal comparison flow
for non-empty rosters.
---
Nitpick comments:
In `@plugins/lacrosse-scoreboard/manager.py`:
- Around line 952-953: Update the favorite-team diagnostic exception handlers in
plugins/lacrosse-scoreboard/manager.py at lines 952-953 and
plugins/nrl-scoreboard/manager.py at lines 476-477 to preserve traceback details
by passing exc_info=True to the logger calls, or narrow the caught exception
boundary; retain the existing skipped-check behavior in both locations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: eb5f7212-4d6a-4c2d-8026-59c944bf8875
📒 Files selected for processing (33)
CLAUDE.mdplugins.jsonplugins/afl-scoreboard/afl_favorite_check.pyplugins/afl-scoreboard/manager.pyplugins/afl-scoreboard/manifest.jsonplugins/baseball-scoreboard/CHANGELOG.mdplugins/baseball-scoreboard/baseball_favorite_check.pyplugins/baseball-scoreboard/manager.pyplugins/baseball-scoreboard/manifest.jsonplugins/basketball-scoreboard/basketball_favorite_check.pyplugins/basketball-scoreboard/config_schema.jsonplugins/basketball-scoreboard/manager.pyplugins/basketball-scoreboard/manifest.jsonplugins/football-scoreboard/CHANGELOG.mdplugins/football-scoreboard/config_schema.jsonplugins/football-scoreboard/football_favorite_check.pyplugins/football-scoreboard/manager.pyplugins/football-scoreboard/manifest.jsonplugins/hockey-scoreboard/config_schema.jsonplugins/hockey-scoreboard/hockey_favorite_check.pyplugins/hockey-scoreboard/manager.pyplugins/hockey-scoreboard/manifest.jsonplugins/hockey-scoreboard/test_favorite_check.pyplugins/lacrosse-scoreboard/CHANGELOG.mdplugins/lacrosse-scoreboard/lacrosse_favorite_check.pyplugins/lacrosse-scoreboard/manager.pyplugins/lacrosse-scoreboard/manifest.jsonplugins/nrl-scoreboard/manager.pyplugins/nrl-scoreboard/manifest.jsonplugins/nrl-scoreboard/nrl_favorite_check.pyplugins/odds-ticker/config_schema.jsonplugins/odds-ticker/manifest.jsonscripts/check_team_pickers.py
A favorite_teams checkbox-group is a hand-maintained copy of a league's
roster, and rosters change — clubs get renamed, relocated, or added. When
the copy drifts nothing errors. The picker simply fails to offer a team
that exists, or offers a code that matches nothing, and the user gets an
empty screen.
odds-ticker's NHL list had both faults at once, and had had them long
enough for a user to hit it: it still listed UTA, a retired code, under
the club's former name, and omitted the Seattle Kraken entirely so that
team could not be selected at all. Nothing in CI could have noticed.
scripts/check_team_pickers.py compares every picker in the repo against
ESPN and separates the two kinds of difference, because they are not
equally serious:
enum which codes exist. A mismatch is a bug, so it fails the check
and names the teams that cannot be selected.
labels the display names. ESPN's own text is sometimes worse than the
hand-written label — it calls the Clippers "LA Clippers" where
the schema says "Los Angeles Clippers" — so this only warns,
and --apply never overwrites an existing label unless asked.
--apply regenerates the enums, fills in labels only where one is missing,
and preserves the file's existing escaping style so a three-line fix does
not arrive as hundreds of lines of reformatting.
Verified by reintroducing the exact pre-fix NHL state, which the checker
catches and reports precisely:
odds-ticker: nhl - offers 1 which ESPN does not have: UTA
odds-ticker: nhl - cannot select 2 real team(s): SEA (Seattle Kraken),
UTAH (Utah Mammoth)
and confirming --apply restores it with the hand-written NBA label intact.
All four current pickers (NFL, NBA, MLB, NHL) pass.
Unreachable ESPN warns rather than fails, so an outage cannot turn into a
red build. A picker keyed by an unknown league is reported rather than
skipped, so adding a league cannot quietly opt out of the check.
Intended to run on a weekly schedule rather than per-PR, since the check
needs the network and a roster does not change because someone opened a
pull request. The workflow file is not included here: pushing
.github/workflows requires an OAuth token with the workflow scope, which
this one does not have. It is ready to add separately.
No plugin code or schema changes here — tooling only.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
ChuckBuilds
force-pushed
the
feat/team-picker-drift-check
branch
from
July 30, 2026 13:19
e2abf8a to
d9c3063
Compare
Owner
Author
|
Heads-up on merge order: this PR must land after #234. The checker correctly fails against current That is the tool working as intended — it is detecting the real bug — but it means CI here goes red until #234 is in. Merge #234 first and this goes green with no changes. (Also rebased onto |
Two issues flagged by Codacy on the previous commit: - Dropped an unused 're' import left over from an earlier draft. - Pinned the URL scheme to https before opening it. The league path is interpolated into the URL, and urlopen would honour file:// or a custom scheme if a path ever arrived from somewhere less trustworthy than the hardcoded table. Bandit's B310 is a syntactic blacklist rule so it still fires on the call itself; annotated with the reason rather than left to look unexamined. Verified: bandit clean, the guard rejects a file:// URL, and the checker still fetches all 32 NHL teams and still exits 1 against main (which is correct until #234 lands). Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ MSG
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scripts/check_team_pickers.py (1)
167-175: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEmpty-but-fetched roster silently reported as "OK".
If
fetch_teamssucceeds but returns an empty dict, this picker is dropped with no DRIFT/OK/warn line and no entry inproblems/warnings, yet it was already counted viachecked += 1(line 158).main()can print"OK: N picker(s) match ESPN."even though N includes pickers never actually compared.🩹 Proposed fix
live = rosters[espn_path] if not live: + warnings.append("{} - ESPN returned no teams for this league".format(where)) continue🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check_team_pickers.py` around lines 167 - 175, Handle an empty roster returned by fetch_teams in the live-roster comparison flow around rosters and live instead of silently continuing. Record the picker as an explicit warning/problem (or otherwise apply the existing non-success reporting path) and ensure checked reflects only pickers that were actually compared, so main() cannot report OK for an unvalidated empty roster.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/check_team_pickers.py`:
- Around line 211-236: Update the --apply path in the pending-schema rewrite
flow to print all accumulated warnings and unresolved problems before returning,
including unknown-league entries and failed ESPN fetches that are not
represented in pending. Preserve the existing rewrite behavior, but only report
successful regeneration after surfacing these diagnostics; ensure hard failures
are tracked and cause the command to return the appropriate nonzero status
instead of exiting 0 silently.
---
Duplicate comments:
In `@scripts/check_team_pickers.py`:
- Around line 167-175: Handle an empty roster returned by fetch_teams in the
live-roster comparison flow around rosters and live instead of silently
continuing. Record the picker as an explicit warning/problem (or otherwise apply
the existing non-success reporting path) and ensure checked reflects only
pickers that were actually compared, so main() cannot report OK for an
unvalidated empty roster.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 617f4b18-25e2-4453-bc01-267de2c70204
📒 Files selected for processing (2)
CLAUDE.mdscripts/check_team_pickers.py
🚧 Files skipped from review as they are similar to previous changes (1)
- CLAUDE.md
An ESPN fetch that succeeds but returns an empty roster was silently skipped and counted toward the final "OK" tally with no warning. Warn instead, distinguishing it from the existing connectivity-failure warning. --apply returned 0 immediately after rewriting fixable schemas, without ever printing warnings or problems that never made it into `pending` — an unknown-league entry is the only such case, and it was silently dropped from --apply's output entirely, exiting 0 on a real unresolved config problem. Track those as `unresolved` separately so --apply always surfaces and fails on them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
Owner
Author
|
Resolving the 5 stale CodeRabbit threads on The two live findings on |
ChuckBuilds
pushed a commit
that referenced
this pull request
Aug 2, 2026
Minor bumps (new user-facing feature) for the seven plugins #235 actually changes, each above main's post-#237 version: afl 1.2.0, baseball 1.21.0, basketball 1.9.0, football 2.10.0, hockey 1.6.0, lacrosse 1.6.0, nrl 1.2.0 odds-ticker is intentionally NOT bumped: its only change in #235 was the NHL picker correction (UTA->UTAH, +Seattle) that came from #234 and is already in main, so it has no net change here. The original bump list also targeted numbers main has since passed via #234/#236/#237/#239; corrected. plugins.json regenerated; every changed plugin is strictly above main. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
ChuckBuilds
added a commit
that referenced
this pull request
Aug 3, 2026
* Fix wrong ESPN team codes in pickers and help text
Several plugins documented — or in one case offered in a picker — team
abbreviations that ESPN does not use, so copying them matched no team and
the plugin silently showed nothing.
odds-ticker's NHL picker was the only one where the user could not work
around it: it listed UTA (a retired code, labelled with the club's former
name "Utah Hockey Club" rather than "Utah Mammoth") and omitted the
Seattle Kraken entirely, so that team could not be selected at all. The
enum and labels are now generated from ESPN's team endpoint and match it
exactly at 32 teams.
The rest are description-only corrections to the favorite_teams examples:
basketball NBA GSW -> GS (Golden State Warriors)
basketball WNBA NYL -> NY (New York Liberty)
basketball WNBA LAS -> LA (Los Angeles Sparks)
basketball NCAAW UCONN -> CONN (UConn Huskies)
basketball NCAAW SCAR -> SC (South Carolina Gamecocks)
football NCAAFB BAMA -> ALA (Alabama Crimson Tide)
hockey NCAAWH WISC -> WIS (Wisconsin Badgers)
Each description now also says these are ESPN's codes and are not always
the ones you would guess, since that is the underlying trap.
Every code here was verified against
site.api.espn.com/apis/site/v2/sports/{sport}/{league}/teams?limit=1000.
The limit matters: without it the default page size truncates the NCAA
responses (362 of 755 teams) and makes valid codes look wrong.
Left alone deliberately: lacrosse-scoreboard's WISC/MINN/OSU and
BU/BC/MICH examples, and baseball-scoreboard's MiLB DUR/SWB/NOR, because
ESPN's lacrosse team endpoints return zero teams and the MiLB one 404s.
Unverifiable, so not guessed at.
No rendering code changed; the safety harness passes for all four plugins
at every size.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
* Explain an empty scoreboard instead of leaving the user guessing
Favorite teams are matched by exact ESPN abbreviation, so a code that is
not real matches no game and the plugin shows nothing — with no hint that
the code is the problem. Out of season, a perfectly correct code produces
the identical empty screen. The two were indistinguishable from the logs,
which is how a user ends up asking whether their config is broken when it
is only July.
Each of these plugins now says which case it is:
WARNING NFL favorite team 'GBP' is not a NFL team code. Closest match
is 'GB' (Green Bay Packers). Every code this league accepts is
listed at https://site.api.espn.com/.../nfl/teams?limit=1000.
INFO NFL favorite teams TB look correct, but the league has nothing
on until 06 August 2026. An empty display until then is
expected, not a configuration problem.
INFO NCAA Baseball favorite teams UGA look correct, but the season
has finished and the next one's fixtures are not published yet.
Suggestions rank word-initial matches first, because string similarity is
useless at three characters: 'MUN' scores identically against 'MAN' and
'SUN', so Manchester United and Sunderland tie and the answer is a coin
flip. Fragments are handled too ('BAMA' is inside 'Alabama' but
abbreviates nothing in it), and a code that only differs in case is told
so rather than guessed at.
Reading the schedule turned out to be the subtle part, and both traps are
real ESPN behaviour confirmed against live endpoints:
- An out-of-season league does not return an empty scoreboard. ESPN rolls
forward to the next day with fixtures, so in July the NHL endpoint
returns seven September games. Emptiness cannot be the signal.
- A *finished* season rolls nowhere and returns its last game instead,
months in the past — so dates must be filtered before the soonest one
means anything. Filtering on "later than now" then wrongly drops games
that started earlier today and reports a live slate as a dead season,
so the window is the last 24 hours.
Verified against every league these plugins cover: MLB, AFL, NRL and WNBA
correctly stay quiet; NFL, NCAA football, NHL, NBA and NCAA men's
basketball report their start dates; NCAA baseball and NCAA women's
hockey report finished seasons.
Safety, since this runs inside update():
- It runs on a daemon thread, so it never delays a frame.
- Once per league per process, re-armed only when the config changes.
- Every failure path is swallowed to a debug line. A plugin whose ESPN
endpoint returns no teams at all (college lacrosse) draws no conclusion
rather than calling a valid code wrong.
Each plugin ships its own copy of the module, since the loader gives
plugins no shared library to import from, under a plugin-unique name per
the module-collision rule. A test asserts the copies stay byte-identical
while they live in one checkout.
Tested: 30 unit tests; safety harness 24/24 PASS per plugin (168 renders,
zero failures); module-collision check clean. Validated end-to-end on real
hardware, where all four message paths appeared as intended with no errors.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
* chore: version the favorite-team diagnostics above current main
Minor bumps (new user-facing feature) for the seven plugins #235 actually
changes, each above main's post-#237 version:
afl 1.2.0, baseball 1.21.0, basketball 1.9.0, football 2.10.0,
hockey 1.6.0, lacrosse 1.6.0, nrl 1.2.0
odds-ticker is intentionally NOT bumped: its only change in #235 was the NHL
picker correction (UTA->UTAH, +Seattle) that came from #234 and is already in
main, so it has no net change here. The original bump list also targeted
numbers main has since passed via #234/#236/#237/#239; corrected. plugins.json
regenerated; every changed plugin is strictly above main.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
---------
Co-authored-by: Claude <noreply@anthropic.com>
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.
Tooling only — no plugin code or schema changes, so this targets
maindirectly and is independent of #234 / #235.Why
A
favorite_teamscheckbox-group is a hand-maintained copy of a league's roster, and rosters change: clubs get renamed, relocated, or added. When the copy drifts, nothing errors — the picker just fails to offer a team that exists, or offers a code that matches nothing, and the user gets an empty screen.odds-ticker's NHL list had both faults at once, and had had them long enough for a user to hit it:
UTA— a retired code, listed under the club's former name ("Utah Hockey Club", now Utah Mammoth)Nothing in CI could have noticed. That's what this gives you a way to check.
What it does
scripts/check_team_pickers.pycompares every picker in the repo against ESPN, and separates the two kinds of difference because they aren't equally serious:--applyregenerates the enums, fills in labels only where one is missing, and preserves the file's existing escaping style — so a three-line fix doesn't arrive as hundreds of lines of reformatting churn.Usage
Run it by hand — before a release, or whenever a league looks off:
Deliberately not wired to CI. There is no workflow in this PR and none intended: rosters do not change because someone opened a pull request, so a PR trigger would miss the actual failure mode while adding a network dependency to every build. It is a check you reach for, not one that runs itself.
Verification
Reintroduced the exact pre-fix NHL state. The checker catches it and says precisely what is wrong:
Exit code 1. Then
--applyrestores it and the hand-written NBA label survives untouched.Two deliberate robustness choices:
Note on merge order
This should land after #234. Against current
mainthe checker correctly fails, because main still contains the NHL picker bug that #234 fixes — the tool working as intended, but worth knowing before you run it on main.Summary by CodeRabbit
New Features
Documentation