feat(soccer-scoreboard): adopt the core scroll orchestration (2.6.0) - #248
Conversation
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
|
Warning Review limit reached
Next review available in: 21 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 (6)
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 | 2 high |
🟢 Metrics 92 complexity
Metric Results Complexity 92
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.
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
…nnotation 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
2 participants
Third B5 adoption — completes the pilot set, one plugin per lineage (baseball #245, football #247, soccer here).
Soccer is the newest lineage (soccer/afl/nrl) and the most divergent of the three. The docs predicted that; they were right.
scroll_display.pygoes 733 → 378 lines.What made this one different
scroll_modeblock, not per-league keysSCROLL_LEAGUE_KEYS = (),SCROLL_CONFIG_KEY = "scroll_mode"— core supports exactly thisscroll_settings_defaults()plugin_dirThe
max_durationone is worth calling out: core's default is 600 where this lineage uses 300, so leaving it implicit would have silently doubled the cap. The byte-identical gate is what forced me to get these right rather than assume.On
plugin_dir— it was only ever the directory the module lives in, which the legacy manager computed itself. Deriving it in the subclass keeps core'sget_scroll_display()working unchanged, so no manager override.Two pieces of dead code dropped
_get_scroll_speed,get_scroll_duration,has_content— called from nowhere, in this plugin or the core.set_scroll_speed()was called twice, first with px/s then with px/frame; only the second took effect. Core makes the one call that mattered.Verified
diff -rempty.test_core_fallback.pyadded, and checked that it bites: sabotage the guard → exit 1, healthy → exit 0.Pilot set summary
The floor stays at
2.0.0on all three — with a working fallback the plugins don't require 3.2.0. It rises at B6.Remaining: the other six (afl, basketball, lacrosse, nrl, ufc, and f1's reduced rewrite).
🤖 Generated with Claude Code
https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5