fix(nrl-scoreboard): match favorite/exclude teams by ESPN team ID, not abbreviation - #189
Merged
Merged
Conversation
Contributor
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughChangesNRL team ID matching
Plugin metadata updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant PluginConfig
participant DynamicTeamResolver
participant ESPN
participant SportsLive
PluginConfig->>DynamicTeamResolver: resolve configured team values
DynamicTeamResolver->>ESPN: request NRL teams
ESPN-->>DynamicTeamResolver: team names, abbreviations, and IDs
DynamicTeamResolver-->>PluginConfig: resolved team IDs
PluginConfig->>SportsLive: provide favorite team IDs
SportsLive-->>PluginConfig: classify games and gate celebrations by team ID
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 | 33 |
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.
…t abbreviation NRL's ESPN abbreviations aren't unique — "NEW" is both Newcastle Knights and New Zealand Warriors, "CAN" is both Canberra Raiders and Canterbury Bulldogs. Matching favorite_teams/exclude_teams by abbreviation silently conflated these teams. DynamicTeamResolver now resolves configured team names/IDs to unique ESPN team IDs (fetching the NRL team list, with a name/ID lookup and an explicit warning on ambiguous or unresolved abbreviations), and every favorite/exclude membership check in sports.py and manager.py now compares against home_id/away_id instead of home_abbr/away_abbr. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rxy8kp5foWywPeBsCmY1Uo
The plugin safety harness (LEDMatrix core's check_plugin.py) expects mock_data in test/harness.json to be a path string to a fixture file, not inline JSON - it does Path(plugin_dir) / mock_data, which raises TypeError against a dict. Every other plugin harness in this repo follows the path convention (e.g. ledmatrix-weather's test/fixtures/mock.json); this plugin's harness.json had the ESPN response inlined instead, which was failing the "safety" CI check on both this PR and #182. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rxy8kp5foWywPeBsCmY1Uo
ChuckBuilds
force-pushed
the
claude/nrl-team-ids-abbrev-lepchm
branch
from
July 16, 2026 01:24
c3ae9d3 to
8c5aafe
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
plugins/nrl-scoreboard/dynamic_team_resolver.py (1)
69-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSimplify list deduplication.
You can replace this manual deduplication loop with
dict.fromkeys(), which preserves order and is more concise.♻️ Proposed refactor
- # Remove duplicates while preserving order - seen = set() - unique_teams = [] - for team in resolved_teams: - if team not in seen: - seen.add(team) - unique_teams.append(team) - - return unique_teams + # Remove duplicates while preserving order + return list(dict.fromkeys(resolved_teams))🤖 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/nrl-scoreboard/dynamic_team_resolver.py` around lines 69 - 77, Replace the manual deduplication loop using seen and unique_teams in the resolver method with an order-preserving dict.fromkeys() expression over resolved_teams, while returning the same list result.plugins/nrl-scoreboard/sports.py (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
str(id) in team_listmatching logic across ~9 sites. The same "is this game a favorite/excluded" ID-membership check is re-implemented independently insports.py(_is_favorite_game,_classify_live_game,_extract_game_details_common,_select_games_for_display,SportsUpcoming.update,_select_recent_games_for_display,SportsRecent.update,_is_favorite) and inmanager.py. Extracting one shared helper (e.g. aSportsCorestatic method_team_in(team_id, team_list)) would reduce the risk of future edits drifting inconsistently between call sites.
plugins/nrl-scoreboard/sports.py#L185-189: introduce and use a shared_team_in/_any_team_inhelper here as the canonical implementation.plugins/nrl-scoreboard/manager.py#L663-676: reuse the same helper (via the manager or a shared utility) instead of re-deriving the membership check inline.♻️ Example helper
+ `@staticmethod` + def _team_in(team_id, team_list) -> bool: + return bool(team_list) and str(team_id) in team_list + def _is_favorite_game(self, game: Dict) -> bool: - return bool(self.favorite_teams) and ( - str(game.get("home_id")) in self.favorite_teams - or str(game.get("away_id")) in self.favorite_teams - ) + return self._team_in(game.get("home_id"), self.favorite_teams) or self._team_in( + game.get("away_id"), self.favorite_teams + )🤖 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/nrl-scoreboard/sports.py` at line 1, Extract the repeated team-ID membership check into a canonical helper on SportsCore, such as _team_in or _any_team_in, and replace the duplicated str(id) in team_list logic in the listed sports.py methods. Update manager.py to reuse that same helper instead of performing the check inline, preserving existing favorite/exclusion behavior.
🤖 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/nrl-scoreboard/dynamic_team_resolver.py`:
- Around line 173-175: Update the team ID extraction in the dynamic team
resolver to convert valid IDs from t.get("id") to strings before storing them in
the cache, while preserving the existing skip behavior for missing IDs. Ensure
favorite_teams uses string IDs consistently with downstream membership checks in
sports.py.
In `@plugins/nrl-scoreboard/README.md`:
- Around line 67-68: Update the favorite_teams documentation to clarify that
ambiguous abbreviations such as NEW and CAN are rejected or left unresolved by
the resolver, which logs an error, rather than matching both teams. Keep the
full team name or ESPN team ID guidance and the exclude_teams cross-reference
unchanged.
---
Nitpick comments:
In `@plugins/nrl-scoreboard/dynamic_team_resolver.py`:
- Around line 69-77: Replace the manual deduplication loop using seen and
unique_teams in the resolver method with an order-preserving dict.fromkeys()
expression over resolved_teams, while returning the same list result.
In `@plugins/nrl-scoreboard/sports.py`:
- Line 1: Extract the repeated team-ID membership check into a canonical helper
on SportsCore, such as _team_in or _any_team_in, and replace the duplicated
str(id) in team_list logic in the listed sports.py methods. Update manager.py to
reuse that same helper instead of performing the check inline, preserving
existing favorite/exclusion behavior.
🪄 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
Run ID: ce8dedba-5282-4d45-a1c0-6b9db89bdc9f
📒 Files selected for processing (10)
plugins.jsonplugins/nrl-scoreboard/README.mdplugins/nrl-scoreboard/config_schema.jsonplugins/nrl-scoreboard/dynamic_team_resolver.pyplugins/nrl-scoreboard/manager.pyplugins/nrl-scoreboard/manifest.jsonplugins/nrl-scoreboard/sports.pyplugins/nrl-scoreboard/test/fixtures/mock.jsonplugins/nrl-scoreboard/test/harness.jsonplugins/nrl-scoreboard/test_nrl_plugin.py
…tching - dynamic_team_resolver.py: simplify the dedup loop to dict.fromkeys(); stringify ESPN team IDs when building the by_name/by_abbr lookup index so an integer ID from ESPN can't silently fail downstream str() comparisons - sports.py: extract a shared SportsCore._team_in(team_id, team_list) helper and use it at every favorite/exclude membership check (previously ~9 independently-reimplemented `str(id) in team_list` sites across _is_favorite_game, _classify_live_game, _extract_game_details_common, SportsUpcoming.update, SportsRecent.update, _is_favorite) - manager.py: reuse the same helper (via the live manager instance) in _has_favorite_or_all_live instead of re-deriving the check inline - README.md, config_schema.json: fix wording - an ambiguous abbreviation (NEW, CAN) is left unresolved and logged as an error, not matched to either team Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rxy8kp5foWywPeBsCmY1Uo
19 tasks
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.
Summary
Fixes a bug reported by a user testing PR #182 (NRL scoreboard):
ESPN's rugby-league team abbreviations are not unique:
NEWis used by both the Newcastle Knights and the New Zealand Warriors, andCANis used by both the Canberra Raiders and the Canterbury Bulldogs.favorite_teams/exclude_teamsmatching previously compared againsthome_abbr/away_abbr, so configuring one of these abbreviations silently matched both teams.Fix
dynamic_team_resolver.py:DynamicTeamResolvernow fetches the NRL team list from ESPN once (in-process cache, 24h TTL) and resolves each configuredfavorite_teams/exclude_teamsentry to a unique ESPN team ID:NEW,CAN) is not silently resolved to either team — it logs an error naming both candidate teams and instructs the user to use the full team name or ID instead.sports.py/manager.py: every favorite/exclude membership check now compareshome_id/away_id(already extracted per-game) against the resolved team-ID list, instead ofhome_abbr/away_abbr. This covers live rotation filtering/classification, live-priority weighting, goal/win celebrations, and upcoming/recent game selection and filtering.config_schema.json/README.md: updated descriptions to steer users toward full team names or ESPN team IDs, with theNEW/CANcollision called out explicitly.test/harness.json:favorite_teamsswitched from["PEN", "BRI"]to the corresponding team IDs (["18", "16"]) so the offline test harness doesn't depend on a live network fetch to resolve favorites.test_nrl_plugin.py: addedTestDynamicTeamResolverAbbreviationCollisionscovering the ambiguous-abbreviation case, full-name disambiguation, unique-abbreviation resolution, numeric ID passthrough, and fetch-failure fallback (mocksrequests.get, no network needed).Test plan
python3 -m py_compileall touched.pyfiles — passes.python3 test_nrl_plugin.py— 11/11 tests pass (6 pre-existing + 5 new).manifest.json,config_schema.json,test/harness.jsonall parse.python scripts/check_module_collisions.py— no cross-plugin collisions.python update_registry.py— registry synced, version bumped to 1.0.1.check_plugin.pycross-size harness (requires a LEDMatrix core checkout — none available in this environment).site.api.espn.com).🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores