Add AFL scoreboard plugin - #183
Conversation
Fork the sport-agnostic soccer-scoreboard into a single-league AFL (Australian Football League) scoreboard. - New afl_managers.py: ESPN australian-football/afl fetch + parsing, with AFL quarter (Q1-Q4) / HALF / Final / pre-game period_text mapping. - New single-league manager.py (AflScoreboardPlugin): afl_live / afl_recent / afl_upcoming modes, keeping switch/scroll display, dynamic duration, live priority + celebration, and Vegas scroll hooks. - Verbatim sport-generic modules (sports.py, game_renderer.py, data_sources.py, base_odds_manager.py, dynamic_team_resolver.py, scroll_display.py) with soccer wording/logo-dir constants updated for AFL. - Flat single-league config_schema.json (draft-07) with full customization parity lifted from soccer's per-league schema. - manifest.json, README.md, requirements.txt, plugins.json registry entry. - test/harness.json fixture (live/recent/upcoming) + test_afl_plugin.py smoke tests (mode routing, live content, AFL quarter parsing). Data source: https://site.api.espn.com/apis/site/v2/sports/australian-football/afl/scoreboard Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
📝 WalkthroughWalkthroughAdds the AFL Scoreboard plugin with live, recent, and upcoming AFL modes, ESPN-backed data retrieval, configurable rendering and scrolling, live celebrations, plugin metadata, configuration schema, documentation, and deterministic smoke-test fixtures. ChangesAFL Scoreboard plugin
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Host
participant AflScoreboardPlugin
participant SportsLive
participant ESPNDataSource
participant GameRenderer
participant DisplayManager
Host->>AflScoreboardPlugin: update()
AflScoreboardPlugin->>SportsLive: update()
SportsLive->>ESPNDataSource: fetch_live_games()
ESPNDataSource-->>SportsLive: live events
Host->>AflScoreboardPlugin: display()
AflScoreboardPlugin->>GameRenderer: render_game_card(game)
GameRenderer-->>AflScoreboardPlugin: rendered image
AflScoreboardPlugin->>DisplayManager: update image
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 | 1139 |
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.
…cepts, unused var) Same fix pattern applied to the sibling nrl-scoreboard plugin (both are forks of the same soccer-scoreboard base and shipped the same issues): - game_renderer.py, sports.py: replace bare `except Exception: pass` with a logged fallback so font/logo-directory failures aren't silently swallowed - afl_managers.py: remove unused `pathlib.Path` import - sports.py: remove duplicate `pathlib.Path` import (already imported above) - base_odds_manager.py: drop unused `as e` — logger.exception() already captures the traceback Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rxy8kp5foWywPeBsCmY1Uo
|
@coderabbitai full review Generated by Claude Code |
|
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 49 minutes. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (3)
plugins/afl-scoreboard/scroll_display.py (1)
333-339: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winA new
GameRenderer(and its fonts) is created on everyprepare_scroll_contentcall.
self._logo_cacheis threaded through correctly, butGameRenderer.__init__also calls_load_fonts(), which does severalImageFont.truetype()disk loads. Since a newGameRendereris instantiated here each time scroll content is (re)prepared, fonts get reloaded from disk repeatedly instead of being cached once. Consider caching the renderer instance (e.g., onself) and just updating its rankings cache/config between calls.🤖 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/afl-scoreboard/scroll_display.py` around lines 333 - 339, The prepare_scroll_content flow recreates GameRenderer and reloads its fonts on every call. Cache a single GameRenderer instance on the owning display object, reuse it across calls, and update any rankings cache or configuration that may change before rendering while preserving the existing logo_cache, logger, dimensions, and config behavior.plugins/afl-scoreboard/game_renderer.py (1)
505-510: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRecord font reloaded from disk on every card render.
ImageFont.truetype("assets/fonts/4x6-font.ttf", 6)is loaded fresh each time_draw_records_or_rankingsruns, i.e. once per game card. In scroll mode,prepare_scroll_contentrenders a card per game, so this repeats needlessly for every game in the scroll. Consider loading it once in_load_fonts/__init__and reusing it, mirroring howself.fontsalready caches the other fonts.🤖 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/afl-scoreboard/game_renderer.py` around lines 505 - 510, Move the record font loading from _draw_records_or_rankings into _load_fonts or __init__, store the result in the existing cached self.fonts structure, and reuse that cached font when drawing records or rankings instead of calling ImageFont.truetype per card render.plugins/afl-scoreboard/manager.py (1)
472-550: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate game-normalization logic (also at Line 933-939).
The
league/status.statenormalization block is duplicated verbatim in_ensure_scroll_content_for_vegas(lines 933-939). Worth extracting into a shared helper (e.g._normalize_games_for_scroll(games, mode_type)) to avoid the two copies drifting apart.🤖 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/afl-scoreboard/manager.py` around lines 472 - 550, Extract the duplicated game normalization logic from _display_scroll_mode and _ensure_scroll_content_for_vegas into a shared helper such as _normalize_games_for_scroll(games, mode_type). Replace both inline blocks with calls to that helper, preserving the league default, status dictionary initialization, and mode-specific state mapping.
🤖 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/afl-scoreboard/afl_managers.py`:
- Around line 212-235: The running-clock condition after the status-state
branches must exclude halftime states, including the STATUS_HALFTIME case
handled by the preceding branch. Update the clock append logic around
period_text and status so HALTIME remains exactly “HALF”, while clocks continue
to append for ordinary live periods.
- Line 15: Rename the plugin’s generic sports module to a domain-prefixed
filename such as afl_scoreboard_sports.py, then update the import in
afl_managers.py to reference the renamed module while preserving the existing
SportsCore, SportsLive, SportsRecent, and SportsUpcoming imports.
In `@plugins/afl-scoreboard/base_odds_manager.py`:
- Around line 99-105: Update the cached-data handling in the relevant lookup
methods to return immediately when cached_data is a dictionary containing the
no_odds marker, instead of continuing to the ESPN request path. Preserve the
existing debug log and return the method’s established no-odds/empty-result
value; apply the same change to both occurrences, including the flow around the
second referenced block.
In `@plugins/afl-scoreboard/config_schema.json`:
- Line 6: Move the customization schema definition into the top-level properties
object so it is accepted under additionalProperties: false and included in
x-propertyOrder. Also update the exclude_teams items description from ENG to
AFL.
In `@plugins/afl-scoreboard/data_sources.py`:
- Around line 223-231: Update fetch_standings to avoid hardcoding leagueId to
103: use the provided league argument when constructing the standings request,
or pass both MLB league IDs 103 and 104 so American and National League teams
are included.
- Around line 245-251: Remove the unused AFLAPIDataSource class, including its
constructor and football-data.org configuration, while leaving the existing
ESPNDataSource implementation and its usage unchanged.
- Line 12: Update fetch_live_games to query a small date range around the host’s
current local date rather than only today’s date, while preserving the existing
live-state filter and using the datetime utilities already imported.
In `@plugins/afl-scoreboard/game_renderer.py`:
- Around line 83-104: The detail font fallback in _load_custom_font is using the
wrong filename. Update the detail font configuration to reference 4x6-font.ttf,
matching the fallback filename used in the surrounding font-loading logic, while
leaving the other font mappings unchanged.
- Around line 165-199: The logo-loading flow in
GameRenderer._load_and_resize_logo currently returns None when the local
logo_path is absent, leaving ScrollDisplay without a logo. Add the
missing-download fallback there using the existing logo_url and download
mechanism, then process the downloaded image through the same RGBA conversion,
transparent cropping, resizing, and _logo_cache storage path before returning
it; preserve the current logging and None fallback when no URL or download is
available.
In `@plugins/afl-scoreboard/manager.py`:
- Around line 166-184: Initialize last_mode_switch in the AFL scoreboard manager
so the first display() call without an explicit display_mode does not
immediately satisfy the duration threshold and advance current_mode_index from
0. Preserve the initial mode-0 render, while keeping subsequent timed cycling
behavior unchanged.
- Around line 383-411: Move the in-flight thread draining in on_config_change
outside the self._config_lock scope so display() and update() are not blocked by
sequential thread.join(timeout=10.0) calls. Snapshot and clear
_active_update_threads before joining, perform all joins without the lock, then
acquire _config_lock to clear scroll state and reinitialize managers; preserve
the existing runtime configuration updates and final state initialization.
- Around line 735-813: Update supports_dynamic_duration and
get_dynamic_duration_cap to accept an explicit mode_type parameter and use it
when resolving dynamic-duration configuration, rather than relying on
_current_display_mode_type. In get_cycle_duration, pass the locally derived
mode_type to both helpers so explicit display-mode queries use the correct mode
limits; preserve existing fallback behavior when no mode type is available.
In `@plugins/afl-scoreboard/scroll_display.py`:
- Around line 31-62: Replace the soccer-specific LEAGUE_NAMES defaults and
league-separator handling with AFL-specific behavior in the scoreboard display,
adding an afl league entry or removing the separator path if multi-league
separators are unnecessary. Update the game league lookup to use an AFL fallback
instead of "eng.1", while preserving correct display behavior for missing league
data.
In `@plugins/afl-scoreboard/sports.py`:
- Around line 1500-1506: Fix the repeated Logo Error text-loss bug in
SportsUpcoming._draw_scorebug_layout
(plugins/afl-scoreboard/sports.py#L1500-L1506),
SportsRecent._draw_scorebug_layout
(plugins/afl-scoreboard/sports.py#L2023-L2029), and
SportsLive._draw_scorebug_layout (plugins/afl-scoreboard/sports.py#L2679-L2685)
by converting main_img once into a local image, drawing onto that image, and
assigning the same image to display_manager.image at each site.
- Around line 2233-2242: Update the empty-state branch in SportsRecent.display
so display_manager.clear() and update_display() run only when force_clear is
true, matching SportsUpcoming.display. Preserve the existing state reset and
return behavior when games_list is empty or the panel is disabled.
---
Nitpick comments:
In `@plugins/afl-scoreboard/game_renderer.py`:
- Around line 505-510: Move the record font loading from
_draw_records_or_rankings into _load_fonts or __init__, store the result in the
existing cached self.fonts structure, and reuse that cached font when drawing
records or rankings instead of calling ImageFont.truetype per card render.
In `@plugins/afl-scoreboard/manager.py`:
- Around line 472-550: Extract the duplicated game normalization logic from
_display_scroll_mode and _ensure_scroll_content_for_vegas into a shared helper
such as _normalize_games_for_scroll(games, mode_type). Replace both inline
blocks with calls to that helper, preserving the league default, status
dictionary initialization, and mode-specific state mapping.
In `@plugins/afl-scoreboard/scroll_display.py`:
- Around line 333-339: The prepare_scroll_content flow recreates GameRenderer
and reloads its fonts on every call. Cache a single GameRenderer instance on the
owning display object, reuse it across calls, and update any rankings cache or
configuration that may change before rendering while preserving the existing
logo_cache, logger, dimensions, and config 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: 9d793550-1efc-4053-87a6-9bacf4623edd
📒 Files selected for processing (17)
plugins.jsonplugins/afl-scoreboard/.gitignoreplugins/afl-scoreboard/LICENSEplugins/afl-scoreboard/README.mdplugins/afl-scoreboard/afl_managers.pyplugins/afl-scoreboard/base_odds_manager.pyplugins/afl-scoreboard/config_schema.jsonplugins/afl-scoreboard/data_sources.pyplugins/afl-scoreboard/dynamic_team_resolver.pyplugins/afl-scoreboard/game_renderer.pyplugins/afl-scoreboard/manager.pyplugins/afl-scoreboard/manifest.jsonplugins/afl-scoreboard/requirements.txtplugins/afl-scoreboard/scroll_display.pyplugins/afl-scoreboard/sports.pyplugins/afl-scoreboard/test/harness.jsonplugins/afl-scoreboard/test_afl_plugin.py
| from typing import Any, Dict, Optional | ||
| import pytz | ||
|
|
||
| from sports import SportsCore, SportsLive, SportsRecent, SportsUpcoming |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Rename the generic sports module before bare-importing it.
Because the loader has no package context, Python can reuse another plugin’s globally cached sports module. Rename it to a domain-prefixed name such as afl_scoreboard_sports.py and update this import.
-from sports import SportsCore, SportsLive, SportsRecent, SportsUpcoming
+from afl_scoreboard_sports import SportsCore, SportsLive, SportsRecent, SportsUpcomingAs per coding guidelines: “Use plugin-unique names (prefixed with plugin domain) … to avoid cross-plugin collisions.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from sports import SportsCore, SportsLive, SportsRecent, SportsUpcoming | |
| from afl_scoreboard_sports import SportsCore, SportsLive, SportsRecent, SportsUpcoming |
🤖 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/afl-scoreboard/afl_managers.py` at line 15, Rename the plugin’s
generic sports module to a domain-prefixed filename such as
afl_scoreboard_sports.py, then update the import in afl_managers.py to reference
the renamed module while preserving the existing SportsCore, SportsLive,
SportsRecent, and SportsUpcoming imports.
Source: Coding guidelines
- afl_managers.py: fix halftime clock-append bug -- the clock-append
condition keyed only on status_state == "in", which ESPN also reports
during halftime (state="in" + name="STATUS_HALFTIME"), corrupting
"HALF" into "HALF <clock>". Added a regression test.
- base_odds_manager.py: get_odds() logged the cached no-odds marker but
fell through to hit ESPN again on every call instead of returning,
defeating the point of caching it.
- config_schema.json: the entire "customization" section (fonts/layout
offsets) was a top-level sibling of "properties" instead of nested
inside it, and missing from x-propertyOrder -- making it invisible to
the schema-driven settings UI even though game_renderer.py/manager.py
already read it. Also fixed a leftover "ENG" description on
exclude_teams.
- data_sources.py: removed AFLAPIDataSource (football-data.org stub) and
MLBAPIDataSource (hardcoded MLB AL-only standings) -- both confirmed
unused anywhere in this plugin. Widened fetch_live_games's date query
to a 3-day window around the host's local date instead of just today,
since AFL games in Australian time zones can fall on a different
ESPN-side date than the host's "today" near a boundary.
- game_renderer.py: fixed a typo default font filename (4x6.ttf ->
4x6-font.ttf) that silently degraded the detail-text font to PIL's
tiny built-in font. Added the missing download-on-demand fallback to
_load_and_resize_logo (matching the mechanism sports.py already uses)
instead of giving up when the local logo file doesn't exist. Cached
the per-render record font instead of reloading it from disk every
card.
- manager.py: last_mode_switch was seeded at 0, so the very first
internal-cycling display() call saw a huge elapsed time and
immediately skipped mode 0 -- seeded with time.time() instead.
Moved on_config_change's in-flight thread draining outside
_config_lock (sequential thread.join(timeout=10.0) calls were
blocking display()/update() for up to 10s per thread). Gave
supports_dynamic_duration/get_dynamic_duration_cap an optional
mode_type param so get_cycle_duration's explicit display-mode queries
use the right mode's limits instead of whatever mode is currently
showing. Extracted the duplicated game-normalization logic from
_display_scroll_mode/_ensure_scroll_content_for_vegas into
_normalize_games_for_scroll.
- scroll_display.py: replaced the leftover soccer LEAGUE_NAMES/separator-
icon tables with AFL-appropriate ones (AFL is single-league, so this
never actually renders more than one separator in practice), and fixed
the "eng.1" fallback default to "afl". Cached the GameRenderer instance
instead of rebuilding it (and reloading fonts from disk) on every
prepare_scroll_content() call.
- sports.py: fixed the repeated "Logo Error" text-loss bug in all 3
_draw_scorebug_layout sites (SportsUpcoming/SportsRecent/SportsLive) --
each called main_img.convert("RGB") twice, drawing the error text onto
one throwaway conversion and then displaying a second, undrawn one.
Fixed SportsRecent.display's empty-state branch to only clear/update
the display on force_clear, matching SportsUpcoming -- it was
previously clearing on every single call while the games list was
empty.
Skipped, with reason:
- Renaming sports.py to a domain-prefixed filename: confirmed via
plugin_loader.py's own docstring that bare-name module collision
(sports.py, scroll_display.py, game_renderer.py) is a deliberate,
already-working framework mechanism (namespaced sys.modules
isolation) used by 4 sibling scoreboard plugins that all keep
sports.py as-is. Renaming AFL's would make it the inconsistent
outlier.
- Fixing MLBAPIDataSource.fetch_standings's hardcoded leagueId=103:
confirmed this method is unreachable dead code (MLBAPIDataSource is
never imported/instantiated anywhere in the plugin) -- removed the
whole class instead, alongside the already-flagged AFLAPIDataSource.
Validated: py_compile on every file, config_schema.json JSON-parses and
lands "customization" correctly under properties/x-propertyOrder, and
the plugin's existing test_afl_plugin.py smoke suite passes -- including
a new assertion that fails without the halftime-clock fix and passes
with it (verified via git stash).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
# Conflicts: # plugins.json
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/afl-scoreboard/data_sources.py (1)
78-79: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPrevent
IndexErrorwhen an event has empty competitions.If an event in the ESPN API response explicitly contains an empty
'competitions': []list,event.get('competitions', [{}])will return that empty list, causing the[0]index access to raise anIndexError. This will crash the comprehension and abort the entire live-games fetch.Verify that the list is non-empty before accessing its first element.
🛡️ Proposed fix
- live_events = [event for event in events - if event.get('competitions', [{}])[0].get('status', {}).get('type', {}).get('state') == 'in'] + live_events = [ + event for event in events + if event.get('competitions') and event['competitions'][0].get('status', {}).get('type', {}).get('state') == 'in' + ]🤖 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/afl-scoreboard/data_sources.py` around lines 78 - 79, Update the live_events comprehension to verify each event’s competitions list is non-empty before accessing its first element, while preserving the existing state == 'in' filter for valid entries.
🧹 Nitpick comments (1)
plugins/afl-scoreboard/manager.py (1)
482-494: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueIn-place mutation of manager-owned game dicts couples scroll normalization to manager internals.
_normalize_games_for_scrollmutates the dicts returned by_get_games_from_manager, which are the same dict objects held in the manager'slive_games/games_list. Once scroll mode runs, the manager's own cached games permanently gain a syntheticstatus.state(andleague) they may not have had before, which the manager's own switch-mode code may not expect. Consider normalizing into shallow copies instead of mutating in place.♻️ Suggested change (copy instead of mutate)
`@staticmethod` def _normalize_games_for_scroll(games: List[Dict], mode_type: str) -> List[Dict]: - """Ensure each game has a league and a status dict with a state, - in place, so the scroll manager gets a consistent shape regardless - of which mode's manager produced it.""" + """Return copies of games with a league and a status dict with a + state, so the scroll manager gets a consistent shape regardless of + which mode's manager produced it, without mutating manager state.""" state_map = {"live": "in", "recent": "post", "upcoming": "pre"} - for game in games: - game.setdefault("league", AFL_LEAGUE_KEY) - if not isinstance(game.get("status"), dict): - game["status"] = {} - if "state" not in game["status"]: - game["status"]["state"] = state_map.get(mode_type, "pre") - return games + normalized = [] + for game in games: + g = dict(game) + g.setdefault("league", AFL_LEAGUE_KEY) + status = dict(g.get("status")) if isinstance(g.get("status"), dict) else {} + status.setdefault("state", state_map.get(mode_type, "pre")) + g["status"] = status + normalized.append(g) + return normalizedSince this depends on how
sports.py/afl_managers.pyconsumegame["status"]elsewhere, please confirm the mappedstatevalues don't collide with logic in those files.🤖 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/afl-scoreboard/manager.py` around lines 482 - 494, Update _normalize_games_for_scroll to build and normalize shallow copies of each game rather than mutating the dictionaries returned by _get_games_from_manager, while preserving the existing league and status/state defaults. Return the copied game list so manager-owned live_games/games_list entries remain unchanged, and verify the mapped state values remain compatible with consumers in sports.py and afl_managers.py.
🤖 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/afl-scoreboard/manager.py`:
- Around line 398-411: Move clearing of _active_update_threads out of the first
_config_lock block and perform it in the second lock block after all threads in
threads_to_drain have finished joining. Keep the entries visible during the join
window so update()’s is_alive() check prevents duplicate manager.update()
threads, while preserving the existing lock-free join behavior.
---
Outside diff comments:
In `@plugins/afl-scoreboard/data_sources.py`:
- Around line 78-79: Update the live_events comprehension to verify each event’s
competitions list is non-empty before accessing its first element, while
preserving the existing state == 'in' filter for valid entries.
---
Nitpick comments:
In `@plugins/afl-scoreboard/manager.py`:
- Around line 482-494: Update _normalize_games_for_scroll to build and normalize
shallow copies of each game rather than mutating the dictionaries returned by
_get_games_from_manager, while preserving the existing league and status/state
defaults. Return the copied game list so manager-owned live_games/games_list
entries remain unchanged, and verify the mapped state values remain compatible
with consumers in sports.py and afl_managers.py.
🪄 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: e993127e-d1e1-4a97-99ec-ab66c0882bec
📒 Files selected for processing (9)
plugins/afl-scoreboard/afl_managers.pyplugins/afl-scoreboard/base_odds_manager.pyplugins/afl-scoreboard/config_schema.jsonplugins/afl-scoreboard/data_sources.pyplugins/afl-scoreboard/game_renderer.pyplugins/afl-scoreboard/manager.pyplugins/afl-scoreboard/scroll_display.pyplugins/afl-scoreboard/sports.pyplugins/afl-scoreboard/test_afl_plugin.py
🚧 Files skipped from review as they are similar to previous changes (7)
- plugins/afl-scoreboard/test_afl_plugin.py
- plugins/afl-scoreboard/config_schema.json
- plugins/afl-scoreboard/scroll_display.py
- plugins/afl-scoreboard/base_odds_manager.py
- plugins/afl-scoreboard/game_renderer.py
- plugins/afl-scoreboard/afl_managers.py
- plugins/afl-scoreboard/sports.py
…ning completes on_config_change() cleared _active_update_threads immediately after snapshotting it (before the join loop, deliberately outside the lock to avoid blocking display()/update()). That left a window where a concurrent update() call would see an empty dict and could start a duplicate thread for a manager whose old update was still actually running in the background, mid-drain. Moved the .clear() into the second lock block, after every drained thread has been joined (or timed out) -- entries stay visible (and correctly report is_alive()) throughout the join window, closing the duplicate- thread race while keeping the lock-free join behavior unchanged. Verified with a standalone reproduction of the locking pattern: mid-drain, the entry is present and is_alive() is True (would correctly block a concurrent update() from starting a duplicate); after the drain completes, the dict is empty. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
…vered test/harness.json declared mock_data as an inline dict of fixture content, but the core harness loader's documented (and only-supported) contract is a string path to a separate fixture file -- every other plugin (e.g. ledmatrix-weather) follows this. load_harness_spec() did `Path(plugin_dir) / mock_rel`, which crashed with "TypeError: unsupported operand type(s) for /: 'PosixPath' and 'dict'" the moment mock_rel turned out to be a dict instead of a string. Fixed by extracting the fixture content into test/fixtures/mock.json and pointing harness.json's mock_data at it as a path string, matching the established convention. Also added freeze_time (harness.json had none), since afl_managers.py's cache key is date-range-based (afl_schedule_<start>-<end>, a rolling 28-day window around "now") -- without freezing time the key is non-deterministic and could never match a static fixture. Computed the exact key for the chosen freeze_time and verified it with freezegun directly. Validating the crash fix against a real check_plugin.py run (core repo + this plugin, matching the actual CI layout) surfaced a second, real bug the crash had been masking: sports.py's _extract_game_details_common read home_team["id"]/away_team["id"], but a competitor object's team ID lives one level deeper at home_team["team"]["id"] (the adjacent home_abbr extraction already correctly reads home_team["team"]["abbreviation"]). This raised KeyError: 'id' on every game, silently caught and logged by the caller, meaning every game would have been silently dropped -- with real ESPN data, not just the mock fixture. Fixed both accesses to go through the nested "team" key. Validated: full check_plugin.py run (replicating the actual CI directory layout: core repo + plugins-repo checkout) goes from crashing immediately to PASS across all 23 size/mode combinations with zero extraction errors; existing test_afl_plugin.py suite still passes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/afl-scoreboard/manager.py (1)
413-415: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename unused loop variable to
_name.The variable
nameis not used within the loop body. Consider prefixing it with an underscore to explicitly mark it as unused and resolve the linter warning.♻️ Proposed refactor
- for name, thread in threads_to_drain: + for _name, thread in threads_to_drain: if thread.is_alive(): thread.join(timeout=10.0)🤖 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/afl-scoreboard/manager.py` around lines 413 - 415, Rename the unused first loop variable in the threads_to_drain iteration to _name, while leaving the thread.is_alive and thread.join behavior unchanged.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.
Nitpick comments:
In `@plugins/afl-scoreboard/manager.py`:
- Around line 413-415: Rename the unused first loop variable in the
threads_to_drain iteration to _name, while leaving the thread.is_alive and
thread.join behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a8b2bec6-b8f1-4102-8c8f-b195623dfc4e
📒 Files selected for processing (5)
plugins.jsonplugins/afl-scoreboard/manager.pyplugins/afl-scoreboard/sports.pyplugins/afl-scoreboard/test/fixtures/mock.jsonplugins/afl-scoreboard/test/harness.json
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins.json
- plugins/afl-scoreboard/sports.py
Summary
Adds a new AFL (Australian Football League) scoreboard plugin, forked from the sport-agnostic
soccer-scoreboard. AFL is a single league, so this is a flattened, simpler version of the multi-league template — noleagues/custom_leaguesnesting, no national-flag handling.It shows live, recent, and upcoming AFL games with real-time scores and game status, mapping AFL's four quarters (Q1–Q4) + HALF + Final + pre-game start time onto the shared
period_text/clockmodel.Data source
ESPN public scoreboard (no auth):
https://site.api.espn.com/apis/site/v2/sports/australian-football/afl/scoreboard→ HTTP 200, real 2026-season data.sport = australian-football,league = afl. Score is a single running integer per team; quarter comes fromstatus.period(1–4), clock fromstatus.displayClock.Parity with soccer-scoreboard
Full feature/customization parity, collapsed to one league:
afl_live,afl_recent,afl_upcomingmode_durationsget_vegas_content_type→multi)customizationblock (fonts/sizes) lifted verbatim from soccer's schemaSport-generic modules (
sports.py,game_renderer.py,data_sources.py,base_odds_manager.py,dynamic_team_resolver.py,scroll_display.py) are copied with only soccer wording / logo-dir constants updated for AFL.Files
plugins/afl-scoreboard/manager.py—AflScoreboardPlugin(single-league orchestrator)plugins/afl-scoreboard/afl_managers.py— ESPN fetch + AFL parsing,create_afl_managersplugins/afl-scoreboard/config_schema.json— flat single-league draft-07 schemaplugins/afl-scoreboard/manifest.json,README.md,requirements.txtplugins/afl-scoreboard/test/harness.json+test_afl_plugin.pyplugins.json— registry entry (plugin_path: plugins/afl-scoreboard)Test plan
python3 -m py_compilepasses on every.pymanifest.json,config_schema.json,test/harness.json,plugins.jsonall parsepython3 test_afl_plugin.pypasses (display modes, mode routing, empty-manager skip, live content, AFL quarterperiod_textmapping)status/competitorsfieldsgrep -ri soccer plugins/afl-scoreboard/clean except two origin-provenance commentsscripts/check_plugin.pyagainsttest/harness.jsonacross matrix sizes (needs a LEDMatrix core checkout)🤖 Generated with Claude Code
Summary by CodeRabbit