feat(sports): adopt the core scroll orchestration in the remaining five - #249
Conversation
Completes B5's scroll adoption for every plugin that shares the shape: hockey, afl, basketball, lacrosse and nrl, following the three pilots. hockey 691 -> 308 afl 694 -> 347 basketball 730 -> 334 lacrosse 672 -> 295 nrl 733 -> 368 ufc and f1 are deliberately NOT included. They are documented forks -- ufc has no ScrollDisplay class at all, just a single 405-line ScrollDisplayManager with _determine_fight_type, and f1's is a reduced rewrite. The docs call both out as genuine forks and they stay that way. Each follows its lineage's pilot: - afl and nrl mirror soccer: one `scroll_mode` config block rather than per-league keys, a 24px gap, min/max duration bounds of 30/300 (core's own max default is 600, so leaving it implicit would double the cap), and the extra plugin_dir constructor argument derived in the subclass so core's get_scroll_display() constructs them unchanged. - hockey, basketball and lacrosse mirror baseball/football: per-league key ladders, game cards pinned at 128px where core sizes them to the panel. Hockey's ladder is five keys (nhl, ncaa_mens, ncaam_hockey, ncaa_womens, ncaaw_hockey), not the three in core's docstring example -- that example is illustrative, not hockey's real ladder. The restructure was mechanised rather than done by hand, lifting content methods verbatim from each legacy source. Transcription error is exactly what the byte-identical gate exists to catch and there was no reason to risk it across five more plugins. Verified: - byte-for-byte identical harness output for all five: afl 24/24, basketball 24/24, hockey 16/16, lacrosse 16/16, nrl 24/24. `diff -r` of before/after is empty in every case. - test_core_fallback.py added to each, and every one checked that it bites: sabotaging the guard to match the unguarded recipe makes it exit 1, the healthy tree exits 0. - module-collision check clean across 42 plugins. - suites: afl 3/3, lacrosse 5/5, nrl 3/3 pass. hockey and basketball each have two failures -- test_hockey_emulator, test_recent_games, test_plugin_syntax, test_score_fix_verification -- all four confirmed failing identically on main, in a worktree, before this branch existed. Floors stay at 2.0.0 throughout: with a working fallback these plugins do not require 3.2.0, they prefer it. The floors rise at B6. 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: 16 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 (15)
📝 WalkthroughWalkthroughFive scoreboard plugins now use shared ChangesSports scroll orchestration migration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ScoreboardPlugin
participant SportsScrollCore
participant LegacyScrollImplementation
participant Display
ScoreboardPlugin->>SportsScrollCore: prepare scroll content
SportsScrollCore->>Display: advance and render frames
SportsScrollCore-->>ScoreboardPlugin: report scroll state
ScoreboardPlugin->>LegacyScrollImplementation: use fallback when core is unavailable
LegacyScrollImplementation->>Display: advance and render frames
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 | 11 high |
🟢 Metrics 710 complexity
Metric Results Complexity 710
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: 8
🧹 Nitpick comments (5)
plugins/afl-scoreboard/scroll_display_legacy.py (1)
568-568: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCorrect the stale annotation.
The annotation names
ScrollDisplay, which this module does not define; the class isLegacyScrollDisplay. Python does not evaluate this annotation, so there is no runtime error, but the name is misleading and Ruff reports F821.♻️ Proposed fix
- self._scroll_displays: Dict[str, ScrollDisplay] = {} + self._scroll_displays: Dict[str, 'LegacyScrollDisplay'] = {}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/afl-scoreboard/scroll_display_legacy.py` at line 568, Update the _scroll_displays annotation to use the locally defined LegacyScrollDisplay class instead of the stale ScrollDisplay name, preserving the existing string-keyed dictionary type.Source: Linters/SAST tools
plugins/lacrosse-scoreboard/scroll_display_legacy.py (1)
563-563: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCorrect the stale annotation.
The annotation names
ScrollDisplay; this module definesLegacyScrollDisplay. Python does not evaluate the annotation, so there is no runtime error, but Ruff reports F821 and the name misleads readers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/lacrosse-scoreboard/scroll_display_legacy.py` at line 563, Update the _scroll_displays annotation to reference the locally defined LegacyScrollDisplay type instead of the undefined ScrollDisplay name, preserving the existing dictionary key type and initialization.Source: Linters/SAST tools
plugins/nrl-scoreboard/scroll_display_legacy.py (1)
607-607: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCorrect the stale annotation.
The annotation names
ScrollDisplay; this module definesLegacyScrollDisplay. Python does not evaluate the annotation, so there is no runtime error, but Ruff reports F821.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/nrl-scoreboard/scroll_display_legacy.py` at line 607, Update the _scroll_displays annotation to reference the module’s LegacyScrollDisplay class instead of the undefined ScrollDisplay symbol, resolving Ruff F821 while preserving the existing dictionary type and behavior.Source: Linters/SAST tools
plugins/nrl-scoreboard/scroll_display.py (2)
223-231: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache the
GameRendereras the AFL module does.This builds a new
GameRendereron everyprepare_scroll_contentcall, which reloads fonts from disk each time.plugins/afl-scoreboard/scroll_display.pylines 198-210 caches the renderer and rebuilds it only whengame_card_widthchanges, and its comment states the reason. Apply the same caching here for consistency across the migrated plugins.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/nrl-scoreboard/scroll_display.py` around lines 223 - 231, The prepare_scroll_content renderer setup currently recreates GameRenderer on every call, causing repeated font loads. Add persistent GameRenderer caching to the scroll display, reusing the cached instance while game_card_width is unchanged and rebuilding it only when that width changes, following the AFL module’s pattern and preserving the existing constructor arguments.
88-120: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueUse the NRL league key for separator loading.
separator_filesmaps 30 soccer league keys before falling back to"eng.1"on missing leagues, but this plugin only uses the NRL slug"3". With noassets/separatorsfiles shipped, that branch will always skip the separator; map"3"and load an NRL separator, or disable/remove the separator path for this single-league plugin.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/nrl-scoreboard/scroll_display.py` around lines 88 - 120, The separator_files dictionary maps 30 soccer league keys but does not include an entry for the NRL league key "3" that this plugin uses, causing the separator to always fail or be skipped. Either add a mapping for "3" to an appropriate NRL separator filename in the separator_files dictionary, or remove the entire separator loading logic from this NRL-focused plugin if separator assets are not available.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/afl-scoreboard/scroll_display.py`:
- Around line 202-209: GameRenderer is instantiated in prepare_scroll_content
but is not imported at module scope in three core-backed scoreboard plugins,
causing NameError at runtime. Add the import statement from game_renderer import
GameRenderer at the top-level module imports (near the existing logging, os,
time, Path, and typing imports) in all three affected files:
plugins/afl-scoreboard/scroll_display.py,
plugins/hockey-scoreboard/scroll_display.py, and
plugins/nrl-scoreboard/scroll_display.py. Keep the import at module scope rather
than deferring it inside a function, as top-level bare-name imports are required
for plugin entry-point loading.
- Around line 109-113: Define the missing LEAGUE_NAMES constant in
scroll_display.py, reusing the league-key mappings expected by
_load_separator_icons. Ensure all debug and error paths in _load_separator_icons
can resolve LEAGUE_NAMES, including when a separator icon is absent, so
ScrollDisplay construction does not raise NameError.
In `@plugins/basketball-scoreboard/scroll_display_legacy.py`:
- Line 606: Update the _scroll_displays attribute annotation in the legacy
manager of plugins/basketball-scoreboard/scroll_display_legacy.py at lines
606-606 and plugins/hockey-scoreboard/scroll_display_legacy.py at lines 582-582
to reference the renamed LegacyScrollDisplay class as a forward reference.
Search the AFL, NRL, and lacrosse legacy modules for the identical Dict[str,
ScrollDisplay] annotation and apply the same LegacyScrollDisplay annotation
where present.
In `@plugins/basketball-scoreboard/scroll_display.py`:
- Around line 21-27: Restore the lifted dependencies in
plugins/basketball-scoreboard/scroll_display.py#L21-L27 by adding the guarded
GameRenderer import and RESAMPLE_FILTER, then declare NBA_SEPARATOR_ICON,
WNBA_SEPARATOR_ICON, NCAA_SEPARATOR_ICON, and MARCH_MADNESS_SEPARATOR_ICON on
ScrollDisplay. Apply the equivalent fix in
plugins/lacrosse-scoreboard/scroll_display.py#L21-L27 with GameRenderer,
RESAMPLE_FILTER, NCAA_SEPARATOR_ICON, and NCAA_LACROSSE_SEPARATOR_ICON. Inspect
the AFL and NRL core-backed modules for the same missing definitions and restore
them if present.
In `@plugins/hockey-scoreboard/manifest.json`:
- Around line 57-62: Update the manifest version entry by renaming the
ledmatrix_min_version key to ledmatrix_min, preserving its existing 2.0.0 value
and the surrounding release metadata.
In `@plugins/hockey-scoreboard/scroll_display.py`:
- Around line 74-124: Update ScrollDisplay._load_separator_icons to use
separator-icon path constants and RESAMPLE_FILTER that are defined in this
module or on ScrollDisplay, rather than relying on LegacyScrollDisplay. Define
or reuse the project’s canonical values for NHL, NCAA men’s, NCAA women’s, and
generic NCAA icons, and ensure the resize filter is available before
_load_separator_icons runs during construction.
In `@plugins/lacrosse-scoreboard/CHANGELOG.md`:
- Line 8: Update the changelog’s safety-harness verification entry to match the
documented four supported panel sizes and two screens, reporting the correct
total of eight renders; do not claim eight panel sizes unless the additional
sizes are explicitly listed.
In `@plugins/nrl-scoreboard/test_core_fallback.py`:
- Around line 105-110: The hasattr checks for ScrollDisplay methods only verify
that attribute names exist, not that the methods actually work when called.
Replace the loop checking hasattr for the ScrollDisplay methods with actual
method invocations: construct a ScrollDisplay instance against a stub display
manager, then call _load_separator_icons() and prepare_scroll_content([],
'live', []) to ensure these methods execute without errors such as NameError
from undefined module-level names. Keep the hasattr check for
ScrollDisplayManager.prepare_and_display since it only needs to exist.
---
Nitpick comments:
In `@plugins/afl-scoreboard/scroll_display_legacy.py`:
- Line 568: Update the _scroll_displays annotation to use the locally defined
LegacyScrollDisplay class instead of the stale ScrollDisplay name, preserving
the existing string-keyed dictionary type.
In `@plugins/lacrosse-scoreboard/scroll_display_legacy.py`:
- Line 563: Update the _scroll_displays annotation to reference the locally
defined LegacyScrollDisplay type instead of the undefined ScrollDisplay name,
preserving the existing dictionary key type and initialization.
In `@plugins/nrl-scoreboard/scroll_display_legacy.py`:
- Line 607: Update the _scroll_displays annotation to reference the module’s
LegacyScrollDisplay class instead of the undefined ScrollDisplay symbol,
resolving Ruff F821 while preserving the existing dictionary type and behavior.
In `@plugins/nrl-scoreboard/scroll_display.py`:
- Around line 223-231: The prepare_scroll_content renderer setup currently
recreates GameRenderer on every call, causing repeated font loads. Add
persistent GameRenderer caching to the scroll display, reusing the cached
instance while game_card_width is unchanged and rebuilding it only when that
width changes, following the AFL module’s pattern and preserving the existing
constructor arguments.
- Around line 88-120: The separator_files dictionary maps 30 soccer league keys
but does not include an entry for the NRL league key "3" that this plugin uses,
causing the separator to always fail or be skipped. Either add a mapping for "3"
to an appropriate NRL separator filename in the separator_files dictionary, or
remove the entire separator loading logic from this NRL-focused plugin if
separator assets are not available.
🪄 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: 2f67cd09-7f7f-4b3e-995d-27ecd3d66ef3
📒 Files selected for processing (22)
plugins.jsonplugins/afl-scoreboard/manifest.jsonplugins/afl-scoreboard/scroll_display.pyplugins/afl-scoreboard/scroll_display_legacy.pyplugins/afl-scoreboard/test_core_fallback.pyplugins/basketball-scoreboard/manifest.jsonplugins/basketball-scoreboard/scroll_display.pyplugins/basketball-scoreboard/scroll_display_legacy.pyplugins/basketball-scoreboard/test_core_fallback.pyplugins/hockey-scoreboard/manifest.jsonplugins/hockey-scoreboard/scroll_display.pyplugins/hockey-scoreboard/scroll_display_legacy.pyplugins/hockey-scoreboard/test_core_fallback.pyplugins/lacrosse-scoreboard/CHANGELOG.mdplugins/lacrosse-scoreboard/manifest.jsonplugins/lacrosse-scoreboard/scroll_display.pyplugins/lacrosse-scoreboard/scroll_display_legacy.pyplugins/lacrosse-scoreboard/test_core_fallback.pyplugins/nrl-scoreboard/manifest.jsonplugins/nrl-scoreboard/scroll_display.pyplugins/nrl-scoreboard/scroll_display_legacy.pyplugins/nrl-scoreboard/test_core_fallback.py
| ### Changed | ||
| - **Scroll display now runs on the core's shared implementation.** Orchestration — scroll-helper configuration, frame pumping, completion, settings resolution, native `global_config['target_fps']` — moves to the core's `src.common.sports_scroll` (LEDMatrix 3.2.0). Only the sport-specific content half stays here. | ||
| - **Nothing changes on an older core.** The import is guarded: a core without that module falls back to `scroll_display_legacy.py` and the plugin behaves exactly as before. The minimum core version is unchanged at 2.0.0 — the plugin does not *require* 3.2.0, it prefers it. | ||
| - Verified byte-for-byte: all 16 safety-harness renders (8 panel sizes × 2 screens) are identical to 1.6.0. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Check the panel-size count in the changelog.
The line states 8 panel sizes × 2 screens = 16 renders. The documented supported matrix sizes are 64×32, 128×32, 128×64, and 256×32. State the count that matches the harness matrix, or name the extra sizes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/lacrosse-scoreboard/CHANGELOG.md` at line 8, Update the changelog’s
safety-harness verification entry to match the documented four supported panel
sizes and two screens, reporting the correct total of eight renders; do not
claim eight panel sizes unless the additional sizes are explicitly listed.
Source: Coding guidelines
There was a problem hiding this comment.
Skipping — the changelog matches the harness; the documented list is what is out of date.
Measured from the actual harness output for this plugin:
64x32 96x48 128x32 128x64 128x96 256x32 256x128 64x64
Eight sizes × 2 screens = 16 renders, which is what the changelog states.
The four sizes you are quoting (64×32, 128×32, 128×64, 256×32) come from CLAUDE.md, and the harness has since grown past them. That is a real inconsistency worth fixing — but in the docs, not in this changelog entry, and not in this PR.
There was a problem hiding this comment.
@ChuckBuilds, confirmed. The changelog correctly reports 8 panel sizes × 2 screens = 16 renders.
My finding relied on the outdated four-size list in CLAUDE.md. The documentation inconsistency is separate from this PR. I am withdrawing the comment.
🐇
✏️ Learnings added
Learnt from: ChuckBuilds
Repo: ChuckBuilds/ledmatrix-plugins PR: 249
File: plugins/lacrosse-scoreboard/CHANGELOG.md:8-8
Timestamp: 2026-08-04T20:05:21.303Z
Learning: For `plugins/lacrosse-scoreboard`, the safety harness currently validates eight panel sizes—64x32, 96x48, 128x32, 128x64, 128x96, 256x32, 256x128, and 64x64—across two screens, for 16 renders. The four-size list in `CLAUDE.md` is outdated.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
| # A fallback that loads but cannot draw is no fallback at all. | ||
| for method in ("prepare_scroll_content", "display_scroll_frame", | ||
| "is_scroll_complete", "get_dynamic_duration", | ||
| "_load_separator_icons"): | ||
| assert hasattr(mod.ScrollDisplay, method), f"fallback lost {method}()" | ||
| assert hasattr(mod.ScrollDisplayManager, "prepare_and_display") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
hasattr does not prove the fallback can draw.
The comment on line 105 states the intent: a fallback that loads but cannot draw is no fallback. The loop below checks only that the attribute names exist. It passes for a class whose methods raise NameError on the first call, because the missing names are resolved at call time, not at class-definition time.
That is not hypothetical for this PR. On the core path, ScrollDisplay._load_separator_icons in the AFL, hockey, and NRL modules references names that those modules do not define, and this suite still passes. Construct a ScrollDisplay against a stub display manager and call _load_separator_icons() and prepare_scroll_content([], 'live', []) so a missing module-level name fails the test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/nrl-scoreboard/test_core_fallback.py` around lines 105 - 110, The
hasattr checks for ScrollDisplay methods only verify that attribute names exist,
not that the methods actually work when called. Replace the loop checking
hasattr for the ScrollDisplay methods with actual method invocations: construct
a ScrollDisplay instance against a stub display manager, then call
_load_separator_icons() and prepare_scroll_content([], 'live', []) to ensure
these methods execute without errors such as NameError from undefined
module-level names. Keep the hasattr check for
ScrollDisplayManager.prepare_and_display since it only needs to exist.
There was a problem hiding this comment.
Agreed, and fixed — this one found a real bug.
hasattr proved only that a method name existed. The methods existed; what they reached for did not. The mechanised restructure lifted the content methods but left GameRenderer, ScrollHelper, RESAMPLE_FILTER, LEAGUE_NAMES and the separator-path constants behind in the legacy modules, so on a 3.2.0 core — the primary path — _load_separator_icons and prepare_scroll_content raised NameError. Scroll mode was broken in six of the eight adopted plugins.
Neither of my gates caught it: the safety harness renders the scoreboard screens rather than scroll mode, so "byte-for-byte identical" was true and said nothing about this code.
The test now resolves, for every method on both the display and manager classes, the globals it reads, and fails on any the module cannot supply — on both import paths. It walks the AST for Name loads rather than bytecode co_names (which mixes in attribute names, reading Image.Resampling.LANCZOS as a missing global) and excludes locals, arguments, imports and except-handler names. Reading the source rather than calling the method is deliberate: constructing a real display needs a display manager, fonts and assets, but an unresolvable global is a load-time fact.
Verified it bites: on the unfixed tree all five fail naming exactly the missing symbols. It then independently found the stale Dict[str, ScrollDisplay] annotations you flagged separately.
There was a problem hiding this comment.
@ChuckBuilds, thank you for the detailed confirmation.
The AST-based check covers the actual failure mode: unresolved module globals in methods that exist but fail when called. It also avoids the invalid bytecode interpretation of attribute names as globals.
The affected scroll mode issue was valid. Your fix and the expanded coverage address this review finding.
🐇 ✅
✏️ Learnings added
Learnt from: ChuckBuilds
Repo: ChuckBuilds/ledmatrix-plugins PR: 249
File: plugins/nrl-scoreboard/test_core_fallback.py:105-110
Timestamp: 2026-08-04T20:04:52.765Z
Learning: For the conditional core/legacy scroll-display migrations in `plugins/*-scoreboard/scroll_display.py`, method existence checks such as `hasattr` do not verify that the core-backed implementation can execute. Validate each relevant `ScrollDisplay` and `ScrollDisplayManager` method for unresolved module-level global names on both the core and legacy import paths. AST-based `Name`-load analysis is suitable when real display construction requires unavailable runtime dependencies such as a display manager, fonts, or assets; exclude locals, parameters, imports, and exception-handler bindings.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
CodeRabbit caught this on #249 and it is a real bug I shipped. The mechanised restructure lifted the content methods out of each legacy module but left the definitions they reach for behind: GameRenderer, ScrollHelper, RESAMPLE_FILTER, LEAGUE_NAMES and the separator-path constants. On a core that ships src.common.sports_scroll -- the primary path -- _load_separator_icons and prepare_scroll_content raise NameError. Scroll mode was broken in all five. Neither gate caught it, and that is the more important half: - The safety harness renders the scoreboard screens, not scroll mode, so "byte-for-byte identical" was true and said nothing about this code. - test_core_fallback.py checked class identity and hasattr. The methods existed; what they reached for did not. It never constructed anything. I ran a sabotage check on those tests and concluded they "bite" -- they bite for the one failure mode they model, and I generalised that into confidence they had not earned. So the test now resolves, for every method on the display class, the globals it reads, and fails on any the module cannot supply. It walks the AST for Name loads rather than bytecode co_names, which mixes in attribute names -- Image.Resampling.LANCZOS read as a missing global -- and excludes locals, arguments, imports and except-handler names. Checking the source rather than calling the method is deliberate: building a real display needs a display manager, fonts and assets, but an unresolvable global is a load-time fact. Verified the check catches the bug it was written for: on the unfixed tree all five fail naming exactly the missing symbols. After the fix all five pass, and the harness is still byte-for-byte identical to the pre-adoption baseline -- hockey 16/16, afl 24/24, basketball 24/24, lacrosse 16/16, nrl 24/24. Module-collision check clean across 42 plugins. soccer needs the same fix; baseball and football were hand-written and are already clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Same bug as the other five (#249): the restructure lifted the content methods but left GameRenderer and LEAGUE_NAMES behind in the legacy module, so prepare_scroll_content raised NameError on the core path -- the default path on a 3.2.0 core. Adds the resolvability check to this plugin's test_core_fallback.py, which is what found it. That check also needed a fix of its own: it resolved names against the module it imported rather than the module the class actually lives in, so on the fallback path -- where ScrollDisplay is LegacyScrollDisplay, whose globals are the legacy module's -- every plugin looked broken. Verified: all 24 harness renders still byte-for-byte identical to the pre-adoption baseline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Extends the resolvability check to the manager as well as the display, on both import paths. It only looked at the display class, which is how a manager referencing something its module cannot supply slipped through. The extension immediately found it, independently of the review: all five legacy managers still annotate `Dict[str, ScrollDisplay]`, a class that no longer exists under that name after the rename. Corrected. On Python 3.13 that annotation is not evaluated, so the reported NameError does not reproduce there -- measured, not assumed. But it names a class that is gone, this repo supports 3.10 through 3.13, and I cannot test the older three, so it is fixed rather than argued about. Not changed, with reasons: - lacrosse's changelog panel count is correct. Measured from the harness output: 8 panel sizes x 2 screens = 16 renders, which is what it says. - hockey's versions[0] floor already uses ledmatrix_min_version. That is the spelling the migration in #244 moves toward, and the core reads both. Verified: all five pass, and the harness is still byte-for-byte identical to the pre-adoption baselines -- hockey 16/16, afl 24/24, basketball 24/24, lacrosse 16/16, nrl 24/24. 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
…248) * feat(soccer-scoreboard): adopt the core scroll orchestration (2.6.0) Third B5 adoption, completing the pilot set: one plugin per lineage. Soccer is the newest lineage (soccer/afl/nrl) and was the most divergent of the three -- the docs predicted it and they were right. scroll_display.py goes 733 -> 378 lines. What made this one different, and how each was resolved: - Settings come from a single `scroll_mode` block, not per-league keys. Core supports exactly this: SCROLL_LEAGUE_KEYS = () and SCROLL_CONFIG_KEY = "scroll_mode". - This lineage's defaults differ from the shared ones -- a 24px gap rather than 48, min/max duration bounds, cards pinned at 128px. Carried explicitly in scroll_settings_defaults(). Note core's own max_duration default is 600 where this lineage uses 300, so leaving it implicit would have doubled the cap. - The constructor took plugin_dir as an extra positional argument that core's base does not have. It was only ever the directory this module lives in -- the legacy manager computed exactly that -- so the subclass derives it and keeps core's signature, which lets core's get_scroll_display() construct it unchanged. No manager override needed. Two pieces of dead code dropped rather than carried: - _get_scroll_speed, get_scroll_duration and has_content are called from nowhere, in this plugin or the core. - set_scroll_speed() was called twice, first with px/s and then with px/frame; only the second took effect. Core makes the one call that mattered. Guarded exactly as baseball and football: scroll_display.py selects between the core-backed subclass and scroll_display_legacy (frozen) at import, and the except clause matches only the core module's own absence. The floor stays at 2.0.0 -- with a working fallback the plugin does not require 3.2.0, it prefers it -- and rises at B6 when the fallback goes. Verified: - all 24 harness renders (8 sizes x 3 screens) byte-for-byte identical to 2.5.2: `diff -r` of the before/after directories is empty. That gate is what caught the settings differences above being load-bearing. - test_core_fallback.py added and checked that it bites: sabotaging the guard makes it exit 1, the healthy tree exits 0 - 9 of 9 suites pass - 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 * fix(soccer-scoreboard): the core-backed module could not draw Same bug as the other five (#249): the restructure lifted the content methods but left GameRenderer and LEAGUE_NAMES behind in the legacy module, so prepare_scroll_content raised NameError on the core path -- the default path on a 3.2.0 core. Adds the resolvability check to this plugin's test_core_fallback.py, which is what found it. That check also needed a fix of its own: it resolved names against the module it imported rather than the module the class actually lives in, so on the fallback path -- where ScrollDisplay is LegacyScrollDisplay, whose globals are the legacy module's -- every plugin looked broken. Verified: all 24 harness renders still byte-for-byte identical to the pre-adoption baseline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * test(soccer-scoreboard): cover the manager class, and fix the stale annotation The resolvability check only looked at the display class, so a manager referencing something its module cannot supply would still slip through -- which is exactly the shape of the next finding: the legacy managers still annotate `Dict[str, ScrollDisplay]`, a class that no longer exists under that name after the rename. On Python 3.13 that annotation is not evaluated, so it does not raise there, and the reported NameError does not reproduce. But the annotation is simply wrong, this repo supports 3.10 through 3.13, and I cannot test the older three. Corrected rather than argued about. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 --------- Co-authored-by: Claude <noreply@anthropic.com>
2 participants
Completes B5's scroll adoption for every plugin that shares the shape, after the three pilots (#245 baseball, #247 football, #248 soccer).
ufc and f1 are deliberately excluded
They're documented forks, and inspecting them confirmed it: ufc has no
ScrollDisplayclass at all — a single 405-lineScrollDisplayManagerwith_determine_fight_type— and f1's is a reduced rewrite. The docs call both out; they stay forked.So the adoptable set was four, not six as I'd previously said.
Each follows its lineage's pilot
scroll_modeblock rather than per-league keys, 24px gap, min/max duration 30/300, and the extraplugin_dirconstructor argument derived in the subclass so core'sget_scroll_display()constructs them unchanged.max_durationis the one to watch — core defaults to 600 where the soccer lineage uses 300, so leaving it implicit would silently double the cap. Carried explicitly in all three of that lineage.Hockey's ladder turned out to be five keys (
nhl,ncaa_mens,ncaam_hockey,ncaa_womens,ncaaw_hockey), not the three in core's docstring example — that example is illustrative, not hockey's real ladder.Mechanised, not hand-typed
The restructure was scripted, lifting content methods verbatim from each legacy source. Transcription error is exactly what the byte-identical gate exists to catch, and there was no reason to risk it across five more plugins.
Verified
diff -rof before/after is empty in every case.test_core_fallback.pyadded to each, and every one checked that it bites: sabotaging the guard to match the unguarded recipe makes it exit 1; the healthy tree exits 0.test_hockey_emulator,test_recent_games,test_plugin_syntax,test_score_fix_verification— all four confirmed failing identically onmainin a clean worktree, before this branch existed. They're in the pre-existing tranche catalogued in test: make plugin test results mean something #246.Floors stay at 2.0.0 throughout: with a working fallback these plugins don't require 3.2.0, they prefer it. They rise at B6.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Summary by CodeRabbit
New Features
Bug Fixes
Tests