Performance, accessibility, and test-coverage improvements across the plugin fleet - #239
Merged
Merged
Conversation
hockey-scoreboard/base_classes.py and scoreboard_renderer.py are stale near-duplicates of hockey.py's live code; basketball_helpers.py is an unused font-loading helper. Nothing imports any of them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
- of-the-day: the classic-fallback fonts (used when the core lacks src.element_style) were reloaded from disk on every render; they are config-independent, so load once and reuse. - web-ui-info: display() reloaded the 4x6 font on every call. - on-air: the shrink-to-fit path reloaded the scaled TTF each frame for wide labels; now memoized by (path, size). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
…rness matrix - New 08-shared-sports-code.md: the three scoreboard lineages, per-module drift table, guarded convergence pattern, sunset rule for local copies, and the fix-all-lineage-members-in-one-PR rule (commit 8d33894 as the cautionary example). - 03-advanced-features.md: document the three incompatible scroll_speed unit semantics (px/frame, px/second, inverted frames-per-step divisor). - 07-testing-ci-and-registry.md: the harness default matrix is 8 sizes, not 4. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
… keys Accessibility rollout, additive only — default rendering is byte-identical in every plugin (custom faces load only when config differs from the schema default, so web-UI merged-defaults configs are unaffected): - clock-simple: honor the customization font/font_size keys the schema already declared (code previously read only text_color); existing golden images pass unchanged. - news: per-element headline_text/source_text font+size; source line color was previously hardcoded (150,150,150) and is now configurable. - mqtt-notifications: message_text font face+size (single text style). - countdown: name_font_family — the name line previously had per-element size/color but shared the value line's font face. - tide-display: tide_text/label_text font+size+color, routed by the existing palette constants; chart colors untouched. - youtube-stats: channel_name/subscriber_count/view_count font+size+ color (plugin previously had zero styling config). - x-advanced added to scroll_speed/scroll_delay fine-tuning keys in nrl, ufc, elections, stocks, leaderboard, news, odds-ticker, march-madness, text-display schemas (UI hint only). text-display and on-air deliberately unchanged: their existing config surface already covers font face, size, and colors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
…madness, text-display Non-scoreboard half of the global-FPS rollout (canonical pattern from ledmatrix-leaderboard): read global_config target_fps/scroll_target_fps, probe set_target_fps with hasattr, clamp 30-200 with a frame_time_target fallback for older ScrollHelper builds. - elections, stocks: adopt the block (previously ignored the global FPS). - nfl-draft: also set an explicit scroll_delay (pacing was previously left at the helper default and unconfigured). - march-madness: plugin-level display_options.target_fps still wins, falls back to the global; added the missing older-core fallback branch. - text-display: added the hasattr guard + fallback (its 240 ceiling was already clamped to 200 inside the core helper). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
…4px overflow Deterministic test/harness.json for countdown, christmas-countdown, 7-segment-clock, text-display, static-image, web-ui-info, of-the-day, on-air (config + freeze_time archetype; static-image renders a bundled test-pattern PNG). Golden images committed for the six deterministic renderers — every golden was generated twice and byte-compared before committing. No goldens for web-ui-info (renders the host IP) or on-air (active state is event-driven). The of-the-day fixture immediately exposed a real bug: on 64px-wide panels the title (PressStart2P@8) and its underline drew past the right edge, and body lines ran past the bottom on 64x32. Titles now ellipsize to the panel width, the underline is clamped inside the panel, and body lines stop before the bottom edge. Wider panels render identically. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
Sports scoreboards still receiving font-cache/odds/fps work (afl, baseball, football, soccer, lacrosse, f1) will be bumped with those changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds plugin font and color customization, global scroll pacing, shared sports-code guidance, deterministic safety-harness coverage, rendering safeguards, workflow failure reporting, and updated plugin release metadata. ChangesPlugin behavior and rendering
Scroll and validation support
Release metadata
Estimated code review effort: 4 (Complex) | ~60 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 |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 279 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/youtube-stats/manager.py (1)
271-304: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTruncation and row spacing do not scale with the new customizable font sizes.
max_chars(line 284) still assumes 8 pixels per character, andline_height(line 271) is still a fixed10. The newcustomizationschema allowschannel_name,subscriber_count, andview_countfonts up to 16px (double the previous fixed 8px). At larger sizes, or on the smaller 64×32/128×32 panels, the character-count heuristic can under-truncatechannel_nameso the measured text still overflows the available width, and the fixed row height can make adjacent rows overlap.Measure the actual rendered width for truncation (the codebase already has this pattern in
news.py's_truncate_to_width) and deriveline_heightfrom the selected fonts' bounding boxes instead of a fixed constant.As per path instructions, "Ensure plugins render correctly at all supported panel sizes: 64×32, 128×32, 128×64, and 256×32, without overflow, crashes, or incorrect scaling."
🐛 Proposed fix for truncation and row spacing
- # Calculate text positions - line_height = 10 # Approximate line height for PressStart2P font at size 8 - total_text_height = line_height * 3 # 3 lines of text - start_y = (matrix_height - total_text_height) // 2 - # Per-element fonts: None override means the classic self.font. # Measurement (textbbox) below always uses the same face as the draw. name_font = self.name_font_override or self.font subs_font = self.subs_font_override or self.font views_font = self.views_font_override or self.font + # Calculate text positions using the actual selected fonts' metrics + row_h = max( + draw.textbbox((0, 0), "Ag", font=name_font)[3], + draw.textbbox((0, 0), "Ag", font=subs_font)[3], + draw.textbbox((0, 0), "Ag", font=views_font)[3], + 10, + ) + line_height = row_h + 2 + total_text_height = line_height * 3 # 3 lines of text + start_y = (matrix_height - total_text_height) // 2 + # Draw channel name (top) channel_name = channel_stats['title'] - # Truncate channel name if too long - max_chars = (matrix_width - right_section_x - 4) // 8 # 8 pixels per character - if len(channel_name) > max_chars: - channel_name = channel_name[:max_chars-3] + "..." - name_bbox = draw.textbbox((0, 0), channel_name, font=name_font) + avail_width = matrix_width - right_section_x - 4 + name_bbox = draw.textbbox((0, 0), channel_name, font=name_font) + if name_bbox[2] - name_bbox[0] > avail_width: + while channel_name and draw.textbbox((0, 0), channel_name + "...", font=name_font)[2] > avail_width: + channel_name = channel_name[:-1] + channel_name = (channel_name + "...") if channel_name else "..." + name_bbox = draw.textbbox((0, 0), channel_name, font=name_font)🤖 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/youtube-stats/manager.py` around lines 271 - 304, Update the rendering flow containing name_font, subs_font, and views_font to truncate channel_name using measured text widths, following news.py’s _truncate_to_width pattern, instead of the fixed 8-pixel max_chars heuristic. Derive line_height from the selected fonts’ text bounding boxes, ensuring the calculated three-row layout and truncation fit all supported panel sizes without overlap or overflow.Source: Path instructions
🧹 Nitpick comments (6)
plugins/youtube-stats/manager.py (2)
43-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
_rgbdoes not validate color length; also duplicates other plugins' color parsing.
_rgb(lines 48-52) clamps channel values but never checks thatvaluehas exactly 3 elements, unlike_parse_colorinplugins/news/manager.py(which returnsdefaultwhenlen(color) != 3). A malformedtext_color(for example a hand-edited config that isn't length-3) would silently produce a 2- or 4-element color tuple used indraw.text(fill=...).
_create_displaycatches all exceptions (lines 307-309), so the practical effect today is a skipped redraw rather than a crash — still worth aligning with the safer pattern. See the consolidated comment for the related duplication across 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/youtube-stats/manager.py` around lines 43 - 67, Update the local _rgb helper used by the YouTube customization color fields to validate that value is a sequence of exactly three channels before clamping and converting them; return the provided default for invalid lengths or types. Align this behavior with the existing _parse_color pattern in the news plugin while preserving the current defaults for name_color, subs_color, and views_color.
84-120: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftDuplicate font-resolution logic across plugins.
This
_load_element_fontduplicates the near-identical methods added inplugins/mqtt-notifications/manager.py,plugins/news/manager.py, andplugins/tide-display/manager.py. See the consolidated comment for the full list of duplicate sites and a suggested shared-helper approach.🤖 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/youtube-stats/manager.py` around lines 84 - 120, Consolidate _load_element_font and the equivalent font-resolution methods in mqtt-notifications, news, and tide-display into one shared helper, then update each manager to call it while preserving default-font handling, caching, lookup paths, and warning behavior. Remove the duplicated per-plugin implementations and reuse the shared helper’s result.plugins/news/manager.py (1)
328-366: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftDuplicate font-resolution logic across plugins.
This
_load_element_fontimplementation is the most generalized of the four near-duplicate copies added in this PR (it parameterizesdefault_name/default_size, while the mqtt-notifications, youtube-stats, and tide-display copies hardcode their defaults). Use this version as the basis for a shared helper onBasePluginand have the other plugins call it instead of duplicating the loop and cache logic. See the consolidated comment for the full list of duplicate sites.🤖 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/news/manager.py` around lines 328 - 366, Move the generalized font-resolution logic from _load_element_font into a shared BasePlugin helper, preserving its default handling, cache behavior, candidate paths, warnings, and fallback semantics. Update the mqtt-notifications, youtube-stats, and tide-display implementations to call the BasePlugin helper with their respective default font names and sizes, removing their duplicated resolution loops and caches.plugins/tide-display/manager.py (2)
167-201: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftDuplicate font-resolution logic across plugins.
This
_load_element_fontduplicates the near-identical methods added inplugins/mqtt-notifications/manager.py,plugins/news/manager.py, andplugins/youtube-stats/manager.py. See the consolidated comment for the full list of duplicate sites and a suggested shared-helper approach.🤖 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/tide-display/manager.py` around lines 167 - 201, Replace the duplicate font-resolution implementation in _load_element_font with the shared font-loading helper used by the MQTT notifications, news, and YouTube stats managers. Preserve the existing default-font detection, size parsing, caching, fallback-to-None behavior, and warning behavior through the shared helper rather than maintaining another local copy.
146-165: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
_crgbduplicates the existing_rgbclosure in__init__, and neither validates color length.
_crgb(lines 153-157) repeats the same clamp-and-cast logic as the pre-existing_rgbclosure in__init__(lines 105-109), instead of reusing it. Neither closure checks that the parsed value has exactly 3 elements, unlike_parse_colorinplugins/news/manager.py(which returnsdefaultwhen the length is wrong).Because the result only reaches drawing through
_raw_txt, which is wrapped intry/except, a malformedtext_colordegrades gracefully today rather than crashing — but consolidating both closures into one validated instance method would remove the duplication and the latent gap at the same time. See the consolidated comment.🤖 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/tide-display/manager.py` around lines 146 - 165, Consolidate the duplicated color parsing in `_apply_customization` and the `__init__` `_rgb` closure into one validated instance method, reusing it for both text and label colors. Ensure the shared parser returns the supplied default unless the input contains exactly three values, while preserving per-color clamping and integer conversion behavior.plugins/mqtt-notifications/manager.py (1)
182-221: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftDuplicate font-resolution logic across plugins.
_load_element_fonthere duplicates the almost-identical method added inplugins/news/manager.py,plugins/youtube-stats/manager.py, andplugins/tide-display/manager.py. Only the hardcoded default font name/size differs between copies.Extract a single shared helper (for example on
BasePlugin) that accepts the element config, default name, default size, and a cache dict, and reuse it everywhere. This reduces maintenance cost when the font-resolution logic changes. See the consolidated comment for the full list of duplicate sites.🤖 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/mqtt-notifications/manager.py` around lines 182 - 221, The _load_element_font method duplicates font-resolution logic across multiple plugin managers. Extract the shared implementation into a reusable BasePlugin helper accepting element configuration, default font name, default size, and cache, then update _load_element_font in the MQTT, news, youtube-stats, and tide-display managers to delegate to it while preserving each plugin’s defaults and existing fallback behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/clock-simple/manager.py`:
- Around line 391-426: Constrain configurable text to supported panel dimensions
in plugins/clock-simple/manager.py at lines 391-426, 431-466, and 490-516:
update the combined and standard centered time layouts to reflow, reduce, or
omit content when measured width exceeds width, preventing negative x positions;
constrain the date candidate to fit the panel rather than blindly using the
shortest candidate, and calculate date y positions from the selected font bounds
before drawing.
In `@plugins/countdown/test/harness.json`:
- Around line 2-13: Update the harness config object to explicitly set the
schema-backed font and color properties referenced by its comment, using their
current default values so the golden remains stable if defaults change. Keep the
existing countdown and show_expired settings unchanged.
In `@plugins/ledmatrix-elections/manager.py`:
- Around line 95-103: Initialize self.global_config from config.get('global',
{}) during startup and refresh it from the same source in each plugin’s
on_config_change before reading target_fps:
plugins/ledmatrix-elections/manager.py (lines 95-103),
plugins/ledmatrix-stocks/manager.py (lines 88-97), and
plugins/nfl-draft/manager.py (lines 130-143). Preserve the existing FPS
application logic while ensuring reloads use the current global configuration.
In `@plugins/of-the-day/manager.py`:
- Around line 514-515: Update both _display_title and _display_content to clamp
the title’s x-coordinate after applying title_dx, restricting title_x to [0,
width - title_width] for every supported matrix size. Before drawing the
underline, validate that its clipped start coordinate does not exceed its end
coordinate, and skip it when the interval is invalid.
- Around line 396-420: Update _fit_title so it validates the ellipsis itself
against max_width before truncating the title. If "..." cannot fit, return a
fitting fallback such as an empty string; otherwise retain the current
title-fitting behavior and ensure the final truncated result is width-bounded
across all supported panel sizes.
In `@plugins/text-display/manager.py`:
- Around line 172-179: Align the target_fps handling in the configuration schema
and the manager’s fallback branch so both use the same maximum of 200 FPS;
ensure the stored/reporting value matches the effective scroll_helper rate when
set_target_fps is unavailable.
- Around line 172-179: Extract the target-FPS application logic currently split
between initialization and the compatibility branch into a shared helper,
preserving the 30–240 configuration clamp and 30–200 fallback clamp. Update both
constructor initialization and on_config_change to call this helper so older
ScrollHelper implementations do not invoke set_target_fps unconditionally when
scroll_speed, scroll_delay, or target_fps changes.
In `@plugins/tide-display/manager.py`:
- Around line 605-621: Update _raw_txt to use standard multiline try/except
formatting, placing the draw_text call and logger.debug call on separate
indented lines to eliminate Ruff E701; preserve the existing debug message and
drawing behavior.
---
Outside diff comments:
In `@plugins/youtube-stats/manager.py`:
- Around line 271-304: Update the rendering flow containing name_font,
subs_font, and views_font to truncate channel_name using measured text widths,
following news.py’s _truncate_to_width pattern, instead of the fixed 8-pixel
max_chars heuristic. Derive line_height from the selected fonts’ text bounding
boxes, ensuring the calculated three-row layout and truncation fit all supported
panel sizes without overlap or overflow.
---
Nitpick comments:
In `@plugins/mqtt-notifications/manager.py`:
- Around line 182-221: The _load_element_font method duplicates font-resolution
logic across multiple plugin managers. Extract the shared implementation into a
reusable BasePlugin helper accepting element configuration, default font name,
default size, and cache, then update _load_element_font in the MQTT, news,
youtube-stats, and tide-display managers to delegate to it while preserving each
plugin’s defaults and existing fallback behavior.
In `@plugins/news/manager.py`:
- Around line 328-366: Move the generalized font-resolution logic from
_load_element_font into a shared BasePlugin helper, preserving its default
handling, cache behavior, candidate paths, warnings, and fallback semantics.
Update the mqtt-notifications, youtube-stats, and tide-display implementations
to call the BasePlugin helper with their respective default font names and
sizes, removing their duplicated resolution loops and caches.
In `@plugins/tide-display/manager.py`:
- Around line 167-201: Replace the duplicate font-resolution implementation in
_load_element_font with the shared font-loading helper used by the MQTT
notifications, news, and YouTube stats managers. Preserve the existing
default-font detection, size parsing, caching, fallback-to-None behavior, and
warning behavior through the shared helper rather than maintaining another local
copy.
- Around line 146-165: Consolidate the duplicated color parsing in
`_apply_customization` and the `__init__` `_rgb` closure into one validated
instance method, reusing it for both text and label colors. Ensure the shared
parser returns the supplied default unless the input contains exactly three
values, while preserving per-color clamping and integer conversion behavior.
In `@plugins/youtube-stats/manager.py`:
- Around line 43-67: Update the local _rgb helper used by the YouTube
customization color fields to validate that value is a sequence of exactly three
channels before clamping and converting them; return the provided default for
invalid lengths or types. Align this behavior with the existing _parse_color
pattern in the news plugin while preserving the current defaults for name_color,
subs_color, and views_color.
- Around line 84-120: Consolidate _load_element_font and the equivalent
font-resolution methods in mqtt-notifications, news, and tide-display into one
shared helper, then update each manager to call it while preserving default-font
handling, caching, lookup paths, and warning behavior. Remove the duplicated
per-plugin implementations and reuse the shared helper’s result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: eaa277b7-97bf-4514-86b0-382aac34d3ef
⛔ Files ignored due to path filters (49)
plugins/7-segment-clock/test/golden/128x32/7-segment-clock.pngis excluded by!**/*.pngplugins/7-segment-clock/test/golden/128x64/7-segment-clock.pngis excluded by!**/*.pngplugins/7-segment-clock/test/golden/128x96/7-segment-clock.pngis excluded by!**/*.pngplugins/7-segment-clock/test/golden/256x128/7-segment-clock.pngis excluded by!**/*.pngplugins/7-segment-clock/test/golden/256x32/7-segment-clock.pngis excluded by!**/*.pngplugins/7-segment-clock/test/golden/64x32/7-segment-clock.pngis excluded by!**/*.pngplugins/7-segment-clock/test/golden/64x64/7-segment-clock.pngis excluded by!**/*.pngplugins/7-segment-clock/test/golden/96x48/7-segment-clock.pngis excluded by!**/*.pngplugins/christmas-countdown/test/golden/128x32/christmas-countdown.pngis excluded by!**/*.pngplugins/christmas-countdown/test/golden/128x64/christmas-countdown.pngis excluded by!**/*.pngplugins/christmas-countdown/test/golden/128x96/christmas-countdown.pngis excluded by!**/*.pngplugins/christmas-countdown/test/golden/256x128/christmas-countdown.pngis excluded by!**/*.pngplugins/christmas-countdown/test/golden/256x32/christmas-countdown.pngis excluded by!**/*.pngplugins/christmas-countdown/test/golden/64x32/christmas-countdown.pngis excluded by!**/*.pngplugins/christmas-countdown/test/golden/64x64/christmas-countdown.pngis excluded by!**/*.pngplugins/christmas-countdown/test/golden/96x48/christmas-countdown.pngis excluded by!**/*.pngplugins/countdown/test/golden/128x32/countdown.pngis excluded by!**/*.pngplugins/countdown/test/golden/128x64/countdown.pngis excluded by!**/*.pngplugins/countdown/test/golden/128x96/countdown.pngis excluded by!**/*.pngplugins/countdown/test/golden/256x128/countdown.pngis excluded by!**/*.pngplugins/countdown/test/golden/256x32/countdown.pngis excluded by!**/*.pngplugins/countdown/test/golden/64x32/countdown.pngis excluded by!**/*.pngplugins/countdown/test/golden/64x64/countdown.pngis excluded by!**/*.pngplugins/countdown/test/golden/96x48/countdown.pngis excluded by!**/*.pngplugins/of-the-day/test/golden/128x32/of_the_day.pngis excluded by!**/*.pngplugins/of-the-day/test/golden/128x64/of_the_day.pngis excluded by!**/*.pngplugins/of-the-day/test/golden/128x96/of_the_day.pngis excluded by!**/*.pngplugins/of-the-day/test/golden/256x128/of_the_day.pngis excluded by!**/*.pngplugins/of-the-day/test/golden/256x32/of_the_day.pngis excluded by!**/*.pngplugins/of-the-day/test/golden/64x32/of_the_day.pngis excluded by!**/*.pngplugins/of-the-day/test/golden/64x64/of_the_day.pngis excluded by!**/*.pngplugins/of-the-day/test/golden/96x48/of_the_day.pngis excluded by!**/*.pngplugins/static-image/test/fixtures/test-pattern.pngis excluded by!**/*.pngplugins/static-image/test/golden/128x32/static_image.pngis excluded by!**/*.pngplugins/static-image/test/golden/128x64/static_image.pngis excluded by!**/*.pngplugins/static-image/test/golden/128x96/static_image.pngis excluded by!**/*.pngplugins/static-image/test/golden/256x128/static_image.pngis excluded by!**/*.pngplugins/static-image/test/golden/256x32/static_image.pngis excluded by!**/*.pngplugins/static-image/test/golden/64x32/static_image.pngis excluded by!**/*.pngplugins/static-image/test/golden/64x64/static_image.pngis excluded by!**/*.pngplugins/static-image/test/golden/96x48/static_image.pngis excluded by!**/*.pngplugins/text-display/test/golden/128x32/text_display.pngis excluded by!**/*.pngplugins/text-display/test/golden/128x64/text_display.pngis excluded by!**/*.pngplugins/text-display/test/golden/128x96/text_display.pngis excluded by!**/*.pngplugins/text-display/test/golden/256x128/text_display.pngis excluded by!**/*.pngplugins/text-display/test/golden/256x32/text_display.pngis excluded by!**/*.pngplugins/text-display/test/golden/64x32/text_display.pngis excluded by!**/*.pngplugins/text-display/test/golden/64x64/text_display.pngis excluded by!**/*.pngplugins/text-display/test/golden/96x48/text_display.pngis excluded by!**/*.png
📒 Files selected for processing (62)
docs/plugin-development/03-advanced-features.mddocs/plugin-development/07-testing-ci-and-registry.mddocs/plugin-development/08-shared-sports-code.mdplugins.jsonplugins/7-segment-clock/test/harness.jsonplugins/basketball-scoreboard/basketball_helpers.pyplugins/basketball-scoreboard/manifest.jsonplugins/christmas-countdown/test/harness.jsonplugins/clock-simple/manager.pyplugins/clock-simple/manifest.jsonplugins/countdown/config_schema.jsonplugins/countdown/manager.pyplugins/countdown/manifest.jsonplugins/countdown/test/harness.jsonplugins/hockey-scoreboard/base_classes.pyplugins/hockey-scoreboard/manifest.jsonplugins/hockey-scoreboard/scoreboard_renderer.pyplugins/ledmatrix-elections/config_schema.jsonplugins/ledmatrix-elections/manager.pyplugins/ledmatrix-elections/manifest.jsonplugins/ledmatrix-leaderboard/config_schema.jsonplugins/ledmatrix-leaderboard/manifest.jsonplugins/ledmatrix-stocks/config_schema.jsonplugins/ledmatrix-stocks/manager.pyplugins/ledmatrix-stocks/manifest.jsonplugins/march-madness/config_schema.jsonplugins/march-madness/manager.pyplugins/march-madness/manifest.jsonplugins/mqtt-notifications/config_schema.jsonplugins/mqtt-notifications/manager.pyplugins/mqtt-notifications/manifest.jsonplugins/news/config_schema.jsonplugins/news/manager.pyplugins/news/manifest.jsonplugins/nfl-draft/manager.pyplugins/nfl-draft/manifest.jsonplugins/nrl-scoreboard/config_schema.jsonplugins/nrl-scoreboard/manifest.jsonplugins/odds-ticker/config_schema.jsonplugins/odds-ticker/manifest.jsonplugins/of-the-day/manager.pyplugins/of-the-day/manifest.jsonplugins/of-the-day/test/harness.jsonplugins/on-air/manager.pyplugins/on-air/manifest.jsonplugins/on-air/test/harness.jsonplugins/static-image/test/harness.jsonplugins/text-display/config_schema.jsonplugins/text-display/manager.pyplugins/text-display/manifest.jsonplugins/text-display/test/harness.jsonplugins/tide-display/config_schema.jsonplugins/tide-display/manager.pyplugins/tide-display/manifest.jsonplugins/ufc-scoreboard/config_schema.jsonplugins/ufc-scoreboard/manifest.jsonplugins/web-ui-info/manager.pyplugins/web-ui-info/manifest.jsonplugins/web-ui-info/test/harness.jsonplugins/youtube-stats/config_schema.jsonplugins/youtube-stats/manager.pyplugins/youtube-stats/manifest.json
💤 Files with no reviewable changes (3)
- plugins/basketball-scoreboard/basketball_helpers.py
- plugins/hockey-scoreboard/scoreboard_renderer.py
- plugins/hockey-scoreboard/base_classes.py
- elections/stocks/nfl-draft/march-madness: source the FPS target from the plugin config's 'global' section (the news/leaderboard convention) instead of a never-set attribute; elections also refreshes it in on_config_change. - text-display: single _apply_target_fps helper used by init and on_config_change — the latter previously called set_target_fps unconditionally and would raise AttributeError on older cores; the stored value is now the effective 30-200 clamped rate. - of-the-day: clamp title_x after the user layout offset; guard _fit_title against a panel too narrow for the ellipsis itself. - clock-simple: clamp the combined time+AM/PM block to start on-panel when a user-selected font exceeds the width. - youtube-stats: truncate the channel name by measured width and derive row height from the selected fonts when a custom font is set (identical layout at the monospace default); validate color length in _rgb. - tide-display: same color-length validation; unfold the one-line try/except flagged by Ruff E701. - countdown fixture: pin font/color schema defaults explicitly so a future default change cannot silently invalidate the goldens. All affected plugins re-verified with the harness; clock-simple, countdown, and of-the-day goldens pass unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
Owner
Author
|
Review findings addressed in fd81aed: FPS-target sourcing from the plugin One suggestion not actionable here: consolidating the four Generated by Claude Code |
hockey 1.5.1->1.6.0, basketball 1.8.1->1.9.0; odds-ticker moves to 1.1.9 (main took 1.1.8). plugins.json regenerated via update_registry.py. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
…st cwd CI runs the safety harness from the workspace root rather than the core checkout, so the plugin's relative font_path missed assets/fonts and fell back to PIL's default font — mismatching the committed goldens (generated with the real font). Add a resolution strategy that walks up from the display manager's module location to find the core's assets, making font loading cwd-independent. Goldens now pass from both working directories. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
… per frame The scorebug draw paths (SportsUpcoming/SportsRecent in sports.py, FootballLive in football.py) reloaded 4x6-font.ttf from disk on every rendered frame when records/rankings are shown; game_renderer's _draw_records_or_rankings did the same unconditionally. The face is now cached once in _load_fonts (fonts['record']) with a lazy memo in the renderer, using the accessor pattern ufc-scoreboard already ships. Rendering is pixel-identical: the full adaptive-layout and score- celebration golden suites (46 tests) pass unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
…per frame Same treatment as football-scoreboard: fonts['record'] and fonts['shots'] cached in _load_fonts, scorebug draw sites use the accessor pattern, and game_renderer's fallback disk load is memoized (its detail-font primary lookup — a real lineage difference from football — is preserved). Also fixes a latent crash: hockey.py's shots font load had no try/except, so a missing 4x6-font.ttf killed the live render instead of degrading. Harness output is byte-identical to the pre-change baselines for both plugins; offline test scripts (favorite-live-boost, non-favorite-live- duration, timezone-resolution) all pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
…ng change Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
…ding per frame Completes the font-caching sweep started for football/hockey/lacrosse (cf93709, 640c081). Same treatment across the three remaining sports-scoreboard lineages: - afl-scoreboard: sports.py had three separate uncached reload sites (SportsUpcoming/SportsRecent ranking overlays); game_renderer.py was already caching fonts["record"] in _load_fonts. Added the same cache to sports.py's _load_fonts and switched all three sites to the self.fonts.get("record") accessor. - baseball-scoreboard: sports.py had two uncached reload sites; fixed the same way. game_renderer.py already reused fonts['detail'], no change needed there. - soccer-scoreboard: sports.py had three uncached reload sites, and game_renderer.py's _draw_records_or_rankings reloaded unconditionally on every call (no fonts-dict entry at all) -- given the football game_renderer lazy-memo treatment (getattr(self, '_record_font', ...)). f1-scoreboard checked and needs no change: its renderer loads all fonts once in __init__ (f1_renderer.py:180), no per-frame reload pattern exists there. Verified pixel-identical: rendered afl/baseball/soccer live+recent+ upcoming at 128x64 with show_records and show_ranking forced on, before and after the change (via git stash) -- byte-for-byte identical PNGs in all cases. Full safety harness (all 8 sizes) passes for all three plugins with no new warnings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
Completes the lineage-wide font-caching change (football, hockey, lacrosse, afl, soccer landed earlier): - basketball: record + tournament-date fonts cached in _load_fonts; game_renderer memoizes its record font. - nrl: record font cached; game_renderer memoized. - ufc: upcoming/recent scorebug paths use the cached record font the plugin already loaded for its live layout. - baseball: (name,size)-keyed memo in _load_custom_font_from_element_config plus a BDF native-size cache, collapsing the per-frame 10-rung font ladder walks in the traditional-scoreboard and at-bat screens into dict lookups; the record-font block moves after the config fallback so it is set on every path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
… fallback The eight non-UFC scoreboards imported their local base_odds_manager copy unconditionally. They now try src.base_odds_manager first (a functional superset: adds cache_ttl support) and fall back to the bundled copy on cores that don't ship it. Both branches are module-level, so they stay collision-safe under the loader's bare-name isolation. In afl/nrl/soccer/basketball manager.py the odds import sat inside the combined BasePlugin guard, so a missing odds module would have nulled BasePlugin (and in nrl's case NameError'd — its except branch never set BaseOddsManager). It now has its own nested guard. UFC keeps its local copy unconditionally: it is a genuine MMA fork (athlete odds), not a drifted duplicate. Local copies stay bundled per the sunset rule in docs/plugin-development/08-shared-sports-code.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
The safety harness on CI runs from the workspace root, not the core checkout, so the core FontManager's cwd-relative assets/fonts scan came up empty and resolve_font silently degraded to PIL's default face — drifting every committed golden (the named failure in the last two safety runs). The type of the degraded font gives no signal (current Pillow's load_default() is itself a FreeTypeFont), so the miss is detected via the manager's font catalog, and the family's real file is then resolved against the core install the display manager was loaded from (same strategy text-display already uses). Verified: goldens pass from both the core root and a foreign cwd. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
…l mode The ten scroll_display.py files paced frames only from scroll_delay, so the global smooth-scrolling FPS setting (target_fps / scroll_target_fps) silently never reached the sports scoreboards. ScrollDisplay and ScrollDisplayManager now accept a trailing global_config kwarg (threaded from every manager construction site), and _configure_scroll_helper applies the canonical set_target_fps block with the clamped frame_time_target fallback for older cores. When no global value is set, pacing is unchanged. Also removes the dead _get_target_fps helpers in afl/nrl/soccer whose value was computed but never applied, and bumps f1 to 1.8.0 (its only change in this PR). Verified: harness green on all ten plugins (f1's committed goldens pass unchanged); football adaptive/celebration and soccer celebration pytest suites green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
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 (1)
plugins/tide-display/manager.py (1)
235-238: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep custom-font measurement and drawing together.
When
get_text_widthfails,_twfalls back to the defaultlen(text) * 4estimate while_txtstill draws with the configured custom font. The affected string is then clipped on small panels. Measure with the fallback size, or truncate the string before_txt.Also applies to: 707-708, 849-851, 945-946, 963-965.
🤖 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/tide-display/manager.py` around lines 235 - 238, Update the text rendering flow around _tw and _txt so fallback width calculations remain consistent with the font used for drawing: when custom-font measurement fails, either draw using the fallback font/size or truncate the string before _txt based on the fallback width. Apply the same correction to the related rendering paths at the other indicated locations, preserving custom-font measurement when it succeeds.Source: Coding guidelines
🧹 Nitpick comments (1)
plugins/text-display/manager.py (1)
307-308: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReport failures from the module-relative font probe.
inspect.getfile()and filesystem operations can fail during the core-install fallback. Catch only expected introspection/OSErrorcases, and log unexpected exceptions at debug level so font-loading failures are not hidden.Proposed fix
- except Exception: - pass + except (OSError, RuntimeError, TypeError) as exc: + self.logger.debug( + "Module-relative font resolution failed: %s", + exc, + exc_info=True, + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/text-display/manager.py` around lines 307 - 308, Update the exception handling around the module-relative font probe in the core-install fallback to catch only expected introspection and OSError failures; log any unexpected exceptions at debug level instead of silently suppressing them. Preserve the existing fallback behavior for expected failures.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins.json`:
- Line 79: Regenerate plugins.json from the corrected manifests so the Baseball
Scoreboard metadata at plugins.json lines 79-79 and AFL Scoreboard metadata at
plugins.json lines 1026-1026 use the new release date 2026-07-31 instead of
2026-07-17, while preserving their latest_version values.
In `@plugins/baseball-scoreboard/sports.py`:
- Around line 403-408: Update the cached “record” font loading in the fonts
initialization block to construct the 4x6 font path from the plugin’s asset root
rather than the process working directory. Keep the existing OSError fallback to
ImageFont.load_default() unchanged.
In `@plugins/lacrosse-scoreboard/manifest.json`:
- Line 4: Classify the scoreboard caching and performance updates as PATCH
releases and regenerate plugins.json. In
plugins/lacrosse-scoreboard/manifest.json at lines 4 and 53-57, use 1.5.1;
plugins/soccer-scoreboard/manifest.json at lines 4 and 29-33, use 2.4.2;
plugins/baseball-scoreboard/manifest.json at lines 4 and 33-37, use 1.20.2;
plugins/afl-scoreboard/manifest.json at lines 4 and 21-25, use 1.1.1; and
plugins/football-scoreboard/manifest.json at lines 4 and 27-31, use 2.9.2.
---
Outside diff comments:
In `@plugins/tide-display/manager.py`:
- Around line 235-238: Update the text rendering flow around _tw and _txt so
fallback width calculations remain consistent with the font used for drawing:
when custom-font measurement fails, either draw using the fallback font/size or
truncate the string before _txt based on the fallback width. Apply the same
correction to the related rendering paths at the other indicated locations,
preserving custom-font measurement when it succeeds.
---
Nitpick comments:
In `@plugins/text-display/manager.py`:
- Around line 307-308: Update the exception handling around the module-relative
font probe in the core-install fallback to catch only expected introspection and
OSError failures; log any unexpected exceptions at debug level instead of
silently suppressing them. Preserve the existing fallback behavior for expected
failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 657cb6da-ad93-49b1-af28-44b105665bfe
📒 Files selected for processing (33)
plugins.jsonplugins/afl-scoreboard/manifest.jsonplugins/afl-scoreboard/sports.pyplugins/baseball-scoreboard/manifest.jsonplugins/baseball-scoreboard/sports.pyplugins/basketball-scoreboard/manifest.jsonplugins/clock-simple/manager.pyplugins/countdown/test/harness.jsonplugins/football-scoreboard/football.pyplugins/football-scoreboard/game_renderer.pyplugins/football-scoreboard/manifest.jsonplugins/football-scoreboard/sports.pyplugins/hockey-scoreboard/game_renderer.pyplugins/hockey-scoreboard/hockey.pyplugins/hockey-scoreboard/manifest.jsonplugins/hockey-scoreboard/sports.pyplugins/lacrosse-scoreboard/game_renderer.pyplugins/lacrosse-scoreboard/lacrosse.pyplugins/lacrosse-scoreboard/manifest.jsonplugins/lacrosse-scoreboard/sports.pyplugins/ledmatrix-elections/manager.pyplugins/ledmatrix-stocks/manager.pyplugins/march-madness/manager.pyplugins/nfl-draft/manager.pyplugins/odds-ticker/config_schema.jsonplugins/odds-ticker/manifest.jsonplugins/of-the-day/manager.pyplugins/soccer-scoreboard/game_renderer.pyplugins/soccer-scoreboard/manifest.jsonplugins/soccer-scoreboard/sports.pyplugins/text-display/manager.pyplugins/tide-display/manager.pyplugins/youtube-stats/manager.py
🚧 Files skipped from review as they are similar to previous changes (12)
- plugins/odds-ticker/config_schema.json
- plugins/odds-ticker/manifest.json
- plugins/nfl-draft/manager.py
- plugins/youtube-stats/manager.py
- plugins/hockey-scoreboard/manifest.json
- plugins/ledmatrix-stocks/manager.py
- plugins/of-the-day/manager.py
- plugins/ledmatrix-elections/manager.py
- plugins/countdown/test/harness.json
- plugins/basketball-scoreboard/manifest.json
- plugins/clock-simple/manager.py
- plugins/march-madness/manager.py
- tide-display: when display-manager measurement fails, fall back to the drawing font's own getbbox metrics before the 4px/char estimate, so a custom face never draws wider than the width reported. - text-display, countdown: log module-relative font-probe failures at debug instead of swallowing them silently. - afl/baseball manifests carried a stale last_updated (2026-07-17) that the registry echoed; set to 2026-07-31 and teach update_registry.py to sync last_updated even when latest_version is unchanged, so catalog timestamps can no longer drift from manifests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
Owner
Author
|
Review disposition (fixes in Fixed
Not taken, with reasons
Note for the maintainer: Codacy reports 1 high / 2 medium security issues on this PR, but the specifics are only visible in the Codacy dashboard — a Bandit-pattern sweep of the added Python lines here came up empty, so they may relate to the workflow file or be tool-specific rules worth a quick look. Generated by Claude Code |
Plugin ids are derived from PR file paths, so in the invalid-id branch the raw string is fork-controllable; echoing it into workflow commands is needless exposure. Redact it and record a placeholder in the summary instead — valid ids (the useful signal) are unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
… scoreboards
The UFC managers pass sport_key='ufc_scoreboard' and SportsCore looked up
config under f'{sport_key}_scoreboard' — the nonexistent
'ufc_scoreboard_scoreboard' — so mode_config was always empty,
is_enabled always False, and every UFC screen rendered blank. The lookup
now uses the key the adapter actually writes ('ufc_scoreboard'). Found
while building harness fixtures: no cache/mock/config contents could
make the managers render.
Adds deterministic harness fixtures (config + cached-schedule mock data
+ frozen clock) for baseball, basketball, football, hockey, lacrosse,
and soccer, so the safety harness renders real game cards for
recent/upcoming (and live where the plugin's live path reads the cache:
basketball, soccer, plus baseball via its test_mode passthrough) instead
of blank no-data screens. Where the live path is a direct network fetch
with no cache read (hockey, football, lacrosse), live is disabled in the
fixture with the reason documented in each harness.json. Verified: all
sizes PASS and two consecutive runs are byte-identical for every plugin.
UFC gets no fixture yet: its fighter-headshot loader has no negative
caching and no config toggle, so offline runs spend ~15s of retries per
headshot per render — needs a small plugin change first (follow-up).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
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 (4)
plugins/tide-display/manager.py (2)
187-203: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winResolve shared fonts independently of the working directory.
The code identifies the default font as
assets/fonts/4x6-font.ttfat Lines [141]-[144]. The candidate list only finds this file when the process runs from the repository root. The__file__candidate points toplugins/tide-display/assets/fonts.When the service or harness starts from another directory, custom fonts fall back to the default even when the shared font exists. Add a repository-root or core-font-resolver candidate.
Proposed candidate
candidates = [ os.path.join('assets', 'fonts', name), + os.path.normpath(os.path.join( + os.path.dirname(os.path.abspath(__file__)), + os.pardir, os.pardir, 'assets', 'fonts', name)), os.path.join(os.path.dirname(os.path.abspath(__file__)), 'assets', 'fonts', name), ]🤖 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/tide-display/manager.py` around lines 187 - 203, Add a font candidate in the font-resolution logic around the existing candidates list and default font configuration so the shared `assets/fonts` location is resolved from the repository or core project root rather than only the current working directory or `plugins/tide-display` directory. Preserve the existing candidate order and fallback behavior, and ensure the shared `4x6-font.ttf` resource is found when launched from another working directory.
714-715: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep custom-font labels inside their layout boxes.
The measured widths are correct, but the positioning code does not handle labels wider than the available space.
- Lines [714]-[715] and [952]-[953] can calculate a negative x-coordinate.
- Lines [856]-[858] and [970]-[972] clamp only one edge, so the right edge can still overflow.
Fit the active font before drawing, fall back to the default font, or constrain the allowed font sizes for every supported matrix size. Add golden coverage for 64×32, 128×32, 128×64, and 256×32.
As per coding guidelines, plugins must render correctly at all supported panel sizes without overflow.
Also applies to: 856-858, 952-953, 970-972
🤖 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/tide-display/manager.py` around lines 714 - 715, Update the label positioning paths using _tw and _txt_s at the referenced locations so every custom-font label remains fully inside its layout box, clamping both left and right edges and preventing negative x-coordinates. Fit the active font, fall back to the default font, or constrain font sizes consistently for all supported panel dimensions. Add golden coverage for 64×32, 128×32, 128×64, and 256×32 panels.Source: Coding guidelines
plugins/soccer-scoreboard/sports.py (1)
491-522: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMove the record-font caching out of the inner try block so it runs on both paths.
The
fonts["record"]block on lines 499-504 only runs when the primarytryon lines 491-498 succeeds. If that block raises, execution jumps toexcept Exception as e:on line 505, and that fallback branch never setsfonts["record"].
plugins/nrl-scoreboard/sports.pyplaces the equivalent block after the full try/except, andplugins/ufc-scoreboard/sports.pysetsfonts["record"]in both the try and except branches. This file is inconsistent with both.Call sites already fall back safely via
self.fonts.get("record") or self.fonts.get("status") or ImageFont.load_default(), so this does not crash, but it silently changes the record font on the rare exception path.♻️ Proposed fix
fonts["rank"] = self._load_custom_font_from_element_config(rank_config, default_size=10) self.logger.info("Successfully loaded fonts from config") - # Record/ranking annotations always use the small 4x6 face; cached here - # so the scorebug draw paths don't reload it from disk every frame. - try: - fonts["record"] = ImageFont.truetype("assets/fonts/4x6-font.ttf", 6) - except OSError: - fonts["record"] = ImageFont.load_default() except Exception as e: self.logger.error(f"Error loading fonts: {e}, using defaults") # Fallback to hardcoded defaults try: fonts["score"] = ImageFont.truetype("assets/fonts/PressStart2P-Regular.ttf", 10) fonts["time"] = ImageFont.truetype("assets/fonts/PressStart2P-Regular.ttf", 8) fonts["team"] = ImageFont.truetype("assets/fonts/PressStart2P-Regular.ttf", 8) fonts["status"] = ImageFont.truetype("assets/fonts/4x6-font.ttf", 6) fonts["detail"] = ImageFont.truetype("assets/fonts/4x6-font.ttf", 6) fonts["rank"] = ImageFont.truetype("assets/fonts/PressStart2P-Regular.ttf", 10) except IOError: self.logger.warning("Fonts not found, using default PIL font.") fonts["score"] = ImageFont.load_default() fonts["time"] = ImageFont.load_default() fonts["team"] = ImageFont.load_default() fonts["status"] = ImageFont.load_default() fonts["detail"] = ImageFont.load_default() fonts["rank"] = ImageFont.load_default() + # Record/ranking annotations always use the small 4x6 face; cached here + # so the scorebug draw paths don't reload it from disk every frame. + try: + fonts["record"] = ImageFont.truetype("assets/fonts/4x6-font.ttf", 6) + except OSError: + fonts["record"] = ImageFont.load_default() return 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/soccer-scoreboard/sports.py` around lines 491 - 522, Move the fonts["record"] caching block out of the inner try in the font-loading method so it executes after the full primary/fallback try-except flow. Preserve its existing 4x6 font loading and ImageFont.load_default() fallback, ensuring fonts["record"] is initialized regardless of whether primary font loading succeeds or the fallback branch runs.plugins/hockey-scoreboard/manager.py (1)
219-237: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove the duplicate
ScrollDisplayManagerinitialization.
__init__buildsScrollDisplayManagertwice: once at Line 134 and again here at Line 223. The second block silently discards and replaces the firstScrollDisplayManagerinstance. Nothing between the two blocks usesself._scroll_manager,self._scroll_prepared, orself._scroll_active, so the first block's work, and this block's re-initialization of_scroll_prepared/_scroll_activeat Lines 236-237 (already set at Lines 148-149), have no effect other than wasted allocation.This PR had to edit both blocks to add
global_config. That is direct evidence the duplication already causes double-maintenance cost. Remove this second block and keep the first.♻️ Proposed fix to remove the duplicate block
- # Initialize scroll display manager if available - self._scroll_manager = None - if SCROLL_AVAILABLE and ScrollDisplayManager: - try: - self._scroll_manager = ScrollDisplayManager( - self.display_manager, - self.config, - self.logger, - global_config=getattr(self, 'global_config', {}) or {} - ) - self.logger.info("Hockey scroll display manager initialized") - except Exception as e: - self.logger.warning(f"Could not initialize scroll display manager: {e}") - else: - self.logger.info("Scroll display not available - scroll mode disabled") - - # Scroll state tracking - self._scroll_prepared = {} # Tracks which scroll modes are prepared - self._scroll_active = {} # Tracks which scroll modes are active - self.logger.info( f"Hockey scoreboard plugin initialized - {self.display_width}x{self.display_height}" )🤖 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/manager.py` around lines 219 - 237, Remove the later ScrollDisplayManager initialization block in __init__, including its availability check, try/except, logging, and duplicate _scroll_prepared/_scroll_active assignments. Keep the earlier initialization block and state setup unchanged, including its global_config handling.
🧹 Nitpick comments (5)
plugins/text-display/manager.py (1)
307-309: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNarrow blind exception fallbacks in the font width path.
Both
get_text_width()andfont.getbbox()fallbacks may mask logic or Pillow errors and force a 4px/char estimate. Restrict the outer handler to metric/API failures and leave the bbox fallback limited to font-metric failures only.🤖 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/text-display/manager.py` around lines 307 - 309, In plugins/text-display/manager.py lines 307-309, narrow the exception handling in get_text_width() so only font metric/API failures trigger the fallback, and restrict the font.getbbox() fallback to font-metric failures rather than catching all exceptions. Apply the same exception narrowing in plugins/tide-display/manager.py lines 238-245, preserving the existing 4px-per-character fallback only for those expected failures.Source: Linters/SAST tools
plugins/baseball-scoreboard/sports.py (2)
297-322: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNarrow the blind
except Exception:per Ruff (BLE001).Line 302 catches
ExceptionaroundImageFont.truetype(font_path, font_size).ImageFont.truetyperaisesOSErrorfor an invalid BDF strike size. The sibling record-font load later in this file already narrows toexcept OSError:. Match that here for consistency and to avoid masking unrelated errors.♻️ Narrow the exception type
try: font = ImageFont.truetype(font_path, font_size) self.logger.debug(f"Loaded BDF font: {font_name} at size {font_size}") self._font_cache[cache_key] = font return font - except Exception: + except OSError: native_size = self._bdf_native_size_cache.get(font_path)🤖 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/sports.py` around lines 297 - 322, In the font-loading block around ImageFont.truetype, replace the broad exception handler with an OSError handler, matching the sibling record-font loading logic. Keep the native-size fallback and warning behavior unchanged, and narrow only the initial BDF load catch in the surrounding font-loading method.Source: Linters/SAST tools
269-286: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueCache misses on the fallback path still hit disk every call.
_font_cacheonly stores successfully loaded TTF/BDF faces. Iffont_pathdoes not exist, or the file type is unsupported, execution falls through to the default-font block (lines 331-341) without caching. A per-frame font-ladder caller with a misconfigured or missing font still re-stats and re-loads the default font on every frame, which is the exact cost this change is meant to remove.Cache the fallback-default font result under
cache_keytoo, so a misconfigured font only pays the disk cost once.🤖 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/sports.py` around lines 269 - 286, Update the fallback-default font path in the font-loading method containing _font_cache so it stores the selected default font under the existing cache_key before returning it. Ensure missing, unsupported, or otherwise unsuccessfully loaded font paths reuse that cached fallback on subsequent calls while preserving successful TTF/BDF caching.plugins/countdown/manager.py (2)
298-334: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the exception before
continuein the candidate-loading loop.The
except Exception: continueat lines 330-331 discards the reason a candidate font path failed to load. Static analysis flags this pattern (Ruff S112, Codacy "Try, Except, Continue detected"). This method exists specifically to fix cwd-dependent font-loading failures; silently swallowing per-candidate errors removes the visibility needed to diagnose future path or file-format issues.♻️ Proposed fix to log the swallowed exception
for candidate in candidates: try: if candidate.exists(): font = ImageFont.truetype(str(candidate), int(size_px)) break - except Exception: - continue + except Exception as exc: + self.logger.debug("Font candidate %s failed to load: %s", candidate, exc) + continue🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/countdown/manager.py` around lines 298 - 334, Update the candidate-loading loop in _load_family_font_direct so the exception handler logs each failed font candidate and its exception through self.logger before continuing. Preserve the existing fallback behavior of trying remaining candidates and caching None when no candidate loads.Source: Linters/SAST tools
363-386: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache the catalog-miss check and confirm the
font_catalogattribute contract.
family_missingre-runsgetattr(fm, 'font_catalog', {}).get(family)andos.path.exists(catalog_path)on every call to_resolve_font, whichdisplay()invokes on every render (lines 693-694, 697-700). This adds an uncached filesystem stat call to a per-render path, even though_load_family_font_directalready caches by(family, size_px). Cachefamily_missingperfamilythe same way.Separately, this logic assumes the core
FontManagerexposes afont_catalogdict keyed by family name resolving to a file path. That attribute is not visible in the provided files. Please confirm this attribute name and shape are accurate; if it doesn't exist or is named differently, the code still degrades safely (falls back to the already-resolved font) but performs unnecessary work on every call.♻️ Proposed fix to cache the catalog-miss result
font = fm.resolve_font( element_key=f"{self.plugin_id}.{countdown_id}.{role}", family=family, size_px=size_px ) - try: - catalog_path = getattr(fm, 'font_catalog', {}).get(family) - family_missing = not catalog_path or not os.path.exists(catalog_path) - except Exception: - family_missing = False + family_missing = self._is_family_missing(fm, family) if font is not None and family_missing:def _is_family_missing(self, fm, family: str) -> bool: cache = getattr(self, '_family_missing_cache', None) if cache is None: cache = self._family_missing_cache = {} if family in cache: return cache[family] try: catalog_path = getattr(fm, 'font_catalog', {}).get(family) missing = not catalog_path or not os.path.exists(catalog_path) except Exception: missing = False cache[family] = missing return missing🤖 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/countdown/manager.py` around lines 363 - 386, Cache the catalog-miss result per family in a helper near _resolve_font, such as _is_family_missing, and have the resolution path reuse that cached value instead of performing repeated font_catalog lookups and filesystem checks on every render. Verify the FontManager attribute contract: use the actual catalog attribute and confirm it is a family-to-path mapping; if unavailable or differently shaped, preserve the safe fallback without repeated unnecessary work.
🤖 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/baseball-scoreboard/scroll_display.py`:
- Around line 141-149: Validate target_fps is numeric before applying the
fallback clamp in the scroll-helper configuration block. Update the
corresponding blocks in plugins/baseball-scoreboard/scroll_display.py lines
141-149, plugins/afl-scoreboard/scroll_display.py lines 171-179,
plugins/f1-scoreboard/scroll_display.py lines 72-80,
plugins/hockey-scoreboard/scroll_display.py lines 169-177,
plugins/lacrosse-scoreboard/scroll_display.py lines 169-177, and
plugins/nrl-scoreboard/scroll_display.py lines 189-197 so non-numeric
configuration values do not reach max/min, while preserving the existing setter
path and numeric clamping behavior.
In `@plugins/baseball-scoreboard/sports.py`:
- Around line 39-46: Rename the bundled fallback helper module to a
plugin-unique name for the baseball scoreboard, update the fallback import in
the BaseOddsManager loading block to use that name, and revise the surrounding
comment to remove the claim that bare-name imports are collision-safe. Apply the
same module-rename/import update to the other listed scoreboard plugins,
preserving the core src.base_odds_manager import path.
In `@plugins/football-scoreboard/scroll_display.py`:
- Around line 163-172: Update _configure_scroll_helper to parse target_fps as a
numeric value, handle invalid configuration values outside the ScrollHelper
availability check, and clamp the validated value to the [30, 200] range once
before the hasattr(self.scroll_helper, 'set_target_fps') branch. Apply this same
clamped value through either the setter or legacy fallback, and reserve
“ScrollHelper not available” for cases where ScrollHelper itself is unavailable.
In `@plugins/nrl-scoreboard/manager.py`:
- Around line 30-40: Remove the unconditional BaseOddsManager = None statement
after the nested import handling in the odds manager initialization, leaving the
assignment only in the innermost ImportError branch so successful core-shipped
or bundled imports remain available.
---
Outside diff comments:
In `@plugins/hockey-scoreboard/manager.py`:
- Around line 219-237: Remove the later ScrollDisplayManager initialization
block in __init__, including its availability check, try/except, logging, and
duplicate _scroll_prepared/_scroll_active assignments. Keep the earlier
initialization block and state setup unchanged, including its global_config
handling.
In `@plugins/soccer-scoreboard/sports.py`:
- Around line 491-522: Move the fonts["record"] caching block out of the inner
try in the font-loading method so it executes after the full primary/fallback
try-except flow. Preserve its existing 4x6 font loading and
ImageFont.load_default() fallback, ensuring fonts["record"] is initialized
regardless of whether primary font loading succeeds or the fallback branch runs.
In `@plugins/tide-display/manager.py`:
- Around line 187-203: Add a font candidate in the font-resolution logic around
the existing candidates list and default font configuration so the shared
`assets/fonts` location is resolved from the repository or core project root
rather than only the current working directory or `plugins/tide-display`
directory. Preserve the existing candidate order and fallback behavior, and
ensure the shared `4x6-font.ttf` resource is found when launched from another
working directory.
- Around line 714-715: Update the label positioning paths using _tw and _txt_s
at the referenced locations so every custom-font label remains fully inside its
layout box, clamping both left and right edges and preventing negative
x-coordinates. Fit the active font, fall back to the default font, or constrain
font sizes consistently for all supported panel dimensions. Add golden coverage
for 64×32, 128×32, 128×64, and 256×32 panels.
---
Nitpick comments:
In `@plugins/baseball-scoreboard/sports.py`:
- Around line 297-322: In the font-loading block around ImageFont.truetype,
replace the broad exception handler with an OSError handler, matching the
sibling record-font loading logic. Keep the native-size fallback and warning
behavior unchanged, and narrow only the initial BDF load catch in the
surrounding font-loading method.
- Around line 269-286: Update the fallback-default font path in the font-loading
method containing _font_cache so it stores the selected default font under the
existing cache_key before returning it. Ensure missing, unsupported, or
otherwise unsuccessfully loaded font paths reuse that cached fallback on
subsequent calls while preserving successful TTF/BDF caching.
In `@plugins/countdown/manager.py`:
- Around line 298-334: Update the candidate-loading loop in
_load_family_font_direct so the exception handler logs each failed font
candidate and its exception through self.logger before continuing. Preserve the
existing fallback behavior of trying remaining candidates and caching None when
no candidate loads.
- Around line 363-386: Cache the catalog-miss result per family in a helper near
_resolve_font, such as _is_family_missing, and have the resolution path reuse
that cached value instead of performing repeated font_catalog lookups and
filesystem checks on every render. Verify the FontManager attribute contract:
use the actual catalog attribute and confirm it is a family-to-path mapping; if
unavailable or differently shaped, preserve the safe fallback without repeated
unnecessary work.
In `@plugins/text-display/manager.py`:
- Around line 307-309: In plugins/text-display/manager.py lines 307-309, narrow
the exception handling in get_text_width() so only font metric/API failures
trigger the fallback, and restrict the font.getbbox() fallback to font-metric
failures rather than catching all exceptions. Apply the same exception narrowing
in plugins/tide-display/manager.py lines 238-245, preserving the existing
4px-per-character fallback only for those expected failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 28ee4508-21f0-4620-9392-1af90ec484b5
📒 Files selected for processing (53)
.github/workflows/test-plugins.ymlplugins.jsonplugins/afl-scoreboard/manager.pyplugins/afl-scoreboard/manifest.jsonplugins/afl-scoreboard/scroll_display.pyplugins/afl-scoreboard/sports.pyplugins/baseball-scoreboard/manager.pyplugins/baseball-scoreboard/manifest.jsonplugins/baseball-scoreboard/scroll_display.pyplugins/baseball-scoreboard/sports.pyplugins/baseball-scoreboard/test/fixtures/mock.jsonplugins/baseball-scoreboard/test/harness.jsonplugins/basketball-scoreboard/basketball.pyplugins/basketball-scoreboard/game_renderer.pyplugins/basketball-scoreboard/manager.pyplugins/basketball-scoreboard/scroll_display.pyplugins/basketball-scoreboard/sports.pyplugins/basketball-scoreboard/test/fixtures/mock.jsonplugins/basketball-scoreboard/test/harness.jsonplugins/countdown/manager.pyplugins/f1-scoreboard/manager.pyplugins/f1-scoreboard/manifest.jsonplugins/f1-scoreboard/scroll_display.pyplugins/football-scoreboard/manager.pyplugins/football-scoreboard/scroll_display.pyplugins/football-scoreboard/sports.pyplugins/football-scoreboard/test/fixtures/mock.jsonplugins/football-scoreboard/test/harness.jsonplugins/hockey-scoreboard/manager.pyplugins/hockey-scoreboard/scroll_display.pyplugins/hockey-scoreboard/sports.pyplugins/hockey-scoreboard/test/fixtures/mock.jsonplugins/hockey-scoreboard/test/harness.jsonplugins/lacrosse-scoreboard/manager.pyplugins/lacrosse-scoreboard/scroll_display.pyplugins/lacrosse-scoreboard/sports.pyplugins/lacrosse-scoreboard/test/fixtures/mock.jsonplugins/lacrosse-scoreboard/test/harness.jsonplugins/nrl-scoreboard/game_renderer.pyplugins/nrl-scoreboard/manager.pyplugins/nrl-scoreboard/scroll_display.pyplugins/nrl-scoreboard/sports.pyplugins/soccer-scoreboard/manager.pyplugins/soccer-scoreboard/scroll_display.pyplugins/soccer-scoreboard/sports.pyplugins/soccer-scoreboard/test/fixtures/mock.jsonplugins/soccer-scoreboard/test/harness.jsonplugins/text-display/manager.pyplugins/tide-display/manager.pyplugins/ufc-scoreboard/manager.pyplugins/ufc-scoreboard/scroll_display.pyplugins/ufc-scoreboard/sports.pyupdate_registry.py
🚧 Files skipped from review as they are similar to previous changes (4)
- plugins/afl-scoreboard/manifest.json
- plugins/lacrosse-scoreboard/sports.py
- plugins/afl-scoreboard/sports.py
- plugins/baseball-scoreboard/manifest.json
…ntdown - nrl manager: remove a stray unconditional BaseOddsManager = None that overwrote a successful bundled-fallback import on older cores (critical review catch). - soccer + afl sports.py: move the cached record-font block after the config/fallback branches so it is set on every path, matching the other seven copies. - hockey manager: drop the duplicate ScrollDisplayManager construction; the first instance (which enable_scrolling reads) now persists instead of being silently replaced. - all ten scroll_display.py: coerce target_fps to float before comparing so a malformed global config degrades to scroll_delay pacing instead of raising in __init__. - tide-display: resolve custom fonts against the core install (cwd- independent, same strategy as text-display/countdown) and clamp the three label positions that could go negative or overflow with large custom fonts. - baseball sports.py: narrow the BDF strike-size retry to OSError and cache the fallback-default font under the requested key so a misconfigured font stops hitting the disk per frame. - countdown: log per-candidate font-load failures instead of silently continuing, and memoize the FontManager catalog-miss check that was statting the filesystem on every render. The bare-name fallback module rename suggestion is deliberately not applied — module-level bare imports are collision-safe under the core loader's isolation rules (see the review reply and 08-shared-sports-code.md). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
target_fps had no copy to converge -- self.config is only the plugin's own
slice, so a device-wide value simply wasn't reachable, which is why the
scoreboards' getattr(self, 'global_config', ...) always saw {}.
Documents the core-side property added in ChuckBuilds/LEDMatrix#424: the
resolution order, reading it as getattr(...) so plugins still load on older
cores, that it is read-only (mutating the live config has bitten this repo
before), and that assignment still overrides it -- which news, stock-news,
ledmatrix-stocks, ledmatrix-elections, ledmatrix-leaderboard and nfl-draft
depend on.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
Contributor
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/baseball-scoreboard/sports.py (1)
39-46: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestrict the bundled fallback to a missing core module.
except ImportErroralso catches anImportErrorraised from insidesrc.base_odds_manager, so a dependency failure in the core helper falls back to the bundled odds manager instead of surfacing the root error.Catch
ModuleNotFoundErroronly. Re-raise missing submodules inside the core path before importingbase_odds_manager.Proposed fix
try: from src.base_odds_manager import BaseOddsManager -except ImportError: +except ModuleNotFoundError as exc: + if exc.name not in {"src", "src.base_odds_manager"}: + raise from base_odds_manager import BaseOddsManager🤖 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/sports.py` around lines 39 - 46, Update the import fallback around BaseOddsManager so only a missing core module triggers the bundled import: catch ModuleNotFoundError, verify it refers to the missing src.base_odds_manager module, and re-raise missing dependencies or other import failures from inside the core helper before importing base_odds_manager.
🧹 Nitpick comments (1)
plugins/tide-display/manager.py (1)
204-205: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog and narrow the font-probe exception.
The empty
except Exceptionhides why module-relative font resolution failed. Ruff and Codacy report this as a blindtry/except/passblock.Catch expected path-inspection errors and log them at debug level.
Proposed fix
-except Exception: - pass +except (OSError, TypeError) as exc: + self.logger.debug("Module-relative font resolution failed: %s", exc)🤖 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/tide-display/manager.py` around lines 204 - 205, Update the font-probe exception handling around the module-relative font resolution to catch only the expected path-inspection errors, such as lookup or filesystem-related exceptions, instead of Exception. Replace the silent pass with a debug-level log that includes the failure details, using the existing logger in the surrounding manager code.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/plugin-development/08-shared-sports-code.md`:
- Around line 82-89: Update the code example to access the configuration through
getattr(self, 'global_config', {}) instead of directly referencing
self.global_config, preserving plugin loading when the property is unavailable
on older cores.
---
Outside diff comments:
In `@plugins/baseball-scoreboard/sports.py`:
- Around line 39-46: Update the import fallback around BaseOddsManager so only a
missing core module triggers the bundled import: catch ModuleNotFoundError,
verify it refers to the missing src.base_odds_manager module, and re-raise
missing dependencies or other import failures from inside the core helper before
importing base_odds_manager.
---
Nitpick comments:
In `@plugins/tide-display/manager.py`:
- Around line 204-205: Update the font-probe exception handling around the
module-relative font resolution to catch only the expected path-inspection
errors, such as lookup or filesystem-related exceptions, instead of Exception.
Replace the silent pass with a debug-level log that includes the failure
details, using the existing logger in the surrounding manager code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cd4a3e9f-388e-4f22-a77d-9c3b291875cb
📒 Files selected for processing (18)
docs/plugin-development/08-shared-sports-code.mdplugins/afl-scoreboard/scroll_display.pyplugins/afl-scoreboard/sports.pyplugins/baseball-scoreboard/scroll_display.pyplugins/baseball-scoreboard/sports.pyplugins/basketball-scoreboard/scroll_display.pyplugins/countdown/manager.pyplugins/f1-scoreboard/scroll_display.pyplugins/football-scoreboard/scroll_display.pyplugins/hockey-scoreboard/manager.pyplugins/hockey-scoreboard/scroll_display.pyplugins/lacrosse-scoreboard/scroll_display.pyplugins/nrl-scoreboard/manager.pyplugins/nrl-scoreboard/scroll_display.pyplugins/soccer-scoreboard/scroll_display.pyplugins/soccer-scoreboard/sports.pyplugins/tide-display/manager.pyplugins/ufc-scoreboard/scroll_display.py
💤 Files with no reviewable changes (2)
- plugins/hockey-scoreboard/manager.py
- plugins/nrl-scoreboard/manager.py
🚧 Files skipped from review as they are similar to previous changes (12)
- plugins/baseball-scoreboard/scroll_display.py
- plugins/ufc-scoreboard/scroll_display.py
- plugins/f1-scoreboard/scroll_display.py
- plugins/afl-scoreboard/scroll_display.py
- plugins/soccer-scoreboard/sports.py
- plugins/football-scoreboard/scroll_display.py
- plugins/hockey-scoreboard/scroll_display.py
- plugins/afl-scoreboard/sports.py
- plugins/lacrosse-scoreboard/scroll_display.py
- plugins/nrl-scoreboard/scroll_display.py
- plugins/basketball-scoreboard/scroll_display.py
- plugins/soccer-scoreboard/scroll_display.py
…be logging) - The odds-manager convergence guards now catch ModuleNotFoundError and check exc.name, so only an absent core module triggers the bundled fallback; an import failure from inside src.base_odds_manager (missing dependency) surfaces instead of being masked. Applied to all eight sports.py guards and the four manager.py nested guards; verified both branches with meta-path simulations. - 08-shared-sports-code.md: the global_config example now uses the getattr form the same section mandates. - tide-display: the module-relative font probe logs failures at debug level instead of a blind except/pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
23 tasks
CONTRIBUTING.md scopes MINOR to new features and schema additions. None of these ten plugins gained either: the changes are font-load caching, the odds-manager import switch, target_fps threading, and test fixtures. The only schema edits (nrl, ufc) add `x-advanced` UI hints, not options. That makes them PATCH. Each new version is set one patch above **main's** current version rather than above the branch's, which also fixes a collision: soccer had been bumped to 2.5.0 here while main independently released its own 2.5.0, so two different sets of changes shared a version number. Now 2.5.1. plugins.json could not simply be regenerated -- update_registry.py refuses to lower a version, so it skipped all ten and left the registry advertising the old MINOR numbers. Reset the generated file to main's state and regenerated from there, which the tool accepts as an increase. Verified afterwards that every latest_version moves up and none moves down. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
The date and weekday draws passed no x, so draw_text auto-centred them with (width - text_width) // 2 and no clamp. _fit_date returns its shortest candidate even when that still overflows -- reachable now that the font is user-configurable -- and a negative x clips the string at *both* ends, losing the leading characters rather than just the tail. Adds _centered_x, which clamps to 0 so overflow clips on the right only. It returns None when measuring raises, deferring to draw_text's own centring rather than guessing, mirroring _text_fits assuming a fit so a measurement failure never hides content. The time and AM/PM paths already clamp (max(0, ...) on time_x, and max(0, min(...)) on ampm_x), so this closes the remaining case. Defaults are unchanged: all three committed goldens still match byte for byte, harness green at all 8 sizes. Verified the fix directly -- "Aug 1st" at 84px on a 64px panel gives draw_text x=-10 vs _centered_x x=0. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
…ovements-le58v6 # Conflicts: # plugins.json # plugins/soccer-scoreboard/manifest.json
23 tasks
Core 3.2.0 ships src/common/sports_scroll.py, the orchestration half of scroll_display.py. The content half (prepare_scroll_content, _load_separator_icons) stays per-plugin permanently -- a survey of the eight copies that share a shape found eight distinct bodies, because each draws its own game card. Same method name, different job. Documents the mechanical adoption once a plugin floors at 3.2.0, the byte-comparison acceptance gate, and the two gotchas the hockey pilot surfaced (the inherited os.path use in _load_separator_icons, and the now-dead scroll_helper guards). Measured on hockey-scoreboard: 691 -> 289 lines, all 16 harness renders byte-for-byte identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
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 (3)
plugins/f1-scoreboard/manifest.json (1)
32-37: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winSynchronize F1 release metadata before publishing.
The manifest and registry disagree on the date for version
1.7.1. The manifest says2026-08-01, while the catalog says2026-07-28.
plugins/f1-scoreboard/manifest.json#L32-L37: update the manifest timestamp metadata to2026-08-01.plugins.json#L218-L218: regenerate the F1 catalog entry solatest_versionandlast_updatedare synchronized.🤖 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/f1-scoreboard/manifest.json` around lines 32 - 37, Synchronize release metadata for F1 version 1.7.1: in plugins/f1-scoreboard/manifest.json lines 32-37, keep the manifest timestamp at 2026-08-01; in plugins.json line 218, regenerate the F1 catalog entry so latest_version and last_updated match version 1.7.1 and the 2026-08-01 date.docs/plugin-development/08-shared-sports-code.md (1)
58-63: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNarrow the fallback catch to missing core modules.
except ImportErroralso catches errors raised insidesrc.base_odds_manager, so a failed core import can load the bundled fallback and hide a broken core installation. Catch onlyModuleNotFoundErrorfor the core module path.Proposed safer example
try: from src.base_odds_manager import BaseOddsManager # core-shipped -except ImportError: +except ModuleNotFoundError as exc: + if exc.name not in {"src", "src.base_odds_manager"}: + raise from base_odds_manager import BaseOddsManager # bundled fallback🤖 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 `@docs/plugin-development/08-shared-sports-code.md` around lines 58 - 63, Update the documented import fallback around BaseOddsManager so it catches only ModuleNotFoundError from the core module path, while allowing other ImportError exceptions raised inside src.base_odds_manager to propagate instead of loading the bundled fallback.plugins/soccer-scoreboard/manager.py (1)
1335-1355: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMove favorite-team diagnostics off the synchronous update path.
update()calls_check_favorite_teams()before collecting and starting manager update threads. For a league with favorites, the diagnostic makes two blockingrequests.get(..., timeout=15)calls. A slow ESPN endpoint can delay updates by up to 30 seconds per league after an initial empty check, and_fetch_league_teams()/_fetch_season_start()bypassself.cache_manageras required by the plugin fetch rules. Run this diagnostic through the shared background service or another bounded async task, and cache the responses withself.cache_manager.🤖 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/manager.py` around lines 1335 - 1355, Move the _check_favorite_teams() diagnostic out of update()’s synchronous path and dispatch it through the shared background service or another bounded asynchronous task so manager updates are not delayed. Update _fetch_league_teams() and _fetch_season_start() to reuse self.cache_manager for their responses while preserving the existing once-per-league behavior and failure handling.Source: Coding guidelines
🧹 Nitpick comments (1)
plugins/clock-simple/manager.py (1)
134-135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNarrow the font and measurement exception handlers.
The fallback behavior is intentional, but
except Exceptionalso catches programming errors and display API failures. The clock can then silently use a default font or fallback centering. Catch only the expected font and measurement failures.Based on static analysis, Ruff reported blind exception handlers at Lines [134] and [253].
Also applies to: 253-257
🤖 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/clock-simple/manager.py` around lines 134 - 135, Replace the broad Exception handlers around the font-loading logic and measurement/centering fallback with only the specific expected font and measurement-related exception types. Update the handlers near the font load warning and the corresponding measurement fallback so programming errors and display API failures propagate instead of silently triggering defaults.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.
Outside diff comments:
In `@docs/plugin-development/08-shared-sports-code.md`:
- Around line 58-63: Update the documented import fallback around
BaseOddsManager so it catches only ModuleNotFoundError from the core module
path, while allowing other ImportError exceptions raised inside
src.base_odds_manager to propagate instead of loading the bundled fallback.
In `@plugins/f1-scoreboard/manifest.json`:
- Around line 32-37: Synchronize release metadata for F1 version 1.7.1: in
plugins/f1-scoreboard/manifest.json lines 32-37, keep the manifest timestamp at
2026-08-01; in plugins.json line 218, regenerate the F1 catalog entry so
latest_version and last_updated match version 1.7.1 and the 2026-08-01 date.
In `@plugins/soccer-scoreboard/manager.py`:
- Around line 1335-1355: Move the _check_favorite_teams() diagnostic out of
update()’s synchronous path and dispatch it through the shared background
service or another bounded asynchronous task so manager updates are not delayed.
Update _fetch_league_teams() and _fetch_season_start() to reuse
self.cache_manager for their responses while preserving the existing
once-per-league behavior and failure handling.
---
Nitpick comments:
In `@plugins/clock-simple/manager.py`:
- Around line 134-135: Replace the broad Exception handlers around the
font-loading logic and measurement/centering fallback with only the specific
expected font and measurement-related exception types. Update the handlers near
the font load warning and the corresponding measurement fallback so programming
errors and display API failures propagate instead of silently triggering
defaults.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 99962145-4833-4efa-9f8d-da33397f6ed1
📒 Files selected for processing (27)
docs/plugin-development/08-shared-sports-code.mdplugins.jsonplugins/afl-scoreboard/manager.pyplugins/afl-scoreboard/manifest.jsonplugins/afl-scoreboard/sports.pyplugins/baseball-scoreboard/manifest.jsonplugins/baseball-scoreboard/sports.pyplugins/basketball-scoreboard/manager.pyplugins/basketball-scoreboard/manifest.jsonplugins/basketball-scoreboard/sports.pyplugins/clock-simple/manager.pyplugins/clock-simple/manifest.jsonplugins/f1-scoreboard/manifest.jsonplugins/football-scoreboard/manifest.jsonplugins/football-scoreboard/sports.pyplugins/hockey-scoreboard/manifest.jsonplugins/hockey-scoreboard/sports.pyplugins/lacrosse-scoreboard/manifest.jsonplugins/lacrosse-scoreboard/sports.pyplugins/nrl-scoreboard/manager.pyplugins/nrl-scoreboard/manifest.jsonplugins/nrl-scoreboard/sports.pyplugins/soccer-scoreboard/manager.pyplugins/soccer-scoreboard/manifest.jsonplugins/soccer-scoreboard/sports.pyplugins/tide-display/manager.pyplugins/ufc-scoreboard/manifest.json
🚧 Files skipped from review as they are similar to previous changes (15)
- plugins/lacrosse-scoreboard/manifest.json
- plugins/football-scoreboard/sports.py
- plugins/baseball-scoreboard/manifest.json
- plugins/soccer-scoreboard/sports.py
- plugins/basketball-scoreboard/manager.py
- plugins/baseball-scoreboard/sports.py
- plugins/soccer-scoreboard/manifest.json
- plugins/basketball-scoreboard/sports.py
- plugins/afl-scoreboard/sports.py
- plugins/afl-scoreboard/manager.py
- plugins/hockey-scoreboard/sports.py
- plugins/nrl-scoreboard/manager.py
- plugins/nrl-scoreboard/sports.py
- plugins/tide-display/manager.py
- plugins/lacrosse-scoreboard/sports.py
Review follow-ups on #239. f1-scoreboard's manifest disagreed with itself: last_updated said 2026-07-28 while its newest versions[] entry for the same 1.7.1 said 2026-08-01. update_registry.py mirrors last_updated into the catalog, so the stale date propagated there too. Set it from the release entry and regenerated plugins.json. The 08-shared-sports-code doc still showed `except ImportError` for the base_odds_manager guard, but the code was narrowed to ModuleNotFoundError with a name check in an earlier round -- so the doc was teaching the pattern its own examples no longer use. A bare ImportError also swallows failures raised *inside* a core module that is present, silently loading the bundled copy and hiding a broken install. clock-simple's font loader caught bare Exception; narrowed to OSError, which is what FreeType raises for a missing, unreadable or malformed face. All three committed goldens pass unchanged. Left alone: - The measurement fallback in clock-simple stays broad on purpose. It runs on the render path and deliberately degrades to draw_text's own centring; letting a measurement hiccup propagate would blank a clock. - soccer-scoreboard's _check_favorite_teams blocking update() is real, but it arrived in #233 (49b0fe2) from main and is not this PR's code. - Five other manifests carry the same date drift from earlier PRs; out of scope here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
Owner
Author
|
Addressed in Taken:
Not taken:
Verified: module-collision check clean across 42 plugins, clock-simple harness green at all 8 sizes with goldens intact. Generated by Claude Code |
Owner
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
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 (4)
plugins/clock-simple/manager.py (2)
463-475: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClamp the standard time position for large custom fonts.
The standard branch draws
self.current_timewithout anxcoordinate at Line 459. Iftime_fontis wider than the panel,draw_text()auto-centers it at a negative x coordinate. The first and last glyphs are then clipped. Measuretime_widthbefore drawing, computemax(0, (width - time_width) // 2), and pass that value asx.As per coding guidelines, every plugin must render correctly at 64×32, 128×32, 128×64, and 256×32. Based on learnings, omitted x coordinates are auto-centered without clamping.
🤖 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/clock-simple/manager.py` around lines 463 - 475, Update the standard time-rendering branch around the existing self.current_time draw call to measure the rendered width using the active time font, compute a clamped horizontal position with max(0, (width - time_width) // 2), and pass it as x to draw_text. Preserve the current rendering behavior when the text fits while preventing negative positioning for oversized custom fonts.Sources: Coding guidelines, Learnings
395-397: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftKeep both date rows inside the panel.
font_heightnow usesdate_font, but the row origins still use fixed bottom offsets. With a taller custom font, the adjustment at Lines 511-516 can setdate_ytoheight - 1, which clips the glyph below the panel. Compute row positions from the measured font bounds, or reflow or omit a row when both rows cannot fit.As per coding guidelines, every plugin must render correctly at 64×32, 128×32, 128×64, and 256×32.
🤖 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/clock-simple/manager.py` around lines 395 - 397, Update the date-row layout in the manager’s rendering method to use the measured font bounds from get_font_height rather than fixed bottom offsets, ensuring both date rows remain fully within the panel. Adjust the logic around date_y and the Lines 511-516 overflow correction to reflow or omit a row when the available height cannot fit both, while preserving correct rendering at all required display sizes.Source: Coding guidelines
plugins/soccer-scoreboard/manager.py (2)
237-238: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPass
global_configfrom config reloads.
on_config_changerebuilds managers and reinitializesself._scroll_managerin__init__, butplugins/soccer-scoreboard/manager.py:1269does not refresh it after the super call. A missing coreself.global_configalready passes{}after line 238, and a reload will not propagate later changes totarget_fps/scroll_target_fps. Reload with the plugin config’sglobalsection or requireBasePluginto populateself.global_config.🤖 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/manager.py` around lines 237 - 238, Update on_config_change and the manager reinitialization flow so the reloaded plugin configuration’s global section is propagated into self.global_config before or during the super call, ensuring __init__ passes the refreshed values through its global_config argument to _scroll_manager; preserve the existing {} fallback only when no global configuration is available.
1478-1504: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winUse the shared cache for favorite-team diagnostics.
_fetch_league_teams()and_fetch_season_start()callrequests.get()directly at Lines 1484 and 1504.update()invokes these diagnostics at Line 1542, and configuration reloads clear_favorites_checked. Each reload can therefore make two blocking requests for every enabled league with favorites. Store both payloads inself.cache_managerwith plugin-scoped keys, expiry, and error handling.As per coding guidelines, network-fetched data in
plugins/**/*.pymust useself.cache_manager, and cache keys must include the plugin ID.Also applies to: 1542-1542
🤖 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/manager.py` around lines 1478 - 1504, The diagnostic methods _fetch_league_teams and _fetch_season_start must stop calling requests.get directly. Route both network payloads through self.cache_manager using plugin-scoped keys that include the plugin ID, configured expiry, and the cache manager’s established error-handling pattern, while preserving their existing parsing and return behavior. Ensure update’s diagnostic calls reuse the cached payloads across configuration reloads.Source: Coding guidelines
🧹 Nitpick comments (2)
plugins/clock-simple/manager.py (1)
257-261: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNarrow the measurement exception handler.
Line 257 catches every
Exceptionand returnsNone. This hides unexpecteddisplay_manager.get_text_width()defects. Keep the fallback for documented measurement failures, but catch the expected exception types and log unexpected failures. Confirm theget_text_width()exception contract before selecting the exception tuple.🤖 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/clock-simple/manager.py` around lines 257 - 261, In the measurement helper surrounding display_manager.get_text_width(), replace the broad Exception handler with the documented exception types from get_text_width’s contract, preserving the None fallback for those failures. Add logging for unexpected exceptions before allowing them to propagate, and verify the contract before choosing the exception tuple.Source: Linters/SAST tools
plugins/soccer-scoreboard/manager.py (1)
1353-1358: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNarrow the diagnostic exception handlers.
The handlers at Lines 1355 and 1388 catch every
Exceptionand only log at debug level. This hides programming errors in the fetch or parsing logic. Catch expected request, decoding, and schema failures explicitly. Log unexpected failures at warning or error level.Ruff reports both blind catches as BLE001.
Also applies to: 1386-1391
🤖 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/manager.py` around lines 1353 - 1358, Replace the broad Exception handlers surrounding _fetch_league_teams and the corresponding parsing/fetch operation with explicit handling for expected request, decoding, and schema-related exceptions. Keep expected failures at debug logging, but add a warning or error path that records and propagates unexpected failures instead of silently returning; resolve both BLE001 violations.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.
Outside diff comments:
In `@plugins/clock-simple/manager.py`:
- Around line 463-475: Update the standard time-rendering branch around the
existing self.current_time draw call to measure the rendered width using the
active time font, compute a clamped horizontal position with max(0, (width -
time_width) // 2), and pass it as x to draw_text. Preserve the current rendering
behavior when the text fits while preventing negative positioning for oversized
custom fonts.
- Around line 395-397: Update the date-row layout in the manager’s rendering
method to use the measured font bounds from get_font_height rather than fixed
bottom offsets, ensuring both date rows remain fully within the panel. Adjust
the logic around date_y and the Lines 511-516 overflow correction to reflow or
omit a row when the available height cannot fit both, while preserving correct
rendering at all required display sizes.
In `@plugins/soccer-scoreboard/manager.py`:
- Around line 237-238: Update on_config_change and the manager reinitialization
flow so the reloaded plugin configuration’s global section is propagated into
self.global_config before or during the super call, ensuring __init__ passes the
refreshed values through its global_config argument to _scroll_manager; preserve
the existing {} fallback only when no global configuration is available.
- Around line 1478-1504: The diagnostic methods _fetch_league_teams and
_fetch_season_start must stop calling requests.get directly. Route both network
payloads through self.cache_manager using plugin-scoped keys that include the
plugin ID, configured expiry, and the cache manager’s established error-handling
pattern, while preserving their existing parsing and return behavior. Ensure
update’s diagnostic calls reuse the cached payloads across configuration
reloads.
---
Nitpick comments:
In `@plugins/clock-simple/manager.py`:
- Around line 257-261: In the measurement helper surrounding
display_manager.get_text_width(), replace the broad Exception handler with the
documented exception types from get_text_width’s contract, preserving the None
fallback for those failures. Add logging for unexpected exceptions before
allowing them to propagate, and verify the contract before choosing the
exception tuple.
In `@plugins/soccer-scoreboard/manager.py`:
- Around line 1353-1358: Replace the broad Exception handlers surrounding
_fetch_league_teams and the corresponding parsing/fetch operation with explicit
handling for expected request, decoding, and schema-related exceptions. Keep
expected failures at debug logging, but add a warning or error path that records
and propagates unexpected failures instead of silently returning; resolve both
BLE001 violations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e74746a8-bcf4-415d-8ccb-9af68a2c4ff2
📒 Files selected for processing (27)
docs/plugin-development/08-shared-sports-code.mdplugins.jsonplugins/afl-scoreboard/manager.pyplugins/afl-scoreboard/manifest.jsonplugins/afl-scoreboard/sports.pyplugins/baseball-scoreboard/manifest.jsonplugins/baseball-scoreboard/sports.pyplugins/basketball-scoreboard/manager.pyplugins/basketball-scoreboard/manifest.jsonplugins/basketball-scoreboard/sports.pyplugins/clock-simple/manager.pyplugins/clock-simple/manifest.jsonplugins/f1-scoreboard/manifest.jsonplugins/football-scoreboard/manifest.jsonplugins/football-scoreboard/sports.pyplugins/hockey-scoreboard/manifest.jsonplugins/hockey-scoreboard/sports.pyplugins/lacrosse-scoreboard/manifest.jsonplugins/lacrosse-scoreboard/sports.pyplugins/nrl-scoreboard/manager.pyplugins/nrl-scoreboard/manifest.jsonplugins/nrl-scoreboard/sports.pyplugins/soccer-scoreboard/manager.pyplugins/soccer-scoreboard/manifest.jsonplugins/soccer-scoreboard/sports.pyplugins/tide-display/manager.pyplugins/ufc-scoreboard/manifest.json
🚧 Files skipped from review as they are similar to previous changes (25)
- plugins/hockey-scoreboard/manifest.json
- plugins/lacrosse-scoreboard/manifest.json
- plugins/basketball-scoreboard/manifest.json
- plugins/ufc-scoreboard/manifest.json
- plugins/clock-simple/manifest.json
- plugins/soccer-scoreboard/manifest.json
- plugins/f1-scoreboard/manifest.json
- plugins/football-scoreboard/manifest.json
- plugins/afl-scoreboard/manifest.json
- plugins/afl-scoreboard/manager.py
- plugins/lacrosse-scoreboard/sports.py
- plugins/nrl-scoreboard/manifest.json
- docs/plugin-development/08-shared-sports-code.md
- plugins/hockey-scoreboard/sports.py
- plugins/nrl-scoreboard/sports.py
- plugins/football-scoreboard/sports.py
- plugins/baseball-scoreboard/sports.py
- plugins/basketball-scoreboard/manager.py
- plugins/baseball-scoreboard/manifest.json
- plugins/tide-display/manager.py
- plugins/soccer-scoreboard/sports.py
- plugins/afl-scoreboard/sports.py
- plugins/basketball-scoreboard/sports.py
- plugins/nrl-scoreboard/manager.py
- plugins.json
ChuckBuilds
pushed a commit
that referenced
this pull request
Aug 2, 2026
Resolved version-mechanics conflicts: manifests and plugins.json taken from main (versions corrected in the follow-up commit); the two scoreboard CHANGELOGs keep main's history with the timezone-fix note re-headed to the new patch version. The *_timezone.py fix content applied cleanly.
ChuckBuilds
pushed a commit
that referenced
this pull request
Aug 2, 2026
Corrects the stale version bumps from #237's original commits, which targeted numbers main has since passed (via #234/#239) — 4 were collisions and 2 were downgrades. Each is now one patch above main's current version: baseball 1.20.2->1.20.3, football 2.9.2->2.9.3, hockey 1.5.2->1.5.3, basketball 1.8.2->1.8.3, soccer 2.5.1->2.5.2, ufc 1.3.1->1.3.2, lacrosse 1.5.1->1.5.2, nrl 1.1.1->1.1.2, afl 1.1.1->1.1.2, f1 1.7.1->1.7.2 Patch, per CONTRIBUTING.md: a bug fix with no schema additions. plugins.json regenerated; verified every version is strictly above main and the registry matches all ten manifests. The 10 test_timezone_resolution.py suites pass (17 each). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
ChuckBuilds
added a commit
that referenced
this pull request
Aug 2, 2026
…igs (#237) * fix(sports): auto-heal the stale "timezone": "UTC" left in saved configs Users who updated to the previous release still saw UTC start times until they hand-edited config.json, because the pre-fix write-back bug had already persisted "timezone": "UTC" into their saved plugin config, where an explicit plugin-level value outranks everything else. That stale value is now detected and ignored automatically whenever the global or system timezone disagrees, with a warning naming the source and zone it used instead. No config edit, and no config *write* either -- this is a runtime interpretation, so the plugin never mutates the dict the core owns (that mutation is what created the mess in the first place). The heuristic is scoped to the two plugins that could actually produce the artifact. Only baseball-scoreboard and football-scoreboard ever wrote a timezone back; in the other eight a plugin-level "UTC" can only have come from the user, so it is honored verbatim. _HAD_WRITEBACK_BUG records this per plugin and their tests assert the opposite behaviors. Etc/UTC is the unambiguous opt-in for genuinely wanting UTC -- a spelling the old bug could never have written, so it is always honored. Documented in the schema description and README of the two affected plugins, and named in the warning itself. Also fixes a real gap in the previous release, in all ten plugins: the core's ConfigManager.get_timezone() is self.config.get('timezone', 'UTC'), so it hands back "UTC" for a global config that has no timezone key at all. Resolution took that at face value and so could never reach the host system zone -- the very backstop that release added. It now reads the raw config dict and treats an absent key as absent. Verified against the shipping core (ChuckBuilds/LEDMatrix @ e2acbfb): a new _RealCoreConfigManager test double reproduces get_timezone()'s defaulting behavior. 17 tests per plugin, 170 total. Module-collision check passes; the nine pre-existing failures in these plugins are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QSnspNZceRCdpUtJh2Co6e * fix(sports): address review — narrow except, fix copied docstrings, README Review feedback from CodeRabbit on #237, all four valid: - _validated() caught bare Exception around pytz.timezone(). Now catches UnknownTimeZoneError for the expected case; anything else is logged with exc_info instead of being silently reclassified as "invalid timezone". Kept non-propagating rather than letting it escape: this runs in the render path, and a mislabelled zone beats taking the display down. - _WRITEBACK_FIXED_IN is inert in the eight plugins with _HAD_WRITEBACK_BUG = False, where its old comment ("release that fixed the write-back") also contradicted the docstring saying the plugin never wrote back. Reworded to say it is inert and why it stays -- removing it would leave the shared resolver body referencing an undefined name if the flag were ever flipped. - The nine non-baseball modules claimed start times "arrive from ESPN/MLB", copied from baseball; they are ESPN-only. F1 and UFC additionally described a scroll-mode game_renderer.py neither plugin ships, and called their sessions/fights "games". - baseball README troubleshooting still told users to clear a stuck "timezone": "UTC" by hand and said an explicit value "overrides everything else" -- both untrue as of 1.20.1, and directly contradicting the note added earlier in the same file. No version bump: these plugins are already bumped against main in this PR. 170 tests still pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QSnspNZceRCdpUtJh2Co6e * chore: bump the ten scoreboards for the timezone auto-heal, above main Corrects the stale version bumps from #237's original commits, which targeted numbers main has since passed (via #234/#239) — 4 were collisions and 2 were downgrades. Each is now one patch above main's current version: baseball 1.20.2->1.20.3, football 2.9.2->2.9.3, hockey 1.5.2->1.5.3, basketball 1.8.2->1.8.3, soccer 2.5.1->2.5.2, ufc 1.3.1->1.3.2, lacrosse 1.5.1->1.5.2, nrl 1.1.1->1.1.2, afl 1.1.1->1.1.2, f1 1.7.1->1.7.2 Patch, per CONTRIBUTING.md: a bug fix with no schema additions. plugins.json regenerated; verified every version is strictly above main and the registry matches all ten manifests. The 10 test_timezone_resolution.py suites pass (17 each). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --------- Co-authored-by: Claude <noreply@anthropic.com>
ChuckBuilds
pushed a commit
that referenced
this pull request
Aug 2, 2026
Minor bumps (new user-facing feature) for the seven plugins #235 actually changes, each above main's post-#237 version: afl 1.2.0, baseball 1.21.0, basketball 1.9.0, football 2.10.0, hockey 1.6.0, lacrosse 1.6.0, nrl 1.2.0 odds-ticker is intentionally NOT bumped: its only change in #235 was the NHL picker correction (UTA->UTAH, +Seattle) that came from #234 and is already in main, so it has no net change here. The original bump list also targeted numbers main has since passed via #234/#236/#237/#239; corrected. plugins.json regenerated; every changed plugin is strictly above main. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
ChuckBuilds
added a commit
that referenced
this pull request
Aug 3, 2026
* Fix wrong ESPN team codes in pickers and help text
Several plugins documented — or in one case offered in a picker — team
abbreviations that ESPN does not use, so copying them matched no team and
the plugin silently showed nothing.
odds-ticker's NHL picker was the only one where the user could not work
around it: it listed UTA (a retired code, labelled with the club's former
name "Utah Hockey Club" rather than "Utah Mammoth") and omitted the
Seattle Kraken entirely, so that team could not be selected at all. The
enum and labels are now generated from ESPN's team endpoint and match it
exactly at 32 teams.
The rest are description-only corrections to the favorite_teams examples:
basketball NBA GSW -> GS (Golden State Warriors)
basketball WNBA NYL -> NY (New York Liberty)
basketball WNBA LAS -> LA (Los Angeles Sparks)
basketball NCAAW UCONN -> CONN (UConn Huskies)
basketball NCAAW SCAR -> SC (South Carolina Gamecocks)
football NCAAFB BAMA -> ALA (Alabama Crimson Tide)
hockey NCAAWH WISC -> WIS (Wisconsin Badgers)
Each description now also says these are ESPN's codes and are not always
the ones you would guess, since that is the underlying trap.
Every code here was verified against
site.api.espn.com/apis/site/v2/sports/{sport}/{league}/teams?limit=1000.
The limit matters: without it the default page size truncates the NCAA
responses (362 of 755 teams) and makes valid codes look wrong.
Left alone deliberately: lacrosse-scoreboard's WISC/MINN/OSU and
BU/BC/MICH examples, and baseball-scoreboard's MiLB DUR/SWB/NOR, because
ESPN's lacrosse team endpoints return zero teams and the MiLB one 404s.
Unverifiable, so not guessed at.
No rendering code changed; the safety harness passes for all four plugins
at every size.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
* Explain an empty scoreboard instead of leaving the user guessing
Favorite teams are matched by exact ESPN abbreviation, so a code that is
not real matches no game and the plugin shows nothing — with no hint that
the code is the problem. Out of season, a perfectly correct code produces
the identical empty screen. The two were indistinguishable from the logs,
which is how a user ends up asking whether their config is broken when it
is only July.
Each of these plugins now says which case it is:
WARNING NFL favorite team 'GBP' is not a NFL team code. Closest match
is 'GB' (Green Bay Packers). Every code this league accepts is
listed at https://site.api.espn.com/.../nfl/teams?limit=1000.
INFO NFL favorite teams TB look correct, but the league has nothing
on until 06 August 2026. An empty display until then is
expected, not a configuration problem.
INFO NCAA Baseball favorite teams UGA look correct, but the season
has finished and the next one's fixtures are not published yet.
Suggestions rank word-initial matches first, because string similarity is
useless at three characters: 'MUN' scores identically against 'MAN' and
'SUN', so Manchester United and Sunderland tie and the answer is a coin
flip. Fragments are handled too ('BAMA' is inside 'Alabama' but
abbreviates nothing in it), and a code that only differs in case is told
so rather than guessed at.
Reading the schedule turned out to be the subtle part, and both traps are
real ESPN behaviour confirmed against live endpoints:
- An out-of-season league does not return an empty scoreboard. ESPN rolls
forward to the next day with fixtures, so in July the NHL endpoint
returns seven September games. Emptiness cannot be the signal.
- A *finished* season rolls nowhere and returns its last game instead,
months in the past — so dates must be filtered before the soonest one
means anything. Filtering on "later than now" then wrongly drops games
that started earlier today and reports a live slate as a dead season,
so the window is the last 24 hours.
Verified against every league these plugins cover: MLB, AFL, NRL and WNBA
correctly stay quiet; NFL, NCAA football, NHL, NBA and NCAA men's
basketball report their start dates; NCAA baseball and NCAA women's
hockey report finished seasons.
Safety, since this runs inside update():
- It runs on a daemon thread, so it never delays a frame.
- Once per league per process, re-armed only when the config changes.
- Every failure path is swallowed to a debug line. A plugin whose ESPN
endpoint returns no teams at all (college lacrosse) draws no conclusion
rather than calling a valid code wrong.
Each plugin ships its own copy of the module, since the loader gives
plugins no shared library to import from, under a plugin-unique name per
the module-collision rule. A test asserts the copies stay byte-identical
while they live in one checkout.
Tested: 30 unit tests; safety harness 24/24 PASS per plugin (168 renders,
zero failures); module-collision check clean. Validated end-to-end on real
hardware, where all four message paths appeared as intended with no errors.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
* chore: version the favorite-team diagnostics above current main
Minor bumps (new user-facing feature) for the seven plugins #235 actually
changes, each above main's post-#237 version:
afl 1.2.0, baseball 1.21.0, basketball 1.9.0, football 2.10.0,
hockey 1.6.0, lacrosse 1.6.0, nrl 1.2.0
odds-ticker is intentionally NOT bumped: its only change in #235 was the NHL
picker correction (UTA->UTAH, +Seattle) that came from #234 and is already in
main, so it has no net change here. The original bump list also targeted
numbers main has since passed via #234/#236/#237/#239; corrected. plugins.json
regenerated; every changed plugin is strictly above main.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
---------
Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
Implements five systemic improvements across the plugin fleet: per-render font-load caching (performance on the Pi's render hot path), shared-sports-code convergence groundwork (dead-code removal, guarded core imports, lineage documentation), deterministic safety-harness fixtures with golden images, standardized font/size/color customization for accessibility, and global
target_fpsadoption so the core smoothness setting applies consistently.All five workstreams are complete. (An earlier revision of this description listed the scoreboard font caching, odds-manager convergence, and
target_fpsthreading as pending follow-up commits — those have since landed, with the affected plugins version-bumped.)Type of change
Plugin(s) affected
All ten scoreboards (afl, baseball, basketball, f1, football, hockey, lacrosse, nrl, soccer, ufc) plus clock-simple, countdown, christmas-countdown, 7-segment-clock, news, mqtt-notifications, tide-display, youtube-stats, of-the-day, web-ui-info, on-air, text-display, static-image, ledmatrix-elections, ledmatrix-stocks, ledmatrix-leaderboard, nfl-draft, march-madness, odds-ticker.
Related issues
Complements ChuckBuilds/LEDMatrix#424 (
global_configonBasePlugin) and ChuckBuilds/LEDMatrix#425 (core-side font resolution +element_style). Follow-up issue #240 tracks the deferred cwd-relative font-path migration.Test plan
(
EMULATOR=true python3 run.py)(
scripts/dev_server.py)Details: every changed plugin was run through the core safety harness (
check_plugin.py) at all 8 default sizes — all PASS. clock-simple's, of-the-day's, countdown's and f1's pre-existing golden images pass unchanged, proving the customization and FPS wiring is byte-identical at defaults. All new golden images were generated twice and byte-compared before committing (determinism gate). The three fixtures-only plugins (7-segment-clock, christmas-countdown, static-image) are exempt from CI's change detection (test-path-only), so their harness runs were done locally — all PASS. The football and soccer celebration/adaptive pytest suites pass. Schemas validated as JSON; bumped manifests validated against the coremanifest_schema.json.Required for plugin changes
versioninplugins/<id>/manifest.json(the Plugin Store uses version comparison to ship updates —
forgetting this means users won't receive the change)
class_nameinmanifest.jsonmatches the actual class inmanager.pyexactly (case-sensitive, no spaces)entry_pointmatches the real file (or is omitted to usethe
manager.pydefault)README.mdif config keys changedconfig_schema.jsonis the source of truth for the web UIform — any new option is in the schema with a
default,description, and constraintsplugins.json)SUBMISSION checklist (new plugins only)
N/A — no new plugins.
Checklist
CONTRIBUTING.mdCONTRIBUTING.mdandCODE_OF_CONDUCT.mdNotes for reviewer
sport_key="ufc_scoreboard"whileSportsCorelooked upf"{sport_key}_scoreboard", i.e. the nonexistent"ufc_scoreboard_scoreboard", somode_configwas always empty and every UFC screen rendered blank. Separately, the new of-the-day fixture exposed a genuine overflow on 64px panels (title/underline past the right edge, body past the bottom), fixed with ellipsizing + clamping.truetype("assets/fonts/...")calls are deferred to Resolve or eliminate direct cwd-relative font loads in the scoreboard family #240 — they resolve fine under the supported systemd deployment, and a partial migration would leave the file inconsistent.update_registry.pyrefuses to lower a version, soplugins.jsonwas reset to main's state and regenerated.font_path,font_size, colors) already covers everything a block would add, and a second font knob would create ambiguous precedence.cozette.bdf, which doesn't ship in the core'sassets/fonts/— selecting it silently falls back. Removing the enum value would invalidate saved configs, so left as is; flagging for a core-side decision.scroll_speedunit conventions indocs/plugin-development/03-advanced-features.mdrather than renaming keys (renames would break saved configs).docs/plugin-development/08-shared-sports-code.mdmaps the three sports-code lineages and defines the convergence/sunset rules that the core-side phases build on.🤖 Generated with Claude Code
https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Chores