feat(football-scoreboard): adopt the core scroll orchestration (2.11.0) - #247
Conversation
Second B5 adoption, and the second of the three lineages (baseball is the hockey/lacrosse/baseball/basketball/ufc family; football is its own). Orchestration -- scroll-helper configuration, frame pumping, completion, settings resolution, native global_config['target_fps'] -- now comes from the core's src.common.sports_scroll. Only the football-specific content half stays here: game cards and league separator icons. scroll_display.py goes 679 -> 310 lines. Guarded, exactly as baseball: scroll_display.py selects between the core-backed subclass and scroll_display_legacy (the previous implementation, frozen) at import. The except clause matches only the core module's own absence, so a failure raised inside a present-but-broken core surfaces rather than silently downgrading. The floor stays at 2.0.0. With a working fallback the plugin does not require 3.2.0, it prefers it, so nobody is cut off from updates; the floor rises at B6 when the fallback is deleted. versions[0] uses ledmatrix_min_version. Football needed less than baseball did: its __init__ sets only attributes the core base already provides, so no custom __init__. It does pin game_card_width at 128 where core sizes cards to the panel, so scroll_settings_defaults() overrides that -- the byte-identical gate depends on it. Content methods were lifted verbatim from the legacy source rather than retyped. Verified: - all 16 harness renders (8 sizes x 2 screens) byte-for-byte identical to 2.10.1: `diff -r` of the before/after output directories is empty - test_core_fallback.py added, asserting all three configurations this plugin can meet, and checked that it bites: sabotaging the guard to match the unguarded recipe makes it exit 1, the healthy tree exits 0 - 10 of 11 suites pass; the one failure is test_football_plugin.py, an interactive script whose skip guard is in the not-yet-merged #246 - module-collision check clean across 42 plugins Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
|
Warning Review limit reached
Next review available in: 19 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe football scoreboard now delegates scroll orchestration to shared sports-scroll classes when available. It includes a legacy implementation for older cores, adds fallback tests, and publishes version 2.11.0 metadata dated 2026-08-04. ChangesFootball scoreboard scrolling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ScrollDisplayModule
participant SportsScrollCore
participant LegacyScrollModule
participant ScrollDisplayManager
ScrollDisplayModule->>SportsScrollCore: import shared display and manager classes
alt Core module is available
SportsScrollCore-->>ScrollDisplayModule: provide shared classes
else Core module is absent
ScrollDisplayModule->>LegacyScrollModule: import fallback classes
LegacyScrollModule-->>ScrollDisplayModule: provide legacy classes
end
ScrollDisplayModule->>ScrollDisplayManager: assign football ScrollDisplay
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 1 high |
🟢 Metrics 84 complexity
Metric Results Complexity 84
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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
plugins/football-scoreboard/scroll_display_legacy.py (1)
1-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicated header line in the module docstring.
Line 8 repeats the original title after the legacy notice, so the docstring reads as two stacked headers.
✏️ Proposed edit
here, so this file is frozen: fix bugs in the core module, not here. It goes away at the B6 sunset, once cores without that module are gone. -Scroll Display Handler for Football Scoreboard Plugin Implements high-FPS horizontal scrolling of all matching games with league separator icons.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/football-scoreboard/scroll_display_legacy.py` around lines 1 - 19, Remove the duplicated “Scroll Display Handler for Football Scoreboard Plugin” title from the module docstring, leaving the legacy fallback notice and remaining descriptive content unchanged.plugins/football-scoreboard/test_core_fallback.py (1)
94-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInstantiate the fallback classes in this test.
The test checks only class-level attributes with
hasattr. It therefore passes even when the legacy classes cannot be constructed. The staleScrollDisplayannotation atscroll_display_legacy.pyline 555 raisesNameErrorinsideLegacyScrollDisplayManager.__init__, and this suite does not detect it. Construct the manager with a stub display manager and callget_scroll_display("live").💚 Proposed addition
assert hasattr(mod.ScrollDisplayManager, "prepare_and_display") + + # Attribute checks pass on classes that cannot be constructed. + class _Matrix: + width, height = 128, 32 + + class _DM: + matrix = _Matrix() + + manager = mod.ScrollDisplayManager(_DM(), {}) + assert manager.get_scroll_display("live") is not None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/football-scoreboard/test_core_fallback.py` around lines 94 - 110, Extend test_core_absent_falls_back_and_still_works to instantiate the fallback classes, not only inspect class attributes: create a stub display manager, construct LegacyScrollDisplayManager, and call get_scroll_display("live"). Assert the returned fallback display is usable, exposing construction-time annotation errors such as the stale ScrollDisplay reference.
🤖 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/football-scoreboard/scroll_display_legacy.py`:
- Line 555: Update the _scroll_displays annotation in
LegacyScrollDisplayManager.__init__ to reference the renamed LegacyScrollDisplay
class, ensuring manager instantiation works in the fallback path. Add or update
the fallback test to instantiate LegacyScrollDisplayManager and verify
initialization succeeds.
In `@plugins/football-scoreboard/scroll_display.py`:
- Around line 279-309: Add a get_all_vegas_content_items method to
ScrollDisplayManager (or SportsScrollDisplayManager) matching the core manager’s
Vegas-item collection behavior, so manager.get_vegas_content() works on
core-backed paths without AttributeError. Reuse the existing scroll-display data
and established collection semantics rather than changing get_dynamic_duration
or has_cached_content.
---
Nitpick comments:
In `@plugins/football-scoreboard/scroll_display_legacy.py`:
- Around line 1-19: Remove the duplicated “Scroll Display Handler for Football
Scoreboard Plugin” title from the module docstring, leaving the legacy fallback
notice and remaining descriptive content unchanged.
In `@plugins/football-scoreboard/test_core_fallback.py`:
- Around line 94-110: Extend test_core_absent_falls_back_and_still_works to
instantiate the fallback classes, not only inspect class attributes: create a
stub display manager, construct LegacyScrollDisplayManager, and call
get_scroll_display("live"). Assert the returned fallback display is usable,
exposing construction-time annotation errors such as the stale ScrollDisplay
reference.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a37cdef-83b0-4611-93d2-6fe4e2dd7edd
📒 Files selected for processing (6)
plugins.jsonplugins/football-scoreboard/CHANGELOG.mdplugins/football-scoreboard/manifest.jsonplugins/football-scoreboard/scroll_display.pyplugins/football-scoreboard/scroll_display_legacy.pyplugins/football-scoreboard/test_core_fallback.py
…otation Brings this plugin's fallback test up to the version that found the missing imports in the other six -- it resolves the globals each method reads, across both the display and manager classes and on both import paths, rather than checking that method names exist. This plugin's header was hand-written and is already clean; the point is that the old check could not have told us that. Also corrects the legacy manager's `Dict[str, ScrollDisplay]` annotation to the post-rename name. On Python 3.13 that annotation is not evaluated, so the reported NameError does not reproduce there, but it is wrong regardless and this repo supports 3.10 through 3.13. Not changed: get_all_vegas_content_items. It is not lost -- the core-backed manager inherits it from SportsScrollDisplayManager, and diffing the two shows the only differences are the type annotation, docstring, loop variable name and quote style. The logic is identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
2 participants
Second B5 adoption, and the second of the three lineages — baseball is the hockey/lacrosse/baseball/basketball/ufc family, football is its own. Same guarded pattern as #245.
scroll_display.pygoes 679 → 310 lines. Orchestration comes from the core'ssrc.common.sports_scroll; only the football-specific content half stays here.Guarded, as with baseball
The import selects between the core-backed subclass and
scroll_display_legacy(the previous implementation, frozen) — theexceptmatches only the core module's own absence, so a failure raised inside a present-but-broken core surfaces rather than silently downgrading.The floor stays at
2.0.0. With a working fallback the plugin doesn't require 3.2.0, it prefers it, so nobody is cut off from updates. It rises at B6 when the fallback is deleted.versions[0]usesledmatrix_min_version.Football needed less than baseball
Its
__init__sets only attributes the core base already provides, so no custom__init__— where baseball needed_game_rendererinitialised beforesuper().__init__().The one real difference: football pins
game_card_widthat 128 where core sizes cards to the panel, soscroll_settings_defaults()overrides it. The byte-identical gate depends on that.Verified
diff -rof before/after is empty.test_core_fallback.pyadded, asserting all three configurations this plugin can meet (core present / core absent with fallback / neither). And I checked it bites: sabotaging the guard to match the unguarded recipe makes it exit 1; the healthy tree exits 0.test_football_plugin.py, an interactive script whose skip guard is in the not-yet-merged test: make plugin test results mean something #246 — it will report SKIP once that lands.Remaining
Soccer is the third lineage (soccer/afl/nrl) and would complete the pilot set; then the other six.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Summary by CodeRabbit
New Features
Tests
Chores