fix(hockey): draw date and time on upcoming scroll/Vegas cards - #261
Open
ChuckBuilds wants to merge 9 commits into
Open
fix(hockey): draw date and time on upcoming scroll/Vegas cards#261ChuckBuilds wants to merge 9 commits into
ChuckBuilds wants to merge 9 commits into
Conversation
Upcoming hockey games scrolled past as two logos and a bare "VS" — no date, no time — which also left the two logos looking pressed together with nothing between them. The cause was a key mismatch, not missing data. ESPN supplies the start time (shortDetail is "9/19 - 7:00 PM EDT"), but the scroll/Vegas path feeds cards straight from the sports extractor, which emits flat game_date/game_time plus a start_time_utc datetime. The card instead read status.short_detail with a fallback to start_time — neither of which that payload contains — so both branches found empty strings and drew nothing. The peer sports (football, basketball) read the flat keys and render correctly, which is why hockey alone looked wrong. _upcoming_date_and_time() now resolves all three payload shapes: the flat extractor keys, the nested shape built by data_fetcher.py, and a raw start time parsed as a last resort. Time renders top-center and date bottom-center, matching the other sports' cards. The combined nested string is split and trimmed of its timezone suffix so it fits a card. Logo geometry is unchanged: football uses the identical logo_slot formula, so the crowding was a symptom of the blank card, not a separate layout bug. Verified against real ESPN data and the safety harness — 16/16 PASS across every supported panel size, no text overflow in any of them.
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 20 high |
🟢 Metrics 730 complexity
Metric Results Complexity 730
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.
Contributor
📝 WalkthroughWalkthroughThe scoreboard plugins add configurable upcoming-card content, timezone-aware date handling, center-gap logo sizing, and embedded scroll spacing. Plugin versions and release metadata are updated for the 2026-08-06 releases. ChangesScoreboard scroll-card update
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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: 2
🤖 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 3: Update the hockey-scoreboard 1.8.1 release date in the registry source
to 2026-08-06, then run update_registry.py to regenerate plugins.json; do not
modify plugins.json manually.
In `@plugins/hockey-scoreboard/game_renderer.py`:
- Around line 688-695: Update the date rendering flow in the surrounding
game-rendering method and coordinate it with _draw_records_or_rankings so
date_text never occupies the same bottom region when show_records or
show_ranking is enabled. Reserve non-overlapping vertical space or suppress the
secondary content for upcoming cards when no lane is available, while preserving
correct scaling and bounds across every supported panel size.
🪄 Autofix
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: 97de0b71-2191-4d1e-b36f-3a25b271b39a
📒 Files selected for processing (3)
plugins.jsonplugins/hockey-scoreboard/game_renderer.pyplugins/hockey-scoreboard/manifest.json
Following the dead expression at hockey.py:79. The expression itself was harmless — _extract_game_details_common already stores that value as details["status_text"] (sports.py:908), so it was a refactor leftover, sibling to the commented-out line below it. Removed. Looking at why it was there surfaced a real bug on the same path. Live scroll/Vegas cards render "P2" instead of "P2 12:34": the clock is silently dropped. _normalize_game_payload writes the flat clock into status["clock"], but _draw_live_game_status reads status["display_clock"] — the canonical key that data_fetcher.py builds. Nothing reads status["clock"] at all, so the value was written and never used. The full-screen scorebug is unaffected because it reads the flat game["clock"] directly (hockey.py:223), which is why this only shows up in scroll/Vegas. The normalizer now emits the canonical nested shape: display_clock alongside clock, short_detail from status_text, and a state derived from the extractor's is_live/is_final/is_upcoming flags when absent. That last one makes render_game_card correct standalone — _collect_games_for_scroll injects state from the mode today, so a caller that skips it would otherwise land in the wrong branch and draw nothing. Verified live "P2 12:34", recent "Final", and upcoming date/time all render, including with no injected state. Safety harness 16/16 PASS; the widest live string measures 64px, fitting the narrowest panel exactly.
Two layout complaints from the Vegas strip, both confirmed in renders. Score drawn over the logos. Logos were thumbnailed into a display_height box while logo_slot was also min(display_height, width // 2), so on a 128x64 card two 64px logos exactly filled it and met in the middle — precisely where the score and "VS" are centred. The card now reserves a centre gap before sizing the logos, and the slot is still capped at display_height, so the sizes that already had a wide middle (128x32, 256x32) come out byte-identical; only 128x64-style cards shrink. On a 128px card the gap is 36px against a 30px "1-2". Override with customization.center_gap; 0 restores the old edge-to-edge look. The logo cache key now includes the box, since one cache is shared by renderers built for different card sizes. Cards too close together. gap_between_games never reached Vegas, which stitches _vegas_content_items itself and never sees the scroll helper's item_gap — so Vegas always ran at the fixed 12px-per-side padding regardless of the setting. That padding also carried a stale comment about logos at -10/display_width+10, a layout this renderer no longer uses. Spacing is now baked into each card as half the gap per side and item_gap drops to 0, so both paths separate cards by exactly gap_between_games. The code default moves 24 -> 48, matching what config_schema.json has advertised all along. Safety harness 16/16 PASS. Centre text clears the logos at every supported size except "1-2" on 64x32, where a 64px-wide card cannot hold two logos and a 30px score at once; "VS" still fits there.
…eboards Applies the hockey card layout to every sports plugin, and makes the pieces configurable. Scoped deliberately to game_renderer.py and scroll_display*.py, which are reached only from scroll and Vegas modes -- the full-screen scoreboard is a separate code path and is untouched. No 0-0 before a game starts. basketball, football, afl, nrl and soccer drew the score unconditionally, so an upcoming game rendered the extractor's placeholder 0-0 between the logos. The score is now gated to live/recent, and upcoming cards get "VS" instead. hockey and lacrosse already gated it. The score no longer sits on the logos. Logos were sized into a display_height box while the slot was min(display_height, width // 2), so on a 128x64 card two 64px logos filled it and met exactly where the score is centred. A centre gap is reserved before the logos are sized, and the slot stays capped at display_height so the wide/short cards (128x32, 256x32) are unchanged. Logo cache keys now carry the slot size, since one cache is shared by renderers built for different card widths. Dates read "Sep 19". The position is measured from the text rather than a fixed display_height - 7, which was only ever right for a 6px font -- soccer and nrl use a 10px detail font, where "Sep 19" ran 5px past the bottom of the card. The descender that exposed this does not exist in the old "9/19". gap_between_games now applies in Vegas. Vegas stitches its own content items and never sees the scroll helper's item_gap, so it always ran at the fixed 12px-per-side padding whatever the setting said. Spacing is baked into each card as half the gap per side with item_gap dropped to 0, so both paths separate cards by exactly gap_between_games. The code default moves 24 -> 48, which is what config_schema.json has advertised all along. New scroll_card config block in all eight schemas: upcoming_center (vs | date_time), date_format (abbrev | numeric) and center_gap in pixels (0 restores edge-to-edge logos). Safety harness: 168/168 PASS across all eight plugins and every supported panel size. Verified upcoming renders for all eight show VS, the time and "Sep 19" with no 0-0, and the date seated at 63/64px with no clipping.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
plugins/afl-scoreboard/scroll_display.py (1)
83-83: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStale
gap_between_gamesclass default keeps the gap at 24. Both plugins raised the inlinescroll_settings.get("gap_between_games", 48)fallback but kept"gap_between_games": 24inscroll_settings_defaults._get_scroll_settingsmerges those defaults, so the key is always present and the inline fallback is unreachable. Combined with the newitem_gap=0, total card separation drops to 24 pixels instead of rising to 48.
plugins/afl-scoreboard/scroll_display.py#L83-L83: change the default to 48, or remove the override so the core default applies.plugins/nrl-scoreboard/scroll_display.py#L111-L111: apply the identical change.🤖 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` at line 83, The scroll_settings_defaults override must allow the intended 48-pixel gap. In plugins/afl-scoreboard/scroll_display.py:83 and plugins/nrl-scoreboard/scroll_display.py:111, change gap_between_games from 24 to 48 or remove the override so the core default applies; keep the existing _get_scroll_settings merging behavior unchanged.plugins/baseball-scoreboard/game_renderer.py (1)
169-173: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winScope the logo cache key to the logo slot size.
Line 171 keys the cache by
leagueandteam_abbrevonly. Line 198 now sizes the thumbnail from_logo_slot_width(), which depends ondisplay_width.plugins/baseball-scoreboard/scroll_display.pyshares one_logo_cachedict across renderers and rebuilds the renderer whengame_card_widthchanges, so a logo sized for a wide slot is served to a narrow card.The AFL and soccer renderers added a
_logo_cache_keyhelper for this reason. Add the same scoping here.🐛 Proposed fix
- cache_key = f"{league}_{team_abbrev}" + cache_key = f"{league}_{team_abbrev}@{self._logo_slot_width()}x{self.display_height}"Note that the
milbbranch sizes fromdisplay_width // 3, so include that in the key or reuse a shared helper that returns the effective box.Also applies to: 198-200
🤖 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/baseball-scoreboard/game_renderer.py` around lines 169 - 173, Update _load_and_resize_logo and the logo cache key logic so entries are scoped by the effective logo slot size as well as league and team_abbrev. Reuse or add a _logo_cache_key helper consistent with the AFL and soccer renderers, ensuring the milb display_width // 3 sizing is included, and use that key for all cache reads and writes.plugins/hockey-scoreboard/game_renderer.py (1)
663-672: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe day stays zero-padded in the raw-timestamp fallback.
strftime("%m/%d").lstrip("0")removes only the leading zero of the month. A date such as 5 September renders as9/05. The flat extractor keys use9/5, andnumericdate format shows this string unchanged, so the two sources disagree. The time string has the same issue for the minutes-free case only in the hour, whichlstrip("0")already handles.🔧 Proposed fix
- return local_dt.strftime("%m/%d").lstrip("0"), local_dt.strftime("%I:%M%p").lstrip("0") + return ( + f"{local_dt.month}/{local_dt.day}", + local_dt.strftime("%I:%M%p").lstrip("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/hockey-scoreboard/game_renderer.py` around lines 663 - 672, Update the raw-timestamp formatting in the datetime conversion block to remove zero-padding from both month and day, so dates such as September 5 render as 9/5 and match flat extractor keys; keep the existing timezone conversion, time formatting, and error handling unchanged.plugins/lacrosse-scoreboard/game_renderer.py (1)
528-547: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
_draw_upcoming_centeris never called, sodate_timemode renders an empty card center.
render_game_carddraws the score only forliveandrecent. It has noupcomingbranch, unlikeplugins/basketball-scoreboard/game_renderer.pyLine 503,plugins/football-scoreboard/game_renderer.pyLine 587, andplugins/hockey-scoreboard/game_renderer.pyLine 563._draw_upcoming_game_statusat Line 736 returns immediately when the mode is notvs. When a user setsscroll_card.upcoming_centertodate_time, the lacrosse upcoming card shows the two logos and nothing else. Invsmode the card also loses the central "VS" that the other plugins draw.🐛 Proposed fix
self._draw_text_with_outline( draw_overlay, score_text, (score_x, score_y), self.fonts['score'], fill=self._score_color_for(game, game_type) ) + elif game_type == "upcoming": + self._draw_upcoming_center(draw_overlay, game) # Draw period/status based on game type🤖 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/game_renderer.py` around lines 528 - 547, Update render_game_card’s center-content branch to handle upcoming games by calling _draw_upcoming_center with the overlay and game data, matching the corresponding basketball, football, and hockey renderers. Preserve the existing live/recent score rendering and ensure upcoming cards display either the configured date/time content or the central VS instead of an empty center.plugins/baseball-scoreboard/scroll_display.py (1)
246-273: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winBaseball cards lose all separation on the default path. Both baseball scroll paths default
game_card_widthtoself.display_width, so the width condition suppresses the card padding, whileitem_gapis now 0. Nothing supplies the gap, andgap_between_gameshas no effect.
plugins/baseball-scoreboard/scroll_display.py#L246-L273: pad every card withmax(4, gap_between_games // 2), or passgap_between_gamesasitem_gapwhen the card is unpadded.plugins/baseball-scoreboard/scroll_display_legacy.py#L400-L427: apply the same change so the legacy path matches.🤖 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/baseball-scoreboard/scroll_display.py` around lines 246 - 273, Baseball cards lack separation because full-width cards suppress padding while ScrollHelper receives item_gap=0. In plugins/baseball-scoreboard/scroll_display.py lines 246-273, ensure each card contributes max(4, gap_between_games // 2) padding or pass gap_between_games as item_gap when unpadded; apply the same correction in plugins/baseball-scoreboard/scroll_display_legacy.py lines 400-427 so both scroll paths honor gap_between_games.plugins/nrl-scoreboard/game_renderer.py (1)
161-174: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winUse the dimension-scoped key for cache membership checks.
Lines 161 and 191 store logos with
_logo_cache_key(). The checks at Lines 151 and 173 still test the bare abbreviation. Each preload and render misses the cache and reloads the image from disk.Create one
cache_keyvalue and use it for both membership tests and dictionary access.Also applies to: 189-191
🤖 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/game_renderer.py` around lines 161 - 174, Use a single cache_key derived from _logo_cache_key() in _load_and_resize_logo and the preload/render lookup paths; apply it consistently for cache membership checks and dictionary access instead of testing the bare team abbreviation, while preserving the existing dimension-scoped storage behavior.
🧹 Nitpick comments (3)
plugins/lacrosse-scoreboard/scroll_display.py (1)
256-261: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the stale layout comment.
Lines 256-257 state that logos sit at
-10anddisplay_width+10. The renderer no longer uses that layout, and the padding now exists to create the inter-card gap.plugins/hockey-scoreboard/scroll_display.pyalready documents this correctly. The same stale text remains in the AFL, NRL, and basketball copies.🤖 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/scroll_display.py` around lines 256 - 261, Remove the stale logo-positioning comments above the padding calculation in the Lacrosse scroll display and the corresponding AFL, NRL, and basketball copies. Keep the comments describing padding’s purpose and the calculation of padding = max(4, gap_between_games // 2) unchanged.plugins/baseball-scoreboard/game_renderer.py (1)
732-737: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid formatting the date twice.
Line 732 applies
_format_game_date. Line 736 passes the result into_draw_upcoming_center, which applies_format_game_dateagain. The second pass is currently a no-op because "Sep 19" does not split into two numeric parts. The behavior depends on that coincidence.Pass the raw numeric date into
_draw_upcoming_centerand format only for the top/bottom placement.🤖 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/baseball-scoreboard/game_renderer.py` around lines 732 - 737, Update the game rendering flow around _format_game_date and _draw_upcoming_center so the raw numeric game_date is passed into _draw_upcoming_center, avoiding the current pre-formatting in the caller. Keep date formatting limited to the top/bottom placement logic.plugins/afl-scoreboard/config_schema.json (1)
7-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
scroll_cardto the top-levelx-propertyOrderarray.
plugins/afl-scoreboard/config_schema.jsondefinesscroll_card, but the file-levelx-propertyOrderarray does not include it. Addscroll_cardto align this new settings group with the schema order convention.🤖 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/config_schema.json` around lines 7 - 41, Add scroll_card to the top-level x-propertyOrder array in config_schema.json, preserving the existing ordering of the other settings groups and placing it consistently with the schema’s scroll-card configuration.
🤖 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/game_renderer.py`:
- Around line 187-188: The logo cache guards use the bare abbreviation while
reads use the transformed key. In both renderer files, update the relevant
cache-hit and cache-miss guards to compute one cache key via
_logo_cache_key(abbr/team_abbrev), then use that key consistently for membership
checks and reads; apply this to plugins/afl-scoreboard/game_renderer.py lines
165 and 187-188 and plugins/soccer-scoreboard/game_renderer.py lines 151 and
173-174.
In `@plugins/afl-scoreboard/scroll_display_legacy.py`:
- Line 302: Update the default gap values to 48 pixels in the scroll settings:
change the scroll_mode fallback in
plugins/afl-scoreboard/scroll_display_legacy.py at lines 129 and 302, the
scroll_mode fallback in plugins/nrl-scoreboard/scroll_display_legacy.py at lines
147 and 345, and the scroll_settings_defaults() default in
plugins/soccer-scoreboard/scroll_display.py at lines 110 and 263. Preserve the
existing settings lookup behavior while ensuring all default paths use 48.
In `@plugins/basketball-scoreboard/game_renderer.py`:
- Around line 217-220: Align logo-cache key construction, lookup, preload, and
storage across the three renderers. In
plugins/basketball-scoreboard/game_renderer.py lines 217-220, 190-196, 240, and
259, build one _logo_cache_key-scoped league/abbreviation key, use it
consistently with .get(), and store under that key; in
plugins/football-scoreboard/game_renderer.py lines 319-321, 298, and 338, scope
the abbreviation key consistently for lookup, preload, and storage; in
plugins/lacrosse-scoreboard/game_renderer.py lines 194-201 and 225, scope both
league-prefixed and backward-compatible bare lookups and store using the same
scoped key.
In `@plugins/soccer-scoreboard/scroll_display_legacy.py`:
- Line 345: Update the gap_between_games default in _get_scroll_settings from 24
to 48 so soccer uses the intended spacing when the setting is absent; keep the
existing lookup in the display flow unchanged.
---
Outside diff comments:
In `@plugins/afl-scoreboard/scroll_display.py`:
- Line 83: The scroll_settings_defaults override must allow the intended
48-pixel gap. In plugins/afl-scoreboard/scroll_display.py:83 and
plugins/nrl-scoreboard/scroll_display.py:111, change gap_between_games from 24
to 48 or remove the override so the core default applies; keep the existing
_get_scroll_settings merging behavior unchanged.
In `@plugins/baseball-scoreboard/game_renderer.py`:
- Around line 169-173: Update _load_and_resize_logo and the logo cache key logic
so entries are scoped by the effective logo slot size as well as league and
team_abbrev. Reuse or add a _logo_cache_key helper consistent with the AFL and
soccer renderers, ensuring the milb display_width // 3 sizing is included, and
use that key for all cache reads and writes.
In `@plugins/baseball-scoreboard/scroll_display.py`:
- Around line 246-273: Baseball cards lack separation because full-width cards
suppress padding while ScrollHelper receives item_gap=0. In
plugins/baseball-scoreboard/scroll_display.py lines 246-273, ensure each card
contributes max(4, gap_between_games // 2) padding or pass gap_between_games as
item_gap when unpadded; apply the same correction in
plugins/baseball-scoreboard/scroll_display_legacy.py lines 400-427 so both
scroll paths honor gap_between_games.
In `@plugins/hockey-scoreboard/game_renderer.py`:
- Around line 663-672: Update the raw-timestamp formatting in the datetime
conversion block to remove zero-padding from both month and day, so dates such
as September 5 render as 9/5 and match flat extractor keys; keep the existing
timezone conversion, time formatting, and error handling unchanged.
In `@plugins/lacrosse-scoreboard/game_renderer.py`:
- Around line 528-547: Update render_game_card’s center-content branch to handle
upcoming games by calling _draw_upcoming_center with the overlay and game data,
matching the corresponding basketball, football, and hockey renderers. Preserve
the existing live/recent score rendering and ensure upcoming cards display
either the configured date/time content or the central VS instead of an empty
center.
In `@plugins/nrl-scoreboard/game_renderer.py`:
- Around line 161-174: Use a single cache_key derived from _logo_cache_key() in
_load_and_resize_logo and the preload/render lookup paths; apply it consistently
for cache membership checks and dictionary access instead of testing the bare
team abbreviation, while preserving the existing dimension-scoped storage
behavior.
---
Nitpick comments:
In `@plugins/afl-scoreboard/config_schema.json`:
- Around line 7-41: Add scroll_card to the top-level x-propertyOrder array in
config_schema.json, preserving the existing ordering of the other settings
groups and placing it consistently with the schema’s scroll-card configuration.
In `@plugins/baseball-scoreboard/game_renderer.py`:
- Around line 732-737: Update the game rendering flow around _format_game_date
and _draw_upcoming_center so the raw numeric game_date is passed into
_draw_upcoming_center, avoiding the current pre-formatting in the caller. Keep
date formatting limited to the top/bottom placement logic.
In `@plugins/lacrosse-scoreboard/scroll_display.py`:
- Around line 256-261: Remove the stale logo-positioning comments above the
padding calculation in the Lacrosse scroll display and the corresponding AFL,
NRL, and basketball copies. Keep the comments describing padding’s purpose and
the calculation of padding = max(4, gap_between_games // 2) unchanged.
🪄 Autofix
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: 9629a992-c18f-4d6a-acdf-69bfddea2c79
📒 Files selected for processing (42)
plugins.jsonplugins/afl-scoreboard/config_schema.jsonplugins/afl-scoreboard/game_renderer.pyplugins/afl-scoreboard/manifest.jsonplugins/afl-scoreboard/scroll_display.pyplugins/afl-scoreboard/scroll_display_legacy.pyplugins/baseball-scoreboard/config_schema.jsonplugins/baseball-scoreboard/game_renderer.pyplugins/baseball-scoreboard/manifest.jsonplugins/baseball-scoreboard/scroll_display.pyplugins/baseball-scoreboard/scroll_display_legacy.pyplugins/basketball-scoreboard/config_schema.jsonplugins/basketball-scoreboard/game_renderer.pyplugins/basketball-scoreboard/manifest.jsonplugins/basketball-scoreboard/scroll_display.pyplugins/basketball-scoreboard/scroll_display_legacy.pyplugins/football-scoreboard/config_schema.jsonplugins/football-scoreboard/game_renderer.pyplugins/football-scoreboard/manifest.jsonplugins/football-scoreboard/scroll_display.pyplugins/football-scoreboard/scroll_display_legacy.pyplugins/hockey-scoreboard/config_schema.jsonplugins/hockey-scoreboard/game_renderer.pyplugins/hockey-scoreboard/hockey.pyplugins/hockey-scoreboard/manifest.jsonplugins/hockey-scoreboard/scroll_display.pyplugins/hockey-scoreboard/scroll_display_legacy.pyplugins/lacrosse-scoreboard/config_schema.jsonplugins/lacrosse-scoreboard/game_renderer.pyplugins/lacrosse-scoreboard/manifest.jsonplugins/lacrosse-scoreboard/scroll_display.pyplugins/lacrosse-scoreboard/scroll_display_legacy.pyplugins/nrl-scoreboard/config_schema.jsonplugins/nrl-scoreboard/game_renderer.pyplugins/nrl-scoreboard/manifest.jsonplugins/nrl-scoreboard/scroll_display.pyplugins/nrl-scoreboard/scroll_display_legacy.pyplugins/soccer-scoreboard/config_schema.jsonplugins/soccer-scoreboard/game_renderer.pyplugins/soccer-scoreboard/manifest.jsonplugins/soccer-scoreboard/scroll_display.pyplugins/soccer-scoreboard/scroll_display_legacy.py
💤 Files with no reviewable changes (1)
- plugins/hockey-scoreboard/hockey.py
Comment on lines
187
to
+188
| if team_abbrev in self._logo_cache: | ||
| return self._logo_cache[team_abbrev] | ||
| return self._logo_cache[self._logo_cache_key(team_abbrev)] |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔴 Critical | ⚡ Quick win
Logo cache guard tests a different key than the read. Both renderers added _logo_cache_key for the store and the read, but left the membership guards testing the bare team abbreviation. No code path inserts a bare key, so the guard never matches, the cache never hits, and every card re-opens and re-thumbnails the logo from disk. A bare key written by any other caller would make the read raise KeyError.
plugins/afl-scoreboard/game_renderer.py#L187-L188: computecache_key = self._logo_cache_key(team_abbrev)once, then test and read that key; apply the same change to theabbr not in self._logo_cacheguard at line 165.plugins/soccer-scoreboard/game_renderer.py#L173-L174: apply the identical change, and to theabbr not in self._logo_cacheguard at line 151.
📍 Affects 2 files
plugins/afl-scoreboard/game_renderer.py#L187-L188(this comment)plugins/soccer-scoreboard/game_renderer.py#L173-L174
🤖 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 187 - 188, The logo
cache guards use the bare abbreviation while reads use the transformed key. In
both renderer files, update the relevant cache-hit and cache-miss guards to
compute one cache key via _logo_cache_key(abbr/team_abbrev), then use that key
consistently for membership checks and reads; apply this to
plugins/afl-scoreboard/game_renderer.py lines 165 and 187-188 and
plugins/soccer-scoreboard/game_renderer.py lines 151 and 173-174.
Comment on lines
217
to
+220
| # Use league+abbrev as cache key to avoid cross-league collisions | ||
| cache_key = f"{league}:{team_abbrev}" | ||
| if cache_key in self._logo_cache: | ||
| return self._logo_cache[cache_key] | ||
| return self._logo_cache[self._logo_cache_key(cache_key)] |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Logo cache lookups read a different key than the membership test. The new _logo_cache_key scoping was applied to the reads and writes but not to the in tests, so the tested key is never stored. Each hit branch is unreachable, defeating the cache, and a bare key present in the shared cache dict turns the read into a KeyError.
plugins/basketball-scoreboard/game_renderer.py#L217-L220: buildcache_key = self._logo_cache_key(f"{league}:{team_abbrev}")once, read it with.get(), and store the same key at Lines 240 and 259; alignpreload_logosat Lines 190-196.plugins/football-scoreboard/game_renderer.py#L319-L321: test and readself._logo_cache_key(team_abbrev); alignpreload_logosat Line 298 and the store at Line 338.plugins/lacrosse-scoreboard/game_renderer.py#L194-L201: scope both the league-prefixed and the backward-compatible bare lookups, and store the same scoped key at Line 225.
📍 Affects 3 files
plugins/basketball-scoreboard/game_renderer.py#L217-L220(this comment)plugins/football-scoreboard/game_renderer.py#L319-L321plugins/lacrosse-scoreboard/game_renderer.py#L194-L201
🤖 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/basketball-scoreboard/game_renderer.py` around lines 217 - 220, Align
logo-cache key construction, lookup, preload, and storage across the three
renderers. In plugins/basketball-scoreboard/game_renderer.py lines 217-220,
190-196, 240, and 259, build one _logo_cache_key-scoped league/abbreviation key,
use it consistently with .get(), and store under that key; in
plugins/football-scoreboard/game_renderer.py lines 319-321, 298, and 338, scope
the abbreviation key consistently for lookup, preload, and storage; in
plugins/lacrosse-scoreboard/game_renderer.py lines 194-201 and 225, scope both
league-prefixed and backward-compatible bare lookups and store using the same
scoped key.
| # Get scroll settings | ||
| scroll_settings = self._get_scroll_settings() | ||
| gap_between_games = scroll_settings.get("gap_between_games", 24) | ||
| gap_between_games = scroll_settings.get("gap_between_games", 48) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The new 48 default never applies in this plugin.
_get_scroll_settings always returns a gap_between_games entry, defaulting to 24 (Line 147). The 48 fallback here is therefore unreachable, and soccer keeps a 24-pixel gap while the other scoreboard plugins move to 48. Update the default inside _get_scroll_settings if soccer should match.
🔧 Proposed fix outside the selected range
# plugins/soccer-scoreboard/scroll_display_legacy.py, in _get_scroll_settings
'gap_between_games': scroll_config.get('gap_between_games', 48),🤖 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/soccer-scoreboard/scroll_display_legacy.py` at line 345, Update the
gap_between_games default in _get_scroll_settings from 24 to 48 so soccer uses
the intended spacing when the setting is absent; keep the existing lookup in the
display flow unchanged.
Two gaps found while testing the eight plugins on the panel. football's adaptive renderer was missed. layout_mode "adaptive" returns from _render_game_card_adaptive before any of the classic-path code runs, so it still drew the score unconditionally (0-0 on upcoming games) and used the raw numeric date. devpi runs football with layout_mode: adaptive, so this was exactly the configuration under test. The adaptive path now gates the score to live/recent, draws VS in the score region for upcoming, honours upcoming_center, and formats the date through _format_game_date. Its goldens are regenerated for recent and upcoming; the live goldens came out pixel-identical and are left untouched. League separator icons keep the card rhythm. The leading icon was padded by a fixed 4px per side, which looked wrong once cards moved to 48px apart. It now uses the same gap_between_games // 2 as the cards. Confirmed on devpi: the league icon leads each run, then the games. Harness 168/168 PASS, football adaptive suite 27/27.
Turns the values that were baked into the card into settings, and wires up the layout offsets the schema had been advertising without effect. Still scoped to game_renderer.py and scroll_display*.py, which only scroll and Vegas modes reach; the full-screen scoreboard is untouched. scroll_card grows from three settings to eleven: vs_text the matchup separator -- VS, @, at, v, or blank date_format abbrev | numeric | day_first | numeric_day_first | weekday time_format 12h | 24h show_date draw the date at all show_time draw the start time at all swap_date_time date on top, time along the bottom upcoming_center vs | date_time | none center_gap pin the middle strip, 0 for edge-to-edge logos center_gap_ratio/_min/_max tune the automatic gap customization gains text_color on all six text elements, and customization.layout is now read by this renderer: the x/y offsets for score, date, time, status_text and both logos have been in the schema and the web UI all along, but only sports.py (the full-screen scorebug) consumed them, so nudging them did nothing to the scroll cards. Offsets are read through the same semantics as sports.py _get_layout_offset. On "at" and "@": the away team is drawn in the left slot and the home team in the right, and every score reads away-home, so "A at B" is the correct reading. Verified by holding the away team fixed and changing only the home team -- the left slot stays pixel-identical in all eight plugins. Also restores _logo_cache_key, which the block rewrite dropped while six plugins still called it. Harness 168/168 PASS. football adaptive suite 27/27 (its path needed the new options wired separately, since it returns before the classic code). baseball 126 passed / 4 pre-existing errors, matching baseline. Every setting verified end to end: helper values and the drawn pixels.
Each customization element carries x-propertyOrder ["font", "font_size"], and the config form renders that list rather than the property set — so the text_color field added alongside them was present in the schema, valid, and completely invisible in the UI. Appending it to the order makes it render. Caught by fetching the rendered config partial from the running web service rather than reading the schema: text_color appeared 0 times in the HTML for a schema that declared it six times. baseball is untouched here because its elements have no x-propertyOrder, so its text_color fields were already rendering.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
plugins/football-scoreboard/game_renderer.py (1)
320-321: 🚀 Performance & Scalability | 🔴 Critical | ⚡ Quick winLogo cache guard tests the unscoped abbreviation.
Line 320 tests
team_abbrev, but Line 321 readsself._logo_cache_key(team_abbrev), and Line 338 stores only the scoped key. The cache never hits, so every card reopens and re-thumbnails the logo file. Line 298 inpreload_logoshas the same mismatch.🐛 Proposed fix
- if team_abbrev in self._logo_cache: - return self._logo_cache[self._logo_cache_key(team_abbrev)] + cache_key = self._logo_cache_key(team_abbrev) + if cache_key in self._logo_cache: + return self._logo_cache[cache_key]🤖 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/football-scoreboard/game_renderer.py` around lines 320 - 321, Update the cache guards in the logo lookup logic and preload_logos to test the same scoped key returned by _logo_cache_key(team_abbrev) before reading or storing entries, ensuring cached logos are reused.plugins/nrl-scoreboard/game_renderer.py (1)
173-174: 🚀 Performance & Scalability | 🔴 Critical | ⚡ Quick winLogo cache guard tests the unscoped abbreviation.
Line 173 tests
team_abbrev, but Line 174 readsself._logo_cache_key(team_abbrev). Lines 161 and 191 store only the scoped key. The hit branch is unreachable, so every card reopens and re-thumbnails the logo. Line 151 inpreload_logoshas the same mismatch.🐛 Proposed fix
- if team_abbrev in self._logo_cache: - return self._logo_cache[self._logo_cache_key(team_abbrev)] + cache_key = self._logo_cache_key(team_abbrev) + if cache_key in self._logo_cache: + return self._logo_cache[cache_key]🤖 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/game_renderer.py` around lines 173 - 174, Update the cache-membership checks in the logo-loading flow and preload_logos to use the same scoped key returned by _logo_cache_key(team_abbrev) that is used for cache reads and writes. Preserve the existing cached-logo return behavior so cached logos avoid reopening and re-thumbnailing.Source: Linters/SAST tools
plugins/lacrosse-scoreboard/game_renderer.py (1)
194-201: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winThe new
_logo_cache_keyhelper was applied to reads and writes but not to the membership guards. In both renderers the guard tests an unscoped key while the read and the write use the scoped key, so nothing ever writes the key the guard checks. Every guard misses, and each render reloads and re-thumbnails the logo from disk on the scroll hot path.
plugins/lacrosse-scoreboard/game_renderer.py#L194-L201: computecache_key = self._logo_cache_key(f"{league}_{team_abbrev}")once, use it for the guard, the read, and the write at Line 225; apply the same change to theteam_abbrevfallback and to the guard inpreload_logosat Line 169.plugins/soccer-scoreboard/game_renderer.py#L173-L174: computecache_key = self._logo_cache_key(team_abbrev)once, use it for the guard, the read, and the write at Line 191; apply the same change to the guard inpreload_logosat Line 151.🤖 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/game_renderer.py` around lines 194 - 201, Fix cache-key normalization in plugins/lacrosse-scoreboard/game_renderer.py lines 194-201 and plugins/soccer-scoreboard/game_renderer.py lines 173-174: compute the scoped key with _logo_cache_key before each membership check, then reuse it for the cache read and corresponding write; apply the same normalized-key guard to preload_logos in lacrosse line 169 and soccer line 151, with no direct change needed at those guards beyond using the shared key helper.plugins/baseball-scoreboard/game_renderer.py (1)
194-206: 🎯 Functional Correctness | 🟠 MajorScope the logo cache key to the logo slot width.
_load_and_resize_logo()resizes logos withself._logo_slot_width()andself.display_height, but stores them under{league}_{team_abbrev}.scroll_display.pyandscroll_display_legacy.pyreuseself._logo_cacheacrossGameRendererinstances with differentgame_card_widthvalues, so one renderer can receive a logo sized for another width. Use a renderer-scoped key like the lacrosse/soccer helpers.
[low_effort_and_high.reward]🤖 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/baseball-scoreboard/game_renderer.py` around lines 194 - 206, Update _load_and_resize_logo so its _logo_cache key includes the renderer’s logo slot width, matching the renderer-scoped key pattern used by the lacrosse and soccer helpers. Ensure both cache lookups and stores use this width-scoped key, while preserving the existing league and team abbreviation components.
♻️ Duplicate comments (2)
plugins/afl-scoreboard/game_renderer.py (1)
187-188: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winLogo cache guard still tests the unscoped key.
Line 187 tests
team_abbrev, but Line 188 readsself._logo_cache_key(team_abbrev). No writer stores the bare abbreviation, so the cache never hits, and a bare key written by another caller makes the read raiseKeyError. Line 165 inpreload_logoshas the same mismatch.🐛 Proposed fix
- if team_abbrev in self._logo_cache: - return self._logo_cache[self._logo_cache_key(team_abbrev)] + cache_key = self._logo_cache_key(team_abbrev) + if cache_key in self._logo_cache: + return self._logo_cache[cache_key]🤖 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 187 - 188, Update the cache guards in the logo lookup and preload paths, including preload_logos, to test the same scoped key produced by _logo_cache_key(team_abbrev) before reading or writing _logo_cache. Ensure cache hits use the scoped key consistently and avoid KeyError from unscoped entries.plugins/basketball-scoreboard/game_renderer.py (1)
217-220: 🚀 Performance & Scalability | 🔴 Critical | ⚡ Quick winLogo cache guard tests an unscoped key while the read uses the scoped key.
Line 219 tests
f"{league}:{team_abbrev}", but Line 220 readsself._logo_cache_key(cache_key). Stores at Lines 240 and 259 use the scoped key only. The hit branch is unreachable, so each card re-decodes the logo. Lines 191-196 inpreload_logosrepeat the mismatch.🐛 Proposed fix
- cache_key = f"{league}:{team_abbrev}" - if cache_key in self._logo_cache: - return self._logo_cache[self._logo_cache_key(cache_key)] + cache_key = self._logo_cache_key(f"{league}:{team_abbrev}") + cached = self._logo_cache.get(cache_key) + if cached is not None: + return cachedThen store under
cache_keyat Lines 240 and 259.🤖 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/basketball-scoreboard/game_renderer.py` around lines 217 - 220, Align logo cache key usage in the renderer: update the guard and read in the logo-loading method to use the same scoped key, and store entries under that exact key at both cache-write sites. Apply the same correction in preload_logos so its lookup matches its stored values and cache hits avoid re-decoding logos.
🧹 Nitpick comments (2)
plugins/hockey-scoreboard/game_renderer.py (1)
816-824: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
_card_tzinfoduplicates_display_tzinfo.Lines 816-824 and Lines 903-911 have identical bodies. Keep one method and call it from both sites.
♻️ Proposed refactor
- def _card_tzinfo(self): - """Timezone for weekday/24h conversions; falls back to UTC.""" - try: - configured = (self.config or {}).get("timezone") - if configured: - return ZoneInfo(configured) - except Exception: - pass - return timezone.utc -Then change the call at Line 812 to
self._display_tzinfo().🤖 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/hockey-scoreboard/game_renderer.py` around lines 816 - 824, Remove the duplicate _card_tzinfo method and reuse _display_tzinfo for timezone resolution. Update the call near the card rendering flow to invoke self._display_tzinfo(), preserving the existing UTC fallback and configuration handling.plugins/baseball-scoreboard/game_renderer.py (1)
625-644: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate the two layout-offset helpers.
This class now has
_layout_offsethere and_get_layout_offsetat Line 976. Both readcustomization.layout.<element>.<axis>and both coerce numeric strings. Keep one implementation and route the other name to it, so the odds path and the card path cannot drift.♻️ Proposed refactor
- def _get_layout_offset(self, element: str, axis: str, default: int = 0) -> int: - """Get layout offset for a specific element and axis from config.""" - try: - layout_config = self.config.get('customization', {}).get('layout', {}) - element_config = layout_config.get(element, {}) - offset_value = element_config.get(axis, default) - if offset_value is None: - return default - if isinstance(offset_value, (int, float)): - return int(offset_value) - # Handle string values (e.g. "2.0" from config) - try: - return int(float(offset_value)) - except (ValueError, TypeError): - self.logger.warning(f"Invalid layout offset for {element}.{axis}: '{offset_value}', using default {default}") - return default - except (TypeError, ValueError): - return default + def _get_layout_offset(self, element: str, axis: str, default: int = 0) -> int: + """Deprecated alias kept for the odds path.""" + return self._layout_offset(element, axis, default)🤖 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/baseball-scoreboard/game_renderer.py` around lines 625 - 644, Consolidate the duplicate layout-offset logic in _layout_offset and _get_layout_offset: retain a single implementation for reading and coercing customization.layout.<element>.<axis>, and make the other helper delegate to it. Preserve the existing default and invalid-value behavior for both the odds and card rendering paths.
🤖 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/game_renderer.py`:
- Around line 724-746: Add the required datetime, timezone, and ZoneInfo imports
used by _weekday_for and _card_tzinfo in plugins/afl-scoreboard/game_renderer.py
(724-746), plugins/basketball-scoreboard/game_renderer.py (709-731),
plugins/football-scoreboard/game_renderer.py (1188-1210), and
plugins/nrl-scoreboard/game_renderer.py (699-721).
In `@plugins/hockey-scoreboard/scroll_display.py`:
- Around line 212-214: Update the spacing logic around sep_pad and the
separator/game-card layout to split gap_between_games into left_pad = gap // 2
and right_pad = gap - left_pad, preserving the full configured gap for odd
values. Apply the asymmetric padding consistently to both separators and game
cards, including the related layout blocks near the separator and card rendering
paths.
In `@plugins/lacrosse-scoreboard/game_renderer.py`:
- Around line 772-780: Fix _card_tzinfo in
plugins/lacrosse-scoreboard/game_renderer.py#L772-L780,
plugins/soccer-scoreboard/game_renderer.py#L713-L721, and
plugins/baseball-scoreboard/game_renderer.py#L735-L743 by adding the required
datetime/timezone and ZoneInfo imports, accounting for baseball’s existing
datetime import. Replace each broad silent exception around the ZoneInfo lookup
with a narrow, logged handler so invalid timezone names are visible while
retaining the UTC fallback.
---
Outside diff comments:
In `@plugins/baseball-scoreboard/game_renderer.py`:
- Around line 194-206: Update _load_and_resize_logo so its _logo_cache key
includes the renderer’s logo slot width, matching the renderer-scoped key
pattern used by the lacrosse and soccer helpers. Ensure both cache lookups and
stores use this width-scoped key, while preserving the existing league and team
abbreviation components.
In `@plugins/football-scoreboard/game_renderer.py`:
- Around line 320-321: Update the cache guards in the logo lookup logic and
preload_logos to test the same scoped key returned by
_logo_cache_key(team_abbrev) before reading or storing entries, ensuring cached
logos are reused.
In `@plugins/lacrosse-scoreboard/game_renderer.py`:
- Around line 194-201: Fix cache-key normalization in
plugins/lacrosse-scoreboard/game_renderer.py lines 194-201 and
plugins/soccer-scoreboard/game_renderer.py lines 173-174: compute the scoped key
with _logo_cache_key before each membership check, then reuse it for the cache
read and corresponding write; apply the same normalized-key guard to
preload_logos in lacrosse line 169 and soccer line 151, with no direct change
needed at those guards beyond using the shared key helper.
In `@plugins/nrl-scoreboard/game_renderer.py`:
- Around line 173-174: Update the cache-membership checks in the logo-loading
flow and preload_logos to use the same scoped key returned by
_logo_cache_key(team_abbrev) that is used for cache reads and writes. Preserve
the existing cached-logo return behavior so cached logos avoid reopening and
re-thumbnailing.
---
Duplicate comments:
In `@plugins/afl-scoreboard/game_renderer.py`:
- Around line 187-188: Update the cache guards in the logo lookup and preload
paths, including preload_logos, to test the same scoped key produced by
_logo_cache_key(team_abbrev) before reading or writing _logo_cache. Ensure cache
hits use the scoped key consistently and avoid KeyError from unscoped entries.
In `@plugins/basketball-scoreboard/game_renderer.py`:
- Around line 217-220: Align logo cache key usage in the renderer: update the
guard and read in the logo-loading method to use the same scoped key, and store
entries under that exact key at both cache-write sites. Apply the same
correction in preload_logos so its lookup matches its stored values and cache
hits avoid re-decoding logos.
---
Nitpick comments:
In `@plugins/baseball-scoreboard/game_renderer.py`:
- Around line 625-644: Consolidate the duplicate layout-offset logic in
_layout_offset and _get_layout_offset: retain a single implementation for
reading and coercing customization.layout.<element>.<axis>, and make the other
helper delegate to it. Preserve the existing default and invalid-value behavior
for both the odds and card rendering paths.
In `@plugins/hockey-scoreboard/game_renderer.py`:
- Around line 816-824: Remove the duplicate _card_tzinfo method and reuse
_display_tzinfo for timezone resolution. Update the call near the card rendering
flow to invoke self._display_tzinfo(), preserving the existing UTC fallback and
configuration handling.
🪄 Autofix
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: 57743207-5d5e-455d-9b51-0d49b8a18b81
⛔ Files ignored due to path filters (6)
plugins/football-scoreboard/test/golden-adaptive/128x32/recent.pngis excluded by!**/*.pngplugins/football-scoreboard/test/golden-adaptive/128x32/upcoming.pngis excluded by!**/*.pngplugins/football-scoreboard/test/golden-adaptive/128x64/recent.pngis excluded by!**/*.pngplugins/football-scoreboard/test/golden-adaptive/128x64/upcoming.pngis excluded by!**/*.pngplugins/football-scoreboard/test/golden-adaptive/256x128/recent.pngis excluded by!**/*.pngplugins/football-scoreboard/test/golden-adaptive/256x128/upcoming.pngis excluded by!**/*.png
📒 Files selected for processing (40)
plugins/afl-scoreboard/config_schema.jsonplugins/afl-scoreboard/game_renderer.pyplugins/afl-scoreboard/manifest.jsonplugins/afl-scoreboard/scroll_display.pyplugins/afl-scoreboard/scroll_display_legacy.pyplugins/baseball-scoreboard/config_schema.jsonplugins/baseball-scoreboard/game_renderer.pyplugins/baseball-scoreboard/manifest.jsonplugins/baseball-scoreboard/scroll_display.pyplugins/baseball-scoreboard/scroll_display_legacy.pyplugins/basketball-scoreboard/config_schema.jsonplugins/basketball-scoreboard/game_renderer.pyplugins/basketball-scoreboard/manifest.jsonplugins/basketball-scoreboard/scroll_display.pyplugins/basketball-scoreboard/scroll_display_legacy.pyplugins/football-scoreboard/config_schema.jsonplugins/football-scoreboard/game_renderer.pyplugins/football-scoreboard/manifest.jsonplugins/football-scoreboard/scroll_display.pyplugins/football-scoreboard/scroll_display_legacy.pyplugins/hockey-scoreboard/config_schema.jsonplugins/hockey-scoreboard/game_renderer.pyplugins/hockey-scoreboard/manifest.jsonplugins/hockey-scoreboard/scroll_display.pyplugins/hockey-scoreboard/scroll_display_legacy.pyplugins/lacrosse-scoreboard/config_schema.jsonplugins/lacrosse-scoreboard/game_renderer.pyplugins/lacrosse-scoreboard/manifest.jsonplugins/lacrosse-scoreboard/scroll_display.pyplugins/lacrosse-scoreboard/scroll_display_legacy.pyplugins/nrl-scoreboard/config_schema.jsonplugins/nrl-scoreboard/game_renderer.pyplugins/nrl-scoreboard/manifest.jsonplugins/nrl-scoreboard/scroll_display.pyplugins/nrl-scoreboard/scroll_display_legacy.pyplugins/soccer-scoreboard/config_schema.jsonplugins/soccer-scoreboard/game_renderer.pyplugins/soccer-scoreboard/manifest.jsonplugins/soccer-scoreboard/scroll_display.pyplugins/soccer-scoreboard/scroll_display_legacy.py
🚧 Files skipped from review as they are similar to previous changes (22)
- plugins/lacrosse-scoreboard/manifest.json
- plugins/basketball-scoreboard/manifest.json
- plugins/baseball-scoreboard/manifest.json
- plugins/hockey-scoreboard/scroll_display_legacy.py
- plugins/lacrosse-scoreboard/scroll_display.py
- plugins/lacrosse-scoreboard/scroll_display_legacy.py
- plugins/afl-scoreboard/manifest.json
- plugins/nrl-scoreboard/scroll_display_legacy.py
- plugins/basketball-scoreboard/scroll_display_legacy.py
- plugins/soccer-scoreboard/scroll_display_legacy.py
- plugins/baseball-scoreboard/scroll_display.py
- plugins/afl-scoreboard/scroll_display.py
- plugins/afl-scoreboard/scroll_display_legacy.py
- plugins/hockey-scoreboard/manifest.json
- plugins/soccer-scoreboard/scroll_display.py
- plugins/nrl-scoreboard/manifest.json
- plugins/baseball-scoreboard/scroll_display_legacy.py
- plugins/basketball-scoreboard/scroll_display.py
- plugins/football-scoreboard/scroll_display.py
- plugins/football-scoreboard/manifest.json
- plugins/nrl-scoreboard/scroll_display.py
- plugins/football-scoreboard/scroll_display_legacy.py
Comment on lines
+724
to
+746
| def _weekday_for(self, game: Optional[Dict]) -> str: | ||
| """Weekday abbreviation from the game's start time, or ''.""" | ||
| if not game: | ||
| return "" | ||
| raw = game.get("start_time_utc") or game.get("start_time") | ||
| if not raw: | ||
| return "" | ||
| try: | ||
| start = raw if isinstance(raw, datetime) else datetime.fromisoformat( | ||
| str(raw).replace("Z", "+00:00")) | ||
| return self._WEEKDAY_ABBR[start.astimezone(self._card_tzinfo()).weekday()] | ||
| except (ValueError, TypeError): | ||
| return "" | ||
|
|
||
| def _card_tzinfo(self): | ||
| """Timezone for weekday/24h conversions; falls back to UTC.""" | ||
| try: | ||
| configured = (self.config or {}).get("timezone") | ||
| if configured: | ||
| return ZoneInfo(configured) | ||
| except Exception: | ||
| pass | ||
| return timezone.utc |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Missing datetime, timezone, and ZoneInfo imports raise NameError at render time. The new _weekday_for and _card_tzinfo helpers were copied into four plugins, but only plugins/hockey-scoreboard/game_renderer.py added the imports. Ruff and Codacy report F821 at each site. _format_game_date calls _weekday_for when scroll_card.date_format is "weekday", so that configuration crashes the card render.
plugins/afl-scoreboard/game_renderer.py#L724-L746: addfrom datetime import datetime, timezoneandfrom zoneinfo import ZoneInfoto the module imports.plugins/basketball-scoreboard/game_renderer.py#L709-L731: add the same two imports.plugins/football-scoreboard/game_renderer.py#L1188-L1210: add the same two imports.plugins/nrl-scoreboard/game_renderer.py#L699-L721: add the same two imports.
+from datetime import datetime, timezone
+from zoneinfo import ZoneInfo📝 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.
Suggested change
| def _weekday_for(self, game: Optional[Dict]) -> str: | |
| """Weekday abbreviation from the game's start time, or ''.""" | |
| if not game: | |
| return "" | |
| raw = game.get("start_time_utc") or game.get("start_time") | |
| if not raw: | |
| return "" | |
| try: | |
| start = raw if isinstance(raw, datetime) else datetime.fromisoformat( | |
| str(raw).replace("Z", "+00:00")) | |
| return self._WEEKDAY_ABBR[start.astimezone(self._card_tzinfo()).weekday()] | |
| except (ValueError, TypeError): | |
| return "" | |
| def _card_tzinfo(self): | |
| """Timezone for weekday/24h conversions; falls back to UTC.""" | |
| try: | |
| configured = (self.config or {}).get("timezone") | |
| if configured: | |
| return ZoneInfo(configured) | |
| except Exception: | |
| pass | |
| return timezone.utc | |
| from datetime import datetime, timezone | |
| from zoneinfo import ZoneInfo | |
| def _weekday_for(self, game: Optional[Dict]) -> str: | |
| """Weekday abbreviation from the game's start time, or ''.""" | |
| if not game: | |
| return "" | |
| raw = game.get("start_time_utc") or game.get("start_time") | |
| if not raw: | |
| return "" | |
| try: | |
| start = raw if isinstance(raw, datetime) else datetime.fromisoformat( | |
| str(raw).replace("Z", "+00:00")) | |
| return self._WEEKDAY_ABBR[start.astimezone(self._card_tzinfo()).weekday()] | |
| except (ValueError, TypeError): | |
| return "" | |
| def _card_tzinfo(self): | |
| """Timezone for weekday/24h conversions; falls back to UTC.""" | |
| try: | |
| configured = (self.config or {}).get("timezone") | |
| if configured: | |
| return ZoneInfo(configured) | |
| except Exception: | |
| pass | |
| return timezone.utc |
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[warning] 732-732: plugins/afl-scoreboard/game_renderer.py#L732
undefined name 'datetime' (F821)
[warning] 743-743: plugins/afl-scoreboard/game_renderer.py#L743
undefined name 'ZoneInfo' (F821)
[warning] 744-744: plugins/afl-scoreboard/game_renderer.py#L744
Try, Except, Pass detected.
[warning] 746-746: plugins/afl-scoreboard/game_renderer.py#L746
undefined name 'timezone' (F821)
🪛 Ruff (0.16.1)
[error] 732-732: Undefined name datetime
(F821)
[error] 732-732: Undefined name datetime
(F821)
[error] 743-743: Undefined name ZoneInfo
(F821)
[error] 744-745: try-except-pass detected, consider logging the exception
(S110)
[warning] 744-744: Do not catch blind exception: Exception
(BLE001)
[error] 746-746: Undefined name timezone
(F821)
📍 Affects 4 files
plugins/afl-scoreboard/game_renderer.py#L724-L746(this comment)plugins/basketball-scoreboard/game_renderer.py#L709-L731plugins/football-scoreboard/game_renderer.py#L1188-L1210plugins/nrl-scoreboard/game_renderer.py#L699-L721
🤖 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 724 - 746, Add the
required datetime, timezone, and ZoneInfo imports used by _weekday_for and
_card_tzinfo in plugins/afl-scoreboard/game_renderer.py (724-746),
plugins/basketball-scoreboard/game_renderer.py (709-731),
plugins/football-scoreboard/game_renderer.py (1188-1210), and
plugins/nrl-scoreboard/game_renderer.py (699-721).
Source: Linters/SAST tools
Comment on lines
+212
to
+214
| # Match the gap used between game cards so the leading league | ||
| # icon sits in the same rhythm as the cards that follow it. | ||
| sep_pad = max(4, gap_between_games // 2) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
while IFS= read -r file; do
echo "== $file =="
rg -n -C 5 '"gap_between_games"|"multipleOf"|"minimum"' "$file" || true
done < <(fd -t f 'config_schema.json' plugins)Repository: ChuckBuilds/ledmatrix-plugins
Length of output: 219
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
fd -t f 'config_schema.json|scroll_display.py|.*hockey.*' plugins | sed -n '1,120p'
echo
echo "== schema occurrences =="
sd -i 'gap_between_games|multipleOf|minimum' $(fd -t f 'config_schema.json' plugins) | sed -n '1,200p' || true
echo
echo "== relevant scroll_display.py sections =="
if [ -f plugins/hockey-scoreboard/scroll_display.py ]; then
wc -l plugins/hockey-scoreboard/scroll_display.py
sed -n '180,290p' plugins/hockey-scoreboard/scroll_display.py | nl -ba -v180
fiRepository: ChuckBuilds/ledmatrix-plugins
Length of output: 2959
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== hockey schema gap options =="
grep -RIn -C 4 'gap_between_games' plugins/hockey-scoreboard/config_schema.json || true
echo
echo "== schema occurrences across plugins =="
grep -RIn -C 2 'gap_between_games\|multipleOf\|minimum' plugins | sed -n '1,260p'
echo
echo "== relevant scroll_display.py sections =="
sed -n '180,290p' plugins/hockey-scoreboard/scroll_display.pyRepository: ChuckBuilds/ledmatrix-plugins
Length of output: 30233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== hockey gap schema =="
grep -RIn -C 4 '"gap_between_games"' plugins/hockey-scoreboard/config_schema.json
echo
echo "== all gap_between_games schemas =="
python3 - <<'PY'
import json, pathlib
for path in pathlib.Path('plugins').rglob('config_schema.json'):
try:
data=json.loads(path.read_text(encoding='utf-8'))
except Exception:
continue
# simple recursive walk for object schemas only
def walk(obj, path=''):
if isinstance(obj, dict):
if obj.get('type') == 'object' and obj.get('properties'):
if 'gap_between_games' in obj.get('properties', {}):
p=obj['properties']['gap_between_games']
print(path.strip() or path)
print(json.dumps(p, indent=2, ensure_ascii=False)[:1200])
for k,v in obj.get('properties', {}).items():
walk(v, f"{path}/{k}")
walk(data)
PY
echo
echo "== deterministic gap loss examples =="
python3 - <<'PY'
gaps=[8,9,10,11,48]
for g in gaps:
sep_pad = max(4, g // 2)
item_gap=0
adjacent_gap = sep_pad + sep_pad + item_gap
loss = g - adjacent_gap
print(f"configured={g} sep_pad={sep_pad} adjacent={adjacent_gap} loss={loss}")
PYRepository: ChuckBuilds/ledmatrix-plugins
Length of output: 6867
Divide the configured gap across padding.
gap_between_games is an integer, so odd values are valid. gap_between_games // 2 is applied symmetrically with item_gap=0, so odd gaps render one pixel short between the separator/icon and the next card. Use left_pad = gap // 2 and right_pad = gap - left_pad for the same effective gap, and apply these pads to both separators and game cards.
Also applies to: 245-259, 274-282
🤖 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/hockey-scoreboard/scroll_display.py` around lines 212 - 214, Update
the spacing logic around sep_pad and the separator/game-card layout to split
gap_between_games into left_pad = gap // 2 and right_pad = gap - left_pad,
preserving the full configured gap for odd values. Apply the asymmetric padding
consistently to both separators and game cards, including the related layout
blocks near the separator and card rendering paths.
The config form derives an enum's option text from its value, so the
dropdowns read "Vs", "Abbrev" and "Numeric Day First" — accurate to the
config key and useless as a description of what you get. Supplying
x-options.labels makes each option show its own output:
date_format Sep 19 | 9/19 | 19 Sep | 19/9 | Fri Sep 19
time_format 7:00PM (12-hour) | 19:00 (24-hour)
upcoming_center Matchup separator (VS / @ / at) | Date and time,
stacked | Nothing
Labels are display only, so no saved config changes. Cores that predate
x-options.labels support for plain enums (LEDMatrix PR #442) ignore the
key and fall back to the humanised value, which is the behaviour these
dropdowns have today -- verified against a device running the older
template, so this is safe to ship ahead of that core change.
The timezone lookup used try/except Exception: pass, which Codacy flags as B110 and which is genuinely worse than it looks: any failure inside ZoneInfo() vanished with no record, so a typo in the configured zone looked identical to no zone being set. Now catches the exceptions ZoneInfo actually raises -- KeyError (which covers ZoneInfoNotFoundError), ValueError, TypeError and OSError -- and logs the unusable value at debug before falling back to UTC. The fallback behaviour is unchanged; it just leaves a trace now. Verified: a valid zone resolves, and a bad one, an empty string and None all fall back to UTC without raising. Bandit reports no findings from this PR's changes (the three remaining B110 hits predate it), and the PR now introduces no broad `except Exception` at all. Harness 168/168, football adaptive 27/27.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/lacrosse-scoreboard/game_renderer.py (1)
532-553: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRender upcoming center content.
At Line 532, the upcoming path skips
_draw_upcoming_center(). Withscroll_card.upcoming_centerset to"date_time",_draw_upcoming_game_status()returns at Line 862. The card then shows no date or time.Proposed fix
if game_type in ("live", "recent"): home_score = str(home_team.get("score", "0")) away_score = str(away_team.get("score", "0")) score_text = f"{away_score}-{home_score}" ... self._draw_text_with_outline( draw_overlay, score_text, (score_x, score_y), self.fonts['score'], fill=self._score_color_for(game, game_type) ) + elif game_type == "upcoming": + self._draw_upcoming_center(draw_overlay, game)As per coding guidelines, plugins must render correctly on all supported matrix sizes.
🤖 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/game_renderer.py` around lines 532 - 553, Update the game rendering flow around the live/recent score block and game-type status dispatch to call _draw_upcoming_center() for upcoming games before or alongside _draw_upcoming_game_status(). Ensure upcoming_center set to "date_time" renders its date/time content on every supported matrix size, including when _draw_upcoming_game_status() returns early.Source: Coding guidelines
♻️ Duplicate comments (2)
plugins/afl-scoreboard/game_renderer.py (2)
740-748: 🩺 Stability & Availability | 🟠 MajorImport
ZoneInfobefore using_card_tzinfo().Each method references
ZoneInfo, but the module does not import it. Iftimezoneis configured anddate_formatis"weekday",_card_tzinfo()raisesNameError._weekday_for()does not catch that error.Add
from zoneinfo import ZoneInfoto each module. Confirm that the declared Python target supports the standard-libraryzoneinfomodule.#!/bin/bash set -euo pipefail for file in \ plugins/afl-scoreboard/game_renderer.py \ plugins/baseball-scoreboard/game_renderer.py \ plugins/basketball-scoreboard/game_renderer.py \ plugins/football-scoreboard/game_renderer.py \ plugins/lacrosse-scoreboard/game_renderer.py \ plugins/nrl-scoreboard/game_renderer.py \ plugins/soccer-scoreboard/game_renderer.py do echo "== $file ==" rg -n '^(from zoneinfo import ZoneInfo|import zoneinfo)|\bZoneInfo\(' "$file" done🤖 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 740 - 748, Import ZoneInfo from the standard-library zoneinfo module in each affected game_renderer.py file, so _card_tzinfo() can resolve configured time zones without NameError; verify the declared Python target supports zoneinfo. Apply this in plugins/afl-scoreboard/game_renderer.py:740-748, plugins/baseball-scoreboard/game_renderer.py:737-745, plugins/basketball-scoreboard/game_renderer.py:725-733, plugins/football-scoreboard/game_renderer.py:1204-1212, plugins/lacrosse-scoreboard/game_renderer.py:774-782, plugins/nrl-scoreboard/game_renderer.py:715-723, and plugins/soccer-scoreboard/game_renderer.py:715-723.Source: Linters/SAST tools
165-188: 🚀 Performance & Scalability | 🟠 MajorUse the same dimension-scoped key for every logo-cache operation.
Several renderers store logos under
_logo_cache_key(...)but test membership with an unscoped key. This disables cache hits and reloads logos for each card. Legacy unscoped entries can also causeKeyErrorwhen the subsequent read transforms the key again. Baseball does not scope its shared cache by card dimensions, so it can reuse a logo sized for a different panel.
plugins/afl-scoreboard/game_renderer.py#L165-L188: derive the scoped key once for preload, membership checks, reads, and writes.plugins/baseball-scoreboard/game_renderer.py#L169-L205: add a dimension-scoped key containing league and abbreviation, then use it for reads and writes.plugins/basketball-scoreboard/game_renderer.py#L184-L260: use the scoped league-and-abbreviation key for preload and_load_and_resize_logo().plugins/football-scoreboard/game_renderer.py#L295-L338: use the scoped abbreviation key for preload and_load_and_resize_logo().plugins/lacrosse-scoreboard/game_renderer.py#L163-L225: use the scoped league-and-abbreviation key consistently; read a legacy raw key directly before optional migration.plugins/nrl-scoreboard/game_renderer.py#L148-L191: use the scoped abbreviation key for preload and_load_and_resize_logo().plugins/soccer-scoreboard/game_renderer.py#L148-L191: use the scoped abbreviation key for preload and_load_and_resize_logo().🤖 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 165 - 188, Use dimension-scoped cache keys consistently across the logo preload and _load_and_resize_logo flows: in plugins/afl-scoreboard/game_renderer.py:165-188 derive one _logo_cache_key value for membership, reads, and writes; in plugins/baseball-scoreboard/game_renderer.py:169-205 add and use a league-and-abbreviation key; in plugins/basketball-scoreboard/game_renderer.py:184-260 use its scoped league-and-abbreviation key; in plugins/football-scoreboard/game_renderer.py:295-338 and plugins/nrl-scoreboard/game_renderer.py:148-191 use the scoped abbreviation key; in plugins/soccer-scoreboard/game_renderer.py:148-191 do the same; and in plugins/lacrosse-scoreboard/game_renderer.py:163-225 use the scoped league-and-abbreviation key consistently while reading any legacy raw key directly before optional migration.
🤖 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.
Outside diff comments:
In `@plugins/lacrosse-scoreboard/game_renderer.py`:
- Around line 532-553: Update the game rendering flow around the live/recent
score block and game-type status dispatch to call _draw_upcoming_center() for
upcoming games before or alongside _draw_upcoming_game_status(). Ensure
upcoming_center set to "date_time" renders its date/time content on every
supported matrix size, including when _draw_upcoming_game_status() returns
early.
---
Duplicate comments:
In `@plugins/afl-scoreboard/game_renderer.py`:
- Around line 740-748: Import ZoneInfo from the standard-library zoneinfo module
in each affected game_renderer.py file, so _card_tzinfo() can resolve configured
time zones without NameError; verify the declared Python target supports
zoneinfo. Apply this in plugins/afl-scoreboard/game_renderer.py:740-748,
plugins/baseball-scoreboard/game_renderer.py:737-745,
plugins/basketball-scoreboard/game_renderer.py:725-733,
plugins/football-scoreboard/game_renderer.py:1204-1212,
plugins/lacrosse-scoreboard/game_renderer.py:774-782,
plugins/nrl-scoreboard/game_renderer.py:715-723, and
plugins/soccer-scoreboard/game_renderer.py:715-723.
- Around line 165-188: Use dimension-scoped cache keys consistently across the
logo preload and _load_and_resize_logo flows: in
plugins/afl-scoreboard/game_renderer.py:165-188 derive one _logo_cache_key value
for membership, reads, and writes; in
plugins/baseball-scoreboard/game_renderer.py:169-205 add and use a
league-and-abbreviation key; in
plugins/basketball-scoreboard/game_renderer.py:184-260 use its scoped
league-and-abbreviation key; in
plugins/football-scoreboard/game_renderer.py:295-338 and
plugins/nrl-scoreboard/game_renderer.py:148-191 use the scoped abbreviation key;
in plugins/soccer-scoreboard/game_renderer.py:148-191 do the same; and in
plugins/lacrosse-scoreboard/game_renderer.py:163-225 use the scoped
league-and-abbreviation key consistently while reading any legacy raw key
directly before optional migration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e34643a0-bb7b-40e6-bffa-9bda87fb48bd
📒 Files selected for processing (16)
plugins/afl-scoreboard/config_schema.jsonplugins/afl-scoreboard/game_renderer.pyplugins/baseball-scoreboard/config_schema.jsonplugins/baseball-scoreboard/game_renderer.pyplugins/basketball-scoreboard/config_schema.jsonplugins/basketball-scoreboard/game_renderer.pyplugins/football-scoreboard/config_schema.jsonplugins/football-scoreboard/game_renderer.pyplugins/hockey-scoreboard/config_schema.jsonplugins/hockey-scoreboard/game_renderer.pyplugins/lacrosse-scoreboard/config_schema.jsonplugins/lacrosse-scoreboard/game_renderer.pyplugins/nrl-scoreboard/config_schema.jsonplugins/nrl-scoreboard/game_renderer.pyplugins/soccer-scoreboard/config_schema.jsonplugins/soccer-scoreboard/game_renderer.py
🚧 Files skipped from review as they are similar to previous changes (6)
- plugins/nrl-scoreboard/config_schema.json
- plugins/afl-scoreboard/config_schema.json
- plugins/hockey-scoreboard/game_renderer.py
- plugins/lacrosse-scoreboard/config_schema.json
- plugins/basketball-scoreboard/config_schema.json
- plugins/football-scoreboard/config_schema.json
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.
The bug
Upcoming hockey games scroll past as two logos and a bare "VS" — no date, no time. It also makes the two logos look pressed together, because nothing is drawn between them.
Root cause: a key mismatch, not missing data
ESPN has the data.
shortDetailfor the next NHL games is"9/19 - 7:00 PM EDT".The scroll/Vegas path feeds cards from the sports extractor, which emits:
sports.py:905-907)_draw_upcoming_game_statusreadgame_time(flat)status.short_detail✗game_date(flat)start_time_utcstart_time✗Neither key existed on that payload, so both branches got empty strings and drew nothing. The peer sports (
football-scoreboard,basketball-scoreboard) read the flat keys and render fine — which is why hockey alone looked broken.The fix
_upcoming_date_and_time()resolves all three payload shapes in priority order:game_date/game_time— what the scroll/Vegas path actually sendsstatus.short_detail— the shapedata_fetcher.pybuilds; split on" - "and trimmed of its timezone suffix, since"7:00 PM EDT"nearly overruns a 128px cardstart_time_utc/start_timeparsed and localized — last resortTime renders top-center, date bottom-center, matching the other sports' cards.
Logo geometry is unchanged.
football-scoreboard/game_renderer.py:564uses the identicallogo_slot = min(display_height, display_width // 2), so the crowding was a symptom of the blank card, not a separate layout bug.Verification
date='9/19',time='7:00PM'check_module_collisions.py: clean across 43 pluginsNote
hockey.py:79is a dead expression (status["type"].get("shortDetail", ""), result discarded). Left alone — it's on the live path, not this one.Summary by CodeRabbit
New Features
VS, date/time, or blank center content.Bug Fixes
0-0scores on upcoming games.Chores