fix(sports): repair scroll mode broken by the core-scroll adoption - #251
Merged
Conversation
Scroll mode did not work on a 3.2.0 core in four of the eight plugins that
adopted the core orchestration. The restructure lifted the content methods
verbatim but left behind state their bodies read off `self`:
hockey, basketball, lacrosse separator-icon path constants stayed on the
legacy class. `_load_separator_icons` reads
them, and the core base calls it from
`__init__` -- so the scroll display could not
be constructed at all.
afl the game-renderer cache (`_game_renderer`,
`_game_renderer_card_width`) was seeded by the
legacy `__init__` and by nothing in the new
one. `prepare_scroll_content` opens by reading
it, so it raised on its first line.
afl is the one that had no symptom: the core base catches exceptions out of
`prepare_scroll_content`, so scroll mode just rendered nothing and logged.
Switch mode and the fallback path were never affected, which is why all 168
harness renders stay byte-for-byte identical across this change.
Why every gate missed it
------------------------
The safety harness renders the scoreboard screens, not scroll mode.
`test_core_fallback.py` checked that methods existed and that their *globals*
resolved -- but `self.NHL_SEPARATOR_ICON` is an attribute read, invisible to an
AST scan for Name loads. Both gates were green on code that could not draw.
So the fallback test now stops proving things about the source and builds the
object instead:
* construct ScrollDisplay and ScrollDisplayManager on the core path and on
the fallback path, and compare the separator icons they end up with. This
catches anything that makes `__init__` raise.
* assert the adopted class ends up with every instance attribute the bundled
one sets. Construction alone cannot catch the afl bug -- the object builds
fine and only fails later, when a lifted method reads what was never set.
Both checks were mutation-tested: removing each constant, and removing the
renderer-cache initialisation, makes the corresponding plugin's suite fail with
the attribute named.
Verification
------------
* 8/8 fallback suites pass; the 5 remaining plugin-test failures are the
known pre-existing stale-test tranche, none scroll-related.
* 168/168 harness renders byte-identical to before the fix.
* Scroll strips rendered from identical synthetic games are pixel-identical
between the core and fallback paths in all 8 plugins.
* Driven end to end against live data, core and fallback agree on frames,
cached game types, dynamic duration, separator icons and Vegas item counts
in all 6 plugins that had games available.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Contributor
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (21)
✨ 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 | 128 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (21)
📝 WalkthroughWalkthroughThe pull request fixes core scroll-display initialization and separator-icon compatibility across scoreboard plugins. It adds cross-path construction tests, broadens test failure handling, updates plugin versions, and records the releases. ChangesScroll display compatibility
Estimated code review effort: 3 (Moderate) | ~25 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 |
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.
Scroll mode does not work on a 3.2.0 core in four of the eight plugins that adopted the core scroll orchestration. The restructure lifted the content methods verbatim but left behind state their bodies read off
self.AttributeErrorin__init__— the scroll display could not be constructed at all_game_renderer/_game_renderer_card_widthprepare_scroll_contentraised on its first lineaflis the one with no visible symptom: the core base catches exceptions out ofprepare_scroll_content, so scroll mode simply rendered nothing.Switch mode and the pre-3.2.0 fallback path were never affected — which is why all 168 harness renders are byte-for-byte identical across this change.
Why every gate missed it
The safety harness renders the scoreboard screens, not scroll mode.
test_core_fallback.pychecked that methods existed and that their globals resolved — butself.NHL_SEPARATOR_ICONis an attribute read, invisible to an AST scan forNameloads. Both gates were green on code that could not draw.So the fallback test stops proving things about the source and builds the object instead:
ScrollDisplayandScrollDisplayManageron both paths and compare the separator icons they end up with — catches anything that makes__init__raise;Both checks were mutation-tested: removing each constant, and removing the renderer-cache initialisation, makes that plugin's suite fail with the attribute named.
Verification
Versions: hockey 1.7.1, basketball 1.10.1, lacrosse 1.7.1, afl 1.3.1 carry the fix; baseball 1.22.1, football 2.11.1, soccer 2.6.1, nrl 1.3.1 are test-only.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Summary by CodeRabbit
Bug Fixes
Tests
Chores